Skip to content

Commit

Permalink
feat: PR comment, refactor limitCertSize
Browse files Browse the repository at this point in the history
  • Loading branch information
@joanestebanr
joanestebanr committed Dec 3, 2024
1 parent f2152a9 commit 12c6561
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 117 deletions.
94 changes: 39 additions & 55 deletions aggsender/aggsender.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,25 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
return nil, nil
}

toBlock, bridges, claims, err := a.limitCertSize(ctx, fromBlock, toBlock)
claims, err := a.l2Syncer.GetClaims(ctx, fromBlock, toBlock)
if err != nil {
return nil, fmt.Errorf("error getting claims: %w", err)
}
certificateParams := &types.CertificateLocalParams{
FromBlock: fromBlock,
ToBlock: toBlock,
Bridges: bridges,
Claims: claims,
}

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

certificate, err := a.buildCertificate(ctx, bridges, claims, lastSentCertificateInfo, toBlock)
certificate, err := a.buildCertificate(ctx, certificateParams, lastSentCertificateInfo)
if err != nil {
return nil, fmt.Errorf("error building certificate: %w", err)
}
Expand Down Expand Up @@ -265,57 +276,36 @@ func (a *AggSender) saveCertificateToStorage(ctx context.Context, cert types.Cer
return nil
}

const (
estimatedSizeBridgeExit = 250
estimatedSizeClaim = 44000
)

func (a *AggSender) limitCertSize(ctx context.Context, fromBlock, toBlock uint64) (uint64,
[]bridgesync.Bridge, []bridgesync.Claim, error) {
func (a *AggSender) limitCertSize(fullCert *types.CertificateLocalParams) (*types.CertificateLocalParams, error) {
currentCert := fullCert
var previousCert *types.CertificateLocalParams
var err error
var bridges, previousBridges []bridgesync.Bridge
var claims, previousClaims []bridgesync.Claim
if toBlock < fromBlock {
return toBlock, nil, nil, fmt.Errorf("invalid call, toBlock(%d)<fromBlock(%d)", toBlock, fromBlock)
}
for {
bridges, err = a.l2Syncer.GetBridgesPublished(ctx, fromBlock, toBlock)
if err != nil {
return 0, nil, nil, fmt.Errorf("error getting bridges: %w", err)
}
claims, err = a.l2Syncer.GetClaims(ctx, fromBlock, toBlock)
if err != nil {
return 0, nil, nil, fmt.Errorf("error getting claims: %w", err)
}

if len(bridges) == 0 {
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",
a.estimateSizeCert(len(previousBridges), len(previousClaims)), a.cfg.MaxCertSize)
return toBlock + 1, previousBridges, previousClaims, nil
previousCert.EstimatedSize(), a.cfg.MaxCertSize)
return previousCert, nil
}

if a.cfg.MaxCertSize == 0 || a.estimateSizeCert(len(bridges), len(claims)) < a.cfg.MaxCertSize {
return toBlock, bridges, claims, nil
if a.cfg.MaxCertSize == 0 || currentCert.EstimatedSize() < a.cfg.MaxCertSize {
return currentCert, nil
}

// Minimum size of the certificate
if fromBlock == toBlock {
if currentCert.NumberOfBlocks() <= 1 {
a.log.Warnf("reach the minium num blocks [%d to %d].Certificate size: %d >max size: %d",
fromBlock, toBlock, a.estimateSizeCert(len(bridges), len(claims)), a.cfg.MaxCertSize)
return toBlock, bridges, claims, nil
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)
}

// We have to reduce the number of blocks
toBlock--
previousBridges = bridges
previousClaims = claims
}
}

func (a *AggSender) estimateSizeCert(numBridges, numClaims int) uint {
return uint(numBridges*estimatedSizeBridgeExit + numClaims*estimatedSizeClaim)
}

// 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 @@ -380,25 +370,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,
toBlock uint64) (*agglayer.Certificate, error) {
if len(bridges) == 0 && len(claims) == 0 {
certParams *types.CertificateLocalParams,
lastSentCertificateInfo *types.CertificateInfo) (*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 @@ -417,7 +401,7 @@ func (a *AggSender) buildCertificate(ctx context.Context,
BridgeExits: bridgeExits,
ImportedBridgeExits: importedBridgeExits,
Height: height,
Metadata: createCertificateMetadata(toBlock),
Metadata: createCertificateMetadata(certParams.ToBlock),
}, nil
}

Expand Down
125 changes: 63 additions & 62 deletions aggsender/aggsender_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,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.toBlock)

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

