rpc: implement testing_commitBlockV1

[lupin012]

Closes #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:

• 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): 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

[Copilot]

Pull request overview

This PR adds testing_commitBlockV1 to Erigon’s testing_ RPC namespace as the “write” companion to testing_buildBlockV1, and refactors shared block assembly so both endpoints use the same builder path (including proper builder-level extraData application).

Changes:

• Implement testing_commitBlockV1 (assemble → insert → validate → forkchoice update) and preserve safe/finalized hashes.
• Refactor BuildBlockV1 to reuse a shared assembleTestingBlock helper and forward extraData into the block builder instead of patching responses.
• Extend the block builder to accept per-build ExtraData, add comprehensive unit tests, and document the new RPC method.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
execution/engineapi/testing_api.go	Adds CommitBlockV1, introduces shared assembly helper, and forwards extraData into the builder parameters.
execution/engineapi/testing_api_test.go	Extends the execution-module stub and updates BuildBlockV1 tests for builder-level extraData forwarding.
execution/engineapi/testing_api_commit_test.go	New unit tests covering CommitBlockV1 success paths, validation/errors, busy handling, and immutability on failure.
execution/builder/parameters.go	Adds ExtraData to builder parameters to support per-call overrides.
execution/builder/create_block.go	Applies ExtraData override to the block header when provided via builder parameters.
cmd/rpcdaemon/README.md	Documents testing_buildBlockV1 and testing_commitBlockV1 as implemented (testing-only).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yperbasis]

LGTM — verified locally (build, vet, new tests incl. -race on the PR head) and cross-checked against the spec proposal, geth's implementation, and the chainreader/execmodule semantics the new code relies on.

Minor suggestions, none blocking:

• The CommitBlockV1 docstring's "On any failure the canonical head is left unchanged" has one exception: module FCU runs on a background context, so if the slot budget expires mid fork choice, the call returns the busy error but the head can still advance asynchronously afterwards. Worth qualifying in the docstring (or the busy error message).
• InsertBlock is the only step guarded by bare lock.Lock()/Unlock() without defer — a panic would leave the engine lock held. Suggest the closure+defer idiom used elsewhere in the file.
• Optional: a len(*extraData) > 32 pre-check returning -32602 would fail fast instead of building+inserting a block that ValidateChain then rejects with -32000 (and would stop buildBlockV1 from returning an uncommittable payload). Geth has the same gap, so parity holds either way.
• Test nit: the err.(rpc.Error).ErrorCode() assertions in the invalid-fork-choice and reorg-too-deep subtests panic rather than fail if the error type is ever wrong; require.ErrorAs is cleaner.
• Pre-existing, possible follow-up: with extraData null, erigon falls back to BuilderConfig.ExtraData (default erigon-<version>, so block hashes vary by erigon version), while geth's testing path forces empty extra. Spec fixtures always pass "0x" explicitly, but consider matching geth for determinism.

On the Copilot comments: the decodeTxnProvider "testing_buildBlockV1" error-prefix fix and the stale stub comment are worth taking; the deadline point is negligible in practice (CurrentHeader/GetForkChoice are fast read-only lookups, no semaphore) — moving the slotDeadline call to the top of CommitBlockV1 satisfies it for free if you care.

[lupin012]

@yperbasis

1. updated docString
2. Add defer
3. Added check on extraData size not greter 32
4. -38002/-38006 tests — replaced the type assertion (which panicked on mismatch) with require.ErrorAs
5. extradata null -> Aligned to geth