Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: small improvements on the aggsender #189

Merged
merged 2 commits into from
Feb 7, 2025
Merged

feat: small improvements on the aggsender #189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
933ef30
feat: improvements
goran-ethernal Feb 7, 2025
f8d5ce2
fix: markdown warnings
Stefan-Ethernal Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 1 addition & 42 deletions aggsender/aggsender.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/json"
"errors"
"fmt"
"os"
"time"

jRPC "github.com/0xPolygon/cdk-rpc/rpc"
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
35 changes: 0 additions & 35 deletions aggsender/aggsender_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func TestConfigString(t *testing.T) {
URLRPCL2: "http://l2.rpc.url",
BlockFinality: "latestBlock",
EpochNotificationPercentage: 50,
SaveCertificatesToFilesPath: "/path/to/certificates",
Mode: "PP",
}

Expand All @@ -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" +
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 2 additions & 5 deletions aggsender/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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'
Expand Down Expand Up @@ -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" +
Expand Down
50 changes: 9 additions & 41 deletions aggsender/flow_aggchain_prover.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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)
Expand Down
35 changes: 2 additions & 33 deletions aggsender/flow_aggchain_prover_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion aggsender/types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion config/default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ AggsenderPrivateKey = {Path = "{{SequencerPrivateKeyPath}}", Password = "{{Seque
URLRPCL2="{{L2URL}}"
BlockFinality = "LatestBlock"
EpochNotificationPercentage = 50
SaveCertificatesToFilesPath = ""
MaxRetriesStoreCertificate = 3
DelayBeetweenRetries = "60s"
KeepCertificatesHistory = true
Expand Down
Loading
Loading