Skip to content

Commit

Permalink
Invalidate txs with below base-fee-floor gas-price in txpool (#292)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ezdac authored Jan 8, 2025
1 parent dd40725 commit e806411
Show file tree
Hide file tree
Showing 7 changed files with 274 additions and 18 deletions.
2 changes: 1 addition & 1 deletion common/exchange/rates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion core/blockchain_celo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 18 additions & 14 deletions core/celo_genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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: {
Expand Down Expand Up @@ -133,6 +134,9 @@ func celoGenesisAccounts(fundedAddr common.Address) GenesisAlloc {
FaucetAddr: {
Balance: faucetBalance,
},
fundedAddr: {
Balance: DevBalance,
},
}

// FeeCurrencyDirectory
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion core/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
42 changes: 42 additions & 0 deletions core/txpool/celo_validation.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package txpool

import (
"errors"
"math/big"

"github.com/ethereum/go-ethereum/common"
Expand All @@ -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{}

Expand Down Expand Up @@ -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
}
205 changes: 205 additions & 0 deletions core/txpool/legacypool/celo_legacypool_test.go
Original file line number Diff line number Diff line change
@@ -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 = &params.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)
}
}
7 changes: 6 additions & 1 deletion core/txpool/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit e806411

Please sign in to comment.