forked from ethereum-optimism/op-geth
-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Invalidate txs with below base-fee-floor gas-price in txpool #292
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
4500cc1
Invalidate txs with below base-fee-floor gas-price in txpool
ezdac f671284
Add check for effective-gas-tip reaches min-tip in txpool validation
ezdac 515396c
Make genesis devmode alloc function public
ezdac 7c80d9c
Allocate Celo balance also for fundedAccount in genesis
ezdac ff0748a
Add test for celo-legacypool
ezdac 07ff4c9
Use stateful legacypool validation in test
ezdac f3618d2
Fix error/panic messages
ezdac 49b5fa3
Fix min-tip conversion in txpool pre-validation
ezdac 5dcc5a7
Add tests for min-tip validation in txpool
ezdac d35c4b1
Summarize common fixtures to default vars in txpool test
ezdac aa7b736
Fix typo
ezdac File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 = ¶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) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the mintip is 1, but the exchange rate is smaller than 1? Does this get rounded to zero?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it gets rounded to 0.
In the check, if the remaining tip after applying 25 gwei base-fee is larger or equal to the converted min-tip,
the transaction is passed to the pool.
If the default tip is 1, but due to a higher valued fee-currency, it will be converted to 0:
This means that the transaction will not be underpriced, and thus will be forwarded to the txpool.
Within the txpool, the same conversion and check is used to filter out transactions that
after paying for the base-fee, would undercut the required min-tip:
op-geth/core/txpool/legacypool/legacypool.go
Lines 579 to 582 in 002a5c1
In practice that means that if an operator does not change the default value of 1 for the min-tip,
transactions with more valuable currencies than native token would be "free" in regards to block-builder tip.