Add checks to TxSelector & other:

- Upgrade go-merkletree version to include the last changes of Pebble that fixes the cgo issues (which should fix #453), from: c2b05f12d7 - TxSelector - Remove parameter batchNum for GetL2TxSelection & GetL1L2TxSelection - Add checks of ToBJJ & ToEthAddr when ToIdx>255 - Avoid getting the sender account twice to get the TokenID of a l2tx - Add test to check that selected L2Txs are sorted by Nonce - Discard L2Tx that return error at ProcessL2Txs - executed `go mod tidy`
2026-02-06 19:06:42 +01:00 · 2021-01-05 15:19:31 +01:00
parent 6fa4a2e799
commit 962b9e4712
8 changed files with 208 additions and 71 deletions
--- a/txselector/txselector.go
+++ b/txselector/txselector.go
@@ -142,11 +142,11 @@ func (txsel *TxSelector) coordAccountForTokenID(l1CoordinatorTxs []common.L1Tx,
 // but there is a transactions to them and the authorization of account
 // creation exists. The L1UserTxs, L1CoordinatorTxs, PoolL2Txs that will be
 // included in the next batch.
-func (txsel *TxSelector) GetL2TxSelection(selectionConfig *SelectionConfig,
-	batchNum common.BatchNum) ([]common.Idx, [][]byte, []common.L1Tx,
+func (txsel *TxSelector) GetL2TxSelection(selectionConfig *SelectionConfig) ([]common.Idx,
+	[][]byte, []common.L1Tx,
 	[]common.PoolL2Tx, error) {
 	coordIdxs, accCreationAuths, _, l1CoordinatorTxs, l2Txs, err :=
-		txsel.GetL1L2TxSelection(selectionConfig, batchNum, []common.L1Tx{})
+		txsel.GetL1L2TxSelection(selectionConfig, []common.L1Tx{})
 	return coordIdxs, accCreationAuths, l1CoordinatorTxs, l2Txs, tracerr.Wrap(err)
 }

@@ -159,10 +159,8 @@ func (txsel *TxSelector) GetL2TxSelection(selectionConfig *SelectionConfig,
 // creation exists. The L1UserTxs, L1CoordinatorTxs, PoolL2Txs that will be
 // included in the next batch.
 func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
-	batchNum common.BatchNum, l1UserTxs []common.L1Tx) ([]common.Idx, [][]byte,
-	[]common.L1Tx, []common.L1Tx, []common.PoolL2Tx, error) {
-	// TODO input parameter 'batchNum' is not used, delete
-
+	l1UserTxs []common.L1Tx) ([]common.Idx, [][]byte, []common.L1Tx,
+	[]common.L1Tx, []common.PoolL2Tx, error) {
 	// WIP.0: the TxSelector is not optimized and will need a redesign. The
 	// current version is implemented in order to have a functional
 	// implementation that can be used asap.
@@ -208,6 +206,7 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 		if err != nil {
 			return nil, nil, nil, nil, nil, tracerr.Wrap(err)
 		}
+		l2Txs0[i].TokenID = accSender.TokenID
 		// populate the noncesMap used at the next iteration
 		noncesMap[l2Txs0[i].FromIdx] = accSender.Nonce

@@ -278,14 +277,29 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 				validTxs = append(validTxs, *validL2Tx)
 			}
 		} else if l2Txs[i].ToIdx >= common.IdxUserThreshold {
-			_, err = txsel.localAccountsDB.GetAccount(l2Txs[i].ToIdx)
+			receiverAcc, err := txsel.localAccountsDB.GetAccount(l2Txs[i].ToIdx)
 			if err != nil {
 				// tx not valid
 				log.Debugw("invalid L2Tx: ToIdx not found in StateDB",
 					"ToIdx", l2Txs[i].ToIdx)
 				continue
 			}
-			// TODO if EthAddr!=0 or BJJ!=0, check that ToIdxAccount.EthAddr or BJJ
+			if l2Txs[i].ToEthAddr != common.EmptyAddr {
+				if l2Txs[i].ToEthAddr != receiverAcc.EthAddr {
+					log.Debugw("invalid L2Tx: ToEthAddr does not correspond to the Account.EthAddr",
+						"ToIdx", l2Txs[i].ToIdx, "tx.ToEthAddr", l2Txs[i].ToEthAddr,
+						"account.EthAddr", receiverAcc.EthAddr)
+					continue
+				}
+			}
+			if l2Txs[i].ToBJJ != common.EmptyBJJComp {
+				if l2Txs[i].ToBJJ != receiverAcc.PublicKey {
+					log.Debugw("invalid L2Tx: ToBJJ does not correspond to the Account.BJJ",
+						"ToIdx", l2Txs[i].ToIdx, "tx.ToEthAddr", l2Txs[i].ToBJJ,
+						"account.BJJ", receiverAcc.PublicKey)
+					continue
+				}
+			}

 			// Account found in the DB, include the l2Tx in the selection
 			validTxs = append(validTxs, l2Txs[i])
@@ -335,15 +349,26 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 	})

 	// get most profitable L2-tx
-	maxL2Txs := selectionConfig.TxProcessorConfig.MaxTx -
-		uint32(len(l1UserTxs)) - uint32(len(l1CoordinatorTxs))
+	maxL2Txs := int(selectionConfig.TxProcessorConfig.MaxTx) -
+		len(l1UserTxs) - len(l1CoordinatorTxs)

-	selectedL2Txs := txsel.getL2Profitable(l2Txs, maxL2Txs) // TODO this will only need to crop the lasts, as are already sorted
+	selectedL2Txs := l2Txs
+	if len(l2Txs) > maxL2Txs {
+		selectedL2Txs = selectedL2Txs[:maxL2Txs]
+	}
+	var finalL2Txs []common.PoolL2Tx
 	for i := 0; i < len(selectedL2Txs); i++ {
 		_, _, _, err = tp.ProcessL2Tx(coordIdxsMap, nil, nil, &selectedL2Txs[i])
 		if err != nil {
-			return nil, nil, nil, nil, nil, tracerr.Wrap(err)
+			// the error can be due not valid tx data, or due other
+			// cases (such as StateDB error). At this initial
+			// version of the TxSelector, we discard the L2Tx and
+			// log the error, assuming that this will be iterated
+			// in a near future.
+			log.Error(err)
+			continue
 		}
+		finalL2Txs = append(finalL2Txs, selectedL2Txs[i])
 	}

 	// distribute the AccumulatedFees from the processed L2Txs into the
@@ -371,7 +396,7 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 		return nil, nil, nil, nil, nil, tracerr.Wrap(err)
 	}

