Merge pull request #538 from hermeznetwork/fix/txselTransferToBJJ

Fix txselector TransferToBJJ behaviour

common/accountcreationauths.go

@@ -17,6 +17,11 @@ const AccountCreationAuthMsg = "I authorize this babyjubjub key for hermez rollu
 // EthMsgPrefix is the prefix for message signing at the Ethereum ecosystem
 const EthMsgPrefix = "\x19Ethereum Signed Message:\n"
 
+var (
+	// EmptyEthSignature is an ethereum signature of all zeroes
+	EmptyEthSignature = make([]byte, 65)
+)
+
 // AccountCreationAuth authorizations sent by users to the L2DB, to be used for
 // account creations when necessary
 type AccountCreationAuth struct {

txselector/txselector.go

@@ -3,7 +3,6 @@ package txselector
 // current: very simple version of TxSelector
 
 import (
-	"bytes"
 	"fmt"
 	"math/big"
 	"sort"
@@ -236,7 +235,8 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 			if len(l1CoordinatorTxs) >= int(selectionConfig.MaxL1UserTxs)-len(l1UserTxs) {
 				// discard L2Tx, and update Info parameter of
 				// the tx, and add it to the discardedTxs array
-				l2Txs0[i].Info = "Tx not selected due the L2Tx depends on a L1CoordinatorTx and there is not enough space for L1Coordinator"
+				l2Txs0[i].Info = "Tx not selected because the L2Tx depends on a " +
+					"L1CoordinatorTx and there is not enough space for L1Coordinator"
 				discardedL2Txs = append(discardedL2Txs, l2Txs0[i])
 				continue
 			}
@@ -261,7 +261,9 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 			// not valid Amount with current Balance. Discard L2Tx,
 			// and update Info parameter of the tx, and add it to
 			// the discardedTxs array
-			l2Txs[i].Info = fmt.Sprintf("Tx not selected due not enough Balance at the sender. Current sender account Balance: %s, Amount+Fee: %s", balance.String(), feeAndAmount.String())
+			l2Txs[i].Info = fmt.Sprintf("Tx not selected due to not enough Balance at the sender. "+
+				"Current sender account Balance: %s, Amount+Fee: %s",
+				balance.String(), feeAndAmount.String())
 			discardedL2Txs = append(discardedL2Txs, l2Txs[i])
 			continue
 		}
@@ -273,7 +275,8 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 			// not valid Nonce at tx. Discard L2Tx, and update Info
 			// parameter of the tx, and add it to the discardedTxs
 			// array
-			l2Txs[i].Info = fmt.Sprintf("Tx not selected due not current Nonce. Tx.Nonce: %d, Account.Nonce: %d", l2Txs[i].Nonce, nonce)
+			l2Txs[i].Info = fmt.Sprintf("Tx not selected due to not current Nonce. "+
+				"Tx.Nonce: %d, Account.Nonce: %d", l2Txs[i].Nonce, nonce)
 			discardedL2Txs = append(discardedL2Txs, l2Txs[i])
 			continue
 		}
@@ -291,17 +294,30 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 				txsel.processTxToEthAddrBJJ(validTxs, selectionConfig,
 					len(l1UserTxs), l1CoordinatorTxs, positionL1, l2Txs[i])
 			if err != nil {
-				log.Debug(err)
+				log.Debugw("txsel.processTxToEthAddrBJJ", "err", err)
 				// Discard L2Tx, and update Info parameter of
 				// the tx, and add it to the discardedTxs array
-				l2Txs[i].Info = fmt.Sprintf("Tx not selected (in processTxToEthAddrBJJ) due %s", err.Error())
+				l2Txs[i].Info = fmt.Sprintf("Tx not selected (in processTxToEthAddrBJJ) due to %s",
+					err.Error())
 				discardedL2Txs = append(discardedL2Txs, l2Txs[i])
 				continue
 			}
