refactor: remove redundant outputs from kernel procedures by giwaov · Pull Request #2733 · 0xMiden/protocol

giwaov · 2026-04-06T09:50:40Z

This PR addresses #2523 by removing outputs identical to inputs from several kernel procedures.

Changes

Protocol-level procedures (miden-protocol)

note::write_assets_to_memory: output changed from [num_assets, dest_ptr]\ to []\
active_note::get_assets: output changed from [num_assets, dest_ptr]\ to [num_assets]\
input_note::get_assets: output changed from [num_assets, dest_ptr, note_index]\ to [num_assets]\
output_note::get_assets: output changed from [num_assets, dest_ptr, note_index]\ to [num_assets]\
active_note::get_storage: output changed from [num_storage_items, dest_ptr]\ to [num_storage_items]\
faucet::mint: output changed from [NEW_ASSET_VALUE]\ to []\

Standards-level callers (miden-standards)

Updated all callers to account for removed return values:

faucets/mod.masm: reconstructs \ASSET_PTR\ after \get_assets; removes \dropw\ after \aucet::mint\
notes/swap.masm: reconstructs \ASSET_PTR\ after \get_assets; removes \drop\ after \get_storage\
wallets/basic.masm: reconstructs pointer from \locaddr.0\ after \get_assets\
notes/p2id.masm: removes \drop\ after \get_storage\ assertion
notes/p2ide.masm: pushes storage address explicitly after \get_storage\
notes/mint.masm: adjusts \movdn/\drop\ sequences in both public and private note branches after \get_storage\

Closes #2523

Remove outputs identical to inputs from kernel procedures: - note::write_assets_to_memory: output [] instead of [num_assets, dest_ptr] - active_note::get_assets: output [num_assets] instead of [num_assets, dest_ptr] - input_note::get_assets: output [num_assets] instead of [num_assets, dest_ptr, note_index] - output_note::get_assets: output [num_assets] instead of [num_assets, dest_ptr, note_index] - active_note::get_storage: output [num_storage_items] instead of [num_storage_items, dest_ptr] - faucet::mint: output [] instead of [NEW_ASSET_VALUE] Update all callers in miden-standards to reconstruct dropped values where needed. Closes 0xMiden#2523

Update agglayer note scripts (B2AGG, CLAIM, CONFIG_AGG_BRIDGE, UPDATE_GER) to account for removed dest_ptr from get_storage and get_assets outputs. Update kernel tests to not assert removed return values: - test_active_note: remove dest_ptr assertions for get_assets/get_storage - test_faucet: remove minted asset value assertions for mint - test_input_note/test_output_note: remove dest_ptr and note_index from expected get_assets outputs Update faucet_mint_asset API doc and mock faucet comments.

The call frame returns 16 elements. Previously the assert_eqw on the minted asset value consumed a net 4 elements. With that assertion removed, add dropw to keep the stack at 16 elements.

Copilot

Pull request overview

This PR implements the #2523 refactor across the Miden protocol/standards stack by removing kernel procedure return values that were redundant with their inputs, and updates downstream MASM callers and kernel tests to align with the new stack effects.

Changes:

Updated protocol-level MASM procedures to no longer return dest_ptr / note_index echoes (and removed the NEW_ASSET_VALUE return from faucet::mint).
Updated standards and agglayer note scripts to explicitly re-push/reconstruct pointers when needed (and removed now-unnecessary drop/dropw cleanup).
Updated kernel transaction tests and mock faucet wrappers/documentation to match the new stack outputs.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/miden-testing/src/kernel_tests/tx/test_output_note.rs	Updates asset-reading assertions to re-push `dest_ptr` since `output_note::get_assets` no longer returns it.
crates/miden-testing/src/kernel_tests/tx/test_input_note.rs	Same as above for `input_note::get_assets`; adjusts final stack cleanup.
crates/miden-testing/src/kernel_tests/tx/test_faucet.rs	Removes assertions on `faucet::mint` return value and adjusts stack handling after mint.
crates/miden-testing/src/kernel_tests/tx/test_active_note.rs	Updates tests to re-push pointers for asset/storage assertions after reduced outputs.
crates/miden-standards/src/testing/mock_account_code.rs	Updates mock faucet interface docs to reflect `mint` no longer returning `NEW_ASSET_VALUE`.
crates/miden-standards/asm/standards/wallets/basic.masm	Reconstructs asset pointer using `locaddr.0` after `active_note::get_assets` output change.
crates/miden-standards/asm/standards/notes/swap.masm	Removes reliance on returned pointers for `get_storage/get_assets`; pushes constants explicitly for loads.
crates/miden-standards/asm/standards/notes/p2id.masm	Removes now-unnecessary `drop` after `get_storage`.
crates/miden-standards/asm/standards/notes/p2ide.masm	Pushes explicit storage pointer before `mem_loadw_le` after `get_storage` change.
crates/miden-standards/asm/standards/notes/mint.masm	Adjusts stack motion/drop logic in both public/private branches after `get_storage` output change.
crates/miden-standards/asm/standards/faucets/mod.masm	Removes `dropw` after `faucet::mint` and updates asset loading to push `ASSET_PTR`.
crates/miden-protocol/asm/protocol/output_note.masm	Changes `get_assets` to return only `num_assets` and reshuffles stack accordingly.
crates/miden-protocol/asm/protocol/input_note.masm	Same as above for input notes.
crates/miden-protocol/asm/protocol/active_note.masm	Changes `get_assets/get_storage` to drop returned pointers and only return counts.
crates/miden-protocol/asm/protocol/note.masm	Makes `note::write_assets_to_memory` return nothing by dropping echoed inputs.
crates/miden-protocol/asm/protocol/faucet.masm	Makes `faucet::mint` return nothing and updates stack cleanup accordingly.
crates/miden-protocol/asm/kernels/transaction/api.masm	Updates kernel API docs to reflect `faucet_mint_asset` no longer yields a minted value.
crates/miden-agglayer/asm/note_scripts/UPDATE_GER.masm	Removes post-assert `drop` that previously removed returned `dest_ptr`.
crates/miden-agglayer/asm/note_scripts/CONFIG_AGG_BRIDGE.masm	Same as above; keeps stack consistent with new `get_storage` output.
crates/miden-agglayer/asm/note_scripts/CLAIM.masm	Adjusts drop count after `get_storage` since only one value is returned.
crates/miden-agglayer/asm/note_scripts/B2AGG.masm	Pushes explicit pointers before storage/asset loads following output reductions.
CHANGELOG.md	Adds a breaking-change entry documenting the procedure signature/output changes.

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

