refactor(block): save header to context #2346
Conversation
WalkthroughThe changes update block management and header verification mechanisms. The block manager now ignores the Changes
Sequence Diagram(s)sequenceDiagram
participant Manager
participant Context
participant Executor
participant SignedHeader
Manager->>Context: Add block header with HeaderContextKey
Manager->>Executor: Execute transactions with augmented context
Executor->>Context: Access block header via HeaderContextKey
Executor->>SignedHeader: Validate header (may use custom verifier)
SignedHeader-->>Executor: Return validation result
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
types/signed_header.go (1)
126-140: Robust implementation with proper error handling.The conditional logic correctly uses the custom verifier when available and wraps any errors appropriately. The fallback to default marshaling ensures backward compatibility.
Consider adding documentation about the security implications of custom verifiers to help users implement them safely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
block/manager.go(2 hunks)block/publish_block2_test.go(2 hunks)types/signed_header.go(4 hunks)types/state.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: rollkit-docker / docker-build (GHCR; ghcr.io/rollkit/rollkit)
- GitHub Check: test / Run E2E System Tests
- GitHub Check: test / Run EVM Execution Tests
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (9)
types/state.go (1)
15-15: LGTM: Version update for CometBFT compatibility.The block version bump to 11 is well-documented and aligns with CometBFT compatibility requirements for IBC.
block/publish_block2_test.go (2)
73-74: Good cleanup: Leveraging Go's type inference.Removing the explicit generic type parameters improves code readability since the types can be inferred from the function arguments.
107-108: Good cleanup: Leveraging Go's type inference.Removing the explicit generic type parameters improves code readability since the types can be inferred from the function arguments.
block/manager.go (3)
821-825: Excellent context key implementation.The header context key follows Go best practices with an unexported key type and exported variable. This allows safe storage and retrieval of headers in the execution context.
833-833: Good enhancement: Header available during transaction execution.Storing the header in context before executing transactions allows the execution client to access header information when needed, which aligns well with the PR's objective.
753-753:Details
✅ Verification successful
Verify that ignoring the lastState parameter is safe.
The function signature now ignores the
lastStateparameter and usesm.lastStateinstead. While this may be an intentional design improvement, ensure that all callers expect this behavior and that there are no scenarios where the passed state differs from the manager's internal state.
🏁 Script executed:
#!/bin/bash # Search for all calls to execCreateBlock to verify they don't rely on different lastState values ast-grep --pattern 'execCreateBlock($$$)'Length of output: 43
To ensure we didn’t miss any usages, let’s search across all Go files for calls to
execCreateBlock:
🏁 Script executed:
#!/bin/bash # Grep for all occurrences of execCreateBlock in Go files rg --color=never --no-heading --with-filename --line-number 'execCreateBlock\(' --include '*.go'Length of output: 540
To cover all bases, let’s search for any calls to
execCreateBlockacross the entire Go codebase, falling back to grep if ripgrep finds nothing:
🏁 Script executed:
#!/bin/bash # Try ripgrep first if ! rg -n 'execCreateBlock\('; then # Fallback to grep for .go files grep -R -n 'execCreateBlock(' --include='*.go' . fiLength of output: 371
execCreateBlock parameter usage verified
We’ve confirmed that
execCreateBlockis only ever called once (in block/manager.go:699), and it always receivesm.lastState. Changing thelastStateparameter to a blank identifier has no impact on behavior and is safe to keep. No further action needed.types/signed_header.go (3)
19-19: Good practice: Interface compliance check at top.Moving the interface compliance assertion to the top of the file improves visibility and follows Go conventions.
21-23: Well-designed signature verification abstraction.The
SignatureVerifierfunction type provides a clean abstraction for custom signature verification logic while maintaining the same interface contract.
47-54: Good safeguard against verifier overwrites.The check prevents accidental overwriting of an existing custom verifier, which could lead to unexpected behavior or security issues.
|
checking the failing tests, as no new calls have been added here. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2346 +/- ##
==========================================
- Coverage 65.91% 65.84% -0.07%
==========================================
Files 66 66
Lines 6140 6155 +15
==========================================
+ Hits 4047 4053 +6
- Misses 1724 1733 +9
Partials 369 369 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Superseed #2301 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for custom signature verification logic on block headers, allowing advanced users to specify how header signatures are verified. - Enhanced context handling to provide block header information during transaction execution. - **Bug Fixes** - Improved context handling to make block header information available during transaction execution. - **Chores** - Updated the initial block version to 11 for improved compatibility with external blockchains. - Minor test and code cleanups with no impact on functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Superseed #2301 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for custom signature verification logic on block headers, allowing advanced users to specify how header signatures are verified. - Enhanced context handling to provide block header information during transaction execution. - **Bug Fixes** - Improved context handling to make block header information available during transaction execution. - **Chores** - Updated the initial block version to 11 for improved compatibility with external blockchains. - Minor test and code cleanups with no impact on functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Superseed #2301
Summary by CodeRabbit
New Features
Bug Fixes
Chores