diff --git a/CHANGELOG.md b/CHANGELOG.md index 75f355406d..5bdb61264a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ - Fixed `input_note::remove_asset` succeeding when asked to remove an empty or malformed asset ID instead of reporting the asset as not found ([#3592](https://github.com/0xMiden/protocol/pull/3607)). - Faucet asset-callback procedure roots are now verified against the faucet's account code before dispatch, so a misconfigured callback root can no longer make an asset nontransferable ([#3612](https://github.com/0xMiden/protocol/pull/3612)). - [BREAKING] Enforced the limit of 1024 per asset delta op for added and removed account vault deltas inside and outside the tx kernel ([#3623](https://github.com/0xMiden/protocol/pull/3623)). +- [BREAKING] Priced PSWAP fills against the note's initial offered asset rather than its remaining assets ([#3601](https://github.com/0xMiden/protocol/issues/3601)). - Fixed `AccountSchemaCommitment`'s `get_schema_commitment` returning above the 16-element stack depth ([#3645](https://github.com/0xMiden/protocol/pull/3645)). ## v0.16.0 (2026-08-17) diff --git a/crates/miden-standards/asm/standards/notes/pswap.masm b/crates/miden-standards/asm/standards/notes/pswap.masm index d460c28aba..e9315cb2a4 100644 --- a/crates/miden-standards/asm/standards/notes/pswap.masm +++ b/crates/miden-standards/asm/standards/notes/pswap.masm @@ -64,6 +64,10 @@ const PSWAP_CREATOR_PREFIX_ITEM = STORAGE_PTR + 6 const CALC_FILL_AMOUNT = 0 const CALC_FILL_REFERENCE = 1 +# Procedure locals of load_offered_asset +const OFFERED_REMAINING_ASSET_PTR = 0 +const OFFERED_INITIAL_ASSET_PTR = 8 + # create_p2id_note locals const P2ID_NOTE_IDX = 0 const P2ID_REQUESTED_FAUCET_SUFFIX = 1 @@ -100,6 +104,7 @@ const PARENT_ATTACHMENT_DEPTH_OFFSET = 2 const ERR_PSWAP_WRONG_NUMBER_OF_STORAGE_ITEMS="PSWAP script expects exactly 7 note storage items" const ERR_PSWAP_WRONG_NUMBER_OF_ASSETS="PSWAP script requires exactly one note asset" +const ERR_PSWAP_OFFERED_ASSET_ALTERED="PSWAP offered asset differs from the asset the note was created with" 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" @@ -449,23 +454,43 @@ proc is_consumer_creator # => [is_creator] end -#! Loads the offered asset from the active note and validates there is exactly one asset. +#! Loads the note's single offered asset, asserting it equals the amount the note was created with. #! #! Inputs: [] #! Outputs: [ASSET_ID, ASSET_VALUE] #! -@locals(8) +#! Panics if: +#! - the note does not carry exactly one remaining asset. +#! - the note was not created with exactly one asset. +#! - the remaining offered asset differs from the asset the note was created with. +#! +@locals(16) proc load_offered_asset - locaddr.0 exec.active_note::remove_all_assets + locaddr.OFFERED_REMAINING_ASSET_PTR exec.active_note::remove_all_assets # => [num_assets] push.1 eq assert.err=ERR_PSWAP_WRONG_NUMBER_OF_ASSETS # => [] - locaddr.0 - # => [dest_ptr] + # initial assets are unaffected by removals; require exactly one + locaddr.OFFERED_INITIAL_ASSET_PTR exec.active_note::get_initial_assets + # => [num_initial_assets] + + eq.1 assert.err=ERR_PSWAP_OFFERED_ASSET_ALTERED + # => [] + + locaddr.OFFERED_REMAINING_ASSET_PTR exec.asset::load + # => [ASSET_ID, ASSET_VALUE] + + locaddr.OFFERED_INITIAL_ASSET_PTR exec.asset::load + # => [INITIAL_ASSET_ID, INITIAL_ASSET_VALUE, ASSET_ID, ASSET_VALUE] + + # the remaining offered asset must equal the initial one + movupw.2 assert_eqw.err=ERR_PSWAP_OFFERED_ASSET_ALTERED + assert_eqw.err=ERR_PSWAP_OFFERED_ASSET_ALTERED + # => [] - exec.asset::load + locaddr.OFFERED_REMAINING_ASSET_PTR exec.asset::load # => [ASSET_ID, ASSET_VALUE] end @@ -808,6 +833,8 @@ end #! Panics if: #! - the number of note storage items is not `NUM_STORAGE_ITEMS`. #! - the note does not hold exactly one offered asset at consumption time. +#! - the offered asset remaining in the note differs from the asset the note was created with (i.e. +#! it was partially removed before the script ran). #! - the total fill (account_fill + note_fill) overflows u64 or exceeds the max asset amount. #! - the account does not expose `receive_asset` / `move_asset_to_note`. @note_script diff --git a/crates/miden-standards/src/note/pswap.rs b/crates/miden-standards/src/note/pswap.rs index ac360be6c9..0074e820b8 100644 --- a/crates/miden-standards/src/note/pswap.rs +++ b/crates/miden-standards/src/note/pswap.rs @@ -267,6 +267,8 @@ impl From for NoteAttachment { /// `[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. +/// +/// Fills are priced against the note's initial offered asset. #[derive(Debug, Clone, bon::Builder)] #[builder(finish_fn(vis = "", name = build_internal))] pub struct PswapNote { diff --git a/crates/miden-testing/tests/scripts/pswap.rs b/crates/miden-testing/tests/scripts/pswap.rs index f158f2d81a..35d2fe16aa 100644 --- a/crates/miden-testing/tests/scripts/pswap.rs +++ b/crates/miden-testing/tests/scripts/pswap.rs @@ -1,7 +1,14 @@ use std::collections::BTreeMap; use miden_protocol::account::auth::AuthScheme; -use miden_protocol::account::{Account, AccountId, AccountType, AccountVaultPatch}; +use miden_protocol::account::component::AccountComponentMetadata; +use miden_protocol::account::{ + Account, + AccountComponent, + AccountId, + AccountType, + AccountVaultPatch, +}; use miden_protocol::asset::{Asset, AssetAmount, AssetId, FungibleAsset}; use miden_protocol::crypto::rand::{FeltRng, RandomCoin}; use miden_protocol::errors::MasmError; @@ -10,14 +17,22 @@ use miden_protocol::testing::account_id::AccountIdBuilder; use miden_protocol::transaction::{RawOutputNote, RawOutputNotes}; use miden_protocol::{Felt, ONE, Word, ZERO}; use miden_standards::account::wallets::BasicWallet; +use miden_standards::code_builder::CodeBuilder; use miden_standards::errors::standards::{ ERR_PSWAP_FILL_BELOW_MINIMUM, ERR_PSWAP_FILL_SUM_OVERFLOW, ERR_PSWAP_NOT_VALID_ASSET_AMOUNT, + ERR_PSWAP_OFFERED_ASSET_ALTERED, }; use miden_standards::note::{PswapNote, PswapNoteAttachment, PswapNoteStorage}; use miden_standards::testing::note::NoteBuilder; -use miden_testing::{Auth, MockChain, MockChainBuilder, assert_transaction_executor_error}; +use miden_testing::{ + AccountState, + Auth, + MockChain, + MockChainBuilder, + assert_transaction_executor_error, +}; use rand::SeedableRng; use rand::rngs::SmallRng; use rstest::rstest; @@ -2072,3 +2087,131 @@ fn pswap_parse_inputs_roundtrip() { // Verify requested amount from value word assert_eq!(parsed.min_requested_amount(), 25, "Requested amount should be 25"); } + +/// Regression test for the offered-asset drain (issue #3601, PSWAP leg). +/// +/// A PSWAP note offers 1000 USDC for a minimum of 100 ETH. The consuming account exposes an +/// `@account_procedure` performing indexed input-note asset removal (`input_note::remove_asset`), +/// permitted only from the native-account context. An earlier helper input note (index 0) calls it +/// to drain 900 USDC out of the PSWAP note (index 1) before the PSWAP script runs. A partial fill +/// must then abort with `ERR_PSWAP_OFFERED_ASSET_ALTERED`. +#[tokio::test] +async fn pswap_note_offered_asset_drain_is_rejected_test() -> anyhow::Result<()> { + let mut builder = MockChain::builder(); + + let usdc_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "USDC", 10_000, Some(1_000))?; + let eth_faucet = builder.add_existing_basic_faucet(BASIC_AUTH, "ETH", 10_000, Some(100))?; + + let alice = AccountIdBuilder::new().build_with_seed([1; 32]); + + // A component that drains a specified asset from an input note by index into the account's own + // vault. + let drain_component = AccountComponent::new( + CodeBuilder::default().compile_component_code( + "attacker_account", + " + use miden::protocol::asset + use miden::protocol::input_note + use miden::protocol::native_account + + #! Removes the given asset from the input note at `note_index` and credits it to this + #! account's vault. + #! + #! Inputs: [ASSET_ID, ASSET_VALUE, note_index, pad(7)] + #! Outputs: [pad(16)] + @account_procedure + @locals(8) + pub proc drain_note_asset + # keep a copy of the asset so it can be credited to the vault after removal + dupw.1 dupw.1 locaddr.0 exec.asset::store + # => [ASSET_ID, ASSET_VALUE, note_index, pad(7)] + + exec.input_note::remove_asset + # => [FINAL_ASSET_VALUE, pad(7)] + + dropw + # => [pad(7)] + + # credit the drained asset to the consuming account's vault + locaddr.0 exec.asset::load exec.native_account::add_asset dropw + # => [pad(7)] + end + ", + )?, + Vec::new(), + AccountComponentMetadata::mock("attacker_account"), + )?; + + let consumer = builder.add_account_from_builder( + BASIC_AUTH, + Account::builder([9; 32]) + .account_type(AccountType::Public) + .with_component(BasicWallet) + .with_component(drain_component.clone()) + .with_assets([FungibleAsset::new(eth_faucet.id(), 100)?.into()]), + AccountState::Exists, + )?; + + // Alice's PSWAP note offers 1000 USDC for a minimum of 100 ETH. + let offered_asset = FungibleAsset::new(usdc_faucet.id(), 1_000)?; + let min_requested_asset = FungibleAsset::new(eth_faucet.id(), 100)?; + let (_, pswap_note) = build_pswap_note( + &mut builder, + alice, + offered_asset, + min_requested_asset, + NoteType::Public, + )?; + + // Helper note (input note index 0) that drains 900 of the 1000 offered USDC out of the PSWAP + // note (input note index 1) via the consuming account's procedure. + let drained = FungibleAsset::new(usdc_faucet.id(), 900)?; + let pswap_input_note_index = 1u8; + let helper_code = format!( + r#" + use miden::core::sys + + @note_script + pub proc main + # Drain the offered asset from the PSWAP note by index, via the native account's + # procedure, before the PSWAP script gets to consume its own remaining assets. + push.0.0.0.0 push.0.0.0 + push.{pswap_input_note_index} + push.{asset_value} + push.{asset_id} + call.::attacker_account::drain_note_asset + exec.sys::truncate_stack + end + "#, + asset_value = drained.to_value_word(), + asset_id = drained.to_id_word(), + ); + let helper_script = CodeBuilder::with_mock_packages() + .with_dynamically_linked_package(drain_component.component_code())? + .compile_note_script(helper_code)?; + let helper_note = NoteBuilder::new(alice, RandomCoin::new(Word::from([7, 7, 7, 7u32]))) + .note_type(NoteType::Public) + .script(helper_script) + .build()?; + // Commit the helper note so it can be consumed as an authenticated input note (index 0). + let helper_note_id = helper_note.id(); + builder.add_output_note(RawOutputNote::Full(helper_note)); + + let mock_chain = builder.build()?; + + // A 50-of-100 ETH partial fill, which exercises the remainder-note pricing path. + let mut note_args_map = BTreeMap::new(); + note_args_map.insert(pswap_note.id(), PswapNote::create_args(50, 0)?); + + let result = mock_chain + .build_transaction(consumer.id()) + .authenticated_input_notes([helper_note_id, pswap_note.id()]) + .extend_note_args(note_args_map) + .build()? + .execute() + .await; + + assert_transaction_executor_error!(result, ERR_PSWAP_OFFERED_ASSET_ALTERED); + + Ok(()) +}