Fix/unsigned tx state root (#1835)

* fix unsigned tx to use the correct l2block state root * remove unused variables and logs * update mocks * fix unit tests * solve conflicts --------- Co-authored-by: ToniRamirezM <[email protected]>
0xPolygonHermez · Mar 16, 2023 · 992a322 · 992a322
1 parent 3c206a7
commit 992a322
Show file tree

Hide file tree

Showing 8 changed files with 150 additions and 36 deletions.
diff --git a/jsonrpc/endpoints_eth.go b/jsonrpc/endpoints_eth.go
@@ -73,12 +73,7 @@ func (e *EthEndpoints) Call(arg *txnArgs, number *BlockNumber) (interface{}, rpc
 			return rpcErrorResponse(defaultErrorCode, "failed to convert arguments into an unsigned transaction", err)
 		}
 
-		var blockNumberToProcessTx *uint64
-		if number != nil && *number != LatestBlockNumber && *number != PendingBlockNumber {
-			blockNumberToProcessTx = &blockNumber
-		}
-
-		result := e.state.ProcessUnsignedTransaction(ctx, tx, sender, blockNumberToProcessTx, false, dbTx)
+		result := e.state.ProcessUnsignedTransaction(ctx, tx, sender, blockNumber, false, dbTx)
 		if result.Failed() {
 			data := make([]byte, len(result.ReturnValue))
 			copy(data, result.ReturnValue)

diff --git a/jsonrpc/endpoints_eth_test.go b/jsonrpc/endpoints_eth_test.go
@@ -151,8 +151,7 @@ func TestCall(t *testing.T) {
 						tx.Nonce() == nonce
 				})
 				m.State.On("GetNonce", context.Background(), testCase.from, blockNumber, m.DbTx).Return(nonce, nil).Once()
-				var nilBlockNumber *uint64
-				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
+				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, blockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
 			},
 		},
 		{
@@ -179,8 +178,7 @@ func TestCall(t *testing.T) {
 					dataMatch := hex.EncodeToHex(tx.Data()) == hex.EncodeToHex(testCase.data)
 					return hasTx && gasMatch && toMatch && gasPriceMatch && valueMatch && dataMatch
 				})
-				var nilBlockNumber *uint64
-				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
+				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), blockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
 			},
 		},
 		{
@@ -208,8 +206,7 @@ func TestCall(t *testing.T) {
 					dataMatch := hex.EncodeToHex(tx.Data()) == hex.EncodeToHex(testCase.data)
 					return hasTx && gasMatch && toMatch && gasPriceMatch && valueMatch && dataMatch
 				})
-				var nilBlockNumber *uint64
-				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
+				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, common.HexToAddress(c.DefaultSenderAddress), blockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{ReturnValue: testCase.expectedResult}).Once()
 			},
 		},
 		{
@@ -284,8 +281,7 @@ func TestCall(t *testing.T) {
 					return hasTx && gasMatch && toMatch && gasPriceMatch && valueMatch && dataMatch && nonceMatch
 				})
 				m.State.On("GetNonce", context.Background(), testCase.from, blockNumber, m.DbTx).Return(nonce, nil).Once()
-				var nilBlockNumber *uint64
-				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, nilBlockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{Err: errors.New("failed to process unsigned transaction")}).Once()
+				m.State.On("ProcessUnsignedTransaction", context.Background(), txMatchBy, testCase.from, blockNumber, false, m.DbTx).Return(&runtime.ExecutionResult{Err: errors.New("failed to process unsigned transaction")}).Once()
 			},
 		},
 	}

