Merge pull request #194 from hermeznetwork/feature/txsel-noncesorting0

Add TxSel Nonce sort. Fix surplus from refactors

common/l1tx.go

@@ -116,12 +116,12 @@ func (tx *L1Tx) CalcTxID() (*TxID, error) {
 }
 
 // Tx returns a *Tx from the L1Tx
-func (tx *L1Tx) Tx() *Tx {
+func (tx L1Tx) Tx() Tx {
 	f := new(big.Float).SetInt(tx.Amount)
 	amountFloat, _ := f.Float64()
 	userOrigin := new(bool)
 	*userOrigin = tx.UserOrigin
-	genericTx := &Tx{
+	genericTx := Tx{
 		IsL1:            true,
 		TxID:            tx.TxID,
 		Type:            tx.Type,

common/l2tx.go

@@ -81,7 +81,7 @@ func (tx *L2Tx) Tx() *Tx {
 
 // PoolL2Tx returns the data structure of PoolL2Tx with the parameters of a
 // L2Tx filled
-func (tx *L2Tx) PoolL2Tx() *PoolL2Tx {
+func (tx L2Tx) PoolL2Tx() *PoolL2Tx {
 	return &PoolL2Tx{
 		TxID:    tx.TxID,
 		FromIdx: tx.FromIdx,

common/pooll2tx.go

@@ -20,6 +20,7 @@ type PoolL2Tx struct {
 	TxID      TxID               `meddler:"tx_id"`
 	FromIdx   Idx                `meddler:"from_idx"`
 	ToIdx     Idx                `meddler:"to_idx,zeroisnull"`
+	AuxToIdx  Idx                `meddler:"-"` // AuxToIdx is only used internally at the StateDB to avoid repeated computation when processing transactions (from Synchronizer, TxSelector, BatchBuilder)
 	ToEthAddr ethCommon.Address  `meddler:"to_eth_addr"`
 	ToBJJ     *babyjub.PublicKey `meddler:"to_bjj"`
 	TokenID   TokenID            `meddler:"token_id"`
@@ -210,8 +211,8 @@ func (tx *PoolL2Tx) VerifySignature(pk *babyjub.PublicKey) bool {
 }
 
 // L2Tx returns a *L2Tx from the PoolL2Tx
-func (tx *PoolL2Tx) L2Tx() (*L2Tx, error) {
-	return &L2Tx{
+func (tx PoolL2Tx) L2Tx() L2Tx {
+	return L2Tx{
 		TxID:    tx.TxID,
 		FromIdx: tx.FromIdx,
 		ToIdx:   tx.ToIdx,
@@ -219,12 +220,12 @@ func (tx *PoolL2Tx) L2Tx() (*L2Tx, error) {
 		Fee:     tx.Fee,
 		Nonce:   tx.Nonce,
 		Type:    tx.Type,
-	}, nil
+	}
 }
 
 // Tx returns a *Tx from the PoolL2Tx
-func (tx *PoolL2Tx) Tx() (*Tx, error) {
-	return &Tx{
+func (tx PoolL2Tx) Tx() Tx {
+	return Tx{
 		TxID:    tx.TxID,
 		FromIdx: tx.FromIdx,
 		ToIdx:   tx.ToIdx,
@@ -232,18 +233,14 @@ func (tx *PoolL2Tx) Tx() (*Tx, error) {
 		Nonce:   &tx.Nonce,
 		Fee:     &tx.Fee,
 		Type:    tx.Type,
-	}, nil
+	}
 }
 
 // PoolL2TxsToL2Txs returns an array of []L2Tx from an array of []PoolL2Tx
 func PoolL2TxsToL2Txs(txs []PoolL2Tx) ([]L2Tx, error) {
 	var r []L2Tx
 	for _, poolTx := range txs {
-		tx, err := poolTx.L2Tx()
-		if err != nil {
-			return nil, err
-		}
-		r = append(r, *tx)
+		r = append(r, poolTx.L2Tx())
 	}
 	return r, nil
 }

db/statedb/txprocessors.go

@@ -68,9 +68,8 @@ func (s *StateDB) ProcessTxs(l1usertxs, l1coordinatortxs []common.L1Tx, l2txs []
 	}
 
 	// assumption: l1usertx are sorted by L1Tx.Position
-	for i := range l1usertxs {
-		tx := &l1usertxs[i]
-		exitIdx, exitAccount, newExit, err := s.processL1Tx(exitTree, tx)
+	for i := 0; i < len(l1usertxs); i++ {
+		exitIdx, exitAccount, newExit, err := s.processL1Tx(exitTree, &l1usertxs[i])
 		if err != nil {
 			return nil, nil, err
 		}
@@ -86,9 +85,8 @@ func (s *StateDB) ProcessTxs(l1usertxs, l1coordinatortxs []common.L1Tx, l2txs []
 			s.i++
 		}
 	}
-	for i := range l1coordinatortxs {
-		tx := &l1coordinatortxs[i]
-		exitIdx, exitAccount, newExit, err := s.processL1Tx(exitTree, tx)
+	for i := 0; i < len(l1coordinatortxs); i++ {
+		exitIdx, exitAccount, newExit, err := s.processL1Tx(exitTree, &l1coordinatortxs[i])
 		if err != nil {
 			return nil, nil, err
 		}
@@ -107,9 +105,8 @@ func (s *StateDB) ProcessTxs(l1usertxs, l1coordinatortxs []common.L1Tx, l2txs []
 			s.i++
 		}
 	}
-	for i := range l2txs {
-		tx := &l2txs[i]
-		exitIdx, exitAccount, newExit, err := s.processL2Tx(exitTree, tx)
+	for i := 0; i < len(l2txs); i++ {
+		exitIdx, exitAccount, newExit, err := s.processL2Tx(exitTree, &l2txs[i])
 		if err != nil {
 			return nil, nil, err
 		}
@@ -259,6 +256,9 @@ func (s *StateDB) processL1Tx(exitTree *merkletree.MerkleTree, tx *common.L1Tx)
 		if err != nil {
 			return nil, nil, false, err
 		}
+		// TODO applyCreateAccount will return the created account,
+		// which in the case type==TypeSynchronizer will be added to an
+		// array of created accounts that will be returned
 
 		if s.zki != nil {
 			s.zki.AuxFromIdx[s.i] = s.idx.BigInt() // last s.idx is the one used for creating the new account
@@ -308,10 +308,9 @@ func (s *StateDB) processL1Tx(exitTree *merkletree.MerkleTree, tx *common.L1Tx)
 // the Exit created a new Leaf in the ExitTree.
 func (s *StateDB) processL2Tx(exitTree *merkletree.MerkleTree, tx *common.PoolL2Tx) (*common.Idx, *common.Account, bool, error) {
 	var err error
-	var auxToIdx common.Idx
 	// if tx.ToIdx==0, get toIdx by ToEthAddr or ToBJJ
-	if tx.ToIdx == common.Idx(0) {
-		auxToIdx, err = s.GetIdxByEthAddrBJJ(tx.ToEthAddr, tx.ToBJJ)
+	if tx.ToIdx == common.Idx(0) && tx.AuxToIdx == common.Idx(0) {
+		tx.AuxToIdx, err = s.GetIdxByEthAddrBJJ(tx.ToEthAddr, tx.ToBJJ)
 		if err != nil {
 			log.Error(err)
 			return nil, nil, false, err
@@ -331,7 +330,7 @@ func (s *StateDB) processL2Tx(exitTree *merkletree.MerkleTree, tx *common.PoolL2
 			// use toIdx that can have been filled by tx.ToIdx or
 			// if tx.Idx==0 (this case), toIdx is filled by the Idx
 			// from db by ToEthAddr&ToBJJ
-			s.zki.AuxToIdx[s.i] = auxToIdx.BigInt()
+			s.zki.AuxToIdx[s.i] = tx.AuxToIdx.BigInt()
 		}
 
 		if tx.ToBJJ != nil {
@@ -352,36 +351,31 @@ func (s *StateDB) processL2Tx(exitTree *merkletree.MerkleTree, tx *common.PoolL2
 		s.zki.R8y[s.i] = tx.Signature.R8.Y
 	}
 
-	// if StateDB type==TypeSynchronizer, will need to add Nonce and
-	// TokenID to the transaction
+	// if StateDB type==TypeSynchronizer, will need to add Nonce
 	if s.typ == TypeSynchronizer {
+		// as type==TypeSynchronizer, always tx.ToIdx!=0
 		acc, err := s.GetAccount(tx.FromIdx)
 		if err != nil {
 			return nil, nil, false, err
 		}
 		tx.Nonce = acc.Nonce
-		tx.TokenID = acc.TokenID
+		// TokenID is also not set in the L2Txs from the blockchain
+		// that the Synchronizer works with, but does not need to be
+		// defined because is not used later for the L2Txs processing
+		// (Transfer & Exit)
 	}
 
 	switch tx.Type {
 	case common.TxTypeTransfer:
 		// go to the MT account of sender and receiver, and update
 		// balance & nonce
-		tmpTx, err := tx.Tx()
-		if err != nil {
-			return nil, nil, false, err
-		}
-		err = s.applyTransfer(tmpTx, auxToIdx)
+		err = s.applyTransfer(tx.Tx(), tx.AuxToIdx)
 		if err != nil {
 			return nil, nil, false, err
 		}
 	case common.TxTypeExit:
 		// execute exit flow
-		tmpTx, err := tx.Tx()
-		if err != nil {
-			return nil, nil, false, err
-		}
-		exitAccount, newExit, err := s.applyExit(exitTree, tmpTx)
+		exitAccount, newExit, err := s.applyExit(exitTree, tx.Tx())
 		if err != nil {
 			return nil, nil, false, err
 		}
@@ -498,7 +492,7 @@ func (s *StateDB) applyDeposit(tx *common.L1Tx, transfer bool) error {
 // tx.ToIdx==0, then toIdx!=0, and will be used the toIdx parameter as Idx of
 // the receiver. This parameter is used when the tx.ToIdx is not specified and
 // the real ToIdx is found trhrough the ToEthAddr or ToBJJ.
-func (s *StateDB) applyTransfer(tx *common.Tx, auxToIdx common.Idx) error {
+func (s *StateDB) applyTransfer(tx common.Tx, auxToIdx common.Idx) error {
 	if auxToIdx == 0 {
 		auxToIdx = tx.ToIdx
 	}
@@ -622,7 +616,7 @@ func (s *StateDB) applyCreateAccountDepositTransfer(tx *common.L1Tx) error {
 
 // It returns the ExitAccount and a boolean determining if the Exit created a
 // new Leaf in the ExitTree.
-func (s *StateDB) applyExit(exitTree *merkletree.MerkleTree, tx *common.Tx) (*common.Account, bool, error) {
+func (s *StateDB) applyExit(exitTree *merkletree.MerkleTree, tx common.Tx) (*common.Account, bool, error) {
 	// 0. subtract tx.Amount from current Account in StateMT
 	// add the tx.Amount into the Account (tx.FromIdx) in the ExitMT
 	acc, err := s.GetAccount(tx.FromIdx)

test/transakcio/txs.go

@@ -180,11 +180,7 @@ func (tc *TestContext) GenerateBlocks(set string) []BlockData {
 			if err != nil {
 				panic(err)
 			}
-			nL2Tx, err := nTx.L2Tx()
-			if err != nil {
-				panic(err)
-			}
-			tx = *nL2Tx
+			tx = nTx.L2Tx()
 			tx.BatchNum = common.BatchNum(currBatchNum) // when converted to PoolL2Tx BatchNum parameter is lost
 
 			currBatch.L2Txs = append(currBatch.L2Txs, tx)
@@ -201,11 +197,7 @@ func (tc *TestContext) GenerateBlocks(set string) []BlockData {
 			if err != nil {
 				panic(err)
 			}
-			nL2Tx, err := nTx.L2Tx()
-			if err != nil {
-				panic(err)
-			}
-			tx = *nL2Tx
+			tx = nTx.L2Tx()
 			currBatch.L2Txs = append(currBatch.L2Txs, tx)
 		case common.TxTypeForceExit:
 			tx := common.L1Tx{

txselector/txselector.go

@@ -264,10 +264,22 @@ func checkAlreadyPendingToCreate(l1CoordinatorTxs []common.L1Tx, addr ethCommon.
 	return false
 }
 
+// getL2Profitable returns the profitable selection of L2Txssorted by Nonce
 func (txsel *TxSelector) getL2Profitable(txs txs, max uint64) txs {
 	sort.Sort(txs)
 	if len(txs) < int(max) {
 		return txs
 	}
-	return txs[:max]
+	txs = txs[:max]
+
+	// sort l2Txs by Nonce. This can be done in many different ways, what
+	// is needed is to output the txs where the Nonce of txs for each
+	// Account is sorted, but the txs can not be grouped by sender Account
+	// neither by Fee. This is because later on the Nonces will need to be
+	// sequential for the zkproof generation.
+	sort.SliceStable(txs, func(i, j int) bool {
+		return txs[i].Nonce < txs[j].Nonce
+	})
+
+	return txs
 }

txselector/txselector_test.go

@@ -2,7 +2,7 @@ package txselector
 
 /*
    TODO update transactions generation
-func initTest(t *testing.T, testSet string) *TxSelector {
+func initTest(t *testing.T, testSet string, maxL1UserTxs, maxL1OperatorTxs, maxTxs uint64) *TxSelector {
 	pass := os.Getenv("POSTGRES_PASS")
 	db, err := dbUtils.InitSQLDB(5432, "localhost", "hermez", pass, "hermez")
 	require.Nil(t, err)
@@ -15,7 +15,7 @@ func initTest(t *testing.T, testSet string) *TxSelector {
 
 	txselDir, err := ioutil.TempDir("", "tmpTxSelDB")
 	require.Nil(t, err)
-	txsel, err := NewTxSelector(txselDir, sdb, l2DB, 100, 100, 1000)
+	txsel, err := NewTxSelector(txselDir, sdb, l2DB, maxL1UserTxs, maxL1OperatorTxs, maxTxs)
 	require.Nil(t, err)
 
 	return txsel
@@ -29,15 +29,15 @@ func addL2Txs(t *testing.T, txsel *TxSelector, poolL2Txs []common.PoolL2Tx) {
 
 func addTokens(t *testing.T, tokens []common.Token, db *sqlx.DB) {
 	hdb := historydb.NewHistoryDB(db)
-	assert.NoError(t, hdb.Reorg(-1))
-	assert.NoError(t, hdb.AddBlock(&common.Block{
+	assert.Nil(t, hdb.Reorg(-1))
+	assert.Nil(t, hdb.AddBlock(&common.Block{
 		EthBlockNum: 1,
 	}))
-	assert.NoError(t, hdb.AddTokens(tokens))
+	assert.Nil(t, hdb.AddTokens(tokens))
 }
 
 func TestGetL2TxSelection(t *testing.T) {
-	txsel := initTest(t, transakcio.SetPool0)
+	txsel := initTest(t, transakcio.SetPool0, 5, 5, 10)
 	test.CleanL2DB(txsel.l2db.DB())
 
 	   	// generate test transactions
@@ -54,6 +54,12 @@ func TestGetL2TxSelection(t *testing.T) {
 	   	_, _, _, err = txsel.GetL1L2TxSelection(0, l1Txs[0])
 	   	assert.Nil(t, err)
 
+		// TODO once L2DB is updated to return error in case that AddTxTest
+		// fails, and the Transakcio is updated, update this test, checking that the
+		// selected PoolL2Tx are correctly sorted by Nonce
+
+
+
 	   	// txs, err := txsel.GetL2TxSelection(0)
 	   	// assert.Nil(t, err)
 	   	// for _, tx := range txs {