Skip to content

fix(pipe): close gravity-audit#621 + #712 (gas_limit hardcode + budget)#358

Open
nekomoto911 wants to merge 1 commit into
Galxe:mainfrom
nekomoto911:fix/pipe-gas-limit
Open

fix(pipe): close gravity-audit#621 + #712 (gas_limit hardcode + budget)#358
nekomoto911 wants to merge 1 commit into
Galxe:mainfrom
nekomoto911:fix/pipe-gas-limit

Conversation

@nekomoto911

Copy link
Copy Markdown
Collaborator

Summary

Two consensus-critical fixes to the pipe execution layer's block gas_limit plumbing. Both audit issues land on the same code path (create_block_for_executor / execute_ordered_block in crates/pipe-exec-layer-ext-v2/execute/src/lib.rs); fixing them in one PR avoids touch-the-same-line conflicts.

gravity-audit#712 (HIGH — consensus divergence / halt)

Galxe/gravity-audit#712 — block gas_limit was sourced from per-node --gravity.pipe-block-gas-limit CLI flag (get_gravity_config().pipe_block_gas_limit in lib.rs:657/1023 and metadata_txn.rs:91). Validators with mismatched values produced different block hashes from the same OrderedBlock, and the verification path's .unwrap() would panic-halt on mismatch.

Fix: hardcode to a single workspace const gravity_primitives::PIPE_BLOCK_GAS_LIMIT = 1_000_000_000. The CLI flag, Config.pipe_block_gas_limit field, default initializers, and all read-sites are removed. Mainnet is pre-launch so no compat/migration shims.

gravity-audit#621 (HIGH — gas_used > gas_limit)

Galxe/gravity-audit#621 — system txns (metadata onBlockStart + each DKG/JWK validator txn) execute before user-tx filtering, then their gas is appended to the block result via SystemTxnResult::insert_to_executed_ordered_block_result:

L1064  execute_system_transactions(...)      // metadata + validator txns; gas known
L1101  create_block_for_executor(...)        // user-tx filter uses full block.gas_limit ← bug
L1112  executor.execute(&block)              // user gas_used returned
L1129  block.header.gas_used = outcome.gas_used
L1139  metadata_txn_result.insert_to_...(...)  // metadata gas appended after the fact
L1143  validator_result.insert_to_...(...)     // each validator gas appended after the fact

A user block sized to fit exactly under block.gas_limit plus any system overhead pushed header.gas_used > header.gas_limit in release builds. The existing validate_execution_output guard (lib.rs:324) is #[cfg(debug_assertions)]-only so release had no enforcement.

Fix A (per the issue thread): plumb sum_system_gas into create_block_for_executor (computed from already-executed system results, so it's the exact gas, not the tx.gas_limit() upper bound), then apply block.gas_limit.saturating_sub(sum_system_gas) to the budget passed to filter_invalid_txs. The user-tx filter still uses tx.gas_limit() for accumulation — that's a conservative upper bound, so the shrunk budget can over-truncate by at most one tx, never under-truncate. block.header.gas_limit itself is unchanged so RPC / indexer / protocol-layer semantics are preserved.

saturating_sub handles the byzantine path (sum_system_gas ≥ block.gas_limit): budget collapses to 0, every user tx is dropped, the block runs system-only — the safe fallback. New unit test test_filter_invalid_txs_zero_budget_drops_all pins this contract.

Fix B from the issue thread (release-mode gas_used ≤ gas_limit assertion) is intentionally not included here; it's defense-in-depth, and once Fix A is in place the failure mode is provably impossible. Can be done as a follow-up if the team wants the belt-and-suspenders guarantee.

Test plan

  • cargo nextest run -p reth-pipe-exec-layer-ext-v2 --lib — 45/45 pass, including new test_filter_invalid_txs_zero_budget_drops_all
  • cargo nextest run -p reth-engine-tree --lib recovery:: — 6/6 pass (test init shim updated for the removed pipe_block_gas_limit field)
  • cargo +nightly fmt --all --check
  • No new clippy findings on touched files (workspace pre-existing clippy debt in pipe-exec-layer-ext-v2 and pipe-exec-layer-relayer is not introduced by this PR)
  • cargo check -p reth-node-core -p gravity-primitives -p reth-pipe-exec-layer-ext-v2 -p reth-engine-tree
  • Cluster-level repro for #712: two nodes with mismatched gas_limit would diverge; with the flag removed, divergence path is structurally gone (no per-node knob remains)

Files

gravity-primitives (new const, drop field) · node/core/args/gravity (drop CLI flag) · engine/tree/recovery (test shim) · pipe-exec-layer-ext-v2/execute/{lib,onchain_config/metadata_txn,tx_filter}

Two consensus-critical fixes to the pipe execution layer's block gas_limit
plumbing:

#712 (HIGH, consensus divergence): block gas_limit was sourced from per-node
`--gravity.pipe-block-gas-limit` CLI flag, so validators with mismatched
values produced different block hashes from the same OrderedBlock. Hardcoded
to a single workspace const `PIPE_BLOCK_GAS_LIMIT = 1_000_000_000`; CLI flag,
config field, and all init shims removed (mainnet pre-launch, no compat
needed).

#621 (HIGH, gas_used > gas_limit): system txns (metadata + DKG/JWK) run
*before* user-tx filtering, then their gas is appended to the result via
`SystemTxnResult::insert_to_executed_ordered_block_result`. The user-tx
filter received the full `block.gas_limit` as budget, so a saturating user
block plus system overhead could push `header.gas_used > header.gas_limit`
in release builds (`validate_execution_output` is debug-only). Fix A from
the issue thread: pass `sum_system_gas` into `create_block_for_executor`
(computed from already-executed system results, so it's exact, not an
upper bound) and apply `block.gas_limit.saturating_sub(sum_system_gas)` to
the filter budget. `saturating_sub` covers the byzantine path where
`sum_system_gas ≥ block.gas_limit` (all user txs dropped, system-only block
is the safe fallback). `block.header.gas_limit` itself is unchanged so RPC /
indexer / protocol-layer semantics stay stable.
@nekomoto911 nekomoto911 added bug Something isn't working mainnet labels Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working mainnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant