fix(standards): rework SWAP to support public payback notes#2949
Open
JereSalo wants to merge 14 commits into
Open
fix(standards): rework SWAP to support public payback notes#2949JereSalo wants to merge 14 commits into
JereSalo wants to merge 14 commits into
Conversation
Public payback notes were unrecoverable from on-chain data: the previous SWAP precomputed the payback recipient off-line and embedded only the resulting hash, so the consuming script had no preimage to register with the advice provider. Build the payback P2ID recipient at consume time from data available in SWAP storage so the on-chain script can call p2id::new (which also registers the recipient preimage in the advice map): - Embed the creator account ID in storage (hybrid embed, mirroring PSWAP) so the consumer reads it directly instead of going through active_note::get_sender. - Derive the payback serial as swap_serial[0] + 1. - Derive the payback tag from the creator account ID prefix via note_tag::create_account_target. Storage shrinks from 14 to 11 items: the precomputed recipient and tag are no longer stored. The Rust constructor sets creator_id = sender by convention.
JereSalo
added
no changelog
# Conflicts: # crates/miden-standards/src/note/swap.rs
Branch SWAP MASM on payback note type so private paybacks store an opaque precomputed recipient digest (and tag) instead of the creator id. Public paybacks keep the creator id in plaintext since the consumer needs it to reconstruct the recipient via p2id::new. The unified 16-felt storage asserts zero on the slots unused by each branch, making the privacy guarantee structural rather than convention-based.
Centralize the rationale for the SwapPayback::Private vs Public storage shape on the enum itself and drop the repeated explanations from SwapNote::create, SwapNoteStorage, and the constructors. Also remove a stale comment from swap.masm.
partylikeits1983
left a comment
partylikeits1983
left a comment
Contributor
There was a problem hiding this comment.
Looks good! Thank you!
The only comment I have is removing the unnecessary masm stack comment
partylikeits1983
approved these changes
Jun 10, 2026
Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com>
mmagician
reviewed
Jun 11, 2026
mmagician
left a comment
mmagician
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, though I'm not sure about the phrasing around the convention of who the payback target is. AFAIU it doesn't need to be the creator
| # store note storage into memory starting at address 0 | ||
| push.0 exec.active_note::get_storage | ||
| # store note storage into memory starting at STORAGE_PTR | ||
| push.STORAGE_PTR exec.active_note::get_storage |
Comment on lines
+203
to
+204
| /// Account ID of the payback receiver. Today this is the SWAP creator by convention, but | ||
| /// the script does not enforce it: it can be any account in a future iteration. |
Collaborator
There was a problem hiding this comment.
by which convention? the API explicitly allows setting an arbitrary AccountId:
pub fn new_public(
requested_asset: Asset,
payback_target_id: AccountId, # <------ not necessarily sender
payback_tag: NoteTag,
) -> Self {
Comment on lines
+316
to
+320
| /// Deserializes [`SwapNoteStorage`] from a slice of exactly 16 [`Felt`]s. | ||
| impl TryFrom<&[Felt]> for SwapNoteStorage { | ||
| type Error = NoteError; | ||
|
|
||
| fn try_from(note_storage: &[Felt]) -> Result<Self, Self::Error> { |
Collaborator
There was a problem hiding this comment.
is this ever used outside of tests?
| # | ||
| # The payback target id could be derived from `active_note::get_sender` since today the target | ||
| # equals the SWAP sender, but it's stored explicitly because this slot is meant to represent an | ||
| # arbitrary payback target (not necessarily the creator) in a future iteration. |
Collaborator
There was a problem hiding this comment.
same as below, I think this is already supported in this iteration unless I'm missing sth
bobbinth
reviewed
Jun 13, 2026
Comment on lines
+174
to
+179
| /// | `[0..7]` | Requested asset (key + value) | | ||
| /// | `[8..11]` | Payback recipient digest (private mode; zero in public mode) | | ||
| /// | `[12]` | Payback note type | | ||
| /// | `[13]` | Payback note tag | | ||
| /// | `[14]` | Payback target account ID prefix (public mode; zero in private mode) | | ||
| /// | `[15]` | Payback target account ID suffix (public mode; zero in private mode) | |
Contributor
There was a problem hiding this comment.
Question: instead of putting account ID into elements 14 and 15, would it make sense to put them into elements 8 and 9? This would reduce the "waste" a bit. Or does this create too many complications?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Public payback wasn't possible on SWAP notes: the on-chain script lacked the data needed to construct the payback recipient. This PR embeds the creator account ID in SWAP storage so the consumer can derive the payback P2ID recipient at consume time.
Related Issues:
0xMiden/miden-client#1739 - after this PR we'll be able to tackle this on the client side.
#2950 - Partially solves this issue just for SWAP and for the
target_account_idscenario, which we don't send if the payback is private to preserve privacy of the target account (that in the future may be different from the creator account, that's why it makes sense not to reveal that info). We don't change theserial_numlogic here nor we solve it for thePSWAP.