PhilippGackstatter

Thank you! I left a few small comments. Could you also merge latest next into this branch and resolve conflicts?

PhilippGackstatter · 2026-04-28T12:15:39Z

    # write the inputs to the memory using the provided destination pointer
    exec.write_storage_to_memory
    # => [num_storage_items, dest_ptr]
+
+    # drop dest_ptr as it is identical to the input
+    swap drop
+    # => [num_storage_items]


We can remove num_storage_items return value from write_storage_to_memory, then remove the swap drop here.

PhilippGackstatter · 2026-04-28T12:17:06Z

+    movup.6 drop
+    # => [ASSETS_COMMITMENT, num_assets, dest_ptr, num_assets]


We can consume the note_index in the call to get_assets_info, so we can change the dup.1 to a swap and then we don't need to drop it here.

PhilippGackstatter · 2026-04-28T12:18:31Z

+
+    # drop num_assets and dest_ptr as these are identical to the inputs
+    drop drop
+    # OS => []


Instead of dropping them here, we should consume them further above. I think the dup.5 dup.5 can be replaced with movup.x's.

PhilippGackstatter · 2026-04-28T12:18:52Z

+    # save num_assets for the return value and drop note_index
+    dup.4 movdn.7
+    # => [ASSETS_COMMITMENT, num_assets, dest_ptr, note_index, num_assets]
+
+    movup.6 drop
+    # => [ASSETS_COMMITMENT, num_assets, dest_ptr, num_assets]


Same here re consuming rather than dropping.

# Conflicts: # CHANGELOG.md # Cargo.lock # Cargo.toml # crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm # crates/miden-protocol/asm/protocol/note.masm # crates/miden-standards/asm/standards/notes/mint.masm

giwaov · 2026-04-28T19:43:18Z

Thanks for the review! I addressed the comments and merged the latest �next� into the branch.

PhilippGackstatter

I pushed a small doc fix, otherwise looks good to me! Thanks!

giwaov added 3 commits April 6, 2026 10:48

fix: drop extra word after mock_faucet::mint in tests

fe541cd

The call frame returns 16 elements. Previously the assert_eqw on the minted asset value consumed a net 4 elements. With that assertion removed, add dropw to keep the stack at 16 elements.

mmagician requested a review from Copilot April 7, 2026 12:30

Copilot started reviewing on behalf of mmagician April 7, 2026 12:31 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

PhilippGackstatter reviewed Apr 28, 2026

View reviewed changes

giwaov added 2 commits April 28, 2026 20:06

refactor: address kernel output review comments

d88c8db

Merge remote-tracking branch 'origin/next' into pr-2733-review-fixes

177ed82

# Conflicts: # CHANGELOG.md # Cargo.lock # Cargo.toml # crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm # crates/miden-protocol/asm/protocol/note.masm # crates/miden-standards/asm/standards/notes/mint.masm

giwaov and others added 3 commits April 28, 2026 21:19

fix: update callers for reduced note outputs

9e440bd

Merge branch 'next' into refactor/remove-redundant-kernel-outputs

d13a6ed

fix: output stack on write_storage_to_memory

67956f6

PhilippGackstatter approved these changes Apr 29, 2026

View reviewed changes

PhilippGackstatter enabled auto-merge April 29, 2026 06:57

PhilippGackstatter added this pull request to the merge queue Apr 29, 2026

Merged via the queue into 0xMiden:next with commit 5b613f4 Apr 29, 2026
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: remove redundant outputs from kernel procedures#2733

refactor: remove redundant outputs from kernel procedures#2733
PhilippGackstatter merged 8 commits into0xMiden:nextfrom
giwaov:refactor/remove-redundant-kernel-outputs

giwaov commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

PhilippGackstatter left a comment

Uh oh!

PhilippGackstatter Apr 28, 2026

Uh oh!

PhilippGackstatter Apr 28, 2026

Uh oh!

PhilippGackstatter Apr 28, 2026

Uh oh!

PhilippGackstatter Apr 28, 2026

Uh oh!

Uh oh!

Uh oh!

giwaov commented Apr 28, 2026

Uh oh!

PhilippGackstatter left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		movup.6 drop
		# => [ASSETS_COMMITMENT, num_assets, dest_ptr, num_assets]

Conversation

giwaov commented Apr 6, 2026

Changes

Protocol-level procedures (miden-protocol)

Standards-level callers (miden-standards)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

giwaov commented Apr 28, 2026

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants