Skip to content

fix(agglayer): fix incorrect stack comments#2681

Open
partylikeits1983 wants to merge 2 commits intoagglayerfrom
ajl-fix-stack-comments
Open

fix(agglayer): fix incorrect stack comments#2681
partylikeits1983 wants to merge 2 commits intoagglayerfrom
ajl-fix-stack-comments

Conversation

@partylikeits1983
Copy link
Copy Markdown
Contributor

This PR fixes incorrect stack comments found in crates/miden-agglayer/asm/.

Summary:

1. bridge_in.masm — Missing DEFAULT_TAG after push

Location: create_mint_note_with_attachment

After push.DEFAULT_TAG, the comment omitted the newly pushed value:

  push.DEFAULT_TAG
- # => [note_type, MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]
+ # => [tag, note_type, MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]

2. bridge_in.masm — prefix/suffix order swapped in three procedures

Location: build_mint_output_note, create_mint_note_with_attachment

Comments throughout the MINT note creation flow had faucet_id_prefix and faucet_id_suffix in the wrong order. The actual stack has suffix on top (matching key_to_faucet_id's output), but comments said prefix was on top. This was as a result of the VM stack ordering update, these were just comments that didn't get updated.

  exec.write_mint_note_storage
- # => [faucet_id_prefix, faucet_id_suffix]
+ # => [faucet_id_suffix, faucet_id_prefix]

  exec.build_mint_recipient
- # => [MINT_RECIPIENT, faucet_id_prefix, faucet_id_suffix]
+ # => [MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]

- #! Inputs:  [MINT_RECIPIENT, faucet_id_prefix, faucet_id_suffix]
+ #! Inputs:  [MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]

- # network_account_target::new expects [prefix, suffix, exec_hint]
+ # network_account_target::new expects [suffix, prefix, exec_hint]

All downstream comments within create_mint_note_with_attachment were updated to match.


3. leaf_utils.masm — Variable name typo (curr_lsbcurr_msb)

Location: pack_leaf_data

The value at that stack position is the result of u32shr.24, which extracts the upper 8 bits (MSB) of the current u32 element. The existing comment on line 97 already describes this as "extract MSB (upper 8 bits)", and line 109 references it as curr_msb. The intermediate comments on lines 103 and 107 were inconsistent, using curr_lsb instead:

- # => [next_src_addr, curr_lsb, curr_elem, counter]
+ # => [next_src_addr, curr_msb, curr_elem, counter]

- # => [next_elem, curr_lsb, curr_elem, counter]
+ # => [next_elem, curr_msb, curr_elem, counter]

4. bridge_out.masm — Padding cleanup count

Location: convert_asset

The FPI call to asset_to_origin_asset returns [U256(8), addr(5), origin_network, pad(2)] — exactly 2 padding elements. The cleanup was corrected from repeat.3 to repeat.2 and the comment updated accordingly.

@partylikeits1983 partylikeits1983 added no changelog This PR does not require an entry in the `CHANGELOG.md` file 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 24, 2026
@partylikeits1983 partylikeits1983 changed the title fix: fix incorrect stack comments fix(agglayer): fix incorrect stack comments Mar 24, 2026
@partylikeits1983 partylikeits1983 marked this pull request as ready for review March 24, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agglayer PRs or issues related to AggLayer bridging integration no changelog This PR does not require an entry in the `CHANGELOG.md` file pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants