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.

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

fix: cdk603 error calculating previousLocalExitRoot #199

Merged
merged 12 commits into from
Nov 27, 2024
20 changes: 19 additions & 1 deletion agglayer/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestExploratoryGetEpochConfiguration(t *testing.T) {

func TestExploratoryGetLatestKnownCertificateHeader(t *testing.T) {
t.Skip("This test is exploratory and should be skipped")
aggLayerClient := NewAggLayerClient("http://localhost:32781")
aggLayerClient := NewAggLayerClient("http://localhost:32843")
cert, err := aggLayerClient.GetLatestKnownCertificateHeader(1)
require.NoError(t, err)
fmt.Print(cert)
Expand Down Expand Up @@ -116,7 +116,9 @@ func TestGetLatestKnownCertificateHeaderOkResponse(t *testing.T) {
cert, err := sut.GetLatestKnownCertificateHeader(1)
require.NotNil(t, cert)
require.NoError(t, err)
require.Nil(t, cert.PreviousLocalExitRoot)
}

func TestGetLatestKnownCertificateHeaderErrorResponse(t *testing.T) {
sut := NewAggLayerClient(testURL)
jSONRPCCall = func(url, method string, params ...interface{}) (rpc.Response, error) {
Expand All @@ -143,3 +145,19 @@ func TestGetLatestKnownCertificateHeaderResponseBadJson(t *testing.T) {
require.Nil(t, cert)
require.Error(t, err)
}

func TestGetLatestKnownCertificateHeaderWithPrevLERResponse(t *testing.T) {
sut := NewAggLayerClient(testURL)
response := rpc.Response{
Result: []byte(`{"network_id":1,"height":0,"epoch_number":223,"certificate_index":0,"certificate_id":"0xf9179d2fbe535814b5a14496e2eed474f49c6131227a9dfc5d2d8caf9e212054","prev_local_exit_root":"0x27ae5ba08d7291c96c8cbddcc148bf48a6d68c7974b94356f53754ef6171d757","new_local_exit_root":"0x7ae06f4a5d0b6da7dd4973fb6ef40d82c9f2680899b3baaf9e564413b59cc160","metadata":"0x00000000000000000000000000000000000000000000000000000000000001a7","status":"Settled"}`),
}
jSONRPCCall = func(url, method string, params ...interface{}) (rpc.Response, error) {
return response, nil
}
cert, err := sut.GetLatestKnownCertificateHeader(1)

require.NoError(t, err)
require.NotNil(t, cert)

require.Equal(t, "0x27ae5ba08d7291c96c8cbddcc148bf48a6d68c7974b94356f53754ef6171d757", cert.PreviousLocalExitRoot.String())
}
35 changes: 21 additions & 14 deletions agglayer/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ const (
Candidate
InError
Settled

nilStr = "nil"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed, agglayer API?

)

var (
Expand Down Expand Up @@ -574,36 +576,41 @@ func (p *GenericPPError) String() string {

// CertificateHeader is the structure returned by the interop_getCertificateHeader RPC call
type CertificateHeader struct {
NetworkID uint32 `json:"network_id"`
Height uint64 `json:"height"`
EpochNumber *uint64 `json:"epoch_number"`
CertificateIndex *uint64 `json:"certificate_index"`
CertificateID common.Hash `json:"certificate_id"`
NewLocalExitRoot common.Hash `json:"new_local_exit_root"`
Status CertificateStatus `json:"status"`
Metadata common.Hash `json:"metadata"`
Error PPError `json:"-"`
NetworkID uint32 `json:"network_id"`
Height uint64 `json:"height"`
EpochNumber *uint64 `json:"epoch_number"`
CertificateIndex *uint64 `json:"certificate_index"`
CertificateID common.Hash `json:"certificate_id"`
PreviousLocalExitRoot *common.Hash `json:"prev_local_exit_root,omitempty"`
NewLocalExitRoot common.Hash `json:"new_local_exit_root"`
Status CertificateStatus `json:"status"`
Metadata common.Hash `json:"metadata"`
Error PPError `json:"-"`
}

// ID returns a string with the ident of this cert (height/certID)
func (c *CertificateHeader) ID() string {
if c == nil {
return "nil"
return nilStr
}
return fmt.Sprintf("%d/%s", c.Height, c.CertificateID.String())
}

func (c *CertificateHeader) String() string {
if c == nil {
return "nil"
return nilStr
}
errors := ""
if c.Error != nil {
errors = c.Error.String()
}

return fmt.Sprintf("Height: %d, CertificateID: %s, NewLocalExitRoot: %s. Status: %s. Errors: [%s]",
c.Height, c.CertificateID.String(), c.NewLocalExitRoot.String(), c.Status.String(), errors)
previousLocalExitRoot := "nil"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the nilStr constant?

if c.PreviousLocalExitRoot != nil {
previousLocalExitRoot = c.PreviousLocalExitRoot.String()
}
return fmt.Sprintf("Height: %d, CertificateID: %s, previousLocalExitRoot:%s, NewLocalExitRoot: %s. Status: %s."+
" Errors: [%s]",
c.Height, c.CertificateID.String(), previousLocalExitRoot, c.NewLocalExitRoot.String(), c.Status.String(), errors)
}

func (c *CertificateHeader) UnmarshalJSON(data []byte) error {
Expand Down
23 changes: 23 additions & 0 deletions agglayer/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,29 @@ func TestMGenericPPError(t *testing.T) {
require.Equal(t, "Generic error: test: value", err.String())
}

func TestCertificateHeaderID(t *testing.T) {
certificate := CertificateHeader{
Height: 1,
CertificateID: common.HexToHash("0x123"),
}
require.Equal(t, "1/0x0000000000000000000000000000000000000000000000000000000000000123", certificate.ID())

var certNil *CertificateHeader
require.Equal(t, "nil", certNil.ID())
}

func TestCertificateHeaderString(t *testing.T) {
certificate := CertificateHeader{
Height: 1,
CertificateID: common.HexToHash("0x123"),
}
require.Equal(t, "Height: 1, CertificateID: 0x0000000000000000000000000000000000000000000000000000000000000123, previousLocalExitRoot:nil, NewLocalExitRoot: 0x0000000000000000000000000000000000000000000000000000000000000000. Status: Pending. Errors: []",
certificate.String())

var certNil *CertificateHeader
require.Equal(t, "nil", certNil.String())
}

func TestMarshalJSON(t *testing.T) {
cert := SignedCertificate{
Certificate: &Certificate{
Expand Down
103 changes: 66 additions & 37 deletions aggsender/aggsender.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,14 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
if err != nil {
return nil, err
}
if lastSentCertificateInfo == nil {
// There are no certificates, so we set that to a empty one
lastSentCertificateInfo = &types.CertificateInfo{}
}

previousToBlock := lastSentCertificateInfo.ToBlock
if lastSentCertificateInfo.Status == agglayer.InError {
// if the last certificate was in error, we need to resend it
// from the block before the error
previousToBlock = lastSentCertificateInfo.FromBlock - 1
previousToBlock := uint64(0)
if lastSentCertificateInfo != nil {
previousToBlock = lastSentCertificateInfo.ToBlock
if lastSentCertificateInfo.Status == agglayer.InError {
// if the last certificate was in error, we need to resend it
// from the block before the error
previousToBlock = lastSentCertificateInfo.FromBlock - 1
}
}

if previousToBlock >= lasL2BlockSynced {
Expand Down Expand Up @@ -190,7 +188,7 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif

a.log.Infof("building certificate for block: %d to block: %d", fromBlock, toBlock)

certificate, err := a.buildCertificate(ctx, bridges, claims, *lastSentCertificateInfo, toBlock)
certificate, err := a.buildCertificate(ctx, bridges, claims, lastSentCertificateInfo, toBlock)
if err != nil {
return nil, fmt.Errorf("error building certificate: %w", err)
}
Expand All @@ -215,15 +213,17 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
}

createdTime := time.Now().UTC().UnixMilli()
prevLER := common.BytesToHash(certificate.PrevLocalExitRoot[:])
certInfo := types.CertificateInfo{
Height: certificate.Height,
CertificateID: certificateHash,
NewLocalExitRoot: certificate.NewLocalExitRoot,
FromBlock: fromBlock,
ToBlock: toBlock,
CreatedAt: createdTime,
UpdatedAt: createdTime,
SignedCertificate: string(raw),
Height: certificate.Height,
CertificateID: certificateHash,
NewLocalExitRoot: certificate.NewLocalExitRoot,
PreviousLocalExitRoot: &prevLER,
FromBlock: fromBlock,
ToBlock: toBlock,
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
err = a.saveCertificateToStorage(ctx, certInfo, a.cfg.MaxRetriesStoreCertificate)
Expand Down Expand Up @@ -278,31 +278,53 @@ func (a *AggSender) saveCertificateToFile(signedCertificate *agglayer.SignedCert

// getNextHeightAndPreviousLER returns the height and previous LER for the new certificate
func (a *AggSender) getNextHeightAndPreviousLER(
lastSentCertificateInfo *types.CertificateInfo) (uint64, common.Hash) {
height := lastSentCertificateInfo.Height + 1
if lastSentCertificateInfo.Status.IsInError() {
// previous certificate was in error, so we need to resend it
a.log.Debugf("Last certificate %s failed so reusing height %d",
lastSentCertificateInfo.CertificateID, lastSentCertificateInfo.Height)
height = lastSentCertificateInfo.Height
lastSentCertificateInfo *types.CertificateInfo) (uint64, common.Hash, error) {
if lastSentCertificateInfo == nil {
return 0, zeroLER, nil
}

previousLER := lastSentCertificateInfo.NewLocalExitRoot
if lastSentCertificateInfo.NewLocalExitRoot == (common.Hash{}) ||
lastSentCertificateInfo.Height == 0 && lastSentCertificateInfo.Status.IsInError() {
// meaning this is the first certificate
height = 0
previousLER = zeroLER
if !lastSentCertificateInfo.Status.IsClosed() {
return 0, zeroLER, fmt.Errorf("last certificate %s is not closed (status: %s)",
lastSentCertificateInfo.ID(), lastSentCertificateInfo.Status.String())
}
if lastSentCertificateInfo.Status.IsSettled() {
return lastSentCertificateInfo.Height + 1, lastSentCertificateInfo.NewLocalExitRoot, nil
}

return height, previousLER
if lastSentCertificateInfo.Status.IsInError() {
// We can reuse last one of lastCert?
if lastSentCertificateInfo.PreviousLocalExitRoot != nil {
return lastSentCertificateInfo.Height, *lastSentCertificateInfo.PreviousLocalExitRoot, nil
}
// Is the first one, so we can set the zeroLER
if lastSentCertificateInfo.Height == 0 {
return 0, zeroLER, nil
}
// We get previous certificate that must be settled
a.log.Debugf("last certificate %s is in error, getting previous settled certificate height:%d",
lastSentCertificateInfo.Height-1)
lastSettleCert, err := a.storage.GetCertificateByHeight(lastSentCertificateInfo.Height - 1)
if err != nil {
return 0, common.Hash{}, fmt.Errorf("error getting last settled certificate: %w", err)
}
if lastSettleCert == nil {
return 0, common.Hash{}, fmt.Errorf("none settled certificate: %w", err)
}
if !lastSettleCert.Status.IsSettled() {
return 0, common.Hash{}, fmt.Errorf("last settled certificate %s is not settled (status: %s)",
lastSettleCert.ID(), lastSettleCert.Status.String())
}

return lastSentCertificateInfo.Height, lastSettleCert.NewLocalExitRoot, nil
}
return 0, zeroLER, fmt.Errorf("last certificate %s has an unknown status: %s",
lastSentCertificateInfo.ID(), lastSentCertificateInfo.Status.String())
}

// buildCertificate builds a certificate from the bridge events
func (a *AggSender) buildCertificate(ctx context.Context,
bridges []bridgesync.Bridge,
claims []bridgesync.Claim,
lastSentCertificateInfo types.CertificateInfo,
lastSentCertificateInfo *types.CertificateInfo,
toBlock uint64) (*agglayer.Certificate, error) {
if len(bridges) == 0 && len(claims) == 0 {
return nil, errNoBridgesAndClaims
Expand All @@ -325,7 +347,10 @@ func (a *AggSender) buildCertificate(ctx context.Context,
return nil, fmt.Errorf("error getting exit root by index: %d. Error: %w", depositCount, err)
}

height, previousLER := a.getNextHeightAndPreviousLER(&lastSentCertificateInfo)
height, previousLER, err := a.getNextHeightAndPreviousLER(lastSentCertificateInfo)
if err != nil {
return nil, fmt.Errorf("error getting next height and previous LER: %w", err)
}

return &agglayer.Certificate{
NetworkID: a.l2Syncer.OriginNetwork(),
Expand Down Expand Up @@ -709,7 +734,7 @@ func NewCertificateInfoFromAgglayerCertHeader(c *agglayer.CertificateHeader) *ty
return nil
}
now := time.Now().UTC().UnixMilli()
return &types.CertificateInfo{
res := &types.CertificateInfo{
Height: c.Height,
CertificateID: c.CertificateID,
NewLocalExitRoot: c.NewLocalExitRoot,
Expand All @@ -720,4 +745,8 @@ func NewCertificateInfoFromAgglayerCertHeader(c *agglayer.CertificateHeader) *ty
UpdatedAt: now,
SignedCertificate: "na/agglayer header",
}
if c.PreviousLocalExitRoot != nil {
res.PreviousLocalExitRoot = c.PreviousLocalExitRoot
}
return res
}
Loading
Loading