-	return coordIdxs, accAuths, l1UserTxs, l1CoordinatorTxs, selectedL2Txs, nil
+	return coordIdxs, accAuths, l1UserTxs, l1CoordinatorTxs, finalL2Txs, nil
 }

 // processTxsToEthAddrBJJ process the common.PoolL2Tx in the case where
@@ -384,12 +409,6 @@ func (txsel *TxSelector) processTxToEthAddrBJJ(validTxs []common.PoolL2Tx,
 	selectionConfig *SelectionConfig, nL1UserTxs int, l1CoordinatorTxs []common.L1Tx,
 	positionL1 int, l2Tx common.PoolL2Tx) (*common.PoolL2Tx, *common.L1Tx,
 	*common.AccountCreationAuth, error) {
-	accSender, err := txsel.localAccountsDB.GetAccount(l2Tx.FromIdx)
-	if err != nil {
-		return nil, nil, nil, err
-	}
-	l2Tx.TokenID = accSender.TokenID // TODO rm and add it before at GetL1L2TxsSelection
-
 	// if L2Tx needs a new L1CoordinatorTx of CreateAccount type, and a
 	// previous L2Tx in the current process already created a
 	// L1CoordinatorTx of this type, in the DB there still seem that needs
--- a/txselector/txselector_test.go
+++ b/txselector/txselector_test.go
@@ -2,6 +2,7 @@ package txselector

 import (
 	"crypto/ecdsa"
+	"fmt"
 	"io/ioutil"
 	"math/big"
 	"os"
@@ -77,23 +78,6 @@ func initTest(t *testing.T, chainID uint16, hermezContractAddr ethCommon.Address
 	return txsel, tc
 }

-func checkBalance(t *testing.T, tc *til.Context, txsel *TxSelector, username string, tokenid int, expected string) {
-	// Accounts.Idx does not match with the TxSelector tests as we are not
-	// using the Til L1CoordinatorTxs (as are generated by the TxSelector
-	// itself when processing the txs, so the Idxs does not match the Til
-	// idxs). But the Idx is obtained through StateDB.GetIdxByEthAddrBJJ
-	user := tc.Users[username]
-	idx, err := txsel.localAccountsDB.GetIdxByEthAddrBJJ(user.Addr, user.BJJ.Public().Compress(), common.TokenID(tokenid))
-	require.NoError(t, err)
-	checkBalanceByIdx(t, txsel, idx, expected)
-}
-
-func checkBalanceByIdx(t *testing.T, txsel *TxSelector, idx common.Idx, expected string) {
-	acc, err := txsel.localAccountsDB.GetAccount(idx)
-	require.NoError(t, err)
-	assert.Equal(t, expected, acc.Balance.String())
-}
-
 func addAccCreationAuth(t *testing.T, tc *til.Context, txsel *TxSelector, chainID uint16, hermezContractAddr ethCommon.Address, username string) []byte {
 	user := tc.Users[username]
 	a := &common.AccountCreationAuth{
@@ -142,7 +126,36 @@ func addTokens(t *testing.T, tc *til.Context, db *sqlx.DB) {
 	assert.NoError(t, hdb.AddTokens(tokens))
 }

-func TestGetL2TxSelection(t *testing.T) {
+func checkBalance(t *testing.T, tc *til.Context, txsel *TxSelector, username string, tokenid int, expected string) {
+	// Accounts.Idx does not match with the TxSelector tests as we are not
+	// using the Til L1CoordinatorTxs (as are generated by the TxSelector
+	// itself when processing the txs, so the Idxs does not match the Til
+	// idxs). But the Idx is obtained through StateDB.GetIdxByEthAddrBJJ
+	user := tc.Users[username]
+	idx, err := txsel.localAccountsDB.GetIdxByEthAddrBJJ(user.Addr, user.BJJ.Public().Compress(), common.TokenID(tokenid))
+	require.NoError(t, err)
+	checkBalanceByIdx(t, txsel, idx, expected)
+}
+
+func checkBalanceByIdx(t *testing.T, txsel *TxSelector, idx common.Idx, expected string) {
+	acc, err := txsel.localAccountsDB.GetAccount(idx)
+	require.NoError(t, err)
+	assert.Equal(t, expected, acc.Balance.String())
+}
+
+// checkSortedByNonce takes as input testAccNonces map, and the array of
+// common.PoolL2Txs, and checks if the nonces correspond to the accumulated
+// values of the map. Also increases the Nonces computed on the map.
+func checkSortedByNonce(t *testing.T, testAccNonces map[common.Idx]common.Nonce, txs []common.PoolL2Tx) {
+	for _, tx := range txs {
+		assert.True(t, testAccNonces[tx.FromIdx] == tx.Nonce,
+			fmt.Sprintf("Idx: %d, expected: %d, tx.Nonce: %d",
+				tx.FromIdx, testAccNonces[tx.FromIdx], tx.Nonce))
+		testAccNonces[tx.FromIdx] = testAccNonces[tx.FromIdx] + 1
+	}
+}
+
+func TestGetL2TxSelectionMinimumFlow0(t *testing.T) {
 	chainID := uint16(0)
 	hermezContractAddr := ethCommon.HexToAddress("0xc344E203a046Da13b0B4467EB7B3629D0C99F6E6")
 	txsel, tc := initTest(t, chainID, hermezContractAddr, til.SetPool0)
@@ -156,6 +169,7 @@ func TestGetL2TxSelection(t *testing.T) {
 	// restart nonces of TilContext, as will be set by generating directly
 	// the PoolL2Txs for each specific batch with tc.GeneratePoolL2Txs
 	tc.RestartNonces()
+	testAccNonces := make(map[common.Idx]common.Nonce)

 	// add tokens to HistoryDB to avoid breaking FK constrains
 	addTokens(t, tc, txsel.l2db.DB())
@@ -176,7 +190,7 @@ func TestGetL2TxSelection(t *testing.T) {

 	log.Debug("block:0 batch:0")
 	l1UserTxs := []common.L1Tx{}
-	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err := txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err := txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, 0, len(oL1UserTxs))
 	assert.Equal(t, 0, len(oL1CoordTxs))
@@ -186,7 +200,7 @@ func TestGetL2TxSelection(t *testing.T) {

 	log.Debug("block:0 batch:1")
 	l1UserTxs = []common.L1Tx{}
-	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, 0, len(oL1UserTxs))
 	assert.Equal(t, 0, len(oL1CoordTxs))
@@ -196,7 +210,7 @@ func TestGetL2TxSelection(t *testing.T) {

 	log.Debug("block:0 batch:2")
 	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[0].Rollup.Batches[2].Batch.ForgeL1TxsNum])
-	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, 2, len(oL1UserTxs))
 	assert.Equal(t, 0, len(oL1CoordTxs))
@@ -208,7 +222,7 @@ func TestGetL2TxSelection(t *testing.T) {

 	log.Debug("block:0 batch:3")
 	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[0].Rollup.Batches[3].Batch.ForgeL1TxsNum])
-	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, 1, len(oL1UserTxs))
 	assert.Equal(t, 0, len(oL1CoordTxs))
@@ -221,7 +235,7 @@ func TestGetL2TxSelection(t *testing.T) {

 	log.Debug("block:0 batch:4")
 	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[0].Rollup.Batches[4].Batch.ForgeL1TxsNum])
-	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, 0, len(oL1UserTxs))
 	assert.Equal(t, 0, len(oL1CoordTxs))
@@ -234,7 +248,7 @@ func TestGetL2TxSelection(t *testing.T) {

 	log.Debug("block:0 batch:5")
 	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[0].Rollup.Batches[5].Batch.ForgeL1TxsNum])
