From 59ee8ddec288fb6aef5acafedcb49052fcbd125a Mon Sep 17 00:00:00 2001 From: Joan Esteban <129153821+joanestebanr@users.noreply.github.com> Date: Thu, 14 Nov 2024 10:22:54 +0100 Subject: [PATCH] feat: improve aggsender logs (#186) --- agglayer/mock_agglayer_client.go | 150 +++++++++++++++++++++++++++++- agglayer/types.go | 2 +- aggsender/aggsender.go | 43 +++++---- aggsender/aggsender_test.go | 97 +++++++++++++++---- aggsender/types/epoch_notifier.go | 3 + config/default.go | 2 +- test/Makefile | 2 +- 7 files changed, 256 insertions(+), 43 deletions(-) diff --git a/agglayer/mock_agglayer_client.go b/agglayer/mock_agglayer_client.go index 1b756713..b7f70ee8 100644 --- a/agglayer/mock_agglayer_client.go +++ b/agglayer/mock_agglayer_client.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.0. DO NOT EDIT. +// Code generated by mockery. DO NOT EDIT. package agglayer @@ -15,6 +15,14 @@ type AgglayerClientMock struct { mock.Mock } +type AgglayerClientMock_Expecter struct { + mock *mock.Mock +} + +func (_m *AgglayerClientMock) EXPECT() *AgglayerClientMock_Expecter { + return &AgglayerClientMock_Expecter{mock: &_m.Mock} +} + // GetCertificateHeader provides a mock function with given fields: certificateHash func (_m *AgglayerClientMock) GetCertificateHeader(certificateHash common.Hash) (*CertificateHeader, error) { ret := _m.Called(certificateHash) @@ -45,6 +53,34 @@ func (_m *AgglayerClientMock) GetCertificateHeader(certificateHash common.Hash) return r0, r1 } +// AgglayerClientMock_GetCertificateHeader_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetCertificateHeader' +type AgglayerClientMock_GetCertificateHeader_Call struct { + *mock.Call +} + +// GetCertificateHeader is a helper method to define mock.On call +// - certificateHash common.Hash +func (_e *AgglayerClientMock_Expecter) GetCertificateHeader(certificateHash interface{}) *AgglayerClientMock_GetCertificateHeader_Call { + return &AgglayerClientMock_GetCertificateHeader_Call{Call: _e.mock.On("GetCertificateHeader", certificateHash)} +} + +func (_c *AgglayerClientMock_GetCertificateHeader_Call) Run(run func(certificateHash common.Hash)) *AgglayerClientMock_GetCertificateHeader_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(common.Hash)) + }) + return _c +} + +func (_c *AgglayerClientMock_GetCertificateHeader_Call) Return(_a0 *CertificateHeader, _a1 error) *AgglayerClientMock_GetCertificateHeader_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *AgglayerClientMock_GetCertificateHeader_Call) RunAndReturn(run func(common.Hash) (*CertificateHeader, error)) *AgglayerClientMock_GetCertificateHeader_Call { + _c.Call.Return(run) + return _c +} + // GetEpochConfiguration provides a mock function with given fields: func (_m *AgglayerClientMock) GetEpochConfiguration() (*ClockConfiguration, error) { ret := _m.Called() @@ -75,6 +111,33 @@ func (_m *AgglayerClientMock) GetEpochConfiguration() (*ClockConfiguration, erro return r0, r1 } +// AgglayerClientMock_GetEpochConfiguration_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetEpochConfiguration' +type AgglayerClientMock_GetEpochConfiguration_Call struct { + *mock.Call +} + +// GetEpochConfiguration is a helper method to define mock.On call +func (_e *AgglayerClientMock_Expecter) GetEpochConfiguration() *AgglayerClientMock_GetEpochConfiguration_Call { + return &AgglayerClientMock_GetEpochConfiguration_Call{Call: _e.mock.On("GetEpochConfiguration")} +} + +func (_c *AgglayerClientMock_GetEpochConfiguration_Call) Run(run func()) *AgglayerClientMock_GetEpochConfiguration_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *AgglayerClientMock_GetEpochConfiguration_Call) Return(_a0 *ClockConfiguration, _a1 error) *AgglayerClientMock_GetEpochConfiguration_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *AgglayerClientMock_GetEpochConfiguration_Call) RunAndReturn(run func() (*ClockConfiguration, error)) *AgglayerClientMock_GetEpochConfiguration_Call { + _c.Call.Return(run) + return _c +} + // SendCertificate provides a mock function with given fields: certificate func (_m *AgglayerClientMock) SendCertificate(certificate *SignedCertificate) (common.Hash, error) { ret := _m.Called(certificate) @@ -105,6 +168,34 @@ func (_m *AgglayerClientMock) SendCertificate(certificate *SignedCertificate) (c return r0, r1 } +// AgglayerClientMock_SendCertificate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SendCertificate' +type AgglayerClientMock_SendCertificate_Call struct { + *mock.Call +} + +// SendCertificate is a helper method to define mock.On call +// - certificate *SignedCertificate +func (_e *AgglayerClientMock_Expecter) SendCertificate(certificate interface{}) *AgglayerClientMock_SendCertificate_Call { + return &AgglayerClientMock_SendCertificate_Call{Call: _e.mock.On("SendCertificate", certificate)} +} + +func (_c *AgglayerClientMock_SendCertificate_Call) Run(run func(certificate *SignedCertificate)) *AgglayerClientMock_SendCertificate_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(*SignedCertificate)) + }) + return _c +} + +func (_c *AgglayerClientMock_SendCertificate_Call) Return(_a0 common.Hash, _a1 error) *AgglayerClientMock_SendCertificate_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *AgglayerClientMock_SendCertificate_Call) RunAndReturn(run func(*SignedCertificate) (common.Hash, error)) *AgglayerClientMock_SendCertificate_Call { + _c.Call.Return(run) + return _c +} + // SendTx provides a mock function with given fields: signedTx func (_m *AgglayerClientMock) SendTx(signedTx SignedTx) (common.Hash, error) { ret := _m.Called(signedTx) @@ -135,6 +226,34 @@ func (_m *AgglayerClientMock) SendTx(signedTx SignedTx) (common.Hash, error) { return r0, r1 } +// AgglayerClientMock_SendTx_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SendTx' +type AgglayerClientMock_SendTx_Call struct { + *mock.Call +} + +// SendTx is a helper method to define mock.On call +// - signedTx SignedTx +func (_e *AgglayerClientMock_Expecter) SendTx(signedTx interface{}) *AgglayerClientMock_SendTx_Call { + return &AgglayerClientMock_SendTx_Call{Call: _e.mock.On("SendTx", signedTx)} +} + +func (_c *AgglayerClientMock_SendTx_Call) Run(run func(signedTx SignedTx)) *AgglayerClientMock_SendTx_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(SignedTx)) + }) + return _c +} + +func (_c *AgglayerClientMock_SendTx_Call) Return(_a0 common.Hash, _a1 error) *AgglayerClientMock_SendTx_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *AgglayerClientMock_SendTx_Call) RunAndReturn(run func(SignedTx) (common.Hash, error)) *AgglayerClientMock_SendTx_Call { + _c.Call.Return(run) + return _c +} + // WaitTxToBeMined provides a mock function with given fields: hash, ctx func (_m *AgglayerClientMock) WaitTxToBeMined(hash common.Hash, ctx context.Context) error { ret := _m.Called(hash, ctx) @@ -153,6 +272,35 @@ func (_m *AgglayerClientMock) WaitTxToBeMined(hash common.Hash, ctx context.Cont return r0 } +// AgglayerClientMock_WaitTxToBeMined_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WaitTxToBeMined' +type AgglayerClientMock_WaitTxToBeMined_Call struct { + *mock.Call +} + +// WaitTxToBeMined is a helper method to define mock.On call +// - hash common.Hash +// - ctx context.Context +func (_e *AgglayerClientMock_Expecter) WaitTxToBeMined(hash interface{}, ctx interface{}) *AgglayerClientMock_WaitTxToBeMined_Call { + return &AgglayerClientMock_WaitTxToBeMined_Call{Call: _e.mock.On("WaitTxToBeMined", hash, ctx)} +} + +func (_c *AgglayerClientMock_WaitTxToBeMined_Call) Run(run func(hash common.Hash, ctx context.Context)) *AgglayerClientMock_WaitTxToBeMined_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(common.Hash), args[1].(context.Context)) + }) + return _c +} + +func (_c *AgglayerClientMock_WaitTxToBeMined_Call) Return(_a0 error) *AgglayerClientMock_WaitTxToBeMined_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *AgglayerClientMock_WaitTxToBeMined_Call) RunAndReturn(run func(common.Hash, context.Context) error) *AgglayerClientMock_WaitTxToBeMined_Call { + _c.Call.Return(run) + return _c +} + // NewAgglayerClientMock creates a new instance of AgglayerClientMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewAgglayerClientMock(t interface { diff --git a/agglayer/types.go b/agglayer/types.go index b6a3198e..aece93f0 100644 --- a/agglayer/types.go +++ b/agglayer/types.go @@ -556,7 +556,7 @@ func (c CertificateHeader) String() string { errors = c.Error.String() } - return fmt.Sprintf("Height: %d, CertificateID: %s, NewLocalExitRoot: %s. Status: %s. Errors: %s", + return fmt.Sprintf("Height: %d, CertificateID: %s, NewLocalExitRoot: %s. Status: %s. Errors: [%s]", c.Height, c.CertificateID.String(), c.NewLocalExitRoot.String(), c.Status.String(), errors) } diff --git a/aggsender/aggsender.go b/aggsender/aggsender.go index dcbbc268..08730572 100644 --- a/aggsender/aggsender.go +++ b/aggsender/aggsender.go @@ -55,7 +55,7 @@ func New( cfg Config, aggLayerClient agglayer.AgglayerClientInterface, l1InfoTreeSyncer *l1infotreesync.L1InfoTreeSync, - l2Syncer *bridgesync.BridgeSync, + l2Syncer types.L2BridgeSyncer, epochNotifier types.EpochNotifier) (*AggSender, error) { storage, err := db.NewAggSenderSQLStorage(logger, cfg.StoragePath) if err != nil { @@ -93,14 +93,14 @@ func (a *AggSender) sendCertificates(ctx context.Context) { select { case epoch := <-chEpoch: a.log.Infof("Epoch received: %s", epoch.String()) - thereArePendingCerts, err := a.checkPendingCertificatesStatus(ctx) - if err == nil && !thereArePendingCerts { + thereArePendingCerts := a.checkPendingCertificatesStatus(ctx) + if !thereArePendingCerts { if _, err := a.sendCertificate(ctx); err != nil { log.Error(err) } } else { - log.Warnf("Skipping epoch %s because there are pending certificates %v or error: %w", - epoch.String(), thereArePendingCerts, err) + log.Infof("Skipping epoch %s because there are pending certificates", + epoch.String()) } case <-ctx.Done(): a.log.Info("AggSender stopped") @@ -177,7 +177,7 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif } a.saveCertificateToFile(signedCertificate) - a.log.Debugf("certificate ready to be send to AggLayer: %s", signedCertificate.String()) + a.log.Infof("certificate ready to be send to AggLayer: %s", signedCertificate.String()) certificateHash, err := a.aggLayerClient.SendCertificate(signedCertificate) if err != nil { @@ -488,15 +488,14 @@ 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) (bool, 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 true, err + return true } - thereArePendingCertificates := false + thereArePendingCerts := false a.log.Debugf("checkPendingCertificatesStatus num of pendingCertificates: %d", len(pendingCertificates)) for _, certificate := range pendingCertificates { certificateHeader, err := a.aggLayerClient.GetCertificateHeader(certificate.CertificateID) @@ -504,18 +503,17 @@ func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) (bool, e err = fmt.Errorf("error getting certificate header of %d/%s from agglayer: %w", certificate.Height, certificate.String(), err) a.log.Error(err) - return true, err + return true } - if slices.Contains(nonSettledStatuses, certificateHeader.Status) { - thereArePendingCertificates = true - } - a.log.Debugf("aggLayerClient.GetCertificateHeader status [%s] of certificate %s ", + elapsedTime := time.Now().UTC().Sub(time.UnixMilli(certificate.CreatedAt)) + a.log.Debugf("aggLayerClient.GetCertificateHeader status [%s] of certificate %s elapsed time:%s", certificateHeader.Status, - certificateHeader.String()) + certificateHeader.String(), + elapsedTime) if certificateHeader.Status != certificate.Status { - a.log.Infof("certificate %s changed status from [%s] to [%s]", - certificateHeader.String(), certificate.Status, certificateHeader.Status) + a.log.Infof("certificate %s changed status from [%s] to [%s] elapsed time: %s", + certificateHeader.String(), certificate.Status, certificateHeader.Status, elapsedTime) certificate.Status = certificateHeader.Status certificate.UpdatedAt = time.Now().UTC().UnixMilli() @@ -523,11 +521,16 @@ func (a *AggSender) checkPendingCertificatesStatus(ctx context.Context) (bool, e 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 true, err + return true } } + if slices.Contains(nonSettledStatuses, certificateHeader.Status) { + a.log.Infof("certificate %s is still pending, elapsed time:%s ", + certificateHeader.String(), elapsedTime) + thereArePendingCerts = true + } } - return thereArePendingCertificates, nil + return thereArePendingCerts } // shouldSendCertificate checks if a certificate should be sent at given time diff --git a/aggsender/aggsender_test.go b/aggsender/aggsender_test.go index 0d071e76..b9242bdf 100644 --- a/aggsender/aggsender_test.go +++ b/aggsender/aggsender_test.go @@ -280,6 +280,70 @@ func TestGetBridgeExits(t *testing.T) { } } +func TestAggSenderStart(t *testing.T) { + AggLayerMock := agglayer.NewAgglayerClientMock(t) + epochNotifierMock := mocks.NewEpochNotifier(t) + bridgeL2SyncerMock := mocks.NewL2BridgeSyncer(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + aggSender, err := New( + ctx, + log.WithFields("test", "unittest"), + Config{ + StoragePath: "file::memory:?cache=shared", + }, + AggLayerMock, + nil, + bridgeL2SyncerMock, + epochNotifierMock) + require.NoError(t, err) + require.NotNil(t, aggSender) + ch := make(chan aggsendertypes.EpochEvent) + epochNotifierMock.EXPECT().Subscribe("aggsender").Return(ch) + bridgeL2SyncerMock.EXPECT().GetLastProcessedBlock(mock.Anything).Return(uint64(0), nil) + + go aggSender.Start(ctx) + ch <- aggsendertypes.EpochEvent{ + Epoch: 1, + } + time.Sleep(200 * time.Millisecond) +} + +func TestAggSenderSendCertificates(t *testing.T) { + AggLayerMock := agglayer.NewAgglayerClientMock(t) + epochNotifierMock := mocks.NewEpochNotifier(t) + bridgeL2SyncerMock := mocks.NewL2BridgeSyncer(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + aggSender, err := New( + ctx, + log.WithFields("test", "unittest"), + Config{ + StoragePath: "file::memory:?cache=shared", + }, + AggLayerMock, + nil, + bridgeL2SyncerMock, + epochNotifierMock) + require.NoError(t, err) + require.NotNil(t, aggSender) + ch := make(chan aggsendertypes.EpochEvent, 2) + epochNotifierMock.EXPECT().Subscribe("aggsender").Return(ch) + err = aggSender.storage.SaveLastSentCertificate(ctx, aggsendertypes.CertificateInfo{ + Height: 1, + Status: agglayer.Pending, + }) + AggLayerMock.EXPECT().GetCertificateHeader(mock.Anything).Return(&agglayer.CertificateHeader{ + Status: agglayer.Pending, + }, nil) + require.NoError(t, err) + ch <- aggsendertypes.EpochEvent{ + Epoch: 1, + } + go aggSender.sendCertificates(ctx) + time.Sleep(200 * time.Millisecond) +} + //nolint:dupl func TestGetImportedBridgeExits(t *testing.T) { t.Parallel() @@ -751,16 +815,15 @@ func generateTestProof(t *testing.T) treeTypes.Proof { func TestCheckIfCertificatesAreSettled(t *testing.T) { tests := []struct { - name string - pendingCertificates []*aggsendertypes.CertificateInfo - certificateHeaders map[common.Hash]*agglayer.CertificateHeader - getFromDBError error - clientError error - updateDBError error - expectedErrorLogMessages []string - expectedInfoMessages []string - expectedThereArePendingCerts bool - expectedError bool + name string + pendingCertificates []*aggsendertypes.CertificateInfo + certificateHeaders map[common.Hash]*agglayer.CertificateHeader + getFromDBError error + clientError error + updateDBError error + expectedErrorLogMessages []string + expectedInfoMessages []string + expectedError bool }{ { name: "All certificates settled - update successful", @@ -796,8 +859,7 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) { expectedErrorLogMessages: []string{ "error getting pending certificates: %w", }, - expectedThereArePendingCerts: true, - expectedError: true, + expectedError: true, }, { name: "Error getting certificate header", @@ -811,8 +873,7 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) { expectedErrorLogMessages: []string{ "error getting header of certificate %s with height: %d from agglayer: %w", }, - expectedThereArePendingCerts: true, - expectedError: true, + expectedError: true, }, { name: "Error updating certificate status", @@ -829,8 +890,7 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) { expectedInfoMessages: []string{ "certificate %s changed status to %s", }, - expectedThereArePendingCerts: true, - expectedError: true, + expectedError: true, }, } @@ -864,9 +924,8 @@ func TestCheckIfCertificatesAreSettled(t *testing.T) { } ctx := context.TODO() - thereArePendingCerts, err := aggSender.checkPendingCertificatesStatus(ctx) - require.Equal(t, tt.expectedThereArePendingCerts, thereArePendingCerts) - require.Equal(t, tt.expectedError, err != nil) + thereArePendingCerts := aggSender.checkPendingCertificatesStatus(ctx) + require.Equal(t, tt.expectedError, thereArePendingCerts) mockAggLayerClient.AssertExpectations(t) mockStorage.AssertExpectations(t) }) diff --git a/aggsender/types/epoch_notifier.go b/aggsender/types/epoch_notifier.go index 426ad362..045ba7ff 100644 --- a/aggsender/types/epoch_notifier.go +++ b/aggsender/types/epoch_notifier.go @@ -23,3 +23,6 @@ type EpochNotifier interface { Start(ctx context.Context) String() string } + +type BridgeL2Syncer interface { +} diff --git a/config/default.go b/config/default.go index d7188e43..61b099c8 100644 --- a/config/default.go +++ b/config/default.go @@ -343,5 +343,5 @@ URLRPCL2="{{L2URL}}" CheckSettledInterval = "2s" BlockFinality = "LatestBlock" EpochNotificationPercentage = 50 -SaveCertificatesToFiles = false +SaveCertificatesToFilesPath = "" ` diff --git a/test/Makefile b/test/Makefile index 2435730c..12f406fd 100644 --- a/test/Makefile +++ b/test/Makefile @@ -68,7 +68,7 @@ generate-mocks-aggsender: ## Generates mocks for aggsender, using mockery tool .PHONY: generate-mocks-agglayer generate-mocks-agglayer: ## Generates mocks for agglayer, using mockery tool - export "GOROOT=$$(go env GOROOT)" && $$(go env GOPATH)/bin/mockery --name=AgglayerClientInterface --dir=../agglayer --output=../agglayer --outpkg=agglayer --inpackage --structname=AgglayerClientMock --filename=mock_agglayer_client.go + export "GOROOT=$$(go env GOROOT)" && $$(go env GOPATH)/bin/mockery --name=AgglayerClientInterface --dir=../agglayer --output=../agglayer --outpkg=agglayer --inpackage --structname=AgglayerClientMock --filename=mock_agglayer_client.go ${COMMON_MOCKERY_PARAMS} .PHONY: generate-mocks-bridgesync generate-mocks-bridgesync: ## Generates mocks for bridgesync, using mockery tool