chore: opinionated improvements to #2528 by mmagician · Pull Request #2602 · 0xMiden/protocol

mmagician · 2026-03-14T10:07:09Z

Addresses own PR review comments:

refactor: reorient CLAIM note consumption flow #2528 (comment) (no need to return y from verify_u128_to_native_amount_conversion)
avoid store/load of faucet ID to memory, and pass on stack instead (related to refactor: reorient CLAIM note consumption flow #2528 (comment))

Copilot

Pull request overview

This PR updates AggLayer asset conversion verification to be “assert-only” (no stack return value), then propagates the new stack behavior through bridge claim logic and the associated tests.

Changes:

Change verify_u128_to_native_amount_conversion / verify_u256_to_native_amount_conversion MASM procedures to return no outputs (validation via successful execution).
Update bridge claim flow to keep faucet ID on the operand stack instead of storing/loading it from global memory during claim verification and note construction.
Adjust Rust tests to stop asserting on returned stack values for scale-down verification.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
crates/miden-testing/tests/agglayer/asset_conversion.rs	Updates tests to treat successful execution of the verifier as sufficient validation.
crates/miden-agglayer/asm/agglayer/common/asset_conversion.masm	Updates verifier procedures to produce no stack outputs and adjusts internal stack choreography accordingly.
crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm	Refactors claim flow to thread faucet ID via stack and updates verifier usage to match the new “no output” convention.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm

Copilot

Pull request overview

This PR updates Agglayer asset conversion verification procedures to consume their inputs without leaving values on the stack, and adjusts bridge-in flow and tests accordingly.

Changes:

Update verify_u128_to_native_amount_conversion / verify_u256_to_native_amount_conversion to return no outputs and adjust internal stack choreography.
Refactor bridge_in::claim flow to avoid storing faucet account ID in global memory, keeping it on the operand stack across calls instead.
Update Rust tests to treat successful execution of the verifier as validation (since the verifier no longer returns y).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
crates/miden-testing/tests/agglayer/asset_conversion.rs	Adjusts scale-down tests to align with verifier procedures no longer returning `y`.
crates/miden-agglayer/asm/agglayer/common/asset_conversion.masm	Changes verifier procedures to `Outputs: []` and updates stack manipulation accordingly.
crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm	Removes faucet-id global-memory storage and threads faucet ID through claim logic via the stack.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/miden-testing/tests/agglayer/asset_conversion.rs

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

partylikeits1983

Looks good, thank you for making these changes. I checked out the branch and made minor changes to co-pilot's suggestions.

* feat: wip CLAIM note flow reorientation * wip: build MINT note from aggbridge * wip: created MINT note encodes incorrect public P2ID or public P2ID not in DataStore * feat: working e2e CLAIM flow reordering * fix: cleanup rm debug.stack * refactor: improve readability of MINT note memory addresses * refactor: fix stack comments * fix: rm println statements * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/note_scripts/CLAIM.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm Co-authored-by: Andrey Khmuro <[email protected]> * refactor: improve bridge_in doc comments * refactor: add constants for memory addresses * refactor: cleanup memory layout in bridge_in * fix: update comment in CLAIM note * refactor: remove redundant dropw * refactor: use execution_hint::ALWAYS * refactor: rm redundant const note type * refactor: simplify loc_store/load ops in bridge_in * refactor: decompose build_mint_output_note into modular helpers * refactor: remove pad(x) stack comments from exec'ed procs * feat: add constants for attachement types * fix: add call to verify_u256_scale_down procedure * refactor: use ATTACHMENT_KIND_NONE const * fix: use truncate_stack proc for FPI call & exec compatibility * refactor: add get_scale & get_scale_inner for call vs exec * refactor: use local memory for TOKEN_ADDR_HASH_PTR * refactor: look up faucet by token address only once during claim * fix: fix array comment syntax * refactor: rm pad comments from exec'ed proc * refactor: rm swapw & replace w padw * fix: update comment * refactor: refactor is_token_registered proc * chore: more explicit mem constant names * fix: explicit pad before FPI call * refactor: remove unused bridge account ID storage slot from AggLayer faucet * chore: opinionated improvements to #2528 (#2602) * chore: pass faucet ID on stack to verify_claim_amount * chore: pass faucet ID on stack to build_mint_output_note * chore: completely remove CLAIM_FAUCET_ID_PREFIX_MEM_ADDR * chore: drop "y" in verify_u128_to_native_amount_conversion * chore: avoid uplicating y only to drop it later * chore: adjust comments; no longer reading faucetID from mem * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * fix: rename to output & fix copilot suggestion --------- Co-authored-by: Alexander John Lee <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: riemann <[email protected]> * refactor: move bridge & faucet code from lib.rs to dedicated modules --------- Co-authored-by: Andrey Khmuro <[email protected]> Co-authored-by: Marti <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]>

* chore: pass faucet ID on stack to verify_claim_amount * chore: pass faucet ID on stack to build_mint_output_note * chore: completely remove CLAIM_FAUCET_ID_PREFIX_MEM_ADDR * chore: drop "y" in verify_u128_to_native_amount_conversion * chore: avoid uplicating y only to drop it later * chore: adjust comments; no longer reading faucetID from mem * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * fix: rename to output & fix copilot suggestion --------- Co-authored-by: Alexander John Lee <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: riemann <[email protected]>

mmagician added 5 commits March 14, 2026 09:57

chore: pass faucet ID on stack to verify_claim_amount

a2d9165

chore: pass faucet ID on stack to build_mint_output_note

f568a3e

chore: completely remove CLAIM_FAUCET_ID_PREFIX_MEM_ADDR

850fe33

chore: drop "y" in verify_u128_to_native_amount_conversion

527445b

chore: avoid uplicating y only to drop it later

f9bda8c

mmagician added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Mar 14, 2026

mmagician mentioned this pull request Mar 14, 2026

refactor: reorient CLAIM note consumption flow #2528

Merged

mmagician added agglayer PRs or issues related to AggLayer bridging integration pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority labels Mar 14, 2026

mmagician requested review from Copilot and partylikeits1983 March 14, 2026 10:23

Copilot started reviewing on behalf of mmagician March 14, 2026 10:23 View session

Copilot AI reviewed Mar 14, 2026

View reviewed changes

chore: adjust comments; no longer reading faucetID from mem

3f7b671

mmagician requested a review from Copilot March 15, 2026 09:11

Copilot started reviewing on behalf of mmagician March 15, 2026 09:11 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

crates/miden-testing/tests/agglayer/asset_conversion.rs Outdated Show resolved Hide resolved

crates/miden-testing/tests/agglayer/asset_conversion.rs Show resolved Hide resolved

partylikeits1983 and others added 3 commits March 16, 2026 13:41

Potential fix for pull request finding

f683209

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

Potential fix for pull request finding

0231b40

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

fix: rename to output & fix copilot suggestion

70296e1

partylikeits1983 approved these changes Mar 16, 2026

View reviewed changes

partylikeits1983 merged commit dc72124 into ajl-reorient-claim-note-flow Mar 16, 2026
14 of 15 checks passed

partylikeits1983 deleted the mmagician-reorient-claim-faucet-on-stack branch March 16, 2026 11:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: opinionated improvements to #2528#2602

chore: opinionated improvements to #2528#2602
partylikeits1983 merged 9 commits intoajl-reorient-claim-note-flowfrom
mmagician-reorient-claim-faucet-on-stack

mmagician commented Mar 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

partylikeits1983 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mmagician commented Mar 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

partylikeits1983 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants