arnaucube
arnaucube/hermez-nodePublic Self-hosted repository
Merge pull request #504 from hermeznetwork/feature/txsel-checkenoughbalance

Check enough funds on sender at TxSelector and don't include the tx in the selection
Eduard S eduard@iden3.io
Fri, 29 Jan 2021 13:17:48 +0100
commit

bc831af6a8b730c0a2f360afd1c73e8e5a1a1c8c

parent

30973aa292ef7010ba50efb63c61dcab6be6cbb9

M synchronizer/synchronizer_test.go → synchronizer/synchronizer_test.go

@@ -408,9 +408,9 @@ ForceExit(1) A: 100

ForceExit(1) B: 80 ForceTransfer(1) A-D: 100 - Transfer(1) C-A: 100 (200) - Exit(1) C: 50 (200) - Exit(1) D: 30 (200) + Transfer(1) C-A: 100 (126) + Exit(1) C: 50 (100) + Exit(1) D: 30 (100) > batchL1 // forge L1UserTxs{nil}, freeze defined L1UserTxs{3} > batchL1 // forge L1UserTxs{3}, freeze defined L1UserTxs{nil}
M txprocessor/txprocessor.go → txprocessor/txprocessor.go

@@ -55,6 +55,18 @@ CoordinatorIdxsMap map[common.TokenID]common.Idx

CollectedFees map[common.TokenID]*big.Int } +func newErrorNotEnoughBalance(tx common.Tx) error { + var msg error + if tx.IsL1 { + msg = fmt.Errorf("Invalid transaction, not enough balance on sender account. TxID: %s, TxType: %s, FromIdx: %d, ToIdx: %d, Amount: %d", + tx.TxID, tx.Type, tx.FromIdx, tx.ToIdx, tx.Amount) + } else { + msg = fmt.Errorf("Invalid transaction, not enough balance on sender account. TxID: %s, TxType: %s, FromIdx: %d, ToIdx: %d, Amount: %d, Fee: %d", + tx.TxID, tx.Type, tx.FromIdx, tx.ToIdx, tx.Amount, tx.Fee) + } + return tracerr.Wrap(msg) +} + // NewTxProcessor returns a new TxProcessor with the given *StateDB & Config func NewTxProcessor(sdb *statedb.StateDB, config Config) *TxProcessor { return &TxProcessor{

@@ -769,6 +781,9 @@ // add the deposit to the sender

accSender.Balance = new(big.Int).Add(accSender.Balance, tx.EffectiveDepositAmount) // subtract amount to the sender accSender.Balance = new(big.Int).Sub(accSender.Balance, tx.EffectiveAmount) + if accSender.Balance.Cmp(big.NewInt(0)) == -1 { // balance<0 + return newErrorNotEnoughBalance(tx.Tx()) + } // update sender account in localStateDB p, err := tp.s.UpdateAccount(tx.FromIdx, accSender)

