diff --git a/contract_comm/currency/currency.go b/contract_comm/currency/currency.go index d8cd7336ef6e4..a71c288f4b7fe 100644 --- a/contract_comm/currency/currency.go +++ b/contract_comm/currency/currency.go @@ -108,6 +108,10 @@ type exchangeRate struct { } func ConvertToGold(val *big.Int, currencyFrom *common.Address) (*big.Int, error) { + if currencyFrom == nil { + return val, nil + } + celoGoldAddress, err := contract_comm.GetRegisteredAddress(params.GoldTokenRegistryId, nil, nil) if err == errors.ErrSmartContractNotDeployed || err == errors.ErrRegistryContractNotDeployed { log.Warn("Registry address lookup failed", "err", err) @@ -115,12 +119,14 @@ func ConvertToGold(val *big.Int, currencyFrom *common.Address) (*big.Int, error) } if currencyFrom == celoGoldAddress { - return val, err + // This function shouldn't really be called with the token's address, but if it is the value + // is correct, so return nil as the error + return val, nil } else if err != nil { log.Error(err.Error()) return val, err } - return Convert(val, currencyFrom, celoGoldAddress) + return Convert(val, currencyFrom, nil) } // NOTE (jarmg 4/24/19): values are rounded down which can cause @@ -143,6 +149,12 @@ func Convert(val *big.Int, currencyFrom *common.Address, currencyTo *common.Addr // (val * d1 * n2) / (n1 * d2) numerator := new(big.Int).Mul(val, new(big.Int).Mul(exchangeRateFrom.Denominator, exchangeRateTo.Numerator)) denominator := new(big.Int).Mul(exchangeRateFrom.Numerator, exchangeRateTo.Denominator) + + if denominator.Sign() == 0 { + log.Error("Convert - Error in retreiving currency exchange rates") + return nil, errors.ErrExchangeRateZero + } + return new(big.Int).Div(numerator, denominator), nil } diff --git a/contract_comm/errors/errors.go b/contract_comm/errors/errors.go index 0d8925356c36e..f0e2bc45e6f97 100644 --- a/contract_comm/errors/errors.go +++ b/contract_comm/errors/errors.go @@ -9,4 +9,5 @@ var ( ErrSmartContractNotDeployed = errors.New("Contract not in Registry") ErrRegistryContractNotDeployed = errors.New("Registry not deployed") ErrNoInternalEvmHandlerSingleton = errors.New("No internalEvmHandlerSingleton set for contract communication") + ErrExchangeRateZero = errors.New("Exchange rate returned from the network is zero") ) diff --git a/core/tx_list.go b/core/tx_list.go index 18514e7227a54..eb31770425e66 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, gasPriceFloor := range l.gaspricefloors { + if gasPriceMinimum := gasPriceMinimums[feeCurrency]; gasPriceFloor.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 c0b914a424ea3..6bc9fde261971 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -480,7 +480,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) } @@ -1280,6 +1280,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 { @@ -1294,8 +1302,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) @@ -1476,6 +1491,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) @@ -1487,8 +1511,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) @@ -1539,8 +1570,10 @@ func ValidateTransactorBalanceCoversTx(tx *types.Transaction, from common.Addres return err } - gasFee := new(big.Int).Mul(tx.GasPrice(), big.NewInt(int64(tx.Gas()))) - if feeCurrencyBalance.Cmp(new(big.Int).Add(gasFee, tx.GatewayFee())) < 0 { + // 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 + 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 721ce8bbc8ed5..b29e4ace11023 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) +}