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: cherry pick pr 217 and 209 from release/v0.5.0 #224

Merged
merged 5 commits into from
Dec 4, 2024
Merged

feat: cherry pick pr 217 and 209 from release/v0.5.0 #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ceafe0
feat: limit cert by estimated size (#217)
joanestebanr Dec 3, 2024
af863f5
fix: merge conflicts
joanestebanr Dec 4, 2024
807457b
fix: unittest
joanestebanr Dec 4, 2024
9dd0a5e
feat: return an error in case agglayer returns certificate with heigh…
Stefan-Ethernal Nov 29, 2024
d6cea92
fix: unittest
joanestebanr Dec 4, 2024
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
96 changes: 67 additions & 29 deletions aggsender/aggsender.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,22 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
if err != nil {
return nil, fmt.Errorf("error getting claims: %w", err)
}
certificateParams := &types.CertificateBuildParams{
FromBlock: fromBlock,
ToBlock: toBlock,
Bridges: bridges,
Claims: claims,
}

a.log.Infof("building certificate for block: %d to block: %d", fromBlock, toBlock)
certificateParams, err = a.limitCertSize(certificateParams)
if err != nil {
return nil, fmt.Errorf("error limitCertSize: %w", err)
}
a.log.Infof("building certificate for %s estimatedSize=%d",
certificateParams.String(), certificateParams.EstimatedSize())

createdTime := uint64(time.Now().UTC().UnixMilli())
certificate, err := a.buildCertificate(ctx, bridges, claims, lastSentCertificateInfo, fromBlock, toBlock, createdTime)
createdTime := time.Now().UTC().UnixMilli()
certificate, err := a.buildCertificate(ctx, certificateParams, lastSentCertificateInfo, createdTime)
if err != nil {
return nil, fmt.Errorf("error building certificate: %w", err)
}
Expand Down Expand Up @@ -227,8 +238,8 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
PreviousLocalExitRoot: &prevLER,
FromBlock: fromBlock,
ToBlock: toBlock,
CreatedAt: int64(createdTime),
UpdatedAt: int64(createdTime),
CreatedAt: createdTime,
UpdatedAt: createdTime,
SignedCertificate: string(raw),
}
// TODO: Improve this case, if a cert is not save in the storage, we are going to settle a unknown certificate
Expand Down Expand Up @@ -264,6 +275,36 @@ func (a *AggSender) saveCertificateToStorage(ctx context.Context, cert types.Cer
return nil
}

func (a *AggSender) limitCertSize(fullCert *types.CertificateBuildParams) (*types.CertificateBuildParams, error) {
currentCert := fullCert
var previousCert *types.CertificateBuildParams
var err error
for {
if currentCert.NumberOfBridges() == 0 {
// We can't reduce more the certificate, so this is the minium size
a.log.Warnf("We reach the minium size of bridge.Certificate size: %d >max size: %d",
previousCert.EstimatedSize(), a.cfg.MaxCertSize)
return previousCert, nil
}

if a.cfg.MaxCertSize == 0 || currentCert.EstimatedSize() < a.cfg.MaxCertSize {
return currentCert, nil
}

// Minimum size of the certificate
if currentCert.NumberOfBlocks() <= 1 {
a.log.Warnf("reach the minium num blocks [%d to %d].Certificate size: %d >max size: %d",
currentCert.FromBlock, currentCert.ToBlock, currentCert.EstimatedSize(), a.cfg.MaxCertSize)
return currentCert, nil
}
previousCert = currentCert
currentCert, err = currentCert.Range(currentCert.FromBlock, currentCert.ToBlock-1)
if err != nil {
return nil, fmt.Errorf("error reducing certificate: %w", err)
}
}
}