@@ -862,6 +877,9 @@ return tracerr.Wrap(err)

} feeAndAmount := new(big.Int).Add(tx.Amount, fee) accSender.Balance = new(big.Int).Sub(accSender.Balance, feeAndAmount) + if accSender.Balance.Cmp(big.NewInt(0)) == -1 { // balance<0 + return newErrorNotEnoughBalance(tx) + } if _, ok := coordIdxsMap[accSender.TokenID]; ok { accCoord, err := tp.s.GetAccount(coordIdxsMap[accSender.TokenID])

@@ -882,6 +900,9 @@ log.Debugw("No coord Idx to receive fee", "tx", tx)

} } else { accSender.Balance = new(big.Int).Sub(accSender.Balance, tx.Amount) + if accSender.Balance.Cmp(big.NewInt(0)) == -1 { // balance<0 + return newErrorNotEnoughBalance(tx) + } } // update sender account in localStateDB

@@ -962,6 +983,9 @@ }

// subtract amount to the sender accSender.Balance = new(big.Int).Sub(accSender.Balance, tx.EffectiveAmount) + if accSender.Balance.Cmp(big.NewInt(0)) == -1 { // balance<0 + return newErrorNotEnoughBalance(tx.Tx()) + } // create Account of the Sender p, err := tp.s.CreateAccount(common.Idx(tp.s.CurrentIdx()+1), accSender)

@@ -1057,6 +1081,9 @@ return nil, false, tracerr.Wrap(err)

} feeAndAmount := new(big.Int).Add(tx.Amount, fee) acc.Balance = new(big.Int).Sub(acc.Balance, feeAndAmount) + if acc.Balance.Cmp(big.NewInt(0)) == -1 { // balance<0 + return nil, false, newErrorNotEnoughBalance(tx) + } if _, ok := coordIdxsMap[acc.TokenID]; ok { accCoord, err := tp.s.GetAccount(coordIdxsMap[acc.TokenID])

@@ -1078,6 +1105,9 @@ log.Debugw("No coord Idx to receive fee", "tx", tx)

} } else { acc.Balance = new(big.Int).Sub(acc.Balance, tx.Amount) + if acc.Balance.Cmp(big.NewInt(0)) == -1 { // balance<0 + return nil, false, newErrorNotEnoughBalance(tx) + } } p, err := tp.s.UpdateAccount(tx.FromIdx, acc)

@@ -1268,3 +1298,18 @@ tx.EffectiveAmount = big.NewInt(0)

return } } + +// CheckEnoughBalance returns true if the sender of the transaction has enough +// balance in the account to send the Amount+Fee +func (tp *TxProcessor) CheckEnoughBalance(tx common.PoolL2Tx) bool { + acc, err := tp.s.GetAccount(tx.FromIdx) + if err != nil { + return false + } + fee, err := common.CalcFeeAmount(tx.Amount, tx.Fee) + if err != nil { + return false + } + feeAndAmount := new(big.Int).Add(tx.Amount, fee) + return acc.Balance.Cmp(feeAndAmount) != -1 // !=-1 balance<amount +}
M txselector/txselector.go → txselector/txselector.go

@@ -239,10 +239,15 @@

var validTxs []common.PoolL2Tx // iterate over l2TxsRaw // - check Nonces + // - check enough Balance for the Amount+Fee // - if needed, create new L1CoordinatorTxs for unexisting ToIdx // - keep used accAuths // - put the valid txs into validTxs array for i := 0; i < len(l2Txs); i++ { + if !tp.CheckEnoughBalance(l2Txs[i]) { + // not valid Amount with current Balance + continue + } // check if Nonce is correct nonce := noncesMap[l2Txs[i].FromIdx] if l2Txs[i].Nonce == nonce {

@@ -319,9 +324,9 @@ }

// get CoordIdxsMap for the TokenIDs coordIdxsMap := make(map[common.TokenID]common.Idx) - for i := 0; i < len(l2Txs); i++ { + for i := 0; i < len(validTxs); i++ { // get TokenID from tx.Sender - accSender, err := tp.StateDB().GetAccount(l2Txs[i].FromIdx) + accSender, err := tp.StateDB().GetAccount(validTxs[i].FromIdx) if err != nil { return nil, nil, nil, nil, nil, tracerr.Wrap(err) }

@@ -330,7 +335,7 @@

