diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 9c48c650f..963dd0f02 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -75,7 +75,6 @@ var ( utils.TxPoolNoLocalsFlag, utils.TxPoolJournalFlag, utils.TxPoolRejournalFlag, - utils.TxPoolSizeLimitFlag, utils.TxPoolPriceLimitFlag, utils.TxPoolPriceBumpFlag, utils.TxPoolAccountSlotsFlag, diff --git a/cmd/geth/usage.go b/cmd/geth/usage.go index 7f8a20e01..8072514ad 100644 --- a/cmd/geth/usage.go +++ b/cmd/geth/usage.go @@ -117,7 +117,6 @@ var AppHelpFlagGroups = []flagGroup{ utils.TxPoolNoLocalsFlag, utils.TxPoolJournalFlag, utils.TxPoolRejournalFlag, - utils.TxPoolSizeLimitFlag, utils.TxPoolPriceLimitFlag, utils.TxPoolPriceBumpFlag, utils.TxPoolAccountSlotsFlag, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 06cf38fdc..9a9bf7793 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -260,11 +260,6 @@ var ( Usage: "Time interval to regenerate the local transaction journal", Value: core.DefaultTxPoolConfig.Rejournal, } - TxPoolSizeLimitFlag = cli.Uint64Flag{ - Name: "txsizelimit", - Usage: "Maximum size allowed for valid transaction (in KB)", - Value: 32, - } TxPoolPriceLimitFlag = cli.Uint64Flag{ Name: "txpool.pricelimit", Usage: "Minimum gas price limit to enforce for acceptance into the pool", @@ -1010,9 +1005,6 @@ func setTxPool(ctx *cli.Context, cfg *core.TxPoolConfig) { if ctx.GlobalIsSet(TxPoolRejournalFlag.Name) { cfg.Rejournal = ctx.GlobalDuration(TxPoolRejournalFlag.Name) } - if ctx.GlobalIsSet(TxPoolSizeLimitFlag.Name) { - cfg.SizeLimit = ctx.GlobalUint64(TxPoolSizeLimitFlag.Name) - } if ctx.GlobalIsSet(TxPoolPriceLimitFlag.Name) { cfg.PriceLimit = ctx.GlobalUint64(TxPoolPriceLimitFlag.Name) } diff --git a/core/genesis.go b/core/genesis.go index c11ce6e90..bc32c3ea4 100644 --- a/core/genesis.go +++ b/core/genesis.go @@ -164,6 +164,18 @@ func SetupGenesisBlock(db ethdb.Database, genesis *Genesis) (*params.ChainConfig } else { log.Info("Writing custom genesis block") } + + // Set default transaction size limit if not set in genesis + if genesis.Config.SizeLimit == 0 { + genesis.Config.SizeLimit = DefaultTxPoolConfig.SizeLimit + } + + // Check transaction size limit + err := genesis.Config.IsValid() + if err != nil { + return genesis.Config, common.Hash{}, err + } + block, err := genesis.Commit(db) return genesis.Config, block.Hash(), err } diff --git a/core/genesis_test.go b/core/genesis_test.go index 9cae05a2d..75166abed 100644 --- a/core/genesis_test.go +++ b/core/genesis_test.go @@ -20,6 +20,7 @@ import ( "math/big" "reflect" "testing" + "errors" "github.com/davecgh/go-spew/spew" "github.com/ethereum/go-ethereum/common" @@ -85,6 +86,15 @@ func TestSetupGenesis(t *testing.T) { wantHash: params.MainnetGenesisHash, wantConfig: params.MainnetChainConfig, }, + { + name: "genesis with incorrect SizeLimit", + fn: func(db ethdb.Database) (*params.ChainConfig, common.Hash, error) { + customg.Config.SizeLimit = 100000 + return SetupGenesisBlock(db, &customg) + }, + wantErr: errors.New("Genesis transaction size limit must be between 32 and 128"), + wantConfig: customg.Config, + }, // { // name: "custom block in DB, genesis == nil", // fn: func(db ethdb.Database) (*params.ChainConfig, common.Hash, error) { diff --git a/core/tx_pool.go b/core/tx_pool.go index c1f6db5f4..5f5838e11 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -152,7 +152,7 @@ var DefaultTxPoolConfig = TxPoolConfig{ Journal: "transactions.rlp", Rejournal: time.Hour, - SizeLimit: 32, + SizeLimit: 64, PriceLimit: 1, PriceBump: 10, @@ -173,10 +173,6 @@ func (config *TxPoolConfig) sanitize() TxPoolConfig { log.Warn("Sanitizing invalid txpool journal time", "provided", conf.Rejournal, "updated", time.Second) conf.Rejournal = time.Second } - if conf.SizeLimit < 32 || conf.SizeLimit > 128 { - log.Warn("Sanitizing invalid txpool size limit", "provided", conf.SizeLimit, "updated", DefaultTxPoolConfig.SizeLimit) - conf.SizeLimit = DefaultTxPoolConfig.SizeLimit - } if conf.PriceLimit < 1 { log.Warn("Sanitizing invalid txpool price limit", "provided", conf.PriceLimit, "updated", DefaultTxPoolConfig.PriceLimit) conf.PriceLimit = DefaultTxPoolConfig.PriceLimit @@ -568,12 +564,13 @@ func (pool *TxPool) local() map[common.Address]types.Transactions { // rules and adheres to some heuristic limits of the local node (price and size). func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { isQuorum := pool.chainconfig.IsQuorum + sizeLimit := pool.chainconfig.SizeLimit if isQuorum && tx.GasPrice().Cmp(common.Big0) != 0 { return ErrInvalidGasPrice } // Reject transactions over 32KB (or manually set limit) to prevent DOS attacks - if float64(tx.Size()) > float64(pool.config.SizeLimit*1024) { + if float64(tx.Size()) > float64(sizeLimit * 1024) { return ErrOversizedData } // Transactions can't be negative. This may never happen using RLP decoded diff --git a/core/tx_pool_test.go b/core/tx_pool_test.go index e4e54a7a3..e3049a794 100644 --- a/core/tx_pool_test.go +++ b/core/tx_pool_test.go @@ -284,28 +284,28 @@ func TestInvalidTransactions(t *testing.T) { t.Error("expected", ErrOversizedData, "; got", err) } - db, _ := ethdb.NewMemDatabase() - statedb, _ := state.New(common.Hash{}, state.NewDatabase(db)) - blockchain := &testBlockChain{statedb, big.NewInt(1000000), new(event.Feed)} - config := testTxPoolConfig - config.SizeLimit = 128 - pool2 := NewTxPool(config, params.TestChainConfig, blockchain) + statedb, _ := state.New(common.Hash{}, state.NewDatabase(ethdb.NewMemDatabase())) + blockchain := &testBlockChain{statedb, statedb, 1000000, new(event.Feed)} + params.TestChainConfig.SizeLimit = 128 + pool2 := NewTxPool(testTxPoolConfig, params.TestChainConfig, blockchain) + pool2.currentState.AddBalance(from, big.NewInt(0xffffffffffffff)) data2 := make([]byte, (127 * 1024)) - tx3, _ := types.SignTx(types.NewTransaction(2, common.Address{}, big.NewInt(100), big.NewInt(100000), big.NewInt(1), data2), types.HomesteadSigner{}, key) + tx3, _ := types.SignTx(types.NewTransaction(2, common.Address{}, big.NewInt(100), 100000, big.NewInt(1), data2), types.HomesteadSigner{}, key) if err := pool2.AddRemote(tx3); err != ErrIntrinsicGas { t.Error("expected", ErrIntrinsicGas, "; got", err) } data3 := make([]byte, (128*1024)+1) - tx4, _ := types.SignTx(types.NewTransaction(2, common.Address{}, big.NewInt(100), big.NewInt(100000), big.NewInt(1), data3), types.HomesteadSigner{}, key) + tx4, _ := types.SignTx(types.NewTransaction(2, common.Address{}, big.NewInt(100), 100000, big.NewInt(1), data3), types.HomesteadSigner{}, key) if err := pool2.AddRemote(tx4); err != ErrOversizedData { t.Error("expected", ErrOversizedData, "; got", err) } - tx5, _ := types.SignTx(types.NewTransaction(1, common.Address{}, big.NewInt(100), common.Big0, big.NewInt(0), nil), types.HomesteadSigner{}, key) - balance = new(big.Int).Add(tx5.Value(), new(big.Int).Mul(tx5.Gas(), tx5.GasPrice())) + tx5, _ := types.SignTx(types.NewTransaction(1, common.Address{}, big.NewInt(100), 0, big.NewInt(0), nil), types.HomesteadSigner{}, key) + balance = new(big.Int).Add(tx5.Value(), new(big.Int).Mul(new(big.Int).SetUint64(tx5.Gas()), tx5.GasPrice())) + from, _ = deriveSender(tx5) pool.currentState.AddBalance(from, balance) tx5.SetPrivate() diff --git a/params/config.go b/params/config.go index f8ea9fa9d..48168090d 100644 --- a/params/config.go +++ b/params/config.go @@ -20,6 +20,7 @@ import ( "fmt" "math" "math/big" + "errors" "github.com/ethereum/go-ethereum/common" ) @@ -103,19 +104,19 @@ var ( // // This configuration is intentionally not using keyed fields to force anyone // adding flags to the config to also have to set these fields. - AllEthashProtocolChanges = &ChainConfig{big.NewInt(1337), big.NewInt(0), nil, false, big.NewInt(0), common.Hash{}, big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), new(EthashConfig), nil, nil, false} + AllEthashProtocolChanges = &ChainConfig{big.NewInt(1337), big.NewInt(0), nil, false, big.NewInt(0), common.Hash{}, big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), new(EthashConfig), nil, nil, false, 32} // AllCliqueProtocolChanges contains every protocol change (EIPs) introduced // and accepted by the Ethereum core developers into the Clique consensus. // // This configuration is intentionally not using keyed fields to force anyone // adding flags to the config to also have to set these fields. - AllCliqueProtocolChanges = &ChainConfig{big.NewInt(1337), big.NewInt(0), nil, false, big.NewInt(0), common.Hash{}, big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), nil, &CliqueConfig{Period: 0, Epoch: 30000}, nil, false} + AllCliqueProtocolChanges = &ChainConfig{big.NewInt(1337), big.NewInt(0), nil, false, big.NewInt(0), common.Hash{}, big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), nil, &CliqueConfig{Period: 0, Epoch: 30000}, nil, false, 32} - TestChainConfig = &ChainConfig{big.NewInt(10), big.NewInt(0), nil, false, big.NewInt(0), common.Hash{}, big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), new(EthashConfig), nil, nil, false} + TestChainConfig = &ChainConfig{big.NewInt(10), big.NewInt(0), nil, false, big.NewInt(0), common.Hash{}, big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), new(EthashConfig), nil, nil, false, 32} TestRules = TestChainConfig.Rules(new(big.Int)) - QuorumTestChainConfig = &ChainConfig{big.NewInt(10), big.NewInt(0), nil, false, nil, common.Hash{}, nil, nil, nil, nil, new(EthashConfig), nil, nil, true} + QuorumTestChainConfig = &ChainConfig{big.NewInt(10), big.NewInt(0), nil, false, nil, common.Hash{}, nil, nil, nil, nil, new(EthashConfig), nil, nil, true, 64} ) // ChainConfig is the core config which determines the blockchain settings. @@ -147,6 +148,7 @@ type ChainConfig struct { Istanbul *IstanbulConfig `json:"istanbul,omitempty"` IsQuorum bool `json:"isQuorum"` + SizeLimit uint64 `json:"txnSizeLimit"` } // EthashConfig is the consensus engine configs for proof-of-work based sealing. @@ -207,6 +209,14 @@ func (c *ChainConfig) String() string { ) } +func (c *ChainConfig) IsValid() error { + if c.SizeLimit < 32 || c.SizeLimit > 128 { + return errors.New("Genesis transaction size limit must be between 32 and 128") + } + + return nil +} + // IsHomestead returns whether num is either equal to the homestead block or greater. func (c *ChainConfig) IsHomestead(num *big.Int) bool { return isForked(c.HomesteadBlock, num)