-	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, 1, len(oL1UserTxs))
 	assert.Equal(t, 0, len(oL1CoordTxs))
@@ -266,7 +280,7 @@ func TestGetL2TxSelection(t *testing.T) {
 	addL2Txs(t, txsel, poolL2Txs)

 	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[0].Rollup.Batches[6].Batch.ForgeL1TxsNum])
-	coordIdxs, accAuths, oL1UserTxs, oL1CoordTxs, oL2Txs, err := txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	coordIdxs, accAuths, oL1UserTxs, oL1CoordTxs, oL2Txs, err := txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, []common.Idx{261, 262}, coordIdxs)
 	assert.Equal(t, txsel.coordAccount.AccountCreationAuth, accAuths[0])
@@ -287,6 +301,7 @@ func TestGetL2TxSelection(t *testing.T) {
 	checkBalance(t, tc, txsel, "B", 1, "200")
 	checkBalance(t, tc, txsel, "C", 0, "100")
 	checkBalance(t, tc, txsel, "D", 0, "800")
+	checkSortedByNonce(t, testAccNonces, oL2Txs)
 	err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(poolL2Txs), txsel.localAccountsDB.CurrentBatch())
 	require.NoError(t, err)

@@ -302,7 +317,7 @@ func TestGetL2TxSelection(t *testing.T) {
 	require.NoError(t, err)
 	addL2Txs(t, txsel, poolL2Txs)
 	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[0].Rollup.Batches[7].Batch.ForgeL1TxsNum])
