From 9fadd2a8be48ad8f0e9399b250a21e68e485849a Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Tue, 12 Aug 2025 11:24:21 +0200 Subject: [PATCH 1/6] feat: add sanity check --- aggsender/mocks/mock_certificate_querier.go | 57 ++++++++++++++++ aggsender/query/certificate_query.go | 68 ++++++++++++++++--- aggsender/types/interfaces.go | 3 + aggsender/validator/validate_certificate.go | 48 +++++++++++-- .../validator/validate_certificate_test.go | 45 ++++++++++++ 5 files changed, 203 insertions(+), 18 deletions(-) diff --git a/aggsender/mocks/mock_certificate_querier.go b/aggsender/mocks/mock_certificate_querier.go index 29f5baa3c..235318d99 100644 --- a/aggsender/mocks/mock_certificate_querier.go +++ b/aggsender/mocks/mock_certificate_querier.go @@ -128,6 +128,63 @@ func (_c *CertificateQuerier_GetLastSettledCertificateToBlock_Call) RunAndReturn return _c } +// GetNewCertificateToBlock provides a mock function with given fields: ctx, cert +func (_m *CertificateQuerier) GetNewCertificateToBlock(ctx context.Context, cert *agglayertypes.Certificate) (uint64, error) { + ret := _m.Called(ctx, cert) + + if len(ret) == 0 { + panic("no return value specified for GetNewCertificateToBlock") + } + + var r0 uint64 + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *agglayertypes.Certificate) (uint64, error)); ok { + return rf(ctx, cert) + } + if rf, ok := ret.Get(0).(func(context.Context, *agglayertypes.Certificate) uint64); ok { + r0 = rf(ctx, cert) + } else { + r0 = ret.Get(0).(uint64) + } + + if rf, ok := ret.Get(1).(func(context.Context, *agglayertypes.Certificate) error); ok { + r1 = rf(ctx, cert) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// CertificateQuerier_GetNewCertificateToBlock_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetNewCertificateToBlock' +type CertificateQuerier_GetNewCertificateToBlock_Call struct { + *mock.Call +} + +// GetNewCertificateToBlock is a helper method to define mock.On call +// - ctx context.Context +// - cert *agglayertypes.Certificate +func (_e *CertificateQuerier_Expecter) GetNewCertificateToBlock(ctx interface{}, cert interface{}) *CertificateQuerier_GetNewCertificateToBlock_Call { + return &CertificateQuerier_GetNewCertificateToBlock_Call{Call: _e.mock.On("GetNewCertificateToBlock", ctx, cert)} +} + +func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) Run(run func(ctx context.Context, cert *agglayertypes.Certificate)) *CertificateQuerier_GetNewCertificateToBlock_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*agglayertypes.Certificate)) + }) + return _c +} + +func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) Return(_a0 uint64, _a1 error) *CertificateQuerier_GetNewCertificateToBlock_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) RunAndReturn(run func(context.Context, *agglayertypes.Certificate) (uint64, error)) *CertificateQuerier_GetNewCertificateToBlock_Call { + _c.Call.Return(run) + return _c +} + // NewCertificateQuerier creates a new instance of CertificateQuerier. 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 NewCertificateQuerier(t interface { diff --git a/aggsender/query/certificate_query.go b/aggsender/query/certificate_query.go index 42ebe1917..f0edb6f11 100644 --- a/aggsender/query/certificate_query.go +++ b/aggsender/query/certificate_query.go @@ -7,6 +7,7 @@ import ( "github.com/agglayer/aggkit/agglayer" agglayertypes "github.com/agglayer/aggkit/agglayer/types" "github.com/agglayer/aggkit/aggsender/types" + "github.com/ethereum/go-ethereum/common" ) var _ types.CertificateQuerier = (*certificateQuerier)(nil) @@ -65,17 +66,13 @@ func (c *certificateQuerier) GetLastSettledCertificateToBlock( ) // 1. Get the latest settled bridge exit block number - if cert.NewLocalExitRoot != types.EmptyLER { - // if NewLER is not the first empty LER, it means that the certificate - // or certificate before it had bridge exits, so we can use it to - // to determine the last bridge exit block - newLER, err := c.l2BridgeSyncer.GetExitRootByHash(ctx, cert.NewLocalExitRoot) - if err != nil { - return 0, fmt.Errorf("failed to get exit root by hash using NewLocalExitRoot %s: %w", - cert.NewLocalExitRoot.String(), err) - } - - lastBridgeExitBlock = newLER.BlockNum + // if NewLER is not the first empty LER, it means that the certificate + // or certificate before it had bridge exits, so we can use it to + // to determine the last bridge exit block + lastBridgeExitBlock, err = c.getBlockNumFromLER(ctx, cert.NewLocalExitRoot) + if err != nil { + return 0, fmt.Errorf("failed to get exit root by hash using NewLocalExitRoot %s: %w", + cert.NewLocalExitRoot.String(), err) } // TODO - this might need to be changed once agglayer gives support for this @@ -106,6 +103,41 @@ func (c *certificateQuerier) GetLastSettledCertificateToBlock( return max(lastBridgeExitBlock, lastImportedBridgeExitBlock, lastSettledL2BlockNum), nil } +// GetNewCertificateToBlock determines the new certificate To block based on the +// NewLocalExitRoot and the last imported bridge exit block. +func (c *certificateQuerier) GetNewCertificateToBlock( + ctx context.Context, + cert *agglayertypes.Certificate) (uint64, error) { + var ( + lastBridgeExitBlock uint64 + lastImportedBridgeExitBlock uint64 + err error + ) + + // if NewLER is not the first empty LER, it means that the certificate + // or certificate before it had bridge exits, so we can use it to + // to determine the last bridge exit block + lastBridgeExitBlock, err = c.getBlockNumFromLER(ctx, cert.NewLocalExitRoot) + if err != nil { + return 0, fmt.Errorf("failed to get exit root by hash using NewLocalExitRoot %s: %w", + cert.NewLocalExitRoot.String(), err) + } + + if len(cert.ImportedBridgeExits) > 0 { + // if there are imported bridge exits, we can use the last one to determine the new certificate to block + lastImportedBridgeExit := cert.ImportedBridgeExits[len(cert.ImportedBridgeExits)-1] + bigGlobalIndex := lastImportedBridgeExit.GlobalIndex.ToBigInt() + claim, err := c.l2BridgeSyncer.GetClaimByGlobalIndex(ctx, bigGlobalIndex) + if err != nil { + return 0, fmt.Errorf("failed to get claim by global index %s: %w", bigGlobalIndex.String(), err) + } + + lastImportedBridgeExitBlock = claim.BlockNum + } + + return max(lastBridgeExitBlock, lastImportedBridgeExitBlock), nil +} + // CalculateCertificateType determines the type of certificate based on the last block number in certificate func (c *certificateQuerier) CalculateCertificateType(certToBlock uint64) types.CertificateType { if certToBlock == 0 { @@ -118,3 +150,17 @@ func (c *certificateQuerier) CalculateCertificateType(certToBlock uint64) types. return types.CertificateTypeFEP } + +func (c *certificateQuerier) getBlockNumFromLER(ctx context.Context, localExitRoot common.Hash) (uint64, error) { + if localExitRoot == types.EmptyLER { + return 0, nil // Empty LER means no exit root, so return 0 + } + + exitRoot, err := c.l2BridgeSyncer.GetExitRootByHash(ctx, localExitRoot) + if err != nil { + return 0, fmt.Errorf("failed to get local exit root by hash %s: %w", + localExitRoot.String(), err) + } + + return exitRoot.BlockNum, nil +} diff --git a/aggsender/types/interfaces.go b/aggsender/types/interfaces.go index 5a33d19be..edf5afa49 100644 --- a/aggsender/types/interfaces.go +++ b/aggsender/types/interfaces.go @@ -282,5 +282,8 @@ type CertificateQuerier interface { GetLastSettledCertificateToBlock( ctx context.Context, cert *agglayertypes.CertificateHeader) (uint64, error) + GetNewCertificateToBlock( + ctx context.Context, + cert *agglayertypes.Certificate) (uint64, error) CalculateCertificateType(certToBlock uint64) CertificateType } diff --git a/aggsender/validator/validate_certificate.go b/aggsender/validator/validate_certificate.go index e9b73aae9..b1bab3368 100644 --- a/aggsender/validator/validate_certificate.go +++ b/aggsender/validator/validate_certificate.go @@ -68,6 +68,18 @@ func (a *CertificateValidator) ValidateCertificate(ctx context.Context, params t err error ) + if params.PreviousCertificate != nil { + previousCertificateToBlock, err = a.certQuerier.GetLastSettledCertificateToBlock(ctx, params.PreviousCertificate) + if err != nil { + return fmt.Errorf("failed to get last settled certificate block: %w", err) + } + } + + // Validate last L2 block in certificate + if err := a.validateLastL2BlockInCert(ctx, params, previousCertificateToBlock); err != nil { + return fmt.Errorf("failed to validate last L2 block in new certificate: %w", err) + } + // Between cert must be no gap because if there are could be a attack vector if err := a.checkContigousCertificates(params); err != nil { return fmt.Errorf("failed CheckContigousCertificates: %w", err) @@ -77,31 +89,29 @@ func (a *CertificateValidator) ValidateCertificate(ctx context.Context, params t return fmt.Errorf("failed CheckCertificatesContents: %w", err) } - if params.PreviousCertificate != nil { - previousCertificateToBlock, err = a.certQuerier.GetLastSettledCertificateToBlock(ctx, params.PreviousCertificate) - if err != nil { - return fmt.Errorf("failed to get last settled certificate block: %w", err) - } - } - // Build corresponding certificate preBuildParams, err := a.getCertificatePreBuildParams(ctx, params, previousCertificateToBlock) if err != nil { return fmt.Errorf("failed to get certificate pre-build params: %w", err) } + a.log.Debugf("aggsender-validator: preBuild: %s", preBuildParams.String()) + buildParams, err := a.flowPP.GenerateBuildParams(ctx, preBuildParams) if err != nil { return fmt.Errorf("failed flow.GenerateBuildParams: %w", err) } + certificate, err := a.flowPP.BuildCertificate(ctx, buildParams) if err != nil { return fmt.Errorf("failed flow.BuildCertificate: %w", err) } + err = a.compareCertificates(params.Certificate, certificate) if err != nil { return fmt.Errorf("certificate not equal to expected: %w", err) } + return nil } @@ -225,3 +235,27 @@ func (a *CertificateValidator) getCertificatePreBuildParams( }, }, nil } + +// validateLastL2BlockInCert checks that the provided last L2 block in the certificate by the proposer +// is greater or equal to the blocks we see in the new certificate +func (a *CertificateValidator) validateLastL2BlockInCert( + ctx context.Context, + req types.VerifyIncomingRequest, + lastSettledBlock uint64) error { + if req.LastL2BlockInCert <= lastSettledBlock { + return fmt.Errorf("last L2 block in certificate %d must be greater than last settled block %d", + req.LastL2BlockInCert, lastSettledBlock) + } + + newCertToBlock, err := a.certQuerier.GetNewCertificateToBlock(ctx, req.Certificate) + if err != nil { + return fmt.Errorf("failed to get new certificate to block: %w", err) + } + + if newCertToBlock > req.LastL2BlockInCert { + return fmt.Errorf("new certificate to block %d must be less than or equal to last L2 block "+ + "provided by the proposer %d", newCertToBlock, req.LastL2BlockInCert) + } + + return nil +} diff --git a/aggsender/validator/validate_certificate_test.go b/aggsender/validator/validate_certificate_test.go index ffeedf3f9..80570c066 100644 --- a/aggsender/validator/validate_certificate_test.go +++ b/aggsender/validator/validate_certificate_test.go @@ -41,14 +41,51 @@ func TestValidateCertificate(t *testing.T) { require.ErrorContains(t, err, "certificate metadata is expected to be zero hash") }) + t.Run("invalid LastL2BlockInCert - ToBlock in cert larger", func(t *testing.T) { + testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) + err := testData.sut.ValidateCertificate(testData.ctx, types.VerifyIncomingRequest{ + Certificate: &agglayertypes.Certificate{ + Height: 1, + Metadata: aggkitcommon.ZeroHash, + }, + PreviousCertificate: nil, + LastL2BlockInCert: 5, + }) + require.Error(t, err) + require.ErrorContains(t, err, "new certificate to block 10 must be less than or equal to last L2 block provided by the proposer 5") + }) + + t.Run("invalid LastL2BlockInCert - smaller than previous settled block", func(t *testing.T) { + testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetLastSettledCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(20), nil) + err := testData.sut.ValidateCertificate(testData.ctx, types.VerifyIncomingRequest{ + Certificate: &agglayertypes.Certificate{ + Height: 1, + Metadata: aggkitcommon.ZeroHash, + }, + PreviousCertificate: &agglayertypes.CertificateHeader{ + Height: 0, + Metadata: aggkitcommon.ZeroHash, + Status: agglayertypes.Pending, + NewLocalExitRoot: common.HexToHash("0x1"), + }, + LastL2BlockInCert: 10, + }) + require.Error(t, err) + require.ErrorContains(t, err, "last L2 block in certificate 10 must be greater than last settled block 20") + }) + t.Run("first cert bad height", func(t *testing.T) { testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) err := testData.sut.ValidateCertificate(testData.ctx, types.VerifyIncomingRequest{ Certificate: &agglayertypes.Certificate{ Height: 1, Metadata: aggkitcommon.ZeroHash, }, PreviousCertificate: nil, + LastL2BlockInCert: 10, }) require.Error(t, err) require.ErrorContains(t, err, "first certificate must have height 0") @@ -57,6 +94,7 @@ func TestValidateCertificate(t *testing.T) { t.Run("first cert bad previous LER", func(t *testing.T) { testData := newTestDataCertificateValidator(t) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) err := testData.sut.ValidateCertificate(testData.ctx, types.VerifyIncomingRequest{ Certificate: &agglayertypes.Certificate{ Height: 0, @@ -64,6 +102,7 @@ func TestValidateCertificate(t *testing.T) { PrevLocalExitRoot: common.HexToHash("0x1"), }, PreviousCertificate: nil, + LastL2BlockInCert: 10, }) require.Error(t, err) require.ErrorContains(t, err, "first certificate must have correct starting PrevLocalExitRoot") @@ -71,6 +110,8 @@ func TestValidateCertificate(t *testing.T) { t.Run("prev cert bad status", func(t *testing.T) { testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetLastSettledCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(20), nil) err := testData.sut.ValidateCertificate(testData.ctx, types.VerifyIncomingRequest{ Certificate: &agglayertypes.Certificate{ Height: 1, @@ -91,6 +132,7 @@ func TestValidateCertificate(t *testing.T) { t.Run("GetCertificatePreBuildParams error l1infotree", func(t *testing.T) { testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). @@ -110,6 +152,7 @@ func TestValidateCertificate(t *testing.T) { t.Run("fails flowPP.GenerateBuildParams", func(t *testing.T) { testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). @@ -131,6 +174,7 @@ func TestValidateCertificate(t *testing.T) { t.Run("fails flowPP.BuildCertificate", func(t *testing.T) { testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). @@ -154,6 +198,7 @@ func TestValidateCertificate(t *testing.T) { t.Run("fails CompareCertificates", func(t *testing.T) { testData := newTestDataCertificateValidator(t) + testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). From 289ad89b278a79f2bc07f04ecd950a6873a1a610 Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Tue, 12 Aug 2025 14:11:29 +0200 Subject: [PATCH 2/6] feat: UTs --- aggsender/query/certificate_query_test.go | 244 ++++++++++++++++++++++ 1 file changed, 244 insertions(+) diff --git a/aggsender/query/certificate_query_test.go b/aggsender/query/certificate_query_test.go index a7e9c2f98..1f4c23364 100644 --- a/aggsender/query/certificate_query_test.go +++ b/aggsender/query/certificate_query_test.go @@ -193,3 +193,247 @@ func TestGetLastSettledCertificateToBlock(t *testing.T) { }) } } + +func TestCalculateCertificateType(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + certToBlock uint64 + startL2Block uint64 + expectedType types.CertificateType + }{ + { + name: "zero block returns unknown", + certToBlock: 0, + startL2Block: 100, + expectedType: types.CertificateTypeUnknown, + }, + { + name: "block before start L2 block returns PP", + certToBlock: 50, + startL2Block: 100, + expectedType: types.CertificateTypePP, + }, + { + name: "block equal to start L2 block returns FEP", + certToBlock: 100, + startL2Block: 100, + expectedType: types.CertificateTypeFEP, + }, + { + name: "block after start L2 block returns FEP", + certToBlock: 150, + startL2Block: 100, + expectedType: types.CertificateTypeFEP, + }, + { + name: "start L2 block is zero with non-zero cert block returns FEP", + certToBlock: 50, + startL2Block: 0, + expectedType: types.CertificateTypeFEP, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + mockAggchainFEPQuerier := mocks.NewAggchainFEPRollupQuerier(t) + mockAgglayerClient := agglayermocks.NewAgglayerClientMock(t) + mockL2BridgeSyncer := mocks.NewL2BridgeSyncer(t) + + mockAggchainFEPQuerier.EXPECT().StartL2Block().Return(tc.startL2Block).Maybe() + + certQuerier := NewCertificateQuerier( + mockL2BridgeSyncer, + mockAggchainFEPQuerier, + mockAgglayerClient, + ) + + result := certQuerier.CalculateCertificateType(tc.certToBlock) + require.Equal(t, tc.expectedType, result) + + mockAggchainFEPQuerier.AssertExpectations(t) + }) + } +} + +func TestGetNewCertificateToBlock(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + testCases := []struct { + name string + certificate *agglayertypes.Certificate + mockFn func(*mocks.L2BridgeSyncer) + expectedErr string + expectedBlock uint64 + }{ + { + name: "successful with both local exit root and imported bridge exits", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: common.HexToHash("0x123"), + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{ + {GlobalIndex: &agglayertypes.GlobalIndex{}}, + {GlobalIndex: &agglayertypes.GlobalIndex{}}, + }, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0x123")).Return(&treetypes.Root{ + BlockNum: uint64(100), + }, nil) + lastImportedBridgeExit := &agglayertypes.GlobalIndex{} + bridgeSyncer.EXPECT().GetClaimByGlobalIndex(ctx, lastImportedBridgeExit.ToBigInt()).Return(bridgesync.Claim{ + BlockNum: 150, + }, nil) + }, + expectedBlock: 150, // max of 100, 150 + }, + { + name: "empty local exit root with imported bridge exits", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: types.EmptyLER, + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{ + {GlobalIndex: &agglayertypes.GlobalIndex{}}, + }, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + importedBridgeExit := &agglayertypes.GlobalIndex{} + bridgeSyncer.EXPECT().GetClaimByGlobalIndex(ctx, importedBridgeExit.ToBigInt()).Return(bridgesync.Claim{ + BlockNum: 75, + }, nil) + }, + expectedBlock: 75, // max of 0, 75 + }, + { + name: "non-empty local exit root with no imported bridge exits", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: common.HexToHash("0x456"), + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{}, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0x456")).Return(&treetypes.Root{ + BlockNum: uint64(200), + }, nil) + }, + expectedBlock: 200, // max of 200, 0 + }, + { + name: "empty local exit root with no imported bridge exits", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: types.EmptyLER, + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{}, + }, + expectedBlock: 0, // max of 0, 0 + }, + { + name: "nil imported bridge exits", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: common.HexToHash("0x789"), + ImportedBridgeExits: nil, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0x789")).Return(&treetypes.Root{ + BlockNum: uint64(300), + }, nil) + }, + expectedBlock: 300, // max of 300, 0 + }, + { + name: "error getting exit root by hash", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: common.HexToHash("0xabc"), + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{}, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0xabc")).Return(nil, errors.New("exit root not found")) + }, + expectedErr: "failed to get exit root by hash using NewLocalExitRoot", + }, + { + name: "error getting claim by global index", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: types.EmptyLER, + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{ + {GlobalIndex: &agglayertypes.GlobalIndex{}}, + }, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + importedBridgeExit := &agglayertypes.GlobalIndex{} + bridgeSyncer.EXPECT().GetClaimByGlobalIndex(ctx, importedBridgeExit.ToBigInt()).Return(bridgesync.Claim{}, errors.New("claim not found")) + }, + expectedErr: "failed to get claim by global index", + }, + { + name: "multiple imported bridge exits uses last one", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: types.EmptyLER, + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{ + {GlobalIndex: &agglayertypes.GlobalIndex{}}, // First one - should not be used + {GlobalIndex: &agglayertypes.GlobalIndex{}}, // Second one - should not be used + {GlobalIndex: &agglayertypes.GlobalIndex{}}, // Last one - should be used + }, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + // Mock claim by global index for last imported bridge exit only + lastImportedBridgeExit := &agglayertypes.GlobalIndex{} + bridgeSyncer.EXPECT().GetClaimByGlobalIndex(ctx, lastImportedBridgeExit.ToBigInt()).Return(bridgesync.Claim{ + BlockNum: 250, + }, nil) + }, + expectedBlock: 250, // max of 0, 250 + }, + { + name: "local exit root block higher than imported bridge exit block", + certificate: &agglayertypes.Certificate{ + NewLocalExitRoot: common.HexToHash("0xdef"), + ImportedBridgeExits: []*agglayertypes.ImportedBridgeExit{ + {GlobalIndex: &agglayertypes.GlobalIndex{}}, + }, + }, + mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { + bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0xdef")).Return(&treetypes.Root{ + BlockNum: uint64(400), + }, nil) + + importedBridgeExit := &agglayertypes.GlobalIndex{} + bridgeSyncer.EXPECT().GetClaimByGlobalIndex(ctx, importedBridgeExit.ToBigInt()).Return(bridgesync.Claim{ + BlockNum: 100, + }, nil) + }, + expectedBlock: 400, // max of 400, 100 + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + mockAggchainFEPQuerier := mocks.NewAggchainFEPRollupQuerier(t) + mockAgglayerClient := agglayermocks.NewAgglayerClientMock(t) + mockL2BridgeSyncer := mocks.NewL2BridgeSyncer(t) + + if tc.mockFn != nil { + tc.mockFn(mockL2BridgeSyncer) + } + + certQuerier := NewCertificateQuerier( + mockL2BridgeSyncer, + mockAggchainFEPQuerier, + mockAgglayerClient, + ) + + block, err := certQuerier.GetNewCertificateToBlock(ctx, tc.certificate) + if tc.expectedErr != "" { + require.ErrorContains(t, err, tc.expectedErr) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedBlock, block) + } + + mockL2BridgeSyncer.AssertExpectations(t) + }) + } +} From aa01492a9119b51b56a0c0f09a00e8018c0a170c Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Tue, 12 Aug 2025 15:37:53 +0200 Subject: [PATCH 3/6] fix: change CalculateCertificateType --- .../mocks/mock_aggchain_fep_rollup_querier.go | 45 +++ aggsender/mocks/mock_certificate_querier.go | 107 +++++-- aggsender/query/aggchain_fep_rollup_query.go | 9 + aggsender/query/certificate_query.go | 31 +- aggsender/query/certificate_query_test.go | 278 ++++++++++++++---- .../statuschecker/cert_status_checker.go | 2 +- .../statuschecker/cert_status_checker_test.go | 8 +- aggsender/types/interfaces.go | 4 +- aggsender/validator/validate_certificate.go | 2 +- .../validator/validate_certificate_test.go | 10 +- 10 files changed, 383 insertions(+), 113 deletions(-) diff --git a/aggsender/mocks/mock_aggchain_fep_rollup_querier.go b/aggsender/mocks/mock_aggchain_fep_rollup_querier.go index 2835162e7..c9eeaa16b 100644 --- a/aggsender/mocks/mock_aggchain_fep_rollup_querier.go +++ b/aggsender/mocks/mock_aggchain_fep_rollup_querier.go @@ -72,6 +72,51 @@ func (_c *AggchainFEPRollupQuerier_GetLastSettledL2Block_Call) RunAndReturn(run return _c } +// IsFEP provides a mock function with no fields +func (_m *AggchainFEPRollupQuerier) IsFEP() bool { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for IsFEP") + } + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// AggchainFEPRollupQuerier_IsFEP_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'IsFEP' +type AggchainFEPRollupQuerier_IsFEP_Call struct { + *mock.Call +} + +// IsFEP is a helper method to define mock.On call +func (_e *AggchainFEPRollupQuerier_Expecter) IsFEP() *AggchainFEPRollupQuerier_IsFEP_Call { + return &AggchainFEPRollupQuerier_IsFEP_Call{Call: _e.mock.On("IsFEP")} +} + +func (_c *AggchainFEPRollupQuerier_IsFEP_Call) Run(run func()) *AggchainFEPRollupQuerier_IsFEP_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *AggchainFEPRollupQuerier_IsFEP_Call) Return(_a0 bool) *AggchainFEPRollupQuerier_IsFEP_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *AggchainFEPRollupQuerier_IsFEP_Call) RunAndReturn(run func() bool) *AggchainFEPRollupQuerier_IsFEP_Call { + _c.Call.Return(run) + return _c +} + // StartL2Block provides a mock function with no fields func (_m *AggchainFEPRollupQuerier) StartL2Block() uint64 { ret := _m.Called() diff --git a/aggsender/mocks/mock_certificate_querier.go b/aggsender/mocks/mock_certificate_querier.go index 235318d99..4edcc2512 100644 --- a/aggsender/mocks/mock_certificate_querier.go +++ b/aggsender/mocks/mock_certificate_querier.go @@ -5,11 +5,11 @@ package mocks import ( context "context" - agglayertypes "github.com/agglayer/aggkit/agglayer/types" + aggsendertypes "github.com/agglayer/aggkit/aggsender/types" mock "github.com/stretchr/testify/mock" - types "github.com/agglayer/aggkit/aggsender/types" + types "github.com/agglayer/aggkit/agglayer/types" ) // CertificateQuerier is an autogenerated mock type for the CertificateQuerier type @@ -25,19 +25,19 @@ func (_m *CertificateQuerier) EXPECT() *CertificateQuerier_Expecter { return &CertificateQuerier_Expecter{mock: &_m.Mock} } -// CalculateCertificateType provides a mock function with given fields: certToBlock -func (_m *CertificateQuerier) CalculateCertificateType(certToBlock uint64) types.CertificateType { - ret := _m.Called(certToBlock) +// CalculateCertificateType provides a mock function with given fields: cert, certToBlock +func (_m *CertificateQuerier) CalculateCertificateType(cert *types.Certificate, certToBlock uint64) aggsendertypes.CertificateType { + ret := _m.Called(cert, certToBlock) if len(ret) == 0 { panic("no return value specified for CalculateCertificateType") } - var r0 types.CertificateType - if rf, ok := ret.Get(0).(func(uint64) types.CertificateType); ok { - r0 = rf(certToBlock) + var r0 aggsendertypes.CertificateType + if rf, ok := ret.Get(0).(func(*types.Certificate, uint64) aggsendertypes.CertificateType); ok { + r0 = rf(cert, certToBlock) } else { - r0 = ret.Get(0).(types.CertificateType) + r0 = ret.Get(0).(aggsendertypes.CertificateType) } return r0 @@ -49,30 +49,77 @@ type CertificateQuerier_CalculateCertificateType_Call struct { } // CalculateCertificateType is a helper method to define mock.On call +// - cert *types.Certificate +// - certToBlock uint64 +func (_e *CertificateQuerier_Expecter) CalculateCertificateType(cert interface{}, certToBlock interface{}) *CertificateQuerier_CalculateCertificateType_Call { + return &CertificateQuerier_CalculateCertificateType_Call{Call: _e.mock.On("CalculateCertificateType", cert, certToBlock)} +} + +func (_c *CertificateQuerier_CalculateCertificateType_Call) Run(run func(cert *types.Certificate, certToBlock uint64)) *CertificateQuerier_CalculateCertificateType_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(*types.Certificate), args[1].(uint64)) + }) + return _c +} + +func (_c *CertificateQuerier_CalculateCertificateType_Call) Return(_a0 aggsendertypes.CertificateType) *CertificateQuerier_CalculateCertificateType_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *CertificateQuerier_CalculateCertificateType_Call) RunAndReturn(run func(*types.Certificate, uint64) aggsendertypes.CertificateType) *CertificateQuerier_CalculateCertificateType_Call { + _c.Call.Return(run) + return _c +} + +// CalculateCertificateTypeFromToBlock provides a mock function with given fields: certToBlock +func (_m *CertificateQuerier) CalculateCertificateTypeFromToBlock(certToBlock uint64) aggsendertypes.CertificateType { + ret := _m.Called(certToBlock) + + if len(ret) == 0 { + panic("no return value specified for CalculateCertificateTypeFromToBlock") + } + + var r0 aggsendertypes.CertificateType + if rf, ok := ret.Get(0).(func(uint64) aggsendertypes.CertificateType); ok { + r0 = rf(certToBlock) + } else { + r0 = ret.Get(0).(aggsendertypes.CertificateType) + } + + return r0 +} + +// CertificateQuerier_CalculateCertificateTypeFromToBlock_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CalculateCertificateTypeFromToBlock' +type CertificateQuerier_CalculateCertificateTypeFromToBlock_Call struct { + *mock.Call +} + +// CalculateCertificateTypeFromToBlock is a helper method to define mock.On call // - certToBlock uint64 -func (_e *CertificateQuerier_Expecter) CalculateCertificateType(certToBlock interface{}) *CertificateQuerier_CalculateCertificateType_Call { - return &CertificateQuerier_CalculateCertificateType_Call{Call: _e.mock.On("CalculateCertificateType", certToBlock)} +func (_e *CertificateQuerier_Expecter) CalculateCertificateTypeFromToBlock(certToBlock interface{}) *CertificateQuerier_CalculateCertificateTypeFromToBlock_Call { + return &CertificateQuerier_CalculateCertificateTypeFromToBlock_Call{Call: _e.mock.On("CalculateCertificateTypeFromToBlock", certToBlock)} } -func (_c *CertificateQuerier_CalculateCertificateType_Call) Run(run func(certToBlock uint64)) *CertificateQuerier_CalculateCertificateType_Call { +func (_c *CertificateQuerier_CalculateCertificateTypeFromToBlock_Call) Run(run func(certToBlock uint64)) *CertificateQuerier_CalculateCertificateTypeFromToBlock_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(uint64)) }) return _c } -func (_c *CertificateQuerier_CalculateCertificateType_Call) Return(_a0 types.CertificateType) *CertificateQuerier_CalculateCertificateType_Call { +func (_c *CertificateQuerier_CalculateCertificateTypeFromToBlock_Call) Return(_a0 aggsendertypes.CertificateType) *CertificateQuerier_CalculateCertificateTypeFromToBlock_Call { _c.Call.Return(_a0) return _c } -func (_c *CertificateQuerier_CalculateCertificateType_Call) RunAndReturn(run func(uint64) types.CertificateType) *CertificateQuerier_CalculateCertificateType_Call { +func (_c *CertificateQuerier_CalculateCertificateTypeFromToBlock_Call) RunAndReturn(run func(uint64) aggsendertypes.CertificateType) *CertificateQuerier_CalculateCertificateTypeFromToBlock_Call { _c.Call.Return(run) return _c } // GetLastSettledCertificateToBlock provides a mock function with given fields: ctx, cert -func (_m *CertificateQuerier) GetLastSettledCertificateToBlock(ctx context.Context, cert *agglayertypes.CertificateHeader) (uint64, error) { +func (_m *CertificateQuerier) GetLastSettledCertificateToBlock(ctx context.Context, cert *types.CertificateHeader) (uint64, error) { ret := _m.Called(ctx, cert) if len(ret) == 0 { @@ -81,16 +128,16 @@ func (_m *CertificateQuerier) GetLastSettledCertificateToBlock(ctx context.Conte var r0 uint64 var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *agglayertypes.CertificateHeader) (uint64, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, *types.CertificateHeader) (uint64, error)); ok { return rf(ctx, cert) } - if rf, ok := ret.Get(0).(func(context.Context, *agglayertypes.CertificateHeader) uint64); ok { + if rf, ok := ret.Get(0).(func(context.Context, *types.CertificateHeader) uint64); ok { r0 = rf(ctx, cert) } else { r0 = ret.Get(0).(uint64) } - if rf, ok := ret.Get(1).(func(context.Context, *agglayertypes.CertificateHeader) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, *types.CertificateHeader) error); ok { r1 = rf(ctx, cert) } else { r1 = ret.Error(1) @@ -106,14 +153,14 @@ type CertificateQuerier_GetLastSettledCertificateToBlock_Call struct { // GetLastSettledCertificateToBlock is a helper method to define mock.On call // - ctx context.Context -// - cert *agglayertypes.CertificateHeader +// - cert *types.CertificateHeader func (_e *CertificateQuerier_Expecter) GetLastSettledCertificateToBlock(ctx interface{}, cert interface{}) *CertificateQuerier_GetLastSettledCertificateToBlock_Call { return &CertificateQuerier_GetLastSettledCertificateToBlock_Call{Call: _e.mock.On("GetLastSettledCertificateToBlock", ctx, cert)} } -func (_c *CertificateQuerier_GetLastSettledCertificateToBlock_Call) Run(run func(ctx context.Context, cert *agglayertypes.CertificateHeader)) *CertificateQuerier_GetLastSettledCertificateToBlock_Call { +func (_c *CertificateQuerier_GetLastSettledCertificateToBlock_Call) Run(run func(ctx context.Context, cert *types.CertificateHeader)) *CertificateQuerier_GetLastSettledCertificateToBlock_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(*agglayertypes.CertificateHeader)) + run(args[0].(context.Context), args[1].(*types.CertificateHeader)) }) return _c } @@ -123,13 +170,13 @@ func (_c *CertificateQuerier_GetLastSettledCertificateToBlock_Call) Return(_a0 u return _c } -func (_c *CertificateQuerier_GetLastSettledCertificateToBlock_Call) RunAndReturn(run func(context.Context, *agglayertypes.CertificateHeader) (uint64, error)) *CertificateQuerier_GetLastSettledCertificateToBlock_Call { +func (_c *CertificateQuerier_GetLastSettledCertificateToBlock_Call) RunAndReturn(run func(context.Context, *types.CertificateHeader) (uint64, error)) *CertificateQuerier_GetLastSettledCertificateToBlock_Call { _c.Call.Return(run) return _c } // GetNewCertificateToBlock provides a mock function with given fields: ctx, cert -func (_m *CertificateQuerier) GetNewCertificateToBlock(ctx context.Context, cert *agglayertypes.Certificate) (uint64, error) { +func (_m *CertificateQuerier) GetNewCertificateToBlock(ctx context.Context, cert *types.Certificate) (uint64, error) { ret := _m.Called(ctx, cert) if len(ret) == 0 { @@ -138,16 +185,16 @@ func (_m *CertificateQuerier) GetNewCertificateToBlock(ctx context.Context, cert var r0 uint64 var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *agglayertypes.Certificate) (uint64, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, *types.Certificate) (uint64, error)); ok { return rf(ctx, cert) } - if rf, ok := ret.Get(0).(func(context.Context, *agglayertypes.Certificate) uint64); ok { + if rf, ok := ret.Get(0).(func(context.Context, *types.Certificate) uint64); ok { r0 = rf(ctx, cert) } else { r0 = ret.Get(0).(uint64) } - if rf, ok := ret.Get(1).(func(context.Context, *agglayertypes.Certificate) error); ok { + if rf, ok := ret.Get(1).(func(context.Context, *types.Certificate) error); ok { r1 = rf(ctx, cert) } else { r1 = ret.Error(1) @@ -163,14 +210,14 @@ type CertificateQuerier_GetNewCertificateToBlock_Call struct { // GetNewCertificateToBlock is a helper method to define mock.On call // - ctx context.Context -// - cert *agglayertypes.Certificate +// - cert *types.Certificate func (_e *CertificateQuerier_Expecter) GetNewCertificateToBlock(ctx interface{}, cert interface{}) *CertificateQuerier_GetNewCertificateToBlock_Call { return &CertificateQuerier_GetNewCertificateToBlock_Call{Call: _e.mock.On("GetNewCertificateToBlock", ctx, cert)} } -func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) Run(run func(ctx context.Context, cert *agglayertypes.Certificate)) *CertificateQuerier_GetNewCertificateToBlock_Call { +func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) Run(run func(ctx context.Context, cert *types.Certificate)) *CertificateQuerier_GetNewCertificateToBlock_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(*agglayertypes.Certificate)) + run(args[0].(context.Context), args[1].(*types.Certificate)) }) return _c } @@ -180,7 +227,7 @@ func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) Return(_a0 uint64, _ return _c } -func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) RunAndReturn(run func(context.Context, *agglayertypes.Certificate) (uint64, error)) *CertificateQuerier_GetNewCertificateToBlock_Call { +func (_c *CertificateQuerier_GetNewCertificateToBlock_Call) RunAndReturn(run func(context.Context, *types.Certificate) (uint64, error)) *CertificateQuerier_GetNewCertificateToBlock_Call { _c.Call.Return(run) return _c } diff --git a/aggsender/query/aggchain_fep_rollup_query.go b/aggsender/query/aggchain_fep_rollup_query.go index af4afc0bf..3553e4a50 100644 --- a/aggsender/query/aggchain_fep_rollup_query.go +++ b/aggsender/query/aggchain_fep_rollup_query.go @@ -27,6 +27,10 @@ func (n *noOpAggchainFEPRollupQuerier) GetLastSettledL2Block() (uint64, error) { return 0, nil } +func (n *noOpAggchainFEPRollupQuerier) IsFEP() bool { + return false +} + var _ types.AggchainFEPRollupQuerier = (*aggchainFEPRollupQuerier)(nil) // aggchainFEPRollupQuerier encapsulates the necessary information and interfaces required to query @@ -94,6 +98,11 @@ func newAggchainFEPQuerier( }, nil } +// IsFEP returns true if the AggchainFEP querier is for FEP networks, false otherwise. +func (a *aggchainFEPRollupQuerier) IsFEP() bool { + return true +} + // StartL2Block returns the starting L2 block number for the FEP network. func (a *aggchainFEPRollupQuerier) StartL2Block() uint64 { return a.startL2BlockNum diff --git a/aggsender/query/certificate_query.go b/aggsender/query/certificate_query.go index f0edb6f11..4c16c9892 100644 --- a/aggsender/query/certificate_query.go +++ b/aggsender/query/certificate_query.go @@ -139,16 +139,35 @@ func (c *certificateQuerier) GetNewCertificateToBlock( } // CalculateCertificateType determines the type of certificate based on the last block number in certificate -func (c *certificateQuerier) CalculateCertificateType(certToBlock uint64) types.CertificateType { - if certToBlock == 0 { - return types.CertificateTypeUnknown - } +func (c *certificateQuerier) CalculateCertificateType( + cert *agglayertypes.Certificate, certToBlock uint64) types.CertificateType { + if cert.AggchainData != nil { + // if AggchainData is present, we can use it to determine the certificate type + _, ok := cert.AggchainData.(*agglayertypes.AggchainDataProof) + if !ok { + return types.CertificateTypeFEP + } - if certToBlock < c.aggchainFEPQuerier.StartL2Block() { return types.CertificateTypePP } - return types.CertificateTypeFEP + // if AggchainData is not present, must try to determine the type based on the block number + return c.CalculateCertificateTypeFromToBlock(certToBlock) +} + +// CalculateCertificateTypeFromToBlock determines the type of certificate based on the provided ToBlock number +func (c *certificateQuerier) CalculateCertificateTypeFromToBlock(certToBlock uint64) types.CertificateType { + if c.aggchainFEPQuerier.IsFEP() { + // if we are in a FEP network, we can determine the type based on the start of the FEP + if certToBlock < c.aggchainFEPQuerier.StartL2Block() { + // if the certificate is for a block before the start of the FEP, it is a PP certificate + return types.CertificateTypePP + } + // otherwise, it is a FEP certificate + return types.CertificateTypeFEP + } + + return types.CertificateTypePP // Default to PP if not in FEP mode } func (c *certificateQuerier) getBlockNumFromLER(ctx context.Context, localExitRoot common.Hash) (uint64, error) { diff --git a/aggsender/query/certificate_query_test.go b/aggsender/query/certificate_query_test.go index 1f4c23364..44f4a2ed1 100644 --- a/aggsender/query/certificate_query_test.go +++ b/aggsender/query/certificate_query_test.go @@ -194,71 +194,6 @@ func TestGetLastSettledCertificateToBlock(t *testing.T) { } } -func TestCalculateCertificateType(t *testing.T) { - t.Parallel() - - testCases := []struct { - name string - certToBlock uint64 - startL2Block uint64 - expectedType types.CertificateType - }{ - { - name: "zero block returns unknown", - certToBlock: 0, - startL2Block: 100, - expectedType: types.CertificateTypeUnknown, - }, - { - name: "block before start L2 block returns PP", - certToBlock: 50, - startL2Block: 100, - expectedType: types.CertificateTypePP, - }, - { - name: "block equal to start L2 block returns FEP", - certToBlock: 100, - startL2Block: 100, - expectedType: types.CertificateTypeFEP, - }, - { - name: "block after start L2 block returns FEP", - certToBlock: 150, - startL2Block: 100, - expectedType: types.CertificateTypeFEP, - }, - { - name: "start L2 block is zero with non-zero cert block returns FEP", - certToBlock: 50, - startL2Block: 0, - expectedType: types.CertificateTypeFEP, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - mockAggchainFEPQuerier := mocks.NewAggchainFEPRollupQuerier(t) - mockAgglayerClient := agglayermocks.NewAgglayerClientMock(t) - mockL2BridgeSyncer := mocks.NewL2BridgeSyncer(t) - - mockAggchainFEPQuerier.EXPECT().StartL2Block().Return(tc.startL2Block).Maybe() - - certQuerier := NewCertificateQuerier( - mockL2BridgeSyncer, - mockAggchainFEPQuerier, - mockAgglayerClient, - ) - - result := certQuerier.CalculateCertificateType(tc.certToBlock) - require.Equal(t, tc.expectedType, result) - - mockAggchainFEPQuerier.AssertExpectations(t) - }) - } -} - func TestGetNewCertificateToBlock(t *testing.T) { t.Parallel() @@ -437,3 +372,216 @@ func TestGetNewCertificateToBlock(t *testing.T) { }) } } + +func TestCalculateCertificateTypeFromToBlock(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + certToBlock uint64 + isFEP bool + startL2Block uint64 + expectedCertType types.CertificateType + }{ + { + name: "FEP network - certToBlock before startL2Block returns PP", + certToBlock: 50, + isFEP: true, + startL2Block: 100, + expectedCertType: types.CertificateTypePP, + }, + { + name: "FEP network - certToBlock equal to startL2Block returns FEP", + certToBlock: 100, + isFEP: true, + startL2Block: 100, + expectedCertType: types.CertificateTypeFEP, + }, + { + name: "FEP network - certToBlock after startL2Block returns FEP", + certToBlock: 150, + isFEP: true, + startL2Block: 100, + expectedCertType: types.CertificateTypeFEP, + }, + { + name: "FEP network - zero certToBlock and non-zero startL2Block returns PP", + certToBlock: 0, + isFEP: true, + startL2Block: 50, + expectedCertType: types.CertificateTypePP, + }, + { + name: "FEP network - non-zero certToBlock and zero startL2Block returns FEP", + certToBlock: 50, + isFEP: true, + startL2Block: 0, + expectedCertType: types.CertificateTypeFEP, + }, + { + name: "FEP network - both zero values returns FEP", + certToBlock: 0, + isFEP: true, + startL2Block: 0, + expectedCertType: types.CertificateTypeFEP, + }, + { + name: "non-FEP network - any certToBlock returns PP", + certToBlock: 100, + isFEP: false, + startL2Block: 50, // Should not be called + expectedCertType: types.CertificateTypePP, + }, + { + name: "non-FEP network - zero certToBlock returns PP", + certToBlock: 0, + isFEP: false, + startL2Block: 0, // Should not be called + expectedCertType: types.CertificateTypePP, + }, + { + name: "non-FEP network - large certToBlock returns PP", + certToBlock: 999999, + isFEP: false, + startL2Block: 100, // Should not be called + expectedCertType: types.CertificateTypePP, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + mockAggchainFEPQuerier := mocks.NewAggchainFEPRollupQuerier(t) + mockAgglayerClient := agglayermocks.NewAgglayerClientMock(t) + mockL2BridgeSyncer := mocks.NewL2BridgeSyncer(t) + + // Always expect IsFEP call + mockAggchainFEPQuerier.EXPECT().IsFEP().Return(tc.isFEP).Once() + + // Only expect StartL2Block call if IsFEP returns true + if tc.isFEP { + mockAggchainFEPQuerier.EXPECT().StartL2Block().Return(tc.startL2Block).Once() + } + + certQuerier := NewCertificateQuerier( + mockL2BridgeSyncer, + mockAggchainFEPQuerier, + mockAgglayerClient, + ) + + certType := certQuerier.CalculateCertificateTypeFromToBlock(tc.certToBlock) + require.Equal(t, tc.expectedCertType, certType) + + mockAggchainFEPQuerier.AssertExpectations(t) + }) + } +} + +func TestCalculateCertificateType(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + certificate *agglayertypes.Certificate + certToBlock uint64 + isFEP bool + startL2Block uint64 + expectedCertType types.CertificateType + }{ + { + name: "AggchainData is AggchainDataProof - returns PP", + certificate: &agglayertypes.Certificate{ + AggchainData: &agglayertypes.AggchainDataProof{}, + }, + certToBlock: 100, + expectedCertType: types.CertificateTypePP, + }, + { + name: "AggchainData is not AggchainDataProof - returns FEP", + certificate: &agglayertypes.Certificate{ + AggchainData: &agglayertypes.AggchainDataSignature{}, + }, + certToBlock: 100, + expectedCertType: types.CertificateTypeFEP, + }, + { + name: "AggchainData is nil - falls back to CalculateCertificateTypeFromToBlock with FEP network and block before start", + certificate: &agglayertypes.Certificate{ + AggchainData: nil, + }, + certToBlock: 50, + isFEP: true, + startL2Block: 100, + expectedCertType: types.CertificateTypePP, + }, + { + name: "AggchainData is nil - falls back to CalculateCertificateTypeFromToBlock with FEP network and block after start", + certificate: &agglayertypes.Certificate{ + AggchainData: nil, + }, + certToBlock: 150, + isFEP: true, + startL2Block: 100, + expectedCertType: types.CertificateTypeFEP, + }, + { + name: "AggchainData is nil - falls back to CalculateCertificateTypeFromToBlock with non-FEP network", + certificate: &agglayertypes.Certificate{ + AggchainData: nil, + }, + certToBlock: 100, + isFEP: false, + expectedCertType: types.CertificateTypePP, + }, + { + name: "AggchainData is nil - falls back with FEP network and equal blocks", + certificate: &agglayertypes.Certificate{ + AggchainData: nil, + }, + certToBlock: 100, + isFEP: true, + startL2Block: 100, + expectedCertType: types.CertificateTypeFEP, + }, + { + name: "AggchainData is nil - falls back with zero certToBlock in FEP network", + certificate: &agglayertypes.Certificate{ + AggchainData: nil, + }, + certToBlock: 0, + isFEP: true, + startL2Block: 50, + expectedCertType: types.CertificateTypePP, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + mockAggchainFEPQuerier := mocks.NewAggchainFEPRollupQuerier(t) + mockAgglayerClient := agglayermocks.NewAgglayerClientMock(t) + mockL2BridgeSyncer := mocks.NewL2BridgeSyncer(t) + + // Only expect calls to querier if AggchainData is nil (fallback case) + if tc.certificate.AggchainData == nil { + mockAggchainFEPQuerier.EXPECT().IsFEP().Return(tc.isFEP).Once() + if tc.isFEP { + mockAggchainFEPQuerier.EXPECT().StartL2Block().Return(tc.startL2Block).Once() + } + } + + certQuerier := NewCertificateQuerier( + mockL2BridgeSyncer, + mockAggchainFEPQuerier, + mockAgglayerClient, + ) + + certType := certQuerier.CalculateCertificateType(tc.certificate, tc.certToBlock) + require.Equal(t, tc.expectedCertType, certType) + + mockAggchainFEPQuerier.AssertExpectations(t) + }) + } +} diff --git a/aggsender/statuschecker/cert_status_checker.go b/aggsender/statuschecker/cert_status_checker.go index 62efcda23..0a059338e 100644 --- a/aggsender/statuschecker/cert_status_checker.go +++ b/aggsender/statuschecker/cert_status_checker.go @@ -276,7 +276,7 @@ func (c *certStatusChecker) newSettledCertificateInfoFromAgglayerCertHeader( NewLocalExitRoot: cert.NewLocalExitRoot, Status: cert.Status, CertSource: types.CertificateSourceAggLayer, - CertType: c.certQuerier.CalculateCertificateType(toBlock), + CertType: c.certQuerier.CalculateCertificateTypeFromToBlock(toBlock), ToBlock: toBlock, FromBlock: 0, // We don't have block range in the header and we don't use the metadata anymore CreatedAt: 0, // We don't have creation time in the header and we don't use the metadata anymore diff --git a/aggsender/statuschecker/cert_status_checker_test.go b/aggsender/statuschecker/cert_status_checker_test.go index 31bc0ecef..253a7d61c 100644 --- a/aggsender/statuschecker/cert_status_checker_test.go +++ b/aggsender/statuschecker/cert_status_checker_test.go @@ -176,7 +176,7 @@ func TestNewSettledCertificateInfoFromAgglayerCertHeader(t *testing.T) { NewLocalExitRoot: common.HexToHash("0xabc"), Status: agglayertypes.Settled, }).Return(uint64(200), nil) - m.EXPECT().CalculateCertificateType(uint64(200)).Return(types.CertificateTypePP) + m.EXPECT().CalculateCertificateTypeFromToBlock(uint64(200)).Return(types.CertificateTypePP) }, expectedResult: &types.Certificate{ Header: &types.CertificateHeader{ @@ -273,7 +273,7 @@ func TestUpdateLocalStorageWithAggLayerCert(t *testing.T) { if tt.inputCert != nil { mockCertQuerier.EXPECT().GetLastSettledCertificateToBlock(mock.Anything, tt.inputCert).Return(uint64(100), nil) - mockCertQuerier.EXPECT().CalculateCertificateType(uint64(100)).Return(types.CertificateTypePP) + mockCertQuerier.EXPECT().CalculateCertificateTypeFromToBlock(uint64(100)).Return(types.CertificateTypePP) mockStorage.EXPECT().SaveOrUpdateCertificate(mock.Anything, mock.MatchedBy(func(cert types.Certificate) bool { return cert.Header.CertificateID == tt.expectedResult.Header.CertificateID })).Return(tt.saveError) @@ -356,7 +356,7 @@ func TestExecuteInitialStatusAction(t *testing.T) { }, mockFn: func(mockStorage *mocks.AggSenderStorage, mockCertQuerier *mocks.CertificateQuerier) { mockCertQuerier.EXPECT().GetLastSettledCertificateToBlock(ctx, mock.Anything).Return(uint64(10), nil) - mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) + mockCertQuerier.EXPECT().CalculateCertificateTypeFromToBlock(uint64(10)).Return(types.CertificateTypePP) mockStorage.EXPECT().SaveOrUpdateCertificate(ctx, mock.Anything).Return(nil) }, }, @@ -368,7 +368,7 @@ func TestExecuteInitialStatusAction(t *testing.T) { }, mockFn: func(mockStorage *mocks.AggSenderStorage, mockCertQuerier *mocks.CertificateQuerier) { mockCertQuerier.EXPECT().GetLastSettledCertificateToBlock(ctx, mock.Anything).Return(uint64(10), nil) - mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) + mockCertQuerier.EXPECT().CalculateCertificateTypeFromToBlock(uint64(10)).Return(types.CertificateTypePP) mockStorage.EXPECT().SaveOrUpdateCertificate(ctx, mock.Anything).Return(fmt.Errorf("insert error")) }, expectedError: "recovery: error new local storage with agglayer certificate", diff --git a/aggsender/types/interfaces.go b/aggsender/types/interfaces.go index edf5afa49..223e5e3bb 100644 --- a/aggsender/types/interfaces.go +++ b/aggsender/types/interfaces.go @@ -275,6 +275,7 @@ type AggchainFEPCaller interface { type AggchainFEPRollupQuerier interface { StartL2Block() uint64 GetLastSettledL2Block() (uint64, error) + IsFEP() bool } // CertificateQuerier is an interface defining functions that a CertificateQuerier should implement @@ -285,5 +286,6 @@ type CertificateQuerier interface { GetNewCertificateToBlock( ctx context.Context, cert *agglayertypes.Certificate) (uint64, error) - CalculateCertificateType(certToBlock uint64) CertificateType + CalculateCertificateType(cert *agglayertypes.Certificate, certToBlock uint64) CertificateType + CalculateCertificateTypeFromToBlock(certToBlock uint64) CertificateType } diff --git a/aggsender/validator/validate_certificate.go b/aggsender/validator/validate_certificate.go index b1bab3368..44fcec886 100644 --- a/aggsender/validator/validate_certificate.go +++ b/aggsender/validator/validate_certificate.go @@ -216,7 +216,7 @@ func (a *CertificateValidator) getCertificatePreBuildParams( } blockRange := types.NewBlockRange(previousCertToBlock+1, params.LastL2BlockInCert) - certType := a.certQuerier.CalculateCertificateType(params.LastL2BlockInCert) + certType := a.certQuerier.CalculateCertificateType(params.Certificate, params.LastL2BlockInCert) l1InfoRoot, err := a.l1InfoTreeDataQuerier.GetL1InfoRootByLeafIndex(ctx, params.Certificate.L1InfoTreeLeafCount-1) if err != nil { diff --git a/aggsender/validator/validate_certificate_test.go b/aggsender/validator/validate_certificate_test.go index 80570c066..27a8bd50b 100644 --- a/aggsender/validator/validate_certificate_test.go +++ b/aggsender/validator/validate_certificate_test.go @@ -134,7 +134,7 @@ func TestValidateCertificate(t *testing.T) { testData := newTestDataCertificateValidator(t) testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) - testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) + testData.mockCertQuerier.EXPECT().CalculateCertificateType(mock.Anything, uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). GetL1InfoRootByLeafIndex(testData.ctx, uint32(9)).Return(nil, errGenericForTesting) err := testData.sut.ValidateCertificate(testData.ctx, types.VerifyIncomingRequest{ @@ -154,7 +154,7 @@ func TestValidateCertificate(t *testing.T) { testData := newTestDataCertificateValidator(t) testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) - testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) + testData.mockCertQuerier.EXPECT().CalculateCertificateType(mock.Anything, uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). GetL1InfoRootByLeafIndex(testData.ctx, uint32(9)).Return(&testTreeRootIndex9, nil).Maybe() testData.mockFlow.EXPECT(). @@ -176,7 +176,7 @@ func TestValidateCertificate(t *testing.T) { testData := newTestDataCertificateValidator(t) testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) - testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) + testData.mockCertQuerier.EXPECT().CalculateCertificateType(mock.Anything, uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). GetL1InfoRootByLeafIndex(testData.ctx, uint32(9)).Return(&testTreeRootIndex9, nil).Maybe() testData.mockFlow.EXPECT(). @@ -200,7 +200,7 @@ func TestValidateCertificate(t *testing.T) { testData := newTestDataCertificateValidator(t) testData.mockCertQuerier.EXPECT().GetNewCertificateToBlock(testData.ctx, mock.Anything).Return(uint64(10), nil) testData.mockLERQuerier.EXPECT().GetLastLocalExitRoot().Return(types.EmptyLER, nil) - testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(10)).Return(types.CertificateTypePP) + testData.mockCertQuerier.EXPECT().CalculateCertificateType(mock.Anything, uint64(10)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). GetL1InfoRootByLeafIndex(testData.ctx, uint32(9)).Return(&testTreeRootIndex9, nil).Maybe() testData.mockFlow.EXPECT(). @@ -303,7 +303,7 @@ func TestGetCertificatePreBuildParams(t *testing.T) { t.Run("fails GetL1InfoRootByLeafIndex", func(t *testing.T) { testData := newTestDataCertificateValidator(t) - testData.mockCertQuerier.EXPECT().CalculateCertificateType(uint64(20)).Return(types.CertificateTypePP) + testData.mockCertQuerier.EXPECT().CalculateCertificateType(mock.Anything, uint64(20)).Return(types.CertificateTypePP) testData.mockL1InfoTreeQuerier.EXPECT(). GetL1InfoRootByLeafIndex(testData.ctx, uint32(9)).Return(nil, errGenericForTesting) _, err := testData.sut.getCertificatePreBuildParams(testData.ctx, types.VerifyIncomingRequest{ From c26ec0fc78d2e5b08ad3c9667e4a78f3af6b5668 Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Wed, 13 Aug 2025 08:50:43 +0200 Subject: [PATCH 4/6] fix: comments --- aggsender/query/certificate_query.go | 35 ++++++++++++--------- aggsender/query/certificate_query_test.go | 18 ++--------- aggsender/validator/validate_certificate.go | 2 +- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/aggsender/query/certificate_query.go b/aggsender/query/certificate_query.go index 4c16c9892..d36ab8500 100644 --- a/aggsender/query/certificate_query.go +++ b/aggsender/query/certificate_query.go @@ -71,25 +71,23 @@ func (c *certificateQuerier) GetLastSettledCertificateToBlock( // to determine the last bridge exit block lastBridgeExitBlock, err = c.getBlockNumFromLER(ctx, cert.NewLocalExitRoot) if err != nil { - return 0, fmt.Errorf("failed to get exit root by hash using NewLocalExitRoot %s: %w", + return 0, fmt.Errorf("failed to resolve the bridge exit block number for NewLocalExitRoot %s: %w", cert.NewLocalExitRoot.String(), err) } // TODO - this might need to be changed once agglayer gives support for this // 2. Get the latest settled imported bridge exit block number - latestSettledIbe, err := c.agglayerClient.GetLatestSettledImportedBridgeExit(ctx) + latestSettledIbeGlobalIndex, err := c.agglayerClient.GetLatestSettledImportedBridgeExit(ctx) if err != nil { return 0, fmt.Errorf("failed to get latest settled imported bridge exit from agglayer: %w", err) } - if latestSettledIbe != nil { - bigGlobalIndex := latestSettledIbe.ToBigInt() - claim, err := c.l2BridgeSyncer.GetClaimByGlobalIndex(ctx, bigGlobalIndex) + if latestSettledIbeGlobalIndex != nil { + lastImportedBridgeExitBlock, err = c.getBlockNumFromGlobalIndex(ctx, latestSettledIbeGlobalIndex) if err != nil { - return 0, fmt.Errorf("failed to get claim by global index %s: %w", bigGlobalIndex.String(), err) + return 0, fmt.Errorf("failed to resolve the block number for last imported bridge exit %s: %w", + latestSettledIbeGlobalIndex.String(), err) } - - lastImportedBridgeExitBlock = claim.BlockNum } // 3. Get the last settled L2 block number from aggchain FEP contract @@ -119,20 +117,18 @@ func (c *certificateQuerier) GetNewCertificateToBlock( // to determine the last bridge exit block lastBridgeExitBlock, err = c.getBlockNumFromLER(ctx, cert.NewLocalExitRoot) if err != nil { - return 0, fmt.Errorf("failed to get exit root by hash using NewLocalExitRoot %s: %w", + return 0, fmt.Errorf("failed to resolve the bridge exit block number for NewLocalExitRoot %s: %w", cert.NewLocalExitRoot.String(), err) } if len(cert.ImportedBridgeExits) > 0 { // if there are imported bridge exits, we can use the last one to determine the new certificate to block lastImportedBridgeExit := cert.ImportedBridgeExits[len(cert.ImportedBridgeExits)-1] - bigGlobalIndex := lastImportedBridgeExit.GlobalIndex.ToBigInt() - claim, err := c.l2BridgeSyncer.GetClaimByGlobalIndex(ctx, bigGlobalIndex) + lastImportedBridgeExitBlock, err = c.getBlockNumFromGlobalIndex(ctx, lastImportedBridgeExit.GlobalIndex) if err != nil { - return 0, fmt.Errorf("failed to get claim by global index %s: %w", bigGlobalIndex.String(), err) + return 0, fmt.Errorf("failed to resolve the block number for last imported bridge exit %s: %w", + lastImportedBridgeExit.GlobalIndex.String(), err) } - - lastImportedBridgeExitBlock = claim.BlockNum } return max(lastBridgeExitBlock, lastImportedBridgeExitBlock), nil @@ -183,3 +179,14 @@ func (c *certificateQuerier) getBlockNumFromLER(ctx context.Context, localExitRo return exitRoot.BlockNum, nil } + +func (c *certificateQuerier) getBlockNumFromGlobalIndex( + ctx context.Context, globalIndex *agglayertypes.GlobalIndex) (uint64, error) { + bigGlobalIndex := globalIndex.ToBigInt() + claim, err := c.l2BridgeSyncer.GetClaimByGlobalIndex(ctx, bigGlobalIndex) + if err != nil { + return 0, fmt.Errorf("failed to get claim by global index %s: %w", bigGlobalIndex.String(), err) + } + + return claim.BlockNum, nil +} diff --git a/aggsender/query/certificate_query_test.go b/aggsender/query/certificate_query_test.go index 44f4a2ed1..6775f7eb9 100644 --- a/aggsender/query/certificate_query_test.go +++ b/aggsender/query/certificate_query_test.go @@ -40,21 +40,17 @@ func TestGetLastSettledCertificateToBlock(t *testing.T) { NewLocalExitRoot: common.HexToHash("0x123"), }, mockFn: func(aggchainQuerier *mocks.AggchainFEPRollupQuerier, agglayerClient *agglayermocks.AgglayerClientMock, bridgeSyncer *mocks.L2BridgeSyncer) { - // Mock exit root by hash bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0x123")).Return(&treetypes.Root{ BlockNum: uint64(100), }, nil) - // Mock latest settled imported bridge exit importedBridgeExit := &agglayertypes.GlobalIndex{} agglayerClient.EXPECT().GetLatestSettledImportedBridgeExit(ctx).Return(importedBridgeExit, nil) - // Mock claim by global index bridgeSyncer.EXPECT().GetClaimByGlobalIndex(ctx, importedBridgeExit.ToBigInt()).Return(bridgesync.Claim{ BlockNum: 150, }, nil) - // Mock last settled L2 block aggchainQuerier.EXPECT().GetLastSettledL2Block().Return(uint64(200), nil) }, expectedBlock: 200, // max of 100, 150, 200 @@ -66,16 +62,11 @@ func TestGetLastSettledCertificateToBlock(t *testing.T) { NewLocalExitRoot: types.EmptyLER, }, mockFn: func(aggchainQuerier *mocks.AggchainFEPRollupQuerier, agglayerClient *agglayermocks.AgglayerClientMock, bridgeSyncer *mocks.L2BridgeSyncer) { - // Mock latest settled imported bridge exit importedBridgeExit := &agglayertypes.GlobalIndex{} agglayerClient.EXPECT().GetLatestSettledImportedBridgeExit(ctx).Return(importedBridgeExit, nil) - - // Mock claim by global index bridgeSyncer.EXPECT().GetClaimByGlobalIndex(ctx, importedBridgeExit.ToBigInt()).Return(bridgesync.Claim{ BlockNum: 50, }, nil) - - // Mock last settled L2 block aggchainQuerier.EXPECT().GetLastSettledL2Block().Return(uint64(75), nil) }, expectedBlock: 75, // max of 0, 50, 75 @@ -87,15 +78,10 @@ func TestGetLastSettledCertificateToBlock(t *testing.T) { NewLocalExitRoot: common.HexToHash("0x456"), }, mockFn: func(aggchainQuerier *mocks.AggchainFEPRollupQuerier, agglayerClient *agglayermocks.AgglayerClientMock, bridgeSyncer *mocks.L2BridgeSyncer) { - // Mock exit root by hash bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0x456")).Return(&treetypes.Root{ BlockNum: uint64(300), }, nil) - - // Mock no imported bridge exit agglayerClient.EXPECT().GetLatestSettledImportedBridgeExit(ctx).Return(nil, nil) - - // Mock last settled L2 block aggchainQuerier.EXPECT().GetLastSettledL2Block().Return(uint64(250), nil) }, expectedBlock: 300, // max of 300, 0, 250 @@ -109,7 +95,7 @@ func TestGetLastSettledCertificateToBlock(t *testing.T) { mockFn: func(aggchainQuerier *mocks.AggchainFEPRollupQuerier, agglayerClient *agglayermocks.AgglayerClientMock, bridgeSyncer *mocks.L2BridgeSyncer) { bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0x789")).Return(nil, errors.New("exit root not found")) }, - expectedErr: "failed to get exit root by hash", + expectedErr: "failed to resolve the bridge exit block number for NewLocalExitRoot", }, { name: "error getting latest settled imported bridge exit", @@ -285,7 +271,7 @@ func TestGetNewCertificateToBlock(t *testing.T) { mockFn: func(bridgeSyncer *mocks.L2BridgeSyncer) { bridgeSyncer.EXPECT().GetExitRootByHash(ctx, common.HexToHash("0xabc")).Return(nil, errors.New("exit root not found")) }, - expectedErr: "failed to get exit root by hash using NewLocalExitRoot", + expectedErr: "failed to resolve the bridge exit block number for NewLocalExitRoot", }, { name: "error getting claim by global index", diff --git a/aggsender/validator/validate_certificate.go b/aggsender/validator/validate_certificate.go index 44fcec886..e753ede43 100644 --- a/aggsender/validator/validate_certificate.go +++ b/aggsender/validator/validate_certificate.go @@ -243,7 +243,7 @@ func (a *CertificateValidator) validateLastL2BlockInCert( req types.VerifyIncomingRequest, lastSettledBlock uint64) error { if req.LastL2BlockInCert <= lastSettledBlock { - return fmt.Errorf("last L2 block in certificate %d must be greater than last settled block %d", + return fmt.Errorf("the last L2 block in the certificate (%d) must be greater than the last settled block (%d)", req.LastL2BlockInCert, lastSettledBlock) } From 24fc74782ed72fd4c1903444265abb81cefb910f Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Wed, 13 Aug 2025 08:53:31 +0200 Subject: [PATCH 5/6] fix: comment --- aggsender/validator/validate_certificate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aggsender/validator/validate_certificate.go b/aggsender/validator/validate_certificate.go index e753ede43..7f632f43c 100644 --- a/aggsender/validator/validate_certificate.go +++ b/aggsender/validator/validate_certificate.go @@ -80,7 +80,7 @@ func (a *CertificateValidator) ValidateCertificate(ctx context.Context, params t return fmt.Errorf("failed to validate last L2 block in new certificate: %w", err) } - // Between cert must be no gap because if there are could be a attack vector + // Between cert must be no gap because if there are could be an attack vector if err := a.checkContigousCertificates(params); err != nil { return fmt.Errorf("failed CheckContigousCertificates: %w", err) } From 2bdd48e9e886a6730020ff7ef3c495e77b62474e Mon Sep 17 00:00:00 2001 From: Goran Rojovic Date: Wed, 13 Aug 2025 09:06:22 +0200 Subject: [PATCH 6/6] fix: UT --- aggsender/validator/validate_certificate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aggsender/validator/validate_certificate_test.go b/aggsender/validator/validate_certificate_test.go index 27a8bd50b..777215085 100644 --- a/aggsender/validator/validate_certificate_test.go +++ b/aggsender/validator/validate_certificate_test.go @@ -73,7 +73,7 @@ func TestValidateCertificate(t *testing.T) { LastL2BlockInCert: 10, }) require.Error(t, err) - require.ErrorContains(t, err, "last L2 block in certificate 10 must be greater than last settled block 20") + require.ErrorContains(t, err, "the last L2 block in the certificate (10) must be greater than the last settled block (20)") }) t.Run("first cert bad height", func(t *testing.T) {