From 1d31b1edc1c3fbcf07b9ed989ddfe2510c6c8ece Mon Sep 17 00:00:00 2001 From: Karl Bartel Date: Fri, 3 May 2024 14:54:31 +0200 Subject: [PATCH] txpool: Check for fee currency and native cost The previous implementation only checked for the costs in fee currency or in native token, but both can happen at the same time when a fee currency tx is created that also sends native tokens to a contract. Closes https://github.com/celo-org/op-geth/issues/110 --- core/txpool/blobpool/blobpool.go | 14 +++---- core/txpool/legacypool/celo_list.go | 1 + core/txpool/legacypool/legacypool.go | 21 ++++++---- core/txpool/legacypool/list.go | 35 +++++++++++------ core/txpool/legacypool/list_test.go | 3 +- core/txpool/validation.go | 57 ++++++++++++++++++++-------- core/types/transaction.go | 23 +++++++++-- 7 files changed, 107 insertions(+), 47 deletions(-) diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go index 2e3be8c484..470cf7633b 100644 --- a/core/txpool/blobpool/blobpool.go +++ b/core/txpool/blobpool/blobpool.go @@ -117,7 +117,7 @@ func newBlobTxMeta(id uint64, size uint32, tx *types.Transaction) *blobTxMeta { id: id, size: size, nonce: tx.Nonce(), - costCap: uint256.MustFromBig(tx.Cost()), + costCap: uint256.MustFromBig(tx.NativeCost()), execTipCap: uint256.MustFromBig(tx.GasTipCap()), execFeeCap: uint256.MustFromBig(tx.GasFeeCap()), blobFeeCap: uint256.MustFromBig(tx.BlobGasFeeCap()), @@ -1114,18 +1114,18 @@ func (p *BlobPool) validateTx(tx *types.Transaction) error { } return have, maxTxsPerAccount - have }, - ExistingExpenditure: func(addr common.Address) *big.Int { + ExistingExpenditure: func(addr common.Address) (*big.Int, *big.Int) { if spent := p.spent[addr]; spent != nil { - return spent.ToBig() + return new(big.Int), spent.ToBig() } - return new(big.Int) + return new(big.Int), new(big.Int) }, - ExistingCost: func(addr common.Address, nonce uint64) *big.Int { + ExistingCost: func(addr common.Address, nonce uint64) (*big.Int, *big.Int) { next := p.state.GetNonce(addr) if uint64(len(p.index[addr])) > nonce-next { - return p.index[addr][int(tx.Nonce()-next)].costCap.ToBig() + return new(big.Int), p.index[addr][int(tx.Nonce()-next)].costCap.ToBig() } - return nil + return nil, nil }, ExistingBalance: func(addr common.Address, feeCurrency *common.Address) *big.Int { return contracts.GetFeeBalance(p.celoBackend, addr, feeCurrency) diff --git a/core/txpool/legacypool/celo_list.go b/core/txpool/legacypool/celo_list.go index 05212074df..3884a772cf 100644 --- a/core/txpool/legacypool/celo_list.go +++ b/core/txpool/legacypool/celo_list.go @@ -43,6 +43,7 @@ func (l *list) dropInvalidsAfterRemovalAndReheap(removed types.Transactions) typ func (l *list) FeeCurrencies() []common.Address { currencySet := make(map[common.Address]interface{}) + currencySet[getCurrencyKey(nil)] = struct{}{} // Always include native token to handle potential value transfers for _, tx := range l.txs.items { // native currency (nil) represented as Zero address currencySet[getCurrencyKey(tx.FeeCurrency())] = struct{}{} diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index a2717d8cfa..7abd75203a 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -673,25 +673,30 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction, local bool) error { } return have, math.MaxInt }, - ExistingExpenditure: func(addr common.Address) *big.Int { + ExistingExpenditure: func(addr common.Address) (*big.Int, *big.Int) { if list := pool.pending[addr]; list != nil { - return list.TotalCostFor(tx.FeeCurrency()).ToBig() + return list.TotalCostFor(tx.FeeCurrency()).ToBig(), list.TotalCostFor(nil).ToBig() } - return new(big.Int) + return new(big.Int), new(big.Int) }, - ExistingCost: func(addr common.Address, nonce uint64) *big.Int { + ExistingCost: func(addr common.Address, nonce uint64) (*big.Int, *big.Int) { if list := pool.pending[addr]; list != nil { + feeCurrency := tx.FeeCurrency() if tx := list.txs.Get(nonce); tx != nil { - cost := tx.Cost() + feeCurrencyCost, nativeCost := tx.Cost() if pool.l1CostFn != nil { if l1Cost := pool.l1CostFn(tx.RollupCostData()); l1Cost != nil { // add rollup cost - cost = cost.Add(cost, l1Cost) + nativeCost = nativeCost.Add(nativeCost, l1Cost) } } - return cost + if tx.FeeCurrency() != feeCurrency { + // We are only interested in costs in the same currency + feeCurrencyCost = new(big.Int) + } + return feeCurrencyCost, nativeCost } } - return nil + return nil, nil }, L1CostFn: pool.l1CostFn, ExistingBalance: func(addr common.Address, feeCurrency *common.Address) *big.Int { diff --git a/core/txpool/legacypool/list.go b/core/txpool/legacypool/list.go index 93a2a6d2c8..65a8ae83d4 100644 --- a/core/txpool/legacypool/list.go +++ b/core/txpool/legacypool/list.go @@ -345,13 +345,20 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64, _ txpool.L1CostFunc, // Old is being replaced, subtract old cost l.subTotalCost([]*types.Transaction{old}) } - // Add new tx cost to totalcost - cost, overflow := uint256.FromBig(tx.Cost()) + // Add new tx cost to total cost + feeCurrencyTc := l.totalCostVar(tx.FeeCurrency()) + nativeTc := l.totalCostVar(&common.ZeroAddress) + feeCurrencyCostBig, nativeCostBig := tx.Cost() + feeCurrencyCost, overflow := uint256.FromBig(feeCurrencyCostBig) if overflow { return false, nil } - tc := l.totalCostVar(tx.FeeCurrency()) - tc.Add(tc, cost) + nativeCost, overflow := uint256.FromBig(nativeCostBig) + if overflow { + return false, nil + } + feeCurrencyTc.Add(feeCurrencyTc, feeCurrencyCost) + nativeTc.Add(nativeTc, nativeCost) // TODO: manage l1 cost // if l1CostFn != nil { // if l1Cost := l1CostFn(tx.RollupDataGas()); l1Cost != nil { // add rollup cost @@ -360,7 +367,8 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64, _ txpool.L1CostFunc, // } // Otherwise overwrite the old transaction with the current one l.txs.Put(tx) - l.updateCostCapFor(tx.FeeCurrency(), cost) + l.updateCostCapFor(tx.FeeCurrency(), feeCurrencyCost) + l.updateCostCapFor(&common.ZeroAddress, nativeCost) if gas := tx.Gas(); l.gascap < gas { l.gascap = gas } @@ -395,8 +403,8 @@ func (l *list) Filter(costLimits map[common.Address]*uint256.Int, gasLimit uint6 // Filter out all the transactions above the account's funds removed := l.txs.Filter(func(tx *types.Transaction) bool { - costcap := l.costCapFor(tx.FeeCurrency()) - return tx.Gas() > gasLimit || tx.Cost().Cmp(costcap.ToBig()) > 0 + feeCurrencyCost, nativeCost := tx.Cost() + return tx.Gas() > gasLimit || feeCurrencyCost.Cmp(l.costCapFor(tx.FeeCurrency()).ToBig()) > 0 || nativeCost.Cmp(l.costCapFor(&common.ZeroAddress).ToBig()) > 0 }) if len(removed) == 0 { @@ -478,11 +486,16 @@ func (l *list) LastElement() *types.Transaction { // total cost of all transactions. func (l *list) subTotalCost(txs []*types.Transaction) { for _, tx := range txs { - // _, underflow := l.totalcost.SubOverflow(l.totalcost, uint256.MustFromBig(tx.Cost())) - tc := l.totalCostVar(tx.FeeCurrency()) - _, underflow := tc.SubOverflow(tc, uint256.MustFromBig(tx.Cost())) + feeCurrencyCost, nativeCost := tx.Cost() + feeCurrencyTc := l.totalCostVar(tx.FeeCurrency()) + nativeTc := l.totalCostVar(&common.ZeroAddress) + _, underflow := feeCurrencyTc.SubOverflow(feeCurrencyTc, uint256.MustFromBig(feeCurrencyCost)) + if underflow { + panic("totalcost underflow (feecurrency)") + } + _, underflow = nativeTc.SubOverflow(nativeTc, uint256.MustFromBig(nativeCost)) if underflow { - panic("totalcost underflow") + panic("totalcost underflow (native)") } } } diff --git a/core/txpool/legacypool/list_test.go b/core/txpool/legacypool/list_test.go index e494925061..55537e5965 100644 --- a/core/txpool/legacypool/list_test.go +++ b/core/txpool/legacypool/list_test.go @@ -62,7 +62,8 @@ func TestListAddVeryExpensive(t *testing.T) { gasprice, _ := new(big.Int).SetString("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", 0) gaslimit := uint64(i) tx, _ := types.SignTx(types.NewTransaction(uint64(i), common.Address{}, value, gaslimit, gasprice, nil), types.HomesteadSigner{}, key) - t.Logf("cost: %x bitlen: %d\n", tx.Cost(), tx.Cost().BitLen()) + costFeeCurrency, costNative := tx.Cost() + t.Logf("cost: %x %x\n", costFeeCurrency, costNative) list.Add(tx, DefaultConfig.PriceBump, nil, nil) } } diff --git a/core/txpool/validation.go b/core/txpool/validation.go index 695fe6f815..f8eebeeb55 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -217,11 +217,21 @@ type ValidationOptionsWithState struct { // ExistingExpenditure is a mandatory callback to retrieve the cumulative // cost of the already pooled transactions to check for overdrafts. - ExistingExpenditure func(addr common.Address) *big.Int + // Returns (feeCurrencySpent, nativeSpent), where feeCurrencySpent only + // includes the spending for txs with the relevant feeCurrency. + // The feeCurrency relevant for feeCurrencySpent is fixed for every + // instance of ExistingExpenditure and therefore not passed in as a + // parameter. + ExistingExpenditure func(addr common.Address) (*big.Int, *big.Int) // ExistingCost is a mandatory callback to retrieve an already pooled // transaction's cost with the given nonce to check for overdrafts. - ExistingCost func(addr common.Address, nonce uint64) *big.Int + // Returns (feeCurrencyCost, nativeCost), where feeCurrencyCost only + // includes the spending for txs with the relevant feeCurrency. + // The feeCurrency relevant for feeCurrencyCost is fixed for every + // instance of ExistingCost, and therefore not passed in as a + // parameter. + ExistingCost func(addr common.Address, nonce uint64) (*big.Int, *big.Int) // L1CostFn is an optional extension, to validate L1 rollup costs of a tx L1CostFn L1CostFunc @@ -255,30 +265,45 @@ func ValidateTransactionWithState(tx *types.Transaction, signer types.Signer, op } // Ensure the transactor has enough funds to cover the transaction costs var ( - balance = opts.ExistingBalance(from, tx.FeeCurrency()) - cost = tx.Cost() + feeCurrencyBalance = opts.ExistingBalance(from, tx.FeeCurrency()) + nativeBalance = opts.ExistingBalance(from, &common.ZeroAddress) + feeCurrencyCost, nativeCost = tx.Cost() ) if opts.L1CostFn != nil { if l1Cost := opts.L1CostFn(tx.RollupCostData()); l1Cost != nil { // add rollup cost - cost = cost.Add(cost, l1Cost) + nativeCost = nativeCost.Add(nativeCost, l1Cost) } } - if balance.Cmp(cost) < 0 { - return fmt.Errorf("%w: balance %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, balance, cost, new(big.Int).Sub(cost, balance)) + if feeCurrencyBalance.Cmp(feeCurrencyCost) < 0 { + return fmt.Errorf("%w: balance %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, feeCurrencyBalance, feeCurrencyCost, new(big.Int).Sub(feeCurrencyCost, feeCurrencyBalance)) + } + if nativeBalance.Cmp(nativeCost) < 0 { + return fmt.Errorf("%w: balance %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, nativeBalance, nativeCost, new(big.Int).Sub(nativeCost, nativeBalance)) } // Ensure the transactor has enough funds to cover for replacements or nonce // expansions without overdrafts - spent := opts.ExistingExpenditure(from) - if prev := opts.ExistingCost(from, tx.Nonce()); prev != nil { - bump := new(big.Int).Sub(cost, prev) - need := new(big.Int).Add(spent, bump) - if balance.Cmp(need) < 0 { - return fmt.Errorf("%w: balance %v, queued cost %v, tx bumped %v, overshot %v", core.ErrInsufficientFunds, balance, spent, bump, new(big.Int).Sub(need, balance)) + feeCurrencySpent, nativeSpent := opts.ExistingExpenditure(from) + if feeCurrencyPrev, nativePrev := opts.ExistingCost(from, tx.Nonce()); feeCurrencyPrev != nil { + // Costs from all transactions refer to the same currency, + // which is ensured by ExistingCost and ExistingExpenditure. + feeCurrencyBump := new(big.Int).Sub(feeCurrencyCost, feeCurrencyPrev) + feeCurrencyNeed := new(big.Int).Add(feeCurrencySpent, feeCurrencyBump) + nativeBump := new(big.Int).Sub(nativeCost, nativePrev) + nativeNeed := new(big.Int).Add(nativeSpent, nativeBump) + if feeCurrencyBalance.Cmp(feeCurrencyNeed) < 0 { + return fmt.Errorf("%w: balance %v, queued cost %v, tx bumped %v, overshot %v, feeCurrency %v", core.ErrInsufficientFunds, feeCurrencyBalance, feeCurrencySpent, feeCurrencyBump, new(big.Int).Sub(feeCurrencyNeed, feeCurrencyBalance), tx.FeeCurrency()) + } + if nativeBalance.Cmp(nativeNeed) < 0 { + return fmt.Errorf("%w: balance %v, queued cost %v, tx bumped %v, overshot %v", core.ErrInsufficientFunds, nativeBalance, nativeSpent, nativeBump, new(big.Int).Sub(nativeNeed, nativeBalance)) } } else { - need := new(big.Int).Add(spent, cost) - if balance.Cmp(need) < 0 { - return fmt.Errorf("%w: balance %v, queued cost %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, balance, spent, cost, new(big.Int).Sub(need, balance)) + feeCurrencyNeed := new(big.Int).Add(feeCurrencySpent, feeCurrencyCost) + nativeNeed := new(big.Int).Add(nativeSpent, nativeCost) + if feeCurrencyBalance.Cmp(feeCurrencyNeed) < 0 { + return fmt.Errorf("%w: balance %v, queued cost %v, tx cost %v, overshot %v, feeCurrency %v", core.ErrInsufficientFunds, feeCurrencyBalance, feeCurrencySpent, feeCurrencyCost, new(big.Int).Sub(feeCurrencyNeed, feeCurrencyBalance), tx.FeeCurrency()) + } + if nativeBalance.Cmp(nativeNeed) < 0 { + return fmt.Errorf("%w: balance %v, queued cost %v, tx cost %v, overshot %v", core.ErrInsufficientFunds, nativeBalance, nativeSpent, nativeCost, new(big.Int).Sub(nativeNeed, nativeBalance)) } // Transaction takes a new nonce value out of the pool. Ensure it doesn't // overflow the number of permitted transactions from a single account diff --git a/core/types/transaction.go b/core/types/transaction.go index 7959113963..1c2524bd5c 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -367,14 +367,29 @@ func (tx *Transaction) IsSystemTx() bool { return tx.inner.isSystemTx() } -// Cost returns (gas * gasPrice) + (blobGas * blobGasPrice) + value. -func (tx *Transaction) Cost() *big.Int { +// Cost returns both components of the tx costs: +// - cost in feeCurrency: (gas * gasPrice) + (blobGas * blobGasPrice) +// - native token cost: value sent to target contract +// For non-feeCurrency transactions, the first value is zero and the second is the total cost. +func (tx *Transaction) Cost() (*big.Int, *big.Int) { total := new(big.Int).Mul(tx.GasPrice(), new(big.Int).SetUint64(tx.Gas())) if tx.Type() == BlobTxType { total.Add(total, new(big.Int).Mul(tx.BlobGasFeeCap(), new(big.Int).SetUint64(tx.BlobGas()))) } - total.Add(total, tx.Value()) - return total + if tx.FeeCurrency() == nil { + nativeCost := total.Add(total, tx.Value()) + return new(big.Int), nativeCost + } else { + // Will need to be updated for CIP-66 + nativeCost := tx.Value() + return total, nativeCost + } +} + +// Returns the native token component of the transaction cost. +func (tx *Transaction) NativeCost() *big.Int { + _, nativeCost := tx.Cost() + return nativeCost } // RollupCostData caches the information needed to efficiently compute the data availability fee