fix(genesis): reject zero block gas limit

[Kewe63]

Summary

Fixes #321

This updates the genesis ProtocolConfig schema so blockGasLimit must be greater than zero.

Previously, genesis validation accepted blockGasLimit: 0n:

blockGasLimit: schemaBigInt.min(0n).max(maxUint64)

but the runtime ProtocolConfig API rejects zero block gas limits. That allowed genesis configuration to bypass the same invariant enforced by normal protocol config updates.

---

Changes

• Change the genesis blockGasLimit lower bound from 0n to 1n.
• Add unit coverage for:
  • accepting blockGasLimit: 1n
  • rejecting blockGasLimit: 0n
  • rejecting blockGasLimit above uint64

---

Why

Genesis configuration should enforce the same basic invariant as runtime config updates. A zero block gas limit can create an unusable or inconsistent protocol configuration, and it should fail during genesis schema validation instead of being accepted.

---

Tests

Regression test before the fix:

npx mocha -r ts-node/register ./tests/unit/protocol-config-genesis.test.ts

Result before fix:

2 passing
1 failing

Failure:

AssertionError: expected [Function] to throw an error

After the fix:

npx mocha -r ts-node/register ./tests/unit/protocol-config-genesis.test.ts

Result:

3 passing

Formatting:

npx prettier --config ./.prettierrc --check scripts/genesis/ProtocolConfig.ts tests/unit/protocol-config-genesis.test.ts

Result:

All matched files use Prettier code style!

Lint:

npx eslint scripts/genesis/ProtocolConfig.ts tests/unit/protocol-config-genesis.test.ts

Result:

passed

---

Checklist

• Tests pass — 3/3, confirmed failing before fix
• Prettier / ESLint clean
• Follows Conventional Commits
• Changes scoped to this fix only

---

Risk & Impact

Low. The bound only rejects blockGasLimit: 0n, which was already invalid per the runtime ProtocolConfig API — any genesis config using a positive gas limit is unaffected. Verified the regression test fails against the old schema and passes with the fix, confirming it exercises the actual invariant mismatch.

Type: 🐛 Bug fix
Fixes: #321

[osr21]

Reviewed at a5917d0. The fix is correct and the diagnosis is right — I confirmed both require sites it mirrors. But I think the PR undersells its own motivation, and there are two things worth acting on before this lands: a merge conflict with your own #326, and an inconsistency between how these two PRs apply the same principle.

Verified

• .min(1n) exactly mirrors the runtime rule, which is enforced in two places, not one: ProtocolConfig.sol:141 (require(newParams.blockGasLimit > 0, InvalidBlockGasLimit()) in updateFeeParams) and ProtocolConfig.sol:175 (same error in updateBlockGasLimit). The bound matches both.
• Backward compatibility holds — every committed genesis config (devnet, localdev, mainnet, testnet, both config.json and genesis.config.ts forms) uses 30_000_000. Nothing starts failing.

The reachability story is better than "could bypass"

This isn't only theoretical. assets/localdev/genesis.config.ts:134 reads:

blockGasLimit: blockGasLimit ?? 30_000_000n,

...where blockGasLimit is an optional builder option (z.bigint().optional(), line 41). Nullish coalescing does not filter zero:

0n ?? 30_000_000n  ->  0n

So an explicit blockGasLimit: 0n passed to the localdev builder flows straight past the default and into the schema. Pre-fix, make genesis would happily emit a genesis with a zero gas limit. Worth putting in the description — it turns "invariant mismatch" into a concrete trigger path.

Why this class of bug exists at all (stronger framing than the PR gives)

The description says genesis "bypasses the same invariant enforced by normal protocol config updates." It's more absolute than that. Genesis never calls initialize() or any setter — scripts/genesis/ProtocolConfig.ts writes ERC-7201 storage slots directly via StorageSlot(slotIndex(PROTOCOL_CONFIG_STORAGE_LOCATION + n), toBytes32(...)).

So none of the contract's requires ever execute on the genesis path. The zod schema isn't a convenient early check duplicating an on-chain guard — for genesis-authored values it is the only validation that will ever run. That's the real argument for this PR, and for treating every unmirrored require as a live gap rather than defence-in-depth.

This fixes one of three gaps

Comparing the contract's invariants against the genesis schema field by field:

Runtime invariant	Contract	Genesis schema
blockGasLimit > 0	:141, :175	fixed by this PR
minBaseFee <= maxBaseFee	:140 InvalidBaseFeeRange	not enforced
7x timeout*Ms > 0	:156–:162	not enforced

The minBaseFee <= maxBaseFee one is the quieter of the two: it's cross-field, and the schema's .superRefine() currently only runs enforceOperatorsNotProxyAdmin. There is no cross-field fee check anywhere in the file. A genesis with minBaseFee above maxBaseFee validates clean today and gets written to storage in a state updateFeeParams would reject outright.

Not asking you to expand this PR — scoping it tightly is right. But it's worth a follow-up issue so the class gets closed rather than just this instance.

⚠️ Inconsistency with #326 — worth fixing there, and it's the same principle

#326 bounds the seven consensus timeouts as schemaUint16 = schemaBigInt.min(0n).max(65535n). By the exact reasoning in this PR, .min(0n) is the wrong floor for those seven — the contract requires > 0 for every one of them (:156–:162). After both PRs merge, genesis would still accept timeoutProposeMs: 0n, which updateConsensusParams reverts on with InvalidTimeoutProposeMs. Same invariant-mismatch class this PR exists to close.

What makes this clean rather than a judgement call: targetBlockTimeMs is the only consensus field with no > 0 requirement in the contract. It appears exactly once in the whole contract tree — IProtocolConfig.sol:42 as uint16 targetBlockTimeMs — with no setter and no validation. That lines up with crates/eth-engine/src/abi_utils.rs:140, which treats targetBlockTimeMs == 0 as absent (target_block_time() returns None).

So the contract tells you precisely which bound each field wants: .min(1n) for the seven timeouts, .min(0n) for targetBlockTimeMs alone. Cheap change in #326, and it makes the two PRs tell one consistent story.

🔴 #326 and #327 conflict — both add the same file

Both PRs add tests/unit/protocol-config-genesis.test.ts as a new file (#326: +82, #327: +79). Verified empirically rather than assumed:

main + #326                -> clean
main + #326 + #327         -> CONFLICT (add/add): tests/unit/protocol-config-genesis.test.ts

The ProtocolConfig.ts hunks auto-merge fine (different lines — blockGasLimit vs. the consensus block); it's purely the test file. Both PRs are currently mergeable_state: blocked, so whichever lands second needs a rebase regardless.

Since both files test the same schema with near-identical fixtures, merging them into one file with two describe blocks is probably cleaner than renaming — but either works. Flagging now because it's invisible from inside a single PR.

Minor

1. maxUint64 is hand-redeclared. The test defines const maxUint64 = 18446744073709551615n, and ProtocolConfig.ts:46 does the same. But viem — already imported in that file on line 30 — exports it:

viem.maxUint64 -> 18446744073709551615n
viem.maxUint16 -> 65535n

Three hand-written copies of the same constant across two PRs (#326 adds a local maxUint16 too) is a small drift risk for zero benefit. import { maxUint16, maxUint64 } from 'viem' covers both.

2. The uint64 test doesn't exercise this change. .max(maxUint64) is untouched by the diff, so that third case passes identically before and after. Fine as regression coverage — just noting the "1 failing before the fix" figure comes from the zero case alone.

3. Same CI caveat as #326. tests/unit/*.test.ts is globbed by make test-unit-hardhat (npx hardhat test ... --no-compile), but no workflow in .github/workflows/ invokes that target or hardhat at all — ci.yml is Rust fmt/sort/clippy/nextest, proto, contracts lint/build/forge, and eslint. The test will pass locally and never run in CI. Pre-existing, not yours. Also worth re-running under make test-unit-hardhat rather than bare npx mocha, since that's the harness the repo actually uses (it loads hardhat-toolbox-viem and the custom matchers plugin).

Good, tightly-scoped fix with a correct root cause. The conflict with #326 and the .min(0n) floor on the timeouts are the two things I'd resolve before either merges.