From 3a2a2fcbef5afd4ac7fe010d1fb1d0a049227e9e Mon Sep 17 00:00:00 2001 From: Joshua Gutow Date: Fri, 5 Feb 2021 14:38:58 -0500 Subject: [PATCH] Improve non-native currency fee tx handling (#2) Improves multi-currency fee support in the tx pool queue and pending lists. Previously when paying for a tx in a non-native currency, the balance of the account would only be checked when inserting the transaction into the tx pool, but as the balance of the account can change after the tx is admitted, transactions are periodically checked for validity in `txList.Filter`. This PR changes txList to properly track balances in non-native currencies. This also adds support for checking the gas price minimum in the txList. This PR also converts gas prices to CELO prior to comparing if a new transaction at the same nonce should override an old transaction at the same nonce. It also refactors calculations of `tx.Fee()` --- core/tx_list.go | 176 ++++++++++++++++++++++++++++++++------ core/tx_pool.go | 41 +++++++-- core/types/transaction.go | 8 ++ 3 files changed, 194 insertions(+), 31 deletions(-) diff --git a/core/tx_list.go b/core/tx_list.go index 8a0f4b5a54..0d5e77be77 100644 --- a/core/tx_list.go +++ b/core/tx_list.go @@ -224,17 +224,23 @@ type txList struct { strict bool // Whether nonces are strictly continuous or not txs *txSortedMap // Heap indexed sorted hash map of the transactions - costcap *big.Int // Price of the highest costing transaction (reset only if exceeds balance) - gascap uint64 // Gas limit of the highest spending transaction (reset only if exceeds block limit) + nativecostcap *big.Int // Price of the highest costing transaction paid with native fees (reset only if exceeds balance) + feecaps map[common.Address]*big.Int // Price of the highest costing transaction per fee currency (reset only if exceeds balance) + nativegaspricefloor *big.Int // Lowest gas price minimum in the native currency + gaspricefloors map[common.Address]*big.Int // Lowest gas price minimum per currency (reset only if it is below the gpm) + gascap uint64 // Gas limit of the highest spending transaction (reset only if exceeds block limit) } // newTxList create a new transaction list for maintaining nonce-indexable fast, // gapped, sortable transaction lists. func newTxList(strict bool) *txList { return &txList{ - strict: strict, - txs: newTxSortedMap(), - costcap: new(big.Int), + strict: strict, + txs: newTxSortedMap(), + nativecostcap: new(big.Int), + feecaps: make(map[common.Address]*big.Int), + nativegaspricefloor: nil, + gaspricefloors: make(map[common.Address]*big.Int), } } @@ -244,27 +250,75 @@ func (l *txList) Overlaps(tx *types.Transaction) bool { return l.txs.Get(tx.Nonce()) != nil } +// FeeCurrencies returns a list of each fee currency used to pay for gas in the txList +func (l *txList) FeeCurrencies() []common.Address { + var feeCurrencies []common.Address + for feeCurrency := range l.feecaps { + feeCurrencies = append(feeCurrencies, feeCurrency) + } + return feeCurrencies +} + // Add tries to insert a new transaction into the list, returning whether the // transaction was accepted, and if yes, any previous transaction it replaced. // -// If the new transaction is accepted into the list, the lists' cost and gas -// thresholds are also potentially updated. +// If the new transaction is accepted into the list, the lists' cost, gas and +// gasPriceMinimum thresholds are also potentially updated. func (l *txList) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transaction) { // If there's an older better transaction, abort old := l.txs.Get(tx.Nonce()) + var err error if old != nil { - threshold := new(big.Int).Div(new(big.Int).Mul(old.GasPrice(), big.NewInt(100+int64(priceBump))), big.NewInt(100)) + var oldPrice, newPrice *big.Int + // Short circuit conversion if both are the same currency + if old.FeeCurrency() == tx.FeeCurrency() { + oldPrice = old.GasPrice() + newPrice = tx.GasPrice() + } else { + if fc := old.FeeCurrency(); fc != nil { + if oldPrice, err = currency.ConvertToGold(old.GasPrice(), fc); err != nil { + return false, nil + } + } else { + oldPrice = old.GasPrice() + } + if fc := tx.FeeCurrency(); fc != nil { + if newPrice, err = currency.ConvertToGold(tx.GasPrice(), fc); err != nil { + return false, nil + } + } else { + newPrice = tx.GasPrice() + } + } + threshold := new(big.Int).Div(new(big.Int).Mul(oldPrice, big.NewInt(100+int64(priceBump))), big.NewInt(100)) // Have to ensure that the new gas price is higher than the old gas // price as well as checking the percentage threshold to ensure that // this is accurate for low (Wei-level) gas price replacements - if old.GasPrice().Cmp(tx.GasPrice()) >= 0 || threshold.Cmp(tx.GasPrice()) > 0 { + if oldPrice.Cmp(newPrice) >= 0 || threshold.Cmp(newPrice) > 0 { return false, nil } } // Otherwise overwrite the old transaction with the current one + // caps can only increase and floors can only decrease in this function l.txs.Put(tx) - if cost := tx.Cost(); l.costcap.Cmp(cost) < 0 { - l.costcap = cost + if feeCurrency := tx.FeeCurrency(); feeCurrency == nil { + if cost := tx.Cost(); l.nativecostcap.Cmp(cost) < 0 { + l.nativecostcap = cost + } + if gasPrice := tx.GasPrice(); l.nativegaspricefloor == nil || l.nativegaspricefloor.Cmp(gasPrice) > 0 { + l.nativegaspricefloor = gasPrice + } + } else { + fee := tx.Fee() + if oldFee, ok := l.feecaps[*feeCurrency]; !ok || oldFee.Cmp(fee) < 0 { + l.feecaps[*feeCurrency] = fee + } + if gasFloor, ok := l.gaspricefloors[*feeCurrency]; !ok || gasFloor.Cmp(tx.GasPrice()) > 0 { + l.gaspricefloors[*feeCurrency] = tx.GasPrice() + } + if value := tx.Value(); l.nativecostcap.Cmp(value) < 0 { + l.nativecostcap = value + } } if gas := tx.Gas(); l.gascap < gas { l.gascap = gas @@ -287,29 +341,99 @@ func (l *txList) Forward(threshold uint64) types.Transactions { // This method uses the cached costcap and gascap to quickly decide if there's even // a point in calculating all the costs or if the balance covers all. If the threshold // is lower than the costgas cap, the caps will be reset to a new high after removing -// the newly invalidated transactions. -func (l *txList) Filter(costLimit *big.Int, gasLimit uint64) (types.Transactions, types.Transactions) { - if costLimit == nil { - costLimit = l.costcap +func (l *txList) Filter(nativeCostLimit, nativeGasPriceMinimum *big.Int, feeLimits, gasPriceMinimums map[common.Address]*big.Int, gasLimit uint64) (types.Transactions, types.Transactions) { + // native gas price floor is not necessarily set in txList.Add unlike the rest of caps/floors + if l.nativegaspricefloor == nil { + l.nativegaspricefloor = new(big.Int).Set(nativeGasPriceMinimum) + } + // check if we can bail & lower caps & raise floors at the same time + canBail := true + // Ensure that the cost cap <= the cost limit + if l.nativecostcap.Cmp(nativeCostLimit) > 0 { + canBail = false + l.nativecostcap = new(big.Int).Set(nativeCostLimit) + } + // Ensure that native gas price floor >= the native gas price minimum + if l.nativegaspricefloor.Cmp(nativeGasPriceMinimum) < 0 { + canBail = false + l.nativegaspricefloor = new(big.Int).Set(nativeGasPriceMinimum) + } + // Ensure that the gas cap <= the gas limit + if l.gascap > gasLimit { + canBail = false + l.gascap = gasLimit + } + // Ensure that each cost cap <= the per currency cost limit. + for feeCurrency, feeLimit := range feeLimits { + if l.feecaps[feeCurrency].Cmp(feeLimit) > 0 { + canBail = false + l.feecaps[feeCurrency] = new(big.Int).Set(feeLimit) + } } - // If all transactions are below the threshold, short circuit - if l.costcap.Cmp(costLimit) <= 0 && l.gascap <= gasLimit { + // Ensure that each gas price floor >= the gas price minimum. + for feeCurrency, gasPriceMinimum := range gasPriceMinimums { + if l.gaspricefloors[feeCurrency].Cmp(gasPriceMinimum) < 0 { + canBail = false + l.gaspricefloors[feeCurrency] = new(big.Int).Set(gasPriceMinimum) + } + } + if canBail { return nil, nil } - l.costcap = new(big.Int).Set(costLimit) // Lower the caps to the thresholds - l.gascap = gasLimit // Filter out all the transactions above the account's funds removed := l.txs.Filter(func(tx *types.Transaction) bool { - if tx.FeeCurrency() == nil { - log.Trace("Transaction Filter", "hash", tx.Hash(), "Fee currency", tx.FeeCurrency(), "Cost", tx.Cost(), "Cost Limit", costLimit, "Gas", tx.Gas(), "Gas Limit", gasLimit) - return tx.Cost().Cmp(costLimit) > 0 || tx.Gas() > gasLimit + if feeCurrency := tx.FeeCurrency(); feeCurrency == nil { + log.Trace("Transaction Filter", "hash", tx.Hash(), "Fee currency", tx.FeeCurrency(), "Cost", tx.Cost(), "Cost Limit", nativeCostLimit, "Gas", tx.Gas(), "Gas Limit", gasLimit) + return tx.Cost().Cmp(nativeCostLimit) > 0 || tx.Gas() > gasLimit || tx.GasPrice().Cmp(nativeGasPriceMinimum) < 0 } else { - // If the fees are being paid in the non-native currency, ensure that the `tx.Value` is less than costLimit - // as the fees will be deducted in the non-native currency. - log.Trace("Transaction Filter", "hash", tx.Hash(), "Fee currency", tx.FeeCurrency(), "Value", tx.Value(), "Cost Limit", costLimit, "Gas", tx.Gas(), "Gas Limit", gasLimit) - return tx.Value().Cmp(costLimit) > 0 || tx.Gas() > gasLimit + feeLimit := feeLimits[*feeCurrency] + fee := tx.Fee() + log.Trace("Transaction Filter", "hash", tx.Hash(), "Fee currency", tx.FeeCurrency(), "Value", tx.Value(), "Cost Limit", feeLimit, "Gas", tx.Gas(), "Gas Limit", gasLimit) + // If any of the following is true, the transaction is invalid + // The fees are greater than or equal to the balance in the currency + return fee.Cmp(feeLimit) >= 0 || + // The value of the tx is greater than the native balance of the account + tx.Value().Cmp(nativeCostLimit) > 0 || + // The gas price is less than the gas price minimum + tx.GasPrice().Cmp(gasPriceMinimums[*feeCurrency]) < 0 || + // The gas used is greater than the gas limit + tx.Gas() > gasLimit + } + }) + + // If the list was strict, filter anything above the lowest nonce + var invalids types.Transactions + + if l.strict && len(removed) > 0 { + lowest := uint64(math.MaxUint64) + for _, tx := range removed { + if nonce := tx.Nonce(); lowest > nonce { + lowest = nonce + } } + invalids = l.txs.Filter(func(tx *types.Transaction) bool { return tx.Nonce() > lowest }) + } + return removed, invalids +} + +// FilterOnGasLimit removes all transactions from the list with a gas limit higher +// than the provided thresholds. Every removed transaction is returned for any +// post-removal maintenance. Strict-mode invalidated transactions are also +// returned. +// +// This method uses the cached gascap to quickly decide if there's even +// a point in calculating all the gas used +func (l *txList) FilterOnGasLimit(gasLimit uint64) (types.Transactions, types.Transactions) { + // We can bail if the gas cap <= the gas limit + if l.gascap <= gasLimit { + return nil, nil + } + l.gascap = gasLimit + + // Filter out all the transactions above the account's funds + removed := l.txs.Filter(func(tx *types.Transaction) bool { + return tx.Gas() > gasLimit }) // If the list was strict, filter anything above the lowest nonce diff --git a/core/tx_pool.go b/core/tx_pool.go index f5882bbe0b..7ae70e6091 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -492,7 +492,7 @@ func (pool *TxPool) SetGasLimit(gasLimit uint64) { pool.demoteUnexecutables() for _, list := range pool.queue { - rm, _ := list.Filter(nil, gasLimit) + rm, _ := list.FilterOnGasLimit(gasLimit) for _, tx := range rm { pool.removeTx(tx.Hash(), false) } @@ -1286,6 +1286,14 @@ func (pool *TxPool) reset(oldHead, newHead *types.Header) { func (pool *TxPool) promoteExecutables(accounts []common.Address) []*types.Transaction { // Track the promoted transactions to broadcast them at once var promoted []*types.Transaction + // build gas price minimums + gasPriceMinimums := make(map[common.Address]*big.Int) + allCurrencies, _ := currency.CurrencyWhitelist(nil, nil) + for _, currency := range allCurrencies { + gasPriceMinimum, _ := gpm.GetGasPriceMinimum(¤cy, nil, nil) + gasPriceMinimums[currency] = gasPriceMinimum + } + nativeGPM, _ := gpm.GetGasPriceMinimum(nil, nil, nil) // Iterate over all accounts and promote any executable transactions for _, addr := range accounts { @@ -1300,8 +1308,15 @@ func (pool *TxPool) promoteExecutables(accounts []common.Address) []*types.Trans pool.all.Remove(hash) log.Trace("Removed old queued transaction", "hash", hash) } + // Get balances in each currency + balances := make(map[common.Address]*big.Int) + allCurrencies := list.FeeCurrencies() + for _, feeCurrency := range allCurrencies { + feeCurrencyBalance, _, _ := currency.GetBalanceOf(addr, feeCurrency, params.MaxGasToReadErc20Balance, nil, nil) + balances[feeCurrency] = feeCurrencyBalance + } // Drop all transactions that are too costly (low balance or out of gas) - drops, _ := list.Filter(pool.currentState.GetBalance(addr), pool.currentMaxGas) + drops, _ := list.Filter(pool.currentState.GetBalance(addr), nativeGPM, balances, gasPriceMinimums, pool.currentMaxGas) for _, tx := range drops { hash := tx.Hash() pool.all.Remove(hash) @@ -1482,6 +1497,15 @@ func (pool *TxPool) truncateQueue() { // executable/pending queue and any subsequent transactions that become unexecutable // are moved back into the future queue. func (pool *TxPool) demoteUnexecutables() { + // build gas price minimums + gasPriceMinimums := make(map[common.Address]*big.Int) + allCurrencies, _ := currency.CurrencyWhitelist(nil, nil) + for _, currency := range allCurrencies { + gasPriceMinimum, _ := gpm.GetGasPriceMinimum(¤cy, nil, nil) + gasPriceMinimums[currency] = gasPriceMinimum + } + nativeGPM, _ := gpm.GetGasPriceMinimum(nil, nil, nil) + // Iterate over all accounts and demote any non-executable transactions for addr, list := range pool.pending { nonce := pool.currentState.GetNonce(addr) @@ -1493,8 +1517,15 @@ func (pool *TxPool) demoteUnexecutables() { pool.all.Remove(hash) log.Trace("Removed old pending transaction", "hash", hash) } + // Get balances in each currency + balances := make(map[common.Address]*big.Int) + allCurrencies := list.FeeCurrencies() + for _, feeCurrency := range allCurrencies { + feeCurrencyBalance, _, _ := currency.GetBalanceOf(addr, feeCurrency, params.MaxGasToReadErc20Balance, nil, nil) + balances[feeCurrency] = feeCurrencyBalance + } // Drop all transactions that are too costly (low balance or out of gas), and queue any invalids back for later - drops, invalids := list.Filter(pool.currentState.GetBalance(addr), pool.currentMaxGas) + drops, invalids := list.Filter(pool.currentState.GetBalance(addr), nativeGPM, balances, gasPriceMinimums, pool.currentMaxGas) for _, tx := range drops { hash := tx.Hash() log.Trace("Removed unpayable pending transaction", "hash", hash) @@ -1545,10 +1576,10 @@ func ValidateTransactorBalanceCoversTx(tx *types.Transaction, from common.Addres return err } - gasFee := new(big.Int).Mul(tx.GasPrice(), big.NewInt(int64(tx.Gas()))) // To match the logic in canPayFee() state_transition.go, we require the balance to be strictly greater than the fee, // which means we reject the transaction if balance <= fee - if feeCurrencyBalance.Cmp(new(big.Int).Add(gasFee, tx.GatewayFee())) <= 0 { + fee := tx.Fee() + if feeCurrencyBalance.Cmp(fee) <= 0 { log.Debug("validateTx insufficient fee currency", "feeCurrency", tx.FeeCurrency(), "feeCurrencyBalance", feeCurrencyBalance) return ErrInsufficientFunds } diff --git a/core/types/transaction.go b/core/types/transaction.go index 5c9f4f973d..010988a725 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -193,6 +193,10 @@ func (tx *Transaction) GatewayFee() *big.Int { return tx.data.Ga func (tx *Transaction) Value() *big.Int { return new(big.Int).Set(tx.data.Amount) } func (tx *Transaction) Nonce() uint64 { return tx.data.AccountNonce } func (tx *Transaction) CheckNonce() bool { return true } +func (tx *Transaction) Fee() *big.Int { + gasFee := new(big.Int).Mul(tx.data.Price, big.NewInt(int64(tx.data.GasLimit))) + return gasFee.Add(gasFee, tx.data.GatewayFee) +} // To returns the recipient address of the transaction. // It returns nil if the transaction is a contract creation. @@ -461,3 +465,7 @@ func (m Message) Gas() uint64 { return m.gasLimit } func (m Message) Nonce() uint64 { return m.nonce } func (m Message) Data() []byte { return m.data } func (m Message) CheckNonce() bool { return m.checkNonce } +func (m Message) Fee() *big.Int { + gasFee := new(big.Int).Mul(m.gasPrice, big.NewInt(int64(m.gasLimit))) + return gasFee.Add(gasFee, m.gatewayFee) +}