feat: small improvements on the aggsender (#189)

Co-authored-by: Stefan Negovanović <[email protected]>
agglayer · Feb 7, 2025 · be291b6 · be291b6
1 parent ecd86ea
commit be291b6
Show file tree

Hide file tree

Showing 9 changed files with 25 additions and 163 deletions.
diff --git a/aggsender/aggsender.go b/aggsender/aggsender.go
@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"os"
 	"time"
 
 	jRPC "github.com/0xPolygon/cdk-rpc/rpc"
@@ -87,7 +86,7 @@ func New(
 		flowManager         types.AggsenderFlow
 	)
 
-	if types.AggsenderMode(cfg.Mode) == types.AggchainProverMode {
+	if types.AggsenderMode(cfg.Mode) == types.AggchainProofMode {
 		if cfg.AggchainProofURL == "" {
 			return nil, fmt.Errorf("aggchain prover mode requires AggchainProofURL")
 		}
@@ -244,16 +243,6 @@ func (a *AggSender) sendCertificates(ctx context.Context, returnAfterNIterations
 func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertificate, error) {
 	a.log.Infof("trying to send a new certificate...")
 
-	shouldSend, err := a.shouldSendCertificate()
-	if err != nil {
-		return nil, err
-	}
-
-	if !shouldSend {
-		a.log.Infof("waiting for pending certificates to be settled")
-		return nil, nil
-	}
-
 	certificateParams, err := a.flow.GetCertificateBuildParams(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("error getting certificate build params: %w", err)
@@ -283,7 +272,6 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
 		return nil, fmt.Errorf("forbidden to send certificate due epoch percentage")
 	}
 
-	a.saveCertificateToFile(signedCertificate)
 	a.log.Infof("certificate ready to be send to AggLayer: %s", signedCertificate.Brief())
 	if a.cfg.DryRun {
 		a.log.Warn("dry run mode enabled, skipping sending certificate")
@@ -360,24 +348,6 @@ func (a *AggSender) saveCertificateToStorage(ctx context.Context, cert types.Cer
 	return nil
 }
 
-// saveCertificate saves the certificate to a tmp file
-func (a *AggSender) saveCertificateToFile(signedCertificate *agglayer.SignedCertificate) {
-	if signedCertificate == nil || a.cfg.SaveCertificatesToFilesPath == "" {
-		return
-	}
-	fn := fmt.Sprintf("%s/certificate_%04d-%07d.json",
-		a.cfg.SaveCertificatesToFilesPath, signedCertificate.Height, time.Now().Unix())
-	a.log.Infof("saving certificate to file: %s", fn)
-	jsonData, err := json.MarshalIndent(signedCertificate, "", "  ")
-	if err != nil {
-		a.log.Errorf("error marshalling certificate: %w", err)
-	}
-
-	if err = os.WriteFile(fn, jsonData, 0644); err != nil { //nolint:gosec,mnd // we are writing to a tmp file
-		a.log.Errorf("error writing certificate to file: %w", err)
-	}
-}
-
 // signCertificate signs a certificate with the sequencer key
 func (a *AggSender) signCertificate(certificate *agglayer.Certificate) (*agglayer.SignedCertificate, error) {
 	hashToSign := certificate.HashToSign()
@@ -484,17 +454,6 @@ func (a *AggSender) updateCertificateStatus(ctx context.Context,
 	return nil
 }
 
-// shouldSendCertificate checks if a certificate should be sent at given time
-// if we have pending certificates, then we wait until they are settled
-func (a *AggSender) shouldSendCertificate() (bool, error) {
-	pendingCertificates, err := a.storage.GetCertificatesByStatus(agglayer.NonSettledStatuses)
-	if err != nil {
-		return false, fmt.Errorf("error getting pending certificates: %w", err)
-	}
-
-	return len(pendingCertificates) == 0, nil
-}
-
 // checkLastCertificateFromAgglayer checks the last certificate from agglayer
 func (a *AggSender) checkLastCertificateFromAgglayer(ctx context.Context) error {
 	networkID := a.l2Syncer.OriginNetwork()

diff --git a/aggsender/aggsender_test.go b/aggsender/aggsender_test.go
@@ -45,7 +45,6 @@ func TestConfigString(t *testing.T) {
 		URLRPCL2:                    "http://l2.rpc.url",
 		BlockFinality:               "latestBlock",
 		EpochNotificationPercentage: 50,
-		SaveCertificatesToFilesPath: "/path/to/certificates",
 		Mode:                        "PP",
 	}
 
@@ -55,7 +54,6 @@ func TestConfigString(t *testing.T) {
 		"URLRPCL2: http://l2.rpc.url\n" +
 		"BlockFinality: latestBlock\n" +
 		"EpochNotificationPercentage: 50\n" +
-		"SaveCertificatesToFilesPath: /path/to/certificates\n" +
 		"DryRun: false\n" +
 		"EnableRPC: false\n" +
 		"AggchainProofURL: \n" +
@@ -459,7 +457,6 @@ func TestSendCertificate_NoClaims(t *testing.T) {
 		rateLimiter:      aggkitcommon.NewRateLimit(aggkitcommon.RateLimitConfig{}),
 	}
 
-	mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 	mockStorage.On("GetLastSentCertificate").Return(&aggsendertypes.CertificateInfo{
 		NewLocalExitRoot: common.HexToHash("0x123"),
 		Height:           1,
@@ -741,38 +738,12 @@ func TestSendCertificate(t *testing.T) {
 		mockFn        func(*mocks.AggSenderStorage, *mocks.AggsenderFlow, *mocks.L1InfoTreeSyncer, *agglayer.AgglayerClientMock)
 		expectedError string
 	}{
-		{
-			name: "error getting pending certificates",
-			mockFn: func(mockStorage *mocks.AggSenderStorage,
-				mockFlow *mocks.AggsenderFlow,
-				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
-				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return(nil, errors.New("some error")).Once()
-			},
-			expectedError: "error getting pending certificates",
-		},
-		{
-			name: "has pending certificates",
-			mockFn: func(mockStorage *mocks.AggSenderStorage,
-				mockFlow *mocks.AggsenderFlow,
-				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
-				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return(
-					[]*aggsendertypes.CertificateInfo{
-						{
-							Height: 0,
-							Status: agglayer.Pending,
-						},
-					}, nil).Once()
-			},
-		},
 		{
 			name: "error getting certificate build params",
 			mockFn: func(mockStorage *mocks.AggSenderStorage,
 				mockFlow *mocks.AggsenderFlow,
 				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
 				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 				mockFlow.On("GetCertificateBuildParams", mock.Anything).Return(nil, errors.New("some error")).Once()
 			},
 			expectedError: "error getting certificate build params",
@@ -783,7 +754,6 @@ func TestSendCertificate(t *testing.T) {
 				mockFlow *mocks.AggsenderFlow,
 				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
 				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 				mockFlow.On("GetCertificateBuildParams", mock.Anything).Return(&aggsendertypes.CertificateBuildParams{
 					Bridges: []bridgesync.Bridge{},
 				}, nil).Once()
@@ -795,7 +765,6 @@ func TestSendCertificate(t *testing.T) {
 				mockFlow *mocks.AggsenderFlow,
 				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
 				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 				mockFlow.On("GetCertificateBuildParams", mock.Anything).Return(&aggsendertypes.CertificateBuildParams{
 					Bridges: []bridgesync.Bridge{{}},
 				}, nil).Once()
@@ -809,7 +778,6 @@ func TestSendCertificate(t *testing.T) {
 				mockFlow *mocks.AggsenderFlow,
 				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
 				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 				mockFlow.On("GetCertificateBuildParams", mock.Anything).Return(&aggsendertypes.CertificateBuildParams{
 					Bridges: []bridgesync.Bridge{{}},
 				}, nil).Once()
@@ -829,7 +797,6 @@ func TestSendCertificate(t *testing.T) {
 				mockFlow *mocks.AggsenderFlow,
 				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
 				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 				mockFlow.On("GetCertificateBuildParams", mock.Anything).Return(&aggsendertypes.CertificateBuildParams{
 					Bridges: []bridgesync.Bridge{{}},
 				}, nil).Once()
@@ -850,7 +817,6 @@ func TestSendCertificate(t *testing.T) {
 				mockFlow *mocks.AggsenderFlow,
 				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
 				mockAgglayerClient *agglayer.AgglayerClientMock) {
-				mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 				mockFlow.On("GetCertificateBuildParams", mock.Anything).Return(&aggsendertypes.CertificateBuildParams{
 					Bridges: []bridgesync.Bridge{{}},
 				}, nil).Once()
@@ -912,7 +878,6 @@ func TestLimitEpochPercent_Greater(t *testing.T) {
 	testData.sut.cfg.MaxEpochPercentageAllowedToSendCertificate = 80
 
 	ctx := context.TODO()
-	testData.storageMock.EXPECT().GetCertificatesByStatus(mock.Anything).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
 	testData.l2syncerMock.EXPECT().GetLastProcessedBlock(ctx).Return(uint64(100), nil).Once()
 	testData.storageMock.EXPECT().GetLastSentCertificate().Return(&aggsendertypes.CertificateInfo{
 		FromBlock: 1,

diff --git a/aggsender/config.go b/aggsender/config.go
@@ -24,8 +24,6 @@ type Config struct {
 	// 0 -> Begin
 	// 50 -> Middle
 	EpochNotificationPercentage uint `mapstructure:"EpochNotificationPercentage"`
-	// SaveCertificatesToFilesPath if != "" tells  the AggSender to save the certificates to a file in this path
-	SaveCertificatesToFilesPath string `mapstructure:"SaveCertificatesToFilesPath"`
 	// MaxRetriesStoreCertificate is the maximum number of retries to store a certificate
 	// 0 is infinite
 	MaxRetriesStoreCertificate int `mapstructure:"MaxRetriesStoreCertificate"`
@@ -46,8 +44,8 @@ type Config struct {
 	EnableRPC bool `mapstructure:"EnableRPC"`
 	// AggchainProofURL is the URL of the AggkitProver
 	AggchainProofURL string `mapstructure:"AggchainProofURL"`
-	// Mode is the mode of the AggSender (regular pessimistic proof mode or the aggchain prover mode)
-	Mode string `jsonschema:"enum=PessimisticProof, enum=AggchainProver" mapstructure:"Mode"`
+	// Mode is the mode of the AggSender (regular pessimistic proof mode or the aggchain proof mode)
+	Mode string `jsonschema:"enum=PessimisticProof, enum=AggchainProof" mapstructure:"Mode"`
 	// CheckStatusCertificateInterval is the interval at which the AggSender will check the certificate status in Agglayer
 	CheckStatusCertificateInterval types.Duration `mapstructure:"CheckStatusCertificateInterval"`
 	// RetryCertAfterInError when a cert pass to 'InError'
@@ -76,7 +74,6 @@ func (c Config) String() string {
 		"URLRPCL2: " + c.URLRPCL2 + "\n" +
 		"BlockFinality: " + c.BlockFinality + "\n" +
 		"EpochNotificationPercentage: " + fmt.Sprintf("%d", c.EpochNotificationPercentage) + "\n" +
-		"SaveCertificatesToFilesPath: " + c.SaveCertificatesToFilesPath + "\n" +
 		"DryRun: " + fmt.Sprintf("%t", c.DryRun) + "\n" +
 		"EnableRPC: " + fmt.Sprintf("%t", c.EnableRPC) + "\n" +
 		"AggchainProofURL: " + c.AggchainProofURL + "\n" +

diff --git a/aggsender/flow_aggchain_prover.go b/aggsender/flow_aggchain_prover.go
@@ -55,6 +55,10 @@ func (a *aggchainProverFlow) GetCertificateBuildParams(ctx context.Context) (*ty
 		return nil, fmt.Errorf("aggchainProverFlow - error getting last sent certificate: %w", err)
 	}
 
+	var (
+		buildParams *types.CertificateBuildParams
+	)
+
 	if lastSentCertificateInfo != nil && lastSentCertificateInfo.Status == agglayer.InError {
 		// if the last certificate was in error, we need to resend it
 		a.log.Infof("resending the same InError certificate: %s", lastSentCertificateInfo.String())
@@ -72,60 +76,24 @@ func (a *aggchainProverFlow) GetCertificateBuildParams(ctx context.Context) (*ty
 				"but no bridges to resend the same certificate", lastSentCertificateInfo.String())
 		}
 
-		aggProof := lastSentCertificateInfo.AggchainProof
-		toBlock := lastSentCertificateInfo.ToBlock
-
-		if len(aggProof) == 0 {
-			proof, leaf, root, err := a.getFinalizedL1InfoTreeData(ctx)
-			if err != nil {
-				return nil, fmt.Errorf("aggchainProverFlow - error getting finalized L1 Info tree data: %w", err)
-			}
-
-			aggchainProof, err := a.aggchainProofClient.GenerateAggchainProof(lastSentCertificateInfo.FromBlock,
-				lastSentCertificateInfo.ToBlock, root, leaf, proof)
-			if err != nil {
-				return nil, fmt.Errorf("aggchainProverFlow - error fetching aggchain proof for block range %d : %d : %w",
-					lastSentCertificateInfo.FromBlock, lastSentCertificateInfo.ToBlock, err)
-			}
-
-			a.log.Infof("aggchainProverFlow - InError certificate did not have auth proof, "+
-				"so got it from the aggchain prover for range %d : %d. Proof: %s. Requested range: %d : %d",
-				aggchainProof.StartBlock, aggchainProof.EndBlock, aggchainProof.Proof,
-				lastSentCertificateInfo.FromBlock, lastSentCertificateInfo.ToBlock)
-
-			aggProof = aggchainProof.Proof
-
-			if aggchainProof.EndBlock < lastSentCertificateInfo.ToBlock {
-				// aggchain prover can return a proof for a smaller range than requested
-				// so we need to adjust the toBlock
-				toBlock = aggchainProof.EndBlock
-			}
-		}
-
 		// we need to resend the same certificate
-		buildParams := &types.CertificateBuildParams{
+		buildParams = &types.CertificateBuildParams{
 			FromBlock:           lastSentCertificateInfo.FromBlock,
 			ToBlock:             lastSentCertificateInfo.ToBlock,
 			RetryCount:          lastSentCertificateInfo.RetryCount + 1,
 			Bridges:             bridges,
 			Claims:              claims,
 			LastSentCertificate: lastSentCertificateInfo,
 			CreatedAt:           lastSentCertificateInfo.CreatedAt,
-			AggchainProof:       aggProof,
 		}
+	}
 
-		buildParams, err = adjustBlockRange(buildParams, lastSentCertificateInfo.ToBlock, toBlock)
+	if buildParams == nil {
+		// use the old logic, where we build the new certificate
+		buildParams, err = a.baseFlow.GetCertificateBuildParams(ctx)
 		if err != nil {
 			return nil, err
 		}
-
-		return buildParams, nil
-	}
-
-	// use the old logic, where we build the new certificate
-	buildParams, err := a.baseFlow.GetCertificateBuildParams(ctx)
-	if err != nil {
-		return nil, err
 	}
 
 	proof, leaf, root, err := a.getFinalizedL1InfoTreeData(ctx)

diff --git a/aggsender/flow_aggchain_prover_test.go b/aggsender/flow_aggchain_prover_test.go
@@ -63,7 +63,7 @@ func Test_AggchainProverFlow_GetCertificateBuildParams(t *testing.T) {
 			expectedError: "no bridges to resend the same certificate",
 		},
 		{
-			name: "resend InError certificate with no auth proof",
+			name: "resend InError certificate",
 			mockFn: func(mockStorage *mocks.AggSenderStorage,
 				mockL2Syncer *mocks.L2BridgeSyncer,
 				mockProverClient *mocks.AggchainProofClientInterface,
@@ -104,7 +104,7 @@ func Test_AggchainProverFlow_GetCertificateBuildParams(t *testing.T) {
 			},
 		},
 		{
-			name: "resend InError certificate with no auth proof - aggchain prover returned smaller range",
+			name: "resend InError certificate - aggchain prover returned smaller range",
 			mockFn: func(mockStorage *mocks.AggSenderStorage,
 				mockL2Syncer *mocks.L2BridgeSyncer,
 				mockProverClient *mocks.AggchainProofClientInterface,
@@ -146,37 +146,6 @@ func Test_AggchainProverFlow_GetCertificateBuildParams(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "resend InError certificate with auth proof",
-			mockFn: func(mockStorage *mocks.AggSenderStorage,
-				mockL2Syncer *mocks.L2BridgeSyncer,
-				mockProverClient *mocks.AggchainProofClientInterface,
-				mockL1Client *mocks.EthClient,
-				mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer) {
-				mockStorage.On("GetLastSentCertificate").Return(&types.CertificateInfo{
-					FromBlock:     1,
-					ToBlock:       10,
-					Status:        agglayer.InError,
-					AggchainProof: []byte("existing-proof"),
-				}, nil)
-				mockL2Syncer.On("GetBridgesPublished", ctx, uint64(1), uint64(10)).Return([]bridgesync.Bridge{{}}, nil)
-				mockL2Syncer.On("GetClaims", ctx, uint64(1), uint64(10)).Return([]bridgesync.Claim{{}}, nil)
-			},
-			expectedParams: &types.CertificateBuildParams{
-				FromBlock:     1,
-				ToBlock:       10,
-				RetryCount:    1,
-				Bridges:       []bridgesync.Bridge{{}},
-				Claims:        []bridgesync.Claim{{}},
-				AggchainProof: []byte("existing-proof"),
-				LastSentCertificate: &types.CertificateInfo{
-					FromBlock:     1,
-					ToBlock:       10,
-					Status:        agglayer.InError,
-					AggchainProof: []byte("existing-proof"),
-				},
-			},
-		},
 		{
 			name: "error fetching aggchain proof for new certificate",
 			mockFn: func(mockStorage *mocks.AggSenderStorage,

diff --git a/aggsender/types/types.go b/aggsender/types/types.go
@@ -20,7 +20,7 @@ type AggsenderMode string
 
 const (
 	PessimisticProofMode AggsenderMode = "PessimisticProof"
-	AggchainProverMode   AggsenderMode = "AggchainProver"
+	AggchainProofMode    AggsenderMode = "AggchainProof"
 )
 
 // AggsenderFlow is an interface that defines the methods to manage the flow of the AggSender

diff --git a/config/default.go b/config/default.go
@@ -210,7 +210,6 @@ AggsenderPrivateKey = {Path = "{{SequencerPrivateKeyPath}}", Password = "{{Seque
 URLRPCL2="{{L2URL}}"
 BlockFinality = "LatestBlock"
 EpochNotificationPercentage = 50
-SaveCertificatesToFilesPath = ""
 MaxRetriesStoreCertificate = 3
 DelayBeetweenRetries = "60s"
 KeepCertificatesHistory = true