From e806411c1d9daf47b8da9b7df44621b8f791d70f Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Wed, 8 Jan 2025 11:32:41 +0100 Subject: [PATCH] Invalidate txs with below base-fee-floor gas-price in txpool (#292) * Invalidate txs with below base-fee-floor gas-price in txpool Fixes #291 Before, the transaction pool accepted transactions that have a gas-price below the configured base-fee-floor, which would never be executed. Now, the transaction pool rejects those transactions immediately - giving the submitting user an immediate response and better UX. * Add check for effective-gas-tip reaches min-tip in txpool validation * Make genesis devmode alloc function public * Allocate Celo balance also for fundedAccount in genesis * Add test for celo-legacypool * Use stateful legacypool validation in test * Fix error/panic messages * Fix min-tip conversion in txpool pre-validation Fixes #304 * Add tests for min-tip validation in txpool * Summarize common fixtures to default vars in txpool test * Fix typo --- common/exchange/rates.go | 2 +- core/blockchain_celo_test.go | 2 +- core/celo_genesis.go | 32 +-- core/genesis.go | 2 +- core/txpool/celo_validation.go | 42 ++++ .../txpool/legacypool/celo_legacypool_test.go | 205 ++++++++++++++++++ core/txpool/validation.go | 7 +- 7 files changed, 274 insertions(+), 18 deletions(-) create mode 100644 core/txpool/legacypool/celo_legacypool_test.go diff --git a/common/exchange/rates.go b/common/exchange/rates.go index 221ba480ef..6c308eb964 100644 --- a/common/exchange/rates.go +++ b/common/exchange/rates.go @@ -34,7 +34,7 @@ func ConvertCurrencyToCelo(exchangeRates common.ExchangeRates, feeCurrency *comm return currencyAmount, nil } if currencyAmount == nil { - return nil, fmt.Errorf("Can't convert nil amount to CELO.") + return nil, fmt.Errorf("could not convert nil amount to CELO") } exchangeRate, ok := exchangeRates[*feeCurrency] if !ok { diff --git a/core/blockchain_celo_test.go b/core/blockchain_celo_test.go index e1b9a85124..d0f2a4bb1d 100644 --- a/core/blockchain_celo_test.go +++ b/core/blockchain_celo_test.go @@ -67,7 +67,7 @@ func testNativeTransferWithFeeCurrency(t *testing.T, scheme string, feeCurrencyA funds = DevBalance gspec = &Genesis{ Config: &config, - Alloc: celoGenesisAccounts(addr1), + Alloc: CeloGenesisAccounts(addr1), } ) gspec.Config.Cel2Time = uint64ptr(0) diff --git a/core/celo_genesis.go b/core/celo_genesis.go index 1d9192772c..1e37f55b8f 100644 --- a/core/celo_genesis.go +++ b/core/celo_genesis.go @@ -41,19 +41,20 @@ var ( DevAddr = common.BytesToAddress(DevAddr32.Bytes()) DevAddr32 = common.HexToHash("0x42cf1bbc38BaAA3c4898ce8790e21eD2738c6A4a") - DevFeeCurrencyAddr = common.HexToAddress("0x000000000000000000000000000000000000ce16") // worth half as much as native CELO - DevFeeCurrencyAddr2 = common.HexToAddress("0x000000000000000000000000000000000000ce17") // 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) - rateDenominator, _ = new(big.Int).SetString("1000000000000000000000000", 10) - mockOracleAddr = common.HexToAddress("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb0001") - mockOracleAddr2 = common.HexToAddress("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb0002") - mockOracleAddr3 = common.HexToAddress("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb0003") - FaucetAddr = common.HexToAddress("0xfcf982bb4015852e706100b14e21f947a5bb718e") + DevFeeCurrencyAddr = common.HexToAddress("0x000000000000000000000000000000000000ce16") // worth half as much as native CELO + DevFeeCurrencyAddr2 = common.HexToAddress("0x000000000000000000000000000000000000ce17") // 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) + rateDenominator, _ = new(big.Int).SetString("1000000000000000000000000", 10) + mockOracleAddr = common.HexToAddress("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb0001") + mockOracleAddr2 = common.HexToAddress("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb0002") + mockOracleAddr3 = common.HexToAddress("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb0003") + FaucetAddr = common.HexToAddress("0xfcf982bb4015852e706100b14e21f947a5bb718e") + FeeCurrencyIntrinsicGas = uint64(50000) ) -func celoGenesisAccounts(fundedAddr common.Address) GenesisAlloc { +func CeloGenesisAccounts(fundedAddr common.Address) GenesisAlloc { // Initialize Bytecodes celoTokenBytecode, err := DecodeHex(celo.CeloTokenBytecodeRaw) if err != nil { @@ -79,7 +80,7 @@ func celoGenesisAccounts(fundedAddr common.Address) GenesisAlloc { faucetBalance, ok := new(big.Int).SetString("500000000000000000000000000", 10) // 500M if !ok { - panic("Couldn not set faucet balance!") + panic("Could not set faucet balance!") } genesisAccounts := map[common.Address]GenesisAccount{ addresses.MainnetAddresses.CeloToken: { @@ -133,6 +134,9 @@ func celoGenesisAccounts(fundedAddr common.Address) GenesisAlloc { FaucetAddr: { Balance: faucetBalance, }, + fundedAddr: { + Balance: DevBalance, + }, } // FeeCurrencyDirectory @@ -160,6 +164,6 @@ func celoGenesisAccounts(fundedAddr common.Address) GenesisAlloc { func addFeeCurrencyToStorage(feeCurrencyAddr common.Address, oracleAddr common.Address, storage map[common.Hash]common.Hash) { structStart := CalcMapAddr(common.HexToHash("0x1"), common.BytesToHash(feeCurrencyAddr.Bytes())) - storage[structStart] = common.BytesToHash(oracleAddr.Bytes()) // oracle - storage[incHash(structStart, 1)] = common.BigToHash(big.NewInt(50000)) // intrinsicGas + storage[structStart] = common.BytesToHash(oracleAddr.Bytes()) // oracle + storage[incHash(structStart, 1)] = common.BigToHash(big.NewInt(int64(FeeCurrencyIntrinsicGas))) // intrinsicGas } diff --git a/core/genesis.go b/core/genesis.go index e2896b5741..27ca1c0241 100644 --- a/core/genesis.go +++ b/core/genesis.go @@ -691,7 +691,7 @@ func DeveloperGenesisBlock(gasLimit uint64, faucet *common.Address) *Genesis { } // Add state from celoGenesisAccounts - for addr, data := range celoGenesisAccounts(common.HexToAddress("0x2")) { + for addr, data := range CeloGenesisAccounts(common.HexToAddress("0x2")) { genesis.Alloc[addr] = data } diff --git a/core/txpool/celo_validation.go b/core/txpool/celo_validation.go index 91e2025ff5..d82b85db74 100644 --- a/core/txpool/celo_validation.go +++ b/core/txpool/celo_validation.go @@ -1,6 +1,7 @@ package txpool import ( + "errors" "math/big" "github.com/ethereum/go-ethereum/common" @@ -9,6 +10,14 @@ import ( "github.com/ethereum/go-ethereum/params" ) +var ( + + // ErrGasPriceDoesNotExceedBaseFeeFloor is returned if the gas price specified is + // lower than the configured base-fee-floor + ErrGasPriceDoesNotExceedBaseFeeFloor = errors.New("gas-price is less than the base-fee-floor") + ErrMinimumEffectiveGasTipBelowMinTip = errors.New("effective gas tip at base-fee-floor is below threshold") +) + // AcceptSet is a set of accepted transaction types for a transaction subpool. type AcceptSet = map[uint8]struct{} @@ -52,9 +61,42 @@ func CeloValidateTransaction(tx *types.Transaction, head *types.Header, if err := ValidateTransaction(tx, head, signer, opts, currencyCtx); err != nil { return err } + if !common.IsCurrencyAllowed(currencyCtx.ExchangeRates, tx.FeeCurrency()) { return exchange.ErrUnregisteredFeeCurrency } + if opts.Config.Celo != nil { + // Make sure that the effective gas tip at the base fee floor is at least the + // requested min-tip. + // The min-tip for local transactions is set to 0, we can skip checking here. + if opts.MinTip != nil && opts.MinTip.Cmp(new(big.Int)) > 0 { + // If not, this would never be included, so we can reject early. + minTip, err := exchange.ConvertCeloToCurrency(currencyCtx.ExchangeRates, tx.FeeCurrency(), opts.MinTip) + if err != nil { + return err + } + baseFeeFloor, err := exchange.ConvertCeloToCurrency(currencyCtx.ExchangeRates, tx.FeeCurrency(), new(big.Int).SetUint64(opts.Config.Celo.EIP1559BaseFeeFloor)) + if err != nil { + return err + } + if tx.EffectiveGasTipIntCmp(minTip, baseFeeFloor) < 0 { + return ErrUnderpriced + } + } + + celoGasPrice, err := exchange.ConvertCurrencyToCelo( + currencyCtx.ExchangeRates, + tx.FeeCurrency(), + tx.GasFeeCap(), + ) + if err != nil { + return err + } + + if new(big.Int).SetUint64(opts.Config.Celo.EIP1559BaseFeeFloor).Cmp(celoGasPrice) == 1 { + return ErrGasPriceDoesNotExceedBaseFeeFloor + } + } return nil } diff --git a/core/txpool/legacypool/celo_legacypool_test.go b/core/txpool/legacypool/celo_legacypool_test.go new file mode 100644 index 0000000000..88a694a74c --- /dev/null +++ b/core/txpool/legacypool/celo_legacypool_test.go @@ -0,0 +1,205 @@ +package legacypool + +import ( + "crypto/ecdsa" + "errors" + "math/big" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/core/state" + "github.com/ethereum/go-ethereum/core/txpool" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/event" + "github.com/ethereum/go-ethereum/params" + "github.com/ethereum/go-ethereum/triedb" +) + +func celoConfig(baseFeeFloor uint64) *params.ChainConfig { + cpy := *params.TestChainConfig + config := &cpy + ct := uint64(0) + config.Cel2Time = &ct + config.Celo = ¶ms.CeloConfig{EIP1559BaseFeeFloor: baseFeeFloor} + return config +} + +var ( + // worth half as much as native celo + feeCurrencyOne = core.DevFeeCurrencyAddr + // worth twice as much as native celo + feeCurrencyTwo = core.DevFeeCurrencyAddr2 + feeCurrencyIntrinsicGas = core.FeeCurrencyIntrinsicGas + defaultBaseFeeFloor = 100 + defaultChainConfig = celoConfig(uint64(defaultBaseFeeFloor)) +) + +func pricedCip64Transaction( + config *params.ChainConfig, + nonce uint64, + gasLimit uint64, + gasFeeCap *big.Int, + gasTipCap *big.Int, + feeCurrency *common.Address, + key *ecdsa.PrivateKey, +) *types.Transaction { + tx, _ := types.SignTx(types.NewTx(&types.CeloDynamicFeeTxV2{ + Nonce: nonce, + To: &common.Address{}, + Value: big.NewInt(100), + Gas: gasLimit, + GasFeeCap: gasFeeCap, + GasTipCap: gasTipCap, + FeeCurrency: feeCurrency, + Data: nil, + }), types.LatestSigner(config), key) + return tx +} + +func newDBWithCeloGenesis(config *params.ChainConfig, fundedAddress common.Address) (state.Database, *types.Block) { + gspec := &core.Genesis{ + Config: config, + Alloc: core.CeloGenesisAccounts(fundedAddress), + } + db := rawdb.NewMemoryDatabase() + triedb := triedb.NewDatabase(db, triedb.HashDefaults) + defer triedb.Close() + block, err := gspec.Commit(db, triedb) + if err != nil { + panic(err) + } + return state.NewDatabase(triedb, nil), block +} + +func setupCeloPoolWithConfig(config *params.ChainConfig) (*LegacyPool, *ecdsa.PrivateKey) { + key, _ := crypto.GenerateKey() + addr := crypto.PubkeyToAddress(key.PublicKey) + + ddb, genBlock := newDBWithCeloGenesis(config, addr) + stateRoot := genBlock.Header().Root + statedb, err := state.New(stateRoot, ddb) + if err != nil { + panic(err) + } + blockchain := newTestBlockChain(config, 10000000, statedb, new(event.Feed)) + pool := New(testTxPoolConfig, blockchain) + + block := blockchain.CurrentBlock() + // inject the state-root from the genesis chain, so + // that the fee-currency allocs are accessible from the state + // and can be used to create the fee-currency context in the txpool + block.Root = stateRoot + if err := pool.Init(testTxPoolConfig.PriceLimit, block, makeAddressReserver()); err != nil { + panic(err) + } + // wait for the pool to initialize + <-pool.initDoneCh + return pool, key +} + +func TestBelowBaseFeeFloorValidityCheck(t *testing.T) { + t.Parallel() + + pool, key := setupCeloPoolWithConfig(defaultChainConfig) + defer pool.Close() + + // gas-price below base-fee-floor should return early + // and thus raise an error in the validation + + // use local transactions here to skip the min-tip conversion + // the PriceLimit config is set to 1, so we need at least a tip of 1 + tx := pricedCip64Transaction(defaultChainConfig, 0, 21000, big.NewInt(99), big.NewInt(0), nil, key) + if err, want := pool.addLocal(tx), txpool.ErrGasPriceDoesNotExceedBaseFeeFloor; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } + // also test with fee currency conversion + tx = pricedCip64Transaction(defaultChainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(198), big.NewInt(0), &feeCurrencyOne, key) + if err, want := pool.addLocal(tx), txpool.ErrGasPriceDoesNotExceedBaseFeeFloor; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } + tx = pricedCip64Transaction(defaultChainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(48), big.NewInt(0), &feeCurrencyTwo, key) + if err, want := pool.addLocal(tx), txpool.ErrGasPriceDoesNotExceedBaseFeeFloor; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } +} + +func TestAboveBaseFeeFloorValidityCheck(t *testing.T) { + t.Parallel() + + pool, key := setupCeloPoolWithConfig(defaultChainConfig) + defer pool.Close() + + // gas-price just at base-fee-floor should be valid, + // this also adds the required min-tip of 1 + tx := pricedCip64Transaction(defaultChainConfig, 0, 21000, big.NewInt(101), big.NewInt(1), nil, key) + assert.NoError(t, pool.addRemote(tx)) + // also test with fee currency conversion, increase nonce because of previous tx was valid + tx = pricedCip64Transaction(defaultChainConfig, 1, 21000+feeCurrencyIntrinsicGas, big.NewInt(202), big.NewInt(2), &feeCurrencyOne, key) + assert.NoError(t, pool.addRemote(tx)) + tx = pricedCip64Transaction(defaultChainConfig, 2, 21000+feeCurrencyIntrinsicGas, big.NewInt(51), big.NewInt(1), &feeCurrencyTwo, key) + assert.NoError(t, pool.addRemote(tx)) +} + +func TestBelowMinTipValidityCheck(t *testing.T) { + t.Parallel() + + pool, key := setupCeloPoolWithConfig(defaultChainConfig) + defer pool.Close() + + // the min-tip is set to 1 per default + + // Gas-price just at base-fee-floor should be valid, + // the effective gas-price would also pass the min-tip restriction of 1. + // However the explicit gas-tip-cap at 0 should reject the transaction. + tx := pricedCip64Transaction(defaultChainConfig, 0, 21000, big.NewInt(101), big.NewInt(0), nil, key) + if err, want := pool.addRemote(tx), txpool.ErrUnderpriced; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } + tx = pricedCip64Transaction(defaultChainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(202), big.NewInt(0), &feeCurrencyOne, key) + if err, want := pool.addRemote(tx), txpool.ErrUnderpriced; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } + + // This passes the check that only checks the actual gas-tip-cap value for the min-tip that was + // tested above. + // Now the effective gas-tip should still be below the min-tip, since we consume everything + // for the base fee floor and thus the tx should get rejected. + tx = pricedCip64Transaction(defaultChainConfig, 0, 21000, big.NewInt(100), big.NewInt(1), nil, key) + if err, want := pool.addRemote(tx), txpool.ErrUnderpriced; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } + tx = pricedCip64Transaction(defaultChainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(200), big.NewInt(2), &feeCurrencyOne, key) + if err, want := pool.addRemote(tx), txpool.ErrUnderpriced; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } +} + +func TestExpectMinTipRoundingFeeCurrency(t *testing.T) { + t.Parallel() + + pool, key := setupCeloPoolWithConfig(defaultChainConfig) + defer pool.Close() + + // the min-tip is set to 1 per default + + // even though the gas-tip-cap as well as the effective gas tip at the base-fee-floor + // is 0, the transaction is still accepted. + // This is because at a min-tip requirement of 1, a more valuable currency than native + // token will get rounded down to a min-tip of 0 during conversion. + tx := pricedCip64Transaction(defaultChainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(50), big.NewInt(0), &feeCurrencyTwo, key) + assert.NoError(t, pool.addRemote(tx)) + + // set the required min-tip to 10 + pool.SetGasTip(big.NewInt(10)) + + // but as soon as we increase the min-tip, the check rejects a gas-tip-cap that is too low after conversion + tx = pricedCip64Transaction(defaultChainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(100), big.NewInt(4), &feeCurrencyTwo, key) + if err, want := pool.addRemote(tx), txpool.ErrUnderpriced; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } +} diff --git a/core/txpool/validation.go b/core/txpool/validation.go index 1c2d0eb424..9e49697a95 100644 --- a/core/txpool/validation.go +++ b/core/txpool/validation.go @@ -23,6 +23,7 @@ import ( "math/big" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/exchange" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" @@ -151,7 +152,11 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types return fmt.Errorf("%w: gas %v, minimum needed %v", core.ErrIntrinsicGas, tx.Gas(), intrGas) } // Ensure the gasprice is high enough to cover the requirement of the calling pool - if tx.GasTipCapIntCmp(opts.MinTip) < 0 { + minTip, err := exchange.ConvertCeloToCurrency(currencyCtx.ExchangeRates, tx.FeeCurrency(), opts.MinTip) + if err != nil { + return err + } + if tx.GasTipCapIntCmp(minTip) < 0 { return fmt.Errorf("%w: gas tip cap %v, minimum needed %v", ErrUnderpriced, tx.GasTipCap(), opts.MinTip) } if tx.Type() == types.BlobTxType {