-			if accAuth != nil && l1CoordinatorTx != nil {
-				accAuths = append(accAuths, accAuth.Signature)
-				l1CoordinatorTxs = append(l1CoordinatorTxs, *l1CoordinatorTx)
-				positionL1++
+			if l1CoordinatorTx != nil {
+				// If ToEthAddr == 0xff.. this means that we
+				// are handling a TransferToBJJ, which doesn't
+				// require an authorization because it doesn't
+				// contain a valid ethereum address.
+				// Otherwise only create the account if we have
+				// the corresponding authorization
+				if validL2Tx.ToEthAddr == common.FFAddr {
+					accAuths = append(accAuths, common.EmptyEthSignature)
+					l1CoordinatorTxs = append(l1CoordinatorTxs, *l1CoordinatorTx)
+					positionL1++
+				} else if accAuth != nil {
+					accAuths = append(accAuths, accAuth.Signature)
+					l1CoordinatorTxs = append(l1CoordinatorTxs, *l1CoordinatorTx)
+					positionL1++
+				}
 			}
 			if validL2Tx != nil {
 				validTxs = append(validTxs, *validL2Tx)
@@ -314,8 +330,8 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 					"ToIdx", l2Txs[i].ToIdx)
 				// Discard L2Tx, and update Info parameter of
 				// the tx, and add it to the discardedTxs array
-				l2Txs[i].Info = fmt.Sprintf("Tx not selected due tx.ToIdx not found in StateDB. ToIdx: %d",
-					l2Txs[i].ToIdx)
+				l2Txs[i].Info = fmt.Sprintf("Tx not selected due to tx.ToIdx not found in StateDB. "+
+					"ToIdx: %d", l2Txs[i].ToIdx)
 				discardedL2Txs = append(discardedL2Txs, l2Txs[i])
 				continue
 			}
@@ -327,7 +343,9 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 					// Discard L2Tx, and update Info
 					// parameter of the tx, and add it to
 					// the discardedTxs array
