feat(deposits): use Berachain deposit event for EIP-6110#226
feat(deposits): use Berachain deposit event for EIP-6110#226
Conversation
Replace the standard EIP-6110 DepositEvent signature with Berachain's custom Deposit event: Deposit(bytes,bytes,uint64,bytes,uint64) → 0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46. The Berachain event differs in two ways: the event name is Deposit (not DepositEvent) and amount/index are uint64 rather than bytes, requiring little-endian conversion when constructing deposit request bytestrings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds EIP-6110 deposit parsing: a new Changes
Sequence DiagramsequenceDiagram
participant Executor as Executor
participant Receipts as "Tx Receipts & Logs"
participant Parser as "Deposit Parser"
participant Buffer as "Deposit Byte Buffer"
participant Requests as "EIP-6110 Requests"
Executor->>Receipts: collect receipts & logs
Executor->>Parser: parse_deposits_from_receipts(deposit_contract, receipts)
Parser->>Receipts: iterate logs
Parser->>Parser: filter by address & signature hash
alt decode success
Parser->>Buffer: encode (pubkey, credentials, amount, signature, index)
Buffer->>Requests: append deposit bytes
Requests-->>Executor: return concatenated Bytes
else decode failure
Parser-->>Executor: return BlockValidationError
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 docstrings
🧪 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 |
| // This value is copied from Reth mainnet. Berachain's deposit contract topic is | ||
| // different but also unused. | ||
| topic: b256!("0x649bbc62d0e31342afea4e5cd82d4049e7e1ee912fc0889aa790803be39038c5"), | ||
| topic: b256!("0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46"), |
There was a problem hiding this comment.
| topic: b256!("0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46"), | |
| // This value is unique to Berachain's deposit event in its deposit contract. | |
| // This is different from Eth mainnet's deposit event signature. | |
| topic: b256!("0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46"), |
![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: c694d33a59
ℹ️ 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".
| out.extend_from_slice(log.pubkey.as_ref()); | ||
| out.extend_from_slice(log.credentials.as_ref()); | ||
| out.extend_from_slice(&log.amount.to_le_bytes()); | ||
| out.extend_from_slice(log.signature.as_ref()); | ||
| out.extend_from_slice(&log.index.to_le_bytes()); |
There was a problem hiding this comment.
Validate pubkey/credentials/signature lengths before extending out: EIP-6110 deposit requests must be fixed-width (192 bytes each), but this code appends whatever lengths are decoded from the log, so a non-canonical Deposit event from the configured deposit contract would produce a malformed request payload that is not a multiple of 192 bytes. In that scenario the execution payload can be rejected by consensus clients, causing block production/validation failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates Berachain’s EIP-6110 deposit-request construction to parse Berachain’s custom Deposit(bytes,bytes,uint64,bytes,uint64) event (including little-endian encoding for amount/index) instead of the standard DepositEvent signature.
Changes:
- Add a new
src/deposits.rsparser for Berachain’s customDepositevent and wire it into block execution request building. - Update the chain spec deposit contract topic to Berachain’s event signature hash.
- Add E2E coverage ensuring deposit requests appear only when a deposit transaction emits the expected log.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/mod.rs | Registers the new deposit_test E2E module. |
| tests/e2e/deposit_test.rs | Adds E2E tests + a mock log-emitter contract to validate deposit requests. |
| src/node/evm/executor.rs | Switches deposit parsing during Prague to the Berachain-specific parser. |
| src/lib.rs | Exposes the new deposits module. |
| src/deposits.rs | Implements Berachain custom deposit log decoding and request-bytes accumulation. |
| src/chainspec/mod.rs | Updates the configured deposit contract topic hash to Berachain’s Deposit signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| out.extend_from_slice(log.pubkey.as_ref()); | ||
| out.extend_from_slice(log.credentials.as_ref()); | ||
| out.extend_from_slice(&log.amount.to_le_bytes()); | ||
| out.extend_from_slice(log.signature.as_ref()); | ||
| out.extend_from_slice(&log.index.to_le_bytes()); |
There was a problem hiding this comment.
accumulate_deposit_from_log appends the raw bytes fields without validating their expected fixed sizes (pubkey=48, credentials=32, signature=96). Because the event uses dynamic bytes, a malformed log could produce a deposit request that is not a multiple of 192 bytes (or cause large allocations), which would make the EIP-6110 request encoding invalid. Consider checking the decoded lengths and returning a BlockValidationError::DepositRequestDecode (or a more specific error) when they don’t match the required sizes before extending out.
![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/chainspec/mod.rs (1)
675-683:⚠️ Potential issue | 🟡 MinorExtract the deposit topic literal to a named constant and remove stale wording.
Line 683 hardcodes a magic hash, and the nearby note still states “same deposit topic as mainnet,” which is now inaccurate for this PR’s Berachain-specific signature.
♻️ Proposed cleanup
+const BERACHAIN_DEPOSIT_EVENT_TOPIC: B256 = + b256!("0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46"); + - // NOTE: in full node, we prune all receipts except the deposit contract's. We do not - // have the deployment block in the genesis file, so we use block zero. We use the same - // deposit topic as the mainnet contract if we have the deposit contract address in the - // genesis json. let deposit_contract = genesis.config.deposit_contract_address.map(|address| DepositContract { address, block: 0, - topic: b256!("0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46"), + topic: BERACHAIN_DEPOSIT_EVENT_TOPIC, });As per coding guidelines, Extract magic numbers into documented constants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/chainspec/mod.rs` around lines 675 - 683, Extract the hardcoded deposit topic hash into a named, documented constant (e.g., DEPOSIT_CONTRACT_TOPIC) and replace the inline b256!("0x68af75...") usage inside the DepositContract construction (where deposit_contract is created from genesis.config.deposit_contract_address) with that constant; also update the surrounding comment/note in mod.rs to remove the phrase "same deposit topic as mainnet" (or reword to reflect the Berachain-specific signature) so the note matches the new constant and avoids stale mainnet wording.
🧹 Nitpick comments (1)
src/deposits.rs (1)
22-22: Add documentation to the constant explaining its composition.As per coding guidelines, documentation should focus on 'why' rather than 'what'. Consider adding a brief doc comment explaining the field breakdown.
📝 Suggested documentation
+/// Deposit request byte layout: pubkey (48) + credentials (32) + amount (8) + signature (96) + index (8) const DEPOSIT_BYTES_SIZE: usize = 48 + 32 + 8 + 96 + 8;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/deposits.rs` at line 22, The constant DEPOSIT_BYTES_SIZE lacks a doc comment explaining why it equals that sum; add a brief doc comment above DEPOSIT_BYTES_SIZE describing the purpose of the constant and the field breakdown (e.g., which fields contribute 48, 32, 8, 96, and 8 bytes) and why those sizes are used so future readers understand the rationale; reference the constant name DEPOSIT_BYTES_SIZE when adding the comment.
🤖 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/chainspec/mod.rs`:
- Around line 675-683: Extract the hardcoded deposit topic hash into a named,
documented constant (e.g., DEPOSIT_CONTRACT_TOPIC) and replace the inline
b256!("0x68af75...") usage inside the DepositContract construction (where
deposit_contract is created from genesis.config.deposit_contract_address) with
that constant; also update the surrounding comment/note in mod.rs to remove the
phrase "same deposit topic as mainnet" (or reword to reflect the
Berachain-specific signature) so the note matches the new constant and avoids
stale mainnet wording.
---
Nitpick comments:
In `@src/deposits.rs`:
- Line 22: The constant DEPOSIT_BYTES_SIZE lacks a doc comment explaining why it
equals that sum; add a brief doc comment above DEPOSIT_BYTES_SIZE describing the
purpose of the constant and the field breakdown (e.g., which fields contribute
48, 32, 8, 96, and 8 bytes) and why those sizes are used so future readers
understand the rationale; reference the constant name DEPOSIT_BYTES_SIZE when
adding the comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4972ffcd-b57a-4dfe-bd66-0f21b70de224
📒 Files selected for processing (15)
src/chainspec/mod.rssrc/consensus/mod.rssrc/deposits.rssrc/engine/validator.rssrc/evm/mod.rssrc/genesis/mod.rssrc/hardforks/mod.rssrc/lib.rssrc/node/evm/executor.rssrc/pool/mod.rssrc/rpc/api.rssrc/rpc/receipt.rssrc/transaction/mod.rstests/e2e/deposit_test.rstests/e2e/mod.rs
calbera
force-pushed
the
claude/update-deposit-event-signature-CnrWq
branch
from
6fd7bf4 to
a447495
Compare
Replace the standard EIP-6110 DepositEvent signature with Berachain's custom Deposit event: Deposit(bytes,bytes,uint64,bytes,uint64) → 0x68af751683498a9f9be59fe8b0d52a64dd155255d85cdb29fea30b1e3f891d46.
The Berachain event differs in two ways: the event name is Deposit (not DepositEvent) and amount/index are uint64 rather than bytes, requiring little-endian conversion when constructing deposit request bytestrings.
Enabled in the CL here
Summary by CodeRabbit
New Features
Tests