etrog: Fix synchronizer OOC sequenced batch (0xPolygonHermez#3095)

* fix. A sequenced batch with OOC error is not a reorg

cmd/run.go

@@ -37,6 +37,7 @@ import (
 	"github.com/0xPolygonHermez/zkevm-node/state/pgstatestorage"
 	"github.com/0xPolygonHermez/zkevm-node/state/runtime/executor"
 	"github.com/0xPolygonHermez/zkevm-node/synchronizer"
+	"github.com/0xPolygonHermez/zkevm-node/synchronizer/common/syncinterfaces"
 	"github.com/jackc/pgx/v4/pgxpool"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/urfave/cli/v2"
@@ -303,7 +304,7 @@ func runSynchronizer(cfg config.Config, etherman *etherman.Client, ethTxManagerS
 	}
 	zkEVMClient := client.NewClient(trustedSequencerURL)
 
-	etherManForL1 := []synchronizer.EthermanInterface{}
+	etherManForL1 := []syncinterfaces.EthermanFullInterface{}
 	// If synchronizer are using sequential mode, we only need one etherman client
 	if cfg.Synchronizer.L1SynchronizationMode == synchronizer.ParallelMode {
 		for i := 0; i < int(cfg.Synchronizer.L1ParallelSynchronization.MaxClients+1); i++ {

state/batchV2.go

@@ -296,8 +296,10 @@ func (s *State) sendBatchRequestToExecutorV2(ctx context.Context, processBatchRe
 		log.Debug(processBatchResponseToString(res, ""))
 		err = executor.ExecutorErr(res.Error)
 		s.eventLog.LogExecutorErrorV2(ctx, res.Error, processBatchRequest)
+	} else if res.ErrorRom != executor.RomError_ROM_ERROR_NO_ERROR && executor.IsROMOutOfCountersError(res.ErrorRom) {
+		log.Warn("OOC error: ", processBatchResponseToString(res, ""))
 	} else if res.ErrorRom != executor.RomError_ROM_ERROR_NO_ERROR {
-		log.Debug(processBatchResponseToString(res, ""))
+		log.Warn(processBatchResponseToString(res, ""))
 		err = executor.RomErr(res.ErrorRom)
 	}
 	//workarroundDuplicatedBlock(res)
@@ -406,7 +408,6 @@ func (s *State) ProcessAndStoreClosedBatchV2(ctx context.Context, processingCtx
 	}
 	if processedBatch.IsRomOOCError {
 		log.Errorf("%s error isRomOOCError: %v", debugPrefix, err)
-		return common.Hash{}, noFlushID, noProverID, ErrExecutingBatchOOC
 	}
 
 	if len(processedBatch.BlockResponses) > 0 && !processedBatch.IsRomOOCError {

state/batchV2_test.go

@@ -104,3 +104,81 @@ func TestProcessAndStoreClosedBatchV2(t *testing.T) {
 
 	// Add assertions as needed
 }
+
+func TestProcessAndStoreClosedBatchV2ErrorOOC(t *testing.T) {
+	stateCfg := state.Config{
+		MaxCumulativeGasUsed: 800000,
+		ChainID:              1000,
+		MaxLogsCount:         10000,
+		MaxLogsBlockRange:    10000,
+		ForkIDIntervals: []state.ForkIDInterval{{
+			FromBatchNumber: 0,
+			ToBatchNumber:   math.MaxUint64,
+			ForkId:          state.FORKID_ETROG,
+			Version:         "",
+		}},
+	}
+
+	ctx := context.Background()
+	mockStorage := mocks.NewStorageMock(t)
+	mockExecutor := mocks.NewExecutorServiceClientMock(t)
+	testState := state.NewState(stateCfg, mockStorage, mockExecutor, nil, nil, nil)
+	mockStorage.EXPECT().Begin(ctx).Return(mocks.NewDbTxMock(t), nil)
+	dbTx, err := testState.BeginStateTransaction(ctx)
+	require.NoError(t, err)
+
+	processingCtx := state.ProcessingContextV2{
+		BatchNumber:    128,
+		Coinbase:       addr1,
+		Timestamp:      &time2,
+		L1InfoRoot:     hash1,
+		BatchL2Data:    &data1,
+		GlobalExitRoot: hash2,
+	}
+	batchContext := state.ProcessingContext{
+		BatchNumber:    processingCtx.BatchNumber,
+		Coinbase:       processingCtx.Coinbase,
+		Timestamp:      *processingCtx.Timestamp,
+		GlobalExitRoot: processingCtx.GlobalExitRoot,
+		ForcedBatchNum: processingCtx.ForcedBatchNum,
+		BatchL2Data:    processingCtx.BatchL2Data,
+	}
+	latestBatch := state.Batch{
+		BatchNumber: 128,
+	}
+	previousBatch := state.Batch{
+		BatchNumber: 127,
+	}
+
+	executorResponse := executor.ProcessBatchResponseV2{
+		Error:            executor.ExecutorError_EXECUTOR_ERROR_NO_ERROR,
+		ErrorRom:         executor.RomError_ROM_ERROR_OUT_OF_COUNTERS_KECCAK,
+		NewStateRoot:     hash3.Bytes(),
+		NewLocalExitRoot: hash4.Bytes(),
+		NewAccInputHash:  hash5.Bytes(),
+	}
+	// IMPORTANT: GlobalExitRoot is not stored in the close call
+	closingReceipt := state.ProcessingReceipt{
+		BatchNumber:   processingCtx.BatchNumber,
+		StateRoot:     hash3,
+		LocalExitRoot: hash4,
+		AccInputHash:  hash5,
+		BatchL2Data:   *processingCtx.BatchL2Data,
+	}
+	// Call the function under test
+	mockStorage.EXPECT().GetLastBatchNumber(ctx, dbTx).Return(uint64(127), nil)
+	mockStorage.EXPECT().IsBatchClosed(ctx, uint64(127), dbTx).Return(true, nil)
+	mockStorage.EXPECT().GetLastBatchTime(ctx, dbTx).Return(time1, nil)
+	// When calls to OpenBatch doesnt store the BatchL2Data yet
+	batchContext.BatchL2Data = nil
+	mockStorage.EXPECT().OpenBatchInStorage(ctx, batchContext, dbTx).Return(nil)
+	mockStorage.EXPECT().GetLastNBatches(ctx, uint(2), dbTx).Return([]*state.Batch{&latestBatch, &previousBatch}, nil)
+	mockStorage.EXPECT().IsBatchClosed(ctx, uint64(128), dbTx).Return(false, nil)
+	mockStorage.EXPECT().GetForkIDByBatchNumber(uint64(128)).Return(uint64(state.FORKID_ETROG))
+	mockExecutor.EXPECT().ProcessBatchV2(ctx, mock.Anything, mock.Anything).Return(&executorResponse, nil)
+	mockStorage.EXPECT().CloseBatchInStorage(ctx, closingReceipt, dbTx).Return(nil)
+	_, _, _, err = testState.ProcessAndStoreClosedBatchV2(ctx, processingCtx, dbTx, metrics.CallerLabel("test"))
+	require.NoError(t, err)
+
+	// Add assertions as needed
+}

state/convertersV2.go

@@ -33,7 +33,7 @@ func (s *State) convertToProcessBatchResponseV2(batchResponse *executor.ProcessB
 	if err != nil {
 		return nil, err
 	}
-
+	isRomOOCError = isRomOOCError || executor.IsROMOutOfCountersError(batchResponse.ErrorRom)
 	readWriteAddresses, err := convertToReadWriteAddressesV2(batchResponse.ReadWriteAddresses)
 	if err != nil {
 		return nil, err

synchronizer/actions/etrog/processor_l1_sequence_batches_test.go

@@ -0,0 +1,115 @@
+package etrog
+
+import (
+	"context"
+	"testing"
+
+	"github.com/0xPolygonHermez/zkevm-node/etherman"
+	"github.com/0xPolygonHermez/zkevm-node/etherman/smartcontracts/polygonzkevm"
+	"github.com/0xPolygonHermez/zkevm-node/state"
+	"github.com/0xPolygonHermez/zkevm-node/synchronizer/actions"
+	syncCommon "github.com/0xPolygonHermez/zkevm-node/synchronizer/common"
+	mock_syncinterfaces "github.com/0xPolygonHermez/zkevm-node/synchronizer/common/syncinterfaces/mocks"
+	syncMocks "github.com/0xPolygonHermez/zkevm-node/synchronizer/mocks"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+var (
+	hashExamplesValues = []string{"0x723e5c4c7ee7890e1e66c2e391d553ee792d2204ecb4fe921830f12f8dcd1a92",
+		"0x9c8fa7ce2e197f9f1b3c30de9f93de3c1cb290e6c118a18446f47a9e1364c3ab",
+		"0x896cfc0684057d0560e950dee352189528167f4663609678d19c7a506a03fe4e",
+		"0xde6d2dac4b6e0cb39ed1924db533558a23e5c56ab60fadac8c7d21e7eceb121a",
+		"0x9883711e78d02992ac1bd6f19de3bf7bb3f926742d4601632da23525e33f8555"}
+
+	addrExampleValues = []string{"0x8dAF17A20c9DBA35f005b6324F493785D239719d",
+		"0xB7f8BC63BbcaD18155201308C8f3540b07f84F5e",
+		"0x5FbDB2315678afecb367f032d93F642f64180aa3",
+		"0x8A791620dd6260079BF849Dc5567aDC3F2FdC318"}
+)
+
+type mocksEtrogProcessorL1 struct {
+	Etherman             *mock_syncinterfaces.EthermanFullInterface
+	State                *mock_syncinterfaces.StateFullInterface
+	Pool                 *mock_syncinterfaces.PoolInterface
+	Synchronizer         *mock_syncinterfaces.SynchronizerFullInterface
+	DbTx                 *syncMocks.DbTxMock
+	TimeProvider         *syncCommon.MockTimerProvider
+	CriticalErrorHandler *mock_syncinterfaces.CriticalErrorHandler
+}
+
+func createMocks(t *testing.T) *mocksEtrogProcessorL1 {
+	mocks := &mocksEtrogProcessorL1{
+		Etherman:     mock_syncinterfaces.NewEthermanFullInterface(t),
+		State:        mock_syncinterfaces.NewStateFullInterface(t),
+		Pool:         mock_syncinterfaces.NewPoolInterface(t),
+		Synchronizer: mock_syncinterfaces.NewSynchronizerFullInterface(t),
+		DbTx:         syncMocks.NewDbTxMock(t),
+		//ZKEVMClient:          mock_syncinterfaces.NewZKEVMClientInterface(t),
+		TimeProvider:         &syncCommon.MockTimerProvider{},
+		CriticalErrorHandler: mock_syncinterfaces.NewCriticalErrorHandler(t),
+		//EventLog:     &eventLogMock{},
+	}
+	return mocks
+}
+
+func createSUT(mocks *mocksEtrogProcessorL1) *ProcessorL1SequenceBatchesEtrog {
+	return NewProcessorL1SequenceBatches(mocks.State, mocks.Etherman, mocks.Pool, mocks.Synchronizer,
+		mocks.TimeProvider, mocks.CriticalErrorHandler)
+}
+
+func TestL1SequenceBatchesNoData(t *testing.T) {
+	mocks := createMocks(t)
+	sut := createSUT(mocks)
+	ctx := context.Background()
+	err := sut.Process(ctx, etherman.Order{}, nil, mocks.DbTx)
+	require.ErrorIs(t, err, actions.ErrInvalidParams)
+}
+
+func TestL1SequenceBatchesWrongOrder(t *testing.T) {
+	mocks := createMocks(t)
+	sut := createSUT(mocks)
+	ctx := context.Background()
+	l1Block := etherman.Block{
+		SequencedBatches: [][]etherman.SequencedBatch{},
+	}
+	err := sut.Process(ctx, etherman.Order{Pos: 1}, &l1Block, mocks.DbTx)
+	require.Error(t, err)
+}
+
+func TestL1SequenceBatchesPermissionlessNewBatchSequenced(t *testing.T) {
+	mocks := createMocks(t)
+	sut := createSUT(mocks)
+	ctx := context.Background()
+	l1Block := etherman.Block{
+		BlockNumber:      123,
+		ReceivedAt:       mocks.TimeProvider.Now(),
+		SequencedBatches: [][]etherman.SequencedBatch{},
+	}
+	l1InfoRoot := common.HexToHash(hashExamplesValues[0])
+	l1Block.SequencedBatches = append(l1Block.SequencedBatches, []etherman.SequencedBatch{})
+	l1Block.SequencedBatches = append(l1Block.SequencedBatches, []etherman.SequencedBatch{
+		{
+			BatchNumber:   3,
+			L1InfoRoot:    &l1InfoRoot,
+			TxHash:        common.HexToHash(hashExamplesValues[1]),
+			Coinbase:      common.HexToAddress(addrExampleValues[0]),
+			SequencerAddr: common.HexToAddress(addrExampleValues[1]),
+			PolygonRollupBaseEtrogBatchData: &polygonzkevm.PolygonRollupBaseEtrogBatchData{
+				Transactions: []byte{},
+			},
+		},
+	})
+	mocks.State.EXPECT().GetL1InfoTreeDataFromBatchL2Data(ctx, mock.Anything, mocks.DbTx).Return(map[uint32]state.L1DataV2{}, state.ZeroHash, state.ZeroHash, nil)
+	mocks.State.EXPECT().GetBatchByNumber(ctx, uint64(3), mocks.DbTx).Return(nil, state.ErrNotFound)
+	mocks.Synchronizer.EXPECT().PendingFlushID(mock.Anything, mock.Anything)
+	mocks.State.EXPECT().AddVirtualBatch(ctx, mock.Anything, mocks.DbTx).Return(nil)
+	mocks.State.EXPECT().AddSequence(ctx, mock.Anything, mocks.DbTx).Return(nil)
+	newStateRoot := common.HexToHash(hashExamplesValues[2])
+	flushID := uint64(1234)
+	proverID := "prover-id"
+	mocks.State.EXPECT().ProcessAndStoreClosedBatchV2(ctx, mock.Anything, mocks.DbTx, mock.Anything).Return(newStateRoot, flushID, proverID, nil)
+	err := sut.Process(ctx, etherman.Order{Pos: 1}, &l1Block, mocks.DbTx)
+	require.NoError(t, err)
+}

synchronizer/common/syncinterfaces/eth_tx_manager.go

@@ -0,0 +1,11 @@
+package syncinterfaces
+
+import (
+	"context"
+
+	"github.com/jackc/pgx/v4"
+)
+
+type EthTxManager interface {
+	Reorg(ctx context.Context, fromBlockNumber uint64, dbTx pgx.Tx) error
+}

synchronizer/common/syncinterfaces/etherman.go

@@ -1,5 +1,25 @@
 package syncinterfaces
 
+import (
+	"context"
+	"math/big"
+
+	"github.com/0xPolygonHermez/zkevm-node/etherman"
+	"github.com/ethereum/go-ethereum/common"
+	ethTypes "github.com/ethereum/go-ethereum/core/types"
+)
+
+// EthermanFullInterface contains the methods required to interact with ethereum.
+type EthermanFullInterface interface {
+	HeaderByNumber(ctx context.Context, number *big.Int) (*ethTypes.Header, error)
+	GetRollupInfoByBlockRange(ctx context.Context, fromBlock uint64, toBlock *uint64) ([]etherman.Block, map[common.Hash][]etherman.Order, error)
+	EthBlockByNumber(ctx context.Context, blockNumber uint64) (*ethTypes.Block, error)
+	GetLatestBatchNumber() (uint64, error)
+	GetTrustedSequencerURL() (string, error)
+	VerifyGenBlockNumber(ctx context.Context, genBlockNumber uint64) (bool, error)
+	GetLatestVerifiedBatchNum() (uint64, error)
+}
+
 type EthermanGetLatestBatchNumber interface {
 	GetLatestBatchNumber() (uint64, error)
 }