-					l2Txs[i].Info = fmt.Sprintf("Tx not selected due ToEthAddr does not correspond to the Account.EthAddr. tx.ToIdx: %d, tx.ToEthAddr: %s, account.EthAddr: %s",
+					l2Txs[i].Info = fmt.Sprintf("Tx not selected because ToEthAddr "+
+						"does not correspond to the Account.EthAddr. "+
+						"tx.ToIdx: %d, tx.ToEthAddr: %s, account.EthAddr: %s",
 						l2Txs[i].ToIdx, l2Txs[i].ToEthAddr, receiverAcc.EthAddr)
 					discardedL2Txs = append(discardedL2Txs, l2Txs[i])
 					continue
@@ -341,7 +359,9 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 					// Discard L2Tx, and update Info
 					// parameter of the tx, and add it to
 					// the discardedTxs array
-					l2Txs[i].Info = fmt.Sprintf("Tx not selected due tx.ToBJJ does not correspond to the Account.BJJ. tx.ToIdx: %d, tx.ToEthAddr: %s, tx.ToBJJ: %s, account.BJJ: %s",
+					l2Txs[i].Info = fmt.Sprintf("Tx not selected because tx.ToBJJ "+
+						"does not correspond to the Account.BJJ. "+
+						"tx.ToIdx: %d, tx.ToEthAddr: %s, tx.ToBJJ: %s, account.BJJ: %s",
 						l2Txs[i].ToIdx, l2Txs[i].ToEthAddr, l2Txs[i].ToBJJ, receiverAcc.BJJ)
 					discardedL2Txs = append(discardedL2Txs, l2Txs[i])
 					continue
@@ -415,7 +435,7 @@ func (txsel *TxSelector) GetL1L2TxSelection(selectionConfig *SelectionConfig,
 			log.Error(err)
 			// Discard L2Tx, and update Info parameter of the tx,
 			// and add it to the discardedTxs array
-			selectedL2Txs[i].Info = fmt.Sprintf("Tx not selected (in ProcessL2Tx) due %s", err.Error())
+			selectedL2Txs[i].Info = fmt.Sprintf("Tx not selected (in ProcessL2Tx) due to %s", err.Error())
 			discardedL2Txs = append(discardedL2Txs, selectedL2Txs[i])
 			continue
 		}
@@ -471,8 +491,7 @@ func (txsel *TxSelector) processTxToEthAddrBJJ(validTxs []common.PoolL2Tx,
 
 	var l1CoordinatorTx *common.L1Tx
 	var accAuth *common.AccountCreationAuth
-	if !bytes.Equal(l2Tx.ToEthAddr.Bytes(), common.EmptyAddr.Bytes()) &&
-		!bytes.Equal(l2Tx.ToEthAddr.Bytes(), common.FFAddr.Bytes()) {
+	if l2Tx.ToEthAddr != common.EmptyAddr && l2Tx.ToEthAddr != common.FFAddr {
 		// case: ToEthAddr != 0x00 neither 0xff
 		if l2Tx.ToBJJ != common.EmptyBJJComp {
 			// case: ToBJJ!=0:
@@ -528,8 +547,7 @@ func (txsel *TxSelector) processTxToEthAddrBJJ(validTxs []common.PoolL2Tx,
 			DepositAmount: big.NewInt(0),
 			Type:          common.TxTypeCreateAccountDeposit,
 		}
-	} else if bytes.Equal(l2Tx.ToEthAddr.Bytes(), common.FFAddr.Bytes()) &&
-		l2Tx.ToBJJ != common.EmptyBJJComp {
+	} else if l2Tx.ToEthAddr == common.FFAddr && l2Tx.ToBJJ != common.EmptyBJJComp {
 		// if idx exist for EthAddr&BJJ use it
 		_, err := txsel.localAccountsDB.GetIdxByEthAddrBJJ(l2Tx.ToEthAddr, l2Tx.ToBJJ,
 			l2Tx.TokenID)
@@ -555,7 +573,8 @@ func (txsel *TxSelector) processTxToEthAddrBJJ(validTxs []common.PoolL2Tx,
 	}
 	if len(l1CoordinatorTxs) >= int(selectionConfig.MaxL1UserTxs)-nL1UserTxs {
 		// L2Tx discarded
-		return nil, nil, nil, tracerr.Wrap(fmt.Errorf("L2Tx discarded due not slots for L1CoordinatorTx to create a new account for receiver of L2Tx"))
+		return nil, nil, nil, tracerr.Wrap(fmt.Errorf("L2Tx discarded due to no available slots " +
+			"for L1CoordinatorTx to create a new account for receiver of L2Tx"))
 	}
 
 	return &l2Tx, l1CoordinatorTx, accAuth, nil
@@ -564,7 +583,7 @@ func (txsel *TxSelector) processTxToEthAddrBJJ(validTxs []common.PoolL2Tx,
 func checkAlreadyPendingToCreate(l1CoordinatorTxs []common.L1Tx, tokenID common.TokenID,
 	addr ethCommon.Address, bjj babyjub.PublicKeyComp) bool {
 	for i := 0; i < len(l1CoordinatorTxs); i++ {
-		if bytes.Equal(l1CoordinatorTxs[i].FromEthAddr.Bytes(), addr.Bytes()) &&
+		if l1CoordinatorTxs[i].FromEthAddr == addr &&
 			l1CoordinatorTxs[i].TokenID == tokenID &&
 			l1CoordinatorTxs[i].FromBJJ == bjj {
 			return true

txselector/txselector_test.go

@@ -48,7 +48,7 @@ func initTest(t *testing.T, chainID uint16, hermezContractAddr ethCommon.Address
 		BJJ:                 coordUser.BJJ.Public().Compress(),
 		AccountCreationAuth: nil,
 	}
-	fmt.Printf("%v", coordAccount)
+	// fmt.Printf("%v\n", coordAccount)
 	auth := common.AccountCreationAuth{
 		EthAddr: coordUser.Addr,
 		BJJ:     coordUser.BJJ.Public().Compress(),
@@ -497,3 +497,134 @@ func TestPoolL2TxsWithoutEnoughBalance(t *testing.T) {
 	err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(oL2Txs), txsel.localAccountsDB.CurrentBatch())
 	require.NoError(t, err)
 }
+
+func TestTransferToBjj(t *testing.T) {
+	set := `
+		Type: Blockchain
+		AddToken(1)
+
+		CreateAccountDeposit(0) Coord: 0
+		CreateAccountDeposit(0) A: 1000
+		CreateAccountDeposit(0) B: 1000
+		CreateAccountDeposit(1) B: 1000
+
+		> batchL1 // freeze L1User{1}
+		> batchL1 // forge L1User{1}
+		> block
+	`
+
+	chainID := uint16(0)
+	tc := til.NewContext(chainID, common.RollupConstMaxL1UserTx)
+	blocks, err := tc.GenerateBlocks(set)
+	assert.NoError(t, err)
+
+	hermezContractAddr := ethCommon.HexToAddress("0xc344E203a046Da13b0B4467EB7B3629D0C99F6E6")
+	txsel := initTest(t, chainID, hermezContractAddr, tc.Users["Coord"])
+
+	// restart nonces of TilContext, as will be set by generating directly
+	// the PoolL2Txs for each specific batch with tc.GeneratePoolL2Txs
+	tc.RestartNonces()
+
+	addTokens(t, tc, txsel.l2db.DB())
+
+	tpc := txprocessor.Config{
+		NLevels:  16,
+		MaxFeeTx: 10,
+		MaxTx:    20,
+		MaxL1Tx:  10,
+		ChainID:  chainID,
+	}
+	selectionConfig := &SelectionConfig{
+		MaxL1UserTxs:      5,
+		TxProcessorConfig: tpc,
+	}
+	// batch1 to create some accounts with positive balance
+	l1UserTxs := []common.L1Tx{}
+	_, _, _, _, _, _, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
+	require.NoError(t, err)
+
+	// Transfer is ToBJJ to a BJJ-only account that doesn't exist
+	// and the coordinator will create it via L1CoordTx.
+
+	batchPoolL2 := `
+	Type: PoolL2
+	PoolTransferToBJJ(0) A-B: 50 (126)
+	`
+	poolL2Txs, err := tc.GeneratePoolL2Txs(batchPoolL2)
+	require.NoError(t, err)
+
+	// add the PoolL2Txs to the l2DB
+	addL2Txs(t, txsel, poolL2Txs)
+
+	l1UserTxs = til.L1TxsToCommonL1Txs(tc.Queues[*blocks[0].Rollup.Batches[1].Batch.ForgeL1TxsNum])
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, discardedL2Txs, err := txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
+	require.NoError(t, err)
+	assert.Equal(t, 4, len(oL1UserTxs))
+	// We expect the coordinator to add an L1CoordTx to create an account for the recipient of the l2tx
+	require.Equal(t, 1, len(oL1CoordTxs))
+	assert.Equal(t, poolL2Txs[0].ToEthAddr, oL1CoordTxs[0].FromEthAddr)
+	assert.Equal(t, poolL2Txs[0].ToBJJ, oL1CoordTxs[0].FromBJJ)
+	// fmt.Printf("DBG l1CoordTx[0]: %+v\n", oL1CoordTxs[0])
+	assert.Equal(t, 1, len(oL2Txs))
+	assert.Equal(t, 0, len(discardedL2Txs))
+	err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(oL2Txs), txsel.localAccountsDB.CurrentBatch())
+	require.NoError(t, err)
+
+	// Now the BJJ-only account for B is already created, so the transfer
+	// happens without an L1CoordTx that creates the user account.
+
+	batchPoolL2 = `
+	Type: PoolL2
+	PoolTransferToBJJ(0) A-B: 50 (126)
+	`
+
+	poolL2Txs, err = tc.GeneratePoolL2Txs(batchPoolL2)
+	require.NoError(t, err)
+	addL2Txs(t, txsel, poolL2Txs)
+
+	l1UserTxs = []common.L1Tx{}
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, discardedL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
+	require.NoError(t, err)
+	assert.Equal(t, 0, len(oL1UserTxs))
+	// Since the BJJ-only account B already exists, the coordinator doesn't add any L1CoordTxs
+	assert.Equal(t, 0, len(oL1CoordTxs))
+	assert.Equal(t, 1, len(oL2Txs))
+	assert.Equal(t, 0, len(discardedL2Txs))
+	err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(oL2Txs), txsel.localAccountsDB.CurrentBatch())
+	require.NoError(t, err)
+
+	// The transfer now is ToBJJ to a BJJ-only account that doesn't exist
+	// and the coordinator will create it via L1CoordTx.   Since it's a
+	// transfer of a token for which the coordinator doesn't have a fee
+	// account, another L1CoordTx will be created for the coordinator to
+	// receive the fees.
+
+	batchPoolL2 = `
+	Type: PoolL2
+	PoolTransferToBJJ(1) B-A: 50 (126)
+	`
+
+	poolL2Txs, err = tc.GeneratePoolL2Txs(batchPoolL2)
+	require.NoError(t, err)
+	addL2Txs(t, txsel, poolL2Txs)
+
+	l1UserTxs = []common.L1Tx{}
+	_, _, oL1UserTxs, oL1CoordTxs, oL2Txs, discardedL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs)
+	require.NoError(t, err)
+	assert.Equal(t, 0, len(oL1UserTxs))
+	// We expect the coordinator to add an L1CoordTx to create an account
+	// to receive the fees by the coordinator and another one for the
+	// recipient of the l2tx
+	assert.Equal(t, 2, len(oL1CoordTxs))
+	// [0] Coordinator account cration for token 1
+	assert.Equal(t, tc.Users["Coord"].Addr, oL1CoordTxs[0].FromEthAddr)
+	// [1] User A BJJ-only account creation for token 1
+	assert.Equal(t, poolL2Txs[0].ToEthAddr, oL1CoordTxs[1].FromEthAddr)
+	assert.Equal(t, poolL2Txs[0].ToBJJ, oL1CoordTxs[1].FromBJJ)
+	assert.Equal(t, common.TokenID(1), oL1CoordTxs[1].TokenID)
+
+	assert.Equal(t, 1, len(oL2Txs))
+	assert.Equal(t, 0, len(discardedL2Txs))
+	err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(oL2Txs), txsel.localAccountsDB.CurrentBatch())
+	require.NoError(t, err)
+}