Fix fee currency bugs in tx pool

core/celo_genesis.go

@@ -41,8 +41,8 @@ var (
 	DevAddr          = common.BytesToAddress(DevAddr32.Bytes())
 	DevAddr32        = common.HexToHash("0x42cf1bbc38BaAA3c4898ce8790e21eD2738c6A4a")
 
-	DevFeeCurrencyAddr  = common.HexToAddress("0xce16") // worth twice as much as native CELO
-	DevFeeCurrencyAddr2 = common.HexToAddress("0xce17") // worth half as much as native CELO
+	DevFeeCurrencyAddr  = common.HexToAddress("0xce16") // worth half as much as native CELO
+	DevFeeCurrencyAddr2 = common.HexToAddress("0xce17") // worth twice as much as native CELO
 	DevBalance, _       = new(big.Int).SetString("100000000000000000000", 10)
 	rateNumerator, _    = new(big.Int).SetString("2000000000000000000000000", 10)
 	rateNumerator2, _   = new(big.Int).SetString("500000000000000000000000", 10)

core/txpool/legacypool/celo_list.go

@@ -125,3 +125,10 @@ func (h *priceHeap) GetNativeBaseFee() *big.Int {
 	}
 	return h.ratesAndFees.GetNativeBaseFee()
 }
+
+func (h *priceHeap) GetBaseFeeIn(feeCurrency *common.Address) *big.Int {
+	if h.ratesAndFees == nil {
+		return nil
+	}
+	return h.ratesAndFees.GetBaseFeeIn(feeCurrency)
+}

core/txpool/legacypool/legacypool.go