diff --git a/jsonrpc/interfaces.go b/jsonrpc/interfaces.go
@@ -53,7 +53,7 @@ type stateInterface interface {
 	GetTransactionReceipt(ctx context.Context, transactionHash common.Hash, dbTx pgx.Tx) (*types.Receipt, error)
 	IsL2BlockConsolidated(ctx context.Context, blockNumber uint64, dbTx pgx.Tx) (bool, error)
 	IsL2BlockVirtualized(ctx context.Context, blockNumber uint64, dbTx pgx.Tx) (bool, error)
-	ProcessUnsignedTransaction(ctx context.Context, tx *types.Transaction, senderAddress common.Address, l2BlockNumber *uint64, noZKEVMCounters bool, dbTx pgx.Tx) *runtime.ExecutionResult
+	ProcessUnsignedTransaction(ctx context.Context, tx *types.Transaction, senderAddress common.Address, l2BlockNumber uint64, noZKEVMCounters bool, dbTx pgx.Tx) *runtime.ExecutionResult
 	RegisterNewL2BlockEventHandler(h state.NewL2BlockEventHandler)
 	GetLastVirtualBatchNum(ctx context.Context, dbTx pgx.Tx) (uint64, error)
 	GetLastVerifiedBatch(ctx context.Context, dbTx pgx.Tx) (*state.VerifiedBatch, error)

diff --git a/jsonrpc/mock_state_test.go b/jsonrpc/mock_state_test.go
diff --git a/state/state.go b/state/state.go
@@ -1211,7 +1211,7 @@ func (s *State) PreProcessTransaction(ctx context.Context, tx *types.Transaction
 		return nil, err
 	}
 
-	response, err := s.internalProcessUnsignedTransaction(ctx, tx, sender, &lastL2BlockNumber, false, true, dbTx)
+	response, err := s.internalProcessUnsignedTransaction(ctx, tx, sender, lastL2BlockNumber, false, dbTx)
 	if err != nil && !errors.Is(err, runtime.ErrExecutionReverted) {
 		return nil, err
 	}
@@ -1220,9 +1220,9 @@ func (s *State) PreProcessTransaction(ctx context.Context, tx *types.Transaction
 }
 
 // ProcessUnsignedTransaction processes the given unsigned transaction.
-func (s *State) ProcessUnsignedTransaction(ctx context.Context, tx *types.Transaction, senderAddress common.Address, l2BlockNumber *uint64, noZKEVMCounters bool, dbTx pgx.Tx) *runtime.ExecutionResult {
+func (s *State) ProcessUnsignedTransaction(ctx context.Context, tx *types.Transaction, senderAddress common.Address, l2BlockNumber uint64, noZKEVMCounters bool, dbTx pgx.Tx) *runtime.ExecutionResult {
 	result := new(runtime.ExecutionResult)
-	response, err := s.internalProcessUnsignedTransaction(ctx, tx, senderAddress, l2BlockNumber, noZKEVMCounters, false, dbTx)
+	response, err := s.internalProcessUnsignedTransaction(ctx, tx, senderAddress, l2BlockNumber, noZKEVMCounters, dbTx)
 	if err != nil {
 		result.Err = err
 	}
@@ -1243,22 +1243,23 @@ func (s *State) ProcessUnsignedTransaction(ctx context.Context, tx *types.Transa
 }
 
 // ProcessUnsignedTransaction processes the given unsigned transaction.
-func (s *State) internalProcessUnsignedTransaction(ctx context.Context, tx *types.Transaction, senderAddress common.Address, l2BlockNumber *uint64, noZKEVMCounters bool, forceNonce bool, dbTx pgx.Tx) (*ProcessBatchResponse, error) {
-	lastBatches, l2BlockStateRoot, err := s.PostgresStorage.GetLastNBatchesByL2BlockNumber(ctx, l2BlockNumber, two, dbTx)
+func (s *State) internalProcessUnsignedTransaction(ctx context.Context, tx *types.Transaction, senderAddress common.Address, l2BlockNumber uint64, noZKEVMCounters bool, dbTx pgx.Tx) (*ProcessBatchResponse, error) {
+	lastBatches, _, err := s.PostgresStorage.GetLastNBatchesByL2BlockNumber(ctx, &l2BlockNumber, two, dbTx)
 	if err != nil {
 		return nil, err
 	}
 
-	var pForcedNonce *uint64
+	l2Block, err := s.GetL2BlockByNumber(ctx, l2BlockNumber, dbTx)
+	if err != nil {
+		log.Errorf("error getting l2 block", err)
+		return nil, err
+	}
 
-	if forceNonce && l2BlockNumber != nil {
-		forcedNonce, err := s.tree.GetNonce(ctx, senderAddress, l2BlockStateRoot.Bytes())
-		if err != nil {
-			return nil, err
-		}
-		tmpNonce := forcedNonce.Uint64()
-		pForcedNonce = &tmpNonce
+	nonce, err := s.tree.GetNonce(ctx, senderAddress, l2Block.Root().Bytes())
+	if err != nil {
+		return nil, err
 	}
+	forcedNonce := nonce.Uint64()
 
 	// Get latest batch from the database to get globalExitRoot and Timestamp
 	lastBatch := lastBatches[0]
@@ -1269,7 +1270,7 @@ func (s *State) internalProcessUnsignedTransaction(ctx context.Context, tx *type
 		previousBatch = lastBatches[1]
 	}
 
-	batchL2Data, err := EncodeUnsignedTransaction(*tx, s.cfg.ChainID, pForcedNonce)
+	batchL2Data, err := EncodeUnsignedTransaction(*tx, s.cfg.ChainID, &forcedNonce)
 	if err != nil {
 		log.Errorf("error encoding unsigned transaction ", err)
 		return nil, err
@@ -1281,7 +1282,7 @@ func (s *State) internalProcessUnsignedTransaction(ctx context.Context, tx *type
 		OldBatchNum:      lastBatch.BatchNumber,
 		BatchL2Data:      batchL2Data,
 		From:             senderAddress.String(),
-		OldStateRoot:     l2BlockStateRoot.Bytes(),
+		OldStateRoot:     l2Block.Root().Bytes(),
 		GlobalExitRoot:   lastBatch.GlobalExitRoot.Bytes(),
 		OldAccInputHash:  previousBatch.AccInputHash.Bytes(),
 		EthTimestamp:     uint64(lastBatch.Timestamp.Unix()),

diff --git a/state/state_test.go b/state/state_test.go
@@ -24,12 +24,15 @@ import (
 	"github.com/0xPolygonHermez/zkevm-node/state/runtime"
 	"github.com/0xPolygonHermez/zkevm-node/state/runtime/executor"
 	executorclientpb "github.com/0xPolygonHermez/zkevm-node/state/runtime/executor/pb"
+	"github.com/0xPolygonHermez/zkevm-node/test/contracts/bin/Counter"
 	"github.com/0xPolygonHermez/zkevm-node/test/dbutils"
+	"github.com/0xPolygonHermez/zkevm-node/test/operations"
 	"github.com/0xPolygonHermez/zkevm-node/test/testutils"
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/ethereum/go-ethereum/trie"
 	"github.com/jackc/pgx/v4/pgxpool"
@@ -1026,7 +1029,7 @@ func TestExecutorRevert(t *testing.T) {
 
 	unsignedTx := types.NewTransaction(2, scAddress, new(big.Int), 40000, new(big.Int).SetUint64(1), common.Hex2Bytes("4abbb40a"))
 
-	result := testState.ProcessUnsignedTransaction(ctx, unsignedTx, auth.From, &lastL2BlockNumber, false, nil)
+	result := testState.ProcessUnsignedTransaction(ctx, unsignedTx, auth.From, lastL2BlockNumber, false, nil)
 	require.NotNil(t, result.Err)
 	assert.Equal(t, fmt.Errorf("execution reverted: Today is not juernes").Error(), result.Err.Error())
 }
@@ -1784,7 +1787,7 @@ func TestExecutorUnsignedTransactions(t *testing.T) {
 	})
 	l2BlockNumber := uint64(3)
 
-	result := testState.ProcessUnsignedTransaction(context.Background(), unsignedTxSecondRetrieve, common.HexToAddress("0x1000000000000000000000000000000000000000"), &l2BlockNumber, true, nil)
+	result := testState.ProcessUnsignedTransaction(context.Background(), unsignedTxSecondRetrieve, common.HexToAddress("0x1000000000000000000000000000000000000000"), l2BlockNumber, true, nil)
 	// assert unsigned tx
 	assert.Nil(t, result.Err)
 	assert.Equal(t, "0000000000000000000000000000000000000000000000000000000000000001", hex.EncodeToString(result.ReturnValue))
@@ -2872,3 +2875,122 @@ func TestExecuteWithoutUpdatingMT(t *testing.T) {
 	assert.Equal(t, executorclientpb.RomError(1), processBatchResponse.Responses[1].Error)
 	assert.Equal(t, "0000000000000000000000000000000000000000000000000000000000000001", hex.EncodeToString(processBatchResponse.Responses[1].ReturnValue))
 }
+
+func TestExecutorUnsignedTransactionsWithCorrectL2BlockStateRoot(t *testing.T) {
+	// Init database instance
+	initOrResetDB()
+
+	// auth
+	privateKey, err := crypto.HexToECDSA(strings.TrimPrefix(operations.DefaultSequencerPrivateKey, "0x"))
+	require.NoError(t, err)
+	auth, err := bind.NewKeyedTransactorWithChainID(privateKey, new(big.Int).SetUint64(stateCfg.ChainID))
+	require.NoError(t, err)
+
+	auth.Nonce = big.NewInt(0)
+	auth.Value = nil
+	auth.GasPrice = big.NewInt(0)
+	auth.GasLimit = uint64(4000000)
+	auth.NoSend = true
+
+	_, scTx, sc, err := Counter.DeployCounter(auth, &ethclient.Client{})
+	require.NoError(t, err)
+
+	auth.Nonce = big.NewInt(1)
+	tx1, err := sc.Increment(auth)
+	require.NoError(t, err)
+
+	auth.Nonce = big.NewInt(2)
+	tx2, err := sc.Increment(auth)
+	require.NoError(t, err)
+
+	auth.Nonce = big.NewInt(3)
+	tx3, err := sc.Increment(auth)
+	require.NoError(t, err)
+
+	dbTx, err := testState.BeginStateTransaction(context.Background())
+	require.NoError(t, err)
+	// Set genesis
+	genesis := state.Genesis{Actions: []*state.GenesisAction{
+		{
+			Address: operations.DefaultSequencerAddress,
+			Type:    int(merkletree.LeafTypeBalance),
+			Value:   "100000000000000000000000",
+		},
+	}}
+	_, err = testState.SetGenesis(ctx, state.Block{}, genesis, dbTx)
+	require.NoError(t, err)
+	batchCtx := state.ProcessingContext{
+		BatchNumber: 1,
+		Coinbase:    common.HexToAddress(operations.DefaultSequencerAddress),
+		Timestamp:   time.Now(),
+	}
+	err = testState.OpenBatch(context.Background(), batchCtx, dbTx)
+	require.NoError(t, err)
+	signedTxs := []types.Transaction{
+		*scTx,
+		*tx1,
+		*tx2,
+		*tx3,
+	}
+
+	batchL2Data, err := state.EncodeTransactions(signedTxs)
+	require.NoError(t, err)
+
+	processBatchResponse, err := testState.ProcessSequencerBatch(context.Background(), 1, batchL2Data, state.SequencerCallerLabel, dbTx)
+	require.NoError(t, err)
+	// assert signed tx do deploy sc
+	assert.Nil(t, processBatchResponse.Responses[0].RomError)
+	assert.NotEqual(t, state.ZeroAddress, processBatchResponse.Responses[0].CreateAddress.Hex())
+	assert.Equal(t, tx1.To().Hex(), processBatchResponse.Responses[0].CreateAddress.Hex())
+
+	// assert signed tx to increment counter
+	assert.Nil(t, processBatchResponse.Responses[1].RomError)
+	assert.Nil(t, processBatchResponse.Responses[2].RomError)
+	assert.Nil(t, processBatchResponse.Responses[3].RomError)
+
+	// Add txs to DB
+	err = testState.StoreTransactions(context.Background(), 1, processBatchResponse.Responses, dbTx)
+	require.NoError(t, err)
+	// Close batch
+	err = testState.CloseBatch(
+		context.Background(),
+		state.ProcessingReceipt{
+			BatchNumber:   1,
+			StateRoot:     processBatchResponse.NewStateRoot,
+			LocalExitRoot: processBatchResponse.NewLocalExitRoot,
+		}, dbTx,
+	)
+	require.NoError(t, err)
+	require.NoError(t, dbTx.Commit(context.Background()))
+
+	getCountFnSignature := crypto.Keccak256Hash([]byte("getCount()")).Bytes()[:4]
+	getCountUnsignedTx := types.NewTx(&types.LegacyTx{
+		To:   &processBatchResponse.Responses[0].CreateAddress,
+		Gas:  uint64(100000),
+		Data: getCountFnSignature,
+	})
+
+	l2BlockNumber := uint64(1)
+	result := testState.ProcessUnsignedTransaction(context.Background(), getCountUnsignedTx, auth.From, l2BlockNumber, true, nil)
+	// assert unsigned tx
+	assert.Nil(t, result.Err)
+	assert.Equal(t, "0000000000000000000000000000000000000000000000000000000000000000", hex.EncodeToString(result.ReturnValue))
+
+	l2BlockNumber = uint64(2)
+	result = testState.ProcessUnsignedTransaction(context.Background(), getCountUnsignedTx, auth.From, l2BlockNumber, true, nil)
+	// assert unsigned tx
+	assert.Nil(t, result.Err)
+	assert.Equal(t, "0000000000000000000000000000000000000000000000000000000000000001", hex.EncodeToString(result.ReturnValue))
+
+	l2BlockNumber = uint64(3)
+	result = testState.ProcessUnsignedTransaction(context.Background(), getCountUnsignedTx, auth.From, l2BlockNumber, true, nil)
+	// assert unsigned tx
+	assert.Nil(t, result.Err)
+	assert.Equal(t, "0000000000000000000000000000000000000000000000000000000000000002", hex.EncodeToString(result.ReturnValue))
+
+	l2BlockNumber = uint64(4)
+	result = testState.ProcessUnsignedTransaction(context.Background(), getCountUnsignedTx, auth.From, l2BlockNumber, true, nil)
+	// assert unsigned tx
+	assert.Nil(t, result.Err)
+	assert.Equal(t, "0000000000000000000000000000000000000000000000000000000000000003", hex.EncodeToString(result.ReturnValue))
+}
diff --git a/synchronizer/mock_etherman.go b/synchronizer/mock_etherman.go
diff --git a/synchronizer/mock_state.go b/synchronizer/mock_state.go