-	coordIdxs, accAuths, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	coordIdxs, accAuths, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, []common.Idx{261, 262}, coordIdxs)
 	assert.Equal(t, 0, len(accAuths))
@@ -321,6 +336,7 @@ func TestGetL2TxSelection(t *testing.T) {
 	checkBalance(t, tc, txsel, "C", 0, "45")
 	checkBalance(t, tc, txsel, "C", 1, "100")
 	checkBalance(t, tc, txsel, "D", 0, "800")
+	checkSortedByNonce(t, testAccNonces, oL2Txs)
 	err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(poolL2Txs), txsel.localAccountsDB.CurrentBatch())
 	require.NoError(t, err)

@@ -335,7 +351,7 @@ func TestGetL2TxSelection(t *testing.T) {
 	require.NoError(t, err)
 	addL2Txs(t, txsel, poolL2Txs)
 	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[1].Rollup.Batches[0].Batch.ForgeL1TxsNum])
-	coordIdxs, accAuths, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, 0, l1UserTxs)
+	coordIdxs, accAuths, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
 	require.NoError(t, err)
 	assert.Equal(t, []common.Idx{262}, coordIdxs)
 	assert.Equal(t, 0, len(accAuths))
@@ -354,10 +370,7 @@ func TestGetL2TxSelection(t *testing.T) {
 	checkBalance(t, tc, txsel, "C", 0, "845")
 	checkBalance(t, tc, txsel, "C", 1, "100")
 	checkBalance(t, tc, txsel, "D", 0, "470")
+	checkSortedByNonce(t, testAccNonces, oL2Txs)
 	err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(poolL2Txs), txsel.localAccountsDB.CurrentBatch())
 	require.NoError(t, err)
-
-	// TODO:
-	// - check PoolL2Txs returned are sorted by nonce
-	// - check poolL2Txs.AbsoluteFee parameters
 }