if tt.expectedError {
require.Error(t, err)
Expand Down Expand Up @@ -1917,83 +1923,53 @@ func TestCheckLastCertificateFromAgglayer_Case4ErrorUpdateStatus(t *testing.T) {

func TestLimitSize_FirstOneFit(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
fromBlock := uint64(1)
toBlock := uint64(20)
// It returns 1 bridge
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, fromBlock, toBlock).Return([]bridgesync.Bridge{
{},
}, nil)
testData.l2syncerMock.EXPECT().GetClaims(testData.ctx, fromBlock, toBlock).Return([]bridgesync.Claim{}, nil)
newToBlock, _, _, err := testData.sut.limitCertSize(testData.ctx, fromBlock, toBlock)
certParams := &aggsendertypes.CertificateLocalParams{
FromBlock: uint64(1),
ToBlock: uint64(20),
Bridges: NewBridgesData(1, []uint64{1}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, toBlock, newToBlock)
require.Equal(t, certParams, newCert)
}

func TestLimitSize_FirstMinusOneFit(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.sut.cfg.MaxCertSize = (estimatedSizeBridgeExit * 3) + 1
fromBlock := uint64(1)
toBlock := uint64(20)
// It returns 4 bridge
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, fromBlock, toBlock).Return([]bridgesync.Bridge{
{}, {}, {}, {},
}, nil).Once()

// fromBlock is 1, toBlock is 20, so it will return 3
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, fromBlock, toBlock-1).Return([]bridgesync.Bridge{
{}, {}, {},
}, nil).Once()
testData.l2syncerMock.EXPECT().GetClaims(testData.ctx, fromBlock, mock.Anything).Return([]bridgesync.Claim{}, nil)
newToBlock, _, _, err := testData.sut.limitCertSize(testData.ctx, fromBlock, toBlock)
testData.sut.cfg.MaxCertSize = (aggsendertypes.EstimatedSizeBridgeExit * 3) + 1
certParams := &aggsendertypes.CertificateLocalParams{
FromBlock: uint64(1),
ToBlock: uint64(20),
Bridges: NewBridgesData(4, []uint64{19, 19, 19, 20}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, toBlock-1, newToBlock)
require.Equal(t, uint64(19), newCert.ToBlock)
}

func TestLimitSize_NoWayToFitInMaxSize(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.sut.cfg.MaxCertSize = (estimatedSizeBridgeExit * 2) + 1
fromBlock := uint64(1)
toBlock := uint64(20)
// It returns 4 bridge
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, fromBlock, toBlock).Return([]bridgesync.Bridge{
{}, {}, {}, {},
}, nil).Once()

// fromBlock is 1, toBlock is 19, so it will return 3 that is bigger hat maxSize
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, fromBlock, toBlock-1).Return([]bridgesync.Bridge{
{}, {}, {},
}, nil).Once()

// fromBlock is 1, toBlock is 18, so it will return 0. So must use previous one even that is > maxSize
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, fromBlock, toBlock-2).Return([]bridgesync.Bridge{}, nil).Once()
testData.l2syncerMock.EXPECT().GetClaims(testData.ctx, fromBlock, mock.Anything).Return([]bridgesync.Claim{}, nil)
newToBlock, _, _, err := testData.sut.limitCertSize(testData.ctx, fromBlock, toBlock)
testData.sut.cfg.MaxCertSize = (aggsendertypes.EstimatedSizeBridgeExit * 2) + 1
certParams := &aggsendertypes.CertificateLocalParams{
FromBlock: uint64(1),
ToBlock: uint64(20),
Bridges: NewBridgesData(4, []uint64{19, 19, 19, 20}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, toBlock-1, newToBlock)
}

func TestLimitSize_InvalidCall(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
_, _, _, err := testData.sut.limitCertSize(testData.ctx, 10, 5) //nolint: dogsled
require.Error(t, err)
require.Equal(t, uint64(19), newCert.ToBlock)
}

func TestLimitSize_MinNumBlocks(t *testing.T) {
testData := newAggsenderTestData(t, testDataFlagMockStorage)
testData.sut.cfg.MaxCertSize = (estimatedSizeBridgeExit * 2) + 1
// It returns 4 bridge
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, uint64(1), uint64(2)).Return([]bridgesync.Bridge{
{}, {}, {}, {},
}, nil).Once()

// fromBlock is 1, toBlock is 19, so it will return 3 that is bigger hat maxSize
testData.l2syncerMock.EXPECT().GetBridgesPublished(testData.ctx, uint64(1), uint64(1)).Return([]bridgesync.Bridge{
{}, {}, {},
}, nil).Once()
testData.l2syncerMock.EXPECT().GetClaims(testData.ctx, uint64(1), mock.Anything).Return([]bridgesync.Claim{}, nil)
newToBlock, _, _, err := testData.sut.limitCertSize(testData.ctx, 1, 2)
testData.sut.cfg.MaxCertSize = (aggsendertypes.EstimatedSizeBridgeExit * 2) + 1
certParams := &aggsendertypes.CertificateLocalParams{
FromBlock: uint64(1),
ToBlock: uint64(2),
Bridges: NewBridgesData(6, []uint64{1, 1, 1, 2, 2, 2}),
}
newCert, err := testData.sut.limitCertSize(certParams)
require.NoError(t, err)
require.Equal(t, uint64(1), newToBlock)
require.Equal(t, uint64(1), newCert.ToBlock)
}

type testDataFlags = int
Expand All @@ -2013,6 +1989,31 @@ type aggsenderTestData struct {
testCerts []aggsendertypes.CertificateInfo
}

func NewBridgesData(num int, blockNum []uint64) []bridgesync.Bridge {
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(num int, blockNum []uint64) []bridgesync.Claim {
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

}

Check failure on line 2015 in aggsender/aggsender_test.go

View workflow job for this annotation

GitHub Actions / lint 

unnecessary trailing newline (whitespace)

func certInfoToCertHeader(certInfo *aggsendertypes.CertificateInfo, networkID uint32) *agglayer.CertificateHeader {
if certInfo == nil {
return nil
Expand Down
Loading

0 comments on commit 12c6561

Please sign in to comment.