Skip to content

Commit

Permalink
feat: PR comments, increase coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
joanestebanr committed Nov 13, 2024
1 parent aac4ed5 commit e730cda
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 13 deletions.
22 changes: 11 additions & 11 deletions aggsender/aggsender.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,14 @@ func (a *AggSender) sendCertificates(ctx context.Context) {
select {
case epoch := <-chEpoch:
a.log.Infof("Epoch received: %s", epoch.String())
err := a.checkPendingCertificatesStatus(ctx)
if err == nil {
thereArePendingCerts := a.checkPendingCertificatesStatus(ctx)
if !thereArePendingCerts {
if _, err := a.sendCertificate(ctx); err != nil {
log.Error(err)
}
} else {
log.Infof("Skipping epoch %s because error: %w",
epoch.String(), err)
log.Infof("Skipping epoch %s because there are pending certificates",
epoch.String())
}
case <-ctx.Done():
a.log.Info("AggSender stopped")
Expand Down Expand Up @@ -488,13 +488,12 @@ func (a *AggSender) signCertificate(certificate *agglayer.Certificate) (*agglaye
// and updates in the storage if it changed on agglayer
// It returns:
// bool -> if there are pending certificates
// error -> if there was an error
func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) error {
func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) bool {
pendingCertificates, err := a.storage.GetCertificatesByStatus(nonSettledStatuses)
if err != nil {
err = fmt.Errorf("error getting pending certificates: %w", err)
a.log.Error(err)
return err
return true
}
logErrMsg := ""
a.log.Debugf("checkPendingCertificatesStatus num of pendingCertificates: %d", len(pendingCertificates))
Expand All @@ -504,7 +503,7 @@ func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) error {
err = fmt.Errorf("error getting certificate header of %d/%s from agglayer: %w",
certificate.Height, certificate.String(), err)
a.log.Error(err)
return err
return true
}
elapsedTime := time.Now().UTC().Sub(time.UnixMilli(certificate.CreatedAt))
a.log.Debugf("aggLayerClient.GetCertificateHeader status [%s] of certificate %s elapsed_time:%s",
Expand All @@ -522,7 +521,7 @@ func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) error {
if err := a.storage.UpdateCertificateStatus(ctx, *certificate); err != nil {
err = fmt.Errorf("error updating certificate %s status in storage: %w", certificateHeader.String(), err)
a.log.Error(err)
return err
return true
}
}
if slices.Contains(nonSettledStatuses, certificateHeader.Status) {

Check failure on line 527 in aggsender/aggsender.go

View workflow job for this annotation

GitHub Actions / lint

unnecessary leading newline (whitespace)
Expand All @@ -532,9 +531,10 @@ func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) error {
}
if logErrMsg != "" {
a.log.Infof(logErrMsg)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to Password
flows to a logging call.
return fmt.Errorf(logErrMsg)
return true

}

Check failure on line 536 in aggsender/aggsender.go

View workflow job for this annotation

GitHub Actions / lint

unnecessary trailing newline (whitespace)
return nil
return false
}

// shouldSendCertificate checks if a certificate should be sent at given time
Expand Down
21 changes: 19 additions & 2 deletions aggsender/aggsender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,23 @@ func TestGetBridgeExits(t *testing.T) {
}
}

func TestNewAggSender(t *testing.T) {
AggLayerMock := agglayer.NewAgglayerClientMock(t)
epochNotifierMock := mocks.NewEpochNotifier(t)

aggSender, err := New(
context.TODO(),
log.WithFields("test", "unittest"),
Config{},
AggLayerMock,
nil,
nil,
epochNotifierMock)
require.NoError(t, err)
require.NotNil(t, aggSender)

}

Check failure on line 298 in aggsender/aggsender_test.go

View workflow job for this annotation

GitHub Actions / lint

unnecessary trailing newline (whitespace)

//nolint:dupl
func TestGetImportedBridgeExits(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -860,8 +877,8 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) {
}

ctx := context.TODO()
err := aggSender.checkPendingCertificatesStatus(ctx)
require.Equal(t, tt.expectedError, err != nil)
thereArePendingCerts := aggSender.checkPendingCertificatesStatus(ctx)
require.Equal(t, tt.expectedError, thereArePendingCerts)
mockAggLayerClient.AssertExpectations(t)
mockStorage.AssertExpectations(t)
})
Expand Down

0 comments on commit e730cda

Please sign in to comment.