chore(node): Untrusted RPC Validation

[refcell]

Description

Add untrusted rpc validation.

By default, this leaves rpc endpoints as trusted to avoid the performance hit.

Down the road, these flags can be set to false by default which will force the providers to validate rpc responses.

Closes #2318

[refcell]

📚 $\text{Stack Overview}$

Pulls submitted in this stack:

• chore(node): Untrusted RPC Validation #2644 👈
• main

This comment was automatically generated by st.

[claude]

Claude finished @refcell's task —— View job

---

PR Review: Untrusted RPC Validation

Reviewing this pull request for code quality, potential issues, performance considerations, security concerns, and test coverage.

Todo List:

• Analyze CLI flag implementation in node binary
• Review service actor changes for validation integration
• Examine provider validation logic implementation
• Check documentation updates for completeness and accuracy
• Assess overall architecture and design decisions
• Identify potential bugs or security issues
• Evaluate performance implications
• Check test coverage
• Provide comprehensive feedback

---

Code Quality & Architecture

✅ Well-structured implementation: The feature is cleanly integrated across multiple layers - CLI, service layer, and provider implementations.

✅ Consistent naming and patterns: The l1_trust_rpc and l2_trust_rpc flags follow existing conventions and are consistently named throughout the codebase.

✅ Proper separation of concerns: Validation logic is contained within the provider layer where it belongs.

Implementation Analysis

CLI Integration (bin/node/src/commands/node.rs:85-108)

• Good: CLI flags are properly documented with help text and environment variable support
• Good: Default values are set to true (trusted) for backward compatibility and performance
• Good: Visible aliases provide user-friendly alternatives

Service Builder (crates/node/service/src/service/standard/builder.rs:68-84)

• Good: Builder pattern properly passes trust settings through to providers
• Good: Trust flags are stored and used consistently in the node structure

Provider Validation Logic

L1 Chain Provider (crates/providers/providers-alloy/src/chain_provider.rs:82-105):

• Good: Hash verification is performed using header.hash_slow() vs expected hash
• Good: Verification is skipped when trust_rpc is true for performance
• Good: Clear error messages with expected vs actual hash values

L2 Chain Provider (crates/providers/providers-alloy/src/l2_chain_provider.rs:102-116):

• Good: Similar verification pattern for L2 blocks
• Good: Consistent error handling and messaging

Security Considerations

✅ Defense in depth: When trust is disabled, the implementation performs hash verification on retrieved blocks, protecting against:

• Data corruption during transmission
• Potentially malicious RPC endpoints
• Man-in-the-middle attacks

✅ Safe defaults: Keeping trust enabled by default maintains backward compatibility while allowing users to opt into stricter validation.

⚠️ Limited validation scope: The current implementation only validates block hashes. Consider if additional validations would be beneficial:

• Transaction list integrity
• Receipt root validation
• State root validation (though this might be too expensive)

Performance Implications

✅ Performance-conscious design:

• Validation is opt-in via flags set to false
• Default trusted mode avoids computational overhead
• Hash computation only occurs when verification is needed

✅ Efficient implementation: Uses hash_slow() which is the appropriate method for verification purposes.

Potential Issues

❌ Missing test coverage: There are no unit tests covering the new validation logic. This is a significant gap.

⚠️ Error handling in L2 provider: The L2 provider uses RpcError::local_usage_str() for hash mismatches (l2_chain_provider.rs:107-110), while L1 provider uses a more structured error approach. Consider consistency.

⚠️ Cache implications: When validation fails, the invalid data might still be cached in some scenarios. The current implementation appears to handle this correctly by performing validation before caching, but this should be verified with tests.

Documentation

✅ Comprehensive documentation: The documentation updates in README.md and docs/ are thorough and provide clear examples.

✅ Security guidance: Good explanation of when to use trusted vs untrusted modes.

Recommendations

Critical - Add Test Coverage

The most important missing piece is test coverage. Recommend adding:

