Skip to content

Rollup deposit stack setup#2576

Merged
mmagician merged 1 commit intommagician-claude/rollup-claimsfrom
mmagician-cursor-rollup-deposit-stack-setup-55fe
Mar 10, 2026
Merged

Rollup deposit stack setup#2576
mmagician merged 1 commit intommagician-claude/rollup-claimsfrom
mmagician-cursor-rollup-deposit-stack-setup-55fe

Conversation

@mmagician
Copy link
Copy Markdown
Collaborator

Remove duplicated stack setup for verify_merkle_proof to fix rollup Merkle proof verification.

@cursor
Copy link
Copy Markdown

cursor bot commented Mar 10, 2026

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on April 10.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@mmagician mmagician merged commit 8dfdc5c into mmagician-claude/rollup-claims Mar 10, 2026
14 of 15 checks passed
@mmagician mmagician deleted the mmagician-cursor-rollup-deposit-stack-setup-55fe branch March 10, 2026 17:16
mmagician added a commit that referenced this pull request Mar 17, 2026
* feat(agglayer): process CLAIM notes from a rollup

Support rollup deposits (mainnet_flag=0) in bridge-in verification.
Previously only mainnet deposits were supported and rollup deposits
would panic. Rollup deposits use two-level Merkle proof verification:
first computing the local exit root from the leaf, then verifying it
against the rollup exit root.

Closes #2394

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: extract leading zeros/flag validation from global index helpers

Address review feedback:
- Remove leading zeros assertion and mainnet flag validation from
  process_global_index_mainnet and process_global_index_rollup helpers.
  These are now done once in verify_leaf before branching.
- The helpers now take [rollup_index_le, leaf_index_le] instead of the
  full 8-element global index.
- In process_global_index_rollup, removed unnecessary byte-swap before
  asserting zero (zero is byte-order-independent). This is now moot
  since the mainnet flag is no longer checked in the helper.
- Updated MASM unit tests to match the new helper signatures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: regenerate agglayer error constants

Include the auto-generated ERR_MAINNET_FLAG_INVALID constant in the
committed errors file to fix the generated files CI check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use non-zero indices in rollup test vectors and fix stack bug

Use non-zero leafIndex (2) and indexRollup (5) in the rollup test
vectors to exercise byte-ordering and stack manipulation paths.

This exposed a bug in verify_leaf's rollup branch: the stack
rearrangement after process_global_index_rollup had leaf_index and
rollup_index in the wrong positions, which was masked when both
were zero.

Also restructure the rollup exit tree test helper to use an
SMT-style approach (setLocalExitRootAt) matching the real
PolygonRollupManager.getRollupExitRoot() construction.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: use distinct error messages for mainnet/rollup flag validation

Split ERR_BRIDGE_NOT_MAINNET into two errors:
- ERR_BRIDGE_NOT_MAINNET: "mainnet flag must be 1 for a mainnet deposit"
- ERR_BRIDGE_NOT_ROLLUP: "mainnet flag must be 0 for a rollup deposit"

The previous error text "bridge not mainnet" read backwards when used
in the rollup helper to reject a mainnet flag.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: clarify mainnet flag comment and add ERR_MAINNET_FLAG_INVALID test

- Fix stale comment in verify_leaf that incorrectly referenced
  "stack position 2" for the mainnet flag (it's at position 5)
- Add explicit MASM test for the mainnet flag boolean validation
  (ERR_MAINNET_FLAG_INVALID) which rejects flag values >= 2

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove stale constant reference and useless inline test

- Fix comment referencing non-existent MAINNET_FLAG_STACK_POS constant
- Remove test_mainnet_flag_rejects_invalid_value: it inlined the same
  logic as verify_leaf rather than testing verify_leaf itself, so it
  only proved u32lt.2 works on the value 3, not that verify_leaf
  actually performs the check

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: unify GlobalIndex validation and reject invalid flag values

Make validate_mainnet() and validate_rollup() private, exposing only
validate() which checks the mainnet flag is a valid boolean (0 or 1)
before dispatching. This fixes a bug where flag values >= 2 were
incorrectly accepted by validate_rollup() since is_mainnet() returned
false for any non-1 value.

Add mainnet_flag() accessor and a test for invalid flag rejection.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: define memory pointers relative to existing constants

Express SMT_PROOF_LOCAL_EXIT_ROOT_PTR and SMT_PROOF_ROLLUP_EXIT_ROOT_PTR
relative to PROOF_DATA_PTR and each other, rather than hard-coding
absolute values.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: regroup and document memory layout constants

Reorganize constants into clear sections: proof data memory layout
(with address map comment), leaf data, calculate_root locals, and
data sizes. Define GLOBAL_INDEX_PTR relative to
SMT_PROOF_ROLLUP_EXIT_ROOT_PTR for a fully chained layout.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Apply suggestions from code review

* Fix rollup deposit merkle proof stack setup (#2576)

Co-authored-by: Cursor Agent <cursoragent@cursor.com>

* Apply suggestions from code review

Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com>

---------

Co-authored-by: Claude (Opus) <noreply@anthropic.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants