feat: compute account delta commitment before fee

CHANGELOG.md

@@ -62,6 +62,7 @@
 - Avoid modifying an asset vault when adding a fungible asset with amount zero and the asset does not already exist ([#1668](https://github.com/0xMiden/miden-base/pull/1668)).
 - [BREAKING] Update `NoteConsumptionChecker::check_notes_consumability` and `TransactionExecutor::try_execute_notes` to return `NoteConsumptionInfo` containing lists of `Note` rather than `NoteId` ([#1680](https://github.com/0xMiden/miden-base/pull/1680)).
 - Refactor epilogue to run as much code as possible before fees are computed ([#1698](https://github.com/0xMiden/miden-base/pull/1698)).
+- Refactor epilogue to run as much code as possible before fees are computed ([#1698](https://github.com/0xMiden/miden-base/pull/1698), [#1705](https://github.com/0xMiden/miden-base/pull/1705)).
 - [BREAKING] Remove note script utils and rename `note::add_note_assets_to_account` to `note::add_assets_to_account` ([#1694](https://github.com/0xMiden/miden-base/pull/1694)).
 - [BREAKING] Move `IncrNonceAuthComponent`, `ConditionalAuthComponent` and `AccountMockComponent` to `miden-lib` ([#1722](https://github.com/0xMiden/miden-base/pull/1722)).
 - Refactor `contracts::auth::basic` into a reusable library procedure `auth::rpo_falcon512` ([#1712](https://github.com/0xMiden/miden-base/pull/1712)).

bin/bench-tx/bench-tx.json

@@ -1,23 +1,23 @@
 {
   "simple": {
-    "prologue": 4167,
-    "notes_processing": 2540,
+    "prologue": 4178,
+    "notes_processing": 2624,
     "note_execution": {
-      "0x7fe47b84889353de93aea4bfeaa8da5da0036ea7a84dfc5ff5765ec51440896f": 1266,
-      "0xbf1c92f4f0b35b51d3f3e8f7ba19abf7718a7e087627df0d17987af8a1cb872a": 1233
+      "0x2a96867a0f22d54d1e3530c074c3ad9ba9542b5060edf2f8398202f5d2e3f557": 1308,
+      "0xc52584ba513fd5eaaec47234263a1b20ce4d848488430e577dbdd6d9254d5bae": 1275
     },
-    "tx_script_processing": 45,
-    "epilogue": 1525,
-    "after_compute_fee_cycles": 1143
+    "tx_script_processing": 39,
+    "epilogue": 2368,
+    "after_tx_cycles_obtained": 1205
   },
   "p2id": {
-    "prologue": 2755,
-    "notes_processing": 1572,
+    "prologue": 2779,
+    "notes_processing": 1653,
     "note_execution": {
-      "0x7ff44c53fefcd88ee714511b6d2976189549fec4b26695ee5e2b5293f50a845a": 1539
+      "0xb3044cb915c500d7678047e7f6cdfc9581efd8ebd07ff1977ef86f7072c51463": 1620
     },
-    "tx_script_processing": 45,
-    "epilogue": 62233,
-    "after_compute_fee_cycles": 891
+    "tx_script_processing": 39,
+    "epilogue": 63153,
+    "after_tx_cycles_obtained": 1187
   }
 }

crates/miden-lib/asm/kernels/transaction/lib/epilogue.masm

@@ -322,6 +322,8 @@ proc.compute_and_remove_fee
     # => [FEE_ASSET]
 
     # remove the fee from the native account's vault
+    # note that this modifies the vault delta of the account, but for all purposes, this is ignored
+    # because the account delta commitment is computed *before* this procedure.
     exec.account::remove_asset_from_vault
 end
 
@@ -354,6 +356,7 @@ end
 #! Locals:
 #! - 0..4: OUTPUT_NOTES_COMMITMENT
 #! - 4..8: FEE_ASSET
+#! - 8..12: ACCOUNT_DELTA_COMMITMENT
 #!
 #! Panics if:
 #! - the assets that were in input notes or in the account vault at the beginning of
@@ -364,7 +367,7 @@ end
 #!   changing the account's state. The latter is validated as part of computing the account delta
 #!   commitment.
 #! - the account vault does not contain the computed fee.
-export.finalize_transaction.8
+export.finalize_transaction.12
     # make sure that the context was switched back to the native account
     exec.memory::assert_native_account
 
@@ -404,6 +407,35 @@ export.finalize_transaction.8
     loc_storew.0 dropw
     # => []
 
+    # ------ Compute account delta commitment ------
+
+    exec.account_delta::compute_commitment
+    # => [ACCOUNT_DELTA_COMMITMENT]
+
+    # store commitment in local
+    loc_storew.8
+    # => [ACCOUNT_DELTA_COMMITMENT]
+
+    # ------ Assert that account was changed or notes were consumed ------
+
+    # check if the account delta commitment is the EMPTY_WORD, which is the case when the delta
+    # is empty, i.e. the account did not change in this transaction
+    exec.word::eqz
+    # => [is_delta_commitment_empty]
+
+    # assert that if the delta commitment is empty, the transaction had input notes,
+    # otherwise this transaction is considered empty, which is not allowed
+    exec.memory::get_input_notes_commitment
+    # => [INPUT_NOTES_COMMITMENT, is_delta_commitment_empty]
+
+    # if the input notes commitment is an empty word there were no input notes in this transaction
+    exec.word::eqz and
+    # => [is_delta_commitment_and_input_notes_commitment_empty]
+
+    # assert that either the account was changed or notes were consumed
+    assertz.err=ERR_EPILOGUE_EXECUTED_TRANSACTION_IS_EMPTY
+    # => []
+
     # ------ Compute fees ------
 
     exec.compute_and_remove_fee
@@ -435,33 +467,12 @@ export.finalize_transaction.8
     movup.4 drop movup.4 drop
     # => [FINAL_ACCOUNT_COMMITMENT]
 
-    # ------ Compute account delta commitment ------
-
-    exec.account_delta::compute_commitment
-    # => [ACCOUNT_DELTA_COMMITMENT, FINAL_ACCOUNT_COMMITMENT]
-
-    # ------ Assert that account was changed or notes were consumed ------
-
-    # check if the account delta commitment is the EMPTY_WORD, which is the case when the delta
-    # is empty, i.e. the account did not change in this transaction
-    exec.word::testz
-    # => [is_delta_commitment_empty, ACCOUNT_DELTA_COMMITMENT, FINAL_ACCOUNT_COMMITMENT]
-
-    # assert that if the delta commitment is empty, the transaction had input notes,
-    # otherwise this transaction is considered empty, which is not allowed
-    exec.memory::get_input_notes_commitment
-    # => [INPUT_NOTES_COMMITMENT, is_delta_commitment_empty, ACCOUNT_DELTA_COMMITMENT, FINAL_ACCOUNT_COMMITMENT]
-
-    # if the input notes commitment is an empty word there were no input notes in this transaction
-    exec.word::eqz and
-    # => [is_delta_commitment_and_input_notes_commitment_empty, ACCOUNT_DELTA_COMMITMENT, FINAL_ACCOUNT_COMMITMENT]
+    # ------ Compute and insert account update commitment ------
 
-    # assert that either the account was changed or notes were consumed
-    assertz.err=ERR_EPILOGUE_EXECUTED_TRANSACTION_IS_EMPTY
+    # load account delta commitment from local
+    padw loc_loadw.8
     # => [ACCOUNT_DELTA_COMMITMENT, FINAL_ACCOUNT_COMMITMENT]
 
-    # ------ Compute and insert account update commitment ------
-
     # insert into advice map ACCOUNT_UPDATE_COMMITMENT: (FINAL_ACCOUNT_COMMITMENT, ACCOUNT_DELTA_COMMITMENT),
     # where ACCOUNT_UPDATE_COMMITMENT = hash(FINAL_ACCOUNT_COMMITMENT || ACCOUNT_DELTA_COMMITMENT)
     adv.insert_hdword

crates/miden-lib/src/transaction/mod.rs

@@ -321,7 +321,7 @@ impl TransactionKernel {
 
     /// Returns [TransactionOutputs] constructed from the provided output stack and advice map.
     ///
-    /// The output stack is expected to be arrange as follows:
+    /// The output stack is expected to be arranged as follows:
     ///
     /// ```text
     /// [

crates/miden-objects/src/account/delta/mod.rs

@@ -65,6 +65,9 @@ impl AccountDelta {
         Ok(Self { account_id, storage, vault, nonce_delta })
     }
 
+    // PUBLIC MUTATORS
+    // --------------------------------------------------------------------------------------------
+
     /// Merge another [AccountDelta] into this one.
     pub fn merge(&mut self, other: Self) -> Result<(), AccountDeltaError> {
         let new_nonce_delta = self.nonce_delta + other.nonce_delta;
@@ -83,6 +86,11 @@ impl AccountDelta {
         self.vault.merge(other.vault)
     }
 
+    /// Returns a mutable reference to the account vault delta.
+    pub fn vault_mut(&mut self) -> &mut AccountVaultDelta {
+        &mut self.vault
+    }
+
     // PUBLIC ACCESSORS
     // --------------------------------------------------------------------------------------------
 

crates/miden-objects/src/transaction/proven_tx.rs

@@ -445,6 +445,14 @@ pub struct TxAccountUpdate {
     final_state_commitment: Word,
 
     /// The commitment to the account delta resulting from the execution of the transaction.
+    ///
+    /// This must be the commitment to the account delta as computed by the transaction kernel in
+    /// the epilogue (the "pre-fee" delta). Notably, this _excludes_ the automatically removed fee
+    /// asset. The account delta possibly contained in [`AccountUpdateDetails`] _includes_ the
+    /// _removed_ fee asset, so that it represents the full account delta of the transaction
+    /// (the "post-fee" delta). This mismatch means that in order to validate the delta, the
+    /// fee asset must be _added_ to the delta before checking its commitment against this
+    /// field.
     account_delta_commitment: Word,
 
     /// A set of changes which can be applied the account's state prior to the transaction to

crates/miden-testing/tests/scripts/fee.rs

@@ -0,0 +1,50 @@
+use anyhow::Context;
+use miden_objects::asset::FungibleAsset;
+use miden_objects::{self, Felt, Word};
+use miden_testing::{Auth, MockChain};
+
+use crate::prove_and_verify_transaction;
+
+// FEE TESTS
+// ================================================================================================
+
+/// Tests that a simple account can be created with non-zero fees and the transaction can be proven
+/// and verified.
+///
+/// This is an interesting test case because the prover needs to apply the fee asset to the account
+/// delta in order to prove the correct delta commitment. Once we have other tests with fees, this
+/// test may become obsolete.
+#[test]
+fn prove_account_creation_with_fees() -> anyhow::Result<()> {
+    let amount = 10_000;
+    let mut builder = MockChain::builder().verification_base_fee(50);
+    let account = builder.create_new_wallet(Auth::IncrNonce)?;
+    let fee_note = builder.add_p2id_note_with_fee(account.id(), amount)?;
+    let chain = builder.build()?;
+
+    let tx = chain
+        .build_tx_context(account, &[fee_note.id()], &[])?
+        .build()?
+        .execute_blocking()
+        .context("failed to execute account-creating transaction")?;
+
+    let expected_fee = tx.compute_fee();
+    assert_eq!(expected_fee, tx.fee().amount());
+
+    // We expect that the new account contains the amount minus the paid fee.
+    let added_asset = FungibleAsset::new(chain.native_asset_id(), amount)?.sub(tx.fee())?;
+
+    assert_eq!(tx.account_delta().nonce_delta(), Felt::new(1));
+    // except for the nonce, the storage delta should be empty
+    assert!(tx.account_delta().storage().is_empty());
+    assert_eq!(tx.account_delta().vault().added_assets().count(), 1);
+    assert_eq!(tx.account_delta().vault().removed_assets().count(), 0);
+    assert_eq!(tx.account_delta().vault().added_assets().next().unwrap(), added_asset.into());
+    assert_eq!(tx.final_account().nonce(), Felt::new(1));
+    // account commitment should not be the empty word
+    assert_ne!(tx.account_delta().to_commitment(), Word::empty());
+
+    prove_and_verify_transaction(tx)?;
+
+    Ok(())
+}

crates/miden-testing/tests/scripts/mod.rs

@@ -1,4 +1,5 @@
 mod faucet;
+mod fee;
 mod p2id;
 mod p2ide;
 mod send_note;

crates/miden-tx/src/errors/mod.rs

@@ -10,6 +10,7 @@ use miden_objects::crypto::merkle::SmtProofError;
 use miden_objects::note::NoteId;
 use miden_objects::transaction::TransactionSummary;
 use miden_objects::{
+    AccountDeltaError,
     AccountError,
     Felt,
     ProvenTransactionError,
@@ -51,6 +52,8 @@ pub enum TransactionExecutorError {
         in_kernel_commitment: Word,
         host_commitment: Word,
     },
+    #[error("failed to compute pre-fee delta")]
+    ComputePreFeeDelta(#[source] AccountDeltaError),
     #[error("input account ID {input_id} does not match output account ID {output_id}")]
     InconsistentAccountId {
         input_id: AccountId,
@@ -85,6 +88,8 @@ pub enum TransactionExecutorError {
 pub enum TransactionProverError {
     #[error("failed to apply account delta")]
     AccountDeltaApplyFailed(#[source] AccountError),
+    #[error("failed to compute pre-fee delta")]
+    ComputePreFeeDelta(#[source] AccountDeltaError),
     #[error("failed to construct transaction outputs")]
     TransactionOutputConstructionFailed(#[source] TransactionOutputError),
     #[error("failed to build proven transaction")]

crates/miden-tx/src/executor/mod.rs

@@ -25,7 +25,11 @@ pub use vm_processor::{ExecutionOptions, MastForestStore};
 
 use super::TransactionExecutorError;
 use crate::auth::TransactionAuthenticator;
-use crate::host::{AccountProcedureIndexMap, ScriptMastForestStore};
+use crate::host::{
+    AccountProcedureIndexMap,
+    ScriptMastForestStore,
+    compute_pre_fee_delta_commitment,
+};
 
 mod exec_host;
 pub use exec_host::TransactionExecutorHost;
@@ -471,23 +475,30 @@ fn build_executed_transaction<STORE: DataStore + Sync, AUTH: TransactionAuthenti
     stack_outputs: StackOutputs,
     host: TransactionExecutorHost<STORE, AUTH>,
 ) -> Result<ExecutedTransaction, TransactionExecutorError> {
-    let (account_delta, output_notes, generated_signatures, tx_progress) = host.into_parts();
+    // Note that the account delta already contains the removed transaction fee, so it is the
+    // full delta of the transaction.
+    let (mut post_fee_account_delta, output_notes, generated_signatures, tx_progress) =
+        host.into_parts();
 
     let tx_outputs =
         TransactionKernel::from_transaction_parts(&stack_outputs, &advice_inputs, output_notes)
             .map_err(TransactionExecutorError::TransactionOutputConstructionFailed)?;
 
-    let initial_account = tx_inputs.account();
-    let final_account = &tx_outputs.account;
+    // Compute the delta commitment of the delta before the fee was removed.
+    let pre_fee_delta_commitment =
+        compute_pre_fee_delta_commitment(&mut post_fee_account_delta, tx_outputs.fee)
+            .map_err(TransactionExecutorError::ComputePreFeeDelta)?;
 
-    let host_delta_commitment = account_delta.to_commitment();
-    if tx_outputs.account_delta_commitment != host_delta_commitment {
+    if tx_outputs.account_delta_commitment != pre_fee_delta_commitment {
         return Err(TransactionExecutorError::InconsistentAccountDeltaCommitment {
             in_kernel_commitment: tx_outputs.account_delta_commitment,
-            host_commitment: host_delta_commitment,
+            host_commitment: pre_fee_delta_commitment,
         });
     }
 
+    let initial_account = tx_inputs.account();
+    let final_account = &tx_outputs.account;
+
     if initial_account.id() != final_account.id() {
         return Err(TransactionExecutorError::InconsistentAccountId {
             input_id: initial_account.id(),
@@ -497,10 +508,10 @@ fn build_executed_transaction<STORE: DataStore + Sync, AUTH: TransactionAuthenti
 
     // make sure nonce delta was computed correctly
     let nonce_delta = final_account.nonce() - initial_account.nonce();
-    if nonce_delta != account_delta.nonce_delta() {
+    if nonce_delta != post_fee_account_delta.nonce_delta() {
         return Err(TransactionExecutorError::InconsistentAccountNonceDelta {
             expected: nonce_delta,
-            actual: account_delta.nonce_delta(),
+            actual: post_fee_account_delta.nonce_delta(),
         });
     }
 
@@ -510,7 +521,7 @@ fn build_executed_transaction<STORE: DataStore + Sync, AUTH: TransactionAuthenti
     Ok(ExecutedTransaction::new(
         tx_inputs,
         tx_outputs,
-        account_delta,
+        post_fee_account_delta,
         tx_args,
         advice_inputs,
         tx_progress.into(),

crates/miden-tx/src/host/account_delta_tracker.rs

@@ -1,5 +1,6 @@
 use miden_objects::account::{AccountDelta, AccountId, AccountStorageHeader, AccountVaultDelta};
-use miden_objects::{Felt, FieldElement, ZERO};
+use miden_objects::asset::{Asset, FungibleAsset};
+use miden_objects::{AccountDeltaError, Felt, FieldElement, Word, ZERO};
 
 use crate::host::storage_delta_tracker::StorageDeltaTracker;
 
@@ -74,3 +75,36 @@ impl AccountDeltaTracker {
             .expect("account delta created in delta tracker should be valid")
     }
 }
+
+/// Returns the pre-fee commitment.
+///
+/// This is the account delta commitment before the fee was removed from the account vault in
+/// the epilogue. This is the commitment of the delta that the transaction outputs on the stack
+/// as part of ACCOUNT_UPDATE_COMMITMENT.
+///
+/// This takes &mut self because it temporarily adds the provided asset to the vault and then
+/// removes it again, so the function is guaranteed to leave the delta effectively untouched.
+///
+/// # Errors
+///
+/// Returns `None` if:
+/// - adding or removing the provided fee to the vault fails.
+pub(crate) fn compute_pre_fee_delta_commitment(
+    account_delta: &mut AccountDelta,
+    fee: FungibleAsset,
+) -> Result<Word, AccountDeltaError> {
+    let fee = Asset::from(fee);
+
+    // Because the fee asset is removed from the vault after the commitment is computed in the
+    // tx kernel, we have to *add* it to the post-fee delta (self) before computing the pre-fee
+    // commitment.
+    account_delta.vault_mut().add_asset(fee)?;
+
+    let pre_fee_commitment = account_delta.to_commitment();
+
+    // Now that we have computed the commitment of the pre-feedelta, we revert the above changes
+    // to get back the actual account delta of the transaction.
+    account_delta.vault_mut().remove_asset(fee)?;
+
+    Ok(pre_fee_commitment)
+}

crates/miden-tx/src/host/mod.rs

@@ -1,6 +1,7 @@
 mod account_delta_tracker;
 
 use account_delta_tracker::AccountDeltaTracker;
+pub(crate) use account_delta_tracker::compute_pre_fee_delta_commitment;
 
 mod storage_delta_tracker;