fix L1InfoRoot when an error happens during the process of the L1 inf…

…ormation (0xPolygonHermez#3576)

* fix

* Comments + mock

* avoid error from some L1providers when fromBlock is higher than toBlock

* Revert some changes

* comments

* add L2BlockModulus to L1check

* doc

* fix dbTx = nil

* fix unit tests

config/default.go

@@ -105,7 +105,7 @@ TrustedSequencerURL = "" # If it is empty or not specified, then the value is re
 SyncBlockProtection = "safe" # latest, finalized, safe
 L1SynchronizationMode = "sequential"
 L1SyncCheckL2BlockHash = true
-L1SyncCheckL2BlockNumberhModulus = 30
+L1SyncCheckL2BlockNumberhModulus = 600
 	[Synchronizer.L1BlockCheck]
 		Enable = true
 		L1SafeBlockPoint = "finalized"

docs/config-file/node-config-doc.md

@@ -1433,15 +1433,15 @@ L1SyncCheckL2BlockHash=true
 
 **Type:** : `integer`
 
-**Default:** `30`
+**Default:** `600`
 
 **Description:** L1SyncCheckL2BlockNumberhModulus is the modulus used to choose the l2block to check
 a modules 5, for instance, means check all l2block multiples of 5 (10,15,20,...)
 
-**Example setting the default value** (30):
+**Example setting the default value** (600):
 ```
 [Synchronizer]
-L1SyncCheckL2BlockNumberhModulus=30
+L1SyncCheckL2BlockNumberhModulus=600
 ```
 
 ### <a name="Synchronizer_L1BlockCheck"></a>9.7. `[Synchronizer.L1BlockCheck]`

docs/config-file/node-config-schema.json

@@ -530,7 +530,7 @@
 				"L1SyncCheckL2BlockNumberhModulus": {
 					"type": "integer",
 					"description": "L1SyncCheckL2BlockNumberhModulus is the modulus used to choose the l2block to check\na modules 5, for instance, means check all l2block multiples of 5 (10,15,20,...)",
-					"default": 30
+					"default": 600
 				},
 				"L1BlockCheck": {
 					"properties": {

state/l1infotree.go

@@ -3,6 +3,7 @@ package state
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/0xPolygonHermez/zkevm-node/l1infotree"
 	"github.com/0xPolygonHermez/zkevm-node/log"
@@ -54,6 +55,14 @@ func (s *State) buildL1InfoTreeCacheIfNeed(ctx context.Context, dbTx pgx.Tx) err
 
 // AddL1InfoTreeLeaf adds a new leaf to the L1InfoTree and returns the entry and error
 func (s *State) AddL1InfoTreeLeaf(ctx context.Context, l1InfoTreeLeaf *L1InfoTreeLeaf, dbTx pgx.Tx) (*L1InfoTreeExitRootStorageEntry, error) {
+	var stateTx *StateTx
+	if dbTx != nil {
+		var ok bool
+		stateTx, ok = dbTx.(*StateTx)
+		if !ok {
+			return nil, fmt.Errorf("error casting dbTx to stateTx")
+		}
+	}
 	var newIndex uint32
 	gerIndex, err := s.GetLatestIndex(ctx, dbTx)
 	if err != nil && !errors.Is(err, ErrNotFound) {
@@ -73,6 +82,9 @@ func (s *State) AddL1InfoTreeLeaf(ctx context.Context, l1InfoTreeLeaf *L1InfoTre
 		log.Error("error add new leaf to the L1InfoTree. Error: ", err)
 		return nil, err
 	}
+	if stateTx != nil {
+		stateTx.SetL1InfoTreeModified()
+	}
 	entry := L1InfoTreeExitRootStorageEntry{
 		L1InfoTreeLeaf:  *l1InfoTreeLeaf,
 		L1InfoTreeRoot:  root,

state/reset.go

@@ -18,10 +18,15 @@ func (s *State) Reset(ctx context.Context, blockNumber uint64, dbTx pgx.Tx) erro
 		log.Error("error resetting L1BlockNumber. Error: ", err)
 		return err
 	}
+	s.ResetL1InfoTree()
+	return nil
+}
+
+// ResetL1InfoTree resets the L1InfoTree
+func (s *State) ResetL1InfoTree() {
 	// Discard L1InfoTree cache
 	// We can't rebuild cache, because we are inside a transaction, so we dont known
 	// is going to be a commit or a rollback. So is going to be rebuild on the next
 	// request that needs it.
 	s.l1InfoTree = nil
-	return nil
 }

state/state.go

@@ -62,13 +62,37 @@ func NewState(cfg Config, storage storage, executorClient executor.ExecutorServi
 	return state
 }
 
+// StateTx is the state transaction that extends the database tx
+type StateTx struct {
+	pgx.Tx
+	stateInstance      *State
+	L1InfoTreeModified bool
+}
+
 // BeginStateTransaction starts a state transaction
 func (s *State) BeginStateTransaction(ctx context.Context) (pgx.Tx, error) {
 	tx, err := s.Begin(ctx)
 	if err != nil {
 		return nil, err
 	}
-	return tx, nil
+	res := &StateTx{
+		Tx:            tx,
+		stateInstance: s,
+	}
+	return res, nil
+}
+
+// Rollback do the dbTx rollback + modifications in cache mechanism
+func (tx *StateTx) Rollback(ctx context.Context) error {
+	if tx.L1InfoTreeModified {
+		tx.stateInstance.ResetL1InfoTree()
+	}
+	return tx.Tx.Rollback(ctx)
+}
+
+// SetL1InfoTreeModified sets the flag to true to save that the L1InfoTree has been modified
+func (tx *StateTx) SetL1InfoTreeModified() {
+	tx.L1InfoTreeModified = true
 }
 
 // GetBalance from a given address

synchronizer/actions/check_l2block.go

@@ -36,13 +36,16 @@ type CheckL2BlockHash struct {
 func NewCheckL2BlockHash(state stateGetL2Block,
 	trustedClient trustedRPCGetL2Block,
 	initialL2BlockNumber uint64,
-	modulusBlockNumber uint64) *CheckL2BlockHash {
+	modulusBlockNumber uint64) (*CheckL2BlockHash, error) {
+	if modulusBlockNumber == 0 {
+		return nil, fmt.Errorf("error: modulusBlockNumber is zero")
+	}
 	return &CheckL2BlockHash{
 		state:                 state,
 		trustedClient:         trustedClient,
 		lastL2BlockChecked:    initialL2BlockNumber,
 		modulusL2BlockToCheck: modulusBlockNumber,
-	}
+	}, nil
 }
 
 // CheckL2Block checks the  L2Block hash between the local and the trusted
@@ -74,6 +77,9 @@ func (p *CheckL2BlockHash) GetNextL2BlockToCheck(lastLocalL2BlockNumber, minL2Bl
 		log.Infof("checkL2block: skip check L2block (next to check: %d) currently LastL2BlockNumber: %d", minL2BlockNumberToCheck, lastLocalL2BlockNumber)
 		return false, 0
 	}
+	if l2BlockNumber%p.modulusL2BlockToCheck != 0 {
+		return false, 0
+	}
 	return true, l2BlockNumber
 }
 
@@ -95,7 +101,7 @@ func (p *CheckL2BlockHash) GetL2Blocks(ctx context.Context, blockNumber uint64,
 	trustedL2Block, err := p.trustedClient.BlockByNumber(ctx, big.NewInt(int64(blockNumber)))
 	if err != nil {
 		log.Errorf("checkL2block: Error getting L2Block %d from the Trusted RPC. err:%s", blockNumber, err.Error())
-		return nil, nil, err
+		return nil, nil, nil
 	}
 	return localL2Block, trustedL2Block, nil
 }

synchronizer/actions/check_l2block_test.go

@@ -32,12 +32,15 @@ func TestCheckL2BlockHash_GetMinimumL2BlockToCheck(t *testing.T) {
 		{1, 10, 10},
 		{9, 10, 10},
 		{10, 10, 20},
-		{0, 0, 1},
-		{1, 0, 2},
+		{0, 1, 1},
+		{1, 1, 2},
 	}
+	_, err := actions.NewCheckL2BlockHash(nil, nil, 1, 0)
+	require.Error(t, err)
 	for _, data := range values {
 		// Call the GetNextL2BlockToCheck method
-		checkL2Block := actions.NewCheckL2BlockHash(nil, nil, data.initial, data.modulus)
+		checkL2Block, err := actions.NewCheckL2BlockHash(nil, nil, data.initial, data.modulus)
+		require.NoError(t, err)
 		nextL2Block := checkL2Block.GetMinimumL2BlockToCheck()
 
 		// Assert the expected result
@@ -58,7 +61,9 @@ func newCheckL2BlocksTestData(t *testing.T, initialL2Block, modulus uint64) Chec
 		mockState:   mock_syncinterfaces.NewStateFullInterface(t),
 		zKEVMClient: mock_syncinterfaces.NewZKEVMClientEthereumCompatibleInterface(t),
 	}
-	res.sut = actions.NewCheckL2BlockHash(res.mockState, res.zKEVMClient, initialL2Block, modulus)
+	var err error
+	res.sut, err = actions.NewCheckL2BlockHash(res.mockState, res.zKEVMClient, initialL2Block, modulus)
+	require.NoError(t, err)
 	return res
 }
 func TestCheckL2BlockHash_GetNextL2BlockToCheck(t *testing.T) {
@@ -77,7 +82,8 @@ func TestCheckL2BlockHash_GetNextL2BlockToCheck(t *testing.T) {
 	}
 
 	for _, data := range values {
-		checkL2Block := actions.NewCheckL2BlockHash(nil, nil, 0, 0)
+		checkL2Block, err := actions.NewCheckL2BlockHash(nil, nil, 0, 1)
+		require.NoError(t, err)
 		shouldCheck, nextL2Block := checkL2Block.GetNextL2BlockToCheck(data.lastLocalL2BlockNumber, data.minL2BlockNumberToCheck)
 
 		assert.Equal(t, data.expectedShouldCheck, shouldCheck, data)
@@ -86,7 +92,7 @@ func TestCheckL2BlockHash_GetNextL2BlockToCheck(t *testing.T) {
 }
 
 func TestCheckL2BlockHashMatch(t *testing.T) {
-	data := newCheckL2BlocksTestData(t, 1, 10)
+	data := newCheckL2BlocksTestData(t, 1, 14)
 	lastL2Block := uint64(14)
 	lastL2BlockBigInt := big.NewInt(int64(lastL2Block))
 	gethHeader := types.Header{
@@ -113,7 +119,7 @@ func TestCheckL2BlockHashMatch(t *testing.T) {
 }
 
 func TestCheckL2BlockHashMismatch(t *testing.T) {
-	data := newCheckL2BlocksTestData(t, 1, 10)
+	data := newCheckL2BlocksTestData(t, 1, 14)
 	lastL2Block := uint64(14)
 	lastL2BlockBigInt := big.NewInt(int64(lastL2Block))
 	gethHeader := types.Header{

synchronizer/synchronizer.go

@@ -183,7 +183,11 @@ func NewSynchronizer(
 				log.Errorf("error getting last L2Block number from state. Error: %v", err)
 				return nil, err
 			}
-			l1checkerL2Blocks = actions.NewCheckL2BlockHash(res.state, res.zkEVMClientEthereumCompatible, initialL2Block, cfg.L1SyncCheckL2BlockNumberhModulus)
+			l1checkerL2Blocks, err = actions.NewCheckL2BlockHash(res.state, res.zkEVMClientEthereumCompatible, initialL2Block, cfg.L1SyncCheckL2BlockNumberhModulus)
+			if err != nil {
+				log.Error("error creating new instance of checkL2BlockHash. Error: ", err)
+				return nil, err
+			}
 		} else {
 			log.Infof("Trusted Node can't check L2Block hash, ignoring parameter")
 		}
@@ -604,8 +608,13 @@ func (s *ClientSynchronizer) syncBlocksSequential(lastEthBlockSynced *state.Bloc
 
 	for {
 		if toBlock > lastKnownBlock.Uint64() {
+			log.Debug("Setting toBlock to the lastKnownBlock")
 			toBlock = lastKnownBlock.Uint64()
 		}
+		if fromBlock > toBlock {
+			log.Debug("FromBlock is higher than toBlock. Skipping...")
+			return lastEthBlockSynced, nil
+		}
 		log.Infof("Syncing block %d of %d", fromBlock, lastKnownBlock.Uint64())
 		log.Infof("Getting rollup info from block %d to block %d", fromBlock, toBlock)
 		// This function returns the rollup information contained in the ethereum blocks and an extra param called order.
@@ -730,6 +739,7 @@ func (s *ClientSynchronizer) ProcessBlockRange(blocks []etherman.Block, order ma
 		// Add block information
 		err = s.state.AddBlock(s.ctx, &b, dbTx)
 		if err != nil {
+			// If any goes wrong we ensure that the state is rollbacked
 			log.Errorf("error storing block. BlockNumber: %d, error: %v", blocks[i].BlockNumber, err)
 			rollbackErr := dbTx.Rollback(s.ctx)
 			if rollbackErr != nil {
@@ -766,6 +776,7 @@ func (s *ClientSynchronizer) ProcessBlockRange(blocks []etherman.Block, order ma
 		log.Debug("Checking FlushID to commit L1 data to db")
 		err = s.checkFlushID(dbTx)
 		if err != nil {
+			// If any goes wrong we ensure that the state is rollbacked
 			log.Errorf("error checking flushID. Error: %v", err)
 			rollbackErr := dbTx.Rollback(s.ctx)
 			if rollbackErr != nil {
@@ -776,6 +787,7 @@ func (s *ClientSynchronizer) ProcessBlockRange(blocks []etherman.Block, order ma
 		}
 		err = dbTx.Commit(s.ctx)
 		if err != nil {
+			// If any goes wrong we ensure that the state is rollbacked
 			log.Errorf("error committing state to store block. BlockNumber: %d, err: %v", blocks[i].BlockNumber, err)
 			rollbackErr := dbTx.Rollback(s.ctx)
 			if rollbackErr != nil {