From 4500cc10844518e524507735f04ed95f4a3100eb Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Thu, 5 Dec 2024 16:57:19 -0500 Subject: [PATCH 01/11] 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. --- core/txpool/celo_validation.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/core/txpool/celo_validation.go b/core/txpool/celo_validation.go index 91e2025ff5..018f5c460e 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,10 @@ import ( "github.com/ethereum/go-ethereum/params" ) +// ErrGasPriceDoesNotExceedBaseFeeFloor is returned if the gas price specified is +// lower than the configured base-fee-floor +var ErrGasPriceDoesNotExceedBaseFeeFloor = errors.New("gas-price is less than the base-fee-floor") + // AcceptSet is a set of accepted transaction types for a transaction subpool. type AcceptSet = map[uint8]struct{} @@ -56,5 +61,19 @@ func CeloValidateTransaction(tx *types.Transaction, head *types.Header, return exchange.ErrUnregisteredFeeCurrency } + celoGasPrice, err := exchange.ConvertCurrencyToCelo( + currencyCtx.ExchangeRates, + tx.FeeCurrency(), + tx.GasFeeCap(), + ) + if err != nil { + return err + } + + if opts.Config.Celo != nil { + if new(big.Int).SetUint64(opts.Config.Celo.EIP1559BaseFeeFloor).Cmp(celoGasPrice) == 1 { + return ErrGasPriceDoesNotExceedBaseFeeFloor + } + } return nil } From f67128403ecdc1166cec8fed913f1a6288f6696a Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Thu, 12 Dec 2024 16:00:44 +0100 Subject: [PATCH 02/11] Add check for effective-gas-tip reaches min-tip in txpool validation --- core/txpool/celo_validation.go | 47 +++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/core/txpool/celo_validation.go b/core/txpool/celo_validation.go index 018f5c460e..d82b85db74 100644 --- a/core/txpool/celo_validation.go +++ b/core/txpool/celo_validation.go @@ -10,9 +10,13 @@ import ( "github.com/ethereum/go-ethereum/params" ) -// ErrGasPriceDoesNotExceedBaseFeeFloor is returned if the gas price specified is -// lower than the configured base-fee-floor -var ErrGasPriceDoesNotExceedBaseFeeFloor = errors.New("gas-price is less than the base-fee-floor") +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{} @@ -57,20 +61,39 @@ 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 } - celoGasPrice, err := exchange.ConvertCurrencyToCelo( - currencyCtx.ExchangeRates, - tx.FeeCurrency(), - tx.GasFeeCap(), - ) - if err != nil { - return err - } - 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 } From 515396cbf0020987322d72fad36051d12b50bc8c Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Fri, 20 Dec 2024 16:16:18 +0100 Subject: [PATCH 03/11] Make genesis devmode alloc function public --- core/blockchain_celo_test.go | 2 +- core/celo_genesis.go | 2 +- core/genesis.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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..dd9b6e9c6b 100644 --- a/core/celo_genesis.go +++ b/core/celo_genesis.go @@ -53,7 +53,7 @@ var ( FaucetAddr = common.HexToAddress("0xfcf982bb4015852e706100b14e21f947a5bb718e") ) -func celoGenesisAccounts(fundedAddr common.Address) GenesisAlloc { +func CeloGenesisAccounts(fundedAddr common.Address) GenesisAlloc { // Initialize Bytecodes celoTokenBytecode, err := DecodeHex(celo.CeloTokenBytecodeRaw) if err != nil { diff --git a/core/genesis.go b/core/genesis.go index 9059a3f7c6..5466cddadd 100644 --- a/core/genesis.go +++ b/core/genesis.go @@ -696,7 +696,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 } From 7c80d9c7653c8663fdc35b442253fb9ce1e1e862 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Thu, 12 Dec 2024 15:53:18 +0100 Subject: [PATCH 04/11] Allocate Celo balance also for fundedAccount in genesis --- core/celo_genesis.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/celo_genesis.go b/core/celo_genesis.go index dd9b6e9c6b..b0ec23b29d 100644 --- a/core/celo_genesis.go +++ b/core/celo_genesis.go @@ -133,6 +133,9 @@ func CeloGenesisAccounts(fundedAddr common.Address) GenesisAlloc { FaucetAddr: { Balance: faucetBalance, }, + fundedAddr: { + Balance: DevBalance, + }, } // FeeCurrencyDirectory From ff0748ade77c7770aa937fc57923b6c6a61d7637 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:32:04 +0100 Subject: [PATCH 05/11] Add test for celo-legacypool --- core/celo_genesis.go | 25 ++-- .../txpool/legacypool/celo_legacypool_test.go | 132 ++++++++++++++++++ 2 files changed, 145 insertions(+), 12 deletions(-) create mode 100644 core/txpool/legacypool/celo_legacypool_test.go diff --git a/core/celo_genesis.go b/core/celo_genesis.go index b0ec23b29d..107e156e49 100644 --- a/core/celo_genesis.go +++ b/core/celo_genesis.go @@ -41,16 +41,17 @@ 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 { @@ -163,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/txpool/legacypool/celo_legacypool_test.go b/core/txpool/legacypool/celo_legacypool_test.go new file mode 100644 index 0000000000..4c049cc558 --- /dev/null +++ b/core/txpool/legacypool/celo_legacypool_test.go @@ -0,0 +1,132 @@ +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 ( + baseFeeFloor = 100 + celoTestChainConfig = celoConfig(uint64(baseFeeFloor)) + // worth half as much as native celo + feeCurrencyOne = core.DevFeeCurrencyAddr + // worth twice as much as native celo + feeCurrencyTwo = core.DevFeeCurrencyAddr2 + feeCurrencyIntrinsicGas = core.FeeCurrencyIntrinsicGas +) + +func pricedCip64Transaction( + 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(celoTestChainConfig), 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 TestBaseFeeFloorValidityCheck(t *testing.T) { + t.Parallel() + + pool, key := setupCeloPoolWithConfig(celoTestChainConfig) + defer pool.Close() + + // gas-price below base-fee-floor should return early + // and thus raise an error in the validation + t.Logf("fee-currency ctx: %v", pool.feeCurrencyContext.ExchangeRates) + + // the PriceLimit config is set to 1, so we need at least a tip of 1 + tx := pricedCip64Transaction(0, 21000, big.NewInt(98), big.NewInt(1), nil, key) + + if err, want := pool.validateTxBasics(tx, false), txpool.ErrGasPriceDoesNotExceedBaseFeeFloor; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } + // also test with fee currency conversion + tx = pricedCip64Transaction(0, 21000+feeCurrencyIntrinsicGas, big.NewInt(198), big.NewInt(2), &feeCurrencyOne, key) + if err, want := pool.validateTxBasics(tx, false), txpool.ErrGasPriceDoesNotExceedBaseFeeFloor; !errors.Is(err, want) { + t.Errorf("want %v have %v", want, err) + } + + // gas-price just at base-fee-floor should be valid + tx = pricedCip64Transaction(0, 21000+feeCurrencyIntrinsicGas, big.NewInt(100), big.NewInt(1), nil, key) + assert.NoError(t, pool.validateTxBasics(tx, false)) + // also test with fee currency conversion, increase nonce because of previous tx was valid + tx = pricedCip64Transaction(1, 21000+feeCurrencyIntrinsicGas, big.NewInt(200), big.NewInt(2), &feeCurrencyOne, key) + assert.NoError(t, pool.validateTxBasics(tx, false)) +} From 07ff4c91686fa5021bb11bc0d79444ba1566e6a8 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Thu, 12 Dec 2024 16:01:11 +0100 Subject: [PATCH 06/11] Use stateful legacypool validation in test --- .../txpool/legacypool/celo_legacypool_test.go | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/core/txpool/legacypool/celo_legacypool_test.go b/core/txpool/legacypool/celo_legacypool_test.go index 4c049cc558..1a0c62331c 100644 --- a/core/txpool/legacypool/celo_legacypool_test.go +++ b/core/txpool/legacypool/celo_legacypool_test.go @@ -30,8 +30,6 @@ func celoConfig(baseFeeFloor uint64) *params.ChainConfig { } var ( - baseFeeFloor = 100 - celoTestChainConfig = celoConfig(uint64(baseFeeFloor)) // worth half as much as native celo feeCurrencyOne = core.DevFeeCurrencyAddr // worth twice as much as native celo @@ -40,6 +38,7 @@ var ( ) func pricedCip64Transaction( + config *params.ChainConfig, nonce uint64, gasLimit uint64, gasFeeCap *big.Int, @@ -56,7 +55,7 @@ func pricedCip64Transaction( GasTipCap: gasTipCap, FeeCurrency: feeCurrency, Data: nil, - }), types.LatestSigner(celoTestChainConfig), key) + }), types.LatestSigner(config), key) return tx } @@ -101,32 +100,48 @@ func setupCeloPoolWithConfig(config *params.ChainConfig) (*LegacyPool, *ecdsa.Pr return pool, key } -func TestBaseFeeFloorValidityCheck(t *testing.T) { +func TestBelowBaseFeeFloorValidityCheck(t *testing.T) { t.Parallel() + baseFeeFloor := 100 + chainConfig := celoConfig(uint64(baseFeeFloor)) - pool, key := setupCeloPoolWithConfig(celoTestChainConfig) + pool, key := setupCeloPoolWithConfig(chainConfig) defer pool.Close() // gas-price below base-fee-floor should return early // and thus raise an error in the validation - t.Logf("fee-currency ctx: %v", pool.feeCurrencyContext.ExchangeRates) + // 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(0, 21000, big.NewInt(98), big.NewInt(1), nil, key) - - if err, want := pool.validateTxBasics(tx, false), txpool.ErrGasPriceDoesNotExceedBaseFeeFloor; !errors.Is(err, want) { + tx := pricedCip64Transaction(chainConfig, 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(0, 21000+feeCurrencyIntrinsicGas, big.NewInt(198), big.NewInt(2), &feeCurrencyOne, key) - if err, want := pool.validateTxBasics(tx, false), txpool.ErrGasPriceDoesNotExceedBaseFeeFloor; !errors.Is(err, want) { + tx = pricedCip64Transaction(chainConfig, 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(chainConfig, 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() + + baseFeeFloor := 100 + chainConfig := celoConfig(uint64(baseFeeFloor)) + pool, key := setupCeloPoolWithConfig(chainConfig) + defer pool.Close() // gas-price just at base-fee-floor should be valid - tx = pricedCip64Transaction(0, 21000+feeCurrencyIntrinsicGas, big.NewInt(100), big.NewInt(1), nil, key) - assert.NoError(t, pool.validateTxBasics(tx, false)) + tx := pricedCip64Transaction(chainConfig, 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(1, 21000+feeCurrencyIntrinsicGas, big.NewInt(200), big.NewInt(2), &feeCurrencyOne, key) - assert.NoError(t, pool.validateTxBasics(tx, false)) + tx = pricedCip64Transaction(chainConfig, 1, 21000+feeCurrencyIntrinsicGas, big.NewInt(202), big.NewInt(2), &feeCurrencyOne, key) + assert.NoError(t, pool.addRemote(tx)) + tx = pricedCip64Transaction(chainConfig, 2, 21000+feeCurrencyIntrinsicGas, big.NewInt(51), big.NewInt(1), &feeCurrencyTwo, key) + assert.NoError(t, pool.addRemote(tx)) } From f3618d2f78ed3514e7dd78fa81ecb75ab3683f85 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:32:15 +0100 Subject: [PATCH 07/11] Fix error/panic messages --- common/exchange/rates.go | 2 +- core/celo_genesis.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/celo_genesis.go b/core/celo_genesis.go index 107e156e49..1e37f55b8f 100644 --- a/core/celo_genesis.go +++ b/core/celo_genesis.go @@ -80,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: { From 49b5fa34e9f631b792d280366f4dd493ab6d36c3 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Tue, 7 Jan 2025 16:45:34 +0100 Subject: [PATCH 08/11] Fix min-tip conversion in txpool pre-validation Fixes #304 --- core/txpool/validation.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 { From 5dcc5a7ddf07fb04391ed47aae494d3a6be8bfb0 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Tue, 7 Jan 2025 17:52:12 +0100 Subject: [PATCH 09/11] Add tests for min-tip validation in txpool --- .../txpool/legacypool/celo_legacypool_test.go | 66 ++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/core/txpool/legacypool/celo_legacypool_test.go b/core/txpool/legacypool/celo_legacypool_test.go index 1a0c62331c..f6515973c5 100644 --- a/core/txpool/legacypool/celo_legacypool_test.go +++ b/core/txpool/legacypool/celo_legacypool_test.go @@ -136,7 +136,8 @@ func TestAboveBaseFeeFloorValidityCheck(t *testing.T) { pool, key := setupCeloPoolWithConfig(chainConfig) defer pool.Close() - // gas-price just at base-fee-floor should be valid + // gas-price just at base-fee-floor should be valid, + // this also adds the required min-tip of 1 tx := pricedCip64Transaction(chainConfig, 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 @@ -145,3 +146,66 @@ func TestAboveBaseFeeFloorValidityCheck(t *testing.T) { tx = pricedCip64Transaction(chainConfig, 2, 21000+feeCurrencyIntrinsicGas, big.NewInt(51), big.NewInt(1), &feeCurrencyTwo, key) assert.NoError(t, pool.addRemote(tx)) } + +func TestBelowMinTipValidityCheck(t *testing.T) { + t.Parallel() + + baseFeeFloor := 100 + chainConfig := celoConfig(uint64(baseFeeFloor)) + pool, key := setupCeloPoolWithConfig(chainConfig) + 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(chainConfig, 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(chainConfig, 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 is still be below the tip, since we consume everything + // for the base fee floor and thus the tx should get rejected. + tx = pricedCip64Transaction(chainConfig, 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(chainConfig, 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() + + baseFeeFloor := 100 + chainConfig := celoConfig(uint64(baseFeeFloor)) + pool, key := setupCeloPoolWithConfig(chainConfig) + 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(chainConfig, 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(chainConfig, 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) + } +} From d35c4b14926d811531ef544fe943e2644049eda7 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:04:08 +0100 Subject: [PATCH 10/11] Summarize common fixtures to default vars in txpool test --- .../txpool/legacypool/celo_legacypool_test.go | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/core/txpool/legacypool/celo_legacypool_test.go b/core/txpool/legacypool/celo_legacypool_test.go index f6515973c5..5739bb5219 100644 --- a/core/txpool/legacypool/celo_legacypool_test.go +++ b/core/txpool/legacypool/celo_legacypool_test.go @@ -35,6 +35,8 @@ var ( // worth twice as much as native celo feeCurrencyTwo = core.DevFeeCurrencyAddr2 feeCurrencyIntrinsicGas = core.FeeCurrencyIntrinsicGas + defaultBaseFeeFloor = 100 + defaultChainConfig = celoConfig(uint64(defaultBaseFeeFloor)) ) func pricedCip64Transaction( @@ -102,10 +104,8 @@ func setupCeloPoolWithConfig(config *params.ChainConfig) (*LegacyPool, *ecdsa.Pr func TestBelowBaseFeeFloorValidityCheck(t *testing.T) { t.Parallel() - baseFeeFloor := 100 - chainConfig := celoConfig(uint64(baseFeeFloor)) - pool, key := setupCeloPoolWithConfig(chainConfig) + pool, key := setupCeloPoolWithConfig(defaultChainConfig) defer pool.Close() // gas-price below base-fee-floor should return early @@ -113,16 +113,16 @@ func TestBelowBaseFeeFloorValidityCheck(t *testing.T) { // 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(chainConfig, 0, 21000, big.NewInt(99), big.NewInt(0), nil, key) + 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(chainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(198), big.NewInt(0), &feeCurrencyOne, key) + 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(chainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(48), big.NewInt(0), &feeCurrencyTwo, key) + 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) } @@ -131,28 +131,24 @@ func TestBelowBaseFeeFloorValidityCheck(t *testing.T) { func TestAboveBaseFeeFloorValidityCheck(t *testing.T) { t.Parallel() - baseFeeFloor := 100 - chainConfig := celoConfig(uint64(baseFeeFloor)) - pool, key := setupCeloPoolWithConfig(chainConfig) + 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(chainConfig, 0, 21000, big.NewInt(101), big.NewInt(1), nil, key) + 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(chainConfig, 1, 21000+feeCurrencyIntrinsicGas, big.NewInt(202), big.NewInt(2), &feeCurrencyOne, key) + tx = pricedCip64Transaction(defaultChainConfig, 1, 21000+feeCurrencyIntrinsicGas, big.NewInt(202), big.NewInt(2), &feeCurrencyOne, key) assert.NoError(t, pool.addRemote(tx)) - tx = pricedCip64Transaction(chainConfig, 2, 21000+feeCurrencyIntrinsicGas, big.NewInt(51), big.NewInt(1), &feeCurrencyTwo, key) + 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() - baseFeeFloor := 100 - chainConfig := celoConfig(uint64(baseFeeFloor)) - pool, key := setupCeloPoolWithConfig(chainConfig) + pool, key := setupCeloPoolWithConfig(defaultChainConfig) defer pool.Close() // the min-tip is set to 1 per default @@ -160,11 +156,11 @@ func TestBelowMinTipValidityCheck(t *testing.T) { // 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(chainConfig, 0, 21000, big.NewInt(101), big.NewInt(0), nil, key) + 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(chainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(202), big.NewInt(0), &feeCurrencyOne, key) + 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) } @@ -173,11 +169,11 @@ func TestBelowMinTipValidityCheck(t *testing.T) { // tested above. // Now the effective gas-tip is still be below the tip, since we consume everything // for the base fee floor and thus the tx should get rejected. - tx = pricedCip64Transaction(chainConfig, 0, 21000, big.NewInt(100), big.NewInt(1), nil, key) + 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(chainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(200), big.NewInt(2), &feeCurrencyOne, key) + 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) } @@ -186,9 +182,7 @@ func TestBelowMinTipValidityCheck(t *testing.T) { func TestExpectMinTipRoundingFeeCurrency(t *testing.T) { t.Parallel() - baseFeeFloor := 100 - chainConfig := celoConfig(uint64(baseFeeFloor)) - pool, key := setupCeloPoolWithConfig(chainConfig) + pool, key := setupCeloPoolWithConfig(defaultChainConfig) defer pool.Close() // the min-tip is set to 1 per default @@ -197,14 +191,14 @@ func TestExpectMinTipRoundingFeeCurrency(t *testing.T) { // 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(chainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(50), big.NewInt(0), &feeCurrencyTwo, key) + 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(chainConfig, 0, 21000+feeCurrencyIntrinsicGas, big.NewInt(100), big.NewInt(4), &feeCurrencyTwo, key) + 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) } From aa7b736b055f3a6efed2df401226e474de057646 Mon Sep 17 00:00:00 2001 From: Maximilian Langenfeld <15726643+ezdac@users.noreply.github.com> Date: Wed, 8 Jan 2025 11:32:09 +0100 Subject: [PATCH 11/11] Fix typo --- core/txpool/legacypool/celo_legacypool_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/txpool/legacypool/celo_legacypool_test.go b/core/txpool/legacypool/celo_legacypool_test.go index 5739bb5219..88a694a74c 100644 --- a/core/txpool/legacypool/celo_legacypool_test.go +++ b/core/txpool/legacypool/celo_legacypool_test.go @@ -167,7 +167,7 @@ func TestBelowMinTipValidityCheck(t *testing.T) { // 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 is still be below the tip, since we consume everything + // 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) {