#[cfg(test)]
mod tests {
    // Test hash verification with trust_rpc = false
    // Test bypassing verification with trust_rpc = true  
    // Test error handling for hash mismatches
    // Test caching behavior with validation
}

Minor Improvements

1. Consistent error handling: Consider using the same error type pattern between L1 and L2 providers for hash mismatches.

2. Metrics: Consider adding metrics to track validation failures/successes when trust is disabled.

3. Documentation: Consider adding a brief comment in the provider code explaining the security model.

Overall Assessment

This is a solid, well-architected feature that addresses an important security concern. The implementation follows Rust and codebase conventions, provides appropriate defaults, and includes comprehensive documentation. The main concern is the lack of test coverage, which should be addressed before merging.

Recommendation: Approve after adding unit tests for the validation logic.

---

[Copilot]

Pull Request Overview

This PR adds untrusted RPC validation capabilities to the kona-node, allowing users to enable block hash verification when using public or potentially malicious RPC providers. By default, RPCs remain trusted to preserve performance, but can be configured to perform additional verification.

• Added --l1-trust-rpc and --l2-trust-rpc configuration flags with defaults to true
• Implemented block hash verification in chain providers when trust is disabled
• Updated documentation to explain RPC trust configuration and security recommendations

Reviewed Changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
docs/docs/pages/node/run/docker.mdx	Added RPC trust configuration section for Docker setup
docs/docs/pages/node/run/binary.mdx	Added RPC trust configuration examples for binary usage
docs/docs/pages/node/configuration.mdx	Added comprehensive RPC trust configuration documentation
crates/providers/providers-alloy/src/l2_chain_provider.rs	Added trust_rpc field and hash verification logic for L2 blocks
crates/providers/providers-alloy/src/chain_provider.rs	Added trust_rpc field and hash verification helper for L1 blocks
crates/node/service/src/service/standard/node.rs	Added trust_rpc fields to RollupNode structure
crates/node/service/src/service/standard/builder.rs	Added trust_rpc configuration methods to builder
crates/node/service/src/actors/sequencer/actor.rs	Updated to pass trust_rpc settings to chain providers
crates/node/service/src/actors/derivation.rs	Updated to pass trust_rpc settings to chain providers

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.7%. Comparing base (5d9cf71) to head (b6da68a).
⚠️ Report is 43 commits behind head on main.
✅ All tests successful. No failed tests found.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude finished @refcell's task —— View job

---

PR Review: Untrusted RPC Validation

Comprehensive review of this pull request focusing on code quality, potential issues, performance, security, and test coverage.

Todo List:

• Analyze CLI flag implementation in node binary
• Review service actor changes for validation integration
• Examine provider validation logic implementation
• Check documentation updates for completeness and accuracy
• Assess overall architecture and design decisions
• Identify potential bugs or security issues
• Evaluate performance implications
• Check test coverage
• Provide comprehensive feedback

---

Code Quality & Architecture

✅ Excellent architectural design: The feature is cleanly integrated across multiple layers:

• CLI flags in node.rs:85-108 with proper environment variable support and defaults
• Service layer properly passes trust settings through builders (builder.rs:68-84)
• Provider layer contains the validation logic where it belongs
• Documentation is comprehensive and well-structured

✅ Consistent implementation patterns:

• Both L1 and L2 providers follow the same trust/verification pattern
• Naming conventions are consistent throughout (l1_trust_rpc, l2_trust_rpc)
• Error handling follows established patterns in the codebase

✅ Proper separation of concerns: Validation logic is isolated in provider methods, making it easy to test and maintain.

Implementation Analysis

CLI Integration (bin/node/src/commands/node.rs:85-108)

• Excellent: CLI flags are well-documented with clear help text and environment variable support
• Good: Default values set to true (trusted) for backward compatibility and performance
• Good: Visible aliases provide user-friendly alternatives (l1.trust-rpc, l2.trust-rpc)

Provider Validation Logic

L1 Chain Provider (chain_provider.rs:82-105):

