From bffa606f63f92b67110943989c4dc0511bc28fc3 Mon Sep 17 00:00:00 2001 From: Juan Palacios Date: Mon, 18 Sep 2023 14:03:49 +1000 Subject: [PATCH 1/8] core/txpool/legacypool: Enforce MinTip in the legacypool for local transactions --- core/txpool/legacypool/legacypool_test.go | 25 +++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index a8f3dd7d8624..c98f0ea69dc0 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -1492,6 +1492,31 @@ func TestRepricing(t *testing.T) { } } +func TestMinGasPriceEnforced(t *testing.T) { + t.Parallel() + + // Create the pool to test the pricing enforcement with + pool, _ := setupPoolWithConfig(eip1559Config) + defer pool.Close() + + key, _ := crypto.GenerateKey() + testAddBalance(pool, crypto.PubkeyToAddress(key.PublicKey), big.NewInt(1000000)) + + tx := pricedTransaction(0, 100000, big.NewInt(2), key) + pool.SetGasTip(big.NewInt(tx.GasPrice().Int64() + 1)) + + if err := pool.addLocal(tx); !errors.Is(err, txpool.ErrUnderpriced) { + t.Fatalf("Min tip not enforced") + } + + tx = dynamicFeeTx(0, 100000, big.NewInt(3), big.NewInt(2), key) + pool.SetGasTip(big.NewInt(tx.GasTipCap().Int64() + 1)) + + if err := pool.addLocal(tx); !errors.Is(err, txpool.ErrUnderpriced) { + t.Fatalf("Min tip not enforced") + } +} + // Tests that setting the transaction pool gas price to a higher value correctly // discards everything cheaper (legacy & dynamic fee) than that and moves any // gapped transactions back from the pending pool to the queue. From d5848e3af9110be365d3587d1bddeafa1d8f12b9 Mon Sep 17 00:00:00 2001 From: Juan Palacios Date: Thu, 26 Oct 2023 15:19:29 +1100 Subject: [PATCH 2/8] 28125: check config.NoLocals on legacypool's validateTxBasics --- core/txpool/legacypool/legacypool.go | 2 +- core/txpool/legacypool/legacypool_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 0e3392327405..618e4711c29c 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -596,7 +596,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro MaxSize: txMaxSize, MinTip: pool.gasTip.Load(), } - if local { + if local && !pool.config.NoLocals { opts.MinTip = new(big.Int) } if err := txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts); err != nil { diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index c98f0ea69dc0..a3d5cb5ac1d1 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -1496,7 +1496,13 @@ func TestMinGasPriceEnforced(t *testing.T) { t.Parallel() // Create the pool to test the pricing enforcement with - pool, _ := setupPoolWithConfig(eip1559Config) + statedb, _ := state.New(types.EmptyRootHash, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + blockchain := newTestBlockChain(eip1559Config, 10000000, statedb, new(event.Feed)) + + txPoolConfig := DefaultConfig + txPoolConfig.NoLocals = true + pool := New(txPoolConfig, blockchain) + pool.Init(new(big.Int).SetUint64(txPoolConfig.PriceLimit), blockchain.CurrentBlock(), makeAddressReserver()) defer pool.Close() key, _ := crypto.GenerateKey() From ee8097e44daf25a28002363563f20ad137d46ec4 Mon Sep 17 00:00:00 2001 From: Juan Palacios Date: Tue, 31 Oct 2023 13:40:23 +1100 Subject: [PATCH 3/8] Wrapped in pool.config.PrioritizeLocals() for readbility --- core/txpool/legacypool/legacypool.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 618e4711c29c..88957d6c14c5 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -152,6 +152,10 @@ var DefaultConfig = Config{ Lifetime: 3 * time.Hour, } +func (config *Config) PrioritizeLocals() bool { + return !config.NoLocals +} + // sanitize checks the provided user configurations and changes anything that's // unreasonable or unworkable. func (config *Config) sanitize() Config { @@ -596,7 +600,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro MaxSize: txMaxSize, MinTip: pool.gasTip.Load(), } - if local && !pool.config.NoLocals { + if local && pool.config.PrioritizeLocals() { opts.MinTip = new(big.Int) } if err := txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts); err != nil { From 827f7e693f88eca2ea53a2f0b661845b91fcd468 Mon Sep 17 00:00:00 2001 From: Juan Palacios Date: Wed, 1 Nov 2023 10:29:53 +1100 Subject: [PATCH 4/8] Addressing PR feedback. Now setting local = false when NoLocals is set --- core/txpool/legacypool/legacypool.go | 8 ++------ core/txpool/legacypool/legacypool_test.go | 8 ++++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 88957d6c14c5..f97dae278511 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -152,10 +152,6 @@ var DefaultConfig = Config{ Lifetime: 3 * time.Hour, } -func (config *Config) PrioritizeLocals() bool { - return !config.NoLocals -} - // sanitize checks the provided user configurations and changes anything that's // unreasonable or unworkable. func (config *Config) sanitize() Config { @@ -600,8 +596,8 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro MaxSize: txMaxSize, MinTip: pool.gasTip.Load(), } - if local && pool.config.PrioritizeLocals() { - opts.MinTip = new(big.Int) + if local && pool.config.NoLocals { + local = false } if err := txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts); err != nil { return err diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index a3d5cb5ac1d1..fed3ba02251f 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -1515,12 +1515,20 @@ func TestMinGasPriceEnforced(t *testing.T) { t.Fatalf("Min tip not enforced") } + if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { + t.Fatalf("Min tip not enforced") + } + tx = dynamicFeeTx(0, 100000, big.NewInt(3), big.NewInt(2), key) pool.SetGasTip(big.NewInt(tx.GasTipCap().Int64() + 1)) if err := pool.addLocal(tx); !errors.Is(err, txpool.ErrUnderpriced) { t.Fatalf("Min tip not enforced") } + + if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { + t.Fatalf("Min tip not enforced") + } } // Tests that setting the transaction pool gas price to a higher value correctly From bad6717720180c9b661e61f37994424c96a48110 Mon Sep 17 00:00:00 2001 From: Juan Palacios Date: Wed, 1 Nov 2023 13:12:44 +1100 Subject: [PATCH 5/8] Revert "Addressing PR feedback. Now setting local = false when NoLocals is set" This reverts commit 75d4098bf2cff119ce424840e7786a1592170c52. The local flag doesn't propagate to validation logic, only opts.MinTip --- core/txpool/legacypool/legacypool.go | 8 ++++++-- core/txpool/legacypool/legacypool_test.go | 8 -------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index f97dae278511..88957d6c14c5 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -152,6 +152,10 @@ var DefaultConfig = Config{ Lifetime: 3 * time.Hour, } +func (config *Config) PrioritizeLocals() bool { + return !config.NoLocals +} + // sanitize checks the provided user configurations and changes anything that's // unreasonable or unworkable. func (config *Config) sanitize() Config { @@ -596,8 +600,8 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro MaxSize: txMaxSize, MinTip: pool.gasTip.Load(), } - if local && pool.config.NoLocals { - local = false + if local && pool.config.PrioritizeLocals() { + opts.MinTip = new(big.Int) } if err := txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts); err != nil { return err diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index fed3ba02251f..a3d5cb5ac1d1 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -1515,20 +1515,12 @@ func TestMinGasPriceEnforced(t *testing.T) { t.Fatalf("Min tip not enforced") } - if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { - t.Fatalf("Min tip not enforced") - } - tx = dynamicFeeTx(0, 100000, big.NewInt(3), big.NewInt(2), key) pool.SetGasTip(big.NewInt(tx.GasTipCap().Int64() + 1)) if err := pool.addLocal(tx); !errors.Is(err, txpool.ErrUnderpriced) { t.Fatalf("Min tip not enforced") } - - if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { - t.Fatalf("Min tip not enforced") - } } // Tests that setting the transaction pool gas price to a higher value correctly From 01fa6d99c5299cc5331cf3a0abef0266b11fa718 Mon Sep 17 00:00:00 2001 From: Juan Palacios Date: Mon, 13 Nov 2023 10:29:53 +1100 Subject: [PATCH 6/8] Added missing checks to test --- core/txpool/legacypool/legacypool_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index a3d5cb5ac1d1..fed3ba02251f 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -1515,12 +1515,20 @@ func TestMinGasPriceEnforced(t *testing.T) { t.Fatalf("Min tip not enforced") } + if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { + t.Fatalf("Min tip not enforced") + } + tx = dynamicFeeTx(0, 100000, big.NewInt(3), big.NewInt(2), key) pool.SetGasTip(big.NewInt(tx.GasTipCap().Int64() + 1)) if err := pool.addLocal(tx); !errors.Is(err, txpool.ErrUnderpriced) { t.Fatalf("Min tip not enforced") } + + if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { + t.Fatalf("Min tip not enforced") + } } // Tests that setting the transaction pool gas price to a higher value correctly From 5927f62b76e6559571ad12a531e5918ab90b74fd Mon Sep 17 00:00:00 2001 From: Juan Palacios Date: Tue, 14 Nov 2023 15:25:06 +1100 Subject: [PATCH 7/8] Moved local flag check to the top of Add function --- core/txpool/legacypool/legacypool.go | 9 ++++----- core/txpool/legacypool/legacypool_test.go | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 88957d6c14c5..8450d89a2cf2 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -152,10 +152,6 @@ var DefaultConfig = Config{ Lifetime: 3 * time.Hour, } -func (config *Config) PrioritizeLocals() bool { - return !config.NoLocals -} - // sanitize checks the provided user configurations and changes anything that's // unreasonable or unworkable. func (config *Config) sanitize() Config { @@ -600,7 +596,7 @@ func (pool *LegacyPool) validateTxBasics(tx *types.Transaction, local bool) erro MaxSize: txMaxSize, MinTip: pool.gasTip.Load(), } - if local && pool.config.PrioritizeLocals() { + if local { opts.MinTip = new(big.Int) } if err := txpool.ValidateTransaction(tx, pool.currentHead.Load(), pool.signer, opts); err != nil { @@ -963,6 +959,9 @@ func (pool *LegacyPool) addRemoteSync(tx *types.Transaction) error { // If sync is set, the method will block until all internal maintenance related // to the add is finished. Only use this during tests for determinism! func (pool *LegacyPool) Add(txs []*types.Transaction, local, sync bool) []error { + // Do not treat as local if local transactions have been disabled + local = local && !pool.config.NoLocals + // Filter out known ones without obtaining the pool lock or recovering signatures var ( errs = make([]error, len(txs)) diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index fed3ba02251f..03ca016f4e62 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -1515,7 +1515,7 @@ func TestMinGasPriceEnforced(t *testing.T) { t.Fatalf("Min tip not enforced") } - if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { + if err := pool.Add([]*types.Transaction{tx}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { t.Fatalf("Min tip not enforced") } @@ -1526,7 +1526,7 @@ func TestMinGasPriceEnforced(t *testing.T) { t.Fatalf("Min tip not enforced") } - if err := pool.Add([]*txpool.Transaction{{Tx: tx}}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { + if err := pool.Add([]*types.Transaction{tx}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { t.Fatalf("Min tip not enforced") } } From 0dc44d0a34222e12900f2ac2a6e24ebadcc5f135 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 15 Nov 2023 13:49:31 +0100 Subject: [PATCH 8/8] core/txpool/legacypool: extend test --- core/txpool/legacypool/legacypool_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index 03ca016f4e62..0366a58d61ab 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -1529,6 +1529,11 @@ func TestMinGasPriceEnforced(t *testing.T) { if err := pool.Add([]*types.Transaction{tx}, true, false)[0]; !errors.Is(err, txpool.ErrUnderpriced) { t.Fatalf("Min tip not enforced") } + // Make sure the tx is accepted if locals are enabled + pool.config.NoLocals = false + if err := pool.Add([]*types.Transaction{tx}, true, false)[0]; err != nil { + t.Fatalf("Min tip enforced with locals enabled, error: %v", err) + } } // Tests that setting the transaction pool gas price to a higher value correctly