diff --git a/CHANGELOG.md b/CHANGELOG.md index c3bc32afc6..42efabed75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Added the block kernel skeleton, establishing its public input/output contract and the `BlockExecutor` that runs it ([#3703](https://github.com/0xMiden/protocol/pull/3703)). - Added the `miden-objects` crate with canonical, `no_std`-compatible Protobuf representations and validated conversions for protocol objects exchanged between clients and nodes ([#3707](https://github.com/0xMiden/protocol/pull/3707)). - Added canonical Protobuf representations and validated conversions for `TransactionInputs` ([#3776](https://github.com/0xMiden/protocol/pull/3776)). +- Added `fee::assert_fee_bound`, an opt-in helper invoked between `fee::estimate_fee` and the payment to cap the payment at `num / den` times the computed fee and pin the payment asset to the native fee asset ([#3785](https://github.com/0xMiden/protocol/pull/3785)). ### Changes diff --git a/crates/miden-standards/asm/standards/fee/mod.masm b/crates/miden-standards/asm/standards/fee/mod.masm index 630660db8b..a3675fdf06 100644 --- a/crates/miden-standards/asm/standards/fee/mod.masm +++ b/crates/miden-standards/asm/standards/fee/mod.masm @@ -7,7 +7,7 @@ use miden::protocol::asset use miden::protocol::native_account use miden::protocol::output_note use miden::protocol::tx -use {Asset, AssetId} from miden::protocol::types +use {AccountId, Asset, AssetId} from miden::protocol::types use miden::standards::assets::fungible_asset use miden::standards::fees use miden::standards::notes::tx_fee @@ -19,6 +19,9 @@ use miden::standards::notes::tx_fee #! denominator are arbitrary non-zero field elements. pub type ConversionRate = struct { num: felt, den: felt } +#! An upper bound on a fee payment, expressed as the fraction num / den of the computed fee. +pub type FeeBound = struct { num: felt, den: felt } + #! Fee conversion info: the fungible faucet of the payment asset and the conversion rate from the #! native fee asset, laid out as [faucet_id_suffix, faucet_id_prefix, rate_num, rate_den]. #! The empty word indicates that no conversion info was committed. @@ -86,6 +89,12 @@ const ERR_FEE_CONVERSION_RATE_NUMERATOR_ZERO = "fee conversion rate numerator mu const ERR_FEE_CONVERTED_AMOUNT_OVERFLOW = "converted fee amount does not fit into a fungible asset amount" +const ERR_FEE_BOUND_DENOMINATOR_ZERO = "fee bound denominator must be non-zero" + +const ERR_FEE_PAYMENT_EXCEEDS_BOUND = "the fee payment exceeds the caller's bound on the computed fee" + +const ERR_FEE_PAYMENT_ASSET_NOT_NATIVE = "the fee payment asset must be the native fee asset" + # PROCEDURES # ================================================================================================= @@ -249,6 +258,64 @@ pub proc convert_amount(amount: felt, rate: ConversionRate) -> felt # => [converted_amount] end +#! Asserts that a fee payment is at most bound times the computed fee. +#! +#! The bound is an application-level policy; the caller picks the fraction and invokes this between +#! the fee estimate and the payment. +#! +#! The payment asset is pinned to the native fee asset. +#! +#! Inputs: [bound_num, bound_den, payment_faucet_id_suffix, payment_faucet_id_prefix, +#! payment_amount, fee_amount] +#! Outputs: [payment_faucet_id_suffix, payment_faucet_id_prefix, payment_amount] +#! +#! Where: +#! - bound_num and bound_den are the numerator and denominator of the largest multiple of the +#! computed fee the caller accepts. +#! - payment_faucet_id_{suffix,prefix} identify the faucet issuing the payment asset. +#! - payment_amount is the amount about to be paid. +#! - fee_amount is the fee computed by tx::compute_fee. +#! +#! Panics if: +#! - bound_den is zero. +#! - the payment asset is not the transaction's fee asset. +#! - payment_amount exceeds fee_amount * bound_num / bound_den. +#! +#! Invocation: exec +pub proc assert_fee_bound( + bound: FeeBound, + payment_faucet_id: AccountId, + payment_amount: felt, + fee_amount: felt, +) -> (payment_faucet_id: AccountId, payment_amount: felt) + # a zero denominator would make the bound vacuous + dup.1 neq.0 assert.err=ERR_FEE_BOUND_DENOMINATOR_ZERO + # => [bound_num, bound_den, payment_faucet_id_suffix, payment_faucet_id_prefix, payment_amount, + # fee_amount] + + # the fee is denominated in the native fee asset, so the payment must be too + dup.3 dup.3 exec.fungible_asset::create_id + # => [PAYMENT_ASSET_ID, bound_num, bound_den, payment_faucet_id_suffix, + # payment_faucet_id_prefix, payment_amount, fee_amount] + + exec.tx::get_fee_asset_id exec.word::eq assert.err=ERR_FEE_PAYMENT_ASSET_NOT_NATIVE + # => [bound_num, bound_den, payment_faucet_id_suffix, payment_faucet_id_prefix, payment_amount, + # fee_amount] + + # the bound holds iff payment_amount * bound_den <= fee_amount * bound_num; both sides are + # computed as 128-bit products, so the comparison is exact and cannot overflow + movup.5 u32split movup.2 u32split exec.u64::widening_mul + # => [max0, max1, max2, max3, bound_den, payment_faucet_id_suffix, payment_faucet_id_prefix, + # payment_amount] + + movup.4 u32split dup.8 u32split exec.u64::widening_mul + # => [paid0, paid1, paid2, paid3, max0, max1, max2, max3, payment_faucet_id_suffix, + # payment_faucet_id_prefix, payment_amount] + + swapw exec.u128::lte assert.err=ERR_FEE_PAYMENT_EXCEEDS_BOUND + # => [payment_faucet_id_suffix, payment_faucet_id_prefix, payment_amount] +end + #! Adds the internal cycle margins of a fee flow to the caller's estimate of remaining #! authentication cycles. #! diff --git a/crates/miden-testing/tests/auth/fee_payment/bound.rs b/crates/miden-testing/tests/auth/fee_payment/bound.rs new file mode 100644 index 0000000000..66df3ab235 --- /dev/null +++ b/crates/miden-testing/tests/auth/fee_payment/bound.rs @@ -0,0 +1,134 @@ +use miden_protocol::Word; +use miden_protocol::account::AccountId; +use miden_protocol::account::auth::AuthScheme; +use miden_protocol::asset::FungibleAsset; +use miden_protocol::testing::account_id::{ + ACCOUNT_ID_FEE_FAUCET, + ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_2, +}; +use miden_standards::account::auth::{FeeConversionInfo, commit_fee_conversion_info}; +use miden_standards::code_builder::CodeBuilder; +use miden_standards::errors::standards::{ + ERR_FEE_BOUND_DENOMINATOR_ZERO, + ERR_FEE_PAYMENT_ASSET_NOT_NATIVE, + ERR_FEE_PAYMENT_EXCEEDS_BOUND, +}; +use miden_testing::{Auth, MockChain, MockTransaction, assert_transaction_executor_error}; +use rstest::rstest; + +use super::VERIFICATION_BASE_FEE; + +/// Builds a transaction whose script runs `fee::assert_fee_bound` with the given inputs and +/// asserts that the payment comes back unchanged. +fn bound_transaction( + payment_faucet: AccountId, + payment_amount: u64, + fee_amount: u64, + bound_num: u64, + bound_den: u64, +) -> anyhow::Result { + let fee_faucet_id: AccountId = ACCOUNT_ID_FEE_FAUCET.try_into()?; + let fee_asset = FungibleAsset::new(fee_faucet_id, 1_000_000)?.into(); + + let mut builder = MockChain::builder().verification_base_fee(VERIFICATION_BASE_FEE); + let account = builder.add_existing_wallet_with_assets( + Auth::BasicAuth { + auth_scheme: AuthScheme::Falcon512Poseidon2, + }, + [fee_asset], + )?; + let mock_chain = builder.build()?; + + let src = format!( + r#" + use miden::standards::fee + + @transaction_script + pub proc main + push.{fee_amount}.{payment_amount} + push.{prefix}.{suffix} + push.{bound_den}.{bound_num} + exec.fee::assert_fee_bound + # the payment must come back unchanged + push.{suffix} assert_eq + push.{prefix} assert_eq + push.{payment_amount} assert_eq + end + "#, + suffix = payment_faucet.suffix(), + prefix = payment_faucet.prefix().as_felt(), + ); + let tx_script = CodeBuilder::default().compile_tx_script(&src)?; + + let (args, advice_value) = commit_fee_conversion_info( + FeeConversionInfo::one_to_one(fee_faucet_id), + Word::from([9u32, 10, 11, 12]), + ); + + mock_chain + .build_transaction(account.id()) + .auth_args(args) + .add_advice_map_entry(args, advice_value) + .tx_script(tx_script) + .build() +} + +/// Payments at or below the bound are accepted; the boundary itself is inclusive. +#[rstest] +#[case::exact_one_to_one(100, 100, 1, 1)] +#[case::exact_double(200, 100, 2, 1)] +#[case::exact_three_halves(150, 100, 3, 2)] +#[case::below_bound(1, 100, 2, 1)] +#[case::zero_fee_zero_payment(0, 0, 2, 1)] +// a naive u64 product would wrap 2^32 * 2^32 to zero and reject this +#[case::wide_bound_product(1, 4294967296, 4294967296, 1)] +#[tokio::test] +async fn within_bound_passes( + #[case] paid: u64, + #[case] fee: u64, + #[case] num: u64, + #[case] den: u64, +) -> anyhow::Result<()> { + let fee_faucet_id: AccountId = ACCOUNT_ID_FEE_FAUCET.try_into()?; + bound_transaction(fee_faucet_id, paid, fee, num, den)?.execute().await?; + Ok(()) +} + +/// Payments above the bound are rejected. +#[rstest] +#[case::one_over_double(201, 100, 2, 1)] +#[case::one_over_three_halves(151, 100, 3, 2)] +#[case::nonzero_payment_on_zero_fee(1, 0, 2, 1)] +// a naive u64 product would wrap 2^32 * 2^32 to zero and accept this +#[case::wide_payment_product(4294967296, 1, 1, 4294967296)] +#[tokio::test] +async fn exceeding_bound_aborts( + #[case] paid: u64, + #[case] fee: u64, + #[case] num: u64, + #[case] den: u64, +) -> anyhow::Result<()> { + let fee_faucet_id: AccountId = ACCOUNT_ID_FEE_FAUCET.try_into()?; + let result = bound_transaction(fee_faucet_id, paid, fee, num, den)?.execute().await; + assert_transaction_executor_error!(result, ERR_FEE_PAYMENT_EXCEEDS_BOUND); + Ok(()) +} + +/// A zero denominator would make the bound vacuous, so it is rejected. +#[tokio::test] +async fn zero_denominator_aborts() -> anyhow::Result<()> { + let fee_faucet_id: AccountId = ACCOUNT_ID_FEE_FAUCET.try_into()?; + let result = bound_transaction(fee_faucet_id, 100, 100, 1, 0)?.execute().await; + assert_transaction_executor_error!(result, ERR_FEE_BOUND_DENOMINATOR_ZERO); + Ok(()) +} + +/// The bound compares against a fee denominated in the native fee asset, so a payment in any +/// other asset is rejected. +#[tokio::test] +async fn non_native_payment_asset_aborts() -> anyhow::Result<()> { + let payment_faucet: AccountId = ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_2.try_into()?; + let result = bound_transaction(payment_faucet, 1, 100, 1, 1)?.execute().await; + assert_transaction_executor_error!(result, ERR_FEE_PAYMENT_ASSET_NOT_NATIVE); + Ok(()) +} diff --git a/crates/miden-testing/tests/auth/fee_payment/mod.rs b/crates/miden-testing/tests/auth/fee_payment/mod.rs index 81421cd4a4..09f3d9c652 100644 --- a/crates/miden-testing/tests/auth/fee_payment/mod.rs +++ b/crates/miden-testing/tests/auth/fee_payment/mod.rs @@ -4,6 +4,7 @@ use miden_protocol::testing::account_id::ACCOUNT_ID_FEE_FAUCET; use miden_protocol::transaction::ExecutedTransaction; use miden_standards::note::TxFeeNote; +mod bound; mod multisig; mod network; mod no_auth;