• Good: Uses header.hash_slow() for verification - appropriate for validation purposes
• Good: Verification is properly skipped when trust_rpc is true
• Good: Clear error messages with expected vs actual hash comparison
• Good: Verification occurs before caching to prevent invalid data storage

L2 Chain Provider (l2_chain_provider.rs:82-99):

• Good: Similar verification pattern for consistency
• Good: Proper integration with existing block fetching logic
• Minor: Uses RpcError::local_usage_str() vs L1's structured error approach

Service Integration

• Excellent: Trust settings flow cleanly from CLI through builder to providers
• Good: Both derivation and sequencer actors receive trust settings properly
• Good: No breaking changes to existing APIs

Security Analysis

✅ Strong security benefits: When trust is disabled, the implementation provides:

• Protection against malicious RPC endpoints returning incorrect block data
• Defense against man-in-the-middle attacks
• Validation of block hash integrity

✅ Safe defaults: Trusted mode by default maintains performance while allowing users to opt into stricter validation when needed.

⚠️ Limited validation scope: Currently only validates block hashes. Consider if additional validations would be beneficial (though this is likely sufficient for the threat model).

✅ No information disclosure: Error messages appropriately show expected vs actual hashes without revealing sensitive information.

Performance Considerations

✅ Performance-conscious design:

• Validation is completely optional via flags set to false
• Default trusted mode avoids any computational overhead
• Hash computation using hash_slow() only occurs when needed

✅ Minimal overhead: The verification adds minimal latency - just a hash computation and comparison.

Documentation Quality

✅ Comprehensive documentation:

• CLI reference properly documents both flags with clear descriptions
• Usage examples show both trusted and untrusted scenarios
• Security recommendations are practical and actionable
• Docker and binary usage guides include RPC trust configuration

✅ Clear security guidance: Documentation explains when to use trusted vs untrusted modes with specific examples.

Issues Identified

❌ Missing test coverage: This is the most significant issue. There are no unit tests covering:

• Hash verification logic in both providers
• Error handling for hash mismatches
• Behavior with trust_rpc = true vs false
• Edge cases (invalid hashes, network errors during verification)

⚠️ Minor inconsistency: L1 and L2 providers use slightly different error handling approaches for hash mismatches:

• L1: AlloyChainProviderError::Transport(RpcError::Transport(TransportErrorKind::Custom(...)))
• L2: RpcError::local_usage_str(...)

Consider standardizing on one approach for consistency.

⚠️ Performance note: The Copilot reviewer correctly noted that hash_slow() could be expensive. However, this is by design since:

• It only runs when trust_rpc = false
• The hash needs to be computed for verification anyway
• The alternative (header.hash) might not be available in all contexts

Copilot Reviewer Feedback Assessment

The Copilot reviewer provided two valid suggestions:

1. Performance concern about hash_slow(): Valid but acceptable given the security-first approach when trust is disabled
2. Code duplication suggestion: Good idea to extract verification logic into a shared helper method

Overall Assessment

This is a high-quality, well-architected security feature that addresses an important need for RPC validation. The implementation:

• Follows excellent software engineering practices
• Provides comprehensive documentation
• Uses appropriate defaults for compatibility and performance
• Implements defense-in-depth security measures
• Integrates cleanly with existing architecture

Primary recommendation: Add unit tests for the validation logic before merging.

Secondary recommendations:

1. Consider standardizing error handling between L1 and L2 providers
2. Consider extracting hash verification into a shared helper method to reduce duplication
3. Consider adding metrics to track validation failures when trust is disabled

Final verdict: Approve with minor changes - primarily adding test coverage for the new validation functionality.

---

Test Coverage Gap

The absence of unit tests is concerning for a security-critical feature. Recommended test coverage:

#[cfg(test)]
mod tests {
    // Test hash verification with trust_rpc = false
    // Test bypassing verification with trust_rpc = true  
    // Test error handling for hash mismatches
    // Test integration with caching behavior
}

Despite this gap, the feature is well-implemented and addresses a legitimate security need. The Codecov report shows overall good coverage, but specific tests for this new validation logic would strengthen confidence in the implementation.

---