Filter User Deposits from full node block#69
Conversation
lukeiannucci
requested review from
ImJeremyHe,
Sneh1999,
jbearer,
jjeangal,
philippecamacho,
shenkeyao and
varun-doshi
as code owners
lukeiannucci
changed the title
lukeiannucci
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to strip L1-derived user deposits from full node blocks before comparing them with Espresso batches, as Espresso batches do not carry these deposits. This is implemented via a new filterUserDeposits utility function and verified with comprehensive unit tests. The review feedback highlights opportunities to improve consistency by aligning function names with their comments and test names, and suggests adding a defensive nil check to prevent potential nil pointer dereferences.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Why does filtering user's deposits addresses Piers' concerns "So this means that when a batcher starts they will send ~15 minutes worth of batches where the batcher at the origin block is not the current batcher and the streamer would drop those batches." @lukeiannucci please describe briefly the problem, and the idea of the solution in the description's PR. |
I will, sorry Piers noticed two separate issues. This is addressing this slack comment: |
| // deposit), so strip them from the full node block before comparison | ||
| comparableBlock := filterUserDeposits(fullNodeBlock) | ||
|
|
||
| if err := ensureBlocksMatch(espressoBlock, comparableBlock); err != nil { |
There was a problem hiding this comment.
Suggestion: log also the number of transactions and their types for the espressoBlock and comparableBlock in case there is some issues related to users deposits.
| // TestEnsureBlocksMatch_EpochBoundaryDeposits checks that an epoch-boundary block | ||
| // carrying an L1-derived user deposit verifies against its reconstructed batch | ||
| // once user deposits are stripped, and fails to match if they are not. | ||
| func TestEnsureBlocksMatch_EpochBoundaryDeposits(t *testing.T) { |
There was a problem hiding this comment.
Suggestion: Add a test where there are only user deposits in the block (no sequencer txs).
2 participants
Closes #<ISSUE_NUMBER>
This PR:
As pointed out by cLabs folks, batches dont include user deposit transactions. We do a byte for byte comparison between the Espresso signed batches, and the retrieved full node block. If there is a transaction deposit, espresso would not have this, causing the verification to fail. To fix this, we simply filter it out from the full node block.
This PR does not:
Key places to review: