test(spec): assert full post-state root against fixture postStateRoot

[MegaRedHand]

What

Turns the STF fixtures' postStateRoot field into an actual assertion instead of a parse-only placeholder.

Why

The STF spec-test runner captured postStateRoot only to satisfy deny_unknown_fields. The per-field post checks pin only the fields a fixture chooses to enumerate, leaving any other state field unverified. Comparing the full post-state hash_tree_root against the fixture's postStateRoot pins the entire state on every successful transition.

Changes

File	Change
state_transition/tests/stf_spectests.rs	On a successful STF, assert post_state.hash_tree_root() == postStateRoot.
state_transition/tests/types.rs	Drop #[allow(dead_code)] on post_state_root; document its new role.

Testing

• cargo test -p ethlambda-state-transition --release --test stf_spectests -> 71 passed (59 fixtures exercise the new check).
• Verified the check is live by inverting the comparison (dozens of fixtures failed with matching expected/got roots), then reverted.
• cargo fmt --all, cargo clippy -p ethlambda-rpc -p ethlambda-state-transition --all-targets clean.

Based on test/leanspec-fixture-check-hardening; targets that branch.

[github-actions]

🤖 Codex Code Review

No material findings in the diff.

The STF test change in crates/blockchain/state_transition/tests/stf_spectests.rs:54 strengthens correctness by checking the full SSZ hash_tree_root, which closes the gap left by partial per-field post assertions. The RPC change in crates/net/rpc/src/test_driver.rs:167 is also internally consistent: postStateRoot is derived from the full state root and does not affect consensus execution paths.

Residual risk: the response-schema addition at crates/net/rpc/src/test_driver.rs:167 is not locked by an e2e assertion in crates/net/rpc/tests/test_driver_e2e.rs:225. I would add a check for response["post"]["postStateRoot"] there to catch accidental renames/removals in the Hive contract.

I could not run the Rust tests in this environment because rustup attempted to write under /home/runner/.rustup, which is read-only here.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Kimi Code Review

The PR adds post-state root verification to both the spectest runner and the Hive RPC test driver. This is a good hardening measure that ensures the full SSZ state root matches expectations, catching any state field discrepancies not covered by per-field assertions.

Review:

crates/blockchain/state_transition/tests/stf_spectests.rs (lines 56-69)

• Correctness: Good. The check correctly occurs after compare_post_states, ensuring field-level debugging info is available before the aggregate root check fails.
• Nit: Consider hex-encoding the roots in the error message instead of {:?} debug formatting for better readability in test logs. Standard Ethereum tooling expects 0x-prefixed hex strings for H256 values.
• Performance: Acceptable. hash_tree_root() is expensive but this is test code, and the check only runs when post_state_root is present in the fixture.

crates/blockchain/state_transition/tests/types.rs (lines 36-40)

• Documentation: The updated comment accurately reflects the new validation logic.
• Cleanliness: Removing #[allow(dead_code)] is correct since the field is now actively used.

crates/net/rpc/src/test_driver.rs (lines 45, 172-177, 463)

• Consistency: Good. Mirroring the spectest logic in the Hive driver ensures consistent validation across local and integration tests.
• Import: The HashTreeRoot as _ import is idiomatic Rust.
• API Compatibility: StateTransitionPost derives only Serialize, so adding the new post_state_root field is backward-compatible for the RPC output format (Hive simulator consumers will receive the new field).

Consensus/Security:

• SSZ Correctness: The code assumes State::hash_tree_root() correctly implements the SSZ hash_tree_root algorithm. Ensure this uses the proper SSZ merkleization (likely via ssz_rs or similar), not just a blind serialization hash.
• Determinism: Ensure hash_tree_root() is deterministic and doesn't include transient or non-consensus data (e.g., caching fields, timestamps, or memory addresses).

Verdict: LGTM. The changes improve test robustness without affecting production consensus logic. The only suggestion is improving the error message formatting for the root mismatch case.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[greptile-apps]

Greptile Summary

This PR adds full post-state root checks and exposes the root to Hive. The main changes are:

• Compare successful STF results with fixture postStateRoot values.
• Document the fixture root as an asserted field.
• Add postStateRoot to the Hive transition response.

Confidence Score: 4/5

The fixture assertion path needs a fix before merging because missing roots still bypass the full-state check.

• Successful fixtures without postStateRoot continue to use partial field checks.
• The Hive response field and root calculation are consistent with the fixture-side naming and hashing path.

crates/blockchain/state_transition/tests/stf_spectests.rs

Important Files Changed

Filename	Overview
crates/blockchain/state_transition/tests/stf_spectests.rs	Adds the full-state comparison, but missing fixture roots still disable it.
crates/blockchain/state_transition/tests/types.rs	Updates the field documentation while retaining optional deserialization.
crates/net/rpc/src/test_driver.rs	Adds the full state root to the camelCase Hive response.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/blockchain/state_transition/tests/stf_spectests.rs:60
**Missing Root Skips Full Check**

When a successful fixture omits `postStateRoot`, this branch silently skips the full-state assertion and relies on the partial `post` fields. The current fixture corpus omits this field, so unlisted state changes can still pass; a successful fixture should instead fail when the expected root is missing.

_{Reviews (1): Last reviewed commit: "test(spec): assert full post-state root ..." | Re-trigger Greptile}

[github-actions]

🤖 Claude Code Review

Review: PR 512 — assert full post-state root against fixture postStateRoot

Small, well-scoped test-hardening change. The three files are consistent with each other and with the PR description.

What's solid:

• stf_spectests.rs:56-69 — the new check is placed correctly: only runs after compare_post_states succeeds, guarded by Option, uses the loop's name binding correctly in the error message, and doesn't short-circuit any existing checks.
• types.rs — removing #[allow(dead_code)] and updating the doc comment matches the field now actually being read; no leftover staleness.
• test_driver.rs — the new post_state_root: H256 field is additive-only on a Serialize-only response struct, so it's safe for any Hive simulator consumer that ignores unknown JSON fields, as claimed. The HashTreeRoot as _ import is clean (no duplicate/conflicting imports), and post_summary is only called from the Ok branch of run_state_transition, which is correct — there's no post-state to hash on the error path.
• No production/consensus code paths are touched — this is test-fixture and Hive-driver-only, so no fork-choice, attestation, or STF logic risk.

One point worth confirming with the author (not a blocker):

state_transition() (crates/blockchain/state_transition/src/lib.rs:74-79) already computes state.hash_tree_root() and compares it against the current block's own state_root field on every call, returning Error::StateRootMismatch on divergence. Since the test runner sets post_state = pre_state immediately after the block loop with no extra processing, a successful test already implies post_state.hash_tree_root() == blocks.last().state_root. For that to differ from the fixture's top-level postStateRoot (the new check's target), the fixture generator would need to allow those two values to diverge — which seems unlikely for well-formed positive fixtures generated by the same reference implementation.

If they can't diverge, the new assertion's practical value is defense-in-depth against a hidden implementation change (e.g., if the internal check in state_transition() were ever weakened), rather than the "previously totally unpinned" framing in the PR description — since the full-tree-root already had transitive production-code coverage, not just the enumerated post fields. Worth a quick sanity check against the fixture generator to confirm this is understood, but it doesn't make the change incorrect or harmful either way — it's a cheap, correct, explicit test-level assertion regardless.

No other correctness, security, or performance issues found. Acknowledged: cargo fmt/clippy clean per PR description, and the manual invert-then-revert verification described is a good way to confirm the check is actually live.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt