chore: remove hash ignoring

[tac0turtle]

Description

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Performance improvement
• Refactoring

Related Issues

Fixes #(issue)

Checklist

• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Testing

Additional Notes

[gemini-code-assist]

Summary of Changes

Hello @tac0turtle, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request implements a crucial upgrade to how block hashes are handled, enabling a transition to canonical Keccak block hashes for newer networks. By introducing a configurable activation height, the system can gracefully move away from legacy application-level hashes that caused issues with upstream tooling. This change ensures more consistent and standard block hash validation post-activation, improving compatibility and reliability, while preserving functionality for existing chains.

Highlights

• Canonical Block Hash Activation: Introduced a new hashRewireActivationHeight configuration in the chainspec, allowing networks to opt-in to using canonical Keccak block hashes.
• Conditional Block Hash Generation: The payload builder now dynamically determines whether to use the canonical Keccak hash or a legacy application-level hash for sealed blocks, based on the hashRewireActivationHeight.
• Strict Hash Validation Enforcement: The engine validator has been updated to enforce strict validation of canonical block hashes after the activation height, rejecting payloads with mismatches, while maintaining backward compatibility for pre-activation blocks.
• Comprehensive Documentation: New documentation (docs/canonical-hash-plan.md) and updates to README.md provide detailed explanations of the feature, its configuration, and the transition plan.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[claude]

Claude finished @tac0turtle's task —— View job

---

Code Review Complete

I've thoroughly reviewed this PR that introduces configurable canonical block hash enforcement. Here's my comprehensive feedback:

Tasks:

• Read and analyze changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage
• Provide structured feedback

---

Overall Assessment

This PR implements a well-designed solution to address block hash compatibility issues between ev-node and Ethereum tooling. The code quality is high with good documentation, comprehensive tests, and a clear migration strategy.

Code Quality & Best Practices ✅

Strengths:

• Clear naming: The rename from hashRewireActivationHeight to canonicalHashActivationHeight is much more descriptive
• Comprehensive documentation: Excellent inline documentation explaining the background and migration strategy
• Consistent patterns: Follows established patterns in the codebase for configuration and validation
• Good separation of concerns: Logic properly separated between config, builder, and validator

Minor suggestion:
The gemini-code-assist review correctly points out the builder logic in crates/node/src/builder.rs:195 could be simplified. However, I notice the latest commit actually removed this logic entirely, which is cleaner.

Security Concerns ✅

Well-handled:

• Controlled activation: The activation height mechanism prevents unexpected behavior changes
• Default to secure: New networks default to canonical hash enforcement (None → enforce from genesis)
• Backward compatibility: Legacy networks can continue operating while planning migration
• Clear validation: The validator properly enforces or bypasses validation based on configuration

No security issues identified.

Performance Considerations ✅

Efficient implementation:

• O(1) hash enforcement check: is_canonical_hash_enforced() is a simple comparison
• No extra allocations: The validation logic doesn't introduce unnecessary object creation
• Removed overhead: The latest commit actually removed the hash rewriting logic from the builder, simplifying the code path

Potential Issues - 1 Minor Concern ⚠️

Configuration mismatch risk (crates/node/src/config.rs:198-222):
The method is_canonical_hash_enforced() has inverted logic compared to the old is_hash_rewire_active_for_block():

• Old logic: Some(activation) if block_number >= activation → bypass when active
• New logic: Some(activation) => block_number >= activation → enforce when active

While the logic is correct in the current implementation, this could be confusing during migration. Consider adding a comment to clarify the inversion.

Test Coverage ✅

Excellent test coverage:

• Configuration parsing: Tests cover chainspec deserialization with various combinations
• Activation logic: Tests verify behavior before/at/after activation height
• Edge cases: Tests cover None (default enforcement) and explicit activation heights
• Validator behavior: New tests in validator.rs cover both legacy and canonical modes
• Integration: Tests verify the end-to-end behavior with mismatched block hashes

Particularly well-tested scenarios:

• crates/node/src/config.rs:560-582: Tests canonical hash activation from chainspec
• crates/node/src/validator.rs:213-236: Tests both legacy bypass and canonical rejection modes

Documentation ✅

Outstanding documentation:

• Background explanation: Clear explanation of the historical problem
• Migration strategy: Well-documented approach for existing networks
• Configuration examples: README includes practical examples
• Code comments: Inline documentation explains the rationale

The documentation in docs/canonical-hash-plan.md provides excellent context for the feature.

Suggestions for Improvement

1. Method naming clarity (crates/node/src/config.rs:217):
   Consider renaming is_canonical_hash_enforced() to should_enforce_canonical_hash() for clarity.

2. Error messaging (crates/node/src/validator.rs:97-100):
   The warning message could include the activation height for better debugging:

   tracing::warn!(
       block_number,
       activation_height = ?self.config.canonical_hash_activation_height,
       "canonical hash enforcement active; rejecting mismatched block hash"
   );

Final Verdict: APPROVED ✅

This is a well-implemented feature that:

• Solves a real compatibility problem with Ethereum tooling
• Provides a clear migration path for existing networks
• Includes comprehensive tests and documentation
• Follows good security practices with safe defaults
• Has minimal performance impact

The code quality is high and the approach is sound. The recent commits have actually simplified the implementation by removing the hash rewriting logic from the payload builder, which makes the code cleaner and easier to understand.

---

[gemini-code-assist]

Code Review

This pull request introduces a configurable mechanism to switch between legacy and canonical block hashes, which is a significant improvement for compatibility with standard Ethereum tooling. The changes are well-structured across configuration, the payload builder, and the validator, and are supported by good documentation and tests. I have one suggestion to refactor the payload sealing logic in crates/node/src/builder.rs for better efficiency and clarity.

[gemini-code-assist]

The current implementation for sealing the block can be made more efficient and readable. It involves cloning the SealedBlock and then potentially cloning the inner Block again, which is unnecessary.

A better approach is to decide which hash to use (legacy or canonical) and then seal the UnsealedBlock just once using the appropriate hash. This avoids intermediate clones and makes the logic more direct.

Suggested change

	let mut sealed_block = block.sealed_block().clone();
	if !self.config.is_hash_rewire_active_for_block(block_number) {
	let legacy_hash = sealed_block.header().state_root;
	let legacy_block = sealed_block.clone_block();
	sealed_block = SealedBlock::new_unchecked(legacy_block, legacy_hash);
	}
	let sealed_block = if !self.config.is_hash_rewire_active_for_block(block_number) {
	// Use the legacy hash (state root).
	let legacy_hash = block.header.state_root;
	block.seal(legacy_hash)
	} else {
	// Use the canonical keccak hash of the header.
	block.sealed_block()
	};