Skip to content

Commit

Permalink
feat: cherry pick pr 217 and 209 from release/v0.5.0 (#224)
Browse files Browse the repository at this point in the history 
* feat: limit cert by estimated size (#217)

This set a limit on certificate size that can be set on config file (the size of certificate it's an estimation)
There are a special situation that is going to send a certificate bigger than maximium:
- [ initial_block, end_block ] -> size exceed
- [ initial_block, (end_block -1) ] -> no bridges, so we have to send previous one even that exceed the maximum size

MaxCertSize: max size in bytes of certificate. Default 8Mb. Currently the maximum on Agglayer is 10Mb
```
[AggSender]
MaxCertSize = 8388608
```

* feat: return an error in case agglayer returns certificate with height lower than in local storage (#209)


---------

Co-authored-by: Stefan Negovanović <[email protected]>
  • Loading branch information
@joanestebanr @Stefan-Ethernal
joanestebanr and Stefan-Ethernal authored Dec 4, 2024
1 parent de93631 commit f647eda
Show file tree
Hide file tree
Showing 5 changed files with 293 additions and 53 deletions.
96 changes: 67 additions & 29 deletions aggsender/aggsender.go
View file
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
Original file line number Diff line number Diff line change
Expand Up @@ -784,15 +784,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 @@ -995,7 +993,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 @@ -1645,8 +1643,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 @@ -1689,8 +1685,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 @@ -1802,7 +1798,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 @@ -1818,7 +1814,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 @@ -1839,12 +1835,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 @@ -1858,7 +1867,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 @@ -1871,7 +1880,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 @@ -1883,7 +1892,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 @@ -1899,7 +1908,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 @@ -1911,6 +1920,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 @@ -1928,7 +1988,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 @@ -1975,6 +2068,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

0 comments on commit f647eda

Please sign in to comment.