From 835579e3041c9cef2ed76e3380fa77a3fe634467 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Tue, 23 Dec 2025 18:10:24 +0800 Subject: [PATCH] core/txpool: add validation and comprehensive tests for SetGasPrice (#1876) This commit adds robust input validation to the SetGasPrice method and implements comprehensive table-driven tests to ensure correct behavior. Changes in core/txpool/txpool.go: - Add nil gas price validation with graceful error handling - Add validation to reject negative gas prices - Add validation to reject gas prices exceeding 1000 GWei maximum - Return detailed error messages for each validation failure - Log warnings when invalid gas prices are rejected Changes in eth/api_miner.go: - Update MinerAPI.SetGasPrice to check error return from txPool.SetGasPrice - Return false when validation fails (when SetGasPrice returns error) - Return true when validation succeeds (when SetGasPrice returns nil) New tests in core/txpool/txpool_test.go: - Implement TestSetGasPrice using table-driven test pattern - Test 4 invalid cases: nil, negative, exceed max+1, exceed 10000 GWei - Test 7 valid cases: 0, 1 wei, 1 GWei, 100 GWei, 500 GWei, max-1, max - Each test case includes expected error value for precise validation - All 11 test cases verify both error returns and gas price state - Tests use isolated pool instances to ensure independence --- core/txpool/txpool.go | 22 +++++++- core/txpool/txpool_test.go | 111 +++++++++++++++++++++++++++++++++++++ eth/api_miner.go | 4 +- 3 files changed, 133 insertions(+), 4 deletions(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index db7e9cba76..327a2425e3 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -215,6 +215,8 @@ var DefaultConfig = Config{ Lifetime: 3 * time.Hour, } +var defaultMaxPrice = big.NewInt(1000 * params.GWei) + // sanitize checks the provided user configurations and changes anything that's // unreasonable or unworkable. func (config *Config) sanitize() Config { @@ -466,11 +468,26 @@ func (pool *TxPool) GasPrice() *big.Int { } // SetGasPrice updates the minimum price required by the transaction pool for a -// new transaction, and drops all transactions below this threshold. -func (pool *TxPool) SetGasPrice(price *big.Int) { +// new transaction, and drops all transactions below this threshold. Negative +// gas prices and prices exceeding 1000 GWei are considered invalid and will be +// rejected without updating the threshold. +func (pool *TxPool) SetGasPrice(price *big.Int) error { pool.mu.Lock() defer pool.mu.Unlock() + if price == nil { + log.Warn("Reject nil gas price") + return errors.New("reject nil gas price") + } + if price.Sign() < 0 { + log.Warn("Reject invalid gas price", "price", price) + return fmt.Errorf("reject negative gas price: %v", price) + } + if price.Cmp(defaultMaxPrice) > 0 { + log.Warn("Reject invalid gas price", "price", price, "max", defaultMaxPrice) + return fmt.Errorf("reject too high gas price: %v, maximum: %v", price, defaultMaxPrice) + } + old := pool.gasPrice pool.gasPrice = price // if the min miner fee increased, remove transactions below the new threshold @@ -484,6 +501,7 @@ func (pool *TxPool) SetGasPrice(price *big.Int) { } log.Info("Transaction pool price threshold updated", "price", price) + return nil } // Nonce returns the next nonce of an account, with all transactions executable diff --git a/core/txpool/txpool_test.go b/core/txpool/txpool_test.go index e4eaa9aeb7..f9bea9345e 100644 --- a/core/txpool/txpool_test.go +++ b/core/txpool/txpool_test.go @@ -2693,3 +2693,114 @@ func BenchmarkMultiAccountBatchInsert(b *testing.B) { pool.AddRemotesSync([]*types.Transaction{tx}) } } + +// TestSetGasPrice tests the SetGasPrice validation logic using table-driven tests +func TestSetGasPrice(t *testing.T) { + testCases := []struct { + name string + price *big.Int + wantErr error + description string + }{ + // Invalid cases - should be rejected + { + name: "nil gas price", + price: nil, + wantErr: errors.New("reject nil gas price"), + description: "nil pointer should be rejected gracefully", + }, + { + name: "negative gas price", + price: big.NewInt(-1), + wantErr: fmt.Errorf("reject negative gas price: %v", big.NewInt(-1)), + description: "negative value should be rejected", + }, + { + name: "exceeds maximum by 1", + price: new(big.Int).Add(defaultMaxPrice, big.NewInt(1)), + wantErr: fmt.Errorf("reject too high gas price: %v, maximum: %v", new(big.Int).Add(defaultMaxPrice, big.NewInt(1)), defaultMaxPrice), + description: "value exceeding 1000 GWei should be rejected", + }, + { + name: "exceeds maximum significantly", + price: big.NewInt(10000 * params.GWei), + wantErr: fmt.Errorf("reject too high gas price: %v, maximum: %v", big.NewInt(10000*params.GWei), defaultMaxPrice), + description: "value far exceeding maximum should be rejected", + }, + // Valid cases - should be accepted + { + name: "zero gas price", + price: big.NewInt(0), + wantErr: nil, + description: "zero is valid as it's not negative", + }, + { + name: "minimum positive value", + price: big.NewInt(1), + wantErr: nil, + description: "minimum positive value should be accepted", + }, + { + name: "1 GWei", + price: big.NewInt(params.GWei), + wantErr: nil, + description: "1 GWei should be accepted", + }, + { + name: "100 GWei", + price: big.NewInt(100 * params.GWei), + wantErr: nil, + description: "100 GWei should be accepted", + }, + { + name: "500 GWei", + price: big.NewInt(500 * params.GWei), + wantErr: nil, + description: "500 GWei should be accepted", + }, + { + name: "just below maximum", + price: new(big.Int).Sub(defaultMaxPrice, big.NewInt(1)), + wantErr: nil, + description: "value just below maximum should be accepted", + }, + { + name: "exactly at maximum", + price: defaultMaxPrice, + wantErr: nil, + description: "exactly 1000 GWei should be accepted", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a fresh pool instance for each test case to ensure isolation + pool, _ := setupPool() + defer pool.Stop() + + oldPrice := pool.GasPrice() + haveErr := pool.SetGasPrice(tc.price) + newPrice := pool.GasPrice() + + if tc.wantErr != nil { + // Invalid case: should return error and price should remain unchanged + if haveErr == nil { + t.Errorf("%s: Expected error %q, got nil", tc.description, tc.wantErr) + } else if haveErr.Error() != tc.wantErr.Error() { + t.Errorf("%s: Expected error %q, got %q", tc.description, tc.wantErr, haveErr) + } + if newPrice.Cmp(oldPrice) != 0 { + t.Errorf("%s: Gas price should not change. Expected %v, got %v", tc.description, oldPrice, newPrice) + } + } else { + // Valid case: should return nil error and price should be updated + if haveErr != nil { + t.Errorf("%s: Expected no error, got: %v", tc.description, haveErr) + } + if newPrice.Cmp(tc.price) != 0 { + t.Errorf("%s: Expected gas price to be set to %v, got %v", tc.description, tc.price, newPrice) + } + } + }) + } +} diff --git a/eth/api_miner.go b/eth/api_miner.go index 89433af267..b316d950da 100644 --- a/eth/api_miner.go +++ b/eth/api_miner.go @@ -97,8 +97,8 @@ func (api *MinerAPI) SetGasPrice(gasPrice hexutil.Big) bool { api.e.gasPrice = (*big.Int)(&gasPrice) api.e.lock.Unlock() - api.e.txPool.SetGasPrice((*big.Int)(&gasPrice)) - return true + err := api.e.txPool.SetGasPrice((*big.Int)(&gasPrice)) + return err == nil } // SetEtherbase sets the etherbase of the miner