diff --git a/Cargo.lock b/Cargo.lock index f7061be748..79bf0dac56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4415,6 +4415,7 @@ dependencies = [ "miden-node-tracing", "miden-processor", "miden-protocol", + "miden-standards", "miden-testing", "miden-tx", "reqwest", diff --git a/crates/block-producer/src/domain/transaction.rs b/crates/block-producer/src/domain/transaction.rs index 55a2889864..44e2f2823f 100644 --- a/crates/block-producer/src/domain/transaction.rs +++ b/crates/block-producer/src/domain/transaction.rs @@ -1,36 +1,38 @@ -use miden_protocol::Word; +use miden_protocol::asset::AssetId; use miden_protocol::block::FeeParameters; -use miden_protocol::transaction::ProvenTransaction; +use miden_protocol::transaction::{OutputNote, ProvenTransaction}; use miden_standards::note::TxFeeNote; use crate::errors::MempoolSubmissionError; -/// Ensures that a transaction pays a non-zero fee when fees are enabled. +/// Requires a fee note when the reference block's verification base fee is nonzero. +/// Every fee note must contain exactly one asset with the specified native asset ID. /// -/// A zero verification base fee disables this check. Otherwise, the transaction must contain a -/// canonical fee output note with at least one non-zero asset. Validating that the amount is -/// sufficient for the transaction's execution cost is handled separately. +/// This check does not validate that the fee is sufficient for the transaction execution cost. pub fn ensure_transaction_has_fee( tx: &ProvenTransaction, + fee_asset_id: AssetId, fee_parameters: &FeeParameters, ) -> Result<(), MempoolSubmissionError> { - if fee_parameters.verification_base_fee() == 0 { - return Ok(()); + let fee_script_root = TxFeeNote::script_root(); + let mut contains_fee = false; + for note in tx.output_notes().iter() { + let OutputNote::Public(note) = note else { + continue; + }; + if note.recipient().script().root() != fee_script_root { + continue; + } + if !matches!(note.assets().as_slice(), [asset] if asset.id() == fee_asset_id) { + return Err(MempoolSubmissionError::InvalidFeeAsset { + transaction_id: tx.id(), + fee_asset_id, + }); + } + contains_fee = true; } - let fee_script_root = TxFeeNote::script_root(); - let contains_fee = tx.output_notes().iter().any(|note| { - let has_fee_script = note - .recipient() - .is_some_and(|recipient| recipient.script().root() == fee_script_root); - let has_non_zero_asset = note.assets().is_some_and(|assets| { - assets.iter().any(|asset| asset.to_value_word() != Word::empty()) - }); - - has_fee_script && has_non_zero_asset - }); - - if contains_fee { + if contains_fee || fee_parameters.verification_base_fee() == 0 { Ok(()) } else { Err(MempoolSubmissionError::MissingFee { transaction_id: tx.id() }) @@ -42,8 +44,10 @@ mod tests { use assert_matches::assert_matches; use miden_node_proto::{BuildUnchecked, DecodeMessage}; use miden_protocol::Word; - use miden_protocol::asset::FungibleAsset; + use miden_protocol::asset::{Asset, AssetId, FungibleAsset}; use miden_protocol::block::FeeParameters; + use miden_protocol::note::{Note, NoteAssets}; + use miden_protocol::testing::account_id::ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_1; use miden_protocol::transaction::{OutputNote, ProvenTransaction, PublicOutputNote}; use miden_standards::note::TxFeeNote; @@ -63,55 +67,104 @@ mod tests { assert_eq!(decoded, transaction); } - fn fee_parameters(verification_base_fee: u32) -> FeeParameters { - FeeParameters::new(verification_base_fee) + fn transaction_with_fee_amount(amount: u64) -> ProvenTransaction { + MockProvenTxBuilder::with_account_index(1) + .output_notes(vec![fee_output_note( + &[FungibleAsset::new(FungibleAsset::mock_issuer(), amount).unwrap().into()], + 1, + )]) + .build() } - fn transaction_with_fee_amount(amount: u64) -> ProvenTransaction { - let fee_note = TxFeeNote::builder() + fn fee_asset_id() -> AssetId { + AssetId::new_fungible(FungibleAsset::mock_issuer()) + } + + fn fee_output_note(assets: &[Asset], serial: u32) -> OutputNote { + let template: Note = TxFeeNote::builder() .sender(mock_account_id(1)) - .serial_number(Word::from([1u32, 2, 3, 4])) - .asset(FungibleAsset::new(FungibleAsset::mock_issuer(), amount).unwrap()) + .serial_number(Word::from([serial, 2, 3, 4])) + .asset(FungibleAsset::new(FungibleAsset::mock_issuer(), 1).unwrap()) .build() .unwrap() .into(); - - MockProvenTxBuilder::with_account_index(1) - .output_notes(vec![OutputNote::Public(PublicOutputNote::new(fee_note).unwrap())]) - .build() + let note = Note::new( + NoteAssets::new(assets.to_vec()).unwrap(), + *template.metadata().partial_metadata(), + template.recipient().clone(), + ); + OutputNote::Public(PublicOutputNote::new(note).unwrap()) } #[test] fn transaction_fee_requires_the_canonical_note_script() { let tx = transaction_with_fee_amount(1); - ensure_transaction_has_fee(&tx, &fee_parameters(1)).unwrap(); + ensure_transaction_has_fee(&tx, fee_asset_id(), &FeeParameters::new(1)).unwrap(); } #[test] - fn transaction_without_fee_is_rejected_when_fees_are_enabled() { + fn transaction_without_fee_is_rejected() { let tx = MockProvenTxBuilder::with_account_index(1).build(); assert_matches!( - ensure_transaction_has_fee(&tx, &fee_parameters(1)), + ensure_transaction_has_fee(&tx, fee_asset_id(), &FeeParameters::new(1)), Err(MempoolSubmissionError::MissingFee { transaction_id }) if transaction_id == tx.id() ); } #[test] - fn transaction_with_zero_fee_asset_is_rejected_when_fees_are_enabled() { + fn transaction_without_fee_is_accepted_when_fees_are_zero() { + let tx = MockProvenTxBuilder::with_account_index(1).build(); + + ensure_transaction_has_fee(&tx, fee_asset_id(), &FeeParameters::new(0)).unwrap(); + } + + #[test] + fn transaction_with_zero_fee_asset_is_accepted() { let tx = transaction_with_fee_amount(0); - assert_matches!( - ensure_transaction_has_fee(&tx, &fee_parameters(1)), - Err(MempoolSubmissionError::MissingFee { transaction_id }) if transaction_id == tx.id() - ); + ensure_transaction_has_fee(&tx, fee_asset_id(), &FeeParameters::new(1)).unwrap(); } #[test] - fn transaction_without_fee_is_accepted_when_fees_are_disabled() { - let tx = MockProvenTxBuilder::with_account_index(1).build(); + fn fee_notes_must_contain_only_the_native_asset() { + let native = FungibleAsset::new(FungibleAsset::mock_issuer(), 1).unwrap().into(); + let foreign_faucet = ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_1.try_into().unwrap(); + let foreign = FungibleAsset::new(foreign_faucet, 1).unwrap().into(); + let zero_foreign = FungibleAsset::new(foreign_faucet, 0).unwrap().into(); + for assets in [vec![], vec![foreign], vec![zero_foreign], vec![native, foreign]] { + let tx = MockProvenTxBuilder::with_account_index(1) + .output_notes(vec![fee_output_note(&assets, 1)]) + .build(); + for base_fee in [0, 1] { + assert_matches!( + ensure_transaction_has_fee(&tx, fee_asset_id(), &FeeParameters::new(base_fee)), + Err(MempoolSubmissionError::InvalidFeeAsset { transaction_id, .. }) + if transaction_id == tx.id() + ); + } + } + } - ensure_transaction_has_fee(&tx, &fee_parameters(0)).unwrap(); + #[test] + fn native_fee_note_does_not_allow_other_fee_notes_with_foreign_assets() { + let native = FungibleAsset::new(FungibleAsset::mock_issuer(), 1).unwrap().into(); + let foreign = + FungibleAsset::new(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_1.try_into().unwrap(), 1) + .unwrap() + .into(); + for assets in [[native, foreign], [foreign, native]] { + let tx = MockProvenTxBuilder::with_account_index(1) + .output_notes(vec![ + fee_output_note(&assets[..1], 1), + fee_output_note(&assets[1..], 2), + ]) + .build(); + assert_matches!( + ensure_transaction_has_fee(&tx, fee_asset_id(), &FeeParameters::new(1)), + Err(MempoolSubmissionError::InvalidFeeAsset { .. }) + ); + } } } diff --git a/crates/block-producer/src/errors.rs b/crates/block-producer/src/errors.rs index cb151b1d37..5b60e0e812 100644 --- a/crates/block-producer/src/errors.rs +++ b/crates/block-producer/src/errors.rs @@ -11,6 +11,7 @@ use miden_node_store::{ }; use miden_protocol::Word; use miden_protocol::account::AccountId; +use miden_protocol::asset::AssetId; use miden_protocol::block::BlockNumber; use miden_protocol::crypto::utils::DeserializationError; use miden_protocol::errors::{ProposedBatchError, ProposedBlockError, ProvenBatchError}; @@ -76,7 +77,7 @@ pub enum MempoolSubmissionError { #[error("the mempool is at capacity")] CapacityExceeded, - #[error("transaction {transaction_id} does not contain a non-zero TX_FEE output note")] + #[error("transaction {transaction_id} does not contain a canonical TX_FEE output note")] MissingFee { transaction_id: TransactionId }, #[error("transaction {transaction_id} consumes in-flight TX_FEE notes: {note_ids:?}")] @@ -88,6 +89,14 @@ pub enum MempoolSubmissionError { #[error("mempool lock is poisoned")] #[grpc(internal)] MempoolPoisoned(#[source] MempoolPoisonError), + + #[error( + "transaction {transaction_id} must use only the native asset {fee_asset_id} in each TX_FEE output note" + )] + InvalidFeeAsset { + transaction_id: TransactionId, + fee_asset_id: AssetId, + }, } // Mempool submission conflicts with current state diff --git a/crates/rpc/src/server/api/submit_auth_tx.rs b/crates/rpc/src/server/api/submit_auth_tx.rs index 46e97a2f2a..5c6d77005f 100644 --- a/crates/rpc/src/server/api/submit_auth_tx.rs +++ b/crates/rpc/src/server/api/submit_auth_tx.rs @@ -5,7 +5,7 @@ use miden_node_proto::{DecodeMessageExt, generated as proto}; use miden_node_tracing::ErrorReport; use tonic::Status; -use super::{SequencerInternalService, get_block_header_error_to_status}; +use super::{SequencerInternalService, get_block_header_error_to_status, load_protocol_config}; #[tonic::async_trait] impl sequencer_api::SubmitAuthenticatedTx for SequencerInternalService { @@ -54,8 +54,13 @@ impl sequencer_api::SubmitAuthenticatedTx for SequencerInternalService { ))); } - ensure_transaction_has_fee(tx.raw_proven_transaction(), reference_header.fee_parameters()) - .map_err(Status::from)?; + let protocol_config = load_protocol_config(&self.state.view(), &reference_header).await?; + ensure_transaction_has_fee( + tx.raw_proven_transaction(), + protocol_config.fee_asset_id(), + reference_header.fee_parameters(), + ) + .map_err(Status::from)?; self.block_producer .submit_authenticated_tx(tx) diff --git a/crates/rpc/src/server/api/submit_proven_tx.rs b/crates/rpc/src/server/api/submit_proven_tx.rs index 1b06705d2f..c95c850ff3 100644 --- a/crates/rpc/src/server/api/submit_proven_tx.rs +++ b/crates/rpc/src/server/api/submit_proven_tx.rs @@ -15,7 +15,7 @@ use miden_protocol::transaction::{ }; use tonic::{Request, Status}; -use super::{COMPONENT, RpcBackend, RpcService, submit_tx_to_validators}; +use super::{COMPONENT, RpcBackend, RpcService, load_protocol_config, submit_tx_to_validators}; use crate::LOG_TARGET; #[tonic::async_trait] @@ -76,7 +76,13 @@ impl proto::server::rpc_api::SubmitProvenTx for RpcService { let reference_header = self .verify_reference_commitment(tx.ref_block_num(), tx.ref_block_commitment()) .await?; - ensure_transaction_has_fee(&tx, reference_header.fee_parameters()).map_err(Status::from)?; + let protocol_config = load_protocol_config(&self.state.view(), &reference_header).await?; + ensure_transaction_has_fee( + &tx, + protocol_config.fee_asset_id(), + reference_header.fee_parameters(), + ) + .map_err(Status::from)?; // Rebuild a new ProvenTransaction with decorators removed from output notes let account_update = TxAccountUpdate::new( diff --git a/crates/rpc/src/server/api/submit_proven_tx_batch.rs b/crates/rpc/src/server/api/submit_proven_tx_batch.rs index 6efe93eb0f..11afd6d86c 100644 --- a/crates/rpc/src/server/api/submit_proven_tx_batch.rs +++ b/crates/rpc/src/server/api/submit_proven_tx_batch.rs @@ -76,7 +76,7 @@ impl proto::server::rpc_api::SubmitProvenTxBatch for RpcService { } } - // Verify the reference block is actually part of the chain. + // Verify that the reference block is part of the chain. self.verify_reference_commitment( proven_batch.reference_block_num(), proven_batch.reference_block_commitment(), diff --git a/crates/rpc/src/tests.rs b/crates/rpc/src/tests.rs index 039fd9dc76..dcb95f3ba0 100644 --- a/crates/rpc/src/tests.rs +++ b/crates/rpc/src/tests.rs @@ -6,6 +6,7 @@ use std::time::Duration; use http::header::{ACCEPT, CONTENT_TYPE}; use http::{Extensions, HeaderMap, HeaderValue}; +use miden_node_block_producer::store::get_tx_inputs; use miden_node_block_producer::{BlockProducerApi, BlockProducerApiConfig}; use miden_node_proto::clients::{ Builder, @@ -56,7 +57,7 @@ use miden_protocol::account::{ AccountUpdateDetails, AssetCallbackFlag, }; -use miden_protocol::asset::{Asset, FungibleAsset}; +use miden_protocol::asset::{Asset, AssetId, FungibleAsset}; use miden_protocol::batch::ProposedBatch; use miden_protocol::block::{ BlockSignatures, @@ -77,6 +78,7 @@ use miden_protocol::transaction::{ }; use miden_protocol::utils::serde::Deserializable; use miden_protocol::vm::ExecutionProof; +use miden_standards::account::auth::{FeeConversionInfo, commit_fee_conversion_info}; use miden_standards::account::wallets::BasicWallet; use miden_standards::note::TxFeeNote; use miden_testing::{Auth, MockChainBuilder}; @@ -141,6 +143,16 @@ impl TestStore { self.genesis_commitment } + async fn fee_asset_id(&self) -> AssetId { + let view = self.state.view(); + let header = view.get_block_header(Some(0.into()), false).await.unwrap().0.unwrap(); + view.get_protocol_config(header.protocol_config_commitment()) + .await + .unwrap() + .unwrap() + .fee_asset_id() + } + fn data_directory_path(&self) -> &std::path::Path { &self.data_directory } @@ -246,8 +258,14 @@ fn build_test_proven_tx( account: &Account, patch: &AccountPatch, genesis: Word, + fee_asset_id: AssetId, ) -> ProvenTransaction { - build_test_proven_tx_with_fee(account, patch, genesis, true) + build_test_proven_tx_with_fee( + account, + patch, + genesis, + Some(FungibleAsset::new(fee_asset_id.faucet_id(), 1).unwrap()), + ) } /// Creates a minimal proven transaction, optionally including its canonical fee output note. @@ -255,7 +273,7 @@ fn build_test_proven_tx_with_fee( account: &Account, patch: &AccountPatch, genesis: Word, - include_fee: bool, + fee: Option, ) -> ProvenTransaction { let account_id = AccountId::dummy( [0; 15], @@ -273,8 +291,7 @@ fn build_test_proven_tx_with_fee( ) .unwrap(); - let output_notes = - include_fee.then(|| fee_output_note(account_id)).into_iter().collect::>(); + let output_notes = fee.map(|asset| fee_output_note(account_id, asset)); ProvenTransaction::new( account_update, @@ -288,11 +305,11 @@ fn build_test_proven_tx_with_fee( .unwrap() } -fn fee_output_note(sender: AccountId) -> OutputNote { +fn fee_output_note(sender: AccountId, asset: FungibleAsset) -> OutputNote { let fee_note = TxFeeNote::builder() .sender(sender) .serial_number(Word::from([1u32, 2, 3, 4])) - .asset(FungibleAsset::new(FungibleAsset::mock_issuer(), 1).unwrap()) + .asset(asset) .build() .unwrap() .into(); @@ -305,6 +322,7 @@ fn build_test_proven_tx_with_id( account_id: AccountId, account: &Account, genesis: Word, + fee_asset_id: AssetId, ) -> ProvenTransaction { let patch = AccountPatch::empty(account_id); let account_update = TxAccountUpdate::new( @@ -319,7 +337,10 @@ fn build_test_proven_tx_with_id( ProvenTransaction::new( account_update, Vec::::new(), - [fee_output_note(account_id)], + [fee_output_note( + account_id, + FungibleAsset::new(fee_asset_id.faucet_id(), 1).unwrap(), + )], 0.into(), genesis, u32::MAX.into(), @@ -346,19 +367,25 @@ fn replace_transaction_proof( struct ValidBatchFixture { request: proto::submission::TransactionBatch, + proposed_batch: ProposedBatch, genesis_block: ProvenBlock, protocol_config: ProtocolConfig, } -async fn build_valid_batch_fixture() -> ValidBatchFixture { - let mut mock_chain_builder = MockChainBuilder::new(); - let account = mock_chain_builder - .add_existing_wallet(Auth::BasicAuth { +async fn build_valid_batch_fixture(include_fee: bool) -> ValidBatchFixture { + let mut mock_chain_builder = MockChainBuilder::new() + .fee_faucet_id(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET.try_into().unwrap()) + .verification_base_fee(1); + let auth = if include_fee { + Auth::BasicAuth { auth_scheme: AuthScheme::Falcon512Poseidon2, - }) - .unwrap(); + } + } else { + Auth::IncrNonce + }; + let account = mock_chain_builder.add_existing_wallet(auth).unwrap(); let asset: Asset = - FungibleAsset::new(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET.try_into().unwrap(), 100) + FungibleAsset::new(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET.try_into().unwrap(), 1_000_000) .unwrap() .into(); let note = mock_chain_builder @@ -373,9 +400,16 @@ async fn build_valid_batch_fixture() -> ValidBatchFixture { let genesis_block = mock_chain.latest_block(); let protocol_config = mock_chain.protocol_config().clone(); + let (auth_args, advice) = commit_fee_conversion_info( + FeeConversionInfo::one_to_one(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET.try_into().unwrap()), + Word::from([9u32, 10, 11, 12]), + ); + let tx_context = mock_chain .build_transaction(account.id()) .authenticated_input_note(note.id()) + .auth_args(auth_args) + .add_advice_map_entry(auth_args, advice) .build() .unwrap(); let executed_tx = Box::pin(tx_context.execute()).await.unwrap(); @@ -386,6 +420,10 @@ async fn build_valid_batch_fixture() -> ValidBatchFixture { .unwrap() .unwrap(); + if !include_fee { + assert!(proven_tx.output_notes().is_empty()); + } + let proposed_batch = ProposedBatch::new( vec![Arc::new(proven_tx)], mock_chain.latest_block_header(), @@ -411,7 +449,12 @@ async fn build_valid_batch_fixture() -> ValidBatchFixture { sealed_transaction_inputs: vec![test_sealed_transaction_inputs()], }; - ValidBatchFixture { request, genesis_block, protocol_config } + ValidBatchFixture { + request, + proposed_batch, + genesis_block, + protocol_config, + } } fn assert_beyond_tip(status: &tonic::Status, endpoint: &str) { @@ -566,7 +609,7 @@ async fn rpc_server_rejects_proven_transactions_with_invalid_commitment() { // Build a valid proven transaction let (account, account_patch) = build_test_account([0; 32]); - let tx = build_test_proven_tx(&account, &account_patch, genesis); + let tx = build_test_proven_tx(&account, &account_patch, genesis, store.fee_asset_id().await); // Create an incorrect patch commitment from a different account let (other_account, _) = build_test_account([1; 32]); @@ -596,12 +639,25 @@ async fn rpc_server_rejects_proven_transactions_with_invalid_commitment() { ); } +#[rstest::rstest] +#[case::missing(1, None, &[4], "does not contain a canonical TX_FEE output note")] +#[case::foreign(1, Some(1), &[6], "must use only the native asset")] +#[case::zero_foreign(1, Some(0), &[6], "must use only the native asset")] +#[case::foreign_without_fees(0, Some(1), &[6], "must use only the native asset")] +#[case::missing_without_fees(0, None, &[], "Invalid proof for transaction")] #[tokio::test] -async fn rpc_server_rejects_proven_transactions_without_fees() { - let store = TestStore::start_with_base_fee(1).await; +async fn rpc_server_checks_transaction_fee_notes( + #[case] verification_base_fee: u32, + #[case] foreign_fee_amount: Option, + #[case] expected_details: &[u8], + #[case] expected_error: &str, +) { + let store = TestStore::start_with_base_fee(verification_base_fee).await; let genesis = store.genesis_commitment(); let (account, account_patch) = build_test_account([0; 32]); - let tx = build_test_proven_tx_with_fee(&account, &account_patch, genesis, false); + let fee = foreign_fee_amount + .map(|amount| FungibleAsset::new(FungibleAsset::mock_issuer(), amount).unwrap()); + let tx = build_test_proven_tx_with_fee(&account, &account_patch, genesis, fee); let request = proto::submission::ProvenTransactionSubmission { transaction: Some((&tx).into()), sealed_transaction_inputs: Some(test_sealed_transaction_inputs()), @@ -617,19 +673,31 @@ async fn rpc_server_rejects_proven_transactions_without_fees() { let status = service.submit_proven_tx(Request::new(request)).await.unwrap_err(); assert_eq!(status.code(), tonic::Code::InvalidArgument); - assert_eq!(status.details(), &[4]); + assert_eq!(status.details(), expected_details); assert!( - status.message().contains("does not contain a non-zero TX_FEE output note"), - "expected the missing-fee error, got: {status}" + status.message().contains(expected_error), + "expected {expected_error}, got: {status}" ); } +#[rstest::rstest] +#[case::missing(1, None, 4, "does not contain a canonical TX_FEE output note")] +#[case::foreign(1, Some(1), 6, "must use only the native asset")] +#[case::zero_foreign(1, Some(0), 6, "must use only the native asset")] +#[case::foreign_without_fees(0, Some(1), 6, "must use only the native asset")] #[tokio::test] -async fn sequencer_authenticated_rpc_rejects_transactions_without_fees() { - let store = TestStore::start_with_base_fee(1).await; +async fn sequencer_authenticated_rpc_rejects_transactions_without_native_fees( + #[case] verification_base_fee: u32, + #[case] foreign_fee_amount: Option, + #[case] expected_detail: u8, + #[case] expected_error: &str, +) { + let store = TestStore::start_with_base_fee(verification_base_fee).await; let genesis = store.genesis_commitment(); let (account, account_patch) = build_test_account([0; 32]); - let tx = build_test_proven_tx_with_fee(&account, &account_patch, genesis, false); + let fee = foreign_fee_amount + .map(|amount| FungibleAsset::new(FungibleAsset::mock_issuer(), amount).unwrap()); + let tx = build_test_proven_tx_with_fee(&account, &account_patch, genesis, fee); let inputs = TransactionInputs { account_id: tx.account_id(), account_commitment: Some(tx.account_update().initial_state_commitment()), @@ -656,36 +724,44 @@ async fn sequencer_authenticated_rpc_rejects_transactions_without_fees() { .unwrap_err(); assert_eq!(status.code(), tonic::Code::InvalidArgument); - assert_eq!(status.details(), &[4]); + assert_eq!(status.details(), &[expected_detail]); assert_eq!(block_producer.status().await.mempool_stats.uncommitted_transactions, 0); + assert!( + status.message().contains(expected_error), + "expected {expected_error}, got: {status}" + ); } #[tokio::test] -async fn rpc_server_does_not_require_fees_when_the_base_fee_is_zero() { - let store = TestStore::start().await; - let genesis = store.genesis_commitment(); +async fn sequencer_authenticated_rpc_accepts_transactions_without_notes_when_fees_are_zero() { + let store = TestStore::start_with_base_fee(0).await; let (account, account_patch) = build_test_account([0; 32]); - let tx = build_test_proven_tx_with_fee(&account, &account_patch, genesis, false); - let request = proto::submission::ProvenTransactionSubmission { - transaction: Some((&tx).into()), - sealed_transaction_inputs: Some(test_sealed_transaction_inputs()), + let tx = + build_test_proven_tx_with_fee(&account, &account_patch, store.genesis_commitment(), None); + let inputs = TransactionInputs { + account_id: tx.account_id(), + account_commitment: Some(tx.account_update().initial_state_commitment()), + nullifiers: HashMap::default(), + found_unauthenticated_notes: HashSet::default(), + current_block_height: 0.into(), }; - - let service = RpcService::new( + let tx = AuthenticatedTransaction::new_unchecked(tx.into(), inputs).unwrap(); + let block_producer = BlockProducerApi::new( Arc::clone(&store.state), - RpcBackend::full_node(source_rpc_client(), None), - None, - NonZeroUsize::new(1_000_000).unwrap(), - None, + store.state.committed_tip(), + BlockProducerApiConfig::default(), + CancellationToken::new(), ); + let service = SequencerInternalService { + state: Arc::clone(&store.state), + block_producer: block_producer.clone(), + account_admission: AccountAdmission::enabled(store.bootstrap_allowlist()), + }; - // The dummy proof is rejected later, demonstrating that the transaction passed the fee gate. - let status = service.submit_proven_tx(Request::new(request)).await.unwrap_err(); - assert_ne!(status.details(), &[4]); - assert!( - status.message().contains("Invalid proof for transaction"), - "expected proof validation after the fee gate, got: {status}" - ); + service + .submit_authenticated_tx(Request::new(proto::sequencer::AuthenticatedTransaction::from(tx))) + .await + .expect("zero-fee transactions do not require output notes"); } #[tokio::test] @@ -693,7 +769,8 @@ async fn rpc_server_rejects_invalid_deferred_transaction_proofs() { let store = TestStore::start().await; let genesis = store.genesis_commitment(); let (account, account_patch) = build_test_account([0; 32]); - let transaction = build_test_proven_tx(&account, &account_patch, genesis); + let transaction = + build_test_proven_tx(&account, &account_patch, genesis, store.fee_asset_id().await); let transaction = replace_transaction_proof( &transaction, miden_protocol::testing::dummy_deferred_execution_proof(), @@ -808,7 +885,7 @@ async fn rpc_server_rejects_proven_transactions_with_invalid_reference_block() { // Build a valid proven transaction but with the incorrect hash (empty). let invalid = Word::empty(); let (account, account_patch) = build_test_account([0; 32]); - let tx = build_test_proven_tx(&account, &account_patch, invalid); + let tx = build_test_proven_tx(&account, &account_patch, invalid, store.fee_asset_id().await); let request = proto::submission::ProvenTransactionSubmission { transaction: Some((&tx).into()), @@ -848,7 +925,12 @@ async fn rpc_rejects_post_deployment_network_account_tx() { // Build a non-deployment tx for that account. let (account, _) = build_test_account([0; 32]); - let tx = build_test_proven_tx_with_id(network_account_id, &account, genesis); + let tx = build_test_proven_tx_with_id( + network_account_id, + &account, + genesis, + store.fee_asset_id().await, + ); let request = proto::submission::ProvenTransactionSubmission { transaction: Some((&tx).into()), sealed_transaction_inputs: Some(test_sealed_transaction_inputs()), @@ -1444,9 +1526,12 @@ async fn full_node_preserves_original_accept_metadata_when_forwarding() { ); } +#[rstest::rstest] +#[case::with_fee_notes(true)] +#[case::without_fee_notes(false)] #[tokio::test(flavor = "multi_thread")] -async fn full_node_forwards_complete_transaction_batch_to_source_rpc() { - let fixture = build_valid_batch_fixture().await; +async fn full_node_forwards_complete_transaction_batch_to_source_rpc(#[case] include_fee: bool) { + let fixture = build_valid_batch_fixture(include_fee).await; let (validator, _validator_call_count, _last_accept, _validator_server) = start_validator(test_encryption_key(), None).await; let (source_rpc, _source_store, _source_server) = start_source_rpc_with_genesis( @@ -1483,6 +1568,41 @@ async fn full_node_forwards_complete_transaction_batch_to_source_rpc() { assert_eq!(response.block_num, 0); } +#[tokio::test(flavor = "multi_thread")] +async fn sequencer_authenticated_rpc_accepts_user_batch_without_fee_notes() { + let fixture = build_valid_batch_fixture(false).await; + let store = + TestStore::start_from_mock_genesis(&fixture.genesis_block, &fixture.protocol_config).await; + let guard = TestServerGuard(CancellationToken::new()); + let block_producer = BlockProducerApi::new( + Arc::clone(&store.state), + store.state.committed_tip(), + BlockProducerApiConfig::default(), + guard.0.clone(), + ); + let service = SequencerInternalService { + state: Arc::clone(&store.state), + block_producer, + account_admission: AccountAdmission::enabled(store.bootstrap_allowlist()), + }; + let mut auth_inputs = Vec::new(); + for tx in fixture.proposed_batch.transactions() { + auth_inputs.push(get_tx_inputs(&store.state, tx).await.unwrap().into()); + } + let request = proto::sequencer::AuthenticatedTransactionBatch { + proposed_batch: fixture.request.proposed_batch, + auth_inputs, + }; + + let response = service + .submit_authenticated_tx_batch(Request::new(request)) + .await + .expect("the sequencer should accept a user batch without fee output notes") + .into_inner(); + + assert_eq!(response.block_num, 0); +} + #[tokio::test] async fn authenticated_batch_defers_validation_to_async_handler() { let request = proto::sequencer::AuthenticatedTransactionBatch { @@ -1544,7 +1664,7 @@ async fn rpc_server_rejects_tx_submissions_without_genesis() { .connect_lazy::(); let (account, account_patch) = build_test_account([0; 32]); - let tx = build_test_proven_tx(&account, &account_patch, genesis); + let tx = build_test_proven_tx(&account, &account_patch, genesis, store.fee_asset_id().await); let request = proto::submission::ProvenTransactionSubmission { transaction: Some((&tx).into()), diff --git a/crates/rpc/src/tests/allowlist.rs b/crates/rpc/src/tests/allowlist.rs index 88a371f631..92e266da85 100644 --- a/crates/rpc/src/tests/allowlist.rs +++ b/crates/rpc/src/tests/allowlist.rs @@ -1,6 +1,5 @@ use std::collections::BTreeMap; -use miden_node_proto::generated::submission::SealedTransactionInputs; use miden_protocol::batch::{ProposedBatch, ProvenBatch}; use miden_standards::account::auth::NetworkAccount; use miden_standards::account::fees::{BasicConstantFeePolicy, FeePolicyManager}; @@ -9,18 +8,48 @@ use super::*; impl TestStore { async fn with_account_creation_batch() -> (Self, ProposedBatch) { - let mut builder = MockChainBuilder::new(); + let mut builder = MockChainBuilder::new() + .fee_faucet_id(FungibleAsset::mock_issuer()) + .verification_base_fee(1); let accounts = [ - builder.create_new_wallet(Auth::IncrNonce).unwrap(), - builder.create_new_wallet(Auth::IncrNonce).unwrap(), + builder + .create_new_wallet(Auth::BasicAuth { + auth_scheme: AuthScheme::Falcon512Poseidon2, + }) + .unwrap(), + builder + .create_new_wallet(Auth::BasicAuth { + auth_scheme: AuthScheme::Falcon512Poseidon2, + }) + .unwrap(), ]; + let notes = accounts.each_ref().map(|account| { + builder + .add_p2id_note( + account.id(), + account.id(), + &[FungibleAsset::mock(1_000_000)], + NoteType::Private, + ) + .unwrap() + }); let chain = builder.build().unwrap(); let store = Self::start_from_mock_genesis(&chain.latest_block(), chain.protocol_config()).await; let mut transactions = Vec::new(); // Batch decoding verifies each transaction proof before the admission check. - for account in accounts { - let context = chain.build_transaction(account).build().unwrap(); + for (account, note) in accounts.into_iter().zip(notes) { + let (auth_args, advice) = commit_fee_conversion_info( + FeeConversionInfo::one_to_one(FungibleAsset::mock_issuer()), + Word::from([9u32, 10, 11, 12]), + ); + let context = chain + .build_transaction(account) + .authenticated_input_note(note.id()) + .auth_args(auth_args) + .add_advice_map_entry(auth_args, advice) + .build() + .unwrap(); let executed = Box::pin(context.execute()).await.unwrap(); let inputs = executed.tx_inputs().clone(); let proven = spawn_blocking_in_current_span(move || { @@ -31,11 +60,12 @@ impl TestStore { .unwrap(); transactions.push(Arc::new(proven)); } - let batch = ProposedBatch::new_unverified( + let batch = ProposedBatch::new( transactions, chain.latest_block_header(), chain.latest_partial_blockchain(), BTreeMap::new(), + miden_protocol::MIN_PROOF_SECURITY_LEVEL, ) .unwrap(); (store, batch) @@ -187,7 +217,7 @@ async fn account_admission_only_restricts_new_non_network_accounts() { assert!(!allowlist.contains_account(creation.account_id()).await.unwrap()); } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn submission_endpoints_reject_unregistered_creation_without_partial_batch_admission() { let (store, batch) = TestStore::with_account_creation_batch().await; let transactions = batch.transactions(); @@ -236,29 +266,27 @@ async fn submission_endpoints_reject_unregistered_creation_without_partial_batch transaction: Some(transactions[1].as_ref().into()), ..Default::default() }; + let mut auth_inputs = Vec::new(); + for tx in transactions { + auth_inputs.push(get_tx_inputs(&store.state, tx).await.unwrap().into()); + } let authenticated_batch = proto::sequencer::AuthenticatedTransactionBatch { proposed_batch: Some((&batch).into()), - auth_inputs: transactions - .iter() - .map(|tx| proto::sequencer::AuthInputs { - account_id: Some(tx.account_id().into()), - ..Default::default() - }) - .collect(), + auth_inputs, }; for result in [ public .submit_proven_tx(Request::new(proto::submission::ProvenTransactionSubmission { transaction: Some(transactions[1].as_ref().into()), - sealed_transaction_inputs: Some(SealedTransactionInputs::default()), + sealed_transaction_inputs: Some(test_sealed_transaction_inputs()), })) .await, public .submit_proven_tx_batch(Request::new(proto::submission::TransactionBatch { batch: Some((&proven_batch).into()), proposed_batch: Some((&batch).into()), - sealed_transaction_inputs: vec![SealedTransactionInputs::default(); 2], + sealed_transaction_inputs: vec![test_sealed_transaction_inputs(); 2], })) .await, internal.submit_authenticated_tx(Request::new(tx)).await, diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index 8d789d1f98..e1a3dbd59a 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -21,7 +21,7 @@ doctest = false rocksdb = ["dep:miden-crypto", "miden-crypto/rocksdb"] # Enables test-only utilities. testing = ["miden-protocol/testing"] -testing-prover = ["dep:miden-processor", "dep:miden-testing", "dep:miden-tx", "testing"] +testing-prover = ["dep:miden-processor", "dep:miden-standards", "dep:miden-testing", "dep:miden-tx", "testing"] [dependencies] anyhow = { workspace = true } @@ -35,6 +35,7 @@ lru = { workspace = true } miden-node-tracing = { workspace = true } miden-processor = { optional = true, workspace = true } miden-protocol = { workspace = true } +miden-standards = { optional = true, workspace = true } miden-testing = { optional = true, workspace = true } miden-tx = { optional = true, workspace = true } reqwest = { workspace = true } diff --git a/crates/utils/src/testing.rs b/crates/utils/src/testing.rs index 58a87fd8bb..a4cc45da6c 100644 --- a/crates/utils/src/testing.rs +++ b/crates/utils/src/testing.rs @@ -1,8 +1,8 @@ //! Real transaction proofs for submission tests. use miden_processor::{ExecutionOptions, FastProcessor}; -use miden_protocol::MIN_PROOF_SECURITY_LEVEL; use miden_protocol::account::AccountUpdateDetails; +use miden_protocol::asset::FungibleAsset; use miden_protocol::block::{BlockSignatures, SignedBlock}; use miden_protocol::note::NoteType; use miden_protocol::transaction::{ @@ -13,6 +13,8 @@ use miden_protocol::transaction::{ TxAccountUpdate, }; use miden_protocol::vm::{ExecutionProof, PrecompileStatus}; +use miden_protocol::{MIN_PROOF_SECURITY_LEVEL, Word}; +use miden_standards::account::auth::{FeeConversionInfo, commit_fee_conversion_info}; use miden_testing::{Auth, MockChainBuilder}; use miden_tx::{ AccountProcedureIndexMap, @@ -35,17 +37,27 @@ pub async fn deferred_transaction_fixture() -> &'static DeferredTransactionFixtu static FIXTURE: OnceCell = OnceCell::const_new(); FIXTURE .get_or_init(|| async { - let mut builder = MockChainBuilder::new().verification_base_fee(0); + let mut builder = MockChainBuilder::new() + .fee_faucet_id(FungibleAsset::mock_issuer()) + .verification_base_fee(1); let account = builder.add_existing_wallet(Auth::basic_ecdsa()).unwrap(); - let note = builder.add_p2any_note(account.id(), NoteType::Private, []).unwrap(); + let note = builder + .add_p2any_note(account.id(), NoteType::Private, [FungibleAsset::mock(1_000_000)]) + .unwrap(); let chain = builder.build().unwrap(); let (header, body, ..) = chain.latest_block().into_parts(); let genesis = SignedBlock::new_unchecked(header, body, BlockSignatures::new(Vec::new()).unwrap()); + let (auth_args, advice) = commit_fee_conversion_info( + FeeConversionInfo::one_to_one(FungibleAsset::mock_issuer()), + Word::from([9u32, 10, 11, 12]), + ); let executed = Box::pin( chain .build_transaction(account.id()) .authenticated_input_note(note.id()) + .auth_args(auth_args) + .add_advice_map_entry(auth_args, advice) .build() .unwrap() .execute(), diff --git a/docs/external/src/rpc/errors-and-limits.md b/docs/external/src/rpc/errors-and-limits.md index 8f172f5f55..2bf8f3067e 100644 --- a/docs/external/src/rpc/errors-and-limits.md +++ b/docs/external/src/rpc/errors-and-limits.md @@ -37,13 +37,14 @@ If you are missing specific error information that could be useful, please open `SubmitProvenTx` and `SubmitProvenTxBatch` may return the following detail codes when a transaction or batch is rejected during submission validation or by the sequencer's mempool. -| Error | Value | gRPC status | Meaning | -| ------------------ | ----- | ------------------ | ------------------------------------ | -| `Internal` | `0` | `INTERNAL` | Internal submission failure | -| `Expired` | `1` | `INVALID_ARGUMENT` | Transaction expired | -| `StateConflict` | `2` | `INVALID_ARGUMENT` | State conflict | -| `CapacityExceeded` | `3` | `INVALID_ARGUMENT` | Mempool capacity exceeded | -| `MissingFee` | `4` | `INVALID_ARGUMENT` | Transaction has no non-zero fee note | +| Error | Value | gRPC status | Meaning | +| ------------------ | ----- | ------------------ | ----------------------------------------------- | +| `Internal` | `0` | `INTERNAL` | Internal submission failure | +| `Expired` | `1` | `INVALID_ARGUMENT` | Transaction expired | +| `StateConflict` | `2` | `INVALID_ARGUMENT` | State conflict | +| `CapacityExceeded` | `3` | `INVALID_ARGUMENT` | Mempool capacity exceeded | +| `MissingFee` | `4` | `INVALID_ARGUMENT` | Transaction has no canonical fee note | +| `InvalidFeeAsset` | `6` | `INVALID_ARGUMENT` | Fee note does not contain only the native asset | `Expired` means the transaction or batch has expired, or will expire too soon for the sequencer to consider accepting it. @@ -54,9 +55,13 @@ conflict, and use the detail byte when a client needs stable branching between b `CapacityExceeded` means the mempool capacity has been exhausted and is under load. -`MissingFee` is returned only by `SubmitProvenTx`. It means that the submitted transaction requires a fee but does not -contain an output note with the canonical `TX_FEE` script and a non-zero asset. Transactions that do not require a fee -remain valid without a fee note. This check does not establish that the fee amount is sufficient. +`MissingFee` means that a standalone transaction submitted through `SubmitProvenTx` does not contain an output note with +the `TX_FEE` script when the reference block's verification base fee is nonzero. A transaction can omit the fee note +when that base fee is zero. Each fee note must contain exactly one native asset, as specified by the reference block's +protocol configuration, even when fees are zero. `InvalidFeeAsset` means that a fee note does not meet this asset +requirement. The internal `SubmitAuthenticatedTx` endpoint applies the same checks. Transactions within user-submitted +batches are exempt because those batches handle their own fee collection. These checks do not establish that the fee +amount is sufficient. A fee note can contain a zero-valued native asset when the required fee is zero. ### Encrypted input errors