From c7e6267189bf45c5ca4d07697f7091433a0c5cf2 Mon Sep 17 00:00:00 2001 From: Eduard S Date: Thu, 11 Feb 2021 17:35:35 +0100 Subject: [PATCH] Fix txselector TransferToBJJ behaviour --- common/accountcreationauths.go | 5 ++ txselector/txselector.go | 61 +++++++++------ txselector/txselector_test.go | 133 ++++++++++++++++++++++++++++++++- 3 files changed, 177 insertions(+), 22 deletions(-) diff --git a/common/accountcreationauths.go b/common/accountcreationauths.go index 21b7c1d..0e588dc 100644 --- a/common/accountcreationauths.go +++ b/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 { diff --git a/txselector/txselector.go b/txselector/txselector.go index c3780cd..4bf5f41 100644 --- a/txselector/txselector.go +++ b/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 diff --git a/txselector/txselector_test.go b/txselector/txselector_test.go index 5170e5e..900aaa2 100644 --- a/txselector/txselector_test.go +++ b/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) +}