@@ -28,6 +28,7 @@ import (
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/exchange"
 	"github.com/ethereum/go-ethereum/common/prque"
 	"github.com/ethereum/go-ethereum/consensus/misc/eip1559"
 	"github.com/ethereum/go-ethereum/contracts"
@@ -575,7 +576,8 @@ func (pool *LegacyPool) Pending(filter txpool.PendingFilter) map[common.Address]
 		// If the miner requests tip enforcement, cap the lists now
 		if minTipBig != nil && !pool.locals.contains(addr) {
 			for i, tx := range txs {
-				if tx.EffectiveGasTipIntCmp(minTipBig, pool.priced.urgent.GetNativeBaseFee()) < 0 {
+				minTipInFeeCurrency, err := exchange.ConvertCeloToCurrency(pool.feeCurrencyContext.ExchangeRates, tx.FeeCurrency(), minTipBig)
+				if err != nil || tx.EffectiveGasTipIntCmp(minTipInFeeCurrency, pool.priced.urgent.GetBaseFeeIn(tx.FeeCurrency())) < 0 {
 					txs = txs[:i]
 					break
 				}

e2e_test/js-tests/send_tx.mjs

@@ -85,7 +85,6 @@ async function main() {
     gas: 90000,
     feeCurrency,
     maxFeePerGas: 2000000000n,
-    maxPriorityFeePerGas: 0n,
   });
 
   var hash;

e2e_test/js-tests/test_viem_tx.mjs

@@ -132,7 +132,7 @@ describe("viem send tx", () => {
 			gas: 90000,
 			feeCurrency: process.env.FEE_CURRENCY,
 			maxFeePerGas: 2000000000n,
-			maxPriorityFeePerGas: 0n,
+			maxPriorityFeePerGas: 2n,
 		});
 		const signature = await walletClient.signTransaction(request);
 		const hash = await walletClient.sendRawTransaction({
@@ -188,7 +188,7 @@ describe("viem send tx", () => {
 			value: 2,
 			feeCurrency: process.env.FEE_CURRENCY,
 			maxFeePerGas: 2000000000n,
-			maxPriorityFeePerGas: 0n,
+			maxPriorityFeePerGas: 2n,
 		});
 		const signature = await walletClient.signTransaction(request);
 		const hash = await walletClient.sendRawTransaction({
@@ -253,7 +253,7 @@ describe("viem send tx", () => {
 			gas: 90000,
 			feeCurrency: "0x000000000000000000000000000000000badc310",
 			maxFeePerGas: 1000000000n,
-			maxPriorityFeePerGas: 0n,
+			maxPriorityFeePerGas: 1n,
 		});
 		const signature = await walletClient.signTransaction(request);
 		try {
@@ -270,4 +270,28 @@ describe("viem send tx", () => {
 			}
 		}
 	}).timeout(10_000);
+
+	it("send fee currency tx with just high enough gas price", async () => {
+		const block = await publicClient.getBlock({});
+		// Actually, the base fee will be a bit lower next block, since our blocks
+		// are always mostly empty. But the difference will be much less than the
+		// exchange rate of 2, so the test will still check that we take the fee
+		// currency into account everywhere.
+		const maxFeePerGas = block.baseFeePerGas / 2n;
+		const request = await walletClient.prepareTransactionRequest({
+			account,
+			to: "0x00000000000000000000000000000000DeaDBeef",
+			value: 2,
+			gas: 90000,
+			feeCurrency: process.env.FEE_CURRENCY2,
+			maxFeePerGas,
+			maxPriorityFeePerGas: 1n,
+		});
+		const signature = await walletClient.signTransaction(request);
+		const hash = await walletClient.sendRawTransaction({
+			serializedTransaction: signature,
+		});
+		const receipt = await publicClient.waitForTransactionReceipt({ hash });
+		assert.equal(receipt.status, "success", "receipt status 'failure'");
+	}).timeout(5_000);
 });

e2e_test/run_all_tests.sh

@@ -10,7 +10,7 @@ TEST_GLOB=$1
 cd "$SCRIPT_DIR/.." || exit 1
 make geth
 trap 'kill %%' EXIT # kill bg job at exit
-build/bin/geth --dev --http --http.api eth,web3,net &>"$SCRIPT_DIR/geth.log" &
+build/bin/geth --dev --http --http.api eth,web3,net --txpool.nolocals &>"$SCRIPT_DIR/geth.log" &
 
 # Wait for geth to be ready
 for _ in {1..10}; do

e2e_test/shared.sh

@@ -11,6 +11,7 @@ export REGISTRY_ADDR=0x000000000000000000000000000000000000ce10
 export TOKEN_ADDR=0x471ece3750da237f93b8e339c536989b8978a438
 export FEE_CURRENCY_DIRECTORY_ADDR=0x9212Fb72ae65367A7c887eC4Ad9bE310BAC611BF
 export FEE_CURRENCY=0x000000000000000000000000000000000000ce16
+export FEE_CURRENCY2=0x000000000000000000000000000000000000ce17
 export FEE_HANDLER=0xcd437749e43a154c07f3553504c68fbfd56b8778
 export ORACLE3=0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb0003
 

miner/ordering.go

@@ -21,6 +21,7 @@ import (
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/exchange"
 	"github.com/ethereum/go-ethereum/core/txpool"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/holiman/uint256"
@@ -36,17 +37,36 @@ type txWithMinerFee struct {
 // newTxWithMinerFee creates a wrapped transaction, calculating the effective
 // miner gasTipCap if a base fee is provided.
 // Returns error in case of a negative effective miner gasTipCap.
-func newTxWithMinerFee(tx *txpool.LazyTransaction, from common.Address, baseFee *uint256.Int) (*txWithMinerFee, error) {
+func newTxWithMinerFee(tx *txpool.LazyTransaction, from common.Address, baseFee *uint256.Int, rates common.ExchangeRates) (*txWithMinerFee, error) {
 	tip := new(uint256.Int).Set(tx.GasTipCap)
 	if baseFee != nil {
-		if tx.GasFeeCap.Cmp(baseFee) < 0 {
+		baseFeeConverted := baseFee
+		if tx.FeeCurrency != nil {
+			baseFeeBig, err := exchange.ConvertCeloToCurrency(rates, tx.FeeCurrency, baseFee.ToBig())
+			if err != nil {
+				return nil, err
+			}
+			baseFeeConverted = uint256.MustFromBig(baseFeeBig)
+		}
+
+		if tx.GasFeeCap.Cmp(baseFeeConverted) < 0 {
 			return nil, types.ErrGasFeeCapTooLow
 		}
 		tip = new(uint256.Int).Sub(tx.GasFeeCap, baseFee)
 		if tip.Gt(tx.GasTipCap) {
 			tip = tx.GasTipCap
 		}
 	}
+
+	// Convert tip back into celo if the transaction is in a different currency
+	if tx.FeeCurrency != nil {
+		tipBig, err := exchange.ConvertCurrencyToCelo(rates, tx.FeeCurrency, tip.ToBig())
+		if err != nil {
+			return nil, err
+		}
+		tip = uint256.MustFromBig(tipBig)
+	}
+
 	return &txWithMinerFee{
 		tx:   tx,
 		from: from,
@@ -87,18 +107,19 @@ func (s *txByPriceAndTime) Pop() interface{} {
 // transactions in a profit-maximizing sorted order, while supporting removing
 // entire batches of transactions for non-executable accounts.
 type transactionsByPriceAndNonce struct {
-	txs     map[common.Address][]*txpool.LazyTransaction // Per account nonce-sorted list of transactions
-	heads   txByPriceAndTime                             // Next transaction for each unique account (price heap)
-	signer  types.Signer                                 // Signer for the set of transactions
-	baseFee *uint256.Int                                 // Current base fee
+	txs           map[common.Address][]*txpool.LazyTransaction // Per account nonce-sorted list of transactions
+	heads         txByPriceAndTime                             // Next transaction for each unique account (price heap)
+	signer        types.Signer                                 // Signer for the set of transactions
+	baseFee       *uint256.Int                                 // Current base fee
+	exchangeRates common.ExchangeRates
 }
 
 // newTransactionsByPriceAndNonce creates a transaction set that can retrieve
 // price sorted transactions in a nonce-honouring way.
 //
 // Note, the input map is reowned so the caller should not interact any more with
 // if after providing it to the constructor.
-func newTransactionsByPriceAndNonce(signer types.Signer, txs map[common.Address][]*txpool.LazyTransaction, baseFee *big.Int) *transactionsByPriceAndNonce {
+func newTransactionsByPriceAndNonce(signer types.Signer, txs map[common.Address][]*txpool.LazyTransaction, baseFee *big.Int, rates common.ExchangeRates) *transactionsByPriceAndNonce {
 	// Convert the basefee from header format to uint256 format
 	var baseFeeUint *uint256.Int
 	if baseFee != nil {
@@ -107,7 +128,7 @@ func newTransactionsByPriceAndNonce(signer types.Signer, txs map[common.Address]
 	// Initialize a price and received time based heap with the head transactions
 	heads := make(txByPriceAndTime, 0, len(txs))
 	for from, accTxs := range txs {
-		wrapped, err := newTxWithMinerFee(accTxs[0], from, baseFeeUint)
+		wrapped, err := newTxWithMinerFee(accTxs[0], from, baseFeeUint, rates)
 		if err != nil {
 			delete(txs, from)
 			continue
@@ -119,10 +140,11 @@ func newTransactionsByPriceAndNonce(signer types.Signer, txs map[common.Address]
 
 	// Assemble and return the transaction set
 	return &transactionsByPriceAndNonce{
-		txs:     txs,
-		heads:   heads,
-		signer:  signer,
-		baseFee: baseFeeUint,
+		txs:           txs,
+		heads:         heads,
+		signer:        signer,
+		baseFee:       baseFeeUint,
+		exchangeRates: rates,
 	}
 }
 
@@ -138,7 +160,7 @@ func (t *transactionsByPriceAndNonce) Peek() (*txpool.LazyTransaction, *uint256.
 func (t *transactionsByPriceAndNonce) Shift() {
 	acc := t.heads[0].from
 	if txs, ok := t.txs[acc]; ok && len(txs) > 0 {
-		if wrapped, err := newTxWithMinerFee(txs[0], acc, t.baseFee); err == nil {
+		if wrapped, err := newTxWithMinerFee(txs[0], acc, t.baseFee, t.exchangeRates); err == nil {
 			t.heads[0], t.txs[acc] = wrapped, txs[1:]
 			heap.Fix(&t.heads, 0)
 			return

miner/ordering_test.go

@@ -102,7 +102,7 @@ func testTransactionPriceNonceSort(t *testing.T, baseFee *big.Int) {
 		expectedCount += count
 	}
 	// Sort the transactions and cross check the nonce ordering
-	txset := newTransactionsByPriceAndNonce(signer, groups, baseFee)
+	txset := newTransactionsByPriceAndNonce(signer, groups, baseFee, nil)
 
 	txs := types.Transactions{}
 	for tx, _ := txset.Peek(); tx != nil; tx, _ = txset.Peek() {
@@ -168,7 +168,7 @@ func TestTransactionTimeSort(t *testing.T) {
 		})
 	}
 	// Sort the transactions and cross check the nonce ordering
-	txset := newTransactionsByPriceAndNonce(signer, groups, nil)
+	txset := newTransactionsByPriceAndNonce(signer, groups, nil, nil)
 
 	txs := types.Transactions{}
 	for tx, _ := txset.Peek(); tx != nil; tx, _ = txset.Peek() {

miner/worker.go

@@ -62,6 +62,7 @@ type environment struct {
 	gasPool              *core.GasPool      // available gas used to pack transactions
 	multiGasPool         *core.MultiGasPool // available per-fee-currency gas used to pack transactions
 	feeCurrencyAllowlist common.AddressSet
+	exchangeRates        common.ExchangeRates
 	coinbase             common.Address
 
 	header   *types.Header
@@ -273,6 +274,7 @@ func (miner *Miner) prepareWork(genParams *generateParams) (*environment, error)
 		common.CurrencyAllowlist(context.FeeCurrencyContext.ExchangeRates),
 		header,
 	)
+	env.exchangeRates = context.FeeCurrencyContext.ExchangeRates
 	if header.ParentBeaconRoot != nil {
 		vmenv := vm.NewEVM(context, vm.TxContext{}, env.state, miner.chainConfig, vm.Config{})
 		core.ProcessBeaconBlockRoot(*header.ParentBeaconRoot, vmenv, env.state)
@@ -554,16 +556,16 @@ func (miner *Miner) fillTransactions(interrupt *atomic.Int32, env *environment)
 	}
 	// Fill the block with all available pending transactions.
 	if len(localPlainTxs) > 0 || len(localBlobTxs) > 0 {
-		plainTxs := newTransactionsByPriceAndNonce(env.signer, localPlainTxs, env.header.BaseFee)
-		blobTxs := newTransactionsByPriceAndNonce(env.signer, localBlobTxs, env.header.BaseFee)
+		plainTxs := newTransactionsByPriceAndNonce(env.signer, localPlainTxs, env.header.BaseFee, env.exchangeRates)
+		blobTxs := newTransactionsByPriceAndNonce(env.signer, localBlobTxs, env.header.BaseFee, env.exchangeRates)
 
 		if err := miner.commitTransactions(env, plainTxs, blobTxs, interrupt); err != nil {
 			return err
 		}
 	}
 	if len(remotePlainTxs) > 0 || len(remoteBlobTxs) > 0 {
-		plainTxs := newTransactionsByPriceAndNonce(env.signer, remotePlainTxs, env.header.BaseFee)
-		blobTxs := newTransactionsByPriceAndNonce(env.signer, remoteBlobTxs, env.header.BaseFee)
+		plainTxs := newTransactionsByPriceAndNonce(env.signer, remotePlainTxs, env.header.BaseFee, env.exchangeRates)
+		blobTxs := newTransactionsByPriceAndNonce(env.signer, remoteBlobTxs, env.header.BaseFee, env.exchangeRates)
 
 		if err := miner.commitTransactions(env, plainTxs, blobTxs, interrupt); err != nil {
 			return err