feat(consensus): allow configuring block gas limit through `ChainSpec`

[tcoratger]

Should solve #3233.

[codecov[bot]]

Codecov Report

Merging #3253 (0b00744) into main (8dedf0f) will decrease coverage by 0.02%. The diff coverage is 76.47%.

Impacted Files	Coverage Δ
crates/consensus/auto-seal/src/task.rs	0.00% <0.00%> (ø)
crates/payload/basic/src/lib.rs	0.19% <0.00%> (ø)
crates/primitives/src/constants.rs	100.00% <ø> (ø)
crates/transaction-pool/src/validate/eth.rs	0.00% <0.00%> (ø)
crates/primitives/src/chain/spec.rs	97.35% <100.00%> (+0.01%)	:arrow_up:
crates/primitives/src/hardfork.rs	100.00% <100.00%> (ø)
crates/rpc/rpc/src/eth/revm_utils.rs	36.98% <100.00%> (ø)
crates/staged-sync/src/utils/init.rs	97.20% <100.00%> (+0.01%)	:arrow_up:
crates/transaction-pool/src/pool/txpool.rs	60.53% <100.00%> (ø)

... and 4 files with indirect coverage changes

Flag	Coverage Δ
integration-tests	16.28% <41.17%> (+0.01%)	:arrow_up:
unit-tests	64.26% <70.58%> (-0.03%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
reth binary	23.01% <ø> (ø)
blockchain tree	81.69% <ø> (ø)
pipeline	87.89% <100.00%> (+<0.01%)	:arrow_up:
storage (db)	74.17% <ø> (ø)
trie	95.64% <ø> (ø)
txpool	51.37% <33.33%> (-0.65%)	:arrow_down:
networking	77.95% <ø> (+0.04%)	:arrow_up:
rpc	58.12% <100.00%> (ø)
consensus	62.78% <0.00%> (ø)
revm	35.14% <ø> (ø)
payload builder	6.83% <0.00%> (ø)
primitives	88.67% <100.00%> (+0.01%)	:arrow_up:

[onbjerg]

Needs a rebase

[tcoratger]

#3233 is more about being able to set block gas limit through the ChainSpec instance, not just having a default method on it.

e.g. user runs reth node --block-gas-limit 1000000 and everywhere where we currently use a constant, it would instead use the user provided value

@rkrasiuk I just added block_gas_limit to the CLI. I still keep a default value in ChainSpec directly because many tests and functions only ask for a default value and do not have access to the ChainSpec instance provided by the CLI. For example, there is the following implementation for which no argument is provided and therefore only asks for a default value without fetching additional information as I understand it:

impl<T: PoolTransaction> Default for AllTransactions<T> {
    fn default() -> Self {
        Self {
            max_account_slots: MAX_ACCOUNT_SLOTS_PER_SENDER,
            minimal_protocol_basefee: MIN_PROTOCOL_BASE_FEE,
            block_gas_limit: ChainSpec::block_gas_limit_default(),
            by_hash: Default::default(),
            txs: Default::default(),
            tx_counter: Default::default(),
            last_seen_block_number: 0,
            last_seen_block_hash: Default::default(),
            pending_basefee: Default::default(),
        }
    }
}

[onbjerg]

@tcoratger It's still not entirely done, the main issue here is that even if I set a different gas limit in my chainspec, this will not actually affect much of the node. For example:

• All the tracing RPCs still use a gas limit of 30_000_000 because prepare_call_env just does env.tx.gas_limit = ChainSpec::block_gas_limit_default(), which is incorrect. It should use the spec.
• The transaction pool suffers the same issue, it just uses ChainSpec::block_gas_limit_default(), so if I changed the limit, I would still end up building incorrect blocks
• The payload builder also defaults to 30_000_000, but at least this is configurable via the CLI.

[tcoratger]

@tcoratger It's still not entirely done, the main issue here is that even if I set a different gas limit in my chainspec, this will not actually affect much of the node. For example:

• All the tracing RPCs still use a gas limit of 30_000_000 because prepare_call_env just does env.tx.gas_limit = ChainSpec::block_gas_limit_default(), which is incorrect. It should use the spec.
• The transaction pool suffers the same issue, it just uses ChainSpec::block_gas_limit_default(), so if I changed the limit, I would still end up building incorrect blocks
• The payload builder also defaults to 30_000_000, but at least this is configurable via the CLI.

@onbjerg Yes I suspected this problem. This will probably sound like a stupid question to you as I'm sure it's obvious with familiarity with the codebase, but in those two functions you mention, I didn't see any variable that could hold the instance of ChainSpec, so how to access it?

[onbjerg]

@tcoratger if there is not, you'd have to add it, or make the value propagate some other way - follow the code path. Unfortunately I am not super familiar with those 2 parts specifically :)

[gakonst]

@mattsse should we just pass in the chainspec as it'd include default block gas limit and it'd be useful across the board in components? and have our builder structs maybe default to it having some value (eg mainnet)