fix: Add RequestHash header field for SDK compatibility #1133
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes add a new optional field Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Block as Block Object
participant Header as Header Data
Client->>Block: BeaconRoot()
Block->>Header: Retrieve beacon root
Header-->>Block: Beacon value
Block-->>Client: Return beacon value
Client->>Block: RequestsHash()
Block->>Header: Retrieve requests hash
Header-->>Block: Return requests hash
Block-->>Client: Return requests hash
Client->>Block: ExcessBlobGas()
Block->>Header: Retrieve excess blob gas value
Header-->>Block: Return value or nil
Block-->>Client: Return value
Client->>Block: BlobGasUsed()
Block->>Header: Retrieve blob gas used value
Header-->>Block: Return value or nil
Block-->>Client: Return value
sequenceDiagram
participant Caller as Caller
participant CopyFunc as CopyHeader()
participant Header as Header Data
Caller->>CopyFunc: Call CopyHeader(oldHeader)
CopyFunc->>Header: Check if RequestsHash is not nil
alt RequestsHash exists
Header-->>CopyFunc: Provide deep copy of RequestsHash
else No RequestsHash
Header-->>CopyFunc: No action needed
end
CopyFunc-->>Caller: Return copied header
Suggested labels
Suggested reviewers
Poem
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:
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 (
|
![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)
core/types/block.go (1)
403-405: Consider implementing deep copy for consistency with other accessorsThe
BeaconRoot()andRequestsHash()methods return direct pointers to the header fields, which is inconsistent with other similar accessor methods likeExcessBlobGas()andBlobGasUsed()that create deep copies.For consistency with other pointer-returning accessor methods, consider implementing deep copies:
-func (b *Block) BeaconRoot() *common.Hash { return b.header.ParentBeaconRoot } -func (b *Block) RequestsHash() *common.Hash { return b.header.RequestsHash } +func (b *Block) BeaconRoot() *common.Hash { + var beaconRoot *common.Hash + if b.header.ParentBeaconRoot != nil { + beaconRoot = new(common.Hash) + *beaconRoot = *b.header.ParentBeaconRoot + } + return beaconRoot +} + +func (b *Block) RequestsHash() *common.Hash { + var requestsHash *common.Hash + if b.header.RequestsHash != nil { + requestsHash = new(common.Hash) + *requestsHash = *b.header.RequestsHash + } + return requestsHash +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/types/block.go(3 hunks)params/version.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- params/version.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (6)
core/types/block.go (6)
118-119: Field addition aligns with EIP-7685 for SDK compatibilityThe new
RequestsHashfield is correctly added to theHeaderstruct with appropriate JSON and RLP tags. This field supports EIP-7685 compatibility as mentioned in the PR objectives.
336-339: Deep copy implementation is correctThe implementation properly handles the deep copy of the
RequestsHashfield in theCopyHeaderfunction, ensuring that modifications to the copied header won't affect the original.
406-413: ExcessBlobGas() implementation is correctThe method properly handles the nil case and creates a deep copy of the value when returning it, protecting the internal state of the header from external modifications.
415-422: BlobGasUsed() implementation is correctThe implementation follows the same pattern as
ExcessBlobGas(), correctly handling the nil case and creating a deep copy of the value when returning it.
118-119:✅ Verification successful
Verify patch version update in params/version.go
According to the AI summary, this PR also updates the patch version in
params/version.go. Let's verify this update has been properly implemented.
🏁 Script executed:
#!/bin/bash # Check if the patch version has been updated # Look for the version update in params/version.go echo "Checking for patch version update..." cat params/version.go | grep -A 2 VersionPatchLength of output: 369
Patch Version Update Verified in
params/version.go
The patch version is correctly updated to 21 inparams/version.goas expected. The file correctly reflects the change viaVersionPatch = 21along with the associated metadata, and no further modifications are required.
320-339:❓ Verification inconclusive
Verify that all code using these new fields is updated
The changes look good, but it's important to verify that any code that processes block headers is updated to handle the new
RequestsHashfield, particularly in functions that serialize or deserialize block headers.
🏁 Script executed:
#!/bin/bash # Check for other places in the codebase that might need updates for the RequestsHash field # Look for places where block headers are encoded/decoded echo "Checking RLP encoding/decoding implementations..." rg -A 3 -B 3 "func.*Encode.*Header" --type go # Check for places that might need to handle RequestsHash echo "Checking for functions that might need to handle the new RequestsHash field..." rg -A 3 -B 3 "func.*Header" --type go | grep -i "hash\|encode\|decode\|marshal\|unmarshal" # Look for JSON handling of headers that might need to be updated echo "Checking JSON marshaling of headers..." rg -A 3 -B 3 "json.*Header" --type goLength of output: 74601
Review: Verify Serialization & Deserialization Updates for RequestsHash
The new block header copy logic in core/types/block.go (lines 320–339) correctly copies the RequestsHash field. However, our initial searches for header encoding/decoding did not show clear evidence that the RequestsHash field is handled—especially in the JSON serialization (see core/types/gen_header_json.go) or in RLP encoding functions. Please make sure that:
- The JSON marshaling/unmarshaling routines (e.g. in core/types/gen_header_json.go) are updated to include RequestsHash if needed.
- Any RLP encode/decode functions or other serializer/deserializer routines used for block headers account for the new field.
- Downstream components that expect complete header data (such as consensus logic, API endpoints, and tests) correctly process the RequestsHash field.
Please review these areas manually to ensure complete integration of the new RequestsHash field.
1. Purpose or design rationale of this PR
...
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Chores