feat: implement build_note_tag_for_local_account in masm
#2235
Conversation
partylikeits1983
added
contracts
partylikeits1983
requested review from
PhilippGackstatter,
Copilot and
mmagician
There was a problem hiding this comment.
Pull request overview
This PR implements a build_note_tag_for_local_account procedure in MASM to compute a NoteTag for local accounts (private and public) based on their AccountId. This functionality is needed for the agglayer bridge in asset flow and other contexts.
Key Changes
- Added
build_note_tag_for_local_accountMASM procedure that constructs a LocalAny note tag with the top 14 bits from the account ID prefix - Added test case to verify the MASM implementation matches the Rust
NoteTag::from_account_id()behavior for local accounts - Updated CHANGELOG to document the new procedure
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
crates/miden-protocol/asm/protocol/note.masm |
Implements the new build_note_tag_for_local_account procedure with bit manipulation to extract and format account ID bits into a note tag |
crates/miden-testing/src/kernel_tests/tx/test_note.rs |
Adds test that verifies the MASM procedure produces the same tag as the Rust implementation for a private account |
CHANGELOG.md |
Documents the addition of the new procedure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #! The tag is constructed as follows: | ||
| #! - The two most significant bits are set to `0b11` to indicate a LOCAL_ANY tag. | ||
| #! - The next 14 bits are set to the most significant bits of the account ID prefix. | ||
| #! - The remaining bits are set to zero. |
There was a problem hiding this comment.
Makes sense! What is the timeline to merge #2219?
Once 2219 is merged I will update this PR
There was a problem hiding this comment.
| #! Outputs: [local_account_tag] | ||
| #! | ||
| #! Invocation: exec | ||
| pub proc build_note_tag_for_local_account |
There was a problem hiding this comment.
As for the structure/naming, maybe it makes sense to introduce a note_tag.masm module and name the procedure new_account_target so we can use this as note_tag::new_account_target, and for completeness it seems it would make sense to have a new_custom_account_target as well (I think with_ would also work for consistency).
|
Converting to draft until #2219 is merged to |
This PR adds a procedure to be able to compute a
NoteTagfor based on a local accountAccountId. This is needed for for the agglayer bridge in asset flow, but is also useful in other contexts.