// saveCertificate saves the certificate to a tmp file
func (a *AggSender) saveCertificateToFile(signedCertificate *agglayer.SignedCertificate) {
if signedCertificate == nil || a.cfg.SaveCertificatesToFilesPath == "" {
Expand Down Expand Up @@ -328,27 +369,19 @@ func (a *AggSender) getNextHeightAndPreviousLER(

// buildCertificate builds a certificate from the bridge events
func (a *AggSender) buildCertificate(ctx context.Context,
bridges []bridgesync.Bridge,
claims []bridgesync.Claim,
lastSentCertificateInfo *types.CertificateInfo,
fromBlock, toBlock uint64,
createdAt uint64,
) (*agglayer.Certificate, error) {
if len(bridges) == 0 && len(claims) == 0 {
certParams *types.CertificateBuildParams,
lastSentCertificateInfo *types.CertificateInfo, createdAt int64) (*agglayer.Certificate, error) {
if certParams.IsEmpty() {
return nil, errNoBridgesAndClaims
}

bridgeExits := a.getBridgeExits(bridges)
importedBridgeExits, err := a.getImportedBridgeExits(ctx, claims)
bridgeExits := a.getBridgeExits(certParams.Bridges)
importedBridgeExits, err := a.getImportedBridgeExits(ctx, certParams.Claims)
if err != nil {
return nil, fmt.Errorf("error getting imported bridge exits: %w", err)
}

var depositCount uint32

if len(bridges) > 0 {
depositCount = bridges[len(bridges)-1].DepositCount
}
depositCount := certParams.MaxDepoitCount()

exitRoot, err := a.l2Syncer.GetExitRootByIndex(ctx, depositCount)
if err != nil {
Expand All @@ -361,9 +394,9 @@ func (a *AggSender) buildCertificate(ctx context.Context,
}

meta := &types.CertificateMetadata{
FromBlock: fromBlock,
ToBlock: toBlock,
CreatedAt: createdAt,
FromBlock: certParams.FromBlock,
ToBlock: certParams.ToBlock,
CreatedAt: uint64(createdAt),
}

return &agglayer.Certificate{
Expand Down Expand Up @@ -679,19 +712,24 @@ func (a *AggSender) checkLastCertificateFromAgglayer(ctx context.Context) error
return nil
}
// CASE 2.1: certificate in storage but not in agglayer
// this is a non-sense, so thrown an error
// this is a non-sense, so throw an error
if localLastCert != nil && aggLayerLastCert == nil {
return fmt.Errorf("recovery: certificate in storage but not in agglayer. Inconsistency")
return fmt.Errorf("recovery: certificate exists in storage but not in agglayer. Inconsistency")
}
// CASE 3.1: the certificate on the agglayer has less height than the one stored in the local storage
if aggLayerLastCert.Height < localLastCert.Height {
return fmt.Errorf("recovery: the last certificate in the agglayer has less height (%d) "+
"than the one in the local storage (%d)", aggLayerLastCert.Height, localLastCert.Height)
}
// CASE 3: aggsender stopped between sending to agglayer and storing on DB
// CASE 3.2: aggsender stopped between sending to agglayer and storing to the local storage
if aggLayerLastCert.Height == localLastCert.Height+1 {
a.log.Infof("recovery: AggLayer have next cert (height:%d), so is a recovery case: storing cert: %s",
aggLayerLastCert.Height, localLastCert.String())
a.log.Infof("recovery: AggLayer has the next cert (height: %d), so is a recovery case: storing cert: %s",
aggLayerLastCert.Height, aggLayerLastCert.String())
// we need to store the certificate in the local storage.
localLastCert, err = a.updateLocalStorageWithAggLayerCert(ctx, aggLayerLastCert)
if err != nil {
log.Errorf("recovery: error updating status certificate: %s status: %w", aggLayerLastCert.String(), err)
return fmt.Errorf("recovery: error updating certificate status: %w", err)
log.Errorf("recovery: error updating certificate: %s, reason: %w", aggLayerLastCert.String(), err)
return fmt.Errorf("recovery: error updating certificate: %w", err)
}
}
// CASE 4: AggSender and AggLayer are not on the same page
Expand Down
144 changes: 120 additions & 24 deletions aggsender/aggsender_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,15 +789,13 @@ func TestBuildCertificate(t *testing.T) {
l1infoTreeSyncer: mockL1InfoTreeSyncer,
log: log.WithFields("test", "unittest"),
}
cert, err := aggSender.buildCertificate(
context.Background(),
tt.bridges,
tt.claims,
&tt.lastSentCertificateInfo,
tt.fromBlock,
tt.toBlock,
0,
)

certParam := &aggsendertypes.CertificateBuildParams{
ToBlock: tt.toBlock,
Bridges: tt.bridges,
Claims: tt.claims,
}
cert, err := aggSender.buildCertificate(context.Background(), certParam, &tt.lastSentCertificateInfo, 0)

if tt.expectedError {
require.Error(t, err)
Expand Down Expand Up @@ -1003,7 +1001,7 @@ func TestSendCertificate(t *testing.T) {
mockL2Syncer.On("GetLastProcessedBlock", mock.Anything).Return(cfg.lastL2BlockProcessed...).Once()

if cfg.getBridges != nil {
mockL2Syncer.On("GetBridgesPublished", mock.Anything, mock.Anything, mock.Anything).Return(cfg.getBridges...).Once()
mockL2Syncer.On("GetBridgesPublished", mock.Anything, mock.Anything, mock.Anything).Return(cfg.getBridges...)
}

if cfg.getClaims != nil {
Expand Down Expand Up @@ -1653,8 +1651,6 @@ func TestGetNextHeightAndPreviousLER(t *testing.T) {
}

func TestSendCertificate_NoClaims(t *testing.T) {
t.Parallel()

privateKey, err := crypto.GenerateKey()
require.NoError(t, err)

Expand Down Expand Up @@ -1700,8 +1696,8 @@ func TestSendCertificate_NoClaims(t *testing.T) {
Metadata: []byte("metadata"),
DepositCount: 1,
},
}, nil).Once()
mockL2Syncer.On("GetClaims", mock.Anything, uint64(11), uint64(50)).Return([]bridgesync.Claim{}, nil).Once()
}, nil)
mockL2Syncer.On("GetClaims", mock.Anything, uint64(11), uint64(50)).Return([]bridgesync.Claim{}, nil)
mockL2Syncer.On("GetExitRootByIndex", mock.Anything, uint32(1)).Return(treeTypes.Root{}, nil).Once()
mockL2Syncer.On("OriginNetwork").Return(uint32(1), nil).Once()
mockAggLayerClient.On("SendCertificate", mock.Anything).Return(common.Hash{}, nil).Once()
Expand Down Expand Up @@ -1813,7 +1809,7 @@ func TestCheckLastCertificateFromAgglayer_Case2NoCertLocalCertRemote(t *testing.
testData := newAggsenderTestData(t, testDataFlagNone)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(certInfoToCertHeader(&testData.testCerts[0], networkIDTest), nil).Once()
Return(certInfoToCertHeader(t, &testData.testCerts[0], networkIDTest), nil).Once()

err := testData.sut.checkLastCertificateFromAgglayer(testData.ctx)

Expand All @@ -1829,7 +1825,7 @@ func TestCheckLastCertificateFromAgglayer_Case2NoCertLocalCertRemoteErrorStorage
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(certInfoToCertHeader(&testData.testCerts[0], networkIDTest), nil).Once()
Return(certInfoToCertHeader(t, &testData.testCerts[0], networkIDTest), nil).Once()
testData.storageMock.EXPECT().GetLastSentCertificate().Return(nil, nil)
testData.storageMock.EXPECT().SaveLastSentCertificate(mock.Anything, mock.Anything).Return(errTest).Once()
err := testData.sut.checkLastCertificateFromAgglayer(testData.ctx)
Expand All @@ -1850,12 +1846,25 @@ func TestCheckLastCertificateFromAgglayer_Case2_1NoCertRemoteButCertLocal(t *tes
require.Error(t, err)
}

// CASE 3: AggSender and AggLayer not same certificateID. AggLayer has a new certificate
func TestCheckLastCertificateFromAgglayer_Case3Mismatch(t *testing.T) {
// CASE 3.1: the certificate on the agglayer has less height than the one stored in the local storage
func TestCheckLastCertificateFromAgglayer_Case3_1LessHeight(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(certInfoToCertHeader(&testData.testCerts[1], networkIDTest), nil).Once()
Return(certInfoToCertHeader(t, &testData.testCerts[0], networkIDTest), nil).Once()
testData.storageMock.EXPECT().GetLastSentCertificate().Return(&testData.testCerts[1], nil)

err := testData.sut.checkLastCertificateFromAgglayer(testData.ctx)

require.ErrorContains(t, err, "recovery: the last certificate in the agglayer has less height (1) than the one in the local storage (2)")
}

// CASE 3.2: AggSender and AggLayer not same height. AggLayer has a new certificate
func TestCheckLastCertificateFromAgglayer_Case3_2Mismatch(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(certInfoToCertHeader(t, &testData.testCerts[1], networkIDTest), nil).Once()
testData.storageMock.EXPECT().GetLastSentCertificate().Return(&testData.testCerts[0], nil)
testData.storageMock.EXPECT().SaveLastSentCertificate(mock.Anything, mock.Anything).Return(nil).Once()

Expand All @@ -1869,7 +1878,7 @@ func TestCheckLastCertificateFromAgglayer_Case4Mismatch(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(certInfoToCertHeader(&testData.testCerts[0], networkIDTest), nil).Once()
Return(certInfoToCertHeader(t, &testData.testCerts[0], networkIDTest), nil).Once()
testData.storageMock.EXPECT().GetLastSentCertificate().Return(&testData.testCerts[1], nil)

err := testData.sut.checkLastCertificateFromAgglayer(testData.ctx)
Expand All @@ -1882,7 +1891,7 @@ func TestCheckLastCertificateFromAgglayer_Case5SameStatus(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(certInfoToCertHeader(&testData.testCerts[0], networkIDTest), nil).Once()
Return(certInfoToCertHeader(t, &testData.testCerts[0], networkIDTest), nil).Once()
testData.storageMock.EXPECT().GetLastSentCertificate().Return(&testData.testCerts[0], nil)

err := testData.sut.checkLastCertificateFromAgglayer(testData.ctx)
Expand All @@ -1894,7 +1903,7 @@ func TestCheckLastCertificateFromAgglayer_Case5SameStatus(t *testing.T) {
func TestCheckLastCertificateFromAgglayer_Case5UpdateStatus(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
aggLayerCert := certInfoToCertHeader(&testData.testCerts[0], networkIDTest)
aggLayerCert := certInfoToCertHeader(t, &testData.testCerts[0], networkIDTest)
aggLayerCert.Status = agglayer.Settled
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(aggLayerCert, nil).Once()
Expand All @@ -1910,7 +1919,7 @@ func TestCheckLastCertificateFromAgglayer_Case5UpdateStatus(t *testing.T) {
func TestCheckLastCertificateFromAgglayer_Case4ErrorUpdateStatus(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.l2syncerMock.EXPECT().OriginNetwork().Return(networkIDTest).Once()
aggLayerCert := certInfoToCertHeader(&testData.testCerts[0], networkIDTest)
aggLayerCert := certInfoToCertHeader(t, &testData.testCerts[0], networkIDTest)
aggLayerCert.Status = agglayer.Settled
testData.agglayerClientMock.EXPECT().GetLatestKnownCertificateHeader(networkIDTest).
Return(aggLayerCert, nil).Once()
Expand All @@ -1922,6 +1931,57 @@ func TestCheckLastCertificateFromAgglayer_Case4ErrorUpdateStatus(t *testing.T) {
require.Error(t, err)
}

func TestLimitSize_FirstOneFit(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
certParams := &aggsendertypes.CertificateBuildParams{
FromBlock: uint64(1),
ToBlock: uint64(20),
Bridges: NewBridgesData(t, 1, []uint64{1}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, certParams, newCert)
}

func TestLimitSize_FirstMinusOneFit(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.sut.cfg.MaxCertSize = (aggsendertypes.EstimatedSizeBridgeExit * 3) + 1
certParams := &aggsendertypes.CertificateBuildParams{
FromBlock: uint64(1),
ToBlock: uint64(20),
Bridges: NewBridgesData(t, 0, []uint64{19, 19, 19, 20}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, uint64(19), newCert.ToBlock)
}

func TestLimitSize_NoWayToFitInMaxSize(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.sut.cfg.MaxCertSize = (aggsendertypes.EstimatedSizeBridgeExit * 2) + 1
certParams := &aggsendertypes.CertificateBuildParams{
FromBlock: uint64(1),
ToBlock: uint64(20),
Bridges: NewBridgesData(t, 0, []uint64{19, 19, 19, 20}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, uint64(19), newCert.ToBlock)
}

func TestLimitSize_MinNumBlocks(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.sut.cfg.MaxCertSize = (aggsendertypes.EstimatedSizeBridgeExit * 2) + 1
certParams := &aggsendertypes.CertificateBuildParams{
FromBlock: uint64(1),
ToBlock: uint64(2),
Bridges: NewBridgesData(t, 0, []uint64{1, 1, 1, 2, 2, 2}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, uint64(1), newCert.ToBlock)
}

type testDataFlags = int

const (
Expand All @@ -1939,7 +1999,40 @@ type aggsenderTestData struct {
testCerts []aggsendertypes.CertificateInfo
}

func certInfoToCertHeader(certInfo *aggsendertypes.CertificateInfo, networkID uint32) *agglayer.CertificateHeader {
func NewBridgesData(t *testing.T, num int, blockNum []uint64) []bridgesync.Bridge {
t.Helper()
if num == 0 {
num = len(blockNum)
}
res := make([]bridgesync.Bridge, 0)
for i := 0; i < num; i++ {
res = append(res, bridgesync.Bridge{
BlockNum: blockNum[i%len(blockNum)],
BlockPos: 0,
LeafType: agglayer.LeafTypeAsset.Uint8(),
OriginNetwork: 1,
})
}
return res
}

func NewClaimData(t *testing.T, num int, blockNum []uint64) []bridgesync.Claim {
t.Helper()
if num == 0 {
num = len(blockNum)
}
res := make([]bridgesync.Claim, 0)
for i := 0; i < num; i++ {
res = append(res, bridgesync.Claim{
BlockNum: blockNum[i%len(blockNum)],
BlockPos: 0,
})
}
return res
}

func certInfoToCertHeader(t *testing.T, certInfo *aggsendertypes.CertificateInfo, networkID uint32) *agglayer.CertificateHeader {
t.Helper()
if certInfo == nil {
return nil
}
Expand Down Expand Up @@ -1986,6 +2079,9 @@ func newAggsenderTestData(t *testing.T, creationFlags testDataFlags) *aggsenderT
aggLayerClient: agglayerClientMock,
storage: storage,
l1infoTreeSyncer: l1InfoTreeSyncerMock,
cfg: Config{
MaxCertSize: 1024 * 1024,
},
}
testCerts := []aggsendertypes.CertificateInfo{
{
Expand Down
Loading
Loading