fix(engine): use correct V4P11 response envelope and Prague3 payload handling#239
fix(engine): use correct V4P11 response envelope and Prague3 payload handling#239
Conversation
Wire getPayloadV4P11 response through BerachainExecutionPayloadEnvelopeV4 so parentProposerPubKey is included in the envelope. The EngineTypes type alias was pointing at the upstream ExecutionPayloadEnvelopeV4 which lacks the field. Replace the Prague3 builder hard-stop with an empty block path so the builder gracefully produces a valid block instead of erroring. Add an upfront Prague3 rejection in forkchoiceUpdatedV3P11 so FCU never returns a payloadId the builder cannot serve. https://claude.ai/code/session_011iX4hTaTWntopZYE2ZpfMe
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 53 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe changes add Prague3 fork-era validation and payload-handling logic across the engine layer. The payload builder now skips transaction processing when Prague3 is active, the V4 execution envelope type is updated to use a Berachain-specific version, and RPC fork-choice validation rejects Prague3 timestamps. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
calbera
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 471de6497f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Updates Berachain’s Engine API v4 envelope typing so getPayloadV4P11 responses include the Berachain-specific parentProposerPubKey, and adjusts Prague3 behavior to avoid builder hard failures by producing empty blocks while preventing FCU from returning unservable payload IDs during Prague3.
Changes:
- Route EngineTypes’ v4 envelope type to
BerachainExecutionPayloadEnvelopeV4so the extra Berachain field is serialized in v4 payload envelopes. - Extend v4 payload conversion to populate
parent_proposer_pub_keyfrom the built block header. - Replace the Prague3 builder hard-stop with an “empty block” path and add a Prague3 rejection in
forkchoiceUpdatedV3P11.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/engine/rpc.rs |
Reject forkchoiceUpdatedV3P11 when Berachain Prague3 is active to avoid returning incompatible payload IDs. |
src/engine/payload.rs |
Convert built payloads into Berachain’s v4 envelope type and populate parent_proposer_pub_key. |
src/engine/mod.rs |
Switch EngineTypes::ExecutionPayloadEnvelopeV4 to BerachainExecutionPayloadEnvelopeV4. |
src/engine/builder.rs |
Build empty blocks during Prague3 instead of erroring; refactor transaction inclusion under a !prague3_active guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/engine/mod.rs (1)
111-123:⚠️ Potential issue | 🟠 MajorPreserve the standard
engine_getPayloadV4wire shape.
EngineTypes::ExecutionPayloadEnvelopeV4is used by bothengine_getPayloadV4andengine_getPayloadV4P11, so this remap makes the standard V4 method serializeBerachainExecutionPayloadEnvelopeV4too. As written, pre-Prague1 responses will includeparentProposerPubKey: null, which adds a non-spec field toengine_getPayloadV4. Make the Berachain extension field conditional so only P11/Prague1 responses carry it.Suggested fix
pub struct BerachainExecutionPayloadEnvelopeV4 { /// Inner [`ExecutionPayloadEnvelopeV3`]. #[deref] #[deref_mut] #[serde(flatten)] pub envelope_inner: ExecutionPayloadEnvelopeV3, @@ /// A list of opaque [EIP-7685][eip7685] requests. /// /// [eip7685]: https://eips.ethereum.org/EIPS/eip-7685 pub execution_requests: Requests, /// Introduced in BRIP-0004 + #[serde(default, skip_serializing_if = "Option::is_none")] pub parent_proposer_pub_key: Option<BlsPublicKey>, }Also applies to: 126-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/mod.rs` around lines 111 - 123, The Berachain-specific field parent_proposer_pub_key on BerachainExecutionPayloadEnvelopeV4 is being serialized as null for standard engine_getPayloadV4 responses; update the struct so the field is omitted when empty by adding serde's conditional attr (e.g., #[serde(skip_serializing_if = "Option::is_none")]) to parent_proposer_pub_key on BerachainExecutionPayloadEnvelopeV4 (and the other similar occurrence around lines 126-130), so EngineTypes::ExecutionPayloadEnvelopeV4 used by engine_getPayloadV4 does not include the non-spec field unless present for engine_getPayloadV4P11/Prague1.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/engine/mod.rs`:
- Around line 111-123: The Berachain-specific field parent_proposer_pub_key on
BerachainExecutionPayloadEnvelopeV4 is being serialized as null for standard
engine_getPayloadV4 responses; update the struct so the field is omitted when
empty by adding serde's conditional attr (e.g., #[serde(skip_serializing_if =
"Option::is_none")]) to parent_proposer_pub_key on
BerachainExecutionPayloadEnvelopeV4 (and the other similar occurrence around
lines 126-130), so EngineTypes::ExecutionPayloadEnvelopeV4 used by
engine_getPayloadV4 does not include the non-spec field unless present for
engine_getPayloadV4P11/Prague1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba3223ad-ec7d-4cd2-a9f9-2bcca4d8fa53
📒 Files selected for processing (4)
src/engine/builder.rssrc/engine/mod.rssrc/engine/payload.rssrc/engine/rpc.rs
Exercise the builder path where Prague3 is active and transaction selection is skipped. The test launches a node with a Prague3-at-genesis chainspec, advances one block, and asserts the resulting block contains zero transactions. https://claude.ai/code/session_011iX4hTaTWntopZYE2ZpfMe
Construct a BerachainBuiltPayload with a known prev_proposer_pubkey in the block header, convert via try_into_v4, and verify the resulting envelope carries the correct parent_proposer_pub_key and execution_requests. A second case covers the None-pubkey / no-requests default path. https://claude.ai/code/session_011iX4hTaTWntopZYE2ZpfMe
calbera
changed the title
Prague1+ blocks always contain the PoL system transaction injected by apply_pre_execution_changes. The Prague3 builder skip only bypasses pool transaction selection, so the correct assertion is exactly one transaction (the PoL tx), not zero. https://claude.ai/code/session_011iX4hTaTWntopZYE2ZpfMe
V3P11 is the only FCU path available after Prague1 activation. Rejecting Prague3 timestamps here would prevent the CL from obtaining a payloadId during that window, making the empty-block builder path unreachable. The builder already handles Prague3 gracefully by skipping pool transactions, so FCU should accept the request. https://claude.ai/code/session_011iX4hTaTWntopZYE2ZpfMe
Wire getPayloadV4P11 response through BerachainExecutionPayloadEnvelopeV4
so parentProposerPubKey is included in the envelope. The EngineTypes
type alias was pointing at the upstream ExecutionPayloadEnvelopeV4 which
lacks the field.
Replace the Prague3 builder hard-stop with an empty block path so the
builder gracefully produces a valid block instead of erroring. Add an
upfront Prague3 rejection in forkchoiceUpdatedV3P11 so FCU never returns
a payloadId the builder cannot serve.
Summary by CodeRabbit
Release Notes
New Features
Refactor