coordIdx, err := txsel.getCoordIdx(tokenID) if err != nil { // if err is db.ErrNotFound, should not happen, as all - // the l2Txs.TokenID should have a CoordinatorIdx + // the validTxs.TokenID should have a CoordinatorIdx // created in the DB at this point return nil, nil, nil, nil, nil, tracerr.Wrap(err) }

@@ -352,8 +357,8 @@ // get most profitable L2-tx

maxL2Txs := int(selectionConfig.TxProcessorConfig.MaxTx) - len(l1UserTxs) - len(l1CoordinatorTxs) - selectedL2Txs := l2Txs - if len(l2Txs) > maxL2Txs { + selectedL2Txs := validTxs + if len(validTxs) > maxL2Txs { selectedL2Txs = selectedL2Txs[:maxL2Txs] } var finalL2Txs []common.PoolL2Tx
M txselector/txselector_test.go → txselector/txselector_test.go

@@ -378,3 +378,107 @@ checkSortedByNonce(t, testAccNonces, oL2Txs)

err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(poolL2Txs), txsel.localAccountsDB.CurrentBatch()) require.NoError(t, err) } + +func TestPoolL2TxsWithoutEnoughBalance(t *testing.T) { + set := ` + Type: Blockchain + + CreateAccountDeposit(0) Coord: 0 + CreateAccountDeposit(0) A: 100 + CreateAccountDeposit(0) B: 100 + + > batchL1 // freeze L1User{1} + > batchL1 // forge L1User{1} + > block + ` + + chainID := uint16(0) + tc := til.NewContext(chainID, common.RollupConstMaxL1UserTx) + // generate test transactions, the L1CoordinatorTxs generated by Til + // will be ignored at this test, as will be the TxSelector who + // generates them when needed + 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() + + tpc := txprocessor.Config{ + NLevels: 16, + MaxFeeTx: 10, + MaxTx: 20, + MaxL1Tx: 10, + ChainID: chainID, + } + selectionConfig := &SelectionConfig{ + MaxL1UserTxs: 5, + TxProcessorConfig: tpc, + } + // batch1 + l1UserTxs := []common.L1Tx{} + _, _, _, _, _, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs) + require.NoError(t, err) + + // batch2 + // prepare the PoolL2Txs + batchPoolL2 := ` + Type: PoolL2 + PoolTransferToEthAddr(0) A-B: 100 (126) + PoolExit(0) B: 100 (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, err := txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs) + require.NoError(t, err) + assert.Equal(t, 3, len(oL1UserTxs)) + assert.Equal(t, 0, len(oL1CoordTxs)) + assert.Equal(t, 0, len(oL2Txs)) // should be 0 as the 2 PoolL2Txs does not have enough funds + err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(oL2Txs), txsel.localAccountsDB.CurrentBatch()) + require.NoError(t, err) + + // as the PoolL2Txs have not been really processed, restart nonces + tc.RestartNonces() + + // batch3 + // NOTE: this batch will result with 1 L2Tx, as the PoolExit tx is not + // possible, as the PoolTransferToEthAddr is not processed yet when + // checking availability of PoolExit. This, in a near-future iteration + // of the TxSelector will return the 2 transactions as valid and + // selected, as the TxSelector will handle this kind of combinations. + batchPoolL2 = ` + Type: PoolL2 + PoolTransferToEthAddr(0) A-B: 50 (126)` + poolL2Txs, err = tc.GeneratePoolL2Txs(batchPoolL2) + require.NoError(t, err) + addL2Txs(t, txsel, poolL2Txs) + + l1UserTxs = []common.L1Tx{} + _, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs) + require.NoError(t, err) + assert.Equal(t, 0, len(oL1UserTxs)) + assert.Equal(t, 0, len(oL1CoordTxs)) + assert.Equal(t, 1, len(oL2Txs)) // see 'NOTE' at the beginning of 'batch3' of this test + assert.Equal(t, common.TxTypeTransferToEthAddr, oL2Txs[0].Type) + err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(oL2Txs), txsel.localAccountsDB.CurrentBatch()) + require.NoError(t, err) + + // batch4 + // make the selection of another batch, which should include the + // initial PoolExit, which now is valid as B has enough Balance + l1UserTxs = []common.L1Tx{} + _, _, oL1UserTxs, oL1CoordTxs, oL2Txs, err = txsel.GetL1L2TxSelection(selectionConfig, l1UserTxs) + require.NoError(t, err) + assert.Equal(t, 0, len(oL1UserTxs)) + assert.Equal(t, 0, len(oL1CoordTxs)) + assert.Equal(t, 1, len(oL2Txs)) + assert.Equal(t, common.TxTypeExit, oL2Txs[0].Type) + err = txsel.l2db.StartForging(common.TxIDsFromPoolL2Txs(oL2Txs), txsel.localAccountsDB.CurrentBatch()) + require.NoError(t, err) +}