diff --git a/CHANGELOG.md b/CHANGELOG.md index 3213ebec94..dd9469a392 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Added getter for proof security level in `ProvenBatch` and `ProvenBlock` (#1259). - [BREAKING] Replaced the `ProvenBatch::new_unchecked` with the `ProvenBatch::new` method to initialize the struct with validations (#1260). - Added a retry strategy for worker's health check (#1255). +- Add `AccountTree` and `PartialAccountTree` wrappers and enforce ID prefix uniqueness (#1254). ## 0.8.1 (2025-03-26) - `miden-objects` and `miden-tx` crates only. diff --git a/crates/miden-block-prover/src/errors.rs b/crates/miden-block-prover/src/errors.rs index bb2d469ba8..f7465398f5 100644 --- a/crates/miden-block-prover/src/errors.rs +++ b/crates/miden-block-prover/src/errors.rs @@ -1,5 +1,4 @@ -use miden_crypto::merkle::MerkleError; -use miden_objects::{Digest, NullifierTreeError, account::AccountId}; +use miden_objects::{AccountTreeError, Digest, NullifierTreeError}; use thiserror::Error; #[derive(Debug, Error)] @@ -7,13 +6,11 @@ pub enum ProvenBlockError { #[error("nullifier witness has a different root than the current nullifier tree root")] NullifierWitnessRootMismatch(#[source] NullifierTreeError), - #[error( - "account witness for account {account_id} has a different root than the current account tree root" - )] - AccountWitnessRootMismatch { - account_id: AccountId, - source: MerkleError, - }, + #[error("failed to track account witness")] + AccountWitnessTracking { source: AccountTreeError }, + + #[error("account ID prefix already exists in the tree")] + AccountIdPrefixDuplicate { source: AccountTreeError }, #[error( "account tree root of the previous block header is {prev_block_account_root} but the root of the partial tree computed from account witnesses is {stale_account_root}, indicating that the witnesses are stale" diff --git a/crates/miden-block-prover/src/local_block_prover.rs b/crates/miden-block-prover/src/local_block_prover.rs index 0685c2b0b7..b11c5a2072 100644 --- a/crates/miden-block-prover/src/local_block_prover.rs +++ b/crates/miden-block-prover/src/local_block_prover.rs @@ -1,14 +1,13 @@ use std::{collections::BTreeMap, vec::Vec}; -use miden_crypto::merkle::{LeafIndex, PartialMerkleTree}; use miden_lib::transaction::TransactionKernel; use miden_objects::{ - Digest, Word, + Digest, account::AccountId, block::{ AccountUpdateWitness, BlockAccountUpdate, BlockHeader, BlockNoteIndex, BlockNoteTree, - BlockNumber, NullifierWitness, OutputNoteBatch, PartialNullifierTree, ProposedBlock, - ProvenBlock, + BlockNumber, NullifierWitness, OutputNoteBatch, PartialAccountTree, PartialNullifierTree, + ProposedBlock, ProvenBlock, }, note::Nullifier, transaction::ChainMmr, @@ -78,18 +77,13 @@ impl LocalBlockProver { let block_num = proposed_block.block_num(); let timestamp = proposed_block.timestamp(); - let tx_commitment = BlockHeader::compute_tx_commitment( - proposed_block - .transactions() - .map(|tx_header| (tx_header.id(), tx_header.account_id())), - ); // Split the proposed block into its parts. // -------------------------------------------------------------------------------------------- let ( batches, - mut account_updated_witnesses, + account_updated_witnesses, output_note_batches, created_nullifiers, chain_mmr, @@ -115,7 +109,7 @@ impl LocalBlockProver { // -------------------------------------------------------------------------------------------- let new_account_root = - compute_account_root(&mut account_updated_witnesses, &prev_block_header)?; + compute_account_root(&account_updated_witnesses, &prev_block_header)?; // Insert the previous block header into the block chain MMR to get the new chain // commitment. @@ -144,6 +138,7 @@ impl LocalBlockProver { // -------------------------------------------------------------------------------------------- let txs = batches.into_transactions(); + let tx_commitment = txs.commitment(); // Construct the new block header. // -------------------------------------------------------------------------------------------- @@ -208,7 +203,7 @@ fn compute_nullifiers( // its corresponding nullifier witness, so we don't have to check again whether they match. for witness in created_nullifiers.into_values() { partial_nullifier_tree - .add_nullifier_witness(witness) + .track_nullifier(witness) .map_err(ProvenBlockError::NullifierWitnessRootMismatch)?; } @@ -248,7 +243,7 @@ fn compute_chain_commitment(mut chain_mmr: ChainMmr, prev_block_header: BlockHea /// It uses a PartialMerkleTree for now while we use a SimpleSmt for the account tree. Once that is /// updated to an Smt, we can use a PartialSmt instead. fn compute_account_root( - updated_accounts: &mut Vec<(AccountId, AccountUpdateWitness)>, + updated_accounts: &[(AccountId, AccountUpdateWitness)], prev_block_header: &BlockHeader, ) -> Result { // If no accounts were updated, the account tree root is unchanged. @@ -256,26 +251,13 @@ fn compute_account_root( return Ok(prev_block_header.account_root()); } - let mut partial_account_tree = PartialMerkleTree::new(); - // First reconstruct the current account tree from the provided merkle paths. - for (account_id, witness) in updated_accounts.iter_mut() { - let account_leaf_index = LeafIndex::from(*account_id); - // Shouldn't the value in PartialMerkleTree::add_path be a Word instead of a Digest? - // PartialMerkleTree::update_leaf (below) takes a Word as a value, so this seems - // inconsistent. - partial_account_tree - .add_path( - account_leaf_index.value(), - witness.initial_state_commitment(), - // We don't need the merkle path later, so we can take it out. - core::mem::take(witness.initial_state_proof_mut()), - ) - .map_err(|source| ProvenBlockError::AccountWitnessRootMismatch { - account_id: *account_id, - source, - })?; - } + // If a witness points to a leaf where multiple account IDs share the same prefix, this will + // return an error. + let mut partial_account_tree = PartialAccountTree::with_witnesses( + updated_accounts.iter().map(|(_, update_witness)| update_witness.to_witness()), + ) + .map_err(|source| ProvenBlockError::AccountWitnessTracking { source })?; // Check the account tree root in the previous block header matches the reconstructed tree's // root. @@ -288,13 +270,14 @@ fn compute_account_root( // Second, update the account tree by inserting the new final account state commitments to // compute the new root of the account tree. - // TODO: Move this loop into a method on `PartialAccountTree` once it is added. - for (account_id, witness) in updated_accounts { - let account_leaf_index = LeafIndex::from(*account_id); - partial_account_tree - .update_leaf(account_leaf_index.value(), Word::from(witness.final_state_commitment())) - .expect("every account leaf should have been inserted in the first loop"); - } + // If an account ID's prefix already exists in the tree, this will return an error. + // Note that we have inserted all witnesses that we want to update into the partial account + // tree, so we should not run into the untracked key error. + partial_account_tree + .upsert_state_commitments(updated_accounts.iter().map(|(account_id, update_witness)| { + (*account_id, update_witness.final_state_commitment()) + })) + .map_err(|source| ProvenBlockError::AccountIdPrefixDuplicate { source })?; Ok(partial_account_tree.root()) } diff --git a/crates/miden-block-prover/src/tests/proven_block_error.rs b/crates/miden-block-prover/src/tests/proven_block_error.rs index 9b6a68a3c5..10d0061b04 100644 --- a/crates/miden-block-prover/src/tests/proven_block_error.rs +++ b/crates/miden-block-prover/src/tests/proven_block_error.rs @@ -1,17 +1,27 @@ use anyhow::Context; use assert_matches::assert_matches; -use miden_crypto::merkle::MerkleError; +use miden_crypto::{EMPTY_WORD, Felt, FieldElement}; +use miden_lib::transaction::TransactionKernel; use miden_objects::{ - NullifierTreeError, + AccountTreeError, Digest, NullifierTreeError, + account::{ + Account, AccountBuilder, AccountId, AccountIdAnchor, StorageSlot, + delta::AccountUpdateDetails, + }, batch::ProvenBatch, - block::{BlockInputs, ProposedBlock}, + block::{BlockInputs, BlockNumber, ProposedBlock}, + testing::account_component::AccountMockComponent, + transaction::{ProvenTransaction, ProvenTransactionBuilder}, + vm::ExecutionProof, }; +use miden_tx::testing::{MockChain, TransactionContextBuilder}; +use winterfell::Proof; use crate::{ LocalBlockProver, ProvenBlockError, tests::utils::{ - TestSetup, generate_batch, generate_executed_tx_with_authenticated_notes, - generate_tracked_note, setup_chain, + ProvenTransactionExt, TestSetup, generate_batch, + generate_executed_tx_with_authenticated_notes, generate_tracked_note, setup_chain, }, }; @@ -186,8 +196,8 @@ fn proven_block_fails_on_account_tree_root_mismatch() -> anyhow::Result<()> { assert_matches!( error, - ProvenBlockError::AccountWitnessRootMismatch { - source: MerkleError::ConflictingRoots { .. }, + ProvenBlockError::AccountWitnessTracking { + source: AccountTreeError::TreeRootConflict { .. }, .. } ); @@ -240,3 +250,198 @@ fn proven_block_fails_on_nullifier_tree_root_mismatch() -> anyhow::Result<()> { Ok(()) } + +/// Tests that creating an account when an existing account with the same account ID prefix exists, +/// results in an error. +#[test] +fn proven_block_fails_on_creating_account_with_existing_account_id_prefix() -> anyhow::Result<()> { + // Construct a new account. + // -------------------------------------------------------------------------------------------- + + let mut mock_chain = MockChain::new(); + let anchor_block = mock_chain.block_header(0); + let (account, seed) = AccountBuilder::new([5; 32]) + .anchor(AccountIdAnchor::try_from(&anchor_block)?) + .with_component( + AccountMockComponent::new_with_slots( + TransactionKernel::testing_assembler(), + vec![StorageSlot::Value([5u32.into(); 4])], + ) + .unwrap(), + ) + .build() + .context("failed to build account")?; + + let new_id = account.id(); + + // Construct a second account whose ID matches the prefix of the first and insert it into the + // chain, as if that account already existed. That way we can check if the block prover errors + // when we attempt to create the first account. + // -------------------------------------------------------------------------------------------- + + // Set some bits on the hash part of the suffix to make the account id distinct from the + // original one, but their prefix is still the same. + let existing_id = AccountId::try_from(u128::from(new_id) | 0xffff00) + .context("failed to convert account ID")?; + + assert_eq!( + new_id.prefix(), + existing_id.prefix(), + "test requires that prefixes are the same" + ); + assert_ne!( + new_id.suffix(), + existing_id.suffix(), + "test should work if suffixes are different, so we want to ensure it" + ); + assert_eq!(account.init_commitment(), miden_objects::Digest::from(EMPTY_WORD)); + + let existing_account = + Account::mock(existing_id.into(), Felt::ZERO, TransactionKernel::testing_assembler()); + mock_chain.add_pending_account(existing_account.clone()); + mock_chain.seal_next_block(); + + // Execute the account-creating transaction. + // -------------------------------------------------------------------------------------------- + + let tx_inputs = mock_chain.get_transaction_inputs(account.clone(), Some(seed), &[], &[]); + let tx_context = TransactionContextBuilder::new(account) + .account_seed(Some(seed)) + .tx_inputs(tx_inputs) + .build(); + let tx = tx_context.execute().context("failed to execute account creating tx")?; + let tx = + ProvenTransaction::from_executed_transaction_mocked(tx, &mock_chain.latest_block_header()); + + let batch = generate_batch(&mut mock_chain, vec![tx]); + let batches = [batch]; + + let block_inputs = mock_chain.get_block_inputs(batches.iter()); + // Sanity check: The mock chain account tree root should match the previous block header's + // account tree root. + assert_eq!(mock_chain.accounts().root(), block_inputs.prev_block_header().account_root()); + assert_eq!(mock_chain.accounts().num_accounts(), 1); + + // Sanity check: The block inputs should contain an account witness whose ID matches the + // existing ID. + assert_eq!(block_inputs.account_witnesses().len(), 1); + let witness = block_inputs + .account_witnesses() + .get(&new_id) + .context("block inputs did not contain witness for id")?; + + // The witness should be for the **existing** account, because that's the one that exists in + // the tree and is therefore in the same SMT leaf that we would insert the new ID into. + assert_eq!(witness.id(), existing_id); + assert_eq!(witness.state_commitment(), existing_account.commitment()); + + let block = mock_chain.propose_block(batches).context("failed to propose block")?; + + let err = LocalBlockProver::new(0).prove_without_batch_verification(block).unwrap_err(); + + // This should fail when we try to _insert_ the same two prefixes into the partial tree. + assert_matches!( + err, + ProvenBlockError::AccountIdPrefixDuplicate { + source: AccountTreeError::DuplicateIdPrefix { duplicate_prefix } + } if duplicate_prefix == new_id.prefix() + ); + + Ok(()) +} + +/// Tests that creating two accounts in the same block whose ID prefixes match, results in an error. +#[test] +fn proven_block_fails_on_creating_account_with_duplicate_account_id_prefix() -> anyhow::Result<()> { + // Construct a new account. + // -------------------------------------------------------------------------------------------- + + let mut mock_chain = MockChain::new(); + let anchor_block = mock_chain.block_header(0); + let (account, _) = AccountBuilder::new([5; 32]) + .anchor(AccountIdAnchor::try_from(&anchor_block)?) + .with_component( + AccountMockComponent::new_with_slots( + TransactionKernel::testing_assembler(), + vec![StorageSlot::Value([5u32.into(); 4])], + ) + .unwrap(), + ) + .build() + .context("failed to build account")?; + + let id0 = account.id(); + + // Construct a second account whose ID matches the prefix of the first. + // -------------------------------------------------------------------------------------------- + + // Set some bits on the hash part of the suffix to make the account id distinct from the + // original one, but their prefix is still the same. + let id1 = + AccountId::try_from(u128::from(id0) | 0xffff00).context("failed to convert account ID")?; + + assert_eq!(id0.prefix(), id1.prefix(), "test requires that prefixes are the same"); + assert_ne!( + id0.suffix(), + id1.suffix(), + "test should work if suffixes are different, so we want to ensure it" + ); + + // Build two mocked proven transactions, each of which creates a new account and both share the + // same ID prefix but not the suffix. + // -------------------------------------------------------------------------------------------- + + let [tx0, tx1] = + [(id0, [0, 0, 0, 1u32]), (id1, [0, 0, 0, 2u32])].map(|(id, final_state_comm)| { + ProvenTransactionBuilder::new( + id, + Digest::default(), + Digest::from(final_state_comm), + anchor_block.block_num(), + anchor_block.commitment(), + BlockNumber::from(u32::MAX), + ExecutionProof::new(Proof::new_dummy(), Default::default()), + ) + .account_update_details(AccountUpdateDetails::Private) + .build() + .unwrap() + }); + + // Build a batch from these transactions and attempt to prove a block. + // -------------------------------------------------------------------------------------------- + + let batch = generate_batch(&mut mock_chain, vec![tx0, tx1]); + let batches = [batch]; + + // Sanity check: The block inputs should contain two account witnesses that point to the same + // empty entry. + let block_inputs = mock_chain.get_block_inputs(batches.iter()); + assert_eq!(block_inputs.account_witnesses().len(), 2); + let witness0 = block_inputs + .account_witnesses() + .get(&id0) + .context("block inputs did not contain witness for id0")?; + let witness1 = block_inputs + .account_witnesses() + .get(&id1) + .context("block inputs did not contain witness for id1")?; + assert_eq!(witness0.id(), id0); + assert_eq!(witness1.id(), id1); + + assert_eq!(witness0.state_commitment(), Digest::default()); + assert_eq!(witness1.state_commitment(), Digest::default()); + + let block = mock_chain.propose_block(batches).context("failed to propose block")?; + + let err = LocalBlockProver::new(0).prove_without_batch_verification(block).unwrap_err(); + + // This should fail when we try to _track_ the same two prefixes in the partial tree. + assert_matches!( + err, + ProvenBlockError::AccountWitnessTracking { + source: AccountTreeError::DuplicateIdPrefix { duplicate_prefix } + } if duplicate_prefix == id0.prefix() + ); + + Ok(()) +} diff --git a/crates/miden-block-prover/src/tests/proven_block_success.rs b/crates/miden-block-prover/src/tests/proven_block_success.rs index 7e6b5071fe..fd833e5b15 100644 --- a/crates/miden-block-prover/src/tests/proven_block_success.rs +++ b/crates/miden-block-prover/src/tests/proven_block_success.rs @@ -1,11 +1,11 @@ use std::{collections::BTreeMap, vec::Vec}; use anyhow::Context; -use miden_crypto::merkle::{LeafIndex, SimpleSmt, Smt}; +use miden_crypto::merkle::Smt; use miden_objects::{ - ACCOUNT_TREE_DEPTH, Felt, FieldElement, MIN_PROOF_SECURITY_LEVEL, + Felt, FieldElement, MIN_PROOF_SECURITY_LEVEL, batch::BatchNoteTree, - block::{BlockInputs, BlockNoteIndex, BlockNoteTree, ProposedBlock}, + block::{AccountTree, BlockInputs, BlockNoteIndex, BlockNoteTree, ProposedBlock}, transaction::InputNoteCommitment, }; use rand::Rng; @@ -104,13 +104,14 @@ fn proven_block_success() -> anyhow::Result<()> { ); } - // Compute expected account root on the full SimpleSmt. + // Compute expected account root on the full account tree. // -------------------------------------------------------------------------------------------- let mut expected_account_tree = chain.accounts().clone(); for (account_id, witness) in proposed_block.updated_accounts() { expected_account_tree - .insert(LeafIndex::from(*account_id), *witness.final_state_commitment()); + .insert(*account_id, witness.final_state_commitment()) + .context("failed to insert account id into account tree")?; } // Prove block. @@ -366,10 +367,7 @@ fn proven_block_succeeds_with_empty_batches() -> anyhow::Result<()> { assert_eq!(latest_block_header.commitment(), blockx.commitment()); // Sanity check: The account and nullifier tree roots should not be the empty tree roots. - assert_ne!( - latest_block_header.account_root(), - SimpleSmt::::new().unwrap().root() - ); + assert_ne!(latest_block_header.account_root(), AccountTree::new().root()); assert_ne!(latest_block_header.nullifier_root(), Smt::new().root()); let (_, empty_chain_mmr) = chain.latest_selective_chain_mmr([]); diff --git a/crates/miden-lib/asm/kernels/transaction/lib/account.masm b/crates/miden-lib/asm/kernels/transaction/lib/account.masm index 9e12d4407c..2f775f7d8d 100644 --- a/crates/miden-lib/asm/kernels/transaction/lib/account.masm +++ b/crates/miden-lib/asm/kernels/transaction/lib/account.masm @@ -61,8 +61,8 @@ const.ERR_FOREIGN_ACCOUNT_ID_IS_ZERO=0x00020181 # Maximum allowed number of foreign account to be loaded (64) was exceeded. const.ERR_FOREIGN_ACCOUNT_MAX_NUMBER_EXCEEDED=0x00020183 -# State of the current foreign account is invalid. -const.ERR_FOREIGN_ACCOUNT_INVALID=0x00020182 +# Commitment of the foreign account in the advice provider does not match the commitment in the account tree. +const.ERR_FOREIGN_ACCOUNT_INVALID_COMMITMENT=0x00020182 # Unknown version in account ID. const.ERR_ACCOUNT_ID_UNKNOWN_VERSION=0x00020146 @@ -1122,23 +1122,21 @@ export.validate_current_foreign_account # => [ACCOUNT_DB_ROOT] # get the current account ID - exec.memory::get_account_id swap drop - # => [account_id_prefix, ACCOUNT_DB_ROOT] + push.0.0 exec.memory::get_account_id + # => [account_id_prefix, account_id_suffix, 0, 0, ACCOUNT_DB_ROOT] - # push the depth of the account database tree - push.ACCOUNT_TREE_DEPTH - # => [depth, account_id_prefix, ACCOUNT_DB_ROOT] - - # get the foreign account commitment - exec.get_current_commitment - # => [FOREIGN_ACCOUNT_COMMITMENT, depth, account_id_prefix, ACCOUNT_DB_ROOT] + # retrieve the commitment of the foreign account from the current account tree + # this would abort if the proof for the commitment was invalid for the account root, + # so this implicitly verifies its correctness + exec.smt::get + # => [FOREIGN_ACCOUNT_COMMITMENT, ACCOUNT_DB_ROOT] - # verify that the account database has the hash of the current foreign account - mtree_verify.err=ERR_FOREIGN_ACCOUNT_INVALID - # => [FOREIGN_ACCOUNT_COMMITMENT, depth, account_id_prefix, ACCOUNT_DB_ROOT] + # get the foreign account's commitment from memory and compare with the verified commitment + exec.get_current_commitment assert_eqw.err=ERR_FOREIGN_ACCOUNT_INVALID_COMMITMENT + # => [ACCOUNT_DB_ROOT] # clean the stack - dropw drop drop dropw + dropw end #! Hashes the provided map key before using it as the key in an SMT. diff --git a/crates/miden-lib/src/errors/tx_kernel_errors.rs b/crates/miden-lib/src/errors/tx_kernel_errors.rs index 5d6d205e8c..25e3a98723 100644 --- a/crates/miden-lib/src/errors/tx_kernel_errors.rs +++ b/crates/miden-lib/src/errors/tx_kernel_errors.rs @@ -144,8 +144,8 @@ pub const ERR_ACCOUNT_STACK_UNDERFLOW: u32 = 0x20156; pub const ERR_FOREIGN_ACCOUNT_CONTEXT_AGAINST_NATIVE_ACCOUNT: u32 = 0x20180; /// ID of the provided foreign account equals zero. pub const ERR_FOREIGN_ACCOUNT_ID_IS_ZERO: u32 = 0x20181; -/// State of the current foreign account is invalid. -pub const ERR_FOREIGN_ACCOUNT_INVALID: u32 = 0x20182; +/// Commitment of the foreign account in the advice provider does not match the commitment in the account tree. +pub const ERR_FOREIGN_ACCOUNT_INVALID_COMMITMENT: u32 = 0x20182; /// Maximum allowed number of foreign account to be loaded (64) was exceeded. pub const ERR_FOREIGN_ACCOUNT_MAX_NUMBER_EXCEEDED: u32 = 0x20183; @@ -275,7 +275,7 @@ pub const TX_KERNEL_ERRORS: [(u32, &str); 88] = [ (ERR_FOREIGN_ACCOUNT_CONTEXT_AGAINST_NATIVE_ACCOUNT, "Creation of a foreign context against the native account is forbidden"), (ERR_FOREIGN_ACCOUNT_ID_IS_ZERO, "ID of the provided foreign account equals zero."), - (ERR_FOREIGN_ACCOUNT_INVALID, "State of the current foreign account is invalid."), + (ERR_FOREIGN_ACCOUNT_INVALID_COMMITMENT, "Commitment of the foreign account in the advice provider does not match the commitment in the account tree."), (ERR_FOREIGN_ACCOUNT_MAX_NUMBER_EXCEEDED, "Maximum allowed number of foreign account to be loaded (64) was exceeded."), (ERR_FAUCET_BURN_CANNOT_EXCEED_EXISTING_TOTAL_SUPPLY, "Asset amount to burn can not exceed the existing total supply"), diff --git a/crates/miden-lib/src/transaction/mod.rs b/crates/miden-lib/src/transaction/mod.rs index 81e7b4f7de..e877784f0f 100644 --- a/crates/miden-lib/src/transaction/mod.rs +++ b/crates/miden-lib/src/transaction/mod.rs @@ -4,8 +4,8 @@ use miden_objects::{ Digest, EMPTY_WORD, Felt, TransactionOutputError, ZERO, account::{AccountCode, AccountHeader, AccountId, AccountStorageHeader}, assembly::{Assembler, DefaultSourceManager, KernelLibrary}, - block::BlockNumber, - crypto::merkle::{MerkleError, MerklePath}, + block::{AccountWitness, BlockNumber}, + crypto::merkle::MerkleError, transaction::{ OutputNote, OutputNotes, TransactionArgs, TransactionInputs, TransactionOutputs, }, @@ -202,7 +202,7 @@ impl TransactionKernel { account_header: &AccountHeader, account_code: &AccountCode, storage_header: &AccountStorageHeader, - merkle_path: &MerklePath, + account_witness: &AccountWitness, ) -> Result<(), MerkleError> { let account_id = account_header.id(); let storage_root = account_header.storage_commitment(); @@ -221,11 +221,17 @@ impl TransactionKernel { (code_root, account_code.as_elements()), ]); - // Extend the advice inputs with Merkle store data + let account_leaf = account_witness.leaf(); + let account_leaf_hash = account_leaf.hash(); + + // extend the merkle store and map with account witnesses merkle path advice_inputs.extend_merkle_store( - // The prefix is the index in the account tree. - merkle_path.inner_nodes(account_id.prefix().as_u64(), account_header.commitment())?, + account_witness + .path() + .inner_nodes(account_id.prefix().as_u64(), account_leaf_hash)?, ); + // populate advice map with the account's leaf + advice_inputs.extend_map([(account_leaf_hash, account_leaf.to_elements())]); Ok(()) } diff --git a/crates/miden-lib/src/transaction/procedures/kernel_v0.rs b/crates/miden-lib/src/transaction/procedures/kernel_v0.rs index 8b9fa19ed3..25543bd8d9 100644 --- a/crates/miden-lib/src/transaction/procedures/kernel_v0.rs +++ b/crates/miden-lib/src/transaction/procedures/kernel_v0.rs @@ -72,7 +72,7 @@ pub const KERNEL0_PROCEDURES: [Digest; 36] = [ // tx_get_block_timestamp digest!("0x786863e6dbcd5026619afd3831b7dcbf824cda54950b0e0724ebf9d9370ec723"), // tx_start_foreign_context - digest!("0x6e7ab409b5661e1d03dc73bd7b6e32ca7785a983e49ab2d056e9c786620e5c20"), + digest!("0xf7075576799b6c2b88bca3f36ed7c3b2560ed4bc0e630f1e30db0be84e746c6e"), // tx_end_foreign_context digest!("0x90a107168d81c1c0c23890e61fb7910a64b4711afd0bf8c3098d74737e4853ba"), // tx_get_expiration_delta diff --git a/crates/miden-objects/src/account/account_id/mod.rs b/crates/miden-objects/src/account/account_id/mod.rs index eef3610ac0..a3e10a901f 100644 --- a/crates/miden-objects/src/account/account_id/mod.rs +++ b/crates/miden-objects/src/account/account_id/mod.rs @@ -26,14 +26,14 @@ use alloc::string::{String, ToString}; use core::fmt; pub use id_version::AccountIdVersion; -use miden_crypto::{merkle::LeafIndex, utils::hex_to_bytes}; +use miden_crypto::utils::hex_to_bytes; use vm_core::{ Felt, Word, utils::{ByteReader, Deserializable, Serializable}, }; use vm_processor::{DeserializationError, Digest}; -use crate::{ACCOUNT_TREE_DEPTH, AccountError, errors::AccountIdError}; +use crate::{AccountError, errors::AccountIdError}; /// The identifier of an [`Account`](crate::account::Account). /// @@ -398,15 +398,6 @@ impl From for u128 { } } -/// Account IDs are used as indexes in the account database, which is a tree of depth 64. -impl From for LeafIndex { - fn from(id: AccountId) -> Self { - match id { - AccountId::V0(account_id) => account_id.into(), - } - } -} - // CONVERSIONS TO ACCOUNT ID // ================================================================================================ diff --git a/crates/miden-objects/src/account/account_id/v0/mod.rs b/crates/miden-objects/src/account/account_id/v0/mod.rs index 08fdbb2564..bd199b58f5 100644 --- a/crates/miden-objects/src/account/account_id/v0/mod.rs +++ b/crates/miden-objects/src/account/account_id/v0/mod.rs @@ -6,7 +6,7 @@ use alloc::{ use core::fmt; use bech32::{Bech32m, primitives::decode::CheckedHrpstring}; -use miden_crypto::{merkle::LeafIndex, utils::hex_to_bytes}; +use miden_crypto::utils::hex_to_bytes; pub use prefix::AccountIdPrefixV0; use vm_core::{ Felt, Word, @@ -15,7 +15,7 @@ use vm_core::{ use vm_processor::{DeserializationError, Digest}; use crate::{ - ACCOUNT_TREE_DEPTH, AccountError, Hasher, + AccountError, Hasher, account::{ AccountIdAnchor, AccountIdVersion, AccountStorageMode, AccountType, account_id::{ @@ -327,13 +327,6 @@ impl From for u128 { } } -/// Account IDs are used as indexes in the account database, which is a tree of depth 64. -impl From for LeafIndex { - fn from(id: AccountIdV0) -> Self { - LeafIndex::new_max_depth(id.prefix().as_u64()) - } -} - // CONVERSIONS TO ACCOUNT ID // ================================================================================================ diff --git a/crates/miden-objects/src/batch/ordered_batches.rs b/crates/miden-objects/src/batch/ordered_batches.rs index 32f534a6c0..2a8a7aa052 100644 --- a/crates/miden-objects/src/batch/ordered_batches.rs +++ b/crates/miden-objects/src/batch/ordered_batches.rs @@ -1,12 +1,9 @@ use alloc::vec::Vec; -use vm_core::utils::{ByteReader, Deserializable}; -use vm_processor::DeserializationError; - use crate::{ batch::ProvenBatch, transaction::OrderedTransactionHeaders, - utils::{ByteWriter, Serializable}, + utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, }; // ORDERED BATCHES @@ -52,6 +49,11 @@ impl OrderedBatches { ) } + /// Returns the sum of created notes across all batches. + pub fn num_created_notes(&self) -> usize { + self.0.as_slice().iter().fold(0, |acc, batch| acc + batch.output_notes().len()) + } + /// Consumes self and returns the underlying vector of batches. pub fn into_vec(self) -> Vec { self.0 diff --git a/crates/miden-objects/src/block/account_tree.rs b/crates/miden-objects/src/block/account_tree.rs new file mode 100644 index 0000000000..2ab57867d4 --- /dev/null +++ b/crates/miden-objects/src/block/account_tree.rs @@ -0,0 +1,432 @@ +use miden_crypto::merkle::{MerkleError, MutationSet, Smt, SmtLeaf}; +use vm_processor::SMT_DEPTH; + +use crate::{ + Digest, Felt, FieldElement, Word, + account::{AccountId, AccountIdPrefix}, + block::AccountWitness, + errors::AccountTreeError, +}; + +// ACCOUNT TREE +// ================================================================================================ + +/// The sparse merkle tree of all accounts in the blockchain. +/// +/// The key is the [`AccountId`] while the value is the current state commitment of the account, +/// i.e. [`Account::commitment`](crate::account::Account::commitment). If the account is new, then +/// the commitment is the [`EMPTY_WORD`](crate::EMPTY_WORD). +/// +/// Each account ID occupies exactly one leaf in the tree, which is identified by its +/// [`AccountId::prefix`]. In other words, account ID prefixes are unique in the blockchain. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AccountTree { + smt: Smt, +} + +impl AccountTree { + // CONSTANTS + // -------------------------------------------------------------------------------------------- + + /// The depth of the account tree. + pub const DEPTH: u8 = SMT_DEPTH; + + /// The index of the account ID suffix in the SMT key. + pub(super) const KEY_SUFFIX_IDX: usize = 2; + /// The index of the account ID prefix in the SMT key. + pub(super) const KEY_PREFIX_IDX: usize = 3; + + // CONSTRUCTORS + // -------------------------------------------------------------------------------------------- + + /// Creates a new, empty account tree. + pub fn new() -> Self { + AccountTree { smt: Smt::new() } + } + + /// Returns a new [`Smt`] instantiated with the provided entries. + /// + /// If the `concurrent` feature of `miden-crypto` is enabled, this function uses a parallel + /// implementation to process the entries efficiently, otherwise it defaults to the + /// sequential implementation. + /// + /// # Errors + /// + /// Returns an error if: + /// - the provided entries contain multiple commitments for the same account ID. + /// - multiple account IDs share the same prefix. + pub fn with_entries( + entries: impl IntoIterator, + ) -> Result { + let smt = Smt::with_entries( + entries + .into_iter() + .map(|(id, commitment)| (Self::id_to_smt_key(id), Word::from(commitment))), + ) + .map_err(|err| { + let MerkleError::DuplicateValuesForIndex(leaf_idx) = err else { + unreachable!("the only error returned by Smt::with_entries is of this type"); + }; + + // SAFETY: Since we only inserted account IDs into the SMT, it is guaranteed that + // the leaf_idx is a valid Felt as well as a valid account ID prefix. + AccountTreeError::DuplicateStateCommitments { + prefix: AccountIdPrefix::new_unchecked( + Felt::try_from(leaf_idx).expect("leaf index should be a valid felt"), + ), + } + })?; + + for (leaf_idx, leaf) in smt.leaves() { + if leaf.num_entries() >= 2 { + // SAFETY: Since we only inserted account IDs into the SMT, it is guaranteed that + // the leaf_idx is a valid Felt as well as a valid account ID prefix. + return Err(AccountTreeError::DuplicateIdPrefix { + duplicate_prefix: AccountIdPrefix::new_unchecked( + Felt::try_from(leaf_idx.value()) + .expect("leaf index should be a valid felt"), + ), + }); + } + } + + Ok(AccountTree { smt }) + } + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns an opening of the leaf associated with the `account_id`. This is a proof of the + /// current state commitment of the given account ID. + /// + /// Conceptually, an opening is a Merkle path to the leaf, as well as the leaf itself. + pub fn open(&self, account_id: AccountId) -> AccountWitness { + let key = Self::id_to_smt_key(account_id); + let proof = self.smt.open(&key); + + AccountWitness::from_smt_proof(account_id, proof) + } + + /// Returns the current state commitment of the given account ID. + pub fn get(&self, account_id: AccountId) -> Digest { + let key = Self::id_to_smt_key(account_id); + Digest::from(self.smt.get_value(&key)) + } + + /// Returns the root of the tree. + pub fn root(&self) -> Digest { + self.smt.root() + } + + /// Returns the number of account IDs in this tree. + pub fn num_accounts(&self) -> usize { + // Because each ID's prefix is unique in the tree and occupies a single leaf, the number of + // IDs in the tree is equivalent to the number of leaves in the tree. + self.smt.num_leaves() + } + + /// Returns an iterator over the account ID state commitment pairs in the tree. + pub fn account_commitments(&self) -> impl Iterator { + self.smt.leaves().map(|(_leaf_idx, leaf)| { + // SAFETY: By construction no Multiple variant is ever present in the tree. + // The Empty variant is not returned by Smt::leaves, because it only returns leaves that + // are actually present. + let SmtLeaf::Single((key, commitment)) = leaf else { + unreachable!("empty and multiple variant should never be encountered") + }; + + ( + // SAFETY: By construction, the tree only contains valid IDs. + AccountId::try_from([key[Self::KEY_PREFIX_IDX], key[Self::KEY_SUFFIX_IDX]]) + .expect("account tree should only contain valid IDs"), + Digest::from(commitment), + ) + }) + } + + /// Computes the necessary changes to insert the specified (account ID, state commitment) pairs + /// into this tree, allowing for validation before applying those changes. + /// + /// [`Self::apply_mutations`] can be used in order to commit these changes to the tree. + /// + /// If the `concurrent` feature of `miden-crypto` is enabled, this function uses a parallel + /// implementation to compute the mutations, otherwise it defaults to the sequential + /// implementation. + /// + /// This is a thin wrapper around [`Smt::compute_mutations`]. See its documentation for more + /// details. + pub fn compute_mutations( + &self, + account_commitments: impl IntoIterator, + ) -> AccountMutationSet { + let mutation_set = self.smt.compute_mutations( + account_commitments + .into_iter() + .map(|(id, commitment)| (Self::id_to_smt_key(id), Word::from(commitment))), + ); + + AccountMutationSet::new(mutation_set) + } + + // PUBLIC MUTATORS + // -------------------------------------------------------------------------------------------- + + /// Inserts the state commitment for the given account ID, returning the previous state + /// commitment associated with that ID. + /// + /// This also recomputes all hashes between the leaf (associated with the key) and the root, + /// updating the root itself. + /// + /// # Errors + /// + /// Returns an error if: + /// - the prefix of the account ID already exists in the tree. + pub fn insert( + &mut self, + account_id: AccountId, + state_commitment: Digest, + ) -> Result { + let key = Self::id_to_smt_key(account_id); + let prev_value = Digest::from(self.smt.insert(key, Word::from(state_commitment))); + + // If the leaf of the account ID now has two or more entries, we've inserted a duplicate + // prefix. + if self.smt.get_leaf(&key).num_entries() >= 2 { + return Err(AccountTreeError::DuplicateIdPrefix { + duplicate_prefix: account_id.prefix(), + }); + } + + Ok(prev_value) + } + + /// Applies the prospective mutations computed with [`Self::compute_mutations`] to this tree. + /// + /// # Errors + /// + /// Returns an error if: + /// - `mutations` was computed on a tree with a different root than this one. + pub fn apply_mutations( + &mut self, + mutations: AccountMutationSet, + ) -> Result<(), AccountTreeError> { + self.smt + .apply_mutations(mutations.into_mutation_set()) + .map_err(AccountTreeError::ApplyMutations) + } + + // HELPERS + // -------------------------------------------------------------------------------------------- + + /// Returns the SMT key of the given account ID. + pub(super) fn id_to_smt_key(account_id: AccountId) -> Digest { + // We construct this in such a way that we're forced to use the constants, so that when + // they're updated, the other usages of the constants are also updated. + let mut key = [Felt::ZERO, Felt::ZERO, Felt::ZERO, Felt::ZERO]; + key[Self::KEY_SUFFIX_IDX] = account_id.suffix(); + key[Self::KEY_PREFIX_IDX] = account_id.prefix().as_felt(); + + Digest::from(key) + } + + /// Returns the [`AccountId`] recovered from the given SMT key. + /// + /// # Panics + /// + /// Panics if: + /// - the key is not a valid account ID. This should not happen when used on keys from (partial) + /// account tree. + pub(super) fn smt_key_to_id(key: Digest) -> AccountId { + AccountId::try_from([key[Self::KEY_PREFIX_IDX], key[Self::KEY_SUFFIX_IDX]]) + .expect("account tree should only contain valid IDs") + } +} + +impl Default for AccountTree { + fn default() -> Self { + Self::new() + } +} + +// ACCOUNT MUTATION SET +// ================================================================================================ + +/// A newtype wrapper around a [`MutationSet`] which exists for type safety in some [`AccountTree`] +/// methods. +/// +/// It is returned by and used in methods on the [`AccountTree`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AccountMutationSet { + mutation_set: MutationSet<{ AccountTree::DEPTH }, Digest, Word>, +} + +impl AccountMutationSet { + // CONSTRUCTORS + // -------------------------------------------------------------------------------------------- + + /// Creates a new [`AccountMutationSet`] from the provided raw mutation set. + fn new(mutation_set: MutationSet<{ AccountTree::DEPTH }, Digest, Word>) -> Self { + Self { mutation_set } + } + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns a reference to the underlying [`MutationSet`]. + pub fn as_mutation_set(&self) -> &MutationSet<{ AccountTree::DEPTH }, Digest, Word> { + &self.mutation_set + } + + // PUBLIC MUTATORS + // -------------------------------------------------------------------------------------------- + + /// Consumes self and returns the underlying [`MutationSet`]. + pub fn into_mutation_set(self) -> MutationSet<{ AccountTree::DEPTH }, Digest, Word> { + self.mutation_set + } +} + +// TESTS +// ================================================================================================ + +#[cfg(test)] +pub(super) mod tests { + use std::vec::Vec; + + use assert_matches::assert_matches; + use vm_core::EMPTY_WORD; + + use super::*; + use crate::{ + account::{AccountStorageMode, AccountType}, + testing::account_id::{AccountIdBuilder, account_id}, + }; + + pub(crate) fn setup_duplicate_prefix_ids() -> [(AccountId, Digest); 2] { + let id0 = AccountId::try_from(account_id( + AccountType::FungibleFaucet, + AccountStorageMode::Public, + 0xaabb_ccdd, + )) + .unwrap(); + let id1 = AccountId::try_from(account_id( + AccountType::FungibleFaucet, + AccountStorageMode::Public, + 0xaabb_ccff, + )) + .unwrap(); + assert_eq!(id0.prefix(), id1.prefix(), "test requires that these ids have the same prefix"); + + let commitment0 = Digest::from([Felt::ZERO, Felt::ZERO, Felt::ZERO, Felt::new(42)]); + let commitment1 = Digest::from([Felt::ZERO, Felt::ZERO, Felt::ZERO, Felt::new(24)]); + + assert_eq!(id0.prefix(), id1.prefix(), "test requires that these ids have the same prefix"); + [(id0, commitment0), (id1, commitment1)] + } + + #[test] + fn insert_fails_on_duplicate_prefix() { + let mut tree = AccountTree::new(); + let [(id0, commitment0), (id1, commitment1)] = setup_duplicate_prefix_ids(); + + tree.insert(id0, commitment0).unwrap(); + assert_eq!(tree.get(id0), commitment0); + + let err = tree.insert(id1, commitment1).unwrap_err(); + + assert_matches!(err, AccountTreeError::DuplicateIdPrefix { + duplicate_prefix + } if duplicate_prefix == id0.prefix()); + } + + #[test] + fn with_entries_fails_on_duplicate_prefix() { + let entries = setup_duplicate_prefix_ids(); + + let err = AccountTree::with_entries(entries.iter().copied()).unwrap_err(); + + assert_matches!(err, AccountTreeError::DuplicateIdPrefix { + duplicate_prefix + } if duplicate_prefix == entries[0].0.prefix()); + } + + #[test] + fn insert_succeeds_on_multiple_updates() { + let mut tree = AccountTree::new(); + let [(id0, commitment0), (_, commitment1)] = setup_duplicate_prefix_ids(); + + tree.insert(id0, commitment0).unwrap(); + tree.insert(id0, commitment1).unwrap(); + assert_eq!(tree.get(id0), commitment1); + } + + #[test] + fn apply_mutations() { + let id0 = AccountIdBuilder::new().build_with_seed([5; 32]); + let id1 = AccountIdBuilder::new().build_with_seed([6; 32]); + let id2 = AccountIdBuilder::new().build_with_seed([7; 32]); + + let digest0 = Digest::from([0, 0, 0, 1u32]); + let digest1 = Digest::from([0, 0, 0, 2u32]); + let digest2 = Digest::from([0, 0, 0, 3u32]); + let digest3 = Digest::from([0, 0, 0, 4u32]); + + let mut tree = AccountTree::with_entries([(id0, digest0), (id1, digest1)]).unwrap(); + + let mutations = tree.compute_mutations([(id0, digest1), (id1, digest2), (id2, digest3)]); + + tree.apply_mutations(mutations).unwrap(); + + assert_eq!(tree.num_accounts(), 3); + assert_eq!(tree.get(id0), digest1); + assert_eq!(tree.get(id1), digest2); + assert_eq!(tree.get(id2), digest3); + } + + #[test] + fn account_commitments() { + let id0 = AccountIdBuilder::new().build_with_seed([5; 32]); + let id1 = AccountIdBuilder::new().build_with_seed([6; 32]); + let id2 = AccountIdBuilder::new().build_with_seed([7; 32]); + + let digest0 = Digest::from([0, 0, 0, 1u32]); + let digest1 = Digest::from([0, 0, 0, 2u32]); + let digest2 = Digest::from([0, 0, 0, 3u32]); + let empty_digest = Digest::from(EMPTY_WORD); + + let mut tree = + AccountTree::with_entries([(id0, digest0), (id1, digest1), (id2, digest2)]).unwrap(); + + // remove id2 + tree.insert(id2, empty_digest).unwrap(); + + assert_eq!(tree.num_accounts(), 2); + + let accounts: Vec<_> = tree.account_commitments().collect(); + assert_eq!(accounts.len(), 2); + assert!(accounts.contains(&(id0, digest0))); + assert!(accounts.contains(&(id1, digest1))); + } + + #[test] + fn account_witness() { + let id0 = AccountIdBuilder::new().build_with_seed([5; 32]); + let id1 = AccountIdBuilder::new().build_with_seed([6; 32]); + + let digest0 = Digest::from([0, 0, 0, 1u32]); + let digest1 = Digest::from([0, 0, 0, 2u32]); + + let tree = AccountTree::with_entries([(id0, digest0), (id1, digest1)]).unwrap(); + + assert_eq!(tree.num_accounts(), 2); + + for id in [id0, id1] { + let (control_path, control_leaf) = + tree.smt.open(&AccountTree::id_to_smt_key(id)).into_parts(); + let witness = tree.open(id); + + assert_eq!(witness.leaf(), control_leaf); + assert_eq!(witness.path(), &control_path); + } + } +} diff --git a/crates/miden-objects/src/block/account_update_witness.rs b/crates/miden-objects/src/block/account_update_witness.rs index 3b26eb0221..7809a2be05 100644 --- a/crates/miden-objects/src/block/account_update_witness.rs +++ b/crates/miden-objects/src/block/account_update_witness.rs @@ -1,15 +1,15 @@ use crate::{ Digest, account::delta::AccountUpdateDetails, - crypto::merkle::MerklePath, + block::AccountWitness, utils::serde::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, }; /// This type encapsulates essentially three components: /// - The initial and final state commitment of the account update. -/// - The witness is a merkle path of the initial state commitment of the account before the block -/// in which the witness is included, that is, in the account tree at the state of the previous -/// block header. +/// - The witness is an smt proof of the initial state commitment of the account before the block in +/// which the witness is included, that is, in the account tree at the state of the previous block +/// header. /// - The account update details represent the delta between the state of the account before the /// block and the state after this block. #[derive(Debug, Clone, PartialEq, Eq)] @@ -18,9 +18,9 @@ pub struct AccountUpdateWitness { initial_state_commitment: Digest, /// The state commitment after the update. final_state_commitment: Digest, - /// The merkle path for the account tree proving that the initial state commitment is the - /// current state. - initial_state_proof: MerklePath, + /// The account witness proving that the initial state commitment is the current state in the + /// account tree. + initial_state_proof: AccountWitness, /// A set of changes which can be applied to the previous account state (i.e., the state as of /// the last block, equivalent to `initial_state_commitment`) to get the new account state. For /// private accounts, this is set to [`AccountUpdateDetails::Private`]. @@ -35,7 +35,7 @@ impl AccountUpdateWitness { pub fn new( initial_state_commitment: Digest, final_state_commitment: Digest, - initial_state_proof: MerklePath, + initial_state_proof: AccountWitness, details: AccountUpdateDetails, ) -> Self { Self { @@ -60,10 +60,15 @@ impl AccountUpdateWitness { } /// Returns a reference to the initial state proof of the account. - pub fn initial_state_proof(&self) -> &MerklePath { + pub fn as_witness(&self) -> &AccountWitness { &self.initial_state_proof } + /// Returns the [`AccountWitness`] of this update witness. + pub fn to_witness(&self) -> AccountWitness { + self.initial_state_proof.clone() + } + /// Returns a reference to the underlying [`AccountUpdateDetails`] of this update, representing /// the state transition of the account from the previous block to the block this witness is /// for. @@ -75,12 +80,12 @@ impl AccountUpdateWitness { // -------------------------------------------------------------------------------------------- /// Returns a mutable reference to the initial state proof of the account. - pub fn initial_state_proof_mut(&mut self) -> &mut MerklePath { + pub fn initial_state_proof_mut(&mut self) -> &mut AccountWitness { &mut self.initial_state_proof } /// Consumes self and returns its parts. - pub fn into_parts(self) -> (Digest, Digest, MerklePath, AccountUpdateDetails) { + pub fn into_parts(self) -> (Digest, Digest, AccountWitness, AccountUpdateDetails) { ( self.initial_state_commitment, self.final_state_commitment, diff --git a/crates/miden-objects/src/block/account_witness.rs b/crates/miden-objects/src/block/account_witness.rs index 518514668b..d1f3260706 100644 --- a/crates/miden-objects/src/block/account_witness.rs +++ b/crates/miden-objects/src/block/account_witness.rs @@ -1,34 +1,184 @@ -use crate::{Digest, crypto::merkle::MerklePath}; +use alloc::string::ToString; + +use miden_crypto::merkle::{LeafIndex, MerklePath, SMT_DEPTH, SmtLeaf, SmtProof, SmtProofError}; + +use crate::{ + AccountTreeError, Digest, Word, + account::AccountId, + block::AccountTree, + utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, +}; // ACCOUNT WITNESS // ================================================================================================ -/// A proof that a certain account is in the account tree and whose current state is the contained -/// initial state commitment. -#[derive(Debug, Clone)] +/// A specialized version of an [`SmtProof`] for use in [`AccountTree`] and +/// [`PartialAccountTree`](crate::block::PartialAccountTree). It proves the inclusion of an account +/// ID at a certain state (i.e. [`Account::commitment`](crate::account::Account::commitment)) in the +/// [`AccountTree`]. +/// +/// By construction the witness can only represent the equivalent of an [`SmtLeaf`] with zero or one +/// entries, which guarantees that the account ID prefix it represents is unique in the tree. +/// +/// # Guarantees +/// +/// This type guarantees that: +/// - its MerklePath is of depth [`SMT_DEPTH`]. +/// - converting this type into an [`SmtProof`] results in a leaf with zero or one entries, i.e. the +/// account ID prefix is unique. +#[derive(Debug, Clone, PartialEq, Eq)] pub struct AccountWitness { - initial_state_commitment: Digest, - proof: MerklePath, + /// The account ID that this witness proves inclusion for. + id: AccountId, + /// The state commitment of the account ID. + commitment: Digest, + /// The merkle path of the account witness. + path: MerklePath, } impl AccountWitness { /// Constructs a new [`AccountWitness`] from the provided parts. - pub fn new(initial_state_commitment: Digest, proof: MerklePath) -> Self { - Self { initial_state_commitment, proof } + /// + /// # Errors + /// + /// Returns an error if: + /// - the merkle path's depth is not [`AccountTree::DEPTH`]. + pub fn new( + account_id: AccountId, + commitment: Digest, + path: MerklePath, + ) -> Result { + if path.len() != SMT_DEPTH as usize { + return Err(AccountTreeError::WitnessMerklePathDepthDoesNotMatchAccountTreeDepth( + path.len(), + )); + } + + Ok(Self::new_unchecked(account_id, commitment, path)) + } + + /// Creates an [`AccountWitness`] from the provided proof and the account ID for which the proof + /// was requested. + /// + /// # Warning + /// + /// This should only be called on SMT proofs retrieved from (partial) account tree, because it + /// relies on the guarantees of those types. + /// + /// # Panics + /// + /// Panics if: + /// - the merkle path in the proof does not have depth equal to [`AccountTree::DEPTH`]. + /// - the proof contains an SmtLeaf::Multiple. + pub(super) fn from_smt_proof(requested_account_id: AccountId, proof: SmtProof) -> Self { + // Check which account ID this proof actually contains. We rely on the fact that the + // trees only contain zero or one entry per account ID prefix. + // + // If the requested account ID matches an existing ID's prefix but their suffixes do + // not match, then this witness is for the _existing ID_. + // + // Otherwise, if the ID matches the one in the leaf or if it's empty, the witness is + // for the requested ID. + let witness_id = match proof.leaf() { + SmtLeaf::Empty(_) => requested_account_id, + SmtLeaf::Single((key_in_leaf, _)) => { + // SAFETY: By construction, the tree only contains valid IDs. + AccountTree::smt_key_to_id(*key_in_leaf) + }, + SmtLeaf::Multiple(_) => { + unreachable!("account tree should only contain zero or one entry per ID prefix") + }, + }; + + let commitment = Digest::from( + proof + .get(&AccountTree::id_to_smt_key(witness_id)) + .expect("we should have received a proof for the witness key"), + ); + + // SAFETY: The proof is guaranteed to have depth AccountTree::DEPTH if it comes from one of + // the account trees. + debug_assert_eq!(proof.path().depth(), AccountTree::DEPTH); + + AccountWitness::new_unchecked(witness_id, commitment, proof.into_parts().0) + } + + /// Constructs a new [`AccountWitness`] from the provided parts. + /// + /// # Warning + /// + /// This does not validate any of the guarantees of this type. + pub(super) fn new_unchecked( + account_id: AccountId, + commitment: Digest, + path: MerklePath, + ) -> Self { + Self { id: account_id, commitment, path } + } + + /// Returns the underlying [`AccountId`] that this witness proves inclusion for. + pub fn id(&self) -> AccountId { + self.id + } + + /// Returns the state commitment of the account witness. + pub fn state_commitment(&self) -> Digest { + self.commitment + } + + /// Returns the [`MerklePath`] of the account witness. + pub fn path(&self) -> &MerklePath { + &self.path + } + + /// Returns the [`SmtLeaf`] of the account witness. + pub fn leaf(&self) -> SmtLeaf { + if self.commitment == Digest::default() { + let leaf_idx = LeafIndex::from(AccountTree::id_to_smt_key(self.id)); + SmtLeaf::new_empty(leaf_idx) + } else { + let key = AccountTree::id_to_smt_key(self.id); + SmtLeaf::new_single(key, Word::from(self.commitment)) + } + } + + /// Consumes self and returns the inner proof. + pub fn into_proof(self) -> SmtProof { + let leaf = self.leaf(); + SmtProof::new(self.path, leaf) + .expect("merkle path depth should be the SMT depth by construction") } +} - /// Returns the initial state commitment that this witness proves is the current state. - pub fn initial_state_commitment(&self) -> Digest { - self.initial_state_commitment +impl From for SmtProof { + fn from(witness: AccountWitness) -> Self { + witness.into_proof() } +} + +// SERIALIZATION +// ================================================================================================ - /// Returns the merkle path for the account tree of this witness. - pub fn proof(&self) -> &MerklePath { - &self.proof +impl Serializable for AccountWitness { + fn write_into(&self, target: &mut W) { + self.id.write_into(target); + self.commitment.write_into(target); + self.path.write_into(target); } +} + +impl Deserializable for AccountWitness { + fn read_from(source: &mut R) -> Result { + let id = AccountId::read_from(source)?; + let commitment = Digest::read_from(source)?; + let path = MerklePath::read_from(source)?; + + if path.len() != SMT_DEPTH as usize { + return Err(DeserializationError::InvalidValue( + SmtProofError::InvalidMerklePathLength(path.len()).to_string(), + )); + } - /// Consumes self and returns the parts of the witness. - pub fn into_parts(self) -> (Digest, MerklePath) { - (self.initial_state_commitment, self.proof) + Ok(Self { id, commitment, path }) } } diff --git a/crates/miden-objects/src/block/header.rs b/crates/miden-objects/src/block/header.rs index 7eec38ef9e..e1afd99188 100644 --- a/crates/miden-objects/src/block/header.rs +++ b/crates/miden-objects/src/block/header.rs @@ -2,9 +2,7 @@ use alloc::vec::Vec; use crate::{ Digest, Felt, Hasher, ZERO, - account::AccountId, block::BlockNumber, - transaction::TransactionId, utils::serde::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, }; @@ -224,20 +222,6 @@ impl BlockHeader { elements.extend([block_num.into(), version.into(), timestamp.into(), ZERO]); Hasher::hash_elements(&elements) } - - /// Computes a commitment to the provided list of transactions. - pub fn compute_tx_commitment( - updated_accounts: impl Iterator, - ) -> Digest { - let mut elements = vec![]; - for (transaction_id, account_id) in updated_accounts { - let [account_id_prefix, account_id_suffix] = <[Felt; 2]>::from(account_id); - elements.extend_from_slice(transaction_id.as_elements()); - elements.extend_from_slice(&[account_id_prefix, account_id_suffix, ZERO, ZERO]); - } - - Hasher::hash_elements(&elements) - } } // SERIALIZATION diff --git a/crates/miden-objects/src/block/mod.rs b/crates/miden-objects/src/block/mod.rs index e119b94e61..7cc41c30f7 100644 --- a/crates/miden-objects/src/block/mod.rs +++ b/crates/miden-objects/src/block/mod.rs @@ -13,6 +13,12 @@ pub use proven_block::ProvenBlock; mod nullifier_witness; pub use nullifier_witness::NullifierWitness; +mod partial_account_tree; +pub use partial_account_tree::PartialAccountTree; + +pub(super) mod account_tree; +pub use account_tree::AccountTree; + mod partial_nullifier_tree; pub use partial_nullifier_tree::PartialNullifierTree; diff --git a/crates/miden-objects/src/block/partial_account_tree.rs b/crates/miden-objects/src/block/partial_account_tree.rs new file mode 100644 index 0000000000..620b40a18e --- /dev/null +++ b/crates/miden-objects/src/block/partial_account_tree.rs @@ -0,0 +1,296 @@ +use miden_crypto::merkle::SmtLeaf; + +use crate::{ + Digest, Word, + account::AccountId, + block::{AccountTree, AccountWitness}, + crypto::merkle::PartialSmt, + errors::AccountTreeError, +}; + +/// The partial sparse merkle tree containing the state commitments of accounts in the chain. +/// +/// This is the partial version of [`AccountTree`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PartialAccountTree { + smt: PartialSmt, +} + +impl PartialAccountTree { + // CONSTRUCTORS + // -------------------------------------------------------------------------------------------- + + /// Creates a new, empty partial account tree. + pub fn new() -> Self { + PartialAccountTree { smt: PartialSmt::new() } + } + + /// Returns a new [`PartialAccountTree`] instantiated with the provided entries. + /// + /// # Errors + /// + /// Returns an error if: + /// - the merkle paths of the witnesses do not result in the same tree root. + /// - there are multiple witnesses for the same ID _prefix_. + pub fn with_witnesses( + witnesses: impl IntoIterator, + ) -> Result { + let mut tree = Self::new(); + + for witness in witnesses { + tree.track_account(witness)?; + } + + Ok(tree) + } + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns an opening of the leaf associated with the `account_id`. This is a proof of the + /// current state commitment of the given account ID. + /// + /// Conceptually, an opening is a Merkle path to the leaf, as well as the leaf itself. + /// + /// # Errors + /// + /// Returns an error if: + /// - the account ID is not tracked by this account tree. + pub fn open(&self, account_id: AccountId) -> Result { + let key = AccountTree::id_to_smt_key(account_id); + + self.smt + .open(&key) + .map(|proof| AccountWitness::from_smt_proof(account_id, proof)) + .map_err(|source| AccountTreeError::UntrackedAccountId { id: account_id, source }) + } + + /// Returns the current state commitment of the given account ID. + /// + /// # Errors + /// + /// Returns an error if: + /// - the account ID is not tracked by this account tree. + pub fn get(&self, account_id: AccountId) -> Result { + let key = AccountTree::id_to_smt_key(account_id); + self.smt + .get_value(&key) + .map(Digest::from) + .map_err(|source| AccountTreeError::UntrackedAccountId { id: account_id, source }) + } + + /// Returns the root of the tree. + pub fn root(&self) -> Digest { + self.smt.root() + } + + // PUBLIC MUTATORS + // -------------------------------------------------------------------------------------------- + + /// Adds the given account witness to the partial tree and tracks it. Once an account has + /// been added to the tree, it can be updated using [`Self::upsert_state_commitments`]. + /// + /// # Errors + /// + /// Returns an error if: + /// - after the witness' merkle path was added, the partial account tree has a different root + /// than before it was added (except when the first witness is added). + /// - there exists a leaf in the tree whose account ID prefix matches the one in the provided + /// witness. + pub fn track_account(&mut self, witness: AccountWitness) -> Result<(), AccountTreeError> { + let id_prefix = witness.id().prefix(); + let id_key = AccountTree::id_to_smt_key(witness.id()); + let (path, leaf) = witness.into_proof().into_parts(); + + // If a leaf with the same prefix is already tracked by this partial tree, consider it an + // error. + // + // We return an error even for empty leaves, because tracking the same ID prefix twice + // indicates that different IDs are attempted to be tracked. It would technically + // not violate the invariant of the tree that it only tracks zero or one entries per leaf, + // but since tracking the same ID twice should practically never happen, we return an error, + // out of an abundance of caution. + if self.smt.get_leaf(&id_key).is_ok() { + return Err(AccountTreeError::DuplicateIdPrefix { duplicate_prefix: id_prefix }); + } + + self.smt.add_path(leaf, path).map_err(AccountTreeError::TreeRootConflict)?; + + Ok(()) + } + + /// Inserts or updates the provided account ID -> state commitment updates into the partial tree + /// which results in a new tree root. + /// + /// # Errors + /// + /// Returns an error if: + /// - the prefix of the account ID already exists in the tree. + /// - the account_id is not tracked by this partial account tree. + pub fn upsert_state_commitments( + &mut self, + updates: impl IntoIterator, + ) -> Result<(), AccountTreeError> { + for (account_id, state_commitment) in updates { + self.insert(account_id, state_commitment)?; + } + + Ok(()) + } + + /// Inserts the state commitment for the given account ID, returning the previous state + /// commitment associated with that ID. + /// + /// This also recomputes all hashes between the leaf (associated with the key) and the root, + /// updating the root itself. + /// + /// # Errors + /// + /// Returns an error if: + /// - the prefix of the account ID already exists in the tree. + /// - the account_id is not tracked by this partial account tree. + fn insert( + &mut self, + account_id: AccountId, + state_commitment: Digest, + ) -> Result { + let key = AccountTree::id_to_smt_key(account_id); + + // If there exists a tracked leaf whose key is _not_ the one we're about to overwrite, then + // we would insert the new commitment next to an existing account ID with the same prefix, + // which is an error. + // Note that if the leaf is empty, that's fine. It means it is tracked by the partial SMT, + // but no account ID is inserted yet. + // Also note that the multiple variant cannot occur by construction of the tree. + if let Ok(SmtLeaf::Single((existing_key, _))) = self.smt.get_leaf(&key) { + if key != existing_key { + return Err(AccountTreeError::DuplicateIdPrefix { + duplicate_prefix: account_id.prefix(), + }); + } + } + + self.smt + .insert(key, Word::from(state_commitment)) + .map(Digest::from) + .map_err(|source| AccountTreeError::UntrackedAccountId { id: account_id, source }) + } +} + +impl Default for PartialAccountTree { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use assert_matches::assert_matches; + use miden_crypto::merkle::Smt; + + use super::*; + use crate::block::account_tree::tests::setup_duplicate_prefix_ids; + + #[test] + fn insert_fails_on_duplicate_prefix() { + let mut full_tree = AccountTree::new(); + let mut partial_tree = PartialAccountTree::new(); + + let [(id0, commitment0), (id1, commitment1)] = setup_duplicate_prefix_ids(); + + full_tree.insert(id0, commitment0).unwrap(); + let witness = full_tree.open(id0); + + partial_tree.track_account(witness).unwrap(); + + partial_tree.insert(id0, commitment0).unwrap(); + assert_eq!(partial_tree.get(id0).unwrap(), commitment0); + + let err = partial_tree.insert(id1, commitment1).unwrap_err(); + + assert_matches!(err, AccountTreeError::DuplicateIdPrefix { + duplicate_prefix + } if duplicate_prefix == id0.prefix()); + + partial_tree.upsert_state_commitments([(id1, commitment1)]).unwrap_err(); + + assert_matches!(err, AccountTreeError::DuplicateIdPrefix { + duplicate_prefix + } if duplicate_prefix == id0.prefix()); + } + + #[test] + fn insert_succeeds_on_multiple_updates() { + let mut full_tree = AccountTree::new(); + let mut partial_tree = PartialAccountTree::new(); + let [(id0, commitment0), (_, commitment1)] = setup_duplicate_prefix_ids(); + + full_tree.insert(id0, commitment0).unwrap(); + let witness = full_tree.open(id0); + + partial_tree.track_account(witness.clone()).unwrap(); + assert_eq!( + partial_tree.open(id0).unwrap(), + witness, + "full tree witness and partial tree witness should be the same" + ); + assert_eq!( + partial_tree.root(), + full_tree.root(), + "full tree root and partial tree root should be the same" + ); + + partial_tree.insert(id0, commitment0).unwrap(); + partial_tree.insert(id0, commitment1).unwrap(); + assert_eq!(partial_tree.get(id0).unwrap(), commitment1); + } + + #[test] + fn upsert_state_commitments_fails_on_untracked_key() { + let mut partial_tree = PartialAccountTree::new(); + let [update, _] = setup_duplicate_prefix_ids(); + + let err = partial_tree.upsert_state_commitments([update]).unwrap_err(); + assert_matches!(err, AccountTreeError::UntrackedAccountId { id, .. } + if id == update.0 + ) + } + + #[test] + fn track_fails_on_duplicate_prefix() { + // Use a raw Smt since an account tree would not allow us to get the witnesses for two + // account IDs with the same prefix. + let full_tree = Smt::with_entries( + setup_duplicate_prefix_ids() + .map(|(id, commitment)| (AccountTree::id_to_smt_key(id), Word::from(commitment))), + ) + .unwrap(); + + let [(id0, _), (id1, _)] = setup_duplicate_prefix_ids(); + + let key0 = AccountTree::id_to_smt_key(id0); + let key1 = AccountTree::id_to_smt_key(id1); + let proof0 = full_tree.open(&key0); + let proof1 = full_tree.open(&key1); + assert_eq!(proof0.leaf(), proof1.leaf()); + + let witness0 = AccountWitness::new_unchecked( + id0, + proof0.get(&key0).unwrap().into(), + proof0.into_parts().0, + ); + let witness1 = AccountWitness::new_unchecked( + id1, + proof1.get(&key1).unwrap().into(), + proof1.into_parts().0, + ); + + let mut partial_tree = PartialAccountTree::new(); + partial_tree.track_account(witness0).unwrap(); + let err = partial_tree.track_account(witness1).unwrap_err(); + + assert_matches!(err, AccountTreeError::DuplicateIdPrefix { duplicate_prefix, .. } + if duplicate_prefix == id1.prefix() + ) + } +} diff --git a/crates/miden-objects/src/block/partial_nullifier_tree.rs b/crates/miden-objects/src/block/partial_nullifier_tree.rs index 7ac5b9cc79..625508a62a 100644 --- a/crates/miden-objects/src/block/partial_nullifier_tree.rs +++ b/crates/miden-objects/src/block/partial_nullifier_tree.rs @@ -25,12 +25,9 @@ impl PartialNullifierTree { /// # Errors /// /// Returns an error if: - /// - after the witness' merkle path was added the partial nullifier tree has a different root + /// - after the witness' merkle path was added, the partial nullifier tree has a different root /// than before it was added. - pub fn add_nullifier_witness( - &mut self, - witness: NullifierWitness, - ) -> Result<(), NullifierTreeError> { + pub fn track_nullifier(&mut self, witness: NullifierWitness) -> Result<(), NullifierTreeError> { let (path, leaf) = witness.into_proof().into_parts(); self.0.add_path(leaf, path).map_err(NullifierTreeError::TreeRootConflict) } @@ -130,8 +127,8 @@ mod tests { let mut partial = PartialNullifierTree::new(); - partial.add_nullifier_witness(NullifierWitness::new(stale_proof0)).unwrap(); - let error = partial.add_nullifier_witness(NullifierWitness::new(proof2)).unwrap_err(); + partial.track_nullifier(NullifierWitness::new(stale_proof0)).unwrap(); + let error = partial.track_nullifier(NullifierWitness::new(proof2)).unwrap_err(); assert_matches!(error, NullifierTreeError::TreeRootConflict(_)); } diff --git a/crates/miden-objects/src/block/proposed_block.rs b/crates/miden-objects/src/block/proposed_block.rs index 59124bf6c1..a4d034e29f 100644 --- a/crates/miden-objects/src/block/proposed_block.rs +++ b/crates/miden-objects/src/block/proposed_block.rs @@ -652,10 +652,17 @@ impl AccountUpdateAggregator { /// chronological order using the state commitments to link them. fn aggregate_account( account_id: AccountId, - witness: AccountWitness, + initial_state_proof: AccountWitness, mut updates: BTreeMap, ) -> Result { - let (initial_state_commitment, initial_state_proof) = witness.into_parts(); + // The account witness could prove inclusion of a different ID in which case the initial + // state commitment of the current ID is the empty word. + let initial_state_commitment = if account_id == initial_state_proof.id() { + initial_state_proof.state_commitment() + } else { + Digest::from(EMPTY_WORD) + }; + let mut details: Option = None; let mut current_commitment = initial_state_commitment; diff --git a/crates/miden-objects/src/errors.rs b/crates/miden-objects/src/errors.rs index 5a959bcf0a..4a52ac155c 100644 --- a/crates/miden-objects/src/errors.rs +++ b/crates/miden-objects/src/errors.rs @@ -168,6 +168,31 @@ pub enum AccountIdError { Bech32DecodeError(#[source] Bech32Error), } +// ACCOUNT TREE ERROR +// ================================================================================================ + +#[derive(Debug, Error)] +pub enum AccountTreeError { + #[error( + "account tree contains multiple account IDs that share the same prefix {duplicate_prefix}" + )] + DuplicateIdPrefix { duplicate_prefix: AccountIdPrefix }, + #[error( + "entries passed to account tree contain multiple state commitments for the same account ID prefix {prefix}" + )] + DuplicateStateCommitments { prefix: AccountIdPrefix }, + #[error("untracked account ID {id} used in partial account tree")] + UntrackedAccountId { id: AccountId, source: MerkleError }, + #[error("new tree root after account witness insertion does not match previous tree root")] + TreeRootConflict(#[source] MerkleError), + #[error("failed to apply mutations to account tree")] + ApplyMutations(#[source] MerkleError), + #[error("smt leaf's index is not a valid account ID prefix")] + InvalidAccountIdPrefix(#[source] AccountIdError), + #[error("account witness merkle path depth {0} does not match AccountTree::DEPTH")] + WitnessMerklePathDepthDoesNotMatchAccountTreeDepth(usize), +} + // BECH32 ERROR // ================================================================================================ diff --git a/crates/miden-objects/src/lib.rs b/crates/miden-objects/src/lib.rs index fac839ae1c..2b745fac76 100644 --- a/crates/miden-objects/src/lib.rs +++ b/crates/miden-objects/src/lib.rs @@ -24,7 +24,7 @@ mod errors; pub use constants::*; pub use errors::{ - AccountDeltaError, AccountError, AccountIdError, AssetError, AssetVaultError, + AccountDeltaError, AccountError, AccountIdError, AccountTreeError, AssetError, AssetVaultError, BatchAccountUpdateError, ChainMmrError, NetworkIdError, NoteError, NullifierTreeError, ProposedBatchError, ProposedBlockError, ProvenBatchError, ProvenTransactionError, TransactionInputError, TransactionOutputError, TransactionScriptError, diff --git a/crates/miden-objects/src/testing/account_id.rs b/crates/miden-objects/src/testing/account_id.rs index fb9b26a654..06f45286b1 100644 --- a/crates/miden-objects/src/testing/account_id.rs +++ b/crates/miden-objects/src/testing/account_id.rs @@ -92,8 +92,8 @@ pub const ACCOUNT_ID_MAX_ZEROES: u128 = /// then the layout of the generated ID will be: /// /// ```text -/// 1st felt: [0xaa | 5 zero bytes | 0xbb | metadata byte] -/// 2nd felt: [2 zero bytes (epoch) | 0xcc | 3 zero bytes | 0xdd | zero byte] +/// prefix: [0xaa | 5 zero bytes | 0xbb | metadata byte] +/// suffix: [2 zero bytes (epoch) | 0xcc | 3 zero bytes | 0xdd | zero byte] /// ``` pub const fn account_id( account_type: AccountType, diff --git a/crates/miden-objects/src/testing/block.rs b/crates/miden-objects/src/testing/block.rs index 70081abfef..ff2a2be3c2 100644 --- a/crates/miden-objects/src/testing/block.rs +++ b/crates/miden-objects/src/testing/block.rs @@ -1,15 +1,12 @@ -use alloc::vec::Vec; - -use miden_crypto::merkle::SimpleSmt; -use vm_core::Felt; -use vm_processor::Digest; #[cfg(not(target_family = "wasm"))] use winter_rand_utils::{rand_array, rand_value}; +#[cfg(not(target_family = "wasm"))] +use crate::Felt; use crate::{ - ACCOUNT_TREE_DEPTH, + Digest, account::Account, - block::{BlockHeader, BlockNumber}, + block::{AccountTree, BlockHeader, BlockNumber}, }; impl BlockHeader { @@ -26,20 +23,9 @@ impl BlockHeader { accounts: &[Account], tx_kernel_commitment: Digest, ) -> Self { - let acct_db = SimpleSmt::::with_leaves( - accounts - .iter() - .flat_map(|acct| { - if acct.is_new() { - None - } else { - let felt_id: Felt = acct.id().prefix().into(); - Some((felt_id.as_int(), *acct.commitment())) - } - }) - .collect::>(), - ) - .expect("failed to create account db"); + let acct_db = + AccountTree::with_entries(accounts.iter().map(|acct| (acct.id(), acct.commitment()))) + .expect("failed to create account db"); let account_root = acct_db.root(); #[cfg(not(target_family = "wasm"))] diff --git a/crates/miden-objects/src/transaction/ordered_transactions.rs b/crates/miden-objects/src/transaction/ordered_transactions.rs index e5bbe0ac80..5a8f9c97dd 100644 --- a/crates/miden-objects/src/transaction/ordered_transactions.rs +++ b/crates/miden-objects/src/transaction/ordered_transactions.rs @@ -1,11 +1,10 @@ use alloc::vec::Vec; -use vm_core::utils::{ByteReader, Deserializable}; -use vm_processor::DeserializationError; - use crate::{ - transaction::TransactionHeader, - utils::{ByteWriter, Serializable}, + Digest, Felt, Hasher, ZERO, + account::AccountId, + transaction::{TransactionHeader, TransactionId}, + utils::{ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable}, }; // ORDERED TRANSACTION HEADERS @@ -37,6 +36,13 @@ impl OrderedTransactionHeaders { Self(transactions) } + /// Computes a commitment to the list of transactions. + /// + /// This is a sequential hash over each transaction's ID and its account ID. + pub fn commitment(&self) -> Digest { + Self::compute_commitment(self.0.as_slice().iter().map(|tx| (tx.id(), tx.account_id()))) + } + /// Returns a reference to the underlying transaction headers. pub fn as_slice(&self) -> &[TransactionHeader] { &self.0 @@ -46,6 +52,26 @@ impl OrderedTransactionHeaders { pub fn into_vec(self) -> Vec { self.0 } + + // PUBLIC HELPERS + // -------------------------------------------------------------------------------------------- + + /// Computes a commitment to the provided list of transactions. + /// + /// Each transaction is represented by a transaction ID and an account ID which it was executed + /// against. The commitment is a sequential hash over (transaction_id, account_id) tuples. + pub fn compute_commitment( + transactions: impl Iterator, + ) -> Digest { + let mut elements = vec![]; + for (transaction_id, account_id) in transactions { + let [account_id_prefix, account_id_suffix] = <[Felt; 2]>::from(account_id); + elements.extend_from_slice(transaction_id.as_elements()); + elements.extend_from_slice(&[account_id_prefix, account_id_suffix, ZERO, ZERO]); + } + + Hasher::hash_elements(&elements) + } } // SERIALIZATION diff --git a/crates/miden-tx/src/testing/mock_chain/mod.rs b/crates/miden-tx/src/testing/mock_chain/mod.rs index f7e21fa95d..88ca9ad51e 100644 --- a/crates/miden-tx/src/testing/mock_chain/mod.rs +++ b/crates/miden-tx/src/testing/mock_chain/mod.rs @@ -9,7 +9,7 @@ use miden_lib::{ transaction::{TransactionKernel, memory}, }; use miden_objects::{ - ACCOUNT_TREE_DEPTH, AccountError, NoteError, ProposedBatchError, ProposedBlockError, + AccountError, NoteError, ProposedBatchError, ProposedBlockError, account::{ Account, AccountBuilder, AccountComponent, AccountDelta, AccountId, AccountIdAnchor, AccountType, AuthSecretKey, delta::AccountUpdateDetails, @@ -17,12 +17,12 @@ use miden_objects::{ asset::{Asset, FungibleAsset, TokenSymbol}, batch::{ProposedBatch, ProvenBatch}, block::{ - AccountWitness, BlockAccountUpdate, BlockHeader, BlockInputs, BlockNoteIndex, + AccountTree, AccountWitness, BlockAccountUpdate, BlockHeader, BlockInputs, BlockNoteIndex, BlockNoteTree, BlockNumber, NullifierWitness, OutputNoteBatch, ProposedBlock, ProvenBlock, }, crypto::{ dsa::rpo_falcon512::SecretKey, - merkle::{LeafIndex, Mmr, Smt}, + merkle::{Mmr, Smt}, }, note::{Note, NoteHeader, NoteId, NoteInclusionProof, NoteType, Nullifier}, testing::account_code::DEFAULT_AUTH_SCRIPT, @@ -34,10 +34,7 @@ use miden_objects::{ }; use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha20Rng; -use vm_processor::{ - Digest, Felt, Word, ZERO, - crypto::{RpoRandomCoin, SimpleSmt}, -}; +use vm_processor::{Digest, Felt, Word, ZERO, crypto::RpoRandomCoin}; use super::TransactionContextBuilder; use crate::auth::BasicAuthenticator; @@ -260,8 +257,8 @@ pub struct MockChain { /// Tree containing the latest `Nullifier`'s tree. nullifiers: Smt, - /// Tree containing the latest hash of each account. - accounts: SimpleSmt, + /// Tree containing the latest state commitment of each account. + account_tree: AccountTree, /// Objects that have not yet been finalized. /// @@ -288,7 +285,7 @@ impl Default for MockChain { chain: Mmr::default(), blocks: vec![], nullifiers: Smt::default(), - accounts: SimpleSmt::::new().expect("depth too big for SimpleSmt"), + account_tree: AccountTree::new(), pending_objects: PendingObjects::new(), available_notes: BTreeMap::new(), available_accounts: BTreeMap::new(), @@ -635,8 +632,7 @@ impl MockChain { self.available_accounts .insert(account.id(), MockAccount::new(account.clone(), seed, authenticator)); - self.accounts - .insert(LeafIndex::from(account.id()), Word::from(account.commitment())); + self.account_tree.insert(account.id(), account.commitment()).unwrap(); account } @@ -826,8 +822,9 @@ impl MockChain { for current_block_num in next_block_num..=target_block_num { for update in self.pending_objects.updated_accounts.iter() { - self.accounts - .insert(update.account_id().into(), *update.final_state_commitment()); + self.account_tree + .insert(update.account_id(), update.final_state_commitment()) + .unwrap(); if let Some(mock_account) = self.available_accounts.get(&update.account_id()) { let account = match update.details() { @@ -856,7 +853,7 @@ impl MockChain { let previous = self.blocks.last(); let peaks = self.chain.peaks(); let chain_commitment: Digest = peaks.hash_peaks(); - let account_root = self.accounts.root(); + let account_root = self.account_tree.root(); let prev_block_commitment = previous.map_or(Digest::default(), |block| block.commitment()); let nullifier_root = self.nullifiers.root(); @@ -879,7 +876,7 @@ impl MockChain { } } - let tx_commitment = BlockHeader::compute_tx_commitment( + let tx_commitment = OrderedTransactionHeaders::compute_commitment( self.pending_objects.included_transactions.clone().into_iter(), ); @@ -1011,8 +1008,8 @@ impl MockChain { let mut account_witnesses = BTreeMap::new(); for account_id in account_ids { - let proof = self.accounts.open(&account_id.into()); - account_witnesses.insert(account_id, AccountWitness::new(proof.value, proof.path)); + let witness = self.account_tree.open(account_id); + account_witnesses.insert(account_id, witness); } account_witnesses @@ -1090,9 +1087,9 @@ impl MockChain { .account() } - /// Get the reference to the accounts hash tree. - pub fn accounts(&self) -> &SimpleSmt { - &self.accounts + /// Get the reference to the account tree. + pub fn accounts(&self) -> &AccountTree { + &self.account_tree } } diff --git a/crates/miden-tx/src/tests/kernel_tests/test_fpi.rs b/crates/miden-tx/src/tests/kernel_tests/test_fpi.rs index 6427468b44..a370201e14 100644 --- a/crates/miden-tx/src/tests/kernel_tests/test_fpi.rs +++ b/crates/miden-tx/src/tests/kernel_tests/test_fpi.rs @@ -1,9 +1,9 @@ -use alloc::vec::Vec; -use std::{string::ToString, vec}; +use alloc::{string::ToString, vec, vec::Vec}; use miden_lib::{ errors::tx_kernel_errors::{ - ERR_FOREIGN_ACCOUNT_CONTEXT_AGAINST_NATIVE_ACCOUNT, ERR_FOREIGN_ACCOUNT_MAX_NUMBER_EXCEEDED, + ERR_FOREIGN_ACCOUNT_CONTEXT_AGAINST_NATIVE_ACCOUNT, ERR_FOREIGN_ACCOUNT_INVALID_COMMITMENT, + ERR_FOREIGN_ACCOUNT_MAX_NUMBER_EXCEEDED, }, transaction::{ TransactionKernel, @@ -16,18 +16,17 @@ use miden_lib::{ }, }; use miden_objects::{ - ACCOUNT_TREE_DEPTH, + FieldElement, account::{ Account, AccountBuilder, AccountComponent, AccountProcedureInfo, AccountStorage, StorageSlot, }, - crypto::merkle::{LeafIndex, MerklePath}, testing::{account_component::AccountMockComponent, storage::STORAGE_LEAVES_2}, transaction::TransactionScript, }; use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha20Rng; -use vm_processor::AdviceInputs; +use vm_processor::{AdviceInputs, Felt}; use super::{Process, Word, ZERO}; use crate::{ @@ -1173,6 +1172,107 @@ fn test_nested_fpi_native_account_invocation() { assert_execution_error!(Err::<(), _>(err), ERR_FOREIGN_ACCOUNT_CONTEXT_AGAINST_NATIVE_ACCOUNT); } +/// Test that providing an account whose commitment does not match the one in the account tree +/// results in an error. +#[test] +fn test_fpi_stale_account() { + // Prepare the test data + let foreign_account_code_source = " + use.miden::account + + # code is not used in this test + export.set_some_item_foreign + push.34.1 + exec.account::set_item + end + "; + + let foreign_account_component = AccountComponent::compile( + foreign_account_code_source, + TransactionKernel::testing_assembler(), + vec![AccountStorage::mock_item_0().slot], + ) + .unwrap() + .with_supports_all_types(); + + let mut foreign_account = AccountBuilder::new([5; 32]) + .with_component(foreign_account_component) + .build_existing() + .unwrap(); + + let native_account = AccountBuilder::new([4; 32]) + .with_component( + AccountMockComponent::new_with_slots( + TransactionKernel::testing_assembler(), + vec![AccountStorage::mock_item_2().slot], + ) + .unwrap(), + ) + .build_existing() + .unwrap(); + + let mut mock_chain = + MockChain::with_accounts(&[native_account.clone(), foreign_account.clone()]); + mock_chain.seal_next_block(); + + // Make the foreign account invalid. + // -------------------------------------------------------------------------------------------- + + // Modify the account's storage to change its storage commitment and in turn the account + // commitment. + foreign_account + .storage_mut() + .set_item(0, Word::from([Felt::ONE, Felt::ONE, Felt::ONE, Felt::ONE])) + .unwrap(); + + // Place the modified account in the advice provider, which will cause the commitment mismatch. + let advice_inputs = get_mock_fpi_adv_inputs(vec![&foreign_account], &mock_chain); + + // The account tree from which the transaction inputs are fetched here has the state from the + // original unmodified foreign account. This should result in the foreign account's proof to be + // invalid for this account tree root. + let tx_context = mock_chain + .build_tx_context(native_account.id(), &[], &[]) + .foreign_account_codes(vec![foreign_account.code().clone()]) + .advice_inputs(advice_inputs.clone()) + .build(); + + // Attempt to run FPI. + // -------------------------------------------------------------------------------------------- + + let code = format!( + " + use.std::sys + + use.kernel::prologue + use.miden::tx + + begin + exec.prologue::prepare_transaction + + # pad the stack for the `execute_foreign_procedure` execution + padw padw padw padw + # => [pad(16)] + + # push some hash onto the stack - for this test it does not matter + push.0.0.0.0 + # => [FOREIGN_PROC_ROOT, pad(16)] + + # push the foreign account ID + push.{foreign_suffix}.{foreign_prefix} + # => [foreign_account_id_prefix, foreign_account_id_suffix, FOREIGN_PROC_ROOT, pad(16)] + + exec.tx::execute_foreign_procedure + end + ", + foreign_prefix = foreign_account.id().prefix().as_felt(), + foreign_suffix = foreign_account.id().suffix(), + ); + + let result = tx_context.execute_code(&code).map(|_| ()); + assert_execution_error!(result, ERR_FOREIGN_ACCOUNT_INVALID_COMMITMENT); +} + // HELPER FUNCTIONS // ================================================================================================ @@ -1191,14 +1291,7 @@ fn get_mock_fpi_adv_inputs( // Provide the merkle path of the foreign account to be able to verify that the account // tree has the commitment of this foreign account. Verification is done during the // execution of the `kernel::account::validate_current_foreign_account` procedure. - &MerklePath::new( - mock_chain - .accounts() - // TODO: Update. - .open(&LeafIndex::::new(foreign_account.id().prefix().as_felt().as_int()).unwrap()) - .path - .into(), - ), + &mock_chain.accounts().open(foreign_account.id()), ) .unwrap(); diff --git a/docs/src/state.md b/docs/src/state.md index 55f196ed6e..ca899e9242 100644 --- a/docs/src/state.md +++ b/docs/src/state.md @@ -26,7 +26,7 @@ The Miden node maintains three databases to describe `State`: 3. Nullifiers

- State + State

### Account database