Fix txselector TransferToBJJ behaviour
Eduard S eduard@iden3.io
Thu, 11 Feb 2021 17:35:35 +0100
3 files changed,
177 insertions(+),
22 deletions(-)
M
common/accountcreationauths.go
→
common/accountcreationauths.go
@@ -17,6 +17,11 @@
// 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 {
M
txselector/txselector.go
→
txselector/txselector.go
@@ -3,7 +3,6 @@
// current: very simple version of TxSelector import ( - "bytes" "fmt" "math/big" "sort"@@ -236,7 +235,8 @@ // if there is no space for the L1CoordinatorTx, discard the L2Tx
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 @@ if !enoughBalance {
// 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 @@ } else {
// 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 @@ validL2Tx, l1CoordinatorTx, accAuth, err :=
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 @@ log.Debugw("invalid L2Tx: ToIdx not found in StateDB",
"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 @@ l2Txs[i].ToEthAddr, "account.EthAddr", receiverAcc.EthAddr)
// 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 @@ "account.BJJ", receiverAcc.BJJ)
// 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 @@ // in a near future.
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 @@ }
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 @@ Amount: big.NewInt(0),
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 @@ }
} 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 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
M
txselector/txselector_test.go
→
txselector/txselector_test.go
@@ -48,7 +48,7 @@ Addr: coordUser.Addr,
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 @@ assert.Equal(t, common.TxTypeExit, oL2Txs[0].Type)
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) +}