Skip to content

testing: add testing_commitBlockV1 spec, fixtures, and shared gas limit env#801

Open
MysticRyuujin wants to merge 5 commits into
ethereum:mainfrom
MysticRyuujin:testing-commit-block-v1
Open

testing: add testing_commitBlockV1 spec, fixtures, and shared gas limit env#801
MysticRyuujin wants to merge 5 commits into
ethereum:mainfrom
MysticRyuujin:testing-commit-block-v1

Conversation

@MysticRyuujin

@MysticRyuujin MysticRyuujin commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the spec yaml and make fill fixtures for testing_commitBlockV1, the write companion of testing_buildBlockV1. The method builds a block from the supplied attributes on top of the current canonical head, inserts it, and sets it as the new head — the read/write counterpart of testing_buildBlockV1, skipping the engine_newPayload + engine_forkchoiceUpdated round-trip used by consensus clients.

This refines the original spec proposal in #787 by dropping the parentHash parameter to match the Nethermind reference implementation in NethermindEth/nethermind#11385. The method always operates on the current canonical head.

Companion PRs

This PR is the anchor for cross-client interop. The other three implementations agreeing with this spec ship in:

What's in this PR

  • src/testing/testing_commitBlockV1.yaml — OpenRPC spec for the new method.
  • tools/testgen/generators.go — five subtests: empty txs, with-transactions, with-extra-data, invalid-transaction, and from-mempool (SpecOnly). Subtest names are lexically ordered to match hive's filepath.Walk replay order so chain-head progression at fill time matches at replay time.
  • tests/testing_commitBlockV1/*.io — recorded fixtures via go-ethereum + make fill. Re-running make fill produces byte-identical output (full corpus diff is empty across consecutive runs).
  • tools/chain/forkenv.json + tests/forkenv.json — adds HIVE_TARGET_GAS_LIMIT=60000000 (geth's default --miner.gaslimit). With both clients targeting the same ceiling, CalcGasLimit produces the same gas limit at each block, so the recorded fixtures replay byte-exact on both. This also rescues the existing testing_buildBlockV1 strict-match tests, which silently diverged on Nethermind before this fix.

Why drop parentHash?

The Nethermind reference impl in #11385 takes (payloadAttributes, txRlps, extraData?) and builds on top of the current head — since the method is committing to the canonical chain, there's no useful semantics for an arbitrary parent. The spec yaml here matches that signature exactly.

Test plan

  • make build (specgen) passes
  • make test (speccheck) passes
  • make fill against local go-ethereum produces byte-identical output across consecutive runs
  • hive rpc-compat simulator passes 216/216 against go-ethereum with the new fixtures
  • hive rpc-compat simulator passes 9/9 testing_* tests against patched Nethermind (the 7 remaining Nethermind failures are pre-existing eth_simulateV1 and txpool_content issues unrelated to this work)

Adds the spec yaml and testgen fixtures for testing_commitBlockV1, the
write companion of testing_buildBlockV1. The method builds a block from
the provided parameters on top of the current canonical head, inserts
it, and sets it as the new head, returning the block hash. It's the
read/write counterpart to testing_buildBlockV1, skipping the
engine_newPayload + engine_forkchoiceUpdated round-trip used by
consensus clients.

This refines the original spec proposal in ethereum#787 by dropping the
parentHash parameter (matching the Nethermind impl from
NethermindEth/nethermind#11385).

Also adds HIVE_TARGET_GAS_LIMIT to forkenv.json so geth and Nethermind
target the same gas-limit ceiling under hive — without this both clients
diverge on the target gas limit and produce non-matching block hashes,
which made the testing_buildBlockV1 strict-match tests already on main
fail on Nethermind. Pinning to 60000000 (geth's miner default) keeps
the recorded fixtures identical across clients.

testing_commitBlockV1 mutates the canonical head, so the MethodTests
entry is registered after testing_buildBlockV1 and before the txpool
tests to match hive's lexical replay order. The from-mempool subtest is
named "z-from-mempool" so lexical sort puts it last (its result is
SpecOnly because mempool composition is non-deterministic).
MysticRyuujin added a commit to MysticRyuujin/hive that referenced this pull request May 18, 2026
…E_TARGET_GAS_LIMIT

Without an explicit target, geth and Nethermind use different default
gas-limit policies under the engine API: geth's CalcGasLimit moves the
parent gas limit toward miner.gaslimit (default 60M) by 1/1024 per
block, while Nethermind keeps the parent gas limit unchanged unless
Blocks.TargetBlockGasLimit is set. With the rpc-compat chain head sitting
at ~75M, the two clients diverge from block 1 onward, so blocks produced
by testing_buildBlockV1 / testing_commitBlockV1 in rpc-compat replay get
different hashes between clients and the strict-match tests fail on
Nethermind.

Wiring a new HIVE_TARGET_GAS_LIMIT env var through geth.sh
(--miner.gaslimit) and Nethermind's mkconfig.jq (Blocks.TargetBlockGasLimit)
makes both clients target the same ceiling. With the value set to
60000000 in execution-apis/tools/chain/forkenv.json, CalcGasLimit
produces the same gas limit at every step on both clients, so the
recorded rpc-compat fixtures replay byte-exact on both.

Spec and cross-client fixtures: ethereum/execution-apis#801
LukaszRozmej pushed a commit to NethermindEth/nethermind that referenced this pull request May 18, 2026
The spec for testing_commitBlockV1 defines txRlps as oneOf [array, null]
where null instructs the client that it MAY build from the local txpool.
testing_buildBlockV1 already takes IEnumerable<byte[]>? on this branch,
but the commit variant still takes a non-nullable IEnumerable<byte[]>,
so JSON-RPC requests with a null txRlps fail at parameter binding with
-32602 "Missing parameter does not have a default value" before ever
reaching the implementation.

ProduceBlockAsync already handles `txRlps is null` correctly (it falls
back to env.TxSource.GetTransactions). Making the entry-point parameter
nullable on both ITestingRpcModule and TestingRpcModule is enough to
make the spec-compliant null case dispatch.

Spec and cross-client fixtures: ethereum/execution-apis#801
@MysticRyuujin

Copy link
Copy Markdown
Contributor Author

Nethermind released a fix, the current go-ethereum PR and Nethermind main match and these tests pass

eth_simulateV1 base-fee errors now return -38012 (standardized code)
instead of -32602; mempool-dependent commitBlock/txpool fixtures
refreshed.
pull Bot pushed a commit to Dustin4444/erigon that referenced this pull request Jul 13, 2026
Closes erigontech#22392

Implements `testing_commitBlockV1`, the write companion to
`testing_buildBlockV1`, in the `testing_` RPC namespace (enabled via
`--http.api=...,testing`, never on production networks).

## Spec conformance

Follows the execution-apis proposal
[ethereum/execution-apis#801](ethereum/execution-apis#801):

- Parameters: `payloadAttributes` (required), `transactions` (`array |
null`), `extraData` (optional) — no `parentHash`: the block is always
built on top of the current canonical head, as the spec mandates ("the
client MUST build a new execution payload on top of its current
canonical head").
- `transactions = []` → builds an empty block (mempool bypassed); `null`
→ builds from the local mempool; non-empty array → exactly those
transactions, in order, with a strict nonce pre-check against state.
- On success the block is inserted, validated, and set as the canonical
head through the standard fork-choice path, so the same chain events
fire as for any new head; safe and finalized hashes are preserved.
Returns the new head's block hash.
- On any failure the canonical head is left unchanged (fork choice is
the last step, structurally), with one exception: `UpdateForkChoice`
runs asynchronously in the execution module, so if the slot budget
expires mid fork choice the call returns a busy error but the head can
still advance afterwards.
- Error codes: `-32602` invalid params (including `extraData` over 32
bytes), `-32000` for nonce/validation failures, `-38002`/`-38006` for
invalid fork choice / too-deep reorg.

## Parity with go-ethereum

Mirrors geth's implementation
([ethereum/go-ethereum#34995](ethereum/go-ethereum#34995)):
same signature (`payloadAttributes, *[]hexutil.Bytes, *hexutil.Bytes` →
`common.Hash`), same build → insert → set-canonical flow (erigon-native
equivalent: `AssembleBlock`/`GetAssembledBlock` → `InsertBlock` →
`ValidateChain` → `UpdateForkChoice`), same extraData handling through
the block builder. Erigon is additionally stricter than geth: it
validates timestamp > parent and pre-checks nonces, returning a clear
error instead of a failed build.

As in geth, `extraData` is applied by the block builder itself: it is
now a `builder.Parameters` field consumed in `create_block.go`
(nil-guarded for the production engine path, which never sets it and is
unchanged). This also fixes `testing_buildBlockV1`, which previously
patched extraData onto its response after the fact instead of building
the block with it. Within the `testing_` namespace, a `nil` `extraData`
now forces empty extra data rather than falling back to the builder's
configured default (which embeds the erigon version) — matching geth's
testing path and keeping block hashes deterministic across erigon
versions. `extraData` longer than 32 bytes is rejected up front with
`-32602` instead of building and inserting an uncommittable block.

## Unit tests (TDD)

Written before the implementation (red → green). 24 sub-tests in
`testing_api_commit_test.go`:

- **Validation**: nil payloadAttributes; no canonical head; timestamp
not greater than head; invalid transaction bytes; nonce too high
(rejected before insertion); missing parentBeaconBlockRoot for Cancun+;
missing withdrawals for Shanghai; extraData longer than 32 bytes
- **Happy paths**: commit with mempool build; empty transaction list via
custom provider; explicit valid transaction list (order + exclusivity
via the provider); extraData forwarded to the block builder; nil
extraData forces empty (deterministic) extra data; block access list
propagated to insertion (Amsterdam+)
- **Head immutability on failure**: insert failure; insert error; bad
block on validation (fork choice never runs); fork choice failure
- **Error codes**: `-38002` invalid fork choice; `-38006` reorg too
deep; `-32000` with validation error message
- **Busy handling**: busy on validation; busy on fork choice
(deadline-bounded polling)

Plus updated `BuildBlockV1` tests covering the shared
assembly/validation path and the builder-level extraData propagation
(including the same extraData size and determinism cases).

`InsertBlock` is guarded with the closure+`defer` idiom used elsewhere
in the file, so a panic can't leave the engine lock held. `slotDeadline`
is computed up front in `CommitBlockV1`, before
`CurrentHeader`/`GetForkChoice`, so those calls are covered by the
one-slot budget too.

## Verification

- `make lint` clean, `make test-short` green (only pre-existing
unrelated failure: `cmd/rpcdaemon/graphql`)
- Full `execution/engineapi` suite green (serial exec mode), including
the engineapitester end-to-end block-production tests that exercise the
`create_block.go` change
- Coverage on the new code: 89–100% per function
- **Non-regression on the hive chain**: the full hive `rpc-compat` suite
was run against an erigon image built from this branch — **234 tests, 0
failures**, confirming no regression across the whole RPC surface,
`testing_` namespace included
- **Cross-client validation against geth**: six dedicated rpc-tests were
written for the local hive chain that exercise the `testing_` namespace
on both clients and compare erigon's responses against geth's, verifying
the two implementations behave identically

## Related

- Cross-client integration tests:
[erigontech/rpc-tests#585](erigontech/rpc-tests#585)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant