From b8d78bf67b373ff470d93e8e852bcdfde0d34f00 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Thu, 28 May 2026 17:16:07 +0530 Subject: [PATCH 01/14] refactor(pswap): switch public APIs to AssetAmount and NonZeroU32 - PswapNoteStorage::requested_asset_amount() returns AssetAmount. - PswapNote::create_args takes AssetAmount on both args and is infallible. - PswapNoteAttachment::new takes depth: NonZeroU32 (new is infallible). - parent_depth returns u32; next_depth() yields NonZeroU32 with overflow check. - payback_note/remainder_note drop runtime depth==0 checks (now type-enforced). - Split fill_sum u64-overflow test to hand-built note_args (create_args now rejects > AssetAmount::MAX client-side; MASM check still exercised). Closes group A of issue #3000. --- crates/miden-standards/src/note/pswap.rs | 263 +++++++++++++++----- crates/miden-testing/tests/scripts/pswap.rs | 136 ++++++++-- 2 files changed, 313 insertions(+), 86 deletions(-) diff --git a/crates/miden-standards/src/note/pswap.rs b/crates/miden-standards/src/note/pswap.rs index 14ea70ea7b..d3ee6d4aeb 100644 --- a/crates/miden-standards/src/note/pswap.rs +++ b/crates/miden-standards/src/note/pswap.rs @@ -1,4 +1,5 @@ use alloc::vec; +use core::num::NonZeroU32; use miden_protocol::account::AccountId; use miden_protocol::assembly::Path; @@ -116,8 +117,8 @@ impl PswapNoteStorage { } /// Returns the requested token amount. - pub fn requested_asset_amount(&self) -> u64 { - self.requested_asset.amount().as_u64() + pub fn requested_asset_amount(&self) -> AssetAmount { + self.requested_asset.amount() } } @@ -193,16 +194,20 @@ impl TryFrom<&[Felt]> for PswapNoteStorage { /// Typed attachment carried by both PSWAP output notes, encoded as /// `[amount, order_id, depth, 0]` under [`PswapNote::PSWAP_ATTACHMENT_SCHEME`]. +/// +/// `depth` is [`NonZeroU32`] because attachments are only stamped on payback / remainder notes +/// (depth >= 1); the original PSWAP has no PSWAP-scheme attachment. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct PswapNoteAttachment { amount: AssetAmount, order_id: Felt, - depth: u32, + depth: NonZeroU32, } impl PswapNoteAttachment { - /// Creates a new [`PswapNoteAttachment`]. - pub fn new(amount: AssetAmount, order_id: Felt, depth: u32) -> Self { + /// Creates a new [`PswapNoteAttachment`]. Infallible: depth is non-zero by type, and + /// [`AssetAmount`] is pre-validated. + pub fn new(amount: AssetAmount, order_id: Felt, depth: NonZeroU32) -> Self { Self { amount, order_id, depth } } @@ -214,7 +219,7 @@ impl PswapNoteAttachment { self.order_id } - pub fn depth(&self) -> u32 { + pub fn depth(&self) -> NonZeroU32 { self.depth } } @@ -224,7 +229,7 @@ impl From for NoteAttachment { let word = Word::from([ Felt::from(attachment.amount), attachment.order_id, - Felt::from(attachment.depth), + Felt::from(attachment.depth.get()), ZERO, ]); NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word) @@ -311,34 +316,21 @@ impl PswapNote { PSWAP_SCRIPT.root() } - /// Builds the `NOTE_ARGS` word that the PSWAP script expects when a - /// consumer wants to fill part of the swap: - /// - /// `[account_fill, note_fill, 0, 0]` + /// Builds the `NOTE_ARGS` word that the PSWAP script expects when a consumer wants to fill + /// part of the swap: `[account_fill, note_fill, 0, 0]`. /// /// - `account_fill` is the portion of the requested asset the consumer pays out of their own /// vault. - /// - `note_fill` is the portion sourced from another note in the same transaction (cross-swap / - /// net-zero flow). - /// - /// Both values are in the requested asset's base units. In a network - /// transaction the kernel defaults `NOTE_ARGS` to `[0, 0, 0, 0]` and the - /// script falls back to a full fill, so this helper is only needed for - /// local transactions where the consumer is choosing the fill split. + /// - `note_fill` is the portion sourced from another note in the same transaction (cross-swap + /// / net-zero flow). /// - /// # Errors + /// Both values are in the requested asset's base units. In a network transaction the kernel + /// defaults `NOTE_ARGS` to `[0, 0, 0, 0]` and the script falls back to a full fill, so this + /// helper is only needed for local transactions where the consumer chooses the fill split. /// - /// Returns an error if either value exceeds the Goldilocks field size - /// (i.e. cannot be represented as a [`Felt`]). In practice this cannot - /// happen for any amount that fits in a [`FungibleAsset`] — - /// `FungibleAsset::MAX_AMOUNT` is comfortably below `2^63` — but the - /// conversion is surfaced explicitly rather than hidden behind a panic. - pub fn create_args(account_fill: u64, note_fill: u64) -> Result { - let account_fill = Felt::try_from(account_fill) - .map_err(|e| NoteError::other_with_source("account_fill is not a valid felt", e))?; - let note_fill = Felt::try_from(note_fill) - .map_err(|e| NoteError::other_with_source("note_fill is not a valid felt", e))?; - Ok(Word::from([account_fill, note_fill, ZERO, ZERO])) + /// Infallible: [`AssetAmount`] is bounded by `2^63 - 2^31`, which fits in a [`Felt`]. + pub fn create_args(account_fill: AssetAmount, note_fill: AssetAmount) -> Word { + Word::from([Felt::from(account_fill), Felt::from(note_fill), ZERO, ZERO]) } /// Returns the account ID of the note sender. @@ -387,17 +379,38 @@ impl PswapNote { /// remainder produced by an earlier fill). /// /// The next round's `current_depth` is computed as `parent_depth() + 1`, matching the - /// on-chain `get_current_depth` MASM procedure. - pub fn parent_depth(&self) -> u64 { + /// on-chain `get_current_depth` MASM procedure. Use [`Self::next_depth`] for the typed + /// [`NonZeroU32`] form. + pub fn parent_depth(&self) -> u32 { match self.attachment.as_ref() { Some(att) if att.attachment_scheme() == Self::PSWAP_ATTACHMENT_SCHEME => { - let attachment_word = att.content().as_words()[0]; - attachment_word[Self::PARENT_ATTACHMENT_DEPTH_OFFSET].as_canonical_u64() + // The depth value is written by this crate (Group B's TryFrom enforces u32 + // range on read); treat out-of-range as a corrupted attachment and fall back + // to 0 so discovery on a malformed note degrades to "looks like an original" + // rather than corrupting downstream arithmetic. + u32::try_from( + att.content().as_words()[0][Self::PARENT_ATTACHMENT_DEPTH_OFFSET] + .as_canonical_u64(), + ) + .unwrap_or(0) }, _ => 0, } } + /// Returns the depth that the next-round payback / remainder should carry, equal to + /// `parent_depth() + 1`. Always non-zero by construction. + /// + /// # Errors + /// + /// Returns an error if `parent_depth()` is [`u32::MAX`]. + pub fn next_depth(&self) -> Result { + self.parent_depth() + .checked_add(1) + .and_then(NonZeroU32::new) + .ok_or_else(|| NoteError::other("PSWAP depth overflow")) + } + // INSTANCE METHODS // -------------------------------------------------------------------------------------------- @@ -412,11 +425,12 @@ impl PswapNote { let requested_faucet_id = self.storage.requested_faucet_id(); let total_requested_amount = self.storage.requested_asset_amount(); - let fill_asset = FungibleAsset::new(requested_faucet_id, total_requested_amount) - .map_err(|e| NoteError::other_with_source("failed to create full fill asset", e))? - .with_callbacks(self.storage.requested_asset().callbacks()); + let fill_asset = + FungibleAsset::new(requested_faucet_id, total_requested_amount.as_u64()) + .map_err(|e| NoteError::other_with_source("failed to create full fill asset", e))? + .with_callbacks(self.storage.requested_asset().callbacks()); - self.create_payback_note(consumer_account_id, fill_asset, total_requested_amount) + self.create_payback_note(consumer_account_id, fill_asset, total_requested_amount.as_u64()) } /// Executes the swap, producing the output notes for a given fill. @@ -459,7 +473,7 @@ impl PswapNote { let total_offered_amount = self.offered_asset.amount().as_u64(); let requested_faucet_id = self.storage.requested_faucet_id(); - let total_requested_amount = self.storage.requested_asset_amount(); + let total_requested_amount = self.storage.requested_asset_amount().as_u64(); // Validate fill amount if fill_amount == 0 { @@ -536,7 +550,7 @@ impl PswapNote { /// /// Returns an error if the calculated payout is not a valid asset amount. pub fn calculate_offered_for_requested(&self, fill_amount: u64) -> Result { - let total_requested = self.storage.requested_asset_amount(); + let total_requested = self.storage.requested_asset_amount().as_u64(); let total_offered = self.offered_asset.amount().as_u64(); Self::calculate_output_amount(total_offered, total_requested, fill_amount) @@ -554,17 +568,13 @@ impl PswapNote { /// /// # Errors /// - /// Returns an error if `attachment.depth() == 0` or if the fill amount is not a valid - /// asset amount. + /// Returns an error if the fill amount is not a valid asset amount. pub fn payback_note( &self, consumer_account_id: AccountId, attachment: &PswapNoteAttachment, ) -> Result { - let depth = attachment.depth(); - if depth == 0 { - return Err(NoteError::other("depth must be >= 1")); - } + let depth = attachment.depth().get(); let parent_depth = Felt::from(depth - 1); let p2id_serial = Word::from([ self.serial_number[0] + ONE, @@ -609,8 +619,7 @@ impl PswapNote { /// /// # Errors /// - /// Returns an error if `attachment.depth() == 0` or if any amount is not a valid asset - /// amount. + /// Returns an error if any amount is not a valid asset amount. pub fn remainder_note( &self, consumer_account_id: AccountId, @@ -618,10 +627,7 @@ impl PswapNote { remaining_offered: AssetAmount, remaining_requested: AssetAmount, ) -> Result { - let depth = attachment.depth(); - if depth == 0 { - return Err(NoteError::other("depth must be >= 1")); - } + let depth = attachment.depth().get(); let remainder_serial = Word::from([ self.serial_number[0], self.serial_number[1], @@ -723,17 +729,15 @@ impl PswapNote { /// Builds the [`NoteAttachment`] carried by both PSWAP output notes (payback and /// remainder). /// - /// `amount` is the round's transferred amount on the relevant side of the trade — + /// `amount` is the round's transferred amount on the relevant side of the trade - /// requested-asset units for the payback, offered-asset units for the remainder. fn pswap_output_attachment( amount: u64, order_id: Felt, - depth: u64, + depth: NonZeroU32, ) -> Result { let amount = AssetAmount::new(amount) .map_err(|e| NoteError::other_with_source("amount is not a valid asset amount", e))?; - let depth = u32::try_from(depth) - .map_err(|_| NoteError::other("PSWAP depth does not fit in u32"))?; Ok(PswapNoteAttachment::new(amount, order_id, depth).into()) } @@ -765,7 +769,7 @@ impl PswapNote { let recipient = P2idNoteStorage::new(self.storage.creator_account_id).into_recipient(p2id_serial_num); - let current_depth = self.parent_depth() + 1; + let current_depth = self.next_depth()?; let attachment = Self::pswap_output_attachment(fill_amount, self.order_id(), current_depth)?; @@ -813,7 +817,7 @@ impl PswapNote { self.serial_number[3] + ONE, ]); - let current_depth = self.parent_depth() + 1; + let current_depth = self.next_depth()?; let attachment = Self::pswap_output_attachment(offered_amount_for_fill, self.order_id(), current_depth)?; @@ -1049,7 +1053,7 @@ mod tests { let parsed = PswapNoteStorage::try_from(storage_items.as_slice()).unwrap(); assert_eq!(parsed.creator_account_id(), creator_id); - assert_eq!(parsed.requested_asset_amount(), 500); + assert_eq!(parsed.requested_asset_amount(), AssetAmount::new(500).unwrap()); } #[test] @@ -1066,7 +1070,7 @@ mod tests { let parsed = PswapNoteStorage::try_from(note_storage.items()).unwrap(); assert_eq!(parsed.creator_account_id(), creator_id); - assert_eq!(parsed.requested_asset_amount(), 500); + assert_eq!(parsed.requested_asset_amount(), AssetAmount::new(500).unwrap()); } /// Consumer supplies both an account fill and a note fill, and the sum is below @@ -1104,7 +1108,7 @@ mod tests { // Remainder must exist with the unfilled 50 - 30 = 20 of requested, and the // offered amount reduced proportionally (100 - 30*2 = 40). let remainder = remainder.expect("partial fill should produce remainder"); - assert_eq!(remainder.storage().requested_asset_amount(), 20); + assert_eq!(remainder.storage().requested_asset_amount(), AssetAmount::new(20).unwrap()); assert_eq!(remainder.offered_asset().amount().as_u64(), 40); assert_eq!(remainder.storage().creator_account_id(), creator_id); } @@ -1187,8 +1191,9 @@ mod tests { ); // --- payback_note() reconstruction --- + let depth_one = NonZeroU32::new(1).unwrap(); let payback_attachment = - PswapNoteAttachment::new(AssetAmount::new(20).unwrap(), pswap.order_id(), 1); + PswapNoteAttachment::new(AssetAmount::new(20).unwrap(), pswap.order_id(), depth_one); let reconstructed_payback = pswap.payback_note(consumer_id, &payback_attachment).unwrap(); let Asset::Fungible(fa) = reconstructed_payback.assets().iter().next().unwrap() else { panic!("expected fungible payback asset"); @@ -1201,7 +1206,7 @@ mod tests { // --- remainder_note() reconstruction --- let remainder_attachment = - PswapNoteAttachment::new(AssetAmount::new(40).unwrap(), pswap.order_id(), 1); + PswapNoteAttachment::new(AssetAmount::new(40).unwrap(), pswap.order_id(), depth_one); let reconstructed_remainder = pswap .remainder_note( consumer_id, @@ -1219,4 +1224,136 @@ mod tests { "remainder_note must preserve offered asset's callback flag", ); } + + /// `create_args` is infallible at the type level because `AssetAmount` fits in a `Felt`. + /// `MAX` round-trips through the resulting word. + #[test] + fn create_args_round_trips_max_asset_amount() { + let args = PswapNote::create_args(AssetAmount::MAX, AssetAmount::ZERO); + assert_eq!(args[0], Felt::from(AssetAmount::MAX)); + assert_eq!(args[1], ZERO); + assert_eq!(args[2], ZERO); + assert_eq!(args[3], ZERO); + } + + /// `PswapNoteAttachment` accessors mirror what was passed to `new`. + #[test] + fn pswap_note_attachment_accessors() { + let order_id = Felt::from(42u32); + let depth = NonZeroU32::new(3).unwrap(); + let attachment = PswapNoteAttachment::new(AssetAmount::new(100).unwrap(), order_id, depth); + assert_eq!(attachment.amount(), AssetAmount::new(100).unwrap()); + assert_eq!(attachment.order_id(), order_id); + assert_eq!(attachment.depth(), depth); + } + + /// `From for NoteAttachment` encodes the depth via `.get()`. + #[test] + fn pswap_note_attachment_encodes_depth_via_get() { + let depth = NonZeroU32::new(7).unwrap(); + let attachment = + PswapNoteAttachment::new(AssetAmount::new(50).unwrap(), Felt::from(9u32), depth); + let note_att: NoteAttachment = attachment.into(); + let word = note_att.content().as_words()[0]; + assert_eq!(word[2], Felt::from(7u32)); + } + + /// `parent_depth` returns 0 when the note has no attachment. + #[test] + fn parent_depth_zero_when_no_attachment() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); + assert_eq!(pswap.parent_depth(), 0); + assert_eq!(pswap.next_depth().unwrap().get(), 1); + } + + /// `parent_depth` returns the stored depth when the note carries a PSWAP attachment. + #[test] + fn parent_depth_reads_attachment_depth() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let order_id = Felt::from(1u32); + let attachment = PswapNoteAttachment::new( + AssetAmount::new(10).unwrap(), + order_id, + NonZeroU32::new(4).unwrap(), + ); + let pswap = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .attachment(attachment.into()) + .build() + .unwrap(); + assert_eq!(pswap.parent_depth(), 4); + assert_eq!(pswap.next_depth().unwrap().get(), 5); + } + + /// `parent_depth` falls back to 0 when the attachment encodes a depth outside `u32` range + /// (treated as corrupted by external construction; downstream arithmetic is left + /// well-defined). + #[test] + fn parent_depth_zero_on_out_of_range_attachment() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + // Stamp a raw word with a depth exceeding u32::MAX. + let oversized_depth = Felt::try_from(u64::from(u32::MAX) + 1).unwrap(); + let word = Word::from([Felt::from(1u32), Felt::from(1u32), oversized_depth, ZERO]); + let raw_attachment = + NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + let pswap = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .attachment(raw_attachment) + .build() + .unwrap(); + assert_eq!(pswap.parent_depth(), 0); + } + + /// `next_depth` propagates a `NoteError` if the parent depth is at `u32::MAX`. + #[test] + fn next_depth_errors_on_u32_overflow() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let attachment = PswapNoteAttachment::new( + AssetAmount::new(1).unwrap(), + Felt::from(1u32), + NonZeroU32::new(u32::MAX).unwrap(), + ); + let pswap = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .attachment(attachment.into()) + .build() + .unwrap(); + assert!(pswap.next_depth().is_err()); + } } diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index d8beab912b..792915b777 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -1,3 +1,4 @@ +use core::num::NonZeroU32; use std::collections::BTreeMap; use std::slice; @@ -174,7 +175,10 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( // --- Step 1: Bob fills the PSWAP note --- let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(fill_amount, 0)?); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO), + ); let (p2id_note, remainder_pswap) = pswap.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_amount)?), None)?; @@ -217,8 +221,11 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( ); // Depth = 1 (first fill). Consumer comes from the on-chain payback's metadata sender. - let payback_attachment = - PswapNoteAttachment::new(AssetAmount::new(fill_amount_from_aux)?, pswap.order_id(), 1); + let payback_attachment = PswapNoteAttachment::new( + AssetAmount::new(fill_amount_from_aux)?, + pswap.order_id(), + NonZeroU32::new(1).unwrap(), + ); let reconstructed_payback = pswap.payback_note(output_p2id.metadata().sender(), &payback_attachment)?; @@ -249,7 +256,7 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( let remainder_attachment = PswapNoteAttachment::new( AssetAmount::new(amt_payout_from_attachment)?, pswap.order_id(), - 1, + NonZeroU32::new(1).unwrap(), ); let reconstructed_remainder = pswap.remainder_note( output_remainder.metadata().sender(), @@ -336,7 +343,7 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { let expected_depth = 1u64; // first fill of an original PSWAP let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(fill_amount, 0)?); + note_args_map.insert(pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); let (p2id_note, remainder_pswap) = pswap.execute(bob.id(), Some(eth_20), None)?; let remainder_note = @@ -514,7 +521,7 @@ async fn pswap_fill_test( if !use_network_account { let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(fill_amount, 0)?); + note_args_map.insert(pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); tx_builder = tx_builder.extend_note_args(note_args_map); } @@ -595,8 +602,14 @@ async fn pswap_note_note_fill_cross_swap_test() -> anyhow::Result<()> { // Note args: pure note fill (account_fill = 0, note_fill = full amount) let mut note_args_map = BTreeMap::new(); - note_args_map.insert(alice_pswap_note.id(), PswapNote::create_args(0, 25)?); - note_args_map.insert(bob_pswap_note.id(), PswapNote::create_args(0, 50)?); + note_args_map.insert( + alice_pswap_note.id(), + PswapNote::create_args(AssetAmount::ZERO, AssetAmount::new(25)?), + ); + note_args_map.insert( + bob_pswap_note.id(), + PswapNote::create_args(AssetAmount::ZERO, AssetAmount::new(50)?), + ); // Expected P2ID notes let (alice_p2id_note, _) = alice_pswap.execute(charlie.id(), None, Some(eth_25))?; @@ -697,8 +710,14 @@ async fn pswap_note_combined_account_fill_and_note_fill_test() -> anyhow::Result // Alice's pswap uses a combined fill; Bob's pswap uses pure note_fill. let mut note_args_map = BTreeMap::new(); - note_args_map.insert(alice_pswap_note.id(), PswapNote::create_args(20, 30)?); - note_args_map.insert(bob_pswap_note.id(), PswapNote::create_args(0, 60)?); + note_args_map.insert( + alice_pswap_note.id(), + PswapNote::create_args(AssetAmount::new(20)?, AssetAmount::new(30)?), + ); + note_args_map.insert( + bob_pswap_note.id(), + PswapNote::create_args(AssetAmount::ZERO, AssetAmount::new(60)?), + ); let (alice_p2id_note, alice_remainder) = alice_pswap.execute(charlie.id(), Some(account_fill_eth), Some(note_fill_eth))?; @@ -792,7 +811,6 @@ async fn pswap_note_creator_reclaim_test() -> anyhow::Result<()> { /// `assert_valid_asset_amount` fires instead. #[rstest] #[case::fill_exceeds_requested(30, 0, ERR_PSWAP_FILL_EXCEEDS_REQUESTED)] -#[case::fill_sum_u64_overflow(1u64 << 63, 1u64 << 63, ERR_PSWAP_FILL_SUM_OVERFLOW)] #[case::fill_sum_exceeds_max_asset_amount( FungibleAsset::MAX_AMOUNT.as_u64(), FungibleAsset::MAX_AMOUNT.as_u64(), @@ -828,7 +846,10 @@ async fn pswap_note_invalid_input_test( let mock_chain = builder.build()?; let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(account_fill, note_fill)?); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(account_fill)?, AssetAmount::new(note_fill)?), + ); let tx_context = mock_chain .build_tx_context(bob.id(), &[pswap_note.id()], &[])? @@ -841,6 +862,54 @@ async fn pswap_note_invalid_input_test( Ok(()) } +/// `create_args` rejects values above `AssetAmount::MAX` client-side, so the MASM-side +/// `ERR_PSWAP_FILL_SUM_OVERFLOW` check (u64 wrap) is unreachable through the typed API. +/// This regression test hand-builds the `note_args` word with raw felts to keep that +/// MASM guard covered. +#[tokio::test] +async fn pswap_note_fill_sum_u64_overflow_via_raw_args() -> anyhow::Result<()> { + let mut builder = MockChain::builder(); + + let usdc_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "USDC", 1000, Some(50))?; + let eth_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "ETH", 1000, Some(30))?; + + let alice = builder.add_existing_wallet_with_assets( + BASIC_AUTH, + [FungibleAsset::new(usdc_faucet.id(), 50)?.into()], + )?; + let bob = builder.add_existing_wallet_with_assets( + BASIC_AUTH, + [FungibleAsset::new(eth_faucet.id(), 30)?.into()], + )?; + + let (_, pswap_note) = build_pswap_note( + &mut builder, + alice.id(), + FungibleAsset::new(usdc_faucet.id(), 50)?, + FungibleAsset::new(eth_faucet.id(), 25)?, + NoteType::Public, + )?; + let mock_chain = builder.build()?; + + // Hand-built [account_fill, note_fill, 0, 0] with both fills at 2^63. Their MASM-side + // unchecked u64 sum overflows, tripping ERR_PSWAP_FILL_SUM_OVERFLOW. + let half_u64 = Felt::try_from(1u64 << 63).expect("2^63 fits in a felt"); + let raw_args = Word::from([half_u64, half_u64, ZERO, ZERO]); + + let mut note_args_map = BTreeMap::new(); + note_args_map.insert(pswap_note.id(), raw_args); + + let tx_context = mock_chain + .build_tx_context(bob.id(), &[pswap_note.id()], &[])? + .extend_note_args(note_args_map) + .build()?; + + let result = tx_context.execute().await; + assert_transaction_executor_error!(result, ERR_PSWAP_FILL_SUM_OVERFLOW); + + Ok(()) +} + /// Regression test for the `note_idx` stack-layout bug in `create_p2id_note`'s /// `has_account_fill` branch. /// @@ -890,7 +959,10 @@ async fn pswap_note_idx_nonzero_regression_test() -> anyhow::Result<()> { // Full account-fill: 25 ETH out of bob's vault. Exercises the // `has_account_fill` branch where the `note_idx` bug lives. let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(25, 0)?); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(25)?, AssetAmount::ZERO), + ); let (expected_p2id, _) = pswap.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), 25)?), None)?; @@ -977,7 +1049,7 @@ async fn pswap_multiple_partial_fills_test(#[case] fill_amount: u64) -> anyhow:: let mock_chain = builder.build()?; let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(fill_amount, 0)?); + note_args_map.insert(pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); let payout_amount = pswap.calculate_offered_for_requested(fill_amount)?; let (p2id_note, remainder_pswap) = @@ -1046,7 +1118,10 @@ async fn run_partial_fill_ratio_case( let mock_chain = builder.build()?; let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(fill_eth, 0)?); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(fill_eth)?, AssetAmount::ZERO), + ); let payout_amount = pswap.calculate_offered_for_requested(fill_eth)?; let remaining_offered = offered_usdc - payout_amount; @@ -1229,7 +1304,10 @@ async fn pswap_chained_partial_fills_test( let mock_chain = builder.build()?; let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(*fill_amount, 0)?); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(*fill_amount)?, AssetAmount::ZERO), + ); let payout_amount = pswap.calculate_offered_for_requested(*fill_amount)?; let remaining_offered = current_offered - payout_amount; @@ -1339,7 +1417,11 @@ fn compare_pswap_create_output_notes_vs_test_helper() { // Verify roundtripped PswapNote preserves key fields assert_eq!(pswap.sender(), alice.id(), "Sender mismatch after roundtrip"); assert_eq!(pswap.note_type(), NoteType::Public, "Note type mismatch after roundtrip"); - assert_eq!(pswap.storage().requested_asset_amount(), 25, "Requested amount mismatch"); + assert_eq!( + pswap.storage().requested_asset_amount(), + AssetAmount::new(25).unwrap(), + "Requested amount mismatch", + ); assert_eq!(pswap.storage().creator_account_id(), alice.id(), "Creator ID mismatch"); // Full fill: should produce P2ID note, no remainder @@ -1376,7 +1458,11 @@ fn compare_pswap_create_output_notes_vs_test_helper() { "Remainder creator should be Alice" ); let remaining_requested = remainder_pswap.storage().requested_asset_amount(); - assert_eq!(remaining_requested, 15, "Remaining requested should be 15"); + assert_eq!( + remaining_requested, + AssetAmount::new(15).unwrap(), + "Remaining requested should be 15", + ); } /// Test that PswapNote::parse_inputs roundtrips correctly @@ -1517,7 +1603,7 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result let mut current_requested = initial_requested; for (idx, fill_amount) in fills.iter().copied().enumerate() { - let depth = (idx + 1) as u32; + let depth = NonZeroU32::new((idx + 1) as u32).expect("idx + 1 is always >= 1"); // --- Bob fills the current PSWAP --- let payout_amount = current_pswap.calculate_offered_for_requested(fill_amount)?; @@ -1541,7 +1627,7 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result }; let mut note_args_map = BTreeMap::new(); - note_args_map.insert(current_pswap_note.id(), PswapNote::create_args(fill_amount, 0)?); + note_args_map.insert(current_pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); let bob_tx = mock_chain .build_tx_context(bob.id(), &[current_pswap_note.id()], &[])? @@ -1693,8 +1779,8 @@ async fn pswap_disambiguates_multiple_creator_pswaps_in_same_tx() -> anyhow::Res // Bob partially fills BOTH PSWAPs in the same tx — 10 ETH from each. let fill_each = 10u64; let mut note_args = BTreeMap::new(); - note_args.insert(note_a.id(), PswapNote::create_args(fill_each, 0)?); - note_args.insert(note_b.id(), PswapNote::create_args(fill_each, 0)?); + note_args.insert(note_a.id(), PswapNote::create_args(AssetAmount::new(fill_each)?, AssetAmount::ZERO)); + note_args.insert(note_b.id(), PswapNote::create_args(AssetAmount::new(fill_each)?, AssetAmount::ZERO)); let (payback_a, remainder_a) = pswap_a.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_each)?), None)?; @@ -1788,5 +1874,9 @@ fn pswap_parse_inputs_roundtrip() { assert_eq!(parsed.creator_account_id(), alice.id(), "Creator ID roundtrip failed!"); // Verify requested amount from value word - assert_eq!(parsed.requested_asset_amount(), 25, "Requested amount should be 25"); + assert_eq!( + parsed.requested_asset_amount(), + AssetAmount::new(25).unwrap(), + "Requested amount should be 25", + ); } From cf8fee4e973da17a3aee5e873585cd3ecc22abc6 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Thu, 28 May 2026 17:25:37 +0530 Subject: [PATCH 02/14] refactor(pswap): harden PswapNoteAttachment parsing - Add TryFrom<&NoteAttachment> for PswapNoteAttachment validating scheme, num_words == 1, AssetAmount range, and non-zero u32 depth. - Reroute PswapNote::parent_depth through the typed TryFrom so any malformed attachment degrades to 0 in a single place. - Inline the deleted pswap_output_attachment helper at the two callers using PswapNoteAttachment::new(...).into(); construction stays infallible apart from the AssetAmount::new check that was already there. - Migrate the integration tests' first_attachment_word helper to first_pswap_attachment, exercising the new TryFrom on every output note (9 sites total). Closes group B of issue #3000. --- crates/miden-standards/src/note/pswap.rs | 146 +++++++++++++++----- crates/miden-testing/tests/scripts/pswap.rs | 51 ++++--- 2 files changed, 137 insertions(+), 60 deletions(-) diff --git a/crates/miden-standards/src/note/pswap.rs b/crates/miden-standards/src/note/pswap.rs index d3ee6d4aeb..96b13f0224 100644 --- a/crates/miden-standards/src/note/pswap.rs +++ b/crates/miden-standards/src/note/pswap.rs @@ -236,6 +236,43 @@ impl From for NoteAttachment { } } +/// Parses a [`NoteAttachment`] back into a typed [`PswapNoteAttachment`]. +/// +/// Validates that: +/// - the scheme is [`PswapNote::PSWAP_ATTACHMENT_SCHEME`]; +/// - the content is exactly one word (`num_words == 1`); +/// - the word's `amount` slot is a valid [`AssetAmount`]; +/// - the word's `depth` slot fits in a `u32` and is non-zero. +/// +/// The trailing slot (`word[3]`) is not asserted to be zero: PSWAP_ATTACHMENT_SCHEME's +/// canonical encoding leaves it for forward compatibility. +impl TryFrom<&NoteAttachment> for PswapNoteAttachment { + type Error = NoteError; + + fn try_from(attachment: &NoteAttachment) -> Result { + if attachment.attachment_scheme() != PswapNote::PSWAP_ATTACHMENT_SCHEME { + return Err(NoteError::other("attachment scheme is not PSWAP_ATTACHMENT_SCHEME")); + } + let words = attachment.content().as_words(); + if words.len() != 1 { + return Err(NoteError::other("PSWAP attachment must encode exactly one word")); + } + let word = words[0]; + + let amount = AssetAmount::new(word[0].as_canonical_u64()).map_err(|e| { + NoteError::other_with_source("PSWAP attachment amount is not a valid asset amount", e) + })?; + + let depth_raw = word[PswapNote::PARENT_ATTACHMENT_DEPTH_OFFSET].as_canonical_u64(); + let depth_u32 = u32::try_from(depth_raw) + .map_err(|_| NoteError::other("PSWAP attachment depth does not fit in u32"))?; + let depth = NonZeroU32::new(depth_u32) + .ok_or_else(|| NoteError::other("PSWAP attachment depth must be non-zero"))?; + + Ok(Self { amount, order_id: word[1], depth }) + } +} + // PSWAP NOTE // ================================================================================================ @@ -380,22 +417,15 @@ impl PswapNote { /// /// The next round's `current_depth` is computed as `parent_depth() + 1`, matching the /// on-chain `get_current_depth` MASM procedure. Use [`Self::next_depth`] for the typed - /// [`NonZeroU32`] form. + /// [`NonZeroU32`] form. A malformed `PSWAP_ATTACHMENT_SCHEME` attachment (depth out of + /// `u32` range, depth == 0, etc.) is treated as if no attachment is present, so discovery + /// degrades to "looks like an original" rather than corrupting downstream arithmetic. pub fn parent_depth(&self) -> u32 { - match self.attachment.as_ref() { - Some(att) if att.attachment_scheme() == Self::PSWAP_ATTACHMENT_SCHEME => { - // The depth value is written by this crate (Group B's TryFrom enforces u32 - // range on read); treat out-of-range as a corrupted attachment and fall back - // to 0 so discovery on a malformed note degrades to "looks like an original" - // rather than corrupting downstream arithmetic. - u32::try_from( - att.content().as_words()[0][Self::PARENT_ATTACHMENT_DEPTH_OFFSET] - .as_canonical_u64(), - ) - .unwrap_or(0) - }, - _ => 0, - } + self.attachment + .as_ref() + .and_then(|att| PswapNoteAttachment::try_from(att).ok()) + .map(|att| att.depth().get()) + .unwrap_or(0) } /// Returns the depth that the next-round payback / remainder should carry, equal to @@ -726,20 +756,6 @@ impl PswapNote { Ok(amount) } - /// Builds the [`NoteAttachment`] carried by both PSWAP output notes (payback and - /// remainder). - /// - /// `amount` is the round's transferred amount on the relevant side of the trade - - /// requested-asset units for the payback, offered-asset units for the remainder. - fn pswap_output_attachment( - amount: u64, - order_id: Felt, - depth: NonZeroU32, - ) -> Result { - let amount = AssetAmount::new(amount) - .map_err(|e| NoteError::other_with_source("amount is not a valid asset amount", e))?; - Ok(PswapNoteAttachment::new(amount, order_id, depth).into()) - } /// Builds a payback note (P2ID) that delivers the filled assets to the swap creator. /// @@ -770,8 +786,11 @@ impl PswapNote { P2idNoteStorage::new(self.storage.creator_account_id).into_recipient(p2id_serial_num); let current_depth = self.next_depth()?; - let attachment = - Self::pswap_output_attachment(fill_amount, self.order_id(), current_depth)?; + let fill_amount_typed = AssetAmount::new(fill_amount).map_err(|e| { + NoteError::other_with_source("fill amount is not a valid asset amount", e) + })?; + let attachment: NoteAttachment = + PswapNoteAttachment::new(fill_amount_typed, self.order_id(), current_depth).into(); let p2id_assets = NoteAssets::new(vec![payback_asset.into()])?; let p2id_metadata = @@ -818,8 +837,11 @@ impl PswapNote { ]); let current_depth = self.next_depth()?; - let attachment = - Self::pswap_output_attachment(offered_amount_for_fill, self.order_id(), current_depth)?; + let payout_typed = AssetAmount::new(offered_amount_for_fill).map_err(|e| { + NoteError::other_with_source("payout amount is not a valid asset amount", e) + })?; + let attachment: NoteAttachment = + PswapNoteAttachment::new(payout_typed, self.order_id(), current_depth).into(); PswapNote::builder() .sender(consumer_account_id) @@ -1329,6 +1351,64 @@ mod tests { assert_eq!(pswap.parent_depth(), 0); } + /// `TryFrom<&NoteAttachment>` rejects a wrong scheme. + #[test] + fn try_from_rejects_wrong_scheme() { + let word = Word::from([Felt::from(1u32), Felt::from(2u32), Felt::from(1u32), ZERO]); + // Use NetworkAccountTarget (scheme = 2) instead of PSWAP_ATTACHMENT_SCHEME (3). + let other = NoteAttachment::with_word(StandardNoteAttachment::NetworkAccountTarget.attachment_scheme(), word); + assert!(PswapNoteAttachment::try_from(&other).is_err()); + } + + /// `TryFrom<&NoteAttachment>` rejects depth == 0 (the invariant only one path enforces). + #[test] + fn try_from_rejects_zero_depth() { + let word = Word::from([Felt::from(1u32), Felt::from(2u32), ZERO, ZERO]); + let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + assert!(PswapNoteAttachment::try_from(&att).is_err()); + } + + /// `TryFrom<&NoteAttachment>` rejects a depth value that exceeds `u32::MAX`. + #[test] + fn try_from_rejects_out_of_range_depth() { + let oversized = Felt::try_from(u64::from(u32::MAX) + 1).unwrap(); + let word = Word::from([Felt::from(1u32), Felt::from(2u32), oversized, ZERO]); + let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + assert!(PswapNoteAttachment::try_from(&att).is_err()); + } + + /// `TryFrom<&NoteAttachment>` rejects an amount that exceeds `AssetAmount::MAX`. + #[test] + fn try_from_rejects_invalid_amount() { + // 2^63 > AssetAmount::MAX = 2^63 - 2^31. + let bad_amount = Felt::try_from(1u64 << 63).unwrap(); + let word = Word::from([bad_amount, Felt::from(2u32), Felt::from(1u32), ZERO]); + let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + assert!(PswapNoteAttachment::try_from(&att).is_err()); + } + + /// `TryFrom<&NoteAttachment>` rejects an attachment whose word count is not 1 (covering the + /// MASM `num_words == 1` assert from the Rust side). + #[test] + fn try_from_rejects_wrong_num_words() { + let words = vec![Word::default(), Word::default()]; + let multi = NoteAttachment::with_words(PswapNote::PSWAP_ATTACHMENT_SCHEME, words).unwrap(); + assert!(PswapNoteAttachment::try_from(&multi).is_err()); + } + + /// `TryFrom<&NoteAttachment>` round-trips a valid attachment. + #[test] + fn try_from_round_trips_valid_attachment() { + let original = PswapNoteAttachment::new( + AssetAmount::new(123).unwrap(), + Felt::from(7u32), + NonZeroU32::new(5).unwrap(), + ); + let encoded: NoteAttachment = original.into(); + let decoded = PswapNoteAttachment::try_from(&encoded).unwrap(); + assert_eq!(decoded, original); + } + /// `next_depth` propagates a `NoteError` if the parent depth is at `u32::MAX`. #[test] fn next_depth_errors_on_u32_overflow() { diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index 792915b777..8c125eed2c 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -33,11 +33,11 @@ const BASIC_AUTH: Auth = Auth::BasicAuth { // HELPERS // ================================================================================================ -/// Extracts the first attachment's word content from a `NoteAttachments`. -fn first_attachment_word(attachments: &NoteAttachments) -> Word { - let content = attachments.get(0).expect("expected at least one attachment").content(); - assert_eq!(content.num_words(), 1, "expected single word attachment"); - content.as_words()[0] +/// Parses the first attachment as a [`PswapNoteAttachment`], asserting it conforms to +/// the typed scheme (single word, valid amount, non-zero u32 depth). +fn first_pswap_attachment(attachments: &NoteAttachments) -> PswapNoteAttachment { + let attachment = attachments.get(0).expect("expected at least one attachment"); + PswapNoteAttachment::try_from(attachment).expect("attachment must be a valid PSWAP attachment") } /// Builds a PswapNote, registers it on the builder as an output note, and returns @@ -209,14 +209,14 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( // Read attachments from the executed tx (the body is still here even when the note will // ultimately land on-chain as a header-only private commitment). let output_p2id = executed_transaction.output_notes().get_note(0); - let attachment_word = first_attachment_word(output_p2id.attachments()); - let fill_amount_from_aux = attachment_word[0].as_canonical_u64(); + let on_chain_pswap_att = first_pswap_attachment(output_p2id.attachments()); + let fill_amount_from_aux = on_chain_pswap_att.amount().as_u64(); assert_eq!(fill_amount_from_aux, fill_amount, "fill amount from aux should match the case"); // Parity check: Rust-predicted P2ID attachment must match the MASM output. assert_eq!( - first_attachment_word(p2id_note.attachments()), - attachment_word, + first_pswap_attachment(p2id_note.attachments()), + on_chain_pswap_att, "Rust-predicted P2ID attachment does not match the MASM-produced one", ); @@ -239,8 +239,8 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( if is_partial { let output_remainder = executed_transaction.output_notes().get_note(1); - let remainder_attachment_word = first_attachment_word(output_remainder.attachments()); - let amt_payout_from_attachment = remainder_attachment_word[0].as_canonical_u64(); + let remainder_pswap_att = first_pswap_attachment(output_remainder.attachments()); + let amt_payout_from_attachment = remainder_pswap_att.amount().as_u64(); let expected_payout = pswap.calculate_offered_for_requested(fill_amount_from_aux)?; assert_eq!( @@ -409,16 +409,16 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { "remainder attachment word mismatch: expected [amt_payout, order_id, depth, 0]", ); - // Cross-check: the Rust-predicted notes must produce the same attachment - // words as the on-chain executed ones. + // Cross-check: the Rust-predicted notes must produce the same typed attachment as the + // on-chain executed ones. assert_eq!( - first_attachment_word(p2id_note.attachments()), - p2id_att.content().as_words()[0], + first_pswap_attachment(p2id_note.attachments()), + PswapNoteAttachment::try_from(p2id_att)?, "Rust-predicted P2ID attachment does not match MASM output", ); assert_eq!( - first_attachment_word(remainder_note.attachments()), - remainder_att.content().as_words()[0], + first_pswap_attachment(remainder_note.attachments()), + PswapNoteAttachment::try_from(remainder_att)?, "Rust-predicted remainder attachment does not match MASM output", ); @@ -1642,15 +1642,15 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result let on_chain_payback = bob_tx.output_notes().get_note(0); // --- Alice reconstructs the payback from the on-chain attachment word --- - let attachment_word = first_attachment_word(on_chain_payback.attachments()); - let fill_from_attachment = attachment_word[0].as_canonical_u64(); + let on_chain_payback_att = first_pswap_attachment(on_chain_payback.attachments()); + let fill_from_attachment = on_chain_payback_att.amount().as_u64(); assert_eq!( fill_from_attachment, fill_amount, "round {depth}: attachment fill amount mismatch", ); let payback_attachment = PswapNoteAttachment::new( - AssetAmount::new(fill_from_attachment)?, + on_chain_payback_att.amount(), original_pswap.order_id(), depth, ); @@ -1665,11 +1665,11 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result // --- Alice reconstructs the remainder (when partial) from on-chain data alone --- if next_pswap_opt.is_some() { let on_chain_remainder = bob_tx.output_notes().get_note(1); - let remainder_attachment_word = first_attachment_word(on_chain_remainder.attachments()); - let payout_from_attachment = remainder_attachment_word[0].as_canonical_u64(); + let on_chain_remainder_att = + first_pswap_attachment(on_chain_remainder.attachments()); let remainder_attachment = PswapNoteAttachment::new( - AssetAmount::new(payout_from_attachment)?, + on_chain_remainder_att.amount(), original_pswap.order_id(), depth, ); @@ -1812,11 +1812,8 @@ async fn pswap_disambiguates_multiple_creator_pswaps_in_same_tx() -> anyhow::Res // Each lineage should yield 2 notes (payback + remainder) → preallocate. let mut from_a: Vec = Vec::with_capacity(2); let mut from_b: Vec = Vec::with_capacity(2); - // PswapAttachment word layout is [amount, order_id, depth, 0]; order_id sits at index 1. - const ORDER_ID_INDEX_IN_PSWAP_ATTACHMENT: usize = 1; for i in 0..outputs.num_notes() { - let att_word = first_attachment_word(outputs.get_note(i).attachments()); - let oid = att_word[ORDER_ID_INDEX_IN_PSWAP_ATTACHMENT]; + let oid = first_pswap_attachment(outputs.get_note(i).attachments()).order_id(); let digest = outputs.get_note(i).recipient_digest(); if oid == order_id_a { from_a.push(digest); From a29e336bb0f7d55a217617fcdc90d04970f305a3 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 00:13:09 +0530 Subject: [PATCH 03/14] feat(pswap.masm): assert num_words==1 and depth fits in u32 in get_current_depth - New constant PSWAP_ATTACHMENT_NUM_WORDS = 1 and error code ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS gate the write_attachment_to_memory result before any loc_load, ruling out multi-word attachments overwriting adjacent local memory. - After loc_load.PARENT_ATTACHMENT_DEPTH_OFFSET, split into (lo, hi) and assertz hi == 0 via ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32. Prefer manual hi check over u32assert.err=... so the named error surfaces verbatim at the test boundary (the VM's u32assert wraps its message at runtime). - Sharpened the procedure docstring: the initial (depth-0) PSWAP carries no PSWAP_ATTACHMENT_SCHEME attachment; only payback / remainder notes do. - Integration tests pswap_assert_attachment_wrong_num_words and pswap_assert_attachment_depth_not_u32 stamp malformed attachments via NoteAttachment::{with_words,with_word} and exercise both asserts. Closes group C of issue #3000. --- .../asm/standards/notes/pswap.masm | 30 ++++- crates/miden-testing/tests/scripts/pswap.rs | 117 +++++++++++++++++- 2 files changed, 142 insertions(+), 5 deletions(-) diff --git a/crates/miden-standards/asm/standards/notes/pswap.masm b/crates/miden-standards/asm/standards/notes/pswap.masm index 00837f651c..3dfe83546a 100644 --- a/crates/miden-standards/asm/standards/notes/pswap.masm +++ b/crates/miden-standards/asm/standards/notes/pswap.masm @@ -96,6 +96,8 @@ const EXEC_AMT_REQUESTED_NOTE_FILL = 9 # write the word at PARENT_ATTACHMENT_PTR, read back only the depth at DEPTH_OFFSET. const PARENT_ATTACHMENT_PTR = 0 const PARENT_ATTACHMENT_DEPTH_OFFSET = 2 +# PswapAttachment is always exactly one word - see PSWAP_ATTACHMENT_SCHEME above. +const PSWAP_ATTACHMENT_NUM_WORDS = 1 # ERRORS # ================================================================================================= @@ -106,6 +108,8 @@ const ERR_PSWAP_FILL_EXCEEDS_REQUESTED="PSWAP fill amount exceeds requested amou const ERR_PSWAP_FILL_SUM_OVERFLOW="PSWAP account_fill + note_fill overflows u64" const ERR_PSWAP_NOT_VALID_ASSET_AMOUNT="PSWAP computed amount exceeds max fungible asset amount" const ERR_PSWAP_PAYOUT_OVERFLOW="PSWAP payout quotient does not fit in u64" +const ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS="PSWAP attachment must encode exactly one word" +const ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32="PSWAP attachment depth must fit in u32" # U64 VALIDATION # ================================================================================================= @@ -493,9 +497,13 @@ proc get_order_id end #! Returns current_depth = parent_depth + 1, where parent_depth is the depth carried in the -#! consumed PSWAP note's PswapAttachment word at offset PARENT_ATTACHMENT_DEPTH_OFFSET if such an -#! attachment exists, or 0 if not (i.e., the parent is the original PSWAP, with scheme none() -#! or NetworkAccountTarget). +#! consumed PSWAP note's PswapAttachment word at offset PARENT_ATTACHMENT_DEPTH_OFFSET if such +#! an attachment exists, or 0 if not. +#! +#! The initial (depth-0) PSWAP carries no PSWAP_ATTACHMENT_SCHEME attachment; only payback P2IDs +#! and remainder PSWAPs do. So `is_found = false` here means we are consuming the original PSWAP +#! (or one carrying only an unrelated scheme like NetworkAccountTarget) and the next round is +#! depth 1. #! #! Inputs: [] #! Outputs: [current_depth] @@ -514,11 +522,25 @@ proc get_current_depth exec.active_note::write_attachment_to_memory # => [num_words] - drop + # A PswapAttachment is exactly one word - assert before reading back from @locals(4) to + # rule out an attacker-crafted multi-word attachment overwriting adjacent local memory. + eq.PSWAP_ATTACHMENT_NUM_WORDS + assert.err=ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS # => [] loc_load.PARENT_ATTACHMENT_DEPTH_OFFSET # => [parent_depth] + + # Defense-in-depth: assert depth fits in u32 by checking the high 32 bits are zero. The + # Rust side encodes u32 depths; a larger felt indicates a forged attachment. We avoid + # `u32assert.err=...` because the VM wraps its message at runtime, which would defeat + # exact-match assertions on the named error. + dup u32split + # => [lo, hi, parent_depth] (u32split puts lo on top) + drop + # => [hi, parent_depth] + assertz.err=ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32 + # => [parent_depth] else drop push.0 # => [0] diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index 8c125eed2c..7f664f79d4 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -7,11 +7,13 @@ use miden_protocol::account::{Account, AccountId, AccountType, AccountVaultDelta use miden_protocol::asset::{Asset, AssetAmount, FungibleAsset}; use miden_protocol::crypto::rand::{FeltRng, RandomCoin}; use miden_protocol::errors::MasmError; -use miden_protocol::note::{Note, NoteAttachments, NoteType}; +use miden_protocol::note::{Note, NoteAttachment, NoteAttachments, NoteType}; use miden_protocol::transaction::RawOutputNote; use miden_protocol::{Felt, ONE, Word, ZERO}; use miden_standards::account::wallets::BasicWallet; use miden_standards::errors::standards::{ + ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32, + ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS, ERR_PSWAP_FILL_EXCEEDS_REQUESTED, ERR_PSWAP_FILL_SUM_OVERFLOW, ERR_PSWAP_NOT_VALID_ASSET_AMOUNT, @@ -910,6 +912,119 @@ async fn pswap_note_fill_sum_u64_overflow_via_raw_args() -> anyhow::Result<()> { Ok(()) } +/// `get_current_depth` must reject a PSWAP-scheme attachment whose word count is not 1. +/// Multi-word attachments could overwrite memory beyond `@locals(4)` if consumed without +/// checking, so the MASM asserts `num_words == 1` before any `loc_load`. +#[tokio::test] +async fn pswap_assert_attachment_wrong_num_words() -> anyhow::Result<()> { + let mut builder = MockChain::builder(); + let usdc_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "USDC", 1000, Some(50))?; + let eth_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "ETH", 1000, Some(50))?; + let alice = builder.add_existing_wallet_with_assets( + BASIC_AUTH, + [FungibleAsset::new(usdc_faucet.id(), 50)?.into()], + )?; + let bob = builder.add_existing_wallet_with_assets( + BASIC_AUTH, + [FungibleAsset::new(eth_faucet.id(), 10)?.into()], + )?; + + // Stamp a two-word PSWAP attachment on the original PSWAP note (only possible via the + // raw NoteAttachment::with_words API; the typed PswapNoteAttachment cannot encode this). + let bogus_words = vec![Word::default(), Word::default()]; + let bogus_attachment = + NoteAttachment::with_words(PswapNote::PSWAP_ATTACHMENT_SCHEME, bogus_words)?; + + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(FungibleAsset::new(eth_faucet.id(), 25)?) + .creator_account_id(alice.id()) + .build(); + let pswap = PswapNote::builder() + .sender(alice.id()) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(FungibleAsset::new(usdc_faucet.id(), 50)?) + .attachment(bogus_attachment) + .build()?; + let pswap_note: Note = pswap.clone().into(); + builder.add_output_note(RawOutputNote::Full(pswap_note.clone())); + + let mock_chain = builder.build()?; + + let mut note_args_map = BTreeMap::new(); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(10)?, AssetAmount::ZERO), + ); + + let tx_context = mock_chain + .build_tx_context(bob.id(), &[pswap_note.id()], &[])? + .extend_note_args(note_args_map) + .build()?; + let result = tx_context.execute().await; + assert_transaction_executor_error!(result, ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS); + + Ok(()) +} + +/// `get_current_depth` must reject a PSWAP-scheme attachment whose depth slot exceeds u32::MAX. +/// The Rust side only ever writes u32 depths; a larger felt indicates a forged attachment. +#[tokio::test] +async fn pswap_assert_attachment_depth_not_u32() -> anyhow::Result<()> { + let mut builder = MockChain::builder(); + let usdc_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "USDC", 1000, Some(50))?; + let eth_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "ETH", 1000, Some(50))?; + let alice = builder.add_existing_wallet_with_assets( + BASIC_AUTH, + [FungibleAsset::new(usdc_faucet.id(), 50)?.into()], + )?; + let bob = builder.add_existing_wallet_with_assets( + BASIC_AUTH, + [FungibleAsset::new(eth_faucet.id(), 10)?.into()], + )?; + + // Stamp [amount, order_id, oversized_depth, 0] where oversized_depth > u32::MAX. + let oversized_depth = Felt::try_from(u64::from(u32::MAX) + 1).expect("fits in a felt"); + let bogus_word = Word::from([Felt::from(1u32), Felt::from(1u32), oversized_depth, ZERO]); + let bogus_attachment = + NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, bogus_word); + + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(FungibleAsset::new(eth_faucet.id(), 25)?) + .creator_account_id(alice.id()) + .build(); + let pswap = PswapNote::builder() + .sender(alice.id()) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(FungibleAsset::new(usdc_faucet.id(), 50)?) + .attachment(bogus_attachment) + .build()?; + let pswap_note: Note = pswap.clone().into(); + builder.add_output_note(RawOutputNote::Full(pswap_note.clone())); + + let mock_chain = builder.build()?; + + let mut note_args_map = BTreeMap::new(); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(10)?, AssetAmount::ZERO), + ); + + let tx_context = mock_chain + .build_tx_context(bob.id(), &[pswap_note.id()], &[])? + .extend_note_args(note_args_map) + .build()?; + let result = tx_context.execute().await; + assert_transaction_executor_error!(result, ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32); + + Ok(()) +} + /// Regression test for the `note_idx` stack-layout bug in `create_p2id_note`'s /// `has_account_fill` branch. /// From 54d07fa493ac944887ed50e59ec7cdef8595d561 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 05:32:55 +0530 Subject: [PATCH 04/14] refactor(pswap): split into submodule, introduce OrderId, expose AssetId view - Split crates/miden-standards/src/note/pswap.rs (1430 lines) into pswap/{mod, storage,attachment,tests}.rs. mod.rs now hosts the PswapNote struct and the new OrderId newtype; storage.rs and attachment.rs own their respective types and impls; tests.rs holds the unit tests. git mv preserves history on the old pswap.rs -> pswap/mod.rs path. - Introduce OrderId(Felt) - a transparent newtype that PswapNote::order_id and PswapNoteAttachment{::new,::order_id} now use. From/From keep the MASM bridge cheap. - Add PswapNoteStorage::requested_asset_id() returning the existing AssetId type (suffix, prefix). On-chain layout is unchanged - the typed accessor is a Rust-side convenience. - PARENT_ATTACHMENT_DEPTH_OFFSET elevated to pub(super) so the new attachment.rs TryFrom can consume it without duplicating the literal. - Re-export OrderId from crates/miden-standards/src/note/mod.rs. - New unit tests cover OrderId round-trips, the order_id == serial[1] invariant on PswapNote, and the requested_asset_id() packing. Closes group D of issue #3000. --- crates/miden-standards/src/note/mod.rs | 2 +- crates/miden-standards/src/note/pswap.rs | 1439 ----------------- .../src/note/pswap/attachment.rs | 89 + crates/miden-standards/src/note/pswap/mod.rs | 730 +++++++++ .../miden-standards/src/note/pswap/storage.rs | 170 ++ .../miden-standards/src/note/pswap/tests.rs | 549 +++++++ crates/miden-testing/tests/scripts/pswap.rs | 8 +- 7 files changed, 1544 insertions(+), 1443 deletions(-) delete mode 100644 crates/miden-standards/src/note/pswap.rs create mode 100644 crates/miden-standards/src/note/pswap/attachment.rs create mode 100644 crates/miden-standards/src/note/pswap/mod.rs create mode 100644 crates/miden-standards/src/note/pswap/storage.rs create mode 100644 crates/miden-standards/src/note/pswap/tests.rs diff --git a/crates/miden-standards/src/note/mod.rs b/crates/miden-standards/src/note/mod.rs index 6335fcb407..664bde7b89 100644 --- a/crates/miden-standards/src/note/mod.rs +++ b/crates/miden-standards/src/note/mod.rs @@ -22,7 +22,7 @@ mod p2ide; pub use p2ide::{P2ideNote, P2ideNoteStorage}; mod pswap; -pub use pswap::{PswapNote, PswapNoteAttachment, PswapNoteStorage}; +pub use pswap::{OrderId, PswapNote, PswapNoteAttachment, PswapNoteStorage}; mod swap; pub use swap::{SwapNote, SwapNoteStorage}; diff --git a/crates/miden-standards/src/note/pswap.rs b/crates/miden-standards/src/note/pswap.rs deleted file mode 100644 index 96b13f0224..0000000000 --- a/crates/miden-standards/src/note/pswap.rs +++ /dev/null @@ -1,1439 +0,0 @@ -use alloc::vec; -use core::num::NonZeroU32; - -use miden_protocol::account::AccountId; -use miden_protocol::assembly::Path; -use miden_protocol::asset::{Asset, AssetAmount, AssetCallbackFlag, FungibleAsset}; -use miden_protocol::errors::NoteError; -use miden_protocol::note::{ - Note, - NoteAssets, - NoteAttachment, - NoteAttachmentScheme, - NoteAttachments, - NoteRecipient, - NoteScript, - NoteScriptRoot, - NoteStorage, - NoteTag, - NoteType, - PartialNoteMetadata, -}; -use miden_protocol::utils::sync::LazyLock; -use miden_protocol::{Felt, ONE, Word, ZERO}; - -use crate::StandardsLib; -use crate::note::{P2idNoteStorage, StandardNoteAttachment}; - -// NOTE SCRIPT -// ================================================================================================ - -/// Path to the PSWAP note script procedure in the standards library. -const PSWAP_SCRIPT_PATH: &str = "::miden::standards::notes::pswap::main"; - -// Initialize the PSWAP note script only once -static PSWAP_SCRIPT: LazyLock = LazyLock::new(|| { - let standards_lib = StandardsLib::default(); - let path = Path::new(PSWAP_SCRIPT_PATH); - NoteScript::from_library_reference(standards_lib.as_ref(), path) - .expect("Standards library contains PSWAP note script procedure") -}); - -// PSWAP NOTE STORAGE -// ================================================================================================ - -/// Canonical storage representation for a PSWAP note. -/// -/// Maps to the 7-element [`NoteStorage`] layout consumed by the on-chain MASM script: -/// -/// | Slot | Field | -/// |---------|-------| -/// | `[0]` | Requested asset enable_callbacks flag | -/// | `[1]` | Requested asset faucet ID suffix | -/// | `[2]` | Requested asset faucet ID prefix | -/// | `[3]` | Requested asset amount | -/// | `[4]` | Payback note type (0 = private, 1 = public) | -/// | `[5-6]` | Creator account ID (prefix, suffix) | -/// -/// The payback note tag is derived at runtime from the creator account ID -/// (via `note_tag::create_account_target` in MASM) rather than stored. -/// -/// The PSWAP note's own tag is not stored: it lives in the note's metadata and -/// is lifted from there by the on-chain script when a remainder note is created -/// (the asset pair is unchanged, so the tag carries over unchanged). -#[derive(Debug, Clone, PartialEq, Eq, bon::Builder)] -pub struct PswapNoteStorage { - requested_asset: FungibleAsset, - - creator_account_id: AccountId, - - /// Note type of the payback note produced when the pswap is filled. Defaults to - /// [`NoteType::Private`] because the payback carries the fill asset and is typically - /// consumed directly by the creator — a private note is cheaper in fees and bandwidth - /// and offers the same information (the fill amount is already recorded in the - /// executed transaction's output). - #[builder(default = NoteType::Private)] - payback_note_type: NoteType, -} - -impl PswapNoteStorage { - // CONSTANTS - // -------------------------------------------------------------------------------------------- - - /// Expected number of storage items for the PSWAP note. - pub const NUM_STORAGE_ITEMS: usize = 7; - - /// Consumes the storage and returns a PSWAP [`NoteRecipient`] with the provided serial number. - pub fn into_recipient(self, serial_num: Word) -> NoteRecipient { - NoteRecipient::new(serial_num, PswapNote::script(), NoteStorage::from(self)) - } - - // PUBLIC ACCESSORS - // -------------------------------------------------------------------------------------------- - - /// Returns a reference to the requested [`FungibleAsset`]. - pub fn requested_asset(&self) -> &FungibleAsset { - &self.requested_asset - } - - /// Returns the payback note routing tag, derived from the creator's account ID. - pub fn payback_note_tag(&self) -> NoteTag { - NoteTag::with_account_target(self.creator_account_id) - } - - /// Returns the account ID of the note creator. - pub fn creator_account_id(&self) -> AccountId { - self.creator_account_id - } - - /// Returns the [`NoteType`] used when creating the payback note. - pub fn payback_note_type(&self) -> NoteType { - self.payback_note_type - } - - /// Returns the faucet ID of the requested asset. - pub fn requested_faucet_id(&self) -> AccountId { - self.requested_asset.faucet_id() - } - - /// Returns the requested token amount. - pub fn requested_asset_amount(&self) -> AssetAmount { - self.requested_asset.amount() - } -} - -/// Serializes [`PswapNoteStorage`] into a 7-element [`NoteStorage`]. -impl From for NoteStorage { - fn from(storage: PswapNoteStorage) -> Self { - let storage_items = vec![ - // Requested asset (individual felts) [0-3] - Felt::from(storage.requested_asset.callbacks().as_u8()), - storage.requested_asset.faucet_id().suffix(), - storage.requested_asset.faucet_id().prefix().as_felt(), - Felt::from(storage.requested_asset.amount()), - // Payback note type [4] - Felt::from(storage.payback_note_type.as_u8()), - // Creator ID [5-6] - storage.creator_account_id.prefix().as_felt(), - storage.creator_account_id.suffix(), - ]; - NoteStorage::new(storage_items) - .expect("number of storage items should not exceed max storage items") - } -} - -/// Deserializes [`PswapNoteStorage`] from a slice of exactly 7 [`Felt`]s. -impl TryFrom<&[Felt]> for PswapNoteStorage { - type Error = NoteError; - - fn try_from(note_storage: &[Felt]) -> Result { - if note_storage.len() != Self::NUM_STORAGE_ITEMS { - return Err(NoteError::InvalidNoteStorageLength { - expected: Self::NUM_STORAGE_ITEMS, - actual: note_storage.len(), - }); - } - - // Reconstruct requested asset from individual felts: - // [0] = enable_callbacks, [1] = faucet_id_suffix, [2] = faucet_id_prefix, [3] = amount - let callbacks = AssetCallbackFlag::try_from( - u8::try_from(note_storage[0].as_canonical_u64()) - .map_err(|_| NoteError::other("enable_callbacks exceeds u8"))?, - ) - .map_err(|e| NoteError::other_with_source("failed to parse asset callback flag", e))?; - - let faucet_id = AccountId::try_from_elements(note_storage[1], note_storage[2]) - .map_err(|e| NoteError::other_with_source("failed to parse requested faucet ID", e))?; - - let amount = note_storage[3].as_canonical_u64(); - let requested_asset = FungibleAsset::new(faucet_id, amount) - .map_err(|e| NoteError::other_with_source("failed to create requested asset", e))? - .with_callbacks(callbacks); - - // [4] = payback_note_type - let payback_note_type = NoteType::try_from( - u8::try_from(note_storage[4].as_canonical_u64()) - .map_err(|_| NoteError::other("payback_note_type exceeds u8"))?, - ) - .map_err(|e| NoteError::other_with_source("failed to parse payback note type", e))?; - - // [5-6] = creator account ID (prefix, suffix) - let creator_account_id = AccountId::try_from_elements(note_storage[6], note_storage[5]) - .map_err(|e| NoteError::other_with_source("failed to parse creator account ID", e))?; - - Ok(Self { - requested_asset, - creator_account_id, - payback_note_type, - }) - } -} - -// PSWAP NOTE ATTACHMENT -// ================================================================================================ - -/// Typed attachment carried by both PSWAP output notes, encoded as -/// `[amount, order_id, depth, 0]` under [`PswapNote::PSWAP_ATTACHMENT_SCHEME`]. -/// -/// `depth` is [`NonZeroU32`] because attachments are only stamped on payback / remainder notes -/// (depth >= 1); the original PSWAP has no PSWAP-scheme attachment. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct PswapNoteAttachment { - amount: AssetAmount, - order_id: Felt, - depth: NonZeroU32, -} - -impl PswapNoteAttachment { - /// Creates a new [`PswapNoteAttachment`]. Infallible: depth is non-zero by type, and - /// [`AssetAmount`] is pre-validated. - pub fn new(amount: AssetAmount, order_id: Felt, depth: NonZeroU32) -> Self { - Self { amount, order_id, depth } - } - - pub fn amount(&self) -> AssetAmount { - self.amount - } - - pub fn order_id(&self) -> Felt { - self.order_id - } - - pub fn depth(&self) -> NonZeroU32 { - self.depth - } -} - -impl From for NoteAttachment { - fn from(attachment: PswapNoteAttachment) -> Self { - let word = Word::from([ - Felt::from(attachment.amount), - attachment.order_id, - Felt::from(attachment.depth.get()), - ZERO, - ]); - NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word) - } -} - -/// Parses a [`NoteAttachment`] back into a typed [`PswapNoteAttachment`]. -/// -/// Validates that: -/// - the scheme is [`PswapNote::PSWAP_ATTACHMENT_SCHEME`]; -/// - the content is exactly one word (`num_words == 1`); -/// - the word's `amount` slot is a valid [`AssetAmount`]; -/// - the word's `depth` slot fits in a `u32` and is non-zero. -/// -/// The trailing slot (`word[3]`) is not asserted to be zero: PSWAP_ATTACHMENT_SCHEME's -/// canonical encoding leaves it for forward compatibility. -impl TryFrom<&NoteAttachment> for PswapNoteAttachment { - type Error = NoteError; - - fn try_from(attachment: &NoteAttachment) -> Result { - if attachment.attachment_scheme() != PswapNote::PSWAP_ATTACHMENT_SCHEME { - return Err(NoteError::other("attachment scheme is not PSWAP_ATTACHMENT_SCHEME")); - } - let words = attachment.content().as_words(); - if words.len() != 1 { - return Err(NoteError::other("PSWAP attachment must encode exactly one word")); - } - let word = words[0]; - - let amount = AssetAmount::new(word[0].as_canonical_u64()).map_err(|e| { - NoteError::other_with_source("PSWAP attachment amount is not a valid asset amount", e) - })?; - - let depth_raw = word[PswapNote::PARENT_ATTACHMENT_DEPTH_OFFSET].as_canonical_u64(); - let depth_u32 = u32::try_from(depth_raw) - .map_err(|_| NoteError::other("PSWAP attachment depth does not fit in u32"))?; - let depth = NonZeroU32::new(depth_u32) - .ok_or_else(|| NoteError::other("PSWAP attachment depth must be non-zero"))?; - - Ok(Self { amount, order_id: word[1], depth }) - } -} - -// PSWAP NOTE -// ================================================================================================ - -/// A partially-fillable swap note for decentralized asset exchange. -/// -/// A PSWAP note allows a creator to offer one fungible asset in exchange for another. -/// Unlike a regular SWAP note, consumers may fill it partially — the unfilled portion -/// is re-created as a remainder note with an updated serial number, while the creator -/// receives the filled portion via a payback note. -/// -/// The note can be consumed both in local transactions (where the consumer provides -/// fill amounts via note_args) and in network transactions (where note_args default to -/// `[0, 0, 0, 0]`, triggering a full fill). To route a PSWAP note to a network account, -/// set the `attachment` to a [`NetworkAccountTarget`](crate::note::NetworkAccountTarget) -/// via the builder. -#[derive(Debug, Clone, bon::Builder)] -#[builder(finish_fn(vis = "", name = build_internal))] -pub struct PswapNote { - sender: AccountId, - storage: PswapNoteStorage, - serial_number: Word, - - #[builder(default = NoteType::Private)] - note_type: NoteType, - - offered_asset: FungibleAsset, - - attachment: Option, -} - -impl PswapNoteBuilder -where - S: pswap_note_builder::IsComplete, -{ - /// Validates and builds the [`PswapNote`]. - /// - /// # Errors - /// - /// Returns an error if the offered and requested assets have the same faucet ID. - pub fn build(self) -> Result { - let note = self.build_internal(); - - if note.offered_asset.faucet_id() == note.storage.requested_faucet_id() { - return Err(NoteError::other( - "offered and requested assets must have different faucets", - )); - } - - Ok(note) - } -} - -impl PswapNote { - // CONSTANTS - // -------------------------------------------------------------------------------------------- - - /// Expected number of storage items for the PSWAP note. - pub const NUM_STORAGE_ITEMS: usize = PswapNoteStorage::NUM_STORAGE_ITEMS; - - /// Attachment scheme stamped on both PSWAP output notes (the payback P2ID and the - /// remainder PSWAP). - pub const PSWAP_ATTACHMENT_SCHEME: NoteAttachmentScheme = - StandardNoteAttachment::PswapAttachment.attachment_scheme(); - - /// Offset of the `depth` field within the [`Self::PSWAP_ATTACHMENT_SCHEME`] word. - const PARENT_ATTACHMENT_DEPTH_OFFSET: usize = 2; - - // PUBLIC ACCESSORS - // -------------------------------------------------------------------------------------------- - - /// Returns the compiled PSWAP note script. - pub fn script() -> NoteScript { - PSWAP_SCRIPT.clone() - } - - /// Returns the root hash of the PSWAP note script. - pub fn script_root() -> NoteScriptRoot { - PSWAP_SCRIPT.root() - } - - /// Builds the `NOTE_ARGS` word that the PSWAP script expects when a consumer wants to fill - /// part of the swap: `[account_fill, note_fill, 0, 0]`. - /// - /// - `account_fill` is the portion of the requested asset the consumer pays out of their own - /// vault. - /// - `note_fill` is the portion sourced from another note in the same transaction (cross-swap - /// / net-zero flow). - /// - /// Both values are in the requested asset's base units. In a network transaction the kernel - /// defaults `NOTE_ARGS` to `[0, 0, 0, 0]` and the script falls back to a full fill, so this - /// helper is only needed for local transactions where the consumer chooses the fill split. - /// - /// Infallible: [`AssetAmount`] is bounded by `2^63 - 2^31`, which fits in a [`Felt`]. - pub fn create_args(account_fill: AssetAmount, note_fill: AssetAmount) -> Word { - Word::from([Felt::from(account_fill), Felt::from(note_fill), ZERO, ZERO]) - } - - /// Returns the account ID of the note sender. - pub fn sender(&self) -> AccountId { - self.sender - } - - /// Returns a reference to the PSWAP note storage. - pub fn storage(&self) -> &PswapNoteStorage { - &self.storage - } - - /// Returns the serial number of this note. - pub fn serial_number(&self) -> Word { - self.serial_number - } - - /// Returns the note type (public or private). - pub fn note_type(&self) -> NoteType { - self.note_type - } - - /// Returns a reference to the offered [`FungibleAsset`]. - pub fn offered_asset(&self) -> &FungibleAsset { - &self.offered_asset - } - - /// Returns a reference to the note attachments. - /// - /// For notes targeting a network account, this may contain a - /// [`NetworkAccountTarget`](crate::note::NetworkAccountTarget) with scheme = 2. For a - /// remainder PSWAP this contains the [`Self::PSWAP_ATTACHMENT_SCHEME`] word - /// `[amt_payout, order_id, depth, 0]`. For an original PSWAP (no prior fill), - /// this is typically empty. - pub fn attachments(&self) -> Option<&NoteAttachment> { - self.attachment.as_ref() - } - - /// Returns the order_id of this lineage, equal to `serial_number()[1]`. - pub fn order_id(&self) -> Felt { - self.serial_number[1] - } - - /// Returns the depth carried in this note's [`Self::PSWAP_ATTACHMENT_SCHEME`] attachment, - /// or 0 if the note has no such attachment (i.e., it is the original PSWAP, not a - /// remainder produced by an earlier fill). - /// - /// The next round's `current_depth` is computed as `parent_depth() + 1`, matching the - /// on-chain `get_current_depth` MASM procedure. Use [`Self::next_depth`] for the typed - /// [`NonZeroU32`] form. A malformed `PSWAP_ATTACHMENT_SCHEME` attachment (depth out of - /// `u32` range, depth == 0, etc.) is treated as if no attachment is present, so discovery - /// degrades to "looks like an original" rather than corrupting downstream arithmetic. - pub fn parent_depth(&self) -> u32 { - self.attachment - .as_ref() - .and_then(|att| PswapNoteAttachment::try_from(att).ok()) - .map(|att| att.depth().get()) - .unwrap_or(0) - } - - /// Returns the depth that the next-round payback / remainder should carry, equal to - /// `parent_depth() + 1`. Always non-zero by construction. - /// - /// # Errors - /// - /// Returns an error if `parent_depth()` is [`u32::MAX`]. - pub fn next_depth(&self) -> Result { - self.parent_depth() - .checked_add(1) - .and_then(NonZeroU32::new) - .ok_or_else(|| NoteError::other("PSWAP depth overflow")) - } - - // INSTANCE METHODS - // -------------------------------------------------------------------------------------------- - - /// Executes the swap as a full fill, producing only the payback note (no remainder). - /// - /// Equivalent to calling [`Self::execute`] with `account_fill_asset` set to the full - /// requested amount and `note_fill_asset = None`. It also matches the on-chain - /// behavior when a note is consumed without explicit `note_args` (e.g. in a network - /// transaction, where the kernel defaults `note_args` to `[0, 0, 0, 0]` and the MASM - /// script falls back to a full fill). - pub fn execute_full_fill(&self, consumer_account_id: AccountId) -> Result { - let requested_faucet_id = self.storage.requested_faucet_id(); - let total_requested_amount = self.storage.requested_asset_amount(); - - let fill_asset = - FungibleAsset::new(requested_faucet_id, total_requested_amount.as_u64()) - .map_err(|e| NoteError::other_with_source("failed to create full fill asset", e))? - .with_callbacks(self.storage.requested_asset().callbacks()); - - self.create_payback_note(consumer_account_id, fill_asset, total_requested_amount.as_u64()) - } - - /// Executes the swap, producing the output notes for a given fill. - /// - /// `account_fill_asset` is debited from the consumer's vault; `note_fill_asset` arrives - /// from another note in the same transaction (cross-swap). At least one must be - /// provided. - /// - /// Returns `(payback_note, Option)`. The remainder is - /// `None` when the fill equals the total requested amount (full fill). - /// - /// # Errors - /// - /// Returns an error if: - /// - Both assets are `None`. - /// - The fill amount is zero. - /// - The fill amount exceeds the total requested amount. - pub fn execute( - &self, - consumer_account_id: AccountId, - account_fill_asset: Option, - note_fill_asset: Option, - ) -> Result<(Note, Option), NoteError> { - // Combine account fill and note fill into a single payback asset. - let payback_asset = match (account_fill_asset, note_fill_asset) { - (Some(account_fill), Some(note_fill)) => account_fill.add(note_fill).map_err(|e| { - NoteError::other_with_source( - "failed to combine account fill and note fill assets", - e, - ) - })?, - (Some(asset), None) | (None, Some(asset)) => asset, - (None, None) => { - return Err(NoteError::other( - "at least one of account_fill_asset or note_fill_asset must be provided", - )); - }, - }; - let fill_amount = payback_asset.amount().as_u64(); - - let total_offered_amount = self.offered_asset.amount().as_u64(); - let requested_faucet_id = self.storage.requested_faucet_id(); - let total_requested_amount = self.storage.requested_asset_amount().as_u64(); - - // Validate fill amount - if fill_amount == 0 { - return Err(NoteError::other("Fill amount must be greater than 0")); - } - if fill_amount > total_requested_amount { - return Err(NoteError::other(alloc::format!( - "Fill amount {} exceeds requested amount {}", - fill_amount, - total_requested_amount - ))); - } - - // Calculate payout amounts separately for account fill and note fill, matching the - // MASM which calls calculate_tokens_offered_for_requested twice. This is necessary - // because the account fill portion goes to the consumer's vault while the total - // determines the remainder note's offered amount. - let account_fill_amount = account_fill_asset.as_ref().map_or(0, |a| a.amount().as_u64()); - let note_fill_amount = note_fill_asset.as_ref().map_or(0, |a| a.amount().as_u64()); - let payout_for_account_fill = Self::calculate_output_amount( - total_offered_amount, - total_requested_amount, - account_fill_amount, - )?; - let payout_for_note_fill = Self::calculate_output_amount( - total_offered_amount, - total_requested_amount, - note_fill_amount, - )?; - let offered_amount_for_fill = payout_for_account_fill + payout_for_note_fill; - - let payback_note = - self.create_payback_note(consumer_account_id, payback_asset, fill_amount)?; - - // Create remainder note if partial fill - let remainder = if fill_amount < total_requested_amount { - let remaining_offered = total_offered_amount - offered_amount_for_fill; - let remaining_requested = total_requested_amount - fill_amount; - - let remaining_offered_asset = - FungibleAsset::new(self.offered_asset.faucet_id(), remaining_offered) - .map_err(|e| { - NoteError::other_with_source("failed to create remainder asset", e) - })? - .with_callbacks(self.offered_asset.callbacks()); - - let remaining_requested_asset = - FungibleAsset::new(requested_faucet_id, remaining_requested) - .map_err(|e| { - NoteError::other_with_source( - "failed to create remaining requested asset", - e, - ) - })? - .with_callbacks(self.storage.requested_asset().callbacks()); - - Some(self.create_remainder_pswap_note( - consumer_account_id, - remaining_offered_asset, - remaining_requested_asset, - offered_amount_for_fill, - )?) - } else { - None - }; - - Ok((payback_note, remainder)) - } - - /// Returns how many offered tokens a consumer receives for `fill_amount` of the - /// requested asset, based on this note's current offered/requested ratio. - /// - /// # Errors - /// - /// Returns an error if the calculated payout is not a valid asset amount. - pub fn calculate_offered_for_requested(&self, fill_amount: u64) -> Result { - let total_requested = self.storage.requested_asset_amount().as_u64(); - let total_offered = self.offered_asset.amount().as_u64(); - - Self::calculate_output_amount(total_offered, total_requested, fill_amount) - } - - // LINEAGE DISCOVERY - // -------------------------------------------------------------------------------------------- - - /// Reconstructs the depth-`d` payback P2ID [`Note`], so the creator can consume it as an - /// unauthenticated input note. - /// - /// `consumer_account_id` must be the account that consumed the parent PSWAP in round - /// `depth`: the MASM stamps it as the payback's metadata sender, which feeds into - /// [`Note::details_commitment`]. - /// - /// # Errors - /// - /// Returns an error if the fill amount is not a valid asset amount. - pub fn payback_note( - &self, - consumer_account_id: AccountId, - attachment: &PswapNoteAttachment, - ) -> Result { - let depth = attachment.depth().get(); - let parent_depth = Felt::from(depth - 1); - let p2id_serial = Word::from([ - self.serial_number[0] + ONE, - self.serial_number[1], - self.serial_number[2], - self.serial_number[3] + parent_depth, - ]); - - let recipient = - P2idNoteStorage::new(self.storage.creator_account_id).into_recipient(p2id_serial); - - let fill_asset = - FungibleAsset::new(self.storage.requested_faucet_id(), u64::from(attachment.amount())) - .map_err(|e| NoteError::other_with_source("invalid fill amount", e))? - .with_callbacks(self.storage.requested_asset().callbacks()); - let assets = NoteAssets::new(vec![fill_asset.into()])?; - - let metadata = - PartialNoteMetadata::new(consumer_account_id, self.storage.payback_note_type) - .with_tag(self.storage.payback_note_tag()); - - Ok(Note::with_attachments( - assets, - metadata, - recipient, - NoteAttachments::from(NoteAttachment::from(*attachment)), - )) - } - - /// Reconstructs the depth-`d` remainder PSWAP [`Note`] in this lineage. - /// - /// Called on the original PSWAP, this returns the full Note for the remainder produced - /// in round `depth`. The returned Note matches the created note exactly. - /// - /// - `consumer_account_id` — the account that consumed the parent PSWAP in round `depth`, used - /// as the remainder's sender. - /// - `attachment` — the on-chain `[amount, order_id, depth, 0]` attachment for this round, - /// where `amount` is the offered-asset units paid out. - /// - `remaining_offered` / `remaining_requested` — the leftover amounts that survive into this - /// remainder. Both are required because the price formula uses floor division, so one isn't - /// derivable from the other across rounds in general. - /// - /// # Errors - /// - /// Returns an error if any amount is not a valid asset amount. - pub fn remainder_note( - &self, - consumer_account_id: AccountId, - attachment: &PswapNoteAttachment, - remaining_offered: AssetAmount, - remaining_requested: AssetAmount, - ) -> Result { - let depth = attachment.depth().get(); - let remainder_serial = Word::from([ - self.serial_number[0], - self.serial_number[1], - self.serial_number[2], - self.serial_number[3] + Felt::from(depth), - ]); - - let requested_asset = - FungibleAsset::new(self.storage.requested_faucet_id(), u64::from(remaining_requested)) - .map_err(|e| NoteError::other_with_source("invalid remaining_requested amount", e))? - .with_callbacks(self.storage.requested_asset().callbacks()); - let offered_asset = - FungibleAsset::new(self.offered_asset.faucet_id(), u64::from(remaining_offered)) - .map_err(|e| NoteError::other_with_source("invalid remaining_offered amount", e))? - .with_callbacks(self.offered_asset.callbacks()); - - let new_storage = PswapNoteStorage::builder() - .requested_asset(requested_asset) - .creator_account_id(self.storage.creator_account_id) - .payback_note_type(self.storage.payback_note_type) - .build(); - let recipient = new_storage.into_recipient(remainder_serial); - - let assets = NoteAssets::new(vec![offered_asset.into()])?; - - let tag = Self::create_tag(self.note_type, &offered_asset, &requested_asset); - let metadata = PartialNoteMetadata::new(consumer_account_id, self.note_type).with_tag(tag); - - Ok(Note::with_attachments( - assets, - metadata, - recipient, - NoteAttachments::from(NoteAttachment::from(*attachment)), - )) - } - - // ASSOCIATED FUNCTIONS - // -------------------------------------------------------------------------------------------- - - /// Builds the 32-bit [`NoteTag`] for a PSWAP note. - /// - /// ```text - /// [31..30] note_type (2 bits) - /// [29..16] script_root MSBs (14 bits) - /// [15..8] offered faucet ID (8 bits, top byte of prefix) - /// [7..0] requested faucet ID (8 bits, top byte of prefix) - /// ``` - pub fn create_tag( - note_type: NoteType, - offered_asset: &FungibleAsset, - requested_asset: &FungibleAsset, - ) -> NoteTag { - let pswap_root_bytes = Self::script().root().as_bytes(); - - // Construct the pswap use case ID from the 14 most significant bits of the script root. - // This leaves the two most significant bits zero. - let mut pswap_use_case_id = (pswap_root_bytes[0] as u16) << 6; - pswap_use_case_id |= (pswap_root_bytes[1] >> 2) as u16; - - // Get bits 0..8 from the faucet IDs of both assets which will form the tag payload. - let offered_asset_id: u64 = offered_asset.faucet_id().prefix().into(); - let offered_asset_tag = (offered_asset_id >> 56) as u8; - - let requested_asset_id: u64 = requested_asset.faucet_id().prefix().into(); - let requested_asset_tag = (requested_asset_id >> 56) as u8; - - let asset_pair = ((offered_asset_tag as u16) << 8) | (requested_asset_tag as u16); - - let tag = ((note_type as u8 as u32) << 30) - | ((pswap_use_case_id as u32) << 16) - | asset_pair as u32; - - NoteTag::new(tag) - } - - /// Computes `floor((offered_total * fill_amount) / requested_total)` via a - /// u128 intermediate, mirroring `u64::widening_mul` + `u128::div` on the - /// MASM side. - /// - /// # Errors - /// - /// Returns an error if the result does not fit in a valid [`AssetAmount`]. - fn calculate_output_amount( - offered_total: u64, - requested_total: u64, - fill_amount: u64, - ) -> Result { - let product = (offered_total as u128) * (fill_amount as u128); - let quotient = product / (requested_total as u128); - let amount = u64::try_from(quotient) - .map_err(|_| NoteError::other("payout quotient does not fit in u64"))?; - // Validate the result is a valid fungible asset amount. - AssetAmount::new(amount).map_err(|e| { - NoteError::other_with_source("payout amount exceeds max fungible asset amount", e) - })?; - Ok(amount) - } - - - /// Builds a payback note (P2ID) that delivers the filled assets to the swap creator. - /// - /// The note inherits its type (public/private) from this PSWAP note and derives a - /// deterministic serial number by incrementing the least significant element of the - /// serial number (`serial[0] + 1`). - /// - /// The attachment carries `[fill_amount, order_id, current_depth, 0]` under - /// [`Self::PSWAP_ATTACHMENT_SCHEME`]. `current_depth` is `parent_depth + 1` — i.e., - /// the round number that produced this payback (1-indexed). - fn create_payback_note( - &self, - consumer_account_id: AccountId, - payback_asset: FungibleAsset, - fill_amount: u64, - ) -> Result { - let payback_note_tag = self.storage.payback_note_tag(); - // Derive P2ID serial: increment least significant element (matching MASM add.1) - let p2id_serial_num = Word::from([ - self.serial_number[0] + ONE, - self.serial_number[1], - self.serial_number[2], - self.serial_number[3], - ]); - - // P2ID recipient targets the creator - let recipient = - P2idNoteStorage::new(self.storage.creator_account_id).into_recipient(p2id_serial_num); - - let current_depth = self.next_depth()?; - let fill_amount_typed = AssetAmount::new(fill_amount).map_err(|e| { - NoteError::other_with_source("fill amount is not a valid asset amount", e) - })?; - let attachment: NoteAttachment = - PswapNoteAttachment::new(fill_amount_typed, self.order_id(), current_depth).into(); - - let p2id_assets = NoteAssets::new(vec![payback_asset.into()])?; - let p2id_metadata = - PartialNoteMetadata::new(consumer_account_id, self.storage.payback_note_type) - .with_tag(payback_note_tag); - - Ok(Note::with_attachments( - p2id_assets, - p2id_metadata, - recipient, - NoteAttachments::from(attachment), - )) - } - - /// Builds a remainder PSWAP note carrying the unfilled portion of the swap. - /// - /// The remainder inherits the original creator, tags, and note type, with an updated - /// serial number (`serial[3] + 1`). - /// - /// The attachment carries `[offered_amount_for_fill, order_id, current_depth, 0]` under - /// [`Self::PSWAP_ATTACHMENT_SCHEME`]. The remainder must carry this attachment so that - /// when *it* is later consumed as a parent, `get_current_depth` reads the right scheme - /// and increments depth correctly. - fn create_remainder_pswap_note( - &self, - consumer_account_id: AccountId, - remaining_offered_asset: FungibleAsset, - remaining_requested_asset: FungibleAsset, - offered_amount_for_fill: u64, - ) -> Result { - let new_storage = PswapNoteStorage::builder() - .requested_asset(remaining_requested_asset) - .creator_account_id(self.storage.creator_account_id) - .payback_note_type(self.storage.payback_note_type) - .build(); - - // Remainder serial: increment most significant element (matching MASM movup.3 add.1 - // movdn.3) - let remainder_serial_num = Word::from([ - self.serial_number[0], - self.serial_number[1], - self.serial_number[2], - self.serial_number[3] + ONE, - ]); - - let current_depth = self.next_depth()?; - let payout_typed = AssetAmount::new(offered_amount_for_fill).map_err(|e| { - NoteError::other_with_source("payout amount is not a valid asset amount", e) - })?; - let attachment: NoteAttachment = - PswapNoteAttachment::new(payout_typed, self.order_id(), current_depth).into(); - - PswapNote::builder() - .sender(consumer_account_id) - .storage(new_storage) - .serial_number(remainder_serial_num) - .note_type(self.note_type) - .offered_asset(remaining_offered_asset) - .attachment(attachment) - .build() - } -} - -// CONVERSIONS -// ================================================================================================ - -/// Converts a [`PswapNote`] into a protocol [`Note`], computing the final PSWAP tag. -impl From for Note { - fn from(pswap: PswapNote) -> Self { - let tag = PswapNote::create_tag( - pswap.note_type, - &pswap.offered_asset, - pswap.storage.requested_asset(), - ); - - let recipient = pswap.storage.into_recipient(pswap.serial_number); - - let assets = NoteAssets::new(vec![pswap.offered_asset.into()]) - .expect("single fungible asset should be valid"); - - let metadata = PartialNoteMetadata::new(pswap.sender, pswap.note_type).with_tag(tag); - - let attachments = pswap.attachment.map(NoteAttachments::from).unwrap_or_default(); - - Note::with_attachments(assets, metadata, recipient, attachments) - } -} - -/// Parses a protocol [`Note`] back into a [`PswapNote`] by deserializing its storage. -impl TryFrom<&Note> for PswapNote { - type Error = NoteError; - - fn try_from(note: &Note) -> Result { - if note.recipient().script().root() != PswapNote::script_root() { - return Err(NoteError::other("note script root does not match PSWAP script root")); - } - - let storage = PswapNoteStorage::try_from(note.recipient().storage().items())?; - - if note.assets().num_assets() != 1 { - return Err(NoteError::other("PSWAP note must have exactly one asset")); - } - let offered_asset = match note.assets().iter().next().unwrap() { - Asset::Fungible(fa) => *fa, - Asset::NonFungible(_) => { - return Err(NoteError::other("PSWAP note asset must be fungible")); - }, - }; - - let attachment = match note.attachments().num_attachments() { - 0 => None, - 1 => { - Some(note.attachments().get(0).expect("length should have been validated").clone()) - }, - _ => return Err(NoteError::other("pswap note supports only one attachment")), - }; - - PswapNote::builder() - .sender(note.metadata().sender()) - .storage(storage) - .serial_number(note.recipient().serial_num()) - .note_type(note.metadata().note_type()) - .offered_asset(offered_asset) - .maybe_attachment(attachment) - .build() - } -} - -// TESTS -// ================================================================================================ - -#[cfg(test)] -mod tests { - use miden_protocol::account::{AccountId, AccountIdVersion, AccountType}; - use miden_protocol::asset::FungibleAsset; - use miden_protocol::crypto::rand::{FeltRng, RandomCoin}; - - use super::*; - - // TEST HELPERS - // -------------------------------------------------------------------------------------------- - - fn dummy_faucet_id(byte: u8) -> AccountId { - let mut bytes = [0; 15]; - bytes[0] = byte; - AccountId::dummy(bytes, AccountIdVersion::Version1, AccountType::Public) - } - - fn dummy_creator_id() -> AccountId { - AccountId::dummy([1; 15], AccountIdVersion::Version1, AccountType::Public) - } - - fn dummy_consumer_id() -> AccountId { - AccountId::dummy([2; 15], AccountIdVersion::Version1, AccountType::Public) - } - - fn build_pswap_note( - offered_asset: FungibleAsset, - requested_asset: FungibleAsset, - creator_id: AccountId, - ) -> (PswapNote, Note) { - let mut rng = RandomCoin::new(Word::default()); - let storage = PswapNoteStorage::builder() - .requested_asset(requested_asset) - .creator_account_id(creator_id) - .build(); - let pswap = PswapNote::builder() - .sender(creator_id) - .storage(storage) - .serial_number(rng.draw_word()) - .note_type(NoteType::Public) - .offered_asset(offered_asset) - .build() - .unwrap(); - let note: Note = pswap.clone().into(); - (pswap, note) - } - - // TESTS - // -------------------------------------------------------------------------------------------- - - #[test] - fn pswap_note_creation_and_script() { - let creator_id = dummy_creator_id(); - let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 1000).unwrap(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 500).unwrap(); - - let (pswap, note) = build_pswap_note(offered_asset, requested_asset, creator_id); - - assert_eq!(pswap.sender(), creator_id); - assert_eq!(pswap.note_type(), NoteType::Public); - - let script = PswapNote::script(); - assert!(Word::from(script.root()) != Word::default(), "Script root should not be zero"); - assert_eq!(note.metadata().sender(), creator_id); - assert_eq!(note.metadata().note_type(), NoteType::Public); - assert_eq!(note.assets().num_assets(), 1); - assert_eq!(note.recipient().script().root(), script.root()); - assert_eq!( - note.recipient().storage().num_items(), - PswapNoteStorage::NUM_STORAGE_ITEMS as u16, - ); - } - - #[test] - fn pswap_note_builder() { - let creator_id = dummy_creator_id(); - let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 1000).unwrap(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 500).unwrap(); - - let (pswap, note) = build_pswap_note(offered_asset, requested_asset, creator_id); - - assert_eq!(pswap.sender(), creator_id); - assert_eq!(pswap.note_type(), NoteType::Public); - assert_eq!(note.metadata().sender(), creator_id); - assert_eq!(note.metadata().note_type(), NoteType::Public); - assert_eq!(note.assets().num_assets(), 1); - assert_eq!( - note.recipient().storage().num_items(), - PswapNoteStorage::NUM_STORAGE_ITEMS as u16, - ); - } - - #[test] - fn pswap_tag() { - let mut offered_faucet_bytes = [0; 15]; - offered_faucet_bytes[0] = 0xcd; - offered_faucet_bytes[1] = 0xb1; - - let mut requested_faucet_bytes = [0; 15]; - requested_faucet_bytes[0] = 0xab; - requested_faucet_bytes[1] = 0xec; - - let offered_asset = FungibleAsset::new( - AccountId::dummy(offered_faucet_bytes, AccountIdVersion::Version1, AccountType::Public), - 100, - ) - .unwrap(); - let requested_asset = FungibleAsset::new( - AccountId::dummy( - requested_faucet_bytes, - AccountIdVersion::Version1, - AccountType::Public, - ), - 200, - ) - .unwrap(); - - let tag = PswapNote::create_tag(NoteType::Public, &offered_asset, &requested_asset); - let tag_u32 = u32::from(tag); - - // Verify note_type bits (top 2 bits should be 10 for Public) - let note_type_bits = tag_u32 >> 30; - assert_eq!(note_type_bits, NoteType::Public as u32); - } - - #[test] - fn calculate_output_amount() { - assert_eq!(PswapNote::calculate_output_amount(100, 100, 50).unwrap(), 50); // Equal ratio - assert_eq!(PswapNote::calculate_output_amount(200, 100, 50).unwrap(), 100); // 2:1 ratio - assert_eq!(PswapNote::calculate_output_amount(100, 200, 50).unwrap(), 25); // 1:2 ratio - - // Non-integer ratio (100/73) - let result = PswapNote::calculate_output_amount(100, 73, 7).unwrap(); - assert!(result > 0, "Should produce non-zero output"); - } - - #[test] - fn pswap_note_storage_try_from() { - let creator_id = dummy_creator_id(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 500).unwrap(); - - let storage_items = vec![ - Felt::from(requested_asset.callbacks().as_u8()), - requested_asset.faucet_id().suffix(), - requested_asset.faucet_id().prefix().as_felt(), - Felt::from(requested_asset.amount()), - Felt::from(NoteType::Private.as_u8()), // payback_note_type - creator_id.prefix().as_felt(), - creator_id.suffix(), - ]; - - let parsed = PswapNoteStorage::try_from(storage_items.as_slice()).unwrap(); - assert_eq!(parsed.creator_account_id(), creator_id); - assert_eq!(parsed.requested_asset_amount(), AssetAmount::new(500).unwrap()); - } - - #[test] - fn pswap_note_storage_roundtrip() { - let creator_id = dummy_creator_id(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 500).unwrap(); - - let storage = PswapNoteStorage::builder() - .requested_asset(requested_asset) - .creator_account_id(creator_id) - .build(); - - let note_storage = NoteStorage::from(storage.clone()); - let parsed = PswapNoteStorage::try_from(note_storage.items()).unwrap(); - - assert_eq!(parsed.creator_account_id(), creator_id); - assert_eq!(parsed.requested_asset_amount(), AssetAmount::new(500).unwrap()); - } - - /// Consumer supplies both an account fill and a note fill, and the sum is below - /// the requested amount → `execute` must combine them into a single payback note - /// carrying account_fill+note_fill of the requested asset and emit a remainder - /// pswap note for the unfilled portion. - #[test] - fn pswap_execute_combined_account_fill_and_note_fill_partial_fill() { - let creator_id = dummy_creator_id(); - let consumer_id = dummy_consumer_id(); - let offered_faucet = dummy_faucet_id(0xaa); - let requested_faucet = dummy_faucet_id(0xbb); - - // Offer 100 offered, request 50 requested → 2:1 ratio. - let offered_asset = FungibleAsset::new(offered_faucet, 100).unwrap(); - let requested_asset = FungibleAsset::new(requested_faucet, 50).unwrap(); - let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); - - // Account fill = 10, note fill = 20 → total fill = 30 (< 50, so partial). - let account_fill = FungibleAsset::new(requested_faucet, 10).unwrap(); - let note_fill = FungibleAsset::new(requested_faucet, 20).unwrap(); - - let (payback, remainder) = - pswap.execute(consumer_id, Some(account_fill), Some(note_fill)).unwrap(); - - // Payback note must carry the combined 30 of requested asset. - assert_eq!(payback.assets().num_assets(), 1); - let payback_asset = payback.assets().iter().next().unwrap(); - let Asset::Fungible(fa) = payback_asset else { - panic!("expected fungible payback asset"); - }; - assert_eq!(fa.faucet_id(), requested_faucet); - assert_eq!(fa.amount().as_u64(), 30); - - // Remainder must exist with the unfilled 50 - 30 = 20 of requested, and the - // offered amount reduced proportionally (100 - 30*2 = 40). - let remainder = remainder.expect("partial fill should produce remainder"); - assert_eq!(remainder.storage().requested_asset_amount(), AssetAmount::new(20).unwrap()); - assert_eq!(remainder.offered_asset().amount().as_u64(), 40); - assert_eq!(remainder.storage().creator_account_id(), creator_id); - } - - /// Consumer supplies both an account fill and a note fill, and the sum exactly - /// matches the requested amount → `execute` must produce a single payback note for - /// the full amount and no remainder. - #[test] - fn pswap_execute_combined_account_fill_and_note_fill_full_fill() { - let creator_id = dummy_creator_id(); - let consumer_id = dummy_consumer_id(); - let offered_faucet = dummy_faucet_id(0xaa); - let requested_faucet = dummy_faucet_id(0xbb); - - let offered_asset = FungibleAsset::new(offered_faucet, 100).unwrap(); - let requested_asset = FungibleAsset::new(requested_faucet, 50).unwrap(); - let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); - - // Account fill = 30, note fill = 20 → total fill = 50 (exactly requested). - let account_fill = FungibleAsset::new(requested_faucet, 30).unwrap(); - let note_fill = FungibleAsset::new(requested_faucet, 20).unwrap(); - - let (payback, remainder) = - pswap.execute(consumer_id, Some(account_fill), Some(note_fill)).unwrap(); - - // Payback note must carry the full 50 of requested asset. - assert_eq!(payback.assets().num_assets(), 1); - let payback_asset = payback.assets().iter().next().unwrap(); - let Asset::Fungible(fa) = payback_asset else { - panic!("expected fungible payback asset"); - }; - assert_eq!(fa.faucet_id(), requested_faucet); - assert_eq!(fa.amount().as_u64(), 50); - - // Full fill → no remainder note. - assert!(remainder.is_none(), "full fill must not produce a remainder"); - } - - /// Regression for the silent `AssetCallbackFlag` drop: when the PSWAP's requested or - /// offered asset carries `Enabled` callbacks, the on-chain MASM preserves that flag - /// on every output note's asset. The Rust-side `execute`, `payback_note`, and - /// `remainder_note` must do the same — otherwise the reconstructed `Note::details_commitment` - /// diverges from the on-chain leaf and the unauthenticated consume path fails. - #[test] - fn pswap_output_assets_preserve_callback_flag() { - let creator_id = dummy_creator_id(); - let consumer_id = dummy_consumer_id(); - let offered_faucet = dummy_faucet_id(0xaa); - let requested_faucet = dummy_faucet_id(0xbb); - - let offered_asset = FungibleAsset::new(offered_faucet, 100) - .unwrap() - .with_callbacks(AssetCallbackFlag::Enabled); - let requested_asset = FungibleAsset::new(requested_faucet, 50) - .unwrap() - .with_callbacks(AssetCallbackFlag::Enabled); - let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); - - // --- execute() (partial fill) --- - let account_fill = FungibleAsset::new(requested_faucet, 20) - .unwrap() - .with_callbacks(AssetCallbackFlag::Enabled); - let (payback, remainder) = pswap.execute(consumer_id, Some(account_fill), None).unwrap(); - - let Asset::Fungible(fa) = payback.assets().iter().next().unwrap() else { - panic!("expected fungible payback asset"); - }; - assert_eq!(fa.callbacks(), AssetCallbackFlag::Enabled); - - let remainder = remainder.expect("partial fill should produce remainder"); - assert_eq!( - remainder.offered_asset().callbacks(), - AssetCallbackFlag::Enabled, - "remainder offered asset must inherit callbacks", - ); - assert_eq!( - remainder.storage().requested_asset().callbacks(), - AssetCallbackFlag::Enabled, - "remainder storage's requested asset must inherit callbacks", - ); - - // --- payback_note() reconstruction --- - let depth_one = NonZeroU32::new(1).unwrap(); - let payback_attachment = - PswapNoteAttachment::new(AssetAmount::new(20).unwrap(), pswap.order_id(), depth_one); - let reconstructed_payback = pswap.payback_note(consumer_id, &payback_attachment).unwrap(); - let Asset::Fungible(fa) = reconstructed_payback.assets().iter().next().unwrap() else { - panic!("expected fungible payback asset"); - }; - assert_eq!( - fa.callbacks(), - AssetCallbackFlag::Enabled, - "payback_note must preserve requested asset's callback flag", - ); - - // --- remainder_note() reconstruction --- - let remainder_attachment = - PswapNoteAttachment::new(AssetAmount::new(40).unwrap(), pswap.order_id(), depth_one); - let reconstructed_remainder = pswap - .remainder_note( - consumer_id, - &remainder_attachment, - AssetAmount::new(60).unwrap(), - AssetAmount::new(30).unwrap(), - ) - .unwrap(); - let Asset::Fungible(fa) = reconstructed_remainder.assets().iter().next().unwrap() else { - panic!("expected fungible remainder asset"); - }; - assert_eq!( - fa.callbacks(), - AssetCallbackFlag::Enabled, - "remainder_note must preserve offered asset's callback flag", - ); - } - - /// `create_args` is infallible at the type level because `AssetAmount` fits in a `Felt`. - /// `MAX` round-trips through the resulting word. - #[test] - fn create_args_round_trips_max_asset_amount() { - let args = PswapNote::create_args(AssetAmount::MAX, AssetAmount::ZERO); - assert_eq!(args[0], Felt::from(AssetAmount::MAX)); - assert_eq!(args[1], ZERO); - assert_eq!(args[2], ZERO); - assert_eq!(args[3], ZERO); - } - - /// `PswapNoteAttachment` accessors mirror what was passed to `new`. - #[test] - fn pswap_note_attachment_accessors() { - let order_id = Felt::from(42u32); - let depth = NonZeroU32::new(3).unwrap(); - let attachment = PswapNoteAttachment::new(AssetAmount::new(100).unwrap(), order_id, depth); - assert_eq!(attachment.amount(), AssetAmount::new(100).unwrap()); - assert_eq!(attachment.order_id(), order_id); - assert_eq!(attachment.depth(), depth); - } - - /// `From for NoteAttachment` encodes the depth via `.get()`. - #[test] - fn pswap_note_attachment_encodes_depth_via_get() { - let depth = NonZeroU32::new(7).unwrap(); - let attachment = - PswapNoteAttachment::new(AssetAmount::new(50).unwrap(), Felt::from(9u32), depth); - let note_att: NoteAttachment = attachment.into(); - let word = note_att.content().as_words()[0]; - assert_eq!(word[2], Felt::from(7u32)); - } - - /// `parent_depth` returns 0 when the note has no attachment. - #[test] - fn parent_depth_zero_when_no_attachment() { - let creator_id = dummy_creator_id(); - let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); - let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); - assert_eq!(pswap.parent_depth(), 0); - assert_eq!(pswap.next_depth().unwrap().get(), 1); - } - - /// `parent_depth` returns the stored depth when the note carries a PSWAP attachment. - #[test] - fn parent_depth_reads_attachment_depth() { - let creator_id = dummy_creator_id(); - let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); - let mut rng = RandomCoin::new(Word::default()); - let storage = PswapNoteStorage::builder() - .requested_asset(requested_asset) - .creator_account_id(creator_id) - .build(); - let order_id = Felt::from(1u32); - let attachment = PswapNoteAttachment::new( - AssetAmount::new(10).unwrap(), - order_id, - NonZeroU32::new(4).unwrap(), - ); - let pswap = PswapNote::builder() - .sender(creator_id) - .storage(storage) - .serial_number(rng.draw_word()) - .note_type(NoteType::Public) - .offered_asset(offered_asset) - .attachment(attachment.into()) - .build() - .unwrap(); - assert_eq!(pswap.parent_depth(), 4); - assert_eq!(pswap.next_depth().unwrap().get(), 5); - } - - /// `parent_depth` falls back to 0 when the attachment encodes a depth outside `u32` range - /// (treated as corrupted by external construction; downstream arithmetic is left - /// well-defined). - #[test] - fn parent_depth_zero_on_out_of_range_attachment() { - let creator_id = dummy_creator_id(); - let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); - let mut rng = RandomCoin::new(Word::default()); - let storage = PswapNoteStorage::builder() - .requested_asset(requested_asset) - .creator_account_id(creator_id) - .build(); - // Stamp a raw word with a depth exceeding u32::MAX. - let oversized_depth = Felt::try_from(u64::from(u32::MAX) + 1).unwrap(); - let word = Word::from([Felt::from(1u32), Felt::from(1u32), oversized_depth, ZERO]); - let raw_attachment = - NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); - let pswap = PswapNote::builder() - .sender(creator_id) - .storage(storage) - .serial_number(rng.draw_word()) - .note_type(NoteType::Public) - .offered_asset(offered_asset) - .attachment(raw_attachment) - .build() - .unwrap(); - assert_eq!(pswap.parent_depth(), 0); - } - - /// `TryFrom<&NoteAttachment>` rejects a wrong scheme. - #[test] - fn try_from_rejects_wrong_scheme() { - let word = Word::from([Felt::from(1u32), Felt::from(2u32), Felt::from(1u32), ZERO]); - // Use NetworkAccountTarget (scheme = 2) instead of PSWAP_ATTACHMENT_SCHEME (3). - let other = NoteAttachment::with_word(StandardNoteAttachment::NetworkAccountTarget.attachment_scheme(), word); - assert!(PswapNoteAttachment::try_from(&other).is_err()); - } - - /// `TryFrom<&NoteAttachment>` rejects depth == 0 (the invariant only one path enforces). - #[test] - fn try_from_rejects_zero_depth() { - let word = Word::from([Felt::from(1u32), Felt::from(2u32), ZERO, ZERO]); - let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); - assert!(PswapNoteAttachment::try_from(&att).is_err()); - } - - /// `TryFrom<&NoteAttachment>` rejects a depth value that exceeds `u32::MAX`. - #[test] - fn try_from_rejects_out_of_range_depth() { - let oversized = Felt::try_from(u64::from(u32::MAX) + 1).unwrap(); - let word = Word::from([Felt::from(1u32), Felt::from(2u32), oversized, ZERO]); - let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); - assert!(PswapNoteAttachment::try_from(&att).is_err()); - } - - /// `TryFrom<&NoteAttachment>` rejects an amount that exceeds `AssetAmount::MAX`. - #[test] - fn try_from_rejects_invalid_amount() { - // 2^63 > AssetAmount::MAX = 2^63 - 2^31. - let bad_amount = Felt::try_from(1u64 << 63).unwrap(); - let word = Word::from([bad_amount, Felt::from(2u32), Felt::from(1u32), ZERO]); - let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); - assert!(PswapNoteAttachment::try_from(&att).is_err()); - } - - /// `TryFrom<&NoteAttachment>` rejects an attachment whose word count is not 1 (covering the - /// MASM `num_words == 1` assert from the Rust side). - #[test] - fn try_from_rejects_wrong_num_words() { - let words = vec![Word::default(), Word::default()]; - let multi = NoteAttachment::with_words(PswapNote::PSWAP_ATTACHMENT_SCHEME, words).unwrap(); - assert!(PswapNoteAttachment::try_from(&multi).is_err()); - } - - /// `TryFrom<&NoteAttachment>` round-trips a valid attachment. - #[test] - fn try_from_round_trips_valid_attachment() { - let original = PswapNoteAttachment::new( - AssetAmount::new(123).unwrap(), - Felt::from(7u32), - NonZeroU32::new(5).unwrap(), - ); - let encoded: NoteAttachment = original.into(); - let decoded = PswapNoteAttachment::try_from(&encoded).unwrap(); - assert_eq!(decoded, original); - } - - /// `next_depth` propagates a `NoteError` if the parent depth is at `u32::MAX`. - #[test] - fn next_depth_errors_on_u32_overflow() { - let creator_id = dummy_creator_id(); - let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); - let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); - let mut rng = RandomCoin::new(Word::default()); - let storage = PswapNoteStorage::builder() - .requested_asset(requested_asset) - .creator_account_id(creator_id) - .build(); - let attachment = PswapNoteAttachment::new( - AssetAmount::new(1).unwrap(), - Felt::from(1u32), - NonZeroU32::new(u32::MAX).unwrap(), - ); - let pswap = PswapNote::builder() - .sender(creator_id) - .storage(storage) - .serial_number(rng.draw_word()) - .note_type(NoteType::Public) - .offered_asset(offered_asset) - .attachment(attachment.into()) - .build() - .unwrap(); - assert!(pswap.next_depth().is_err()); - } -} diff --git a/crates/miden-standards/src/note/pswap/attachment.rs b/crates/miden-standards/src/note/pswap/attachment.rs new file mode 100644 index 0000000000..d05b20a317 --- /dev/null +++ b/crates/miden-standards/src/note/pswap/attachment.rs @@ -0,0 +1,89 @@ +use core::num::NonZeroU32; + +use miden_protocol::asset::AssetAmount; +use miden_protocol::errors::NoteError; +use miden_protocol::note::NoteAttachment; +use miden_protocol::{Felt, Word, ZERO}; + +use super::{OrderId, PswapNote}; + +/// Typed attachment carried by both PSWAP output notes, encoded as +/// `[amount, order_id, depth, 0]` under [`PswapNote::PSWAP_ATTACHMENT_SCHEME`]. +/// +/// `depth` is [`NonZeroU32`] because attachments are only stamped on payback / remainder notes +/// (depth >= 1); the original PSWAP has no PSWAP-scheme attachment. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PswapNoteAttachment { + amount: AssetAmount, + order_id: OrderId, + depth: NonZeroU32, +} + +impl PswapNoteAttachment { + /// Creates a new [`PswapNoteAttachment`]. Infallible: depth is non-zero by type, and + /// [`AssetAmount`] is pre-validated. + pub fn new(amount: AssetAmount, order_id: OrderId, depth: NonZeroU32) -> Self { + Self { amount, order_id, depth } + } + + pub fn amount(&self) -> AssetAmount { + self.amount + } + + pub fn order_id(&self) -> OrderId { + self.order_id + } + + pub fn depth(&self) -> NonZeroU32 { + self.depth + } +} + +impl From for NoteAttachment { + fn from(attachment: PswapNoteAttachment) -> Self { + let word = Word::from([ + Felt::from(attachment.amount), + Felt::from(attachment.order_id), + Felt::from(attachment.depth.get()), + ZERO, + ]); + NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word) + } +} + +/// Parses a [`NoteAttachment`] back into a typed [`PswapNoteAttachment`]. +/// +/// Validates that: +/// - the scheme is [`PswapNote::PSWAP_ATTACHMENT_SCHEME`]; +/// - the content is exactly one word (`num_words == 1`); +/// - the word's `amount` slot is a valid [`AssetAmount`]; +/// - the word's `depth` slot fits in a `u32` and is non-zero. +/// +/// The trailing slot (`word[3]`) is not asserted to be zero: PSWAP_ATTACHMENT_SCHEME's +/// canonical encoding leaves it for forward compatibility. +impl TryFrom<&NoteAttachment> for PswapNoteAttachment { + type Error = NoteError; + + fn try_from(attachment: &NoteAttachment) -> Result { + if attachment.attachment_scheme() != PswapNote::PSWAP_ATTACHMENT_SCHEME { + return Err(NoteError::other("attachment scheme is not PSWAP_ATTACHMENT_SCHEME")); + } + let words = attachment.content().as_words(); + if words.len() != 1 { + return Err(NoteError::other("PSWAP attachment must encode exactly one word")); + } + let word = words[0]; + + let amount = AssetAmount::new(word[0].as_canonical_u64()).map_err(|e| { + NoteError::other_with_source("PSWAP attachment amount is not a valid asset amount", e) + })?; + + let depth_raw = word[PswapNote::PARENT_ATTACHMENT_DEPTH_OFFSET].as_canonical_u64(); + let depth_u32 = u32::try_from(depth_raw) + .map_err(|_| NoteError::other("PSWAP attachment depth does not fit in u32"))?; + let depth = NonZeroU32::new(depth_u32) + .ok_or_else(|| NoteError::other("PSWAP attachment depth must be non-zero"))?; + + Ok(Self { amount, order_id: OrderId::from(word[1]), depth }) + } +} diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs new file mode 100644 index 0000000000..20aa22fade --- /dev/null +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -0,0 +1,730 @@ +use alloc::vec; +use core::num::NonZeroU32; + +use miden_protocol::account::AccountId; +use miden_protocol::assembly::Path; +use miden_protocol::asset::{Asset, AssetAmount, FungibleAsset}; +use miden_protocol::errors::NoteError; +use miden_protocol::note::{ + Note, + NoteAssets, + NoteAttachment, + NoteAttachmentScheme, + NoteAttachments, + NoteScript, + NoteScriptRoot, + NoteTag, + NoteType, + PartialNoteMetadata, +}; +use miden_protocol::utils::sync::LazyLock; +use miden_protocol::{Felt, ONE, Word, ZERO}; + +use crate::StandardsLib; +use crate::note::{P2idNoteStorage, StandardNoteAttachment}; + +mod attachment; +mod storage; + +#[cfg(test)] +mod tests; + +pub use attachment::PswapNoteAttachment; +pub use storage::PswapNoteStorage; + +// NOTE SCRIPT +// ================================================================================================ + +/// Path to the PSWAP note script procedure in the standards library. +const PSWAP_SCRIPT_PATH: &str = "::miden::standards::notes::pswap::main"; + +// Initialize the PSWAP note script only once +static PSWAP_SCRIPT: LazyLock = LazyLock::new(|| { + let standards_lib = StandardsLib::default(); + let path = Path::new(PSWAP_SCRIPT_PATH); + NoteScript::from_library_reference(standards_lib.as_ref(), path) + .expect("Standards library contains PSWAP note script procedure") +}); + +// ORDER ID +// ================================================================================================ + +/// Identifier of a PSWAP order, stable across the entire payback / remainder lineage. +/// +/// Equal to `serial_number[1]` of the originating PSWAP and stamped verbatim into every +/// downstream payback P2ID and remainder PSWAP's `PswapAttachment` (slot `[1]`). +/// +/// `OrderId` is a transparent newtype around [`Felt`]; conversions are provided so callers can +/// move between the typed and raw forms when interfacing with the MASM bridge. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)] +pub struct OrderId(Felt); + +impl OrderId { + /// Wraps a raw [`Felt`] as an [`OrderId`]. + pub const fn new(value: Felt) -> Self { + Self(value) + } + + /// Returns the underlying [`Felt`]. + pub const fn as_felt(&self) -> Felt { + self.0 + } +} + +impl From for OrderId { + fn from(value: Felt) -> Self { + Self(value) + } +} + +impl From for Felt { + fn from(order_id: OrderId) -> Self { + order_id.0 + } +} + +// PSWAP NOTE +// ================================================================================================ + +/// A partially-fillable swap note for decentralized asset exchange. +/// +/// A PSWAP note allows a creator to offer one fungible asset in exchange for another. +/// Unlike a regular SWAP note, consumers may fill it partially — the unfilled portion +/// is re-created as a remainder note with an updated serial number, while the creator +/// receives the filled portion via a payback note. +/// +/// The note can be consumed both in local transactions (where the consumer provides +/// fill amounts via note_args) and in network transactions (where note_args default to +/// `[0, 0, 0, 0]`, triggering a full fill). To route a PSWAP note to a network account, +/// set the `attachment` to a [`NetworkAccountTarget`](crate::note::NetworkAccountTarget) +/// via the builder. +#[derive(Debug, Clone, bon::Builder)] +#[builder(finish_fn(vis = "", name = build_internal))] +pub struct PswapNote { + sender: AccountId, + storage: PswapNoteStorage, + serial_number: Word, + + #[builder(default = NoteType::Private)] + note_type: NoteType, + + offered_asset: FungibleAsset, + + attachment: Option, +} + +impl PswapNoteBuilder +where + S: pswap_note_builder::IsComplete, +{ + /// Validates and builds the [`PswapNote`]. + /// + /// # Errors + /// + /// Returns an error if the offered and requested assets have the same faucet ID. + pub fn build(self) -> Result { + let note = self.build_internal(); + + if note.offered_asset.faucet_id() == note.storage.requested_faucet_id() { + return Err(NoteError::other( + "offered and requested assets must have different faucets", + )); + } + + Ok(note) + } +} + +impl PswapNote { + // CONSTANTS + // -------------------------------------------------------------------------------------------- + + /// Expected number of storage items for the PSWAP note. + pub const NUM_STORAGE_ITEMS: usize = PswapNoteStorage::NUM_STORAGE_ITEMS; + + /// Attachment scheme stamped on both PSWAP output notes (the payback P2ID and the + /// remainder PSWAP). + pub const PSWAP_ATTACHMENT_SCHEME: NoteAttachmentScheme = + StandardNoteAttachment::PswapAttachment.attachment_scheme(); + + /// Offset of the `depth` field within the [`Self::PSWAP_ATTACHMENT_SCHEME`] word. + pub(super) const PARENT_ATTACHMENT_DEPTH_OFFSET: usize = 2; + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns the compiled PSWAP note script. + pub fn script() -> NoteScript { + PSWAP_SCRIPT.clone() + } + + /// Returns the root hash of the PSWAP note script. + pub fn script_root() -> NoteScriptRoot { + PSWAP_SCRIPT.root() + } + + /// Builds the `NOTE_ARGS` word that the PSWAP script expects when a consumer wants to fill + /// part of the swap: `[account_fill, note_fill, 0, 0]`. + /// + /// - `account_fill` is the portion of the requested asset the consumer pays out of their own + /// vault. + /// - `note_fill` is the portion sourced from another note in the same transaction (cross-swap + /// / net-zero flow). + /// + /// Both values are in the requested asset's base units. In a network transaction the kernel + /// defaults `NOTE_ARGS` to `[0, 0, 0, 0]` and the script falls back to a full fill, so this + /// helper is only needed for local transactions where the consumer chooses the fill split. + /// + /// Infallible: [`AssetAmount`] is bounded by `2^63 - 2^31`, which fits in a [`Felt`]. + pub fn create_args(account_fill: AssetAmount, note_fill: AssetAmount) -> Word { + Word::from([Felt::from(account_fill), Felt::from(note_fill), ZERO, ZERO]) + } + + /// Returns the account ID of the note sender. + pub fn sender(&self) -> AccountId { + self.sender + } + + /// Returns a reference to the PSWAP note storage. + pub fn storage(&self) -> &PswapNoteStorage { + &self.storage + } + + /// Returns the serial number of this note. + pub fn serial_number(&self) -> Word { + self.serial_number + } + + /// Returns the note type (public or private). + pub fn note_type(&self) -> NoteType { + self.note_type + } + + /// Returns a reference to the offered [`FungibleAsset`]. + pub fn offered_asset(&self) -> &FungibleAsset { + &self.offered_asset + } + + /// Returns a reference to the note attachments. + /// + /// For notes targeting a network account, this may contain a + /// [`NetworkAccountTarget`](crate::note::NetworkAccountTarget) with scheme = 2. For a + /// remainder PSWAP this contains the [`Self::PSWAP_ATTACHMENT_SCHEME`] word + /// `[amt_payout, order_id, depth, 0]`. For an original PSWAP (no prior fill), + /// this is typically empty. + pub fn attachments(&self) -> Option<&NoteAttachment> { + self.attachment.as_ref() + } + + /// Returns the [`OrderId`] of this lineage, equal to `serial_number()[1]`. + pub fn order_id(&self) -> OrderId { + OrderId::new(self.serial_number[1]) + } + + /// Returns the depth carried in this note's [`Self::PSWAP_ATTACHMENT_SCHEME`] attachment, + /// or 0 if the note has no such attachment (i.e., it is the original PSWAP, not a + /// remainder produced by an earlier fill). + /// + /// The next round's `current_depth` is computed as `parent_depth() + 1`, matching the + /// on-chain `get_current_depth` MASM procedure. Use [`Self::next_depth`] for the typed + /// [`NonZeroU32`] form. A malformed `PSWAP_ATTACHMENT_SCHEME` attachment (depth out of + /// `u32` range, depth == 0, etc.) is treated as if no attachment is present, so discovery + /// degrades to "looks like an original" rather than corrupting downstream arithmetic. + pub fn parent_depth(&self) -> u32 { + self.attachment + .as_ref() + .and_then(|att| PswapNoteAttachment::try_from(att).ok()) + .map(|att| att.depth().get()) + .unwrap_or(0) + } + + /// Returns the depth that the next-round payback / remainder should carry, equal to + /// `parent_depth() + 1`. Always non-zero by construction. + /// + /// # Errors + /// + /// Returns an error if `parent_depth()` is [`u32::MAX`]. + pub fn next_depth(&self) -> Result { + self.parent_depth() + .checked_add(1) + .and_then(NonZeroU32::new) + .ok_or_else(|| NoteError::other("PSWAP depth overflow")) + } + + // INSTANCE METHODS + // -------------------------------------------------------------------------------------------- + + /// Executes the swap as a full fill, producing only the payback note (no remainder). + /// + /// Equivalent to calling [`Self::execute`] with `account_fill_asset` set to the full + /// requested amount and `note_fill_asset = None`. It also matches the on-chain + /// behavior when a note is consumed without explicit `note_args` (e.g. in a network + /// transaction, where the kernel defaults `note_args` to `[0, 0, 0, 0]` and the MASM + /// script falls back to a full fill). + pub fn execute_full_fill(&self, consumer_account_id: AccountId) -> Result { + let requested_faucet_id = self.storage.requested_faucet_id(); + let total_requested_amount = self.storage.requested_asset_amount(); + + let fill_asset = + FungibleAsset::new(requested_faucet_id, total_requested_amount.as_u64()) + .map_err(|e| NoteError::other_with_source("failed to create full fill asset", e))? + .with_callbacks(self.storage.requested_asset().callbacks()); + + self.create_payback_note(consumer_account_id, fill_asset, total_requested_amount.as_u64()) + } + + /// Executes the swap, producing the output notes for a given fill. + /// + /// `account_fill_asset` is debited from the consumer's vault; `note_fill_asset` arrives + /// from another note in the same transaction (cross-swap). At least one must be + /// provided. + /// + /// Returns `(payback_note, Option)`. The remainder is + /// `None` when the fill equals the total requested amount (full fill). + /// + /// # Errors + /// + /// Returns an error if: + /// - Both assets are `None`. + /// - The fill amount is zero. + /// - The fill amount exceeds the total requested amount. + pub fn execute( + &self, + consumer_account_id: AccountId, + account_fill_asset: Option, + note_fill_asset: Option, + ) -> Result<(Note, Option), NoteError> { + // Combine account fill and note fill into a single payback asset. + let payback_asset = match (account_fill_asset, note_fill_asset) { + (Some(account_fill), Some(note_fill)) => account_fill.add(note_fill).map_err(|e| { + NoteError::other_with_source( + "failed to combine account fill and note fill assets", + e, + ) + })?, + (Some(asset), None) | (None, Some(asset)) => asset, + (None, None) => { + return Err(NoteError::other( + "at least one of account_fill_asset or note_fill_asset must be provided", + )); + }, + }; + let fill_amount = payback_asset.amount().as_u64(); + + let total_offered_amount = self.offered_asset.amount().as_u64(); + let requested_faucet_id = self.storage.requested_faucet_id(); + let total_requested_amount = self.storage.requested_asset_amount().as_u64(); + + // Validate fill amount + if fill_amount == 0 { + return Err(NoteError::other("Fill amount must be greater than 0")); + } + if fill_amount > total_requested_amount { + return Err(NoteError::other(alloc::format!( + "Fill amount {} exceeds requested amount {}", + fill_amount, + total_requested_amount + ))); + } + + // Calculate payout amounts separately for account fill and note fill, matching the + // MASM which calls calculate_tokens_offered_for_requested twice. This is necessary + // because the account fill portion goes to the consumer's vault while the total + // determines the remainder note's offered amount. + let account_fill_amount = account_fill_asset.as_ref().map_or(0, |a| a.amount().as_u64()); + let note_fill_amount = note_fill_asset.as_ref().map_or(0, |a| a.amount().as_u64()); + let payout_for_account_fill = Self::calculate_output_amount( + total_offered_amount, + total_requested_amount, + account_fill_amount, + )?; + let payout_for_note_fill = Self::calculate_output_amount( + total_offered_amount, + total_requested_amount, + note_fill_amount, + )?; + let offered_amount_for_fill = payout_for_account_fill + payout_for_note_fill; + + let payback_note = + self.create_payback_note(consumer_account_id, payback_asset, fill_amount)?; + + // Create remainder note if partial fill + let remainder = if fill_amount < total_requested_amount { + let remaining_offered = total_offered_amount - offered_amount_for_fill; + let remaining_requested = total_requested_amount - fill_amount; + + let remaining_offered_asset = + FungibleAsset::new(self.offered_asset.faucet_id(), remaining_offered) + .map_err(|e| { + NoteError::other_with_source("failed to create remainder asset", e) + })? + .with_callbacks(self.offered_asset.callbacks()); + + let remaining_requested_asset = + FungibleAsset::new(requested_faucet_id, remaining_requested) + .map_err(|e| { + NoteError::other_with_source( + "failed to create remaining requested asset", + e, + ) + })? + .with_callbacks(self.storage.requested_asset().callbacks()); + + Some(self.create_remainder_pswap_note( + consumer_account_id, + remaining_offered_asset, + remaining_requested_asset, + offered_amount_for_fill, + )?) + } else { + None + }; + + Ok((payback_note, remainder)) + } + + /// Returns how many offered tokens a consumer receives for `fill_amount` of the + /// requested asset, based on this note's current offered/requested ratio. + /// + /// # Errors + /// + /// Returns an error if the calculated payout is not a valid asset amount. + pub fn calculate_offered_for_requested(&self, fill_amount: u64) -> Result { + let total_requested = self.storage.requested_asset_amount().as_u64(); + let total_offered = self.offered_asset.amount().as_u64(); + + Self::calculate_output_amount(total_offered, total_requested, fill_amount) + } + + // LINEAGE DISCOVERY + // -------------------------------------------------------------------------------------------- + + /// Reconstructs the depth-`d` payback P2ID [`Note`], so the creator can consume it as an + /// unauthenticated input note. + /// + /// `consumer_account_id` must be the account that consumed the parent PSWAP in round + /// `depth`: the MASM stamps it as the payback's metadata sender, which feeds into + /// [`Note::details_commitment`]. + /// + /// # Errors + /// + /// Returns an error if the fill amount is not a valid asset amount. + pub fn payback_note( + &self, + consumer_account_id: AccountId, + attachment: &PswapNoteAttachment, + ) -> Result { + let depth = attachment.depth().get(); + let parent_depth = Felt::from(depth - 1); + let p2id_serial = Word::from([ + self.serial_number[0] + ONE, + self.serial_number[1], + self.serial_number[2], + self.serial_number[3] + parent_depth, + ]); + + let recipient = + P2idNoteStorage::new(self.storage.creator_account_id()).into_recipient(p2id_serial); + + let fill_asset = + FungibleAsset::new(self.storage.requested_faucet_id(), u64::from(attachment.amount())) + .map_err(|e| NoteError::other_with_source("invalid fill amount", e))? + .with_callbacks(self.storage.requested_asset().callbacks()); + let assets = NoteAssets::new(vec![fill_asset.into()])?; + + let metadata = + PartialNoteMetadata::new(consumer_account_id, self.storage.payback_note_type()) + .with_tag(self.storage.payback_note_tag()); + + Ok(Note::with_attachments( + assets, + metadata, + recipient, + NoteAttachments::from(NoteAttachment::from(*attachment)), + )) + } + + /// Reconstructs the depth-`d` remainder PSWAP [`Note`] in this lineage. + /// + /// Called on the original PSWAP, this returns the full Note for the remainder produced + /// in round `depth`. The returned Note matches the created note exactly. + /// + /// - `consumer_account_id` — the account that consumed the parent PSWAP in round `depth`, used + /// as the remainder's sender. + /// - `attachment` — the on-chain `[amount, order_id, depth, 0]` attachment for this round, + /// where `amount` is the offered-asset units paid out. + /// - `remaining_offered` / `remaining_requested` — the leftover amounts that survive into this + /// remainder. Both are required because the price formula uses floor division, so one isn't + /// derivable from the other across rounds in general. + /// + /// # Errors + /// + /// Returns an error if any amount is not a valid asset amount. + pub fn remainder_note( + &self, + consumer_account_id: AccountId, + attachment: &PswapNoteAttachment, + remaining_offered: AssetAmount, + remaining_requested: AssetAmount, + ) -> Result { + let depth = attachment.depth().get(); + let remainder_serial = Word::from([ + self.serial_number[0], + self.serial_number[1], + self.serial_number[2], + self.serial_number[3] + Felt::from(depth), + ]); + + let requested_asset = + FungibleAsset::new(self.storage.requested_faucet_id(), u64::from(remaining_requested)) + .map_err(|e| NoteError::other_with_source("invalid remaining_requested amount", e))? + .with_callbacks(self.storage.requested_asset().callbacks()); + let offered_asset = + FungibleAsset::new(self.offered_asset.faucet_id(), u64::from(remaining_offered)) + .map_err(|e| NoteError::other_with_source("invalid remaining_offered amount", e))? + .with_callbacks(self.offered_asset.callbacks()); + + let new_storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(self.storage.creator_account_id()) + .payback_note_type(self.storage.payback_note_type()) + .build(); + let recipient = new_storage.into_recipient(remainder_serial); + + let assets = NoteAssets::new(vec![offered_asset.into()])?; + + let tag = Self::create_tag(self.note_type, &offered_asset, &requested_asset); + let metadata = PartialNoteMetadata::new(consumer_account_id, self.note_type).with_tag(tag); + + Ok(Note::with_attachments( + assets, + metadata, + recipient, + NoteAttachments::from(NoteAttachment::from(*attachment)), + )) + } + + // ASSOCIATED FUNCTIONS + // -------------------------------------------------------------------------------------------- + + /// Builds the 32-bit [`NoteTag`] for a PSWAP note. + /// + /// ```text + /// [31..30] note_type (2 bits) + /// [29..16] script_root MSBs (14 bits) + /// [15..8] offered faucet ID (8 bits, top byte of prefix) + /// [7..0] requested faucet ID (8 bits, top byte of prefix) + /// ``` + pub fn create_tag( + note_type: NoteType, + offered_asset: &FungibleAsset, + requested_asset: &FungibleAsset, + ) -> NoteTag { + let pswap_root_bytes = Self::script().root().as_bytes(); + + // Construct the pswap use case ID from the 14 most significant bits of the script root. + // This leaves the two most significant bits zero. + let mut pswap_use_case_id = (pswap_root_bytes[0] as u16) << 6; + pswap_use_case_id |= (pswap_root_bytes[1] >> 2) as u16; + + // Get bits 0..8 from the faucet IDs of both assets which will form the tag payload. + let offered_asset_id: u64 = offered_asset.faucet_id().prefix().into(); + let offered_asset_tag = (offered_asset_id >> 56) as u8; + + let requested_asset_id: u64 = requested_asset.faucet_id().prefix().into(); + let requested_asset_tag = (requested_asset_id >> 56) as u8; + + let asset_pair = ((offered_asset_tag as u16) << 8) | (requested_asset_tag as u16); + + let tag = ((note_type as u8 as u32) << 30) + | ((pswap_use_case_id as u32) << 16) + | asset_pair as u32; + + NoteTag::new(tag) + } + + /// Computes `floor((offered_total * fill_amount) / requested_total)` via a + /// u128 intermediate, mirroring `u64::widening_mul` + `u128::div` on the + /// MASM side. + /// + /// # Errors + /// + /// Returns an error if the result does not fit in a valid [`AssetAmount`]. + fn calculate_output_amount( + offered_total: u64, + requested_total: u64, + fill_amount: u64, + ) -> Result { + let product = (offered_total as u128) * (fill_amount as u128); + let quotient = product / (requested_total as u128); + let amount = u64::try_from(quotient) + .map_err(|_| NoteError::other("payout quotient does not fit in u64"))?; + // Validate the result is a valid fungible asset amount. + AssetAmount::new(amount).map_err(|e| { + NoteError::other_with_source("payout amount exceeds max fungible asset amount", e) + })?; + Ok(amount) + } + + + /// Builds a payback note (P2ID) that delivers the filled assets to the swap creator. + /// + /// The note inherits its type (public/private) from this PSWAP note and derives a + /// deterministic serial number by incrementing the least significant element of the + /// serial number (`serial[0] + 1`). + /// + /// The attachment carries `[fill_amount, order_id, current_depth, 0]` under + /// [`Self::PSWAP_ATTACHMENT_SCHEME`]. `current_depth` is `parent_depth + 1` — i.e., + /// the round number that produced this payback (1-indexed). + fn create_payback_note( + &self, + consumer_account_id: AccountId, + payback_asset: FungibleAsset, + fill_amount: u64, + ) -> Result { + let payback_note_tag = self.storage.payback_note_tag(); + // Derive P2ID serial: increment least significant element (matching MASM add.1) + let p2id_serial_num = Word::from([ + self.serial_number[0] + ONE, + self.serial_number[1], + self.serial_number[2], + self.serial_number[3], + ]); + + // P2ID recipient targets the creator + let recipient = + P2idNoteStorage::new(self.storage.creator_account_id()).into_recipient(p2id_serial_num); + + let current_depth = self.next_depth()?; + let fill_amount_typed = AssetAmount::new(fill_amount).map_err(|e| { + NoteError::other_with_source("fill amount is not a valid asset amount", e) + })?; + let attachment: NoteAttachment = + PswapNoteAttachment::new(fill_amount_typed, self.order_id(), current_depth).into(); + + let p2id_assets = NoteAssets::new(vec![payback_asset.into()])?; + let p2id_metadata = + PartialNoteMetadata::new(consumer_account_id, self.storage.payback_note_type()) + .with_tag(payback_note_tag); + + Ok(Note::with_attachments( + p2id_assets, + p2id_metadata, + recipient, + NoteAttachments::from(attachment), + )) + } + + /// Builds a remainder PSWAP note carrying the unfilled portion of the swap. + /// + /// The remainder inherits the original creator, tags, and note type, with an updated + /// serial number (`serial[3] + 1`). + /// + /// The attachment carries `[offered_amount_for_fill, order_id, current_depth, 0]` under + /// [`Self::PSWAP_ATTACHMENT_SCHEME`]. The remainder must carry this attachment so that + /// when *it* is later consumed as a parent, `get_current_depth` reads the right scheme + /// and increments depth correctly. + fn create_remainder_pswap_note( + &self, + consumer_account_id: AccountId, + remaining_offered_asset: FungibleAsset, + remaining_requested_asset: FungibleAsset, + offered_amount_for_fill: u64, + ) -> Result { + let new_storage = PswapNoteStorage::builder() + .requested_asset(remaining_requested_asset) + .creator_account_id(self.storage.creator_account_id()) + .payback_note_type(self.storage.payback_note_type()) + .build(); + + // Remainder serial: increment most significant element (matching MASM movup.3 add.1 + // movdn.3) + let remainder_serial_num = Word::from([ + self.serial_number[0], + self.serial_number[1], + self.serial_number[2], + self.serial_number[3] + ONE, + ]); + + let current_depth = self.next_depth()?; + let payout_typed = AssetAmount::new(offered_amount_for_fill).map_err(|e| { + NoteError::other_with_source("payout amount is not a valid asset amount", e) + })?; + let attachment: NoteAttachment = + PswapNoteAttachment::new(payout_typed, self.order_id(), current_depth).into(); + + PswapNote::builder() + .sender(consumer_account_id) + .storage(new_storage) + .serial_number(remainder_serial_num) + .note_type(self.note_type) + .offered_asset(remaining_offered_asset) + .attachment(attachment) + .build() + } +} + +// CONVERSIONS +// ================================================================================================ + +/// Converts a [`PswapNote`] into a protocol [`Note`], computing the final PSWAP tag. +impl From for Note { + fn from(pswap: PswapNote) -> Self { + let tag = PswapNote::create_tag( + pswap.note_type, + &pswap.offered_asset, + pswap.storage.requested_asset(), + ); + + let recipient = pswap.storage.into_recipient(pswap.serial_number); + + let assets = NoteAssets::new(vec![pswap.offered_asset.into()]) + .expect("single fungible asset should be valid"); + + let metadata = PartialNoteMetadata::new(pswap.sender, pswap.note_type).with_tag(tag); + + let attachments = pswap.attachment.map(NoteAttachments::from).unwrap_or_default(); + + Note::with_attachments(assets, metadata, recipient, attachments) + } +} + +/// Parses a protocol [`Note`] back into a [`PswapNote`] by deserializing its storage. +impl TryFrom<&Note> for PswapNote { + type Error = NoteError; + + fn try_from(note: &Note) -> Result { + if note.recipient().script().root() != PswapNote::script_root() { + return Err(NoteError::other("note script root does not match PSWAP script root")); + } + + let storage = PswapNoteStorage::try_from(note.recipient().storage().items())?; + + if note.assets().num_assets() != 1 { + return Err(NoteError::other("PSWAP note must have exactly one asset")); + } + let offered_asset = match note.assets().iter().next().unwrap() { + Asset::Fungible(fa) => *fa, + Asset::NonFungible(_) => { + return Err(NoteError::other("PSWAP note asset must be fungible")); + }, + }; + + let attachment = match note.attachments().num_attachments() { + 0 => None, + 1 => { + Some(note.attachments().get(0).expect("length should have been validated").clone()) + }, + _ => return Err(NoteError::other("pswap note supports only one attachment")), + }; + + PswapNote::builder() + .sender(note.metadata().sender()) + .storage(storage) + .serial_number(note.recipient().serial_num()) + .note_type(note.metadata().note_type()) + .offered_asset(offered_asset) + .maybe_attachment(attachment) + .build() + } +} diff --git a/crates/miden-standards/src/note/pswap/storage.rs b/crates/miden-standards/src/note/pswap/storage.rs new file mode 100644 index 0000000000..f93d6f6171 --- /dev/null +++ b/crates/miden-standards/src/note/pswap/storage.rs @@ -0,0 +1,170 @@ +use alloc::vec; + +use miden_protocol::account::AccountId; +use miden_protocol::asset::{AssetAmount, AssetCallbackFlag, AssetId, FungibleAsset}; +use miden_protocol::errors::NoteError; +use miden_protocol::note::{NoteRecipient, NoteStorage, NoteTag, NoteType}; +use miden_protocol::{Felt, Word}; + +use super::PswapNote; + +/// Canonical storage representation for a PSWAP note. +/// +/// Maps to the 7-element [`NoteStorage`] layout consumed by the on-chain MASM script: +/// +/// | Slot | Field | +/// |---------|-------| +/// | `[0]` | Requested asset enable_callbacks flag | +/// | `[1]` | Requested asset faucet ID suffix | +/// | `[2]` | Requested asset faucet ID prefix | +/// | `[3]` | Requested asset amount | +/// | `[4]` | Payback note type (0 = private, 1 = public) | +/// | `[5-6]` | Creator account ID (prefix, suffix) | +/// +/// Slots `[1, 2]` together form a 2-element [`AssetId`] view of the faucet (see +/// [`Self::requested_asset_id`]). The on-chain layout is unchanged - the typed accessor is a +/// Rust-side convenience. +/// +/// The payback note tag is derived at runtime from the creator account ID +/// (via `note_tag::create_account_target` in MASM) rather than stored. +/// +/// The PSWAP note's own tag is not stored: it lives in the note's metadata and +/// is lifted from there by the on-chain script when a remainder note is created +/// (the asset pair is unchanged, so the tag carries over unchanged). +#[derive(Debug, Clone, PartialEq, Eq, bon::Builder)] +pub struct PswapNoteStorage { + requested_asset: FungibleAsset, + + creator_account_id: AccountId, + + /// Note type of the payback note produced when the pswap is filled. Defaults to + /// [`NoteType::Private`] because the payback carries the fill asset and is typically + /// consumed directly by the creator - a private note is cheaper in fees and bandwidth + /// and offers the same information (the fill amount is already recorded in the + /// executed transaction's output). + #[builder(default = NoteType::Private)] + payback_note_type: NoteType, +} + +impl PswapNoteStorage { + // CONSTANTS + // -------------------------------------------------------------------------------------------- + + /// Expected number of storage items for the PSWAP note. + pub const NUM_STORAGE_ITEMS: usize = 7; + + /// Consumes the storage and returns a PSWAP [`NoteRecipient`] with the provided serial number. + pub fn into_recipient(self, serial_num: Word) -> NoteRecipient { + NoteRecipient::new(serial_num, PswapNote::script(), NoteStorage::from(self)) + } + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns a reference to the requested [`FungibleAsset`]. + pub fn requested_asset(&self) -> &FungibleAsset { + &self.requested_asset + } + + /// Returns the payback note routing tag, derived from the creator's account ID. + pub fn payback_note_tag(&self) -> NoteTag { + NoteTag::with_account_target(self.creator_account_id) + } + + /// Returns the account ID of the note creator. + pub fn creator_account_id(&self) -> AccountId { + self.creator_account_id + } + + /// Returns the [`NoteType`] used when creating the payback note. + pub fn payback_note_type(&self) -> NoteType { + self.payback_note_type + } + + /// Returns the faucet ID of the requested asset. + pub fn requested_faucet_id(&self) -> AccountId { + self.requested_asset.faucet_id() + } + + /// Returns the requested faucet's two-felt identity as an [`AssetId`]. + /// + /// Note: [`AssetId`] is the protocol-level vault-key newtype carrying `(suffix, prefix)`. It + /// is independent of the variant ID used for non-fungible assets - this accessor exposes + /// the requested fungible faucet's ID in that shape so callers indexing PSWAP storage by + /// asset can pattern-match alongside other 2-felt asset identifiers. + pub fn requested_asset_id(&self) -> AssetId { + AssetId::new(self.requested_asset.faucet_id().suffix(), self.requested_asset.faucet_id().prefix().as_felt()) + } + + /// Returns the requested token amount. + pub fn requested_asset_amount(&self) -> AssetAmount { + self.requested_asset.amount() + } +} + +/// Serializes [`PswapNoteStorage`] into a 7-element [`NoteStorage`]. +impl From for NoteStorage { + fn from(storage: PswapNoteStorage) -> Self { + let storage_items = vec![ + // Requested asset (individual felts) [0-3] + Felt::from(storage.requested_asset.callbacks().as_u8()), + storage.requested_asset.faucet_id().suffix(), + storage.requested_asset.faucet_id().prefix().as_felt(), + Felt::from(storage.requested_asset.amount()), + // Payback note type [4] + Felt::from(storage.payback_note_type.as_u8()), + // Creator ID [5-6] + storage.creator_account_id.prefix().as_felt(), + storage.creator_account_id.suffix(), + ]; + NoteStorage::new(storage_items) + .expect("number of storage items should not exceed max storage items") + } +} + +/// Deserializes [`PswapNoteStorage`] from a slice of exactly 7 [`Felt`]s. +impl TryFrom<&[Felt]> for PswapNoteStorage { + type Error = NoteError; + + fn try_from(note_storage: &[Felt]) -> Result { + if note_storage.len() != Self::NUM_STORAGE_ITEMS { + return Err(NoteError::InvalidNoteStorageLength { + expected: Self::NUM_STORAGE_ITEMS, + actual: note_storage.len(), + }); + } + + // Reconstruct requested asset from individual felts: + // [0] = enable_callbacks, [1] = faucet_id_suffix, [2] = faucet_id_prefix, [3] = amount + let callbacks = AssetCallbackFlag::try_from( + u8::try_from(note_storage[0].as_canonical_u64()) + .map_err(|_| NoteError::other("enable_callbacks exceeds u8"))?, + ) + .map_err(|e| NoteError::other_with_source("failed to parse asset callback flag", e))?; + + let faucet_id = AccountId::try_from_elements(note_storage[1], note_storage[2]) + .map_err(|e| NoteError::other_with_source("failed to parse requested faucet ID", e))?; + + let amount = note_storage[3].as_canonical_u64(); + let requested_asset = FungibleAsset::new(faucet_id, amount) + .map_err(|e| NoteError::other_with_source("failed to create requested asset", e))? + .with_callbacks(callbacks); + + // [4] = payback_note_type + let payback_note_type = NoteType::try_from( + u8::try_from(note_storage[4].as_canonical_u64()) + .map_err(|_| NoteError::other("payback_note_type exceeds u8"))?, + ) + .map_err(|e| NoteError::other_with_source("failed to parse payback note type", e))?; + + // [5-6] = creator account ID (prefix, suffix) + let creator_account_id = AccountId::try_from_elements(note_storage[6], note_storage[5]) + .map_err(|e| NoteError::other_with_source("failed to parse creator account ID", e))?; + + Ok(Self { + requested_asset, + creator_account_id, + payback_note_type, + }) + } +} diff --git a/crates/miden-standards/src/note/pswap/tests.rs b/crates/miden-standards/src/note/pswap/tests.rs new file mode 100644 index 0000000000..6599a2eb36 --- /dev/null +++ b/crates/miden-standards/src/note/pswap/tests.rs @@ -0,0 +1,549 @@ +use miden_protocol::account::{AccountId, AccountIdVersion, AccountType}; +use miden_protocol::asset::{AssetCallbackFlag, AssetId, FungibleAsset}; +use miden_protocol::crypto::rand::{FeltRng, RandomCoin}; +use miden_protocol::note::NoteStorage; + +use super::*; + +// TEST HELPERS +// -------------------------------------------------------------------------------------------- + +fn dummy_faucet_id(byte: u8) -> AccountId { + let mut bytes = [0; 15]; + bytes[0] = byte; + AccountId::dummy(bytes, AccountIdVersion::Version1, AccountType::Public) +} + +fn dummy_creator_id() -> AccountId { + AccountId::dummy([1; 15], AccountIdVersion::Version1, AccountType::Public) +} + +fn dummy_consumer_id() -> AccountId { + AccountId::dummy([2; 15], AccountIdVersion::Version1, AccountType::Public) +} + +fn build_pswap_note( + offered_asset: FungibleAsset, + requested_asset: FungibleAsset, + creator_id: AccountId, +) -> (PswapNote, Note) { + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let pswap = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .build() + .unwrap(); + let note: Note = pswap.clone().into(); + (pswap, note) +} + +// TESTS +// -------------------------------------------------------------------------------------------- + +#[test] +fn pswap_note_creation_and_script() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 1000).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 500).unwrap(); + + let (pswap, note) = build_pswap_note(offered_asset, requested_asset, creator_id); + + assert_eq!(pswap.sender(), creator_id); + assert_eq!(pswap.note_type(), NoteType::Public); + + let script = PswapNote::script(); + assert!(Word::from(script.root()) != Word::default(), "Script root should not be zero"); + assert_eq!(note.metadata().sender(), creator_id); + assert_eq!(note.metadata().note_type(), NoteType::Public); + assert_eq!(note.assets().num_assets(), 1); + assert_eq!(note.recipient().script().root(), script.root()); + assert_eq!( + note.recipient().storage().num_items(), + PswapNoteStorage::NUM_STORAGE_ITEMS as u16, + ); +} + +#[test] +fn pswap_note_builder() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 1000).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 500).unwrap(); + + let (pswap, note) = build_pswap_note(offered_asset, requested_asset, creator_id); + + assert_eq!(pswap.sender(), creator_id); + assert_eq!(pswap.note_type(), NoteType::Public); + assert_eq!(note.metadata().sender(), creator_id); + assert_eq!(note.metadata().note_type(), NoteType::Public); + assert_eq!(note.assets().num_assets(), 1); + assert_eq!( + note.recipient().storage().num_items(), + PswapNoteStorage::NUM_STORAGE_ITEMS as u16, + ); +} + +#[test] +fn pswap_tag() { + let mut offered_faucet_bytes = [0; 15]; + offered_faucet_bytes[0] = 0xcd; + offered_faucet_bytes[1] = 0xb1; + + let mut requested_faucet_bytes = [0; 15]; + requested_faucet_bytes[0] = 0xab; + requested_faucet_bytes[1] = 0xec; + + let offered_asset = FungibleAsset::new( + AccountId::dummy(offered_faucet_bytes, AccountIdVersion::Version1, AccountType::Public), + 100, + ) + .unwrap(); + let requested_asset = FungibleAsset::new( + AccountId::dummy( + requested_faucet_bytes, + AccountIdVersion::Version1, + AccountType::Public, + ), + 200, + ) + .unwrap(); + + let tag = PswapNote::create_tag(NoteType::Public, &offered_asset, &requested_asset); + let tag_u32 = u32::from(tag); + + // Verify note_type bits (top 2 bits should be 10 for Public) + let note_type_bits = tag_u32 >> 30; + assert_eq!(note_type_bits, NoteType::Public as u32); +} + +#[test] +fn calculate_output_amount() { + assert_eq!(PswapNote::calculate_output_amount(100, 100, 50).unwrap(), 50); // Equal ratio + assert_eq!(PswapNote::calculate_output_amount(200, 100, 50).unwrap(), 100); // 2:1 ratio + assert_eq!(PswapNote::calculate_output_amount(100, 200, 50).unwrap(), 25); // 1:2 ratio + + // Non-integer ratio (100/73) + let result = PswapNote::calculate_output_amount(100, 73, 7).unwrap(); + assert!(result > 0, "Should produce non-zero output"); +} + +#[test] +fn pswap_note_storage_try_from() { + let creator_id = dummy_creator_id(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 500).unwrap(); + + let storage_items = vec![ + Felt::from(requested_asset.callbacks().as_u8()), + requested_asset.faucet_id().suffix(), + requested_asset.faucet_id().prefix().as_felt(), + Felt::from(requested_asset.amount()), + Felt::from(NoteType::Private.as_u8()), // payback_note_type + creator_id.prefix().as_felt(), + creator_id.suffix(), + ]; + + let parsed = PswapNoteStorage::try_from(storage_items.as_slice()).unwrap(); + assert_eq!(parsed.creator_account_id(), creator_id); + assert_eq!(parsed.requested_asset_amount(), AssetAmount::new(500).unwrap()); +} + +#[test] +fn pswap_note_storage_roundtrip() { + let creator_id = dummy_creator_id(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 500).unwrap(); + + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + + let note_storage = NoteStorage::from(storage.clone()); + let parsed = PswapNoteStorage::try_from(note_storage.items()).unwrap(); + + assert_eq!(parsed.creator_account_id(), creator_id); + assert_eq!(parsed.requested_asset_amount(), AssetAmount::new(500).unwrap()); +} + +/// Consumer supplies both an account fill and a note fill, and the sum is below +/// the requested amount → `execute` must combine them into a single payback note +/// carrying account_fill+note_fill of the requested asset and emit a remainder +/// pswap note for the unfilled portion. +#[test] +fn pswap_execute_combined_account_fill_and_note_fill_partial_fill() { + let creator_id = dummy_creator_id(); + let consumer_id = dummy_consumer_id(); + let offered_faucet = dummy_faucet_id(0xaa); + let requested_faucet = dummy_faucet_id(0xbb); + + // Offer 100 offered, request 50 requested → 2:1 ratio. + let offered_asset = FungibleAsset::new(offered_faucet, 100).unwrap(); + let requested_asset = FungibleAsset::new(requested_faucet, 50).unwrap(); + let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); + + // Account fill = 10, note fill = 20 → total fill = 30 (< 50, so partial). + let account_fill = FungibleAsset::new(requested_faucet, 10).unwrap(); + let note_fill = FungibleAsset::new(requested_faucet, 20).unwrap(); + + let (payback, remainder) = + pswap.execute(consumer_id, Some(account_fill), Some(note_fill)).unwrap(); + + // Payback note must carry the combined 30 of requested asset. + assert_eq!(payback.assets().num_assets(), 1); + let payback_asset = payback.assets().iter().next().unwrap(); + let Asset::Fungible(fa) = payback_asset else { + panic!("expected fungible payback asset"); + }; + assert_eq!(fa.faucet_id(), requested_faucet); + assert_eq!(fa.amount().as_u64(), 30); + + // Remainder must exist with the unfilled 50 - 30 = 20 of requested, and the + // offered amount reduced proportionally (100 - 30*2 = 40). + let remainder = remainder.expect("partial fill should produce remainder"); + assert_eq!(remainder.storage().requested_asset_amount(), AssetAmount::new(20).unwrap()); + assert_eq!(remainder.offered_asset().amount().as_u64(), 40); + assert_eq!(remainder.storage().creator_account_id(), creator_id); +} + +/// Consumer supplies both an account fill and a note fill, and the sum exactly +/// matches the requested amount → `execute` must produce a single payback note for +/// the full amount and no remainder. +#[test] +fn pswap_execute_combined_account_fill_and_note_fill_full_fill() { + let creator_id = dummy_creator_id(); + let consumer_id = dummy_consumer_id(); + let offered_faucet = dummy_faucet_id(0xaa); + let requested_faucet = dummy_faucet_id(0xbb); + + let offered_asset = FungibleAsset::new(offered_faucet, 100).unwrap(); + let requested_asset = FungibleAsset::new(requested_faucet, 50).unwrap(); + let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); + + // Account fill = 30, note fill = 20 → total fill = 50 (exactly requested). + let account_fill = FungibleAsset::new(requested_faucet, 30).unwrap(); + let note_fill = FungibleAsset::new(requested_faucet, 20).unwrap(); + + let (payback, remainder) = + pswap.execute(consumer_id, Some(account_fill), Some(note_fill)).unwrap(); + + // Payback note must carry the full 50 of requested asset. + assert_eq!(payback.assets().num_assets(), 1); + let payback_asset = payback.assets().iter().next().unwrap(); + let Asset::Fungible(fa) = payback_asset else { + panic!("expected fungible payback asset"); + }; + assert_eq!(fa.faucet_id(), requested_faucet); + assert_eq!(fa.amount().as_u64(), 50); + + // Full fill → no remainder note. + assert!(remainder.is_none(), "full fill must not produce a remainder"); +} + +/// Regression for the silent `AssetCallbackFlag` drop: when the PSWAP's requested or +/// offered asset carries `Enabled` callbacks, the on-chain MASM preserves that flag +/// on every output note's asset. The Rust-side `execute`, `payback_note`, and +/// `remainder_note` must do the same — otherwise the reconstructed `Note::details_commitment` +/// diverges from the on-chain leaf and the unauthenticated consume path fails. +#[test] +fn pswap_output_assets_preserve_callback_flag() { + let creator_id = dummy_creator_id(); + let consumer_id = dummy_consumer_id(); + let offered_faucet = dummy_faucet_id(0xaa); + let requested_faucet = dummy_faucet_id(0xbb); + + let offered_asset = FungibleAsset::new(offered_faucet, 100) + .unwrap() + .with_callbacks(AssetCallbackFlag::Enabled); + let requested_asset = FungibleAsset::new(requested_faucet, 50) + .unwrap() + .with_callbacks(AssetCallbackFlag::Enabled); + let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); + + // --- execute() (partial fill) --- + let account_fill = FungibleAsset::new(requested_faucet, 20) + .unwrap() + .with_callbacks(AssetCallbackFlag::Enabled); + let (payback, remainder) = pswap.execute(consumer_id, Some(account_fill), None).unwrap(); + + let Asset::Fungible(fa) = payback.assets().iter().next().unwrap() else { + panic!("expected fungible payback asset"); + }; + assert_eq!(fa.callbacks(), AssetCallbackFlag::Enabled); + + let remainder = remainder.expect("partial fill should produce remainder"); + assert_eq!( + remainder.offered_asset().callbacks(), + AssetCallbackFlag::Enabled, + "remainder offered asset must inherit callbacks", + ); + assert_eq!( + remainder.storage().requested_asset().callbacks(), + AssetCallbackFlag::Enabled, + "remainder storage's requested asset must inherit callbacks", + ); + + // --- payback_note() reconstruction --- + let depth_one = NonZeroU32::new(1).unwrap(); + let payback_attachment = + PswapNoteAttachment::new(AssetAmount::new(20).unwrap(), pswap.order_id(), depth_one); + let reconstructed_payback = pswap.payback_note(consumer_id, &payback_attachment).unwrap(); + let Asset::Fungible(fa) = reconstructed_payback.assets().iter().next().unwrap() else { + panic!("expected fungible payback asset"); + }; + assert_eq!( + fa.callbacks(), + AssetCallbackFlag::Enabled, + "payback_note must preserve requested asset's callback flag", + ); + + // --- remainder_note() reconstruction --- + let remainder_attachment = + PswapNoteAttachment::new(AssetAmount::new(40).unwrap(), pswap.order_id(), depth_one); + let reconstructed_remainder = pswap + .remainder_note( + consumer_id, + &remainder_attachment, + AssetAmount::new(60).unwrap(), + AssetAmount::new(30).unwrap(), + ) + .unwrap(); + let Asset::Fungible(fa) = reconstructed_remainder.assets().iter().next().unwrap() else { + panic!("expected fungible remainder asset"); + }; + assert_eq!( + fa.callbacks(), + AssetCallbackFlag::Enabled, + "remainder_note must preserve offered asset's callback flag", + ); +} + +/// `create_args` is infallible at the type level because `AssetAmount` fits in a `Felt`. +/// `MAX` round-trips through the resulting word. +#[test] +fn create_args_round_trips_max_asset_amount() { + let args = PswapNote::create_args(AssetAmount::MAX, AssetAmount::ZERO); + assert_eq!(args[0], Felt::from(AssetAmount::MAX)); + assert_eq!(args[1], ZERO); + assert_eq!(args[2], ZERO); + assert_eq!(args[3], ZERO); +} + +/// `PswapNoteAttachment` accessors mirror what was passed to `new`. +#[test] +fn pswap_note_attachment_accessors() { + let order_id = OrderId::from(Felt::from(42u32)); + let depth = NonZeroU32::new(3).unwrap(); + let attachment = PswapNoteAttachment::new(AssetAmount::new(100).unwrap(), order_id, depth); + assert_eq!(attachment.amount(), AssetAmount::new(100).unwrap()); + assert_eq!(attachment.order_id(), order_id); + assert_eq!(attachment.depth(), depth); +} + +/// `From for NoteAttachment` encodes the depth via `.get()`. +#[test] +fn pswap_note_attachment_encodes_depth_via_get() { + let depth = NonZeroU32::new(7).unwrap(); + let attachment = PswapNoteAttachment::new( + AssetAmount::new(50).unwrap(), + OrderId::from(Felt::from(9u32)), + depth, + ); + let note_att: NoteAttachment = attachment.into(); + let word = note_att.content().as_words()[0]; + assert_eq!(word[2], Felt::from(7u32)); +} + +/// `parent_depth` returns 0 when the note has no attachment. +#[test] +fn parent_depth_zero_when_no_attachment() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); + assert_eq!(pswap.parent_depth(), 0); + assert_eq!(pswap.next_depth().unwrap().get(), 1); +} + +/// `parent_depth` returns the stored depth when the note carries a PSWAP attachment. +#[test] +fn parent_depth_reads_attachment_depth() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let order_id = OrderId::from(Felt::from(1u32)); + let attachment = PswapNoteAttachment::new( + AssetAmount::new(10).unwrap(), + order_id, + NonZeroU32::new(4).unwrap(), + ); + let pswap = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .attachment(attachment.into()) + .build() + .unwrap(); + assert_eq!(pswap.parent_depth(), 4); + assert_eq!(pswap.next_depth().unwrap().get(), 5); +} + +/// `parent_depth` falls back to 0 when the attachment encodes a depth outside `u32` range +/// (treated as corrupted by external construction; downstream arithmetic is left +/// well-defined). +#[test] +fn parent_depth_zero_on_out_of_range_attachment() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + // Stamp a raw word with a depth exceeding u32::MAX. + let oversized_depth = Felt::try_from(u64::from(u32::MAX) + 1).unwrap(); + let word = Word::from([Felt::from(1u32), Felt::from(1u32), oversized_depth, ZERO]); + let raw_attachment = + NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + let pswap = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .attachment(raw_attachment) + .build() + .unwrap(); + assert_eq!(pswap.parent_depth(), 0); +} + +/// `TryFrom<&NoteAttachment>` rejects a wrong scheme. +#[test] +fn try_from_rejects_wrong_scheme() { + let word = Word::from([Felt::from(1u32), Felt::from(2u32), Felt::from(1u32), ZERO]); + // Use NetworkAccountTarget (scheme = 2) instead of PSWAP_ATTACHMENT_SCHEME (3). + let other = NoteAttachment::with_word(StandardNoteAttachment::NetworkAccountTarget.attachment_scheme(), word); + assert!(PswapNoteAttachment::try_from(&other).is_err()); +} + +/// `TryFrom<&NoteAttachment>` rejects depth == 0 (the invariant only one path enforces). +#[test] +fn try_from_rejects_zero_depth() { + let word = Word::from([Felt::from(1u32), Felt::from(2u32), ZERO, ZERO]); + let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + assert!(PswapNoteAttachment::try_from(&att).is_err()); +} + +/// `TryFrom<&NoteAttachment>` rejects a depth value that exceeds `u32::MAX`. +#[test] +fn try_from_rejects_out_of_range_depth() { + let oversized = Felt::try_from(u64::from(u32::MAX) + 1).unwrap(); + let word = Word::from([Felt::from(1u32), Felt::from(2u32), oversized, ZERO]); + let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + assert!(PswapNoteAttachment::try_from(&att).is_err()); +} + +/// `TryFrom<&NoteAttachment>` rejects an amount that exceeds `AssetAmount::MAX`. +#[test] +fn try_from_rejects_invalid_amount() { + // 2^63 > AssetAmount::MAX = 2^63 - 2^31. + let bad_amount = Felt::try_from(1u64 << 63).unwrap(); + let word = Word::from([bad_amount, Felt::from(2u32), Felt::from(1u32), ZERO]); + let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + assert!(PswapNoteAttachment::try_from(&att).is_err()); +} + +/// `TryFrom<&NoteAttachment>` rejects an attachment whose word count is not 1 (covering the +/// MASM `num_words == 1` assert from the Rust side). +#[test] +fn try_from_rejects_wrong_num_words() { + let words = vec![Word::default(), Word::default()]; + let multi = NoteAttachment::with_words(PswapNote::PSWAP_ATTACHMENT_SCHEME, words).unwrap(); + assert!(PswapNoteAttachment::try_from(&multi).is_err()); +} + +/// `TryFrom<&NoteAttachment>` round-trips a valid attachment. +#[test] +fn try_from_round_trips_valid_attachment() { + let original = PswapNoteAttachment::new( + AssetAmount::new(123).unwrap(), + OrderId::from(Felt::from(7u32)), + NonZeroU32::new(5).unwrap(), + ); + let encoded: NoteAttachment = original.into(); + let decoded = PswapNoteAttachment::try_from(&encoded).unwrap(); + assert_eq!(decoded, original); +} + +/// `next_depth` propagates a `NoteError` if the parent depth is at `u32::MAX`. +#[test] +fn next_depth_errors_on_u32_overflow() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let attachment = PswapNoteAttachment::new( + AssetAmount::new(1).unwrap(), + OrderId::from(Felt::from(1u32)), + NonZeroU32::new(u32::MAX).unwrap(), + ); + let pswap = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .attachment(attachment.into()) + .build() + .unwrap(); + assert!(pswap.next_depth().is_err()); +} + +/// `OrderId` is a transparent newtype around `Felt`; both From conversions round-trip. +#[test] +fn order_id_round_trips_through_felt() { + let felt = Felt::from(123u32); + let oid = OrderId::from(felt); + assert_eq!(oid.as_felt(), felt); + assert_eq!(Felt::from(oid), felt); +} + +/// `PswapNote::order_id` is `serial_number[1]` wrapped as [`OrderId`]. +#[test] +fn pswap_note_order_id_equals_serial_1() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); + assert_eq!(Felt::from(pswap.order_id()), pswap.serial_number()[1]); +} + +/// `PswapNoteStorage::requested_asset_id` exposes the faucet ID as a 2-felt [`AssetId`]. +#[test] +fn requested_asset_id_packs_faucet_id() { + let creator_id = dummy_creator_id(); + let faucet_id = dummy_faucet_id(0xcc); + let requested_asset = FungibleAsset::new(faucet_id, 42).unwrap(); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let expected = AssetId::new(faucet_id.suffix(), faucet_id.prefix().as_felt()); + assert_eq!(storage.requested_asset_id(), expected); +} diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index 7f664f79d4..4b7179fb5e 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -385,10 +385,12 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { "remainder must use PSWAP_ATTACHMENT_SCHEME", ); + let order_id_felt = Felt::from(order_id); + // P2ID payback attachment word: [fill_amount, order_id, depth, 0]. let expected_p2id_word = Word::from([ Felt::try_from(fill_amount).expect("fill_amount fits in a felt"), - order_id, + order_id_felt, Felt::try_from(expected_depth).expect("depth fits in a felt"), ZERO, ]); @@ -401,7 +403,7 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { // Remainder PSWAP attachment word: [amt_payout, order_id, depth, 0]. let expected_remainder_word = Word::from([ Felt::try_from(expected_payout).expect("amt_payout fits in a felt"), - order_id, + order_id_felt, Felt::try_from(expected_depth).expect("depth fits in a felt"), ZERO, ]); @@ -425,7 +427,7 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { ); // Sanity: order_id must equal the original PSWAP's serial[1]. - assert_eq!(order_id, pswap.serial_number()[1], "order_id should equal serial[1]"); + assert_eq!(order_id_felt, pswap.serial_number()[1], "order_id should equal serial[1]"); Ok(()) } From a1b07eb542bd89f829d3c4d0e827da3df9c2d9c1 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 18:54:48 +0530 Subject: [PATCH 05/14] fix(pswap): reject zero offered/requested amounts in PswapNote::build Without this guard, calling calculate_offered_for_requested on a PswapNote with requested_amount == 0 panics with an unchecked integer division by zero at u128 / 0 in calculate_output_amount. The Rust panic is reachable only through direct API misuse (execute short-circuits via fill > requested), but it is still a Rust panic rather than a NoteError, which is the wrong failure mode for a typed builder API. Fix the root cause by rejecting zero on both sides in PswapNote::build: - offered_amount == 0 means the note pays out nothing on any fill (useless). - requested_amount == 0 is the div-by-zero path. Placing the check in build covers every constructor: the typed builder itself, and TryFrom<&Note> which funnels through it on the reconstruction path. Storage- and attachment-level reconstruction stays permissive because the on-chain MASM's u128::div would have rejected such a note anyway - the new check just moves that rejection to the Rust layer so callers get a typed NoteError instead of a panic. Two new unit tests cover both rejection paths. --- crates/miden-standards/src/note/pswap/mod.rs | 13 ++++++ .../miden-standards/src/note/pswap/tests.rs | 44 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 20aa22fade..416c6f7e64 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -131,6 +131,19 @@ where )); } + // Reject zero-amount assets: an offered amount of 0 means the note pays out nothing on + // any fill (useless), and a requested amount of 0 would divide by zero in + // `calculate_output_amount`. Catching both here makes the type unconditionally safe + // for all callers of `execute` / `calculate_offered_for_requested`, including the + // `TryFrom<&Note>` reconstruction path which funnels through this builder. + if note.offered_asset.amount() == AssetAmount::ZERO + || note.storage.requested_asset_amount() == AssetAmount::ZERO + { + return Err(NoteError::other( + "PSWAP offered and requested amounts must be non-zero", + )); + } + Ok(note) } } diff --git a/crates/miden-standards/src/note/pswap/tests.rs b/crates/miden-standards/src/note/pswap/tests.rs index 6599a2eb36..9d9f25e9d8 100644 --- a/crates/miden-standards/src/note/pswap/tests.rs +++ b/crates/miden-standards/src/note/pswap/tests.rs @@ -534,6 +534,50 @@ fn pswap_note_order_id_equals_serial_1() { assert_eq!(Felt::from(pswap.order_id()), pswap.serial_number()[1]); } +/// `PswapNote::build` rejects a zero requested amount (which would otherwise divide by zero +/// in `calculate_output_amount`). +#[test] +fn pswap_builder_rejects_zero_requested_amount() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 100).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 0).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let result = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .build(); + assert!(result.is_err(), "zero requested amount must be rejected"); +} + +/// `PswapNote::build` rejects a zero offered amount (the note would pay out nothing on any +/// fill, which is never economically useful). +#[test] +fn pswap_builder_rejects_zero_offered_amount() { + let creator_id = dummy_creator_id(); + let offered_asset = FungibleAsset::new(dummy_faucet_id(0xaa), 0).unwrap(); + let requested_asset = FungibleAsset::new(dummy_faucet_id(0xbb), 50).unwrap(); + let mut rng = RandomCoin::new(Word::default()); + let storage = PswapNoteStorage::builder() + .requested_asset(requested_asset) + .creator_account_id(creator_id) + .build(); + let result = PswapNote::builder() + .sender(creator_id) + .storage(storage) + .serial_number(rng.draw_word()) + .note_type(NoteType::Public) + .offered_asset(offered_asset) + .build(); + assert!(result.is_err(), "zero offered amount must be rejected"); +} + /// `PswapNoteStorage::requested_asset_id` exposes the faucet ID as a 2-felt [`AssetId`]. #[test] fn requested_asset_id_packs_faucet_id() { From 6bafbf2e8f98d821a34fffdb2b72730733b66094 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 18:54:49 +0530 Subject: [PATCH 06/14] fix(pswap): reject wrong-faucet fill assets in execute; pin reconstruction to original Address review feedback on PR inicio-labs/protocol#4: - execute(): when only one fill source is provided, the path bypassed FungibleAsset::add, which is the only place a faucet mismatch would have been caught. A caller could mint a payback note whose asset disagrees with the storage's requested_faucet_id. The MASM rejects this on-chain but the Rust API now catches it client-side via a per-source faucet check at the top of execute(). New unit test exercises both single-source arms. - payback_note() and remainder_note() advance the serial using the absolute lineage depth (depth - 1). Calling them on a remainder mid-lineage over-counts that delta and reconstructs the wrong note. Document the precondition (must be called on the original PSWAP) at the top of each doc comment so the contract is loud, without adding a runtime guard that would change the existing contract surface. --- crates/miden-standards/src/note/pswap/mod.rs | 22 +++++++++++++++++ .../miden-standards/src/note/pswap/tests.rs | 24 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 416c6f7e64..1d473e7d76 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -299,6 +299,7 @@ impl PswapNote { /// /// Returns an error if: /// - Both assets are `None`. + /// - Either fill asset's faucet does not match the requested faucet. /// - The fill amount is zero. /// - The fill amount exceeds the total requested amount. pub fn execute( @@ -307,6 +308,20 @@ impl PswapNote { account_fill_asset: Option, note_fill_asset: Option, ) -> Result<(Note, Option), NoteError> { + // Reject fill assets that aren't of the requested faucet. `FungibleAsset::add` catches + // mismatched faucets only when both fill sources are present; the single-source arms + // below bypass `add`, so a wrong-faucet asset would otherwise mint a payback note whose + // asset disagrees with the storage's `requested_faucet_id` (the MASM rejects it + // on-chain, but client-side reconstruction must catch it first). + let requested_faucet_id = self.storage.requested_faucet_id(); + for fill in [account_fill_asset, note_fill_asset].iter().flatten() { + if fill.faucet_id() != requested_faucet_id { + return Err(NoteError::other( + "fill asset faucet does not match the requested faucet", + )); + } + } + // Combine account fill and note fill into a single payback asset. let payback_asset = match (account_fill_asset, note_fill_asset) { (Some(account_fill), Some(note_fill)) => account_fill.add(note_fill).map_err(|e| { @@ -415,6 +430,10 @@ impl PswapNote { /// Reconstructs the depth-`d` payback P2ID [`Note`], so the creator can consume it as an /// unauthenticated input note. /// + /// Must be called on the original PSWAP (depth-0 root of the lineage); the serial advance + /// uses the absolute lineage depth and produces incorrect serials when called on a + /// remainder mid-lineage. + /// /// `consumer_account_id` must be the account that consumed the parent PSWAP in round /// `depth`: the MASM stamps it as the payback's metadata sender, which feeds into /// [`Note::details_commitment`]. @@ -459,6 +478,9 @@ impl PswapNote { /// Reconstructs the depth-`d` remainder PSWAP [`Note`] in this lineage. /// + /// Must be called on the original PSWAP (depth-0 root of the lineage); calling on a + /// remainder mid-lineage over-advances the serial and reconstructs the wrong note. + /// /// Called on the original PSWAP, this returns the full Note for the remainder produced /// in round `depth`. The returned Note matches the created note exactly. /// diff --git a/crates/miden-standards/src/note/pswap/tests.rs b/crates/miden-standards/src/note/pswap/tests.rs index 9d9f25e9d8..a5f1235924 100644 --- a/crates/miden-standards/src/note/pswap/tests.rs +++ b/crates/miden-standards/src/note/pswap/tests.rs @@ -578,6 +578,30 @@ fn pswap_builder_rejects_zero_offered_amount() { assert!(result.is_err(), "zero offered amount must be rejected"); } +/// `PswapNote::execute` rejects a single fill asset whose faucet differs from the requested +/// faucet (the previously-uncovered single-source path that bypassed `FungibleAsset::add`'s +/// own faucet check). +#[test] +fn pswap_execute_rejects_fill_asset_of_wrong_faucet() { + let creator_id = dummy_creator_id(); + let consumer_id = dummy_consumer_id(); + let offered_faucet = dummy_faucet_id(0xaa); + let requested_faucet = dummy_faucet_id(0xbb); + let wrong_faucet = dummy_faucet_id(0xcc); + + let offered_asset = FungibleAsset::new(offered_faucet, 100).unwrap(); + let requested_asset = FungibleAsset::new(requested_faucet, 50).unwrap(); + let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); + + // Wrong-faucet account fill asset (single source: would have skipped `add`'s check). + let bad_account_fill = FungibleAsset::new(wrong_faucet, 10).unwrap(); + assert!(pswap.execute(consumer_id, Some(bad_account_fill), None).is_err()); + + // Wrong-faucet note fill asset (single source on the other arm). + let bad_note_fill = FungibleAsset::new(wrong_faucet, 10).unwrap(); + assert!(pswap.execute(consumer_id, None, Some(bad_note_fill)).is_err()); +} + /// `PswapNoteStorage::requested_asset_id` exposes the faucet ID as a 2-felt [`AssetId`]. #[test] fn requested_asset_id_packs_faucet_id() { From 259c048318ff311b1a7bf7a3ec1f8e30d9be32c7 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 18:54:49 +0530 Subject: [PATCH 07/14] chore(pswap): apply nightly rustfmt to follow-up changes Re-flows imports, multi-line argument lists, and a few long expressions through cargo +nightly fmt with the repo's rustfmt.toml. No behavior change. Confirmed cargo clippy -p miden-standards --all-targets -D warnings and -p miden-testing --all-targets -D warnings are clean. --- .../src/note/pswap/attachment.rs | 6 +++- crates/miden-standards/src/note/pswap/mod.rs | 16 ++++----- .../miden-standards/src/note/pswap/storage.rs | 5 ++- .../miden-standards/src/note/pswap/tests.rs | 14 ++++---- crates/miden-testing/tests/scripts/pswap.rs | 33 ++++++++++++++----- 5 files changed, 46 insertions(+), 28 deletions(-) diff --git a/crates/miden-standards/src/note/pswap/attachment.rs b/crates/miden-standards/src/note/pswap/attachment.rs index d05b20a317..0ff1f19a13 100644 --- a/crates/miden-standards/src/note/pswap/attachment.rs +++ b/crates/miden-standards/src/note/pswap/attachment.rs @@ -84,6 +84,10 @@ impl TryFrom<&NoteAttachment> for PswapNoteAttachment { let depth = NonZeroU32::new(depth_u32) .ok_or_else(|| NoteError::other("PSWAP attachment depth must be non-zero"))?; - Ok(Self { amount, order_id: OrderId::from(word[1]), depth }) + Ok(Self { + amount, + order_id: OrderId::from(word[1]), + depth, + }) } } diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 1d473e7d76..9ee3cf6764 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -139,9 +139,7 @@ where if note.offered_asset.amount() == AssetAmount::ZERO || note.storage.requested_asset_amount() == AssetAmount::ZERO { - return Err(NoteError::other( - "PSWAP offered and requested amounts must be non-zero", - )); + return Err(NoteError::other("PSWAP offered and requested amounts must be non-zero")); } Ok(note) @@ -181,8 +179,8 @@ impl PswapNote { /// /// - `account_fill` is the portion of the requested asset the consumer pays out of their own /// vault. - /// - `note_fill` is the portion sourced from another note in the same transaction (cross-swap - /// / net-zero flow). + /// - `note_fill` is the portion sourced from another note in the same transaction (cross-swap / + /// net-zero flow). /// /// Both values are in the requested asset's base units. In a network transaction the kernel /// defaults `NOTE_ARGS` to `[0, 0, 0, 0]` and the script falls back to a full fill, so this @@ -278,10 +276,9 @@ impl PswapNote { let requested_faucet_id = self.storage.requested_faucet_id(); let total_requested_amount = self.storage.requested_asset_amount(); - let fill_asset = - FungibleAsset::new(requested_faucet_id, total_requested_amount.as_u64()) - .map_err(|e| NoteError::other_with_source("failed to create full fill asset", e))? - .with_callbacks(self.storage.requested_asset().callbacks()); + let fill_asset = FungibleAsset::new(requested_faucet_id, total_requested_amount.as_u64()) + .map_err(|e| NoteError::other_with_source("failed to create full fill asset", e))? + .with_callbacks(self.storage.requested_asset().callbacks()); self.create_payback_note(consumer_account_id, fill_asset, total_requested_amount.as_u64()) } @@ -601,7 +598,6 @@ impl PswapNote { Ok(amount) } - /// Builds a payback note (P2ID) that delivers the filled assets to the swap creator. /// /// The note inherits its type (public/private) from this PSWAP note and derives a diff --git a/crates/miden-standards/src/note/pswap/storage.rs b/crates/miden-standards/src/note/pswap/storage.rs index f93d6f6171..dde87e57ff 100644 --- a/crates/miden-standards/src/note/pswap/storage.rs +++ b/crates/miden-standards/src/note/pswap/storage.rs @@ -93,7 +93,10 @@ impl PswapNoteStorage { /// the requested fungible faucet's ID in that shape so callers indexing PSWAP storage by /// asset can pattern-match alongside other 2-felt asset identifiers. pub fn requested_asset_id(&self) -> AssetId { - AssetId::new(self.requested_asset.faucet_id().suffix(), self.requested_asset.faucet_id().prefix().as_felt()) + AssetId::new( + self.requested_asset.faucet_id().suffix(), + self.requested_asset.faucet_id().prefix().as_felt(), + ) } /// Returns the requested token amount. diff --git a/crates/miden-standards/src/note/pswap/tests.rs b/crates/miden-standards/src/note/pswap/tests.rs index a5f1235924..32469e3eb5 100644 --- a/crates/miden-standards/src/note/pswap/tests.rs +++ b/crates/miden-standards/src/note/pswap/tests.rs @@ -105,11 +105,7 @@ fn pswap_tag() { ) .unwrap(); let requested_asset = FungibleAsset::new( - AccountId::dummy( - requested_faucet_bytes, - AccountIdVersion::Version1, - AccountType::Public, - ), + AccountId::dummy(requested_faucet_bytes, AccountIdVersion::Version1, AccountType::Public), 200, ) .unwrap(); @@ -415,8 +411,7 @@ fn parent_depth_zero_on_out_of_range_attachment() { // Stamp a raw word with a depth exceeding u32::MAX. let oversized_depth = Felt::try_from(u64::from(u32::MAX) + 1).unwrap(); let word = Word::from([Felt::from(1u32), Felt::from(1u32), oversized_depth, ZERO]); - let raw_attachment = - NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + let raw_attachment = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); let pswap = PswapNote::builder() .sender(creator_id) .storage(storage) @@ -434,7 +429,10 @@ fn parent_depth_zero_on_out_of_range_attachment() { fn try_from_rejects_wrong_scheme() { let word = Word::from([Felt::from(1u32), Felt::from(2u32), Felt::from(1u32), ZERO]); // Use NetworkAccountTarget (scheme = 2) instead of PSWAP_ATTACHMENT_SCHEME (3). - let other = NoteAttachment::with_word(StandardNoteAttachment::NetworkAccountTarget.attachment_scheme(), word); + let other = NoteAttachment::with_word( + StandardNoteAttachment::NetworkAccountTarget.attachment_scheme(), + word, + ); assert!(PswapNoteAttachment::try_from(&other).is_err()); } diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index 4b7179fb5e..55412af275 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -345,7 +345,10 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { let expected_depth = 1u64; // first fill of an original PSWAP let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO), + ); let (p2id_note, remainder_pswap) = pswap.execute(bob.id(), Some(eth_20), None)?; let remainder_note = @@ -525,7 +528,10 @@ async fn pswap_fill_test( if !use_network_account { let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO), + ); tx_builder = tx_builder.extend_note_args(note_args_map); } @@ -1166,7 +1172,10 @@ async fn pswap_multiple_partial_fills_test(#[case] fill_amount: u64) -> anyhow:: let mock_chain = builder.build()?; let mut note_args_map = BTreeMap::new(); - note_args_map.insert(pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); + note_args_map.insert( + pswap_note.id(), + PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO), + ); let payout_amount = pswap.calculate_offered_for_requested(fill_amount)?; let (p2id_note, remainder_pswap) = @@ -1744,7 +1753,10 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result }; let mut note_args_map = BTreeMap::new(); - note_args_map.insert(current_pswap_note.id(), PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO)); + note_args_map.insert( + current_pswap_note.id(), + PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO), + ); let bob_tx = mock_chain .build_tx_context(bob.id(), &[current_pswap_note.id()], &[])? @@ -1782,8 +1794,7 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result // --- Alice reconstructs the remainder (when partial) from on-chain data alone --- if next_pswap_opt.is_some() { let on_chain_remainder = bob_tx.output_notes().get_note(1); - let on_chain_remainder_att = - first_pswap_attachment(on_chain_remainder.attachments()); + let on_chain_remainder_att = first_pswap_attachment(on_chain_remainder.attachments()); let remainder_attachment = PswapNoteAttachment::new( on_chain_remainder_att.amount(), @@ -1896,8 +1907,14 @@ async fn pswap_disambiguates_multiple_creator_pswaps_in_same_tx() -> anyhow::Res // Bob partially fills BOTH PSWAPs in the same tx — 10 ETH from each. let fill_each = 10u64; let mut note_args = BTreeMap::new(); - note_args.insert(note_a.id(), PswapNote::create_args(AssetAmount::new(fill_each)?, AssetAmount::ZERO)); - note_args.insert(note_b.id(), PswapNote::create_args(AssetAmount::new(fill_each)?, AssetAmount::ZERO)); + note_args.insert( + note_a.id(), + PswapNote::create_args(AssetAmount::new(fill_each)?, AssetAmount::ZERO), + ); + note_args.insert( + note_b.id(), + PswapNote::create_args(AssetAmount::new(fill_each)?, AssetAmount::ZERO), + ); let (payback_a, remainder_a) = pswap_a.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_each)?), None)?; From 0565c0fc4ed2669523a017719395d22fa567bbfd Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 18:54:49 +0530 Subject: [PATCH 08/14] review(pswap): apply PR feedback on docs, types, and panic surface Addresses the 26 inline review comments on inicio-labs/protocol#4 in one pass. MASM (pswap.masm): - Drop the redundant 'see PSWAP_ATTACHMENT_SCHEME above' cross-reference and the self-explanatory comment on the num_words == 1 assert. - Shorten the depth-check comment to a one-line note on why u32assert.err is avoided (VM wraps the message at runtime). attachment.rs: - TryFrom now asserts word[3] == 0 (the reserved slot). Matches the encoder's invariant and stops malformed attachments from round-tripping to a different value. New unit test try_from_rejects_non_zero_reserved_slot. storage.rs: - Trim three doc blurbs (the on-chain-layout note, the payback_note_type rationale, the AssetId description) to focus on what the type is, not why. - Replace the .expect on NoteStorage::new with const _: () = assert! + unwrap_or_else(|_| unreachable!()), giving compile-time proof that the serialization path is unreachable on its error branch. mod.rs: - Drop 'lineage' from OrderId / order_id / payback_note / remainder_note docs; drop the network-account paragraphs from PswapNote / attachments() docs; shorten the zero-amount guard comment to one line. - Rename current_depth -> depth in code and docs throughout. - Migrate calculate_output_amount, calculate_offered_for_requested, execute, and create_remainder_pswap_note to take/return AssetAmount end-to-end (callers stop bouncing through u64). - Remove the .iter().flatten() on the 2-element fill array; use a small is_some_and closure instead. - Replace note.assets().iter().next().unwrap() + match Asset variant with note.assets().iter_fungible().next().ok_or_else(...) on the TryFrom<&Note> reconstruction path. - Replace the num_attachments() match + .expect on get(0) with a >1 guard followed by attachments.get(0).cloned() (no panic path). - Drop the now-redundant fill_amount param from create_payback_note; payback_asset.amount() carries it. - Drop the duplicated 'matching MASM add.1' / 'matching MASM movup.3 add.1 movdn.3' inline comments on serial derivations. tests.rs: - Update calculate_output_amount unit test to AssetAmount and assert the exact floor result for the non-integer ratio case (CodeRabbit #5). - Add try_from_rejects_non_zero_reserved_slot covering the new word[3] guard. miden-testing/tests/scripts/pswap.rs: - Update six calculate_offered_for_requested call sites to pass AssetAmount and unwrap the AssetAmount result back to u64 where downstream code needs it for vault/amount arithmetic. Verified: - cargo +nightly fmt --all clean. - cargo clippy -p miden-standards -p miden-testing --all-targets -- -D warnings clean on both crates. - 29 unit tests + 72 integration tests pass. --- .../asm/standards/notes/pswap.masm | 13 +- .../src/note/pswap/attachment.rs | 10 +- crates/miden-standards/src/note/pswap/mod.rs | 183 +++++++----------- .../miden-standards/src/note/pswap/storage.rs | 22 +-- .../miden-standards/src/note/pswap/tests.rs | 31 ++- crates/miden-testing/tests/scripts/pswap.rs | 20 +- 6 files changed, 131 insertions(+), 148 deletions(-) diff --git a/crates/miden-standards/asm/standards/notes/pswap.masm b/crates/miden-standards/asm/standards/notes/pswap.masm index 3dfe83546a..b294cff42a 100644 --- a/crates/miden-standards/asm/standards/notes/pswap.masm +++ b/crates/miden-standards/asm/standards/notes/pswap.masm @@ -96,7 +96,6 @@ const EXEC_AMT_REQUESTED_NOTE_FILL = 9 # write the word at PARENT_ATTACHMENT_PTR, read back only the depth at DEPTH_OFFSET. const PARENT_ATTACHMENT_PTR = 0 const PARENT_ATTACHMENT_DEPTH_OFFSET = 2 -# PswapAttachment is always exactly one word - see PSWAP_ATTACHMENT_SCHEME above. const PSWAP_ATTACHMENT_NUM_WORDS = 1 # ERRORS @@ -522,8 +521,6 @@ proc get_current_depth exec.active_note::write_attachment_to_memory # => [num_words] - # A PswapAttachment is exactly one word - assert before reading back from @locals(4) to - # rule out an attacker-crafted multi-word attachment overwriting adjacent local memory. eq.PSWAP_ATTACHMENT_NUM_WORDS assert.err=ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS # => [] @@ -531,13 +528,9 @@ proc get_current_depth loc_load.PARENT_ATTACHMENT_DEPTH_OFFSET # => [parent_depth] - # Defense-in-depth: assert depth fits in u32 by checking the high 32 bits are zero. The - # Rust side encodes u32 depths; a larger felt indicates a forged attachment. We avoid - # `u32assert.err=...` because the VM wraps its message at runtime, which would defeat - # exact-match assertions on the named error. - dup u32split - # => [lo, hi, parent_depth] (u32split puts lo on top) - drop + # Manual hi==0 check rather than u32assert.err=... because the VM wraps u32assert's + # message at runtime, which would defeat exact-match assertions on the named error. + dup u32split drop # => [hi, parent_depth] assertz.err=ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32 # => [parent_depth] diff --git a/crates/miden-standards/src/note/pswap/attachment.rs b/crates/miden-standards/src/note/pswap/attachment.rs index 0ff1f19a13..85a38dccfd 100644 --- a/crates/miden-standards/src/note/pswap/attachment.rs +++ b/crates/miden-standards/src/note/pswap/attachment.rs @@ -57,10 +57,8 @@ impl From for NoteAttachment { /// - the scheme is [`PswapNote::PSWAP_ATTACHMENT_SCHEME`]; /// - the content is exactly one word (`num_words == 1`); /// - the word's `amount` slot is a valid [`AssetAmount`]; -/// - the word's `depth` slot fits in a `u32` and is non-zero. -/// -/// The trailing slot (`word[3]`) is not asserted to be zero: PSWAP_ATTACHMENT_SCHEME's -/// canonical encoding leaves it for forward compatibility. +/// - the word's `depth` slot fits in a `u32` and is non-zero; +/// - the word's reserved slot (`word[3]`) is zero. impl TryFrom<&NoteAttachment> for PswapNoteAttachment { type Error = NoteError; @@ -84,6 +82,10 @@ impl TryFrom<&NoteAttachment> for PswapNoteAttachment { let depth = NonZeroU32::new(depth_u32) .ok_or_else(|| NoteError::other("PSWAP attachment depth must be non-zero"))?; + if word[3] != ZERO { + return Err(NoteError::other("PSWAP attachment reserved slot (word[3]) must be zero")); + } + Ok(Self { amount, order_id: OrderId::from(word[1]), diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 9ee3cf6764..552d08b523 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -3,7 +3,7 @@ use core::num::NonZeroU32; use miden_protocol::account::AccountId; use miden_protocol::assembly::Path; -use miden_protocol::asset::{Asset, AssetAmount, FungibleAsset}; +use miden_protocol::asset::{AssetAmount, FungibleAsset}; use miden_protocol::errors::NoteError; use miden_protocol::note::{ Note, @@ -49,13 +49,10 @@ static PSWAP_SCRIPT: LazyLock = LazyLock::new(|| { // ORDER ID // ================================================================================================ -/// Identifier of a PSWAP order, stable across the entire payback / remainder lineage. +/// Identifier of a PSWAP order, stable across every payback and remainder derived from it. /// /// Equal to `serial_number[1]` of the originating PSWAP and stamped verbatim into every /// downstream payback P2ID and remainder PSWAP's `PswapAttachment` (slot `[1]`). -/// -/// `OrderId` is a transparent newtype around [`Felt`]; conversions are provided so callers can -/// move between the typed and raw forms when interfacing with the MASM bridge. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)] pub struct OrderId(Felt); @@ -92,12 +89,6 @@ impl From for Felt { /// Unlike a regular SWAP note, consumers may fill it partially — the unfilled portion /// is re-created as a remainder note with an updated serial number, while the creator /// receives the filled portion via a payback note. -/// -/// The note can be consumed both in local transactions (where the consumer provides -/// fill amounts via note_args) and in network transactions (where note_args default to -/// `[0, 0, 0, 0]`, triggering a full fill). To route a PSWAP note to a network account, -/// set the `attachment` to a [`NetworkAccountTarget`](crate::note::NetworkAccountTarget) -/// via the builder. #[derive(Debug, Clone, bon::Builder)] #[builder(finish_fn(vis = "", name = build_internal))] pub struct PswapNote { @@ -131,11 +122,7 @@ where )); } - // Reject zero-amount assets: an offered amount of 0 means the note pays out nothing on - // any fill (useless), and a requested amount of 0 would divide by zero in - // `calculate_output_amount`. Catching both here makes the type unconditionally safe - // for all callers of `execute` / `calculate_offered_for_requested`, including the - // `TryFrom<&Note>` reconstruction path which funnels through this builder. + // Reject zero amounts: requested == 0 divides by zero downstream, offered == 0 is useless. if note.offered_asset.amount() == AssetAmount::ZERO || note.storage.requested_asset_amount() == AssetAmount::ZERO { @@ -218,16 +205,14 @@ impl PswapNote { /// Returns a reference to the note attachments. /// - /// For notes targeting a network account, this may contain a - /// [`NetworkAccountTarget`](crate::note::NetworkAccountTarget) with scheme = 2. For a - /// remainder PSWAP this contains the [`Self::PSWAP_ATTACHMENT_SCHEME`] word + /// For a remainder PSWAP this contains the [`Self::PSWAP_ATTACHMENT_SCHEME`] word /// `[amt_payout, order_id, depth, 0]`. For an original PSWAP (no prior fill), /// this is typically empty. pub fn attachments(&self) -> Option<&NoteAttachment> { self.attachment.as_ref() } - /// Returns the [`OrderId`] of this lineage, equal to `serial_number()[1]`. + /// Returns the [`OrderId`] of this note, equal to `serial_number()[1]`. pub fn order_id(&self) -> OrderId { OrderId::new(self.serial_number[1]) } @@ -236,8 +221,8 @@ impl PswapNote { /// or 0 if the note has no such attachment (i.e., it is the original PSWAP, not a /// remainder produced by an earlier fill). /// - /// The next round's `current_depth` is computed as `parent_depth() + 1`, matching the - /// on-chain `get_current_depth` MASM procedure. Use [`Self::next_depth`] for the typed + /// The next round's depth is computed as `parent_depth() + 1`, matching the on-chain + /// `get_current_depth` MASM procedure. Use [`Self::next_depth`] for the typed /// [`NonZeroU32`] form. A malformed `PSWAP_ATTACHMENT_SCHEME` attachment (depth out of /// `u32` range, depth == 0, etc.) is treated as if no attachment is present, so discovery /// degrades to "looks like an original" rather than corrupting downstream arithmetic. @@ -280,7 +265,7 @@ impl PswapNote { .map_err(|e| NoteError::other_with_source("failed to create full fill asset", e))? .with_callbacks(self.storage.requested_asset().callbacks()); - self.create_payback_note(consumer_account_id, fill_asset, total_requested_amount.as_u64()) + self.create_payback_note(consumer_account_id, fill_asset) } /// Executes the swap, producing the output notes for a given fill. @@ -305,18 +290,14 @@ impl PswapNote { account_fill_asset: Option, note_fill_asset: Option, ) -> Result<(Note, Option), NoteError> { - // Reject fill assets that aren't of the requested faucet. `FungibleAsset::add` catches - // mismatched faucets only when both fill sources are present; the single-source arms - // below bypass `add`, so a wrong-faucet asset would otherwise mint a payback note whose - // asset disagrees with the storage's `requested_faucet_id` (the MASM rejects it - // on-chain, but client-side reconstruction must catch it first). + // Reject fill assets of the wrong faucet (the single-source arms below bypass + // `FungibleAsset::add`, which is the only place a mismatch would otherwise be caught). let requested_faucet_id = self.storage.requested_faucet_id(); - for fill in [account_fill_asset, note_fill_asset].iter().flatten() { - if fill.faucet_id() != requested_faucet_id { - return Err(NoteError::other( - "fill asset faucet does not match the requested faucet", - )); - } + let wrong_faucet = |asset: &Option| -> bool { + asset.is_some_and(|a| a.faucet_id() != requested_faucet_id) + }; + if wrong_faucet(&account_fill_asset) || wrong_faucet(¬e_fill_asset) { + return Err(NoteError::other("fill asset faucet does not match the requested faucet")); } // Combine account fill and note fill into a single payback asset. @@ -334,14 +315,13 @@ impl PswapNote { )); }, }; - let fill_amount = payback_asset.amount().as_u64(); + let fill_amount = payback_asset.amount(); - let total_offered_amount = self.offered_asset.amount().as_u64(); + let total_offered_amount = self.offered_asset.amount(); let requested_faucet_id = self.storage.requested_faucet_id(); - let total_requested_amount = self.storage.requested_asset_amount().as_u64(); + let total_requested_amount = self.storage.requested_asset_amount(); - // Validate fill amount - if fill_amount == 0 { + if fill_amount == AssetAmount::ZERO { return Err(NoteError::other("Fill amount must be greater than 0")); } if fill_amount > total_requested_amount { @@ -352,12 +332,10 @@ impl PswapNote { ))); } - // Calculate payout amounts separately for account fill and note fill, matching the - // MASM which calls calculate_tokens_offered_for_requested twice. This is necessary - // because the account fill portion goes to the consumer's vault while the total - // determines the remainder note's offered amount. - let account_fill_amount = account_fill_asset.as_ref().map_or(0, |a| a.amount().as_u64()); - let note_fill_amount = note_fill_asset.as_ref().map_or(0, |a| a.amount().as_u64()); + // Mirror the MASM, which computes payouts separately for the account and note halves + // (the account half flows to the consumer, the sum drives the remainder's offered). + let account_fill_amount = account_fill_asset.map_or(AssetAmount::ZERO, |a| a.amount()); + let note_fill_amount = note_fill_asset.map_or(AssetAmount::ZERO, |a| a.amount()); let payout_for_account_fill = Self::calculate_output_amount( total_offered_amount, total_requested_amount, @@ -368,25 +346,26 @@ impl PswapNote { total_requested_amount, note_fill_amount, )?; - let offered_amount_for_fill = payout_for_account_fill + payout_for_note_fill; + let offered_amount_for_fill = (payout_for_account_fill + payout_for_note_fill) + .map_err(|e| NoteError::other_with_source("payout sum overflows max amount", e))?; - let payback_note = - self.create_payback_note(consumer_account_id, payback_asset, fill_amount)?; + let payback_note = self.create_payback_note(consumer_account_id, payback_asset)?; - // Create remainder note if partial fill let remainder = if fill_amount < total_requested_amount { - let remaining_offered = total_offered_amount - offered_amount_for_fill; - let remaining_requested = total_requested_amount - fill_amount; + let remaining_offered = (total_offered_amount - offered_amount_for_fill) + .map_err(|e| NoteError::other_with_source("remaining offered underflow", e))?; + let remaining_requested = (total_requested_amount - fill_amount) + .map_err(|e| NoteError::other_with_source("remaining requested underflow", e))?; let remaining_offered_asset = - FungibleAsset::new(self.offered_asset.faucet_id(), remaining_offered) + FungibleAsset::new(self.offered_asset.faucet_id(), remaining_offered.as_u64()) .map_err(|e| { NoteError::other_with_source("failed to create remainder asset", e) })? .with_callbacks(self.offered_asset.callbacks()); let remaining_requested_asset = - FungibleAsset::new(requested_faucet_id, remaining_requested) + FungibleAsset::new(requested_faucet_id, remaining_requested.as_u64()) .map_err(|e| { NoteError::other_with_source( "failed to create remaining requested asset", @@ -414,11 +393,15 @@ impl PswapNote { /// # Errors /// /// Returns an error if the calculated payout is not a valid asset amount. - pub fn calculate_offered_for_requested(&self, fill_amount: u64) -> Result { - let total_requested = self.storage.requested_asset_amount().as_u64(); - let total_offered = self.offered_asset.amount().as_u64(); - - Self::calculate_output_amount(total_offered, total_requested, fill_amount) + pub fn calculate_offered_for_requested( + &self, + fill_amount: AssetAmount, + ) -> Result { + Self::calculate_output_amount( + self.offered_asset.amount(), + self.storage.requested_asset_amount(), + fill_amount, + ) } // LINEAGE DISCOVERY @@ -427,9 +410,8 @@ impl PswapNote { /// Reconstructs the depth-`d` payback P2ID [`Note`], so the creator can consume it as an /// unauthenticated input note. /// - /// Must be called on the original PSWAP (depth-0 root of the lineage); the serial advance - /// uses the absolute lineage depth and produces incorrect serials when called on a - /// remainder mid-lineage. + /// Must be called on the original (depth-0) PSWAP; the serial advance uses the absolute + /// depth and over-counts when called on a remainder. /// /// `consumer_account_id` must be the account that consumed the parent PSWAP in round /// `depth`: the MASM stamps it as the payback's metadata sender, which feeds into @@ -473,10 +455,10 @@ impl PswapNote { )) } - /// Reconstructs the depth-`d` remainder PSWAP [`Note`] in this lineage. + /// Reconstructs the depth-`d` remainder PSWAP [`Note`]. /// - /// Must be called on the original PSWAP (depth-0 root of the lineage); calling on a - /// remainder mid-lineage over-advances the serial and reconstructs the wrong note. + /// Must be called on the original (depth-0) PSWAP; calling on a remainder over-advances + /// the serial and reconstructs the wrong note. /// /// Called on the original PSWAP, this returns the full Note for the remainder produced /// in round `depth`. The returned Note matches the created note exactly. @@ -583,19 +565,17 @@ impl PswapNote { /// /// Returns an error if the result does not fit in a valid [`AssetAmount`]. fn calculate_output_amount( - offered_total: u64, - requested_total: u64, - fill_amount: u64, - ) -> Result { - let product = (offered_total as u128) * (fill_amount as u128); - let quotient = product / (requested_total as u128); - let amount = u64::try_from(quotient) + offered_total: AssetAmount, + requested_total: AssetAmount, + fill_amount: AssetAmount, + ) -> Result { + let product = (offered_total.as_u64() as u128) * (fill_amount.as_u64() as u128); + let quotient = product / (requested_total.as_u64() as u128); + let raw = u64::try_from(quotient) .map_err(|_| NoteError::other("payout quotient does not fit in u64"))?; - // Validate the result is a valid fungible asset amount. - AssetAmount::new(amount).map_err(|e| { + AssetAmount::new(raw).map_err(|e| { NoteError::other_with_source("payout amount exceeds max fungible asset amount", e) - })?; - Ok(amount) + }) } /// Builds a payback note (P2ID) that delivers the filled assets to the swap creator. @@ -604,17 +584,15 @@ impl PswapNote { /// deterministic serial number by incrementing the least significant element of the /// serial number (`serial[0] + 1`). /// - /// The attachment carries `[fill_amount, order_id, current_depth, 0]` under - /// [`Self::PSWAP_ATTACHMENT_SCHEME`]. `current_depth` is `parent_depth + 1` — i.e., - /// the round number that produced this payback (1-indexed). + /// The attachment carries `[fill_amount, order_id, depth, 0]` under + /// [`Self::PSWAP_ATTACHMENT_SCHEME`]. `depth` is `parent_depth + 1` — i.e., the round + /// number that produced this payback (1-indexed). fn create_payback_note( &self, consumer_account_id: AccountId, payback_asset: FungibleAsset, - fill_amount: u64, ) -> Result { let payback_note_tag = self.storage.payback_note_tag(); - // Derive P2ID serial: increment least significant element (matching MASM add.1) let p2id_serial_num = Word::from([ self.serial_number[0] + ONE, self.serial_number[1], @@ -626,12 +604,9 @@ impl PswapNote { let recipient = P2idNoteStorage::new(self.storage.creator_account_id()).into_recipient(p2id_serial_num); - let current_depth = self.next_depth()?; - let fill_amount_typed = AssetAmount::new(fill_amount).map_err(|e| { - NoteError::other_with_source("fill amount is not a valid asset amount", e) - })?; + let depth = self.next_depth()?; let attachment: NoteAttachment = - PswapNoteAttachment::new(fill_amount_typed, self.order_id(), current_depth).into(); + PswapNoteAttachment::new(payback_asset.amount(), self.order_id(), depth).into(); let p2id_assets = NoteAssets::new(vec![payback_asset.into()])?; let p2id_metadata = @@ -651,7 +626,7 @@ impl PswapNote { /// The remainder inherits the original creator, tags, and note type, with an updated /// serial number (`serial[3] + 1`). /// - /// The attachment carries `[offered_amount_for_fill, order_id, current_depth, 0]` under + /// The attachment carries `[offered_amount_for_fill, order_id, depth, 0]` under /// [`Self::PSWAP_ATTACHMENT_SCHEME`]. The remainder must carry this attachment so that /// when *it* is later consumed as a parent, `get_current_depth` reads the right scheme /// and increments depth correctly. @@ -660,7 +635,7 @@ impl PswapNote { consumer_account_id: AccountId, remaining_offered_asset: FungibleAsset, remaining_requested_asset: FungibleAsset, - offered_amount_for_fill: u64, + offered_amount_for_fill: AssetAmount, ) -> Result { let new_storage = PswapNoteStorage::builder() .requested_asset(remaining_requested_asset) @@ -668,8 +643,6 @@ impl PswapNote { .payback_note_type(self.storage.payback_note_type()) .build(); - // Remainder serial: increment most significant element (matching MASM movup.3 add.1 - // movdn.3) let remainder_serial_num = Word::from([ self.serial_number[0], self.serial_number[1], @@ -677,12 +650,9 @@ impl PswapNote { self.serial_number[3] + ONE, ]); - let current_depth = self.next_depth()?; - let payout_typed = AssetAmount::new(offered_amount_for_fill).map_err(|e| { - NoteError::other_with_source("payout amount is not a valid asset amount", e) - })?; + let depth = self.next_depth()?; let attachment: NoteAttachment = - PswapNoteAttachment::new(payout_typed, self.order_id(), current_depth).into(); + PswapNoteAttachment::new(offered_amount_for_fill, self.order_id(), depth).into(); PswapNote::builder() .sender(consumer_account_id) @@ -734,20 +704,17 @@ impl TryFrom<&Note> for PswapNote { if note.assets().num_assets() != 1 { return Err(NoteError::other("PSWAP note must have exactly one asset")); } - let offered_asset = match note.assets().iter().next().unwrap() { - Asset::Fungible(fa) => *fa, - Asset::NonFungible(_) => { - return Err(NoteError::other("PSWAP note asset must be fungible")); - }, - }; - - let attachment = match note.attachments().num_attachments() { - 0 => None, - 1 => { - Some(note.attachments().get(0).expect("length should have been validated").clone()) - }, - _ => return Err(NoteError::other("pswap note supports only one attachment")), - }; + let offered_asset = note + .assets() + .iter_fungible() + .next() + .ok_or_else(|| NoteError::other("PSWAP note asset must be fungible"))?; + + let attachments = note.attachments(); + if attachments.num_attachments() > 1 { + return Err(NoteError::other("pswap note supports only one attachment")); + } + let attachment = attachments.get(0).cloned(); PswapNote::builder() .sender(note.metadata().sender()) diff --git a/crates/miden-standards/src/note/pswap/storage.rs b/crates/miden-standards/src/note/pswap/storage.rs index dde87e57ff..4fa0a06b90 100644 --- a/crates/miden-standards/src/note/pswap/storage.rs +++ b/crates/miden-standards/src/note/pswap/storage.rs @@ -4,7 +4,7 @@ use miden_protocol::account::AccountId; use miden_protocol::asset::{AssetAmount, AssetCallbackFlag, AssetId, FungibleAsset}; use miden_protocol::errors::NoteError; use miden_protocol::note::{NoteRecipient, NoteStorage, NoteTag, NoteType}; -use miden_protocol::{Felt, Word}; +use miden_protocol::{Felt, MAX_NOTE_STORAGE_ITEMS, Word}; use super::PswapNote; @@ -22,8 +22,7 @@ use super::PswapNote; /// | `[5-6]` | Creator account ID (prefix, suffix) | /// /// Slots `[1, 2]` together form a 2-element [`AssetId`] view of the faucet (see -/// [`Self::requested_asset_id`]). The on-chain layout is unchanged - the typed accessor is a -/// Rust-side convenience. +/// [`Self::requested_asset_id`]). /// /// The payback note tag is derived at runtime from the creator account ID /// (via `note_tag::create_account_target` in MASM) rather than stored. @@ -39,9 +38,7 @@ pub struct PswapNoteStorage { /// Note type of the payback note produced when the pswap is filled. Defaults to /// [`NoteType::Private`] because the payback carries the fill asset and is typically - /// consumed directly by the creator - a private note is cheaper in fees and bandwidth - /// and offers the same information (the fill amount is already recorded in the - /// executed transaction's output). + /// consumed directly by the creator. #[builder(default = NoteType::Private)] payback_note_type: NoteType, } @@ -87,11 +84,6 @@ impl PswapNoteStorage { } /// Returns the requested faucet's two-felt identity as an [`AssetId`]. - /// - /// Note: [`AssetId`] is the protocol-level vault-key newtype carrying `(suffix, prefix)`. It - /// is independent of the variant ID used for non-fungible assets - this accessor exposes - /// the requested fungible faucet's ID in that shape so callers indexing PSWAP storage by - /// asset can pattern-match alongside other 2-felt asset identifiers. pub fn requested_asset_id(&self) -> AssetId { AssetId::new( self.requested_asset.faucet_id().suffix(), @@ -105,6 +97,10 @@ impl PswapNoteStorage { } } +/// Compile-time proof that the PSWAP storage layout (7 items) always fits within the protocol's +/// per-note storage cap, so the `NoteStorage::new` call below is unreachable on its error path. +const _: () = assert!(PswapNoteStorage::NUM_STORAGE_ITEMS <= MAX_NOTE_STORAGE_ITEMS); + /// Serializes [`PswapNoteStorage`] into a 7-element [`NoteStorage`]. impl From for NoteStorage { fn from(storage: PswapNoteStorage) -> Self { @@ -120,8 +116,8 @@ impl From for NoteStorage { storage.creator_account_id.prefix().as_felt(), storage.creator_account_id.suffix(), ]; - NoteStorage::new(storage_items) - .expect("number of storage items should not exceed max storage items") + // Unreachable per the `const _: () = assert!` above. + NoteStorage::new(storage_items).unwrap_or_else(|_| unreachable!()) } } diff --git a/crates/miden-standards/src/note/pswap/tests.rs b/crates/miden-standards/src/note/pswap/tests.rs index 32469e3eb5..4ac3d03331 100644 --- a/crates/miden-standards/src/note/pswap/tests.rs +++ b/crates/miden-standards/src/note/pswap/tests.rs @@ -1,5 +1,5 @@ use miden_protocol::account::{AccountId, AccountIdVersion, AccountType}; -use miden_protocol::asset::{AssetCallbackFlag, AssetId, FungibleAsset}; +use miden_protocol::asset::{Asset, AssetCallbackFlag, AssetId, FungibleAsset}; use miden_protocol::crypto::rand::{FeltRng, RandomCoin}; use miden_protocol::note::NoteStorage; @@ -120,13 +120,22 @@ fn pswap_tag() { #[test] fn calculate_output_amount() { - assert_eq!(PswapNote::calculate_output_amount(100, 100, 50).unwrap(), 50); // Equal ratio - assert_eq!(PswapNote::calculate_output_amount(200, 100, 50).unwrap(), 100); // 2:1 ratio - assert_eq!(PswapNote::calculate_output_amount(100, 200, 50).unwrap(), 25); // 1:2 ratio + let amt = |x| AssetAmount::new(x).unwrap(); + assert_eq!( + PswapNote::calculate_output_amount(amt(100), amt(100), amt(50)).unwrap(), + amt(50) + ); // Equal ratio + assert_eq!( + PswapNote::calculate_output_amount(amt(200), amt(100), amt(50)).unwrap(), + amt(100) + ); // 2:1 ratio + assert_eq!( + PswapNote::calculate_output_amount(amt(100), amt(200), amt(50)).unwrap(), + amt(25) + ); // 1:2 ratio - // Non-integer ratio (100/73) - let result = PswapNote::calculate_output_amount(100, 73, 7).unwrap(); - assert!(result > 0, "Should produce non-zero output"); + // Non-integer ratio: floor(100 * 7 / 73) = floor(9.589) = 9 + assert_eq!(PswapNote::calculate_output_amount(amt(100), amt(73), amt(7)).unwrap(), amt(9)); } #[test] @@ -472,6 +481,14 @@ fn try_from_rejects_wrong_num_words() { assert!(PswapNoteAttachment::try_from(&multi).is_err()); } +/// `TryFrom<&NoteAttachment>` rejects an attachment whose reserved slot (word[3]) is non-zero. +#[test] +fn try_from_rejects_non_zero_reserved_slot() { + let word = Word::from([Felt::from(1u32), Felt::from(2u32), Felt::from(1u32), Felt::from(9u32)]); + let att = NoteAttachment::with_word(PswapNote::PSWAP_ATTACHMENT_SCHEME, word); + assert!(PswapNoteAttachment::try_from(&att).is_err()); +} + /// `TryFrom<&NoteAttachment>` round-trips a valid attachment. #[test] fn try_from_round_trips_valid_attachment() { diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index 55412af275..a8d89d2bde 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -244,7 +244,9 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( let remainder_pswap_att = first_pswap_attachment(output_remainder.attachments()); let amt_payout_from_attachment = remainder_pswap_att.amount().as_u64(); - let expected_payout = pswap.calculate_offered_for_requested(fill_amount_from_aux)?; + let expected_payout = pswap + .calculate_offered_for_requested(AssetAmount::new(fill_amount_from_aux)?)? + .as_u64(); assert_eq!( amt_payout_from_attachment, expected_payout, "remainder aux should carry amt_payout matching the Rust-side calc", @@ -515,7 +517,8 @@ async fn pswap_fill_test( }; let is_partial = fill_amount < requested_total; - let payout_amount = pswap.calculate_offered_for_requested(fill_amount)?; + let payout_amount = + pswap.calculate_offered_for_requested(AssetAmount::new(fill_amount)?)?.as_u64(); let mut expected_notes = vec![RawOutputNote::Full(p2id_note.clone())]; if let Some(remainder) = remainder_pswap { @@ -1177,7 +1180,8 @@ async fn pswap_multiple_partial_fills_test(#[case] fill_amount: u64) -> anyhow:: PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO), ); - let payout_amount = pswap.calculate_offered_for_requested(fill_amount)?; + let payout_amount = + pswap.calculate_offered_for_requested(AssetAmount::new(fill_amount)?)?.as_u64(); let (p2id_note, remainder_pswap) = pswap.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_amount)?), None)?; @@ -1249,7 +1253,8 @@ async fn run_partial_fill_ratio_case( PswapNote::create_args(AssetAmount::new(fill_eth)?, AssetAmount::ZERO), ); - let payout_amount = pswap.calculate_offered_for_requested(fill_eth)?; + let payout_amount = + pswap.calculate_offered_for_requested(AssetAmount::new(fill_eth)?)?.as_u64(); let remaining_offered = offered_usdc - payout_amount; assert!(payout_amount > 0, "payout_amount must be > 0"); @@ -1435,7 +1440,8 @@ async fn pswap_chained_partial_fills_test( PswapNote::create_args(AssetAmount::new(*fill_amount)?, AssetAmount::ZERO), ); - let payout_amount = pswap.calculate_offered_for_requested(*fill_amount)?; + let payout_amount = + pswap.calculate_offered_for_requested(AssetAmount::new(*fill_amount)?)?.as_u64(); let remaining_offered = current_offered - payout_amount; let (p2id_note, remainder_pswap) = pswap.execute( bob.id(), @@ -1732,7 +1738,9 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result let depth = NonZeroU32::new((idx + 1) as u32).expect("idx + 1 is always >= 1"); // --- Bob fills the current PSWAP --- - let payout_amount = current_pswap.calculate_offered_for_requested(fill_amount)?; + let payout_amount = current_pswap + .calculate_offered_for_requested(AssetAmount::new(fill_amount)?)? + .as_u64(); let remaining_offered = current_offered - payout_amount; let remaining_requested = current_requested - fill_amount; From ca43a6f5f45651ec5ce79cd122c576689940c92a Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 19:10:31 +0530 Subject: [PATCH 09/14] fix(pswap): remove .expect on NoteAssets::new in From for Note Mirrors the storage.rs fix from the previous commit. The expect was provably unreachable but still a panic-capable expression. Replace with a compile-time assertion (1 <= NoteAssets::MAX_NUM_ASSETS) plus unwrap_or_else(|_| unreachable!()) so the path is unreachable by construction. --- crates/miden-standards/src/note/pswap/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 552d08b523..350a237d2e 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -668,6 +668,10 @@ impl PswapNote { // CONVERSIONS // ================================================================================================ +/// Compile-time proof that a single-asset list always fits within the protocol's +/// per-note asset cap, so the `NoteAssets::new` call below is unreachable on its error path. +const _: () = assert!(1 <= NoteAssets::MAX_NUM_ASSETS); + /// Converts a [`PswapNote`] into a protocol [`Note`], computing the final PSWAP tag. impl From for Note { fn from(pswap: PswapNote) -> Self { @@ -679,8 +683,10 @@ impl From for Note { let recipient = pswap.storage.into_recipient(pswap.serial_number); - let assets = NoteAssets::new(vec![pswap.offered_asset.into()]) - .expect("single fungible asset should be valid"); + // Unreachable per the `const _: () = assert!` above (single-element vec, so the + // duplicate-detection loop never iterates). + let assets = + NoteAssets::new(vec![pswap.offered_asset.into()]).unwrap_or_else(|_| unreachable!()); let metadata = PartialNoteMetadata::new(pswap.sender, pswap.note_type).with_tag(tag); From 420bc00be788d63744d815f8bf04804a2193579c Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Fri, 29 May 2026 19:38:02 +0530 Subject: [PATCH 10/14] review(pswap): take Option in execute; misc cleanups Addresses the second review batch on inicio-labs/protocol#4. API: PswapNote::execute now takes Option on both fill slots instead of Option. The faucet is implicit (it must be the PSWAP's requested faucet), so passing a typed amount removes the only caller-visible mismatch surface. Callers no longer construct FungibleAsset client-side to invoke execute. Knock-on cleanups: - Drop the wrong-faucet guard added in 9365d381 and its unit test (unreachable now that callers can't pass a mismatched faucet). - Drop the 'Mirror the MASM' comment in execute. - Collapse map_or(AssetAmount::ZERO, |a| a.amount()) to a direct unwrap_or(AssetAmount::ZERO) per the AssetAmount signature. - Rename calculate_output_amount's intermediate to . MASM: drop the 2-line u32assert-rationale comment + blank line above the depth check; add a blank line after the # => [hi, parent_depth] annotation so the assertz lines visually separate. Tests: all 24 .execute() call sites in tests/scripts/pswap.rs and src/note/pswap/tests.rs updated to pass AssetAmount (or Asset::amount()) on the fill slots. 28 unit + 72 integration tests pass; cargo +nightly fmt and cargo clippy -p miden-standards -p miden-testing --all-targets -- -D warnings clean. --- .../asm/standards/notes/pswap.masm | 4 +- crates/miden-standards/src/note/pswap/mod.rs | 57 +++++++------------ .../miden-standards/src/note/pswap/tests.rs | 36 ++---------- crates/miden-testing/tests/scripts/pswap.rs | 57 ++++++++----------- 4 files changed, 52 insertions(+), 102 deletions(-) diff --git a/crates/miden-standards/asm/standards/notes/pswap.masm b/crates/miden-standards/asm/standards/notes/pswap.masm index b294cff42a..ad5e5ab115 100644 --- a/crates/miden-standards/asm/standards/notes/pswap.masm +++ b/crates/miden-standards/asm/standards/notes/pswap.masm @@ -527,11 +527,9 @@ proc get_current_depth loc_load.PARENT_ATTACHMENT_DEPTH_OFFSET # => [parent_depth] - - # Manual hi==0 check rather than u32assert.err=... because the VM wraps u32assert's - # message at runtime, which would defeat exact-match assertions on the named error. dup u32split drop # => [hi, parent_depth] + assertz.err=ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32 # => [parent_depth] else diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 350a237d2e..074ed22416 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -270,9 +270,9 @@ impl PswapNote { /// Executes the swap, producing the output notes for a given fill. /// - /// `account_fill_asset` is debited from the consumer's vault; `note_fill_asset` arrives - /// from another note in the same transaction (cross-swap). At least one must be - /// provided. + /// `account_fill` is debited from the consumer's vault; `note_fill` arrives from another + /// note in the same transaction (cross-swap). At least one must be provided. Both are + /// amounts of the requested asset; the faucet is implicit (the PSWAP's requested faucet). /// /// Returns `(payback_note, Option)`. The remainder is /// `None` when the fill equals the total requested amount (full fill). @@ -280,42 +280,26 @@ impl PswapNote { /// # Errors /// /// Returns an error if: - /// - Both assets are `None`. - /// - Either fill asset's faucet does not match the requested faucet. - /// - The fill amount is zero. - /// - The fill amount exceeds the total requested amount. + /// - Both fills are `None`. + /// - The combined fill amount is zero. + /// - The combined fill amount exceeds the total requested amount. pub fn execute( &self, consumer_account_id: AccountId, - account_fill_asset: Option, - note_fill_asset: Option, + account_fill: Option, + note_fill: Option, ) -> Result<(Note, Option), NoteError> { - // Reject fill assets of the wrong faucet (the single-source arms below bypass - // `FungibleAsset::add`, which is the only place a mismatch would otherwise be caught). - let requested_faucet_id = self.storage.requested_faucet_id(); - let wrong_faucet = |asset: &Option| -> bool { - asset.is_some_and(|a| a.faucet_id() != requested_faucet_id) - }; - if wrong_faucet(&account_fill_asset) || wrong_faucet(¬e_fill_asset) { - return Err(NoteError::other("fill asset faucet does not match the requested faucet")); - } - - // Combine account fill and note fill into a single payback asset. - let payback_asset = match (account_fill_asset, note_fill_asset) { - (Some(account_fill), Some(note_fill)) => account_fill.add(note_fill).map_err(|e| { - NoteError::other_with_source( - "failed to combine account fill and note fill assets", - e, - ) - })?, - (Some(asset), None) | (None, Some(asset)) => asset, + let account_fill_amount = account_fill.unwrap_or(AssetAmount::ZERO); + let note_fill_amount = note_fill.unwrap_or(AssetAmount::ZERO); + let fill_amount = match (account_fill, note_fill) { (None, None) => { return Err(NoteError::other( - "at least one of account_fill_asset or note_fill_asset must be provided", + "at least one of account_fill or note_fill must be provided", )); }, + _ => (account_fill_amount + note_fill_amount) + .map_err(|e| NoteError::other_with_source("fill sum overflows max amount", e))?, }; - let fill_amount = payback_asset.amount(); let total_offered_amount = self.offered_asset.amount(); let requested_faucet_id = self.storage.requested_faucet_id(); @@ -332,10 +316,8 @@ impl PswapNote { ))); } - // Mirror the MASM, which computes payouts separately for the account and note halves - // (the account half flows to the consumer, the sum drives the remainder's offered). - let account_fill_amount = account_fill_asset.map_or(AssetAmount::ZERO, |a| a.amount()); - let note_fill_amount = note_fill_asset.map_or(AssetAmount::ZERO, |a| a.amount()); + // Compute payouts separately for the account and note halves: the account portion flows + // to the consumer's vault, and the sum drives the remainder's offered amount. let payout_for_account_fill = Self::calculate_output_amount( total_offered_amount, total_requested_amount, @@ -349,6 +331,9 @@ impl PswapNote { let offered_amount_for_fill = (payout_for_account_fill + payout_for_note_fill) .map_err(|e| NoteError::other_with_source("payout sum overflows max amount", e))?; + let payback_asset = FungibleAsset::new(requested_faucet_id, fill_amount.as_u64()) + .map_err(|e| NoteError::other_with_source("failed to build payback asset", e))? + .with_callbacks(self.storage.requested_asset().callbacks()); let payback_note = self.create_payback_note(consumer_account_id, payback_asset)?; let remainder = if fill_amount < total_requested_amount { @@ -571,9 +556,9 @@ impl PswapNote { ) -> Result { let product = (offered_total.as_u64() as u128) * (fill_amount.as_u64() as u128); let quotient = product / (requested_total.as_u64() as u128); - let raw = u64::try_from(quotient) + let payout = u64::try_from(quotient) .map_err(|_| NoteError::other("payout quotient does not fit in u64"))?; - AssetAmount::new(raw).map_err(|e| { + AssetAmount::new(payout).map_err(|e| { NoteError::other_with_source("payout amount exceeds max fungible asset amount", e) }) } diff --git a/crates/miden-standards/src/note/pswap/tests.rs b/crates/miden-standards/src/note/pswap/tests.rs index 4ac3d03331..4a421f971a 100644 --- a/crates/miden-standards/src/note/pswap/tests.rs +++ b/crates/miden-standards/src/note/pswap/tests.rs @@ -192,8 +192,8 @@ fn pswap_execute_combined_account_fill_and_note_fill_partial_fill() { let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); // Account fill = 10, note fill = 20 → total fill = 30 (< 50, so partial). - let account_fill = FungibleAsset::new(requested_faucet, 10).unwrap(); - let note_fill = FungibleAsset::new(requested_faucet, 20).unwrap(); + let account_fill = AssetAmount::new(10).unwrap(); + let note_fill = AssetAmount::new(20).unwrap(); let (payback, remainder) = pswap.execute(consumer_id, Some(account_fill), Some(note_fill)).unwrap(); @@ -230,8 +230,8 @@ fn pswap_execute_combined_account_fill_and_note_fill_full_fill() { let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); // Account fill = 30, note fill = 20 → total fill = 50 (exactly requested). - let account_fill = FungibleAsset::new(requested_faucet, 30).unwrap(); - let note_fill = FungibleAsset::new(requested_faucet, 20).unwrap(); + let account_fill = AssetAmount::new(30).unwrap(); + let note_fill = AssetAmount::new(20).unwrap(); let (payback, remainder) = pswap.execute(consumer_id, Some(account_fill), Some(note_fill)).unwrap(); @@ -270,9 +270,7 @@ fn pswap_output_assets_preserve_callback_flag() { let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); // --- execute() (partial fill) --- - let account_fill = FungibleAsset::new(requested_faucet, 20) - .unwrap() - .with_callbacks(AssetCallbackFlag::Enabled); + let account_fill = AssetAmount::new(20).unwrap(); let (payback, remainder) = pswap.execute(consumer_id, Some(account_fill), None).unwrap(); let Asset::Fungible(fa) = payback.assets().iter().next().unwrap() else { @@ -593,30 +591,6 @@ fn pswap_builder_rejects_zero_offered_amount() { assert!(result.is_err(), "zero offered amount must be rejected"); } -/// `PswapNote::execute` rejects a single fill asset whose faucet differs from the requested -/// faucet (the previously-uncovered single-source path that bypassed `FungibleAsset::add`'s -/// own faucet check). -#[test] -fn pswap_execute_rejects_fill_asset_of_wrong_faucet() { - let creator_id = dummy_creator_id(); - let consumer_id = dummy_consumer_id(); - let offered_faucet = dummy_faucet_id(0xaa); - let requested_faucet = dummy_faucet_id(0xbb); - let wrong_faucet = dummy_faucet_id(0xcc); - - let offered_asset = FungibleAsset::new(offered_faucet, 100).unwrap(); - let requested_asset = FungibleAsset::new(requested_faucet, 50).unwrap(); - let (pswap, _) = build_pswap_note(offered_asset, requested_asset, creator_id); - - // Wrong-faucet account fill asset (single source: would have skipped `add`'s check). - let bad_account_fill = FungibleAsset::new(wrong_faucet, 10).unwrap(); - assert!(pswap.execute(consumer_id, Some(bad_account_fill), None).is_err()); - - // Wrong-faucet note fill asset (single source on the other arm). - let bad_note_fill = FungibleAsset::new(wrong_faucet, 10).unwrap(); - assert!(pswap.execute(consumer_id, None, Some(bad_note_fill)).is_err()); -} - /// `PswapNoteStorage::requested_asset_id` exposes the faucet ID as a 2-felt [`AssetId`]. #[test] fn requested_asset_id_packs_faucet_id() { diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index a8d89d2bde..267e1659b8 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -183,7 +183,7 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( ); let (p2id_note, remainder_pswap) = - pswap.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_amount)?), None)?; + pswap.execute(bob.id(), Some(AssetAmount::new(fill_amount)?), None)?; let mut expected_output_notes = vec![RawOutputNote::Full(p2id_note.clone())]; let predicted_remainder = if is_partial { @@ -352,7 +352,7 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { PswapNote::create_args(AssetAmount::new(fill_amount)?, AssetAmount::ZERO), ); - let (p2id_note, remainder_pswap) = pswap.execute(bob.id(), Some(eth_20), None)?; + let (p2id_note, remainder_pswap) = pswap.execute(bob.id(), Some(eth_20.amount()), None)?; let remainder_note = Note::from(remainder_pswap.expect("partial fill should produce remainder")); @@ -513,7 +513,7 @@ async fn pswap_fill_test( let p2id = pswap.execute_full_fill(consumer_id)?; (p2id, None) } else { - pswap.execute(consumer_id, Some(fill_asset), None)? + pswap.execute(consumer_id, Some(fill_asset.amount()), None)? }; let is_partial = fill_amount < requested_total; @@ -625,8 +625,8 @@ async fn pswap_note_note_fill_cross_swap_test() -> anyhow::Result<()> { ); // Expected P2ID notes - let (alice_p2id_note, _) = alice_pswap.execute(charlie.id(), None, Some(eth_25))?; - let (bob_p2id_note, _) = bob_pswap.execute(charlie.id(), None, Some(usdc_50))?; + let (alice_p2id_note, _) = alice_pswap.execute(charlie.id(), None, Some(eth_25.amount()))?; + let (bob_p2id_note, _) = bob_pswap.execute(charlie.id(), None, Some(usdc_50.amount()))?; let tx_context = mock_chain .build_tx_context(charlie.id(), &[alice_pswap_note.id(), bob_pswap_note.id()], &[])? @@ -732,15 +732,18 @@ async fn pswap_note_combined_account_fill_and_note_fill_test() -> anyhow::Result PswapNote::create_args(AssetAmount::ZERO, AssetAmount::new(60)?), ); - let (alice_p2id_note, alice_remainder) = - alice_pswap.execute(charlie.id(), Some(account_fill_eth), Some(note_fill_eth))?; + let (alice_p2id_note, alice_remainder) = alice_pswap.execute( + charlie.id(), + Some(account_fill_eth.amount()), + Some(note_fill_eth.amount()), + )?; assert!( alice_remainder.is_none(), "combined fill hits full fill — no remainder expected" ); let (bob_p2id_note, bob_remainder) = - bob_pswap.execute(charlie.id(), None, Some(bob_requested))?; + bob_pswap.execute(charlie.id(), None, Some(bob_requested.amount()))?; assert!(bob_remainder.is_none(), "bob pswap is filled completely via note_fill"); let tx_context = mock_chain @@ -1090,8 +1093,7 @@ async fn pswap_note_idx_nonzero_regression_test() -> anyhow::Result<()> { PswapNote::create_args(AssetAmount::new(25)?, AssetAmount::ZERO), ); - let (expected_p2id, _) = - pswap.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), 25)?), None)?; + let (expected_p2id, _) = pswap.execute(bob.id(), Some(AssetAmount::new(25)?), None)?; // Consume spawn first so the PSWAP-created P2ID gets note_idx == 1. let tx_context = mock_chain @@ -1183,7 +1185,7 @@ async fn pswap_multiple_partial_fills_test(#[case] fill_amount: u64) -> anyhow:: let payout_amount = pswap.calculate_offered_for_requested(AssetAmount::new(fill_amount)?)?.as_u64(); let (p2id_note, remainder_pswap) = - pswap.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_amount)?), None)?; + pswap.execute(bob.id(), Some(AssetAmount::new(fill_amount)?), None)?; let mut expected_notes = vec![RawOutputNote::Full(p2id_note)]; if let Some(remainder) = remainder_pswap { @@ -1261,7 +1263,7 @@ async fn run_partial_fill_ratio_case( assert!(payout_amount <= offered_usdc, "payout_amount > offered"); let (p2id_note, remainder_pswap) = - pswap.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_eth)?), None)?; + pswap.execute(bob.id(), Some(AssetAmount::new(fill_eth)?), None)?; let mut expected_notes = vec![RawOutputNote::Full(p2id_note)]; if remaining_requested > 0 { @@ -1443,11 +1445,8 @@ async fn pswap_chained_partial_fills_test( let payout_amount = pswap.calculate_offered_for_requested(AssetAmount::new(*fill_amount)?)?.as_u64(); let remaining_offered = current_offered - payout_amount; - let (p2id_note, remainder_pswap) = pswap.execute( - bob.id(), - Some(FungibleAsset::new(eth_faucet.id(), *fill_amount)?), - None, - )?; + let (p2id_note, remainder_pswap) = + pswap.execute(bob.id(), Some(AssetAmount::new(*fill_amount)?), None)?; let mut expected_notes = vec![RawOutputNote::Full(p2id_note)]; if remaining_requested > 0 { @@ -1557,9 +1556,8 @@ fn compare_pswap_create_output_notes_vs_test_helper() { assert_eq!(pswap.storage().creator_account_id(), alice.id(), "Creator ID mismatch"); // Full fill: should produce P2ID note, no remainder - let (p2id_note, remainder) = pswap - .execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), 25).unwrap()), None) - .unwrap(); + let (p2id_note, remainder) = + pswap.execute(bob.id(), Some(AssetAmount::new(25).unwrap()), None).unwrap(); assert!(remainder.is_none(), "Full fill should not produce remainder"); // Verify P2ID note properties @@ -1572,9 +1570,8 @@ fn compare_pswap_create_output_notes_vs_test_helper() { ); // Partial fill: should produce P2ID note + remainder - let (p2id_partial, remainder_partial) = pswap - .execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), 10).unwrap()), None) - .unwrap(); + let (p2id_partial, remainder_partial) = + pswap.execute(bob.id(), Some(AssetAmount::new(10).unwrap()), None).unwrap(); let remainder_pswap = remainder_partial.expect("Partial fill should produce remainder"); assert_eq!(p2id_partial.assets().num_assets(), 1); @@ -1660,8 +1657,7 @@ fn pswap_remainder_carries_pswap_scheme() -> anyhow::Result<()> { NoteType::Public, )?; - let account_fill = FungibleAsset::new(eth_faucet.id(), 10)?; - let (_, remainder_pswap) = pswap.execute(bob.id(), Some(account_fill), None)?; + let (_, remainder_pswap) = pswap.execute(bob.id(), Some(AssetAmount::new(10)?), None)?; let remainder_pswap = remainder_pswap.expect("partial fill should produce a remainder"); let att = remainder_pswap.attachments().expect("remainder must carry an attachment"); @@ -1744,11 +1740,8 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result let remaining_offered = current_offered - payout_amount; let remaining_requested = current_requested - fill_amount; - let (predicted_payback_note, predicted_remainder_pswap) = current_pswap.execute( - bob.id(), - Some(FungibleAsset::new(eth_faucet.id(), fill_amount)?), - None, - )?; + let (predicted_payback_note, predicted_remainder_pswap) = + current_pswap.execute(bob.id(), Some(AssetAmount::new(fill_amount)?), None)?; let mut expected_notes = vec![RawOutputNote::Full(predicted_payback_note.clone())]; let next_pswap_opt = if remaining_requested > 0 { @@ -1925,9 +1918,9 @@ async fn pswap_disambiguates_multiple_creator_pswaps_in_same_tx() -> anyhow::Res ); let (payback_a, remainder_a) = - pswap_a.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_each)?), None)?; + pswap_a.execute(bob.id(), Some(AssetAmount::new(fill_each)?), None)?; let (payback_b, remainder_b) = - pswap_b.execute(bob.id(), Some(FungibleAsset::new(eth_faucet.id(), fill_each)?), None)?; + pswap_b.execute(bob.id(), Some(AssetAmount::new(fill_each)?), None)?; let remainder_a_note = Note::from(remainder_a.expect("partial fill A produces remainder")); let remainder_b_note = Note::from(remainder_b.expect("partial fill B produces remainder")); From 5ccde288fa297542a42112cdc438b9f352f78aa0 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Sat, 30 May 2026 06:55:43 +0530 Subject: [PATCH 11/14] docs(pswap): refresh execute_full_fill doc to use new param names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc still referenced `account_fill_asset` / `note_fill_asset` — both renamed to `account_fill` / `note_fill` when execute() switched to Option. --- crates/miden-standards/src/note/pswap/mod.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 074ed22416..749367d0d1 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -252,11 +252,10 @@ impl PswapNote { /// Executes the swap as a full fill, producing only the payback note (no remainder). /// - /// Equivalent to calling [`Self::execute`] with `account_fill_asset` set to the full - /// requested amount and `note_fill_asset = None`. It also matches the on-chain - /// behavior when a note is consumed without explicit `note_args` (e.g. in a network - /// transaction, where the kernel defaults `note_args` to `[0, 0, 0, 0]` and the MASM - /// script falls back to a full fill). + /// Equivalent to calling [`Self::execute`] with `account_fill` set to the full requested + /// amount and `note_fill = None`. It also matches the on-chain behavior when a note is + /// consumed without explicit `note_args` (the kernel defaults `note_args` to + /// `[0, 0, 0, 0]` and the MASM script falls back to a full fill). pub fn execute_full_fill(&self, consumer_account_id: AccountId) -> Result { let requested_faucet_id = self.storage.requested_faucet_id(); let total_requested_amount = self.storage.requested_asset_amount(); From 068299407851faf3928da3f872c4c7283db3224c Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Sat, 30 May 2026 15:20:15 +0530 Subject: [PATCH 12/14] refactor(pswap): script() and script_root() return Result instead of expect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the last .expect on the PSWAP_SCRIPT LazyLock. The init can only fail if the standards library is built without the PSWAP procedure (a build-time invariant), but we now surface the failure as a NoteError so callers control the failure mode rather than getting a panic. Storage of the load result: - LazyLock> (NoteError is not Clone, so the error side is held as a String and re-wrapped into a fresh NoteError on each access via pswap_script_error). API ripple (all build-time-invariant, never expected to fire at runtime): - PswapNote::script() / ::script_root() / ::create_tag() now return Result. - PswapNoteStorage::into_recipient() now returns Result. - From for Note becomes TryFrom for Note. - StandardNote::from_script(), ::from_script_root(), ::script(), ::script_root() now return Result (the PSWAP arm propagates; other arms are wrapped in Ok). from_script* now return Result>. - TryFrom<&Note> for PswapNote uses script_root()? . Call-site updates: - Tests: 24 Note::from(pswap) / pswap.into() sites → Note::try_from(pswap)? . - crates/miden-tx executor + notes_checker: thread the Result through exec_host's StandardNote::from_script_root + .script() path, and treat the build-time failure as 'not a known standard note' in the checker's ordering / consumption-status paths so existing behavior is preserved. - bin/bench-transaction, miden-testing/tests/agglayer/bridge_out, miden-testing/tests/scripts/faucet, miden-testing/src/kernel_tests: minor ? / .unwrap() additions on infallible-by-construction call sites. Verified: cargo +nightly fmt --all clean; cargo clippy on miden-standards, miden-testing, miden-tx --all-targets -- -D warnings clean; 28 unit + 72 integration tests pass. --- bin/bench-transaction/src/context_setups.rs | 2 +- crates/miden-standards/src/note/mod.rs | 52 ++++++++++----- crates/miden-standards/src/note/pswap/mod.rs | 65 +++++++++++++------ .../miden-standards/src/note/pswap/storage.rs | 8 ++- .../miden-standards/src/note/pswap/tests.rs | 6 +- .../kernel_tests/tx/test_account_interface.rs | 2 +- .../tests/agglayer/bridge_out.rs | 2 +- crates/miden-testing/tests/scripts/faucet.rs | 2 +- crates/miden-testing/tests/scripts/pswap.rs | 40 ++++++------ crates/miden-tx/src/executor/exec_host.rs | 29 +++++---- crates/miden-tx/src/executor/notes_checker.rs | 10 ++- 11 files changed, 139 insertions(+), 79 deletions(-) diff --git a/bin/bench-transaction/src/context_setups.rs b/bin/bench-transaction/src/context_setups.rs index 75e47ef1f3..fddfae6e77 100644 --- a/bin/bench-transaction/src/context_setups.rs +++ b/bin/bench-transaction/src/context_setups.rs @@ -474,7 +474,7 @@ pub async fn tx_consume_b2agg_note(pre_populate_leaves: Option) -> Result Option { + /// + /// # Errors + /// + /// Propagates the script-load error from [`PswapNote::script_root`] (build-time invariant). + pub fn from_script(script: &NoteScript) -> Result, NoteError> { Self::from_script_root(script.root()) } /// Returns a [`StandardNote`] instance based on the provided script root. Returns `None` if /// the provided root does not match any standard note script. - pub fn from_script_root(root: NoteScriptRoot) -> Option { + /// + /// # Errors + /// + /// Propagates the script-load error from [`PswapNote::script_root`] (build-time invariant). + pub fn from_script_root(root: NoteScriptRoot) -> Result, NoteError> { if root == P2idNote::script_root() { - return Some(Self::P2ID); + return Ok(Some(Self::P2ID)); } if root == P2ideNote::script_root() { - return Some(Self::P2IDE); + return Ok(Some(Self::P2IDE)); } if root == SwapNote::script_root() { - return Some(Self::SWAP); + return Ok(Some(Self::SWAP)); } - if root == PswapNote::script_root() { - return Some(Self::PSWAP); + if root == PswapNote::script_root()? { + return Ok(Some(Self::PSWAP)); } if root == MintNote::script_root() { - return Some(Self::MINT); + return Ok(Some(Self::MINT)); } if root == BurnNote::script_root() { - return Some(Self::BURN); + return Ok(Some(Self::BURN)); } - None + Ok(None) } // PUBLIC ACCESSORS @@ -112,27 +120,35 @@ impl StandardNote { } /// Returns the note script of the current [StandardNote] instance. - pub fn script(&self) -> NoteScript { - match self { + /// + /// # Errors + /// + /// Propagates the script-load error from [`PswapNote::script`] (build-time invariant). + pub fn script(&self) -> Result { + Ok(match self { Self::P2ID => P2idNote::script(), Self::P2IDE => P2ideNote::script(), Self::SWAP => SwapNote::script(), - Self::PSWAP => PswapNote::script(), + Self::PSWAP => PswapNote::script()?, Self::MINT => MintNote::script(), Self::BURN => BurnNote::script(), - } + }) } /// Returns the script root of the current [StandardNote] instance. - pub fn script_root(&self) -> NoteScriptRoot { - match self { + /// + /// # Errors + /// + /// Propagates the script-load error from [`PswapNote::script_root`] (build-time invariant). + pub fn script_root(&self) -> Result { + Ok(match self { Self::P2ID => P2idNote::script_root(), Self::P2IDE => P2ideNote::script_root(), Self::SWAP => SwapNote::script_root(), - Self::PSWAP => PswapNote::script_root(), + Self::PSWAP => PswapNote::script_root()?, Self::MINT => MintNote::script_root(), Self::BURN => BurnNote::script_root(), - } + }) } /// Performs the inputs check of the provided standard note against the target account and the diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 749367d0d1..375a250c66 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -1,3 +1,4 @@ +use alloc::string::{String, ToString}; use alloc::vec; use core::num::NonZeroU32; @@ -38,14 +39,21 @@ pub use storage::PswapNoteStorage; /// Path to the PSWAP note script procedure in the standards library. const PSWAP_SCRIPT_PATH: &str = "::miden::standards::notes::pswap::main"; -// Initialize the PSWAP note script only once -static PSWAP_SCRIPT: LazyLock = LazyLock::new(|| { +// Cached load result for the PSWAP note script. The error path is reachable only if the +// standards library is built without the PSWAP procedure (a build-time invariant); we still +// surface it as a `Result` rather than panicking so callers control the failure mode. +// `NoteError` is not `Clone`, so the error side is stored as `String` and re-wrapped on each +// access. +static PSWAP_SCRIPT: LazyLock> = LazyLock::new(|| { let standards_lib = StandardsLib::default(); let path = Path::new(PSWAP_SCRIPT_PATH); - NoteScript::from_library_reference(standards_lib.as_ref(), path) - .expect("Standards library contains PSWAP note script procedure") + NoteScript::from_library_reference(standards_lib.as_ref(), path).map_err(|e| e.to_string()) }); +fn pswap_script_error(msg: &str) -> NoteError { + NoteError::other(alloc::format!("failed to load PSWAP note script: {msg}")) +} + // ORDER ID // ================================================================================================ @@ -152,13 +160,25 @@ impl PswapNote { // -------------------------------------------------------------------------------------------- /// Returns the compiled PSWAP note script. - pub fn script() -> NoteScript { - PSWAP_SCRIPT.clone() + /// + /// # Errors + /// + /// Returns an error if the standards library is built without the PSWAP procedure (a + /// build-time invariant; this should not happen in a valid build). + pub fn script() -> Result { + PSWAP_SCRIPT + .as_ref() + .map(NoteScript::clone) + .map_err(|msg| pswap_script_error(msg)) } /// Returns the root hash of the PSWAP note script. - pub fn script_root() -> NoteScriptRoot { - PSWAP_SCRIPT.root() + /// + /// # Errors + /// + /// Same condition as [`Self::script`]. + pub fn script_root() -> Result { + PSWAP_SCRIPT.as_ref().map(|s| s.root()).map_err(|msg| pswap_script_error(msg)) } /// Builds the `NOTE_ARGS` word that the PSWAP script expects when a consumer wants to fill @@ -487,11 +507,11 @@ impl PswapNote { .creator_account_id(self.storage.creator_account_id()) .payback_note_type(self.storage.payback_note_type()) .build(); - let recipient = new_storage.into_recipient(remainder_serial); + let recipient = new_storage.into_recipient(remainder_serial)?; let assets = NoteAssets::new(vec![offered_asset.into()])?; - let tag = Self::create_tag(self.note_type, &offered_asset, &requested_asset); + let tag = Self::create_tag(self.note_type, &offered_asset, &requested_asset)?; let metadata = PartialNoteMetadata::new(consumer_account_id, self.note_type).with_tag(tag); Ok(Note::with_attachments( @@ -517,8 +537,9 @@ impl PswapNote { note_type: NoteType, offered_asset: &FungibleAsset, requested_asset: &FungibleAsset, - ) -> NoteTag { - let pswap_root_bytes = Self::script().root().as_bytes(); + ) -> Result { + let script_root = Self::script_root()?; + let pswap_root_bytes = script_root.as_bytes(); // Construct the pswap use case ID from the 14 most significant bits of the script root. // This leaves the two most significant bits zero. @@ -538,7 +559,7 @@ impl PswapNote { | ((pswap_use_case_id as u32) << 16) | asset_pair as u32; - NoteTag::new(tag) + Ok(NoteTag::new(tag)) } /// Computes `floor((offered_total * fill_amount) / requested_total)` via a @@ -657,15 +678,21 @@ impl PswapNote { const _: () = assert!(1 <= NoteAssets::MAX_NUM_ASSETS); /// Converts a [`PswapNote`] into a protocol [`Note`], computing the final PSWAP tag. -impl From for Note { - fn from(pswap: PswapNote) -> Self { +/// +/// # Errors +/// +/// Propagates the script-load error from [`PswapNote::script`] (build-time invariant). +impl TryFrom for Note { + type Error = NoteError; + + fn try_from(pswap: PswapNote) -> Result { let tag = PswapNote::create_tag( pswap.note_type, &pswap.offered_asset, pswap.storage.requested_asset(), - ); + )?; - let recipient = pswap.storage.into_recipient(pswap.serial_number); + let recipient = pswap.storage.into_recipient(pswap.serial_number)?; // Unreachable per the `const _: () = assert!` above (single-element vec, so the // duplicate-detection loop never iterates). @@ -676,7 +703,7 @@ impl From for Note { let attachments = pswap.attachment.map(NoteAttachments::from).unwrap_or_default(); - Note::with_attachments(assets, metadata, recipient, attachments) + Ok(Note::with_attachments(assets, metadata, recipient, attachments)) } } @@ -685,7 +712,7 @@ impl TryFrom<&Note> for PswapNote { type Error = NoteError; fn try_from(note: &Note) -> Result { - if note.recipient().script().root() != PswapNote::script_root() { + if note.recipient().script().root() != PswapNote::script_root()? { return Err(NoteError::other("note script root does not match PSWAP script root")); } diff --git a/crates/miden-standards/src/note/pswap/storage.rs b/crates/miden-standards/src/note/pswap/storage.rs index 4fa0a06b90..87350e12fe 100644 --- a/crates/miden-standards/src/note/pswap/storage.rs +++ b/crates/miden-standards/src/note/pswap/storage.rs @@ -51,8 +51,12 @@ impl PswapNoteStorage { pub const NUM_STORAGE_ITEMS: usize = 7; /// Consumes the storage and returns a PSWAP [`NoteRecipient`] with the provided serial number. - pub fn into_recipient(self, serial_num: Word) -> NoteRecipient { - NoteRecipient::new(serial_num, PswapNote::script(), NoteStorage::from(self)) + /// + /// # Errors + /// + /// Propagates the error from [`PswapNote::script`] (build-time invariant). + pub fn into_recipient(self, serial_num: Word) -> Result { + Ok(NoteRecipient::new(serial_num, PswapNote::script()?, NoteStorage::from(self))) } // PUBLIC ACCESSORS diff --git a/crates/miden-standards/src/note/pswap/tests.rs b/crates/miden-standards/src/note/pswap/tests.rs index 4a421f971a..072d766c7d 100644 --- a/crates/miden-standards/src/note/pswap/tests.rs +++ b/crates/miden-standards/src/note/pswap/tests.rs @@ -40,7 +40,7 @@ fn build_pswap_note( .offered_asset(offered_asset) .build() .unwrap(); - let note: Note = pswap.clone().into(); + let note: Note = pswap.clone().try_into().unwrap(); (pswap, note) } @@ -58,7 +58,7 @@ fn pswap_note_creation_and_script() { assert_eq!(pswap.sender(), creator_id); assert_eq!(pswap.note_type(), NoteType::Public); - let script = PswapNote::script(); + let script = PswapNote::script().unwrap(); assert!(Word::from(script.root()) != Word::default(), "Script root should not be zero"); assert_eq!(note.metadata().sender(), creator_id); assert_eq!(note.metadata().note_type(), NoteType::Public); @@ -110,7 +110,7 @@ fn pswap_tag() { ) .unwrap(); - let tag = PswapNote::create_tag(NoteType::Public, &offered_asset, &requested_asset); + let tag = PswapNote::create_tag(NoteType::Public, &offered_asset, &requested_asset).unwrap(); let tag_u32 = u32::from(tag); // Verify note_type bits (top 2 bits should be 10 for Public) diff --git a/crates/miden-testing/src/kernel_tests/tx/test_account_interface.rs b/crates/miden-testing/src/kernel_tests/tx/test_account_interface.rs index ba09751323..322e3f093c 100644 --- a/crates/miden-testing/src/kernel_tests/tx/test_account_interface.rs +++ b/crates/miden-testing/src/kernel_tests/tx/test_account_interface.rs @@ -782,7 +782,7 @@ fn create_p2ide_note_with_storage( sender: AccountId, ) -> Note { let serial_num = RandomCoin::new(Default::default()).draw_word(); - let note_script = StandardNote::P2IDE.script(); + let note_script = StandardNote::P2IDE.script().unwrap(); let recipient = NoteRecipient::new( serial_num, note_script, diff --git a/crates/miden-testing/tests/agglayer/bridge_out.rs b/crates/miden-testing/tests/agglayer/bridge_out.rs index b7c336e68a..49882d039f 100644 --- a/crates/miden-testing/tests/agglayer/bridge_out.rs +++ b/crates/miden-testing/tests/agglayer/bridge_out.rs @@ -214,7 +214,7 @@ async fn bridge_out_consecutive() -> anyhow::Result<()> { ); assert_eq!( burn_note.recipient().script().root(), - StandardNote::BURN.script_root(), + StandardNote::BURN.script_root()?, "BURN note should use the BURN script" ); diff --git a/crates/miden-testing/tests/scripts/faucet.rs b/crates/miden-testing/tests/scripts/faucet.rs index c15b364d1d..78d60f8a49 100644 --- a/crates/miden-testing/tests/scripts/faucet.rs +++ b/crates/miden-testing/tests/scripts/faucet.rs @@ -1605,7 +1605,7 @@ async fn test_mint_note_output_note_types(#[case] note_type: NoteType) -> anyhow }, NoteType::Public => { let output_note_tag = NoteTag::with_account_target(target_account.id()); - let p2id_script = StandardNote::P2ID.script(); + let p2id_script = StandardNote::P2ID.script()?; let p2id_storage = vec![target_account.id().suffix(), target_account.id().prefix().as_felt()]; let note_storage = NoteStorage::new(p2id_storage)?; diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index 267e1659b8..d95e8d69e9 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -65,7 +65,7 @@ fn build_pswap_note( .note_type(note_type) .offered_asset(offered_asset) .build()?; - let note: Note = pswap.clone().into(); + let note: Note = pswap.clone().try_into()?; builder.add_output_note(RawOutputNote::Full(note.clone())); Ok((pswap, note)) } @@ -169,7 +169,7 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( .note_type(NoteType::Public) .offered_asset(offered_asset) .build()?; - let pswap_note: Note = pswap.clone().into(); + let pswap_note: Note = pswap.clone().try_into()?; builder.add_output_note(RawOutputNote::Full(pswap_note.clone())); let mut mock_chain = builder.build()?; @@ -188,7 +188,7 @@ async fn pswap_note_alice_reconstructs_and_consumes_p2id( let mut expected_output_notes = vec![RawOutputNote::Full(p2id_note.clone())]; let predicted_remainder = if is_partial { let r = remainder_pswap.expect("partial fill should produce remainder"); - let rn = Note::from(r); + let rn = Note::try_from(r)?; expected_output_notes.push(RawOutputNote::Full(rn.clone())); Some(rn) } else { @@ -354,7 +354,7 @@ async fn pswap_attachment_layout_matches_masm_test() -> anyhow::Result<()> { let (p2id_note, remainder_pswap) = pswap.execute(bob.id(), Some(eth_20.amount()), None)?; let remainder_note = - Note::from(remainder_pswap.expect("partial fill should produce remainder")); + Note::try_from(remainder_pswap.expect("partial fill should produce remainder"))?; let tx_context = mock_chain .build_tx_context(bob.id(), &[pswap_note.id()], &[])? @@ -522,7 +522,7 @@ async fn pswap_fill_test( let mut expected_notes = vec![RawOutputNote::Full(p2id_note.clone())]; if let Some(remainder) = remainder_pswap { - expected_notes.push(RawOutputNote::Full(Note::from(remainder))); + expected_notes.push(RawOutputNote::Full(Note::try_from(remainder)?)); } let mut tx_builder = mock_chain @@ -962,7 +962,7 @@ async fn pswap_assert_attachment_wrong_num_words() -> anyhow::Result<()> { .offered_asset(FungibleAsset::new(usdc_faucet.id(), 50)?) .attachment(bogus_attachment) .build()?; - let pswap_note: Note = pswap.clone().into(); + let pswap_note: Note = pswap.clone().try_into()?; builder.add_output_note(RawOutputNote::Full(pswap_note.clone())); let mock_chain = builder.build()?; @@ -1018,7 +1018,7 @@ async fn pswap_assert_attachment_depth_not_u32() -> anyhow::Result<()> { .offered_asset(FungibleAsset::new(usdc_faucet.id(), 50)?) .attachment(bogus_attachment) .build()?; - let pswap_note: Note = pswap.clone().into(); + let pswap_note: Note = pswap.clone().try_into()?; builder.add_output_note(RawOutputNote::Full(pswap_note.clone())); let mock_chain = builder.build()?; @@ -1189,7 +1189,7 @@ async fn pswap_multiple_partial_fills_test(#[case] fill_amount: u64) -> anyhow:: let mut expected_notes = vec![RawOutputNote::Full(p2id_note)]; if let Some(remainder) = remainder_pswap { - expected_notes.push(RawOutputNote::Full(Note::from(remainder))); + expected_notes.push(RawOutputNote::Full(Note::try_from(remainder)?)); } let tx_context = mock_chain @@ -1267,7 +1267,8 @@ async fn run_partial_fill_ratio_case( let mut expected_notes = vec![RawOutputNote::Full(p2id_note)]; if remaining_requested > 0 { - let remainder = Note::from(remainder_pswap.expect("partial fill should produce remainder")); + let remainder = + Note::try_from(remainder_pswap.expect("partial fill should produce remainder"))?; expected_notes.push(RawOutputNote::Full(remainder)); } @@ -1431,7 +1432,7 @@ async fn pswap_chained_partial_fills_test( .note_type(NoteType::Public) .offered_asset(offered_fungible) .build()?; - let pswap_note: Note = pswap.clone().into(); + let pswap_note: Note = pswap.clone().try_into()?; builder.add_output_note(RawOutputNote::Full(pswap_note.clone())); let mock_chain = builder.build()?; @@ -1451,7 +1452,7 @@ async fn pswap_chained_partial_fills_test( let mut expected_notes = vec![RawOutputNote::Full(p2id_note)]; if remaining_requested > 0 { let remainder = - Note::from(remainder_pswap.expect("partial fill should produce remainder")); + Note::try_from(remainder_pswap.expect("partial fill should produce remainder"))?; expected_notes.push(RawOutputNote::Full(remainder)); } @@ -1540,7 +1541,8 @@ fn compare_pswap_create_output_notes_vs_test_helper() { .offered_asset(FungibleAsset::new(usdc_faucet.id(), 50).unwrap()) .build() .unwrap() - .into(); + .try_into() + .unwrap(); // Roundtrip: try_from -> execute -> verify outputs let pswap = PswapNote::try_from(&pswap_note).unwrap(); @@ -1717,7 +1719,7 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result .note_type(NoteType::Public) .offered_asset(FungibleAsset::new(usdc_faucet.id(), initial_offered)?) .build()?; - let original_pswap_note: Note = original_pswap.clone().into(); + let original_pswap_note: Note = original_pswap.clone().try_into()?; builder.add_output_note(RawOutputNote::Full(original_pswap_note.clone())); let mut mock_chain = builder.build()?; @@ -1747,7 +1749,7 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result let next_pswap_opt = if remaining_requested > 0 { let predicted_remainder = predicted_remainder_pswap.expect("partial fill should produce remainder"); - expected_notes.push(RawOutputNote::Full(Note::from(predicted_remainder.clone()))); + expected_notes.push(RawOutputNote::Full(Note::try_from(predicted_remainder.clone())?)); Some(predicted_remainder) } else { None @@ -1830,7 +1832,7 @@ async fn pswap_creator_reconstructs_lineage_from_attachments() -> anyhow::Result // Advance state for the next round. if let Some(next) = next_pswap_opt { - current_pswap_note = Note::from(next.clone()); + current_pswap_note = Note::try_from(next.clone())?; current_pswap = next; current_offered = remaining_offered; current_requested = remaining_requested; @@ -1899,8 +1901,8 @@ async fn pswap_disambiguates_multiple_creator_pswaps_in_same_tx() -> anyhow::Res assert_ne!(pswap_a.order_id(), pswap_b.order_id(), "test setup: order_ids must differ"); - let note_a: Note = pswap_a.clone().into(); - let note_b: Note = pswap_b.clone().into(); + let note_a: Note = pswap_a.clone().try_into()?; + let note_b: Note = pswap_b.clone().try_into()?; builder.add_output_note(RawOutputNote::Full(note_a.clone())); builder.add_output_note(RawOutputNote::Full(note_b.clone())); let mock_chain = builder.build()?; @@ -1921,8 +1923,8 @@ async fn pswap_disambiguates_multiple_creator_pswaps_in_same_tx() -> anyhow::Res pswap_a.execute(bob.id(), Some(AssetAmount::new(fill_each)?), None)?; let (payback_b, remainder_b) = pswap_b.execute(bob.id(), Some(AssetAmount::new(fill_each)?), None)?; - let remainder_a_note = Note::from(remainder_a.expect("partial fill A produces remainder")); - let remainder_b_note = Note::from(remainder_b.expect("partial fill B produces remainder")); + let remainder_a_note = Note::try_from(remainder_a.expect("partial fill A produces remainder"))?; + let remainder_b_note = Note::try_from(remainder_b.expect("partial fill B produces remainder"))?; let tx_context = mock_chain .build_tx_context(bob.id(), &[note_a.id(), note_b.id()], &[])? diff --git a/crates/miden-tx/src/executor/exec_host.rs b/crates/miden-tx/src/executor/exec_host.rs index 68fd5e1f1f..3d2db0a395 100644 --- a/crates/miden-tx/src/executor/exec_host.rs +++ b/crates/miden-tx/src/executor/exec_host.rs @@ -394,17 +394,24 @@ where ) -> Result, TransactionKernelError> { // Resolve standard note scripts directly, avoiding a data store round-trip. let script_root = NoteScriptRoot::from_raw(script_root); - let note_script: Option = - if let Some(standard_note) = StandardNote::from_script_root(script_root) { - Some(standard_note.script()) - } else { - self.base_host.store().get_note_script(script_root).await.map_err(|err| { - TransactionKernelError::other_with_source( - "failed to retrieve note script from data store", - err, - ) - })? - }; + let standard_note = StandardNote::from_script_root(script_root).map_err(|err| { + TransactionKernelError::other_with_source("failed to resolve standard note", err) + })?; + let note_script: Option = if let Some(standard_note) = standard_note { + Some(standard_note.script().map_err(|err| { + TransactionKernelError::other_with_source( + "failed to load standard note script", + err, + ) + })?) + } else { + self.base_host.store().get_note_script(script_root).await.map_err(|err| { + TransactionKernelError::other_with_source( + "failed to retrieve note script from data store", + err, + ) + })? + }; match note_script { Some(note_script) => { diff --git a/crates/miden-tx/src/executor/notes_checker.rs b/crates/miden-tx/src/executor/notes_checker.rs index 54e1bbf490..959781f36e 100644 --- a/crates/miden-tx/src/executor/notes_checker.rs +++ b/crates/miden-tx/src/executor/notes_checker.rs @@ -187,7 +187,9 @@ where } // Ensure standard notes are ordered first. notes.sort_unstable_by_key(|note| { - StandardNote::from_script_root(note.script().root()).is_none() + // A script-load failure for PSWAP signals a corrupted build; treat the note as + // non-standard for ordering purposes and let downstream execution surface the error. + !matches!(StandardNote::from_script_root(note.script().root()), Ok(Some(_))) }); let notes = InputNotes::from(notes); @@ -219,8 +221,10 @@ where note: InputNote, tx_args: TransactionArgs, ) -> Result { - // Return the consumption status if we manage to determine it from the standard note - if let Some(standard_note) = StandardNote::from_script_root(note.note().script().root()) + // Return the consumption status if we manage to determine it from the standard note. + // A script-load failure (build-time invariant) is treated as "not a known standard + // note" for this resolution path; downstream execution surfaces the underlying issue. + if let Ok(Some(standard_note)) = StandardNote::from_script_root(note.note().script().root()) && let Some(consumption_status) = standard_note.is_consumable(note.note(), target_account_id, block_ref) { From 157eef565fc05008ec4f6c629b71753f406fd269 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Sat, 30 May 2026 15:51:18 +0530 Subject: [PATCH 13/14] docs(pswap): expand SAFETY comments on the two unreachable!() sites - storage.rs: explain that NoteStorage::new only fails on > MAX_NOTE_STORAGE_ITEMS, we always pass exactly NUM_STORAGE_ITEMS = 7, and the const _: () = assert! above proves the inequality at compile time. - mod.rs: explain that NoteAssets::new only fails on > MAX_NUM_ASSETS or on a duplicate, we always pass exactly one asset (covered by the const _: () = assert!), and the duplicate-detection loop skips index 0 so it never runs for a single-element vec. The old comments said 'unreachable per the const assert above' without spelling out WHY both failure conditions are ruled out. Future readers shouldn't have to dig into NoteStorage::new / NoteAssets::new to convince themselves. --- crates/miden-standards/src/note/pswap/mod.rs | 7 +++++-- crates/miden-standards/src/note/pswap/storage.rs | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index 375a250c66..aea076550f 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -694,8 +694,11 @@ impl TryFrom for Note { let recipient = pswap.storage.into_recipient(pswap.serial_number)?; - // Unreachable per the `const _: () = assert!` above (single-element vec, so the - // duplicate-detection loop never iterates). + // SAFETY: `NoteAssets::new` fails on (a) more than `MAX_NUM_ASSETS` assets, or (b) a + // duplicate asset in the list. We always pass exactly one asset, which the + // `const _: () = assert!` above proves to be within `MAX_NUM_ASSETS`. The + // duplicate-detection loop starts at index 1 (`.skip(1)`), so it never executes for + // a single-element vec. Both failure paths are unreachable here. let assets = NoteAssets::new(vec![pswap.offered_asset.into()]).unwrap_or_else(|_| unreachable!()); diff --git a/crates/miden-standards/src/note/pswap/storage.rs b/crates/miden-standards/src/note/pswap/storage.rs index 87350e12fe..d0db149458 100644 --- a/crates/miden-standards/src/note/pswap/storage.rs +++ b/crates/miden-standards/src/note/pswap/storage.rs @@ -120,7 +120,10 @@ impl From for NoteStorage { storage.creator_account_id.prefix().as_felt(), storage.creator_account_id.suffix(), ]; - // Unreachable per the `const _: () = assert!` above. + // SAFETY: `NoteStorage::new` only fails when its input exceeds + // `MAX_NOTE_STORAGE_ITEMS`. We always pass exactly `NUM_STORAGE_ITEMS = 7` items, and + // the `const _: () = assert!` above proves at compile time that + // `NUM_STORAGE_ITEMS <= MAX_NOTE_STORAGE_ITEMS`, so this branch is unreachable. NoteStorage::new(storage_items).unwrap_or_else(|_| unreachable!()) } } From c8164016236bd9e9bfd175fd9faf40e437b0f377 Mon Sep 17 00:00:00 2001 From: Vaibhav Jindal Date: Sat, 30 May 2026 16:33:36 +0530 Subject: [PATCH 14/14] simplify(pswap): use .unwrap() instead of .unwrap_or_else(|_| unreachable!()) The SAFETY comment above each call already proves the path is unreachable; .unwrap() carries the same semantics with less ceremony and gives a more informative panic message (the actual NoteError) if the impossible ever happens. --- crates/miden-standards/src/note/pswap/mod.rs | 3 +-- crates/miden-standards/src/note/pswap/storage.rs | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/miden-standards/src/note/pswap/mod.rs b/crates/miden-standards/src/note/pswap/mod.rs index aea076550f..c1c3269f35 100644 --- a/crates/miden-standards/src/note/pswap/mod.rs +++ b/crates/miden-standards/src/note/pswap/mod.rs @@ -699,8 +699,7 @@ impl TryFrom for Note { // `const _: () = assert!` above proves to be within `MAX_NUM_ASSETS`. The // duplicate-detection loop starts at index 1 (`.skip(1)`), so it never executes for // a single-element vec. Both failure paths are unreachable here. - let assets = - NoteAssets::new(vec![pswap.offered_asset.into()]).unwrap_or_else(|_| unreachable!()); + let assets = NoteAssets::new(vec![pswap.offered_asset.into()]).unwrap(); let metadata = PartialNoteMetadata::new(pswap.sender, pswap.note_type).with_tag(tag); diff --git a/crates/miden-standards/src/note/pswap/storage.rs b/crates/miden-standards/src/note/pswap/storage.rs index d0db149458..8efca61d7d 100644 --- a/crates/miden-standards/src/note/pswap/storage.rs +++ b/crates/miden-standards/src/note/pswap/storage.rs @@ -123,8 +123,8 @@ impl From for NoteStorage { // SAFETY: `NoteStorage::new` only fails when its input exceeds // `MAX_NOTE_STORAGE_ITEMS`. We always pass exactly `NUM_STORAGE_ITEMS = 7` items, and // the `const _: () = assert!` above proves at compile time that - // `NUM_STORAGE_ITEMS <= MAX_NOTE_STORAGE_ITEMS`, so this branch is unreachable. - NoteStorage::new(storage_items).unwrap_or_else(|_| unreachable!()) + // `NUM_STORAGE_ITEMS <= MAX_NOTE_STORAGE_ITEMS`, so this is unreachable. + NoteStorage::new(storage_items).unwrap() } }