From e8c8d30e0aa34f8c3ba30962ca333bf86355efde Mon Sep 17 00:00:00 2001 From: igamigo Date: Fri, 24 Jan 2025 14:23:04 -0300 Subject: [PATCH 01/22] fix: Set supported types correctly (#1103) * fix: Set supported types correctly * test: Add test for supported types --- crates/miden-objects/src/account/component/mod.rs | 3 ++- .../src/account/component/template/storage/mod.rs | 9 ++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/miden-objects/src/account/component/mod.rs b/crates/miden-objects/src/account/component/mod.rs index de273dbe0..dde17e027 100644 --- a/crates/miden-objects/src/account/component/mod.rs +++ b/crates/miden-objects/src/account/component/mod.rs @@ -113,7 +113,8 @@ impl AccountComponent { storage_slots.extend(entry_storage_slots); } - AccountComponent::new(template.library().clone(), storage_slots) + Ok(AccountComponent::new(template.library().clone(), storage_slots)? + .with_supported_types(template.metadata().targets().clone())) } // ACCESSORS diff --git a/crates/miden-objects/src/account/component/template/storage/mod.rs b/crates/miden-objects/src/account/component/template/storage/mod.rs index 00a7087b7..adda76fa8 100644 --- a/crates/miden-objects/src/account/component/template/storage/mod.rs +++ b/crates/miden-objects/src/account/component/template/storage/mod.rs @@ -462,7 +462,7 @@ mod tests { name = "Test Component" description = "This is a test component" version = "1.0.1" - targets = ["FungibleFaucet"] + targets = ["FungibleFaucet", "RegularAccountImmutableCode"] [[storage]] name = "map" @@ -536,6 +536,13 @@ mod tests { let component = AccountComponent::from_template(&template, &storage_placeholders).unwrap(); + assert_eq!( + component.supported_types(), + &[AccountType::FungibleFaucet, AccountType::RegularAccountImmutableCode] + .into_iter() + .collect() + ); + let storage_map = component.storage_slots.first().unwrap(); match storage_map { StorageSlot::Map(storage_map) => assert_eq!(storage_map.entries().count(), 3), From 779b9ac542123fb40a589e1f39e52f38fba4e35e Mon Sep 17 00:00:00 2001 From: igamigo Date: Fri, 24 Jan 2025 17:37:38 -0300 Subject: [PATCH 02/22] docs: missing types (#1100) --- .../miden-objects/src/account/account_id/account_type.rs | 1 + .../miden-objects/src/account/account_id/storage_mode.rs | 3 +++ crates/miden-objects/src/account/delta/mod.rs | 2 ++ crates/miden-objects/src/account/storage/slot/mod.rs | 7 ++++++- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/miden-objects/src/account/account_id/account_type.rs b/crates/miden-objects/src/account/account_id/account_type.rs index 49a442a54..940a604b9 100644 --- a/crates/miden-objects/src/account/account_id/account_type.rs +++ b/crates/miden-objects/src/account/account_id/account_type.rs @@ -13,6 +13,7 @@ pub(super) const NON_FUNGIBLE_FAUCET: u8 = 0b11; pub(super) const REGULAR_ACCOUNT_IMMUTABLE_CODE: u8 = 0b00; pub(super) const REGULAR_ACCOUNT_UPDATABLE_CODE: u8 = 0b01; +/// Represents the different account types recognized by the protocol. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] #[repr(u8)] pub enum AccountType { diff --git a/crates/miden-objects/src/account/account_id/storage_mode.rs b/crates/miden-objects/src/account/account_id/storage_mode.rs index 199fd585e..051b936c2 100644 --- a/crates/miden-objects/src/account/account_id/storage_mode.rs +++ b/crates/miden-objects/src/account/account_id/storage_mode.rs @@ -9,10 +9,13 @@ use crate::errors::AccountIdError; pub(super) const PUBLIC: u8 = 0b00; pub(super) const PRIVATE: u8 = 0b10; +/// Describes where the state of the account is stored. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[repr(u8)] pub enum AccountStorageMode { + /// The account's full state is stored on-chain. Public = PUBLIC, + /// The account's state is stored off-chain, and only a commitment to it is stored on-chain. Private = PRIVATE, } diff --git a/crates/miden-objects/src/account/delta/mod.rs b/crates/miden-objects/src/account/delta/mod.rs index 7bff73ea4..8211e025d 100644 --- a/crates/miden-objects/src/account/delta/mod.rs +++ b/crates/miden-objects/src/account/delta/mod.rs @@ -96,6 +96,8 @@ impl AccountDelta { } } +/// Describes the details of an account state transition resulting from applying a transaction to +/// the account. #[derive(Clone, Debug, PartialEq, Eq)] pub enum AccountUpdateDetails { /// Account is private (no on-chain state change). diff --git a/crates/miden-objects/src/account/storage/slot/mod.rs b/crates/miden-objects/src/account/storage/slot/mod.rs index 4275dbbdd..058ff2415 100644 --- a/crates/miden-objects/src/account/storage/slot/mod.rs +++ b/crates/miden-objects/src/account/storage/slot/mod.rs @@ -12,7 +12,12 @@ pub use r#type::StorageSlotType; // STORAGE SLOT // ================================================================================================ -/// An object that represents the type of a storage slot. +/// An object representing the contents of an account's storage slot. +/// +/// An account storage slot can be of two types: +/// - A simple value which contains a single word (4 field elements or ~32 bytes). +/// - A key value map where both keys and values are words. The capacity of such storage slot is +/// theoretically unlimited. #[derive(Debug, Clone, PartialEq, Eq)] pub enum StorageSlot { Value(Word), From 99fa9d51a4add5176046b6f46381d0182f7f4588 Mon Sep 17 00:00:00 2001 From: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Date: Fri, 24 Jan 2025 12:38:02 -0800 Subject: [PATCH 03/22] chore: increment miden-objects version to v0.7.1 and update changelog (#1106) --- CHANGELOG.md | 7 +++++++ Cargo.lock | 14 +++++++------- crates/miden-objects/Cargo.toml | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68e23fc97..205918360 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 0.7.1 (2025-01-24) - `miden-objects` crate only + +### Fixes + +- Added missing doc comments (#1100). +- Fixed setting of supporting types when instantiating `AccountComponent` from templates (#1103). + ## 0.7.0 (2025-01-22) ### Highlights diff --git a/Cargo.lock b/Cargo.lock index ada6d3c3d..accdaef69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1935,7 +1935,7 @@ dependencies = [ [[package]] name = "miden-objects" -version = "0.7.0" +version = "0.7.1" dependencies = [ "anyhow", "assert_matches", @@ -2352,9 +2352,9 @@ dependencies = [ [[package]] name = "openssl-probe" -version = "0.1.5" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf" +checksum = "d05e27ee213611ffe7d6348b942e8f942b37114c00cc03cec254295a4a17852e" [[package]] name = "openssl-sys" @@ -4462,9 +4462,9 @@ checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" [[package]] name = "trybuild" -version = "1.0.102" +version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f14b5c02a137632f68194ec657ecb92304138948e8957c932127eb1b58c23be" +checksum = "b812699e0c4f813b872b373a4471717d9eb550da14b311058a4d9cf4173cbca6" dependencies = [ "dissimilar", "glob", @@ -4499,9 +4499,9 @@ checksum = "75b844d17643ee918803943289730bec8aac480150456169e647ed0b576ba539" [[package]] name = "unicode-ident" -version = "1.0.14" +version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "adb9e6ca4f869e1180728b7950e35922a7fc6397f7b641499e8f3ef06e50dc83" +checksum = "11cd88e12b17c6494200a9c1b683a04fcac9573ed74cd1b62aeb2727c5592243" [[package]] name = "unicode-linebreak" diff --git a/crates/miden-objects/Cargo.toml b/crates/miden-objects/Cargo.toml index e7ed161a3..21a075244 100644 --- a/crates/miden-objects/Cargo.toml +++ b/crates/miden-objects/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "miden-objects" -version = "0.7.0" +version = "0.7.1" description = "Core components of the Miden rollup" readme = "README.md" categories = ["no-std"] From 6058a36357e5b4ed583ce9f8d43b7184dd658b9f Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Thu, 23 Jan 2025 11:38:41 +0100 Subject: [PATCH 04/22] feat: Add miden-batch-prover crate --- Cargo.lock | 13 +++++++ Cargo.toml | 1 + crates/miden-batch-prover/Cargo.toml | 36 +++++++++++++++++++ crates/miden-batch-prover/README.md | 7 ++++ crates/miden-batch-prover/src/lib.rs | 9 +++++ crates/miden-batch-prover/src/testing/mod.rs | 0 .../tests/integration/main.rs | 1 + 7 files changed, 67 insertions(+) create mode 100644 crates/miden-batch-prover/Cargo.toml create mode 100644 crates/miden-batch-prover/README.md create mode 100644 crates/miden-batch-prover/src/lib.rs create mode 100644 crates/miden-batch-prover/src/testing/mod.rs create mode 100644 crates/miden-batch-prover/tests/integration/main.rs diff --git a/Cargo.lock b/Cargo.lock index 625e70ab0..08a1aac6d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1816,6 +1816,19 @@ dependencies = [ "unicode-width 0.2.0", ] +[[package]] +name = "miden-batch-prover" +version = "0.8.0" +dependencies = [ + "miden-assembly", + "miden-core", + "miden-crypto", + "miden-processor", + "miden-verifier", + "rand", + "thiserror 2.0.11", +] + [[package]] name = "miden-bench-tx" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 6faffb034..a2dcb78a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,7 @@ resolver = "2" members = [ "bin/bench-tx", "bin/proving-service", + "crates/miden-batch-prover", "crates/miden-lib", "crates/miden-objects", "crates/miden-proving-service-client", diff --git a/crates/miden-batch-prover/Cargo.toml b/crates/miden-batch-prover/Cargo.toml new file mode 100644 index 000000000..623cc8156 --- /dev/null +++ b/crates/miden-batch-prover/Cargo.toml @@ -0,0 +1,36 @@ +[package] +name = "miden-batch-prover" +version = "0.8.0" +description = "Miden rollup batch executor and prover" +readme = "README.md" +categories = ["no-std"] +keywords = ["miden", "batch", "prover"] +license.workspace = true +authors.workspace = true +homepage.workspace = true +repository.workspace = true +rust-version.workspace = true +edition.workspace = true + +[[test]] +name = "miden-batch-prover" +path = "tests/integration/main.rs" + +[lib] +bench = false + +[features] +default = ["std"] +std = ["assembly/std", "miden-crypto/std", "miden-verifier/std", "vm-core/std", "vm-processor/std"] +testing = ["dep:rand"] + +[dependencies] +assembly = { workspace = true } +miden-crypto = { workspace = true } +miden-verifier = { workspace = true } +rand = { workspace = true, optional = true } +thiserror = { workspace = true } +vm-core = { workspace = true } +vm-processor = { workspace = true } + +[dev-dependencies] diff --git a/crates/miden-batch-prover/README.md b/crates/miden-batch-prover/README.md new file mode 100644 index 000000000..0e6d3627e --- /dev/null +++ b/crates/miden-batch-prover/README.md @@ -0,0 +1,7 @@ +# Miden Batch Prover + +This crate contains tools for executing and proving Miden rollup transaction batches. + +## License + +This project is [MIT licensed](../LICENSE). diff --git a/crates/miden-batch-prover/src/lib.rs b/crates/miden-batch-prover/src/lib.rs new file mode 100644 index 000000000..bb0119909 --- /dev/null +++ b/crates/miden-batch-prover/src/lib.rs @@ -0,0 +1,9 @@ +#![no_std] + +extern crate alloc; + +#[cfg(feature = "std")] +extern crate std; + +#[cfg(any(feature = "testing", test))] +pub mod testing; diff --git a/crates/miden-batch-prover/src/testing/mod.rs b/crates/miden-batch-prover/src/testing/mod.rs new file mode 100644 index 000000000..e69de29bb diff --git a/crates/miden-batch-prover/tests/integration/main.rs b/crates/miden-batch-prover/tests/integration/main.rs new file mode 100644 index 000000000..92719ae5d --- /dev/null +++ b/crates/miden-batch-prover/tests/integration/main.rs @@ -0,0 +1 @@ +pub fn _placeholder() {} From 2035767294168dd38c1bc90d2194b2ccc4517dff Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 24 Jan 2025 10:22:37 +0100 Subject: [PATCH 05/22] feat: Add `BatchId` --- crates/miden-objects/src/batch/batch_id.rs | 38 ++++++++++++++++++++++ crates/miden-objects/src/batch/mod.rs | 3 ++ 2 files changed, 41 insertions(+) create mode 100644 crates/miden-objects/src/batch/batch_id.rs diff --git a/crates/miden-objects/src/batch/batch_id.rs b/crates/miden-objects/src/batch/batch_id.rs new file mode 100644 index 000000000..7efc1d0b5 --- /dev/null +++ b/crates/miden-objects/src/batch/batch_id.rs @@ -0,0 +1,38 @@ +use alloc::vec::Vec; +use core::borrow::Borrow; + +use miden_crypto::hash::Digest; +use vm_core::crypto::hash::Blake3Digest; +use vm_processor::crypto::Blake3_256; + +use crate::transaction::TransactionId; + +// BATCH ID +// ================================================================================================ + +/// Uniquely identifies a [`TransactionBatch`]. +// TODO: Should this really be a Blake3 hash? We have to compute this in the block kernel +// eventually, so we'd probably want RPO instead? +// TODO: Compute batch ID as hash over tx ID _and_ account ID. +#[derive(Debug, Copy, Clone, Eq, Ord, PartialEq, PartialOrd)] +pub struct BatchId(Blake3Digest<32>); + +impl BatchId { + /// Calculates a batch ID from the given set of transactions. + pub fn compute(txs: impl Iterator) -> Self + where + T: Borrow, + { + let mut buf = Vec::with_capacity(32 * txs.size_hint().0); + for tx in txs { + buf.extend_from_slice(&tx.borrow().as_bytes()); + } + Self(Blake3_256::hash(&buf)) + } +} + +impl std::fmt::Display for BatchId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&miden_crypto::utils::bytes_to_hex_string(self.0.as_bytes())) + } +} diff --git a/crates/miden-objects/src/batch/mod.rs b/crates/miden-objects/src/batch/mod.rs index 23c43becc..568a95b08 100644 --- a/crates/miden-objects/src/batch/mod.rs +++ b/crates/miden-objects/src/batch/mod.rs @@ -1,2 +1,5 @@ mod note_tree; pub use note_tree::BatchNoteTree; + +mod batch_id; +pub use batch_id::BatchId; From e7f84d83a4daa29278daff041c99c174bce1c534 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 24 Jan 2025 13:05:20 +0100 Subject: [PATCH 06/22] Introduce `AccountUpdateError` --- crates/miden-batch-prover/src/testing/mod.rs | 1 + crates/miden-objects/src/errors.rs | 18 ++++++++++++++++++ crates/miden-objects/src/lib.rs | 6 +++--- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/miden-batch-prover/src/testing/mod.rs b/crates/miden-batch-prover/src/testing/mod.rs index e69de29bb..8b1378917 100644 --- a/crates/miden-batch-prover/src/testing/mod.rs +++ b/crates/miden-batch-prover/src/testing/mod.rs @@ -0,0 +1 @@ + diff --git a/crates/miden-objects/src/errors.rs b/crates/miden-objects/src/errors.rs index 38da1fa1d..890eb9c08 100644 --- a/crates/miden-objects/src/errors.rs +++ b/crates/miden-objects/src/errors.rs @@ -22,6 +22,7 @@ use crate::{ }, block::BlockNumber, note::{NoteAssets, NoteExecutionHint, NoteTag, NoteType, Nullifier}, + transaction::TransactionId, ACCOUNT_UPDATE_MAX_SIZE, MAX_INPUTS_PER_NOTE, MAX_INPUT_NOTES_PER_TX, MAX_OUTPUT_NOTES_PER_TX, }; @@ -182,6 +183,23 @@ pub enum AccountDeltaError { NotAFungibleFaucetId(AccountId), } +// ACCOUNT UPDATE ERROR +// ================================================================================================ + +#[derive(Debug, Error)] +pub enum AccountUpdateError { + #[error("account update for account {expected_account_id} cannot be merged with update from transaction {transaction} which was executed against account {actual_account_id}")] + AccountUpdateIdMismatch { + transaction: TransactionId, + expected_account_id: AccountId, + actual_account_id: AccountId, + }, + #[error("final state commitment in account update from transaction {0} does not match initial state of current update")] + AccountUpdateInitialStateMismatch(TransactionId), + #[error("failed to merge account delta from transaction {0}")] + TransactionUpdateMergeError(TransactionId, #[source] AccountDeltaError), +} + // ASSET ERROR // ================================================================================================ diff --git a/crates/miden-objects/src/lib.rs b/crates/miden-objects/src/lib.rs index 069a43230..7eb5534aa 100644 --- a/crates/miden-objects/src/lib.rs +++ b/crates/miden-objects/src/lib.rs @@ -24,9 +24,9 @@ mod errors; pub use constants::*; pub use errors::{ - AccountDeltaError, AccountError, AccountIdError, AssetError, AssetVaultError, BlockError, - ChainMmrError, NoteError, ProvenTransactionError, TransactionInputError, - TransactionOutputError, TransactionScriptError, + AccountDeltaError, AccountError, AccountIdError, AccountUpdateError, AssetError, + AssetVaultError, BlockError, ChainMmrError, NoteError, ProvenTransactionError, + TransactionInputError, TransactionOutputError, TransactionScriptError, }; pub use miden_crypto::hash::rpo::{Rpo256 as Hasher, RpoDigest as Digest}; pub use vm_core::{Felt, FieldElement, StarkField, Word, EMPTY_WORD, ONE, WORD_SIZE, ZERO}; From e32b13482573360f862857d8ed3a9e3ca7a7e15b Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 24 Jan 2025 13:20:42 +0100 Subject: [PATCH 07/22] feat: Add `ProposedBatch` --- Cargo.lock | 1 + crates/miden-batch-prover/Cargo.toml | 1 + crates/miden-batch-prover/src/lib.rs | 3 +++ crates/miden-batch-prover/src/proposed_batch.rs | 15 +++++++++++++++ 4 files changed, 20 insertions(+) create mode 100644 crates/miden-batch-prover/src/proposed_batch.rs diff --git a/Cargo.lock b/Cargo.lock index 08a1aac6d..b54d6958d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1823,6 +1823,7 @@ dependencies = [ "miden-assembly", "miden-core", "miden-crypto", + "miden-objects", "miden-processor", "miden-verifier", "rand", diff --git a/crates/miden-batch-prover/Cargo.toml b/crates/miden-batch-prover/Cargo.toml index 623cc8156..63cf51293 100644 --- a/crates/miden-batch-prover/Cargo.toml +++ b/crates/miden-batch-prover/Cargo.toml @@ -26,6 +26,7 @@ testing = ["dep:rand"] [dependencies] assembly = { workspace = true } +miden-objects = { workspace = true } miden-crypto = { workspace = true } miden-verifier = { workspace = true } rand = { workspace = true, optional = true } diff --git a/crates/miden-batch-prover/src/lib.rs b/crates/miden-batch-prover/src/lib.rs index bb0119909..248f06422 100644 --- a/crates/miden-batch-prover/src/lib.rs +++ b/crates/miden-batch-prover/src/lib.rs @@ -5,5 +5,8 @@ extern crate alloc; #[cfg(feature = "std")] extern crate std; +mod proposed_batch; +pub use proposed_batch::ProposedBatch; + #[cfg(any(feature = "testing", test))] pub mod testing; diff --git a/crates/miden-batch-prover/src/proposed_batch.rs b/crates/miden-batch-prover/src/proposed_batch.rs new file mode 100644 index 000000000..c66e56665 --- /dev/null +++ b/crates/miden-batch-prover/src/proposed_batch.rs @@ -0,0 +1,15 @@ +use alloc::{sync::Arc, vec::Vec}; + +use miden_objects::transaction::ProvenTransaction; + +// TODO: Document. +#[derive(Debug, Clone)] +pub struct ProposedBatch { + transactions: Vec>, +} + +impl ProposedBatch { + pub fn new(transactions: Vec>) -> Self { + Self { transactions } + } +} From 3a7f03fa65dff764c699a116849d725c5e1fcf08 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 24 Jan 2025 13:24:38 +0100 Subject: [PATCH 08/22] feat: Add `LocalBatchProver` --- crates/miden-batch-prover/src/lib.rs | 3 +++ crates/miden-batch-prover/src/local_batch_prover.rs | 9 +++++++++ 2 files changed, 12 insertions(+) create mode 100644 crates/miden-batch-prover/src/local_batch_prover.rs diff --git a/crates/miden-batch-prover/src/lib.rs b/crates/miden-batch-prover/src/lib.rs index 248f06422..dae26e138 100644 --- a/crates/miden-batch-prover/src/lib.rs +++ b/crates/miden-batch-prover/src/lib.rs @@ -8,5 +8,8 @@ extern crate std; mod proposed_batch; pub use proposed_batch::ProposedBatch; +mod local_batch_prover; +pub use local_batch_prover::LocalBatchProver; + #[cfg(any(feature = "testing", test))] pub mod testing; diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs new file mode 100644 index 000000000..f0eca6b40 --- /dev/null +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -0,0 +1,9 @@ +use crate::ProposedBatch; + +pub struct LocalBatchProver {} + +impl LocalBatchProver { + pub fn prove(proposed_batch: ProposedBatch) -> Result<(), ()> { + todo!() + } +} From 179dd656731bb43cdfb894c42e837f9b1fdef9b6 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 24 Jan 2025 13:52:59 +0100 Subject: [PATCH 09/22] feat: Add `ProvenBatch` --- crates/miden-batch-prover/src/lib.rs | 3 + crates/miden-batch-prover/src/proven_batch.rs | 75 +++++++++++++++++++ crates/miden-objects/src/batch/batch_id.rs | 1 + 3 files changed, 79 insertions(+) create mode 100644 crates/miden-batch-prover/src/proven_batch.rs diff --git a/crates/miden-batch-prover/src/lib.rs b/crates/miden-batch-prover/src/lib.rs index dae26e138..f2a56fdd9 100644 --- a/crates/miden-batch-prover/src/lib.rs +++ b/crates/miden-batch-prover/src/lib.rs @@ -5,6 +5,9 @@ extern crate alloc; #[cfg(feature = "std")] extern crate std; +mod proven_batch; +pub use proven_batch::ProvenBatch; + mod proposed_batch; pub use proposed_batch::ProposedBatch; diff --git a/crates/miden-batch-prover/src/proven_batch.rs b/crates/miden-batch-prover/src/proven_batch.rs new file mode 100644 index 000000000..b43d5a719 --- /dev/null +++ b/crates/miden-batch-prover/src/proven_batch.rs @@ -0,0 +1,75 @@ +use alloc::vec::Vec; + +use miden_objects::{ + account::AccountUpdate, + batch::{BatchId, BatchNoteTree}, + transaction::{InputNoteCommitment, OutputNotes}, +}; + +// TODO: Document. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ProvenBatch { + id: BatchId, + account_updates: Vec, + input_notes: Vec, + output_notes_smt: BatchNoteTree, + output_notes: OutputNotes, +} + +impl ProvenBatch { + // CONSTRUCTORS + // -------------------------------------------------------------------------------------------- + + /// Creates a new [`ProvenBatch`] from the provided parts. + pub fn new( + id: BatchId, + account_updates: Vec, + input_notes: Vec, + output_notes_smt: BatchNoteTree, + output_notes: OutputNotes, + ) -> Self { + Self { + id, + account_updates, + input_notes, + output_notes_smt, + output_notes, + } + } + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// The ID of this batch. See [`BatchId`] for details on how it is computed. + pub fn id(&self) -> BatchId { + self.id + } + + /// Returns a slice of [`AccountUpdate`]s - exactly one for each account updated in the batch. + /// + /// If an account was updated by multiple transactions, the returned [`AccountUpdate`] is the + /// result of merging the individual updates. + /// + /// For example, suppose an account's state before this batch is `A` and the batch contains two + /// transactions that updated it. Applying the first transaction results in intermediate state + /// `B`, and applying the second one results in state `C`. Then the returned update represents + /// the state transition from `A` to `C`. + pub fn account_updates(&self) -> &[AccountUpdate] { + &self.account_updates + } + + /// Returns the output notes of the batch. + /// + /// This is the aggregation of all output notes by the contained transactions, except the ones + /// that were consumed within the batch itself. + pub fn output_notes(&self) -> &OutputNotes { + &self.output_notes + } + + /// Returns the [`BatchNoteTree`] representing the output notes of the batch. + /// + /// TODO: More docs? + pub fn output_notes_tree(&self) -> &BatchNoteTree { + &self.output_notes_smt + } +} diff --git a/crates/miden-objects/src/batch/batch_id.rs b/crates/miden-objects/src/batch/batch_id.rs index 7efc1d0b5..c2c471199 100644 --- a/crates/miden-objects/src/batch/batch_id.rs +++ b/crates/miden-objects/src/batch/batch_id.rs @@ -11,6 +11,7 @@ use crate::transaction::TransactionId; // ================================================================================================ /// Uniquely identifies a [`TransactionBatch`]. +// TODO: Document how this is computed. // TODO: Should this really be a Blake3 hash? We have to compute this in the block kernel // eventually, so we'd probably want RPO instead? // TODO: Compute batch ID as hash over tx ID _and_ account ID. From 2119ed1db01928a67139644644eb27de57dcac34 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 24 Jan 2025 17:29:36 +0100 Subject: [PATCH 10/22] feat: Migrate `LocalBatchProver` from node --- crates/miden-batch-prover/src/error.rs | 26 ++++ crates/miden-batch-prover/src/lib.rs | 4 + .../src/local_batch_prover.rs | 142 +++++++++++++++++- .../miden-batch-prover/src/proposed_batch.rs | 23 ++- crates/miden-batch-prover/src/proven_batch.rs | 29 ++-- .../src/block/inclusion_proof.rs | 12 ++ crates/miden-objects/src/block/mod.rs | 5 + .../src/note/authentication_info.rs | 35 +++++ crates/miden-objects/src/note/mod.rs | 3 + 9 files changed, 259 insertions(+), 20 deletions(-) create mode 100644 crates/miden-batch-prover/src/error.rs create mode 100644 crates/miden-objects/src/block/inclusion_proof.rs create mode 100644 crates/miden-objects/src/note/authentication_info.rs diff --git a/crates/miden-batch-prover/src/error.rs b/crates/miden-batch-prover/src/error.rs new file mode 100644 index 000000000..c9bce676b --- /dev/null +++ b/crates/miden-batch-prover/src/error.rs @@ -0,0 +1,26 @@ +use miden_objects::{account::AccountId, note::NoteId, AccountUpdateError}; +use thiserror::Error; +use vm_processor::Digest; + +/// Error encountered while building a batch. +#[derive(Debug, Error)] +pub enum BatchError { + #[error("duplicated unauthenticated transaction input note ID in the batch: {0}")] + DuplicateUnauthenticatedNote(NoteId), + + #[error("duplicated transaction output note ID in the batch: {0}")] + DuplicateOutputNote(NoteId), + + #[error("note hashes mismatch for note {id}: (input: {input_hash}, output: {output_hash})")] + NoteHashesMismatch { + id: NoteId, + input_hash: Digest, + output_hash: Digest, + }, + + #[error("failed to merge transaction delta into account {account_id}")] + AccountUpdateError { + account_id: AccountId, + source: AccountUpdateError, + }, +} diff --git a/crates/miden-batch-prover/src/lib.rs b/crates/miden-batch-prover/src/lib.rs index f2a56fdd9..77ee92c3f 100644 --- a/crates/miden-batch-prover/src/lib.rs +++ b/crates/miden-batch-prover/src/lib.rs @@ -1,5 +1,6 @@ #![no_std] +#[macro_use] extern crate alloc; #[cfg(feature = "std")] @@ -11,6 +12,9 @@ pub use proven_batch::ProvenBatch; mod proposed_batch; pub use proposed_batch::ProposedBatch; +mod error; +pub use error::BatchError; + mod local_batch_prover; pub use local_batch_prover::LocalBatchProver; diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index f0eca6b40..a64b14e3a 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -1,9 +1,145 @@ -use crate::ProposedBatch; +use alloc::{ + collections::{btree_map::Entry, BTreeMap, BTreeSet}, + vec::Vec, +}; + +use miden_objects::{ + account::{AccountId, AccountUpdate}, + batch::{BatchId, BatchNoteTree}, + note::{NoteHeader, NoteId}, + transaction::{InputNoteCommitment, OutputNote, ProvenTransaction}, +}; + +use crate::{BatchError, ProposedBatch, ProvenBatch}; pub struct LocalBatchProver {} impl LocalBatchProver { - pub fn prove(proposed_batch: ProposedBatch) -> Result<(), ()> { - todo!() + pub fn prove(proposed_batch: ProposedBatch) -> Result { + let transactions = proposed_batch.transactions(); + let id = BatchId::compute(transactions.iter().map(|tx| tx.id())); + + // Populate batch output notes and updated accounts. + let mut output_notes = OutputNoteTracker::new(transactions.iter().map(|tx| tx.as_ref()))?; + let mut updated_accounts = BTreeMap::::new(); + let mut unauthenticated_input_notes = BTreeSet::new(); + for tx in transactions { + // Merge account updates so that state transitions A->B->C become A->C. + match updated_accounts.entry(tx.account_id()) { + Entry::Vacant(vacant) => { + vacant.insert(tx.account_update().clone()); + }, + Entry::Occupied(occupied) => { + occupied.into_mut().merge_proven_tx(tx).map_err(|source| { + BatchError::AccountUpdateError { account_id: tx.account_id(), source } + })?; + }, + }; + + // Check unauthenticated input notes for duplicates: + for note in tx.get_unauthenticated_notes() { + let id = note.id(); + if !unauthenticated_input_notes.insert(id) { + return Err(BatchError::DuplicateUnauthenticatedNote(id)); + } + } + } + + // Populate batch produced nullifiers and match output notes with corresponding + // unauthenticated input notes in the same batch, which are removed from the unauthenticated + // input notes set. + // + // One thing to note: + // This still allows transaction `A` to consume an unauthenticated note `x` and output note + // `y` and for transaction `B` to consume an unauthenticated note `y` and output + // note `x` (i.e., have a circular dependency between transactions), but this is not + // a problem. + let mut input_notes = vec![]; + for tx in transactions { + for input_note in tx.input_notes().iter() { + // Header is presented only for unauthenticated input notes. + let input_note = match input_note.header() { + Some(input_note_header) => { + if output_notes.remove_note(input_note_header)? { + continue; + } + + // If an unauthenticated note was found in the store, transform it to an + // authenticated one (i.e. erase additional note details + // except the nullifier) + if proposed_batch + .stored_unauthenticated_notes() + .contains_note(&input_note_header.id()) + { + InputNoteCommitment::from(input_note.nullifier()) + } else { + input_note.clone() + } + }, + None => input_note.clone(), + }; + input_notes.push(input_note); + } + } + + let output_notes = output_notes.into_notes(); + + // Build the output notes SMT. + let output_notes_smt = BatchNoteTree::with_contiguous_leaves( + output_notes.iter().map(|note| (note.id(), note.metadata())), + ) + .expect("Unreachable: fails only if the output note list contains duplicates"); + + Ok(ProvenBatch::new( + id, + updated_accounts, + input_notes, + output_notes_smt, + output_notes, + )) + } +} + +#[derive(Debug)] +struct OutputNoteTracker { + output_notes: Vec>, + output_note_index: BTreeMap, +} + +impl OutputNoteTracker { + fn new<'a>(txs: impl Iterator) -> Result { + let mut output_notes = vec![]; + let mut output_note_index = BTreeMap::new(); + for tx in txs { + for note in tx.output_notes().iter() { + if output_note_index.insert(note.id(), output_notes.len()).is_some() { + return Err(BatchError::DuplicateOutputNote(note.id())); + } + output_notes.push(Some(note.clone())); + } + } + + Ok(Self { output_notes, output_note_index }) + } + + pub fn remove_note(&mut self, input_note_header: &NoteHeader) -> Result { + let id = input_note_header.id(); + if let Some(note_index) = self.output_note_index.remove(&id) { + if let Some(output_note) = core::mem::take(&mut self.output_notes[note_index]) { + let input_hash = input_note_header.hash(); + let output_hash = output_note.hash(); + if output_hash != input_hash { + return Err(BatchError::NoteHashesMismatch { id, input_hash, output_hash }); + } + + return Ok(true); + } + } + + Ok(false) + } + + pub fn into_notes(self) -> Vec { + self.output_notes.into_iter().flatten().collect() } } diff --git a/crates/miden-batch-prover/src/proposed_batch.rs b/crates/miden-batch-prover/src/proposed_batch.rs index c66e56665..d26fc0630 100644 --- a/crates/miden-batch-prover/src/proposed_batch.rs +++ b/crates/miden-batch-prover/src/proposed_batch.rs @@ -1,15 +1,32 @@ use alloc::{sync::Arc, vec::Vec}; -use miden_objects::transaction::ProvenTransaction; +use miden_objects::{note::NoteAuthenticationInfo, transaction::ProvenTransaction}; // TODO: Document. #[derive(Debug, Clone)] pub struct ProposedBatch { transactions: Vec>, + /// This is used to transform unauthenticated notes into authenticated ones. Unauthenticated + /// notes can be consumed by transactions but the notes are in fact part of the chain. + stored_unauthenticated_notes: NoteAuthenticationInfo, } impl ProposedBatch { - pub fn new(transactions: Vec>) -> Self { - Self { transactions } + pub fn new( + transactions: Vec>, + stored_unauthenticated_notes: NoteAuthenticationInfo, + ) -> Self { + Self { + transactions, + stored_unauthenticated_notes, + } + } + + pub fn transactions(&self) -> &[Arc] { + &self.transactions + } + + pub fn stored_unauthenticated_notes(&self) -> &NoteAuthenticationInfo { + &self.stored_unauthenticated_notes } } diff --git a/crates/miden-batch-prover/src/proven_batch.rs b/crates/miden-batch-prover/src/proven_batch.rs index b43d5a719..b54f0dadb 100644 --- a/crates/miden-batch-prover/src/proven_batch.rs +++ b/crates/miden-batch-prover/src/proven_batch.rs @@ -1,19 +1,19 @@ -use alloc::vec::Vec; +use alloc::{collections::BTreeMap, vec::Vec}; use miden_objects::{ - account::AccountUpdate, + account::{AccountId, AccountUpdate}, batch::{BatchId, BatchNoteTree}, - transaction::{InputNoteCommitment, OutputNotes}, + transaction::{InputNoteCommitment, OutputNote}, }; // TODO: Document. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ProvenBatch { id: BatchId, - account_updates: Vec, + account_updates: BTreeMap, input_notes: Vec, output_notes_smt: BatchNoteTree, - output_notes: OutputNotes, + output_notes: Vec, } impl ProvenBatch { @@ -23,10 +23,10 @@ impl ProvenBatch { /// Creates a new [`ProvenBatch`] from the provided parts. pub fn new( id: BatchId, - account_updates: Vec, + account_updates: BTreeMap, input_notes: Vec, output_notes_smt: BatchNoteTree, - output_notes: OutputNotes, + output_notes: Vec, ) -> Self { Self { id, @@ -45,24 +45,25 @@ impl ProvenBatch { self.id } - /// Returns a slice of [`AccountUpdate`]s - exactly one for each account updated in the batch. + /// Returns the map of account IDs mapped to their [`AccountUpdate`]s. /// - /// If an account was updated by multiple transactions, the returned [`AccountUpdate`] is the - /// result of merging the individual updates. + /// If an account was updated by multiple transactions, the [`AccountUpdate`] is the result of + /// merging the individual updates. /// /// For example, suppose an account's state before this batch is `A` and the batch contains two /// transactions that updated it. Applying the first transaction results in intermediate state /// `B`, and applying the second one results in state `C`. Then the returned update represents /// the state transition from `A` to `C`. - pub fn account_updates(&self) -> &[AccountUpdate] { + // TODO: Check if we should return the map or an opaque iterator. + pub fn account_updates(&self) -> &BTreeMap { &self.account_updates } /// Returns the output notes of the batch. /// - /// This is the aggregation of all output notes by the contained transactions, except the ones - /// that were consumed within the batch itself. - pub fn output_notes(&self) -> &OutputNotes { + /// This is the aggregation of all output notes by the transactions in the batch, except the + /// ones that were consumed within the batch itself. + pub fn output_notes(&self) -> &[OutputNote] { &self.output_notes } diff --git a/crates/miden-objects/src/block/inclusion_proof.rs b/crates/miden-objects/src/block/inclusion_proof.rs new file mode 100644 index 000000000..58cd7d82b --- /dev/null +++ b/crates/miden-objects/src/block/inclusion_proof.rs @@ -0,0 +1,12 @@ +use miden_crypto::merkle::MerklePath; + +use crate::block::{BlockHeader, BlockNumber}; + +// TODO: Document. +/// Data required to verify a block's inclusion proof. +#[derive(Clone, Debug)] +pub struct BlockInclusionProof { + pub block_header: BlockHeader, + pub mmr_path: MerklePath, + pub chain_length: BlockNumber, +} diff --git a/crates/miden-objects/src/block/mod.rs b/crates/miden-objects/src/block/mod.rs index 500c24a0e..52eff7e22 100644 --- a/crates/miden-objects/src/block/mod.rs +++ b/crates/miden-objects/src/block/mod.rs @@ -7,11 +7,16 @@ use super::{ mod header; pub use header::BlockHeader; + mod block_number; pub use block_number::BlockNumber; + mod note_tree; pub use note_tree::{BlockNoteIndex, BlockNoteTree}; +mod inclusion_proof; +pub use inclusion_proof::BlockInclusionProof; + use crate::{ account::{delta::AccountUpdateDetails, AccountId}, errors::BlockError, diff --git a/crates/miden-objects/src/note/authentication_info.rs b/crates/miden-objects/src/note/authentication_info.rs new file mode 100644 index 000000000..0f7687497 --- /dev/null +++ b/crates/miden-objects/src/note/authentication_info.rs @@ -0,0 +1,35 @@ +use alloc::{ + collections::{BTreeMap, BTreeSet}, + vec::Vec, +}; + +use crate::{ + block::BlockInclusionProof, + note::{NoteId, NoteInclusionProof}, +}; + +// TODO: Document. +// TODO: Rename to `NoteInclusionProofs`? +#[derive(Clone, Default, Debug)] +pub struct NoteAuthenticationInfo { + block_proofs: Vec, + note_proofs: BTreeMap, +} + +impl NoteAuthenticationInfo { + pub fn block_proofs(&self) -> &[BlockInclusionProof] { + &self.block_proofs + } + + pub fn note_proofs(&self) -> impl Iterator { + self.note_proofs.iter() + } + + pub fn contains_note(&self, note: &NoteId) -> bool { + self.note_proofs.contains_key(note) + } + + pub fn note_ids(&self) -> BTreeSet { + self.note_proofs.keys().copied().collect() + } +} diff --git a/crates/miden-objects/src/note/mod.rs b/crates/miden-objects/src/note/mod.rs index c80452360..8a917aed3 100644 --- a/crates/miden-objects/src/note/mod.rs +++ b/crates/miden-objects/src/note/mod.rs @@ -53,6 +53,9 @@ pub use script::NoteScript; mod file; pub use file::NoteFile; +mod authentication_info; +pub use authentication_info::NoteAuthenticationInfo; + // NOTE // ================================================================================================ From a1ac7104e29fee0885c98e8aab4033e3f34b2b2e Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 27 Jan 2025 10:07:16 +0100 Subject: [PATCH 11/22] chore: Rename `NoteAuthenticationInfo` --- .../src/local_batch_prover.rs | 2 +- .../miden-batch-prover/src/proposed_batch.rs | 18 ++++++++++-------- .../src/note/authentication_info.rs | 5 ++--- crates/miden-objects/src/note/mod.rs | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index a64b14e3a..d3e56b91f 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -68,7 +68,7 @@ impl LocalBatchProver { // authenticated one (i.e. erase additional note details // except the nullifier) if proposed_batch - .stored_unauthenticated_notes() + .note_inclusion_proofs() .contains_note(&input_note_header.id()) { InputNoteCommitment::from(input_note.nullifier()) diff --git a/crates/miden-batch-prover/src/proposed_batch.rs b/crates/miden-batch-prover/src/proposed_batch.rs index d26fc0630..efc3f1d7b 100644 --- a/crates/miden-batch-prover/src/proposed_batch.rs +++ b/crates/miden-batch-prover/src/proposed_batch.rs @@ -1,24 +1,24 @@ use alloc::{sync::Arc, vec::Vec}; -use miden_objects::{note::NoteAuthenticationInfo, transaction::ProvenTransaction}; +use miden_objects::{note::NoteInclusionProofs, transaction::ProvenTransaction}; // TODO: Document. #[derive(Debug, Clone)] pub struct ProposedBatch { transactions: Vec>, - /// This is used to transform unauthenticated notes into authenticated ones. Unauthenticated - /// notes can be consumed by transactions but the notes are in fact part of the chain. - stored_unauthenticated_notes: NoteAuthenticationInfo, + /// The note inclusion proofs for unauthenticated notes that were consumed in the batch which + /// can be authenticated. + authenticatable_unauthenticated_notes: NoteInclusionProofs, } impl ProposedBatch { pub fn new( transactions: Vec>, - stored_unauthenticated_notes: NoteAuthenticationInfo, + authenticatable_unauthenticated_notes: NoteInclusionProofs, ) -> Self { Self { transactions, - stored_unauthenticated_notes, + authenticatable_unauthenticated_notes, } } @@ -26,7 +26,9 @@ impl ProposedBatch { &self.transactions } - pub fn stored_unauthenticated_notes(&self) -> &NoteAuthenticationInfo { - &self.stored_unauthenticated_notes + /// Returns the note inclusion proofs for unauthenticated notes that were consumed in the batch + /// which can be authenticated. + pub fn note_inclusion_proofs(&self) -> &NoteInclusionProofs { + &self.authenticatable_unauthenticated_notes } } diff --git a/crates/miden-objects/src/note/authentication_info.rs b/crates/miden-objects/src/note/authentication_info.rs index 0f7687497..6cd12d51b 100644 --- a/crates/miden-objects/src/note/authentication_info.rs +++ b/crates/miden-objects/src/note/authentication_info.rs @@ -9,14 +9,13 @@ use crate::{ }; // TODO: Document. -// TODO: Rename to `NoteInclusionProofs`? #[derive(Clone, Default, Debug)] -pub struct NoteAuthenticationInfo { +pub struct NoteInclusionProofs { block_proofs: Vec, note_proofs: BTreeMap, } -impl NoteAuthenticationInfo { +impl NoteInclusionProofs { pub fn block_proofs(&self) -> &[BlockInclusionProof] { &self.block_proofs } diff --git a/crates/miden-objects/src/note/mod.rs b/crates/miden-objects/src/note/mod.rs index 8a917aed3..0e0212836 100644 --- a/crates/miden-objects/src/note/mod.rs +++ b/crates/miden-objects/src/note/mod.rs @@ -54,7 +54,7 @@ mod file; pub use file::NoteFile; mod authentication_info; -pub use authentication_info::NoteAuthenticationInfo; +pub use authentication_info::NoteInclusionProofs; // NOTE // ================================================================================================ From 3414313571915160347c76e8888e52d58730e8fd Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 27 Jan 2025 12:35:27 +0100 Subject: [PATCH 12/22] feat:Add batch expiration block num --- .../src/local_batch_prover.rs | 29 +++++++++++++++---- crates/miden-batch-prover/src/proven_batch.rs | 9 ++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index d3e56b91f..7ba8139fd 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -6,6 +6,7 @@ use alloc::{ use miden_objects::{ account::{AccountId, AccountUpdate}, batch::{BatchId, BatchNoteTree}, + block::BlockNumber, note::{NoteHeader, NoteId}, transaction::{InputNoteCommitment, OutputNote, ProvenTransaction}, }; @@ -15,14 +16,24 @@ use crate::{BatchError, ProposedBatch, ProvenBatch}; pub struct LocalBatchProver {} impl LocalBatchProver { + /// Attempts to prove the [`ProposedBatch`] into a [`ProvenBatch`]. + /// + /// Returns an error if: + /// + /// - The transactions in the proposed batch which update the same account are not correctly + /// ordered. That is, if two transactions A and B update the same account in this order, + /// meaning B's initial account state commitment matches the final account state commitment of + /// A, then A must come before B in the [`ProposedBatch`]. + /// - If any note is consumed twice. + /// - If any note is created more than once. pub fn prove(proposed_batch: ProposedBatch) -> Result { let transactions = proposed_batch.transactions(); let id = BatchId::compute(transactions.iter().map(|tx| tx.id())); // Populate batch output notes and updated accounts. - let mut output_notes = OutputNoteTracker::new(transactions.iter().map(|tx| tx.as_ref()))?; let mut updated_accounts = BTreeMap::::new(); let mut unauthenticated_input_notes = BTreeSet::new(); + let mut batch_expiration_block_num = BlockNumber::from(u32::MAX); for tx in transactions { // Merge account updates so that state transitions A->B->C become A->C. match updated_accounts.entry(tx.account_id()) { @@ -30,6 +41,8 @@ impl LocalBatchProver { vacant.insert(tx.account_update().clone()); }, Entry::Occupied(occupied) => { + // This returns an error if the transactions are not correctly ordered, e.g. if + // B comes before A. occupied.into_mut().merge_proven_tx(tx).map_err(|source| { BatchError::AccountUpdateError { account_id: tx.account_id(), source } })?; @@ -43,6 +56,10 @@ impl LocalBatchProver { return Err(BatchError::DuplicateUnauthenticatedNote(id)); } } + + // The expiration block of the batch is the minimum of all transaction's expiration + // block. + batch_expiration_block_num = batch_expiration_block_num.min(tx.expiration_block_num()); } // Populate batch produced nullifiers and match output notes with corresponding @@ -54,19 +71,20 @@ impl LocalBatchProver { // `y` and for transaction `B` to consume an unauthenticated note `y` and output // note `x` (i.e., have a circular dependency between transactions), but this is not // a problem. + let mut output_notes = OutputNoteTracker::new(transactions.iter().map(AsRef::as_ref))?; let mut input_notes = vec![]; for tx in transactions { for input_note in tx.input_notes().iter() { - // Header is presented only for unauthenticated input notes. + // Header is present only for unauthenticated input notes. let input_note = match input_note.header() { Some(input_note_header) => { if output_notes.remove_note(input_note_header)? { continue; } - // If an unauthenticated note was found in the store, transform it to an - // authenticated one (i.e. erase additional note details - // except the nullifier) + // If an inclusion proof for an unauthenticated note is provided and the + // proof is valid, it means the note is part of the chain and we can mark it + // as authenticated by erasing the note header. if proposed_batch .note_inclusion_proofs() .contains_note(&input_note_header.id()) @@ -96,6 +114,7 @@ impl LocalBatchProver { input_notes, output_notes_smt, output_notes, + batch_expiration_block_num, )) } } diff --git a/crates/miden-batch-prover/src/proven_batch.rs b/crates/miden-batch-prover/src/proven_batch.rs index b54f0dadb..ff17bc3c0 100644 --- a/crates/miden-batch-prover/src/proven_batch.rs +++ b/crates/miden-batch-prover/src/proven_batch.rs @@ -3,6 +3,7 @@ use alloc::{collections::BTreeMap, vec::Vec}; use miden_objects::{ account::{AccountId, AccountUpdate}, batch::{BatchId, BatchNoteTree}, + block::BlockNumber, transaction::{InputNoteCommitment, OutputNote}, }; @@ -14,6 +15,7 @@ pub struct ProvenBatch { input_notes: Vec, output_notes_smt: BatchNoteTree, output_notes: Vec, + batch_expiration_block_num: BlockNumber, } impl ProvenBatch { @@ -27,6 +29,7 @@ impl ProvenBatch { input_notes: Vec, output_notes_smt: BatchNoteTree, output_notes: Vec, + batch_expiration_block_num: BlockNumber, ) -> Self { Self { id, @@ -34,6 +37,7 @@ impl ProvenBatch { input_notes, output_notes_smt, output_notes, + batch_expiration_block_num, } } @@ -45,6 +49,11 @@ impl ProvenBatch { self.id } + /// Returns the block number at which the batch will expire. + pub fn batch_expiration_block_num(&self) -> BlockNumber { + self.batch_expiration_block_num + } + /// Returns the map of account IDs mapped to their [`AccountUpdate`]s. /// /// If an account was updated by multiple transactions, the [`AccountUpdate`] is the result of From 523bc16259a5f36ac54e504448ce61fa61396f8a Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 27 Jan 2025 13:20:00 +0100 Subject: [PATCH 13/22] chore: Use core instead of std for `Display` --- crates/miden-objects/src/batch/batch_id.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/miden-objects/src/batch/batch_id.rs b/crates/miden-objects/src/batch/batch_id.rs index c2c471199..085464991 100644 --- a/crates/miden-objects/src/batch/batch_id.rs +++ b/crates/miden-objects/src/batch/batch_id.rs @@ -32,8 +32,8 @@ impl BatchId { } } -impl std::fmt::Display for BatchId { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl core::fmt::Display for BatchId { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.write_str(&miden_crypto::utils::bytes_to_hex_string(self.0.as_bytes())) } } From b0768a5473a2c16c12ef6d0af69763a0db7f3573 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 27 Jan 2025 15:13:27 +0100 Subject: [PATCH 14/22] feat: Migrate `MockProvenTxBuilder` --- Cargo.lock | 14 ++ crates/miden-batch-prover/Cargo.toml | 10 +- .../src/local_batch_prover.rs | 129 +++++++++++++++++- crates/miden-batch-prover/src/testing/mod.rs | 3 +- .../src/testing/proven_tx_builder.rs | 92 +++++++++++++ 5 files changed, 237 insertions(+), 11 deletions(-) create mode 100644 crates/miden-batch-prover/src/testing/proven_tx_builder.rs diff --git a/Cargo.lock b/Cargo.lock index b54d6958d..312112d56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1820,14 +1820,17 @@ dependencies = [ name = "miden-batch-prover" version = "0.8.0" dependencies = [ + "anyhow", "miden-assembly", "miden-core", "miden-crypto", + "miden-lib", "miden-objects", "miden-processor", "miden-verifier", "rand", "thiserror 2.0.11", + "winterfell", ] [[package]] @@ -5106,6 +5109,17 @@ dependencies = [ "winter-utils", ] +[[package]] +name = "winterfell" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6bdcd01333bbf4a349d8d13f269281524bd6d1a36ae3a853187f0665bf1cfd4" +dependencies = [ + "winter-air", + "winter-prover", + "winter-verifier", +] + [[package]] name = "write16" version = "1.0.0" diff --git a/crates/miden-batch-prover/Cargo.toml b/crates/miden-batch-prover/Cargo.toml index 63cf51293..dee1df38c 100644 --- a/crates/miden-batch-prover/Cargo.toml +++ b/crates/miden-batch-prover/Cargo.toml @@ -22,16 +22,20 @@ bench = false [features] default = ["std"] std = ["assembly/std", "miden-crypto/std", "miden-verifier/std", "vm-core/std", "vm-processor/std"] -testing = ["dep:rand"] +testing = ["dep:rand", "dep:miden-lib", "dep:winterfell", "dep:anyhow"] [dependencies] +anyhow = { version = "1.0", default-features = false, optional = true } assembly = { workspace = true } -miden-objects = { workspace = true } miden-crypto = { workspace = true } +miden-lib = { workspace = true, optional = true, features = ["testing"]} +miden-objects = { workspace = true } miden-verifier = { workspace = true } -rand = { workspace = true, optional = true } +rand = { workspace = true, optional = true, features = ["small_rng"]} thiserror = { workspace = true } vm-core = { workspace = true } vm-processor = { workspace = true } +winterfell = { version = "0.11", optional = true } [dev-dependencies] +anyhow = { version = "1.0", features = ["std", "backtrace"] } diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 7ba8139fd..14c6482ec 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -62,22 +62,22 @@ impl LocalBatchProver { batch_expiration_block_num = batch_expiration_block_num.min(tx.expiration_block_num()); } - // Populate batch produced nullifiers and match output notes with corresponding - // unauthenticated input notes in the same batch, which are removed from the unauthenticated - // input notes set. + // Remove all output notes from the batch output note set that are consumed by transactions. // // One thing to note: // This still allows transaction `A` to consume an unauthenticated note `x` and output note // `y` and for transaction `B` to consume an unauthenticated note `y` and output // note `x` (i.e., have a circular dependency between transactions), but this is not // a problem. - let mut output_notes = OutputNoteTracker::new(transactions.iter().map(AsRef::as_ref))?; + let mut output_notes = BatchOutputNoteTracker::new(transactions.iter().map(AsRef::as_ref))?; let mut input_notes = vec![]; for tx in transactions { for input_note in tx.input_notes().iter() { // Header is present only for unauthenticated input notes. let input_note = match input_note.header() { Some(input_note_header) => { + // If a transaction consumes a note that is also created in this batch, the + // note is effectively erased from the overall output note set of the batch. if output_notes.remove_note(input_note_header)? { continue; } @@ -106,7 +106,7 @@ impl LocalBatchProver { let output_notes_smt = BatchNoteTree::with_contiguous_leaves( output_notes.iter().map(|note| (note.id(), note.metadata())), ) - .expect("Unreachable: fails only if the output note list contains duplicates"); + .expect("output note tracker should return an error for duplicate notes"); Ok(ProvenBatch::new( id, @@ -120,12 +120,12 @@ impl LocalBatchProver { } #[derive(Debug)] -struct OutputNoteTracker { +struct BatchOutputNoteTracker { output_notes: Vec>, output_note_index: BTreeMap, } -impl OutputNoteTracker { +impl BatchOutputNoteTracker { fn new<'a>(txs: impl Iterator) -> Result { let mut output_notes = vec![]; let mut output_note_index = BTreeMap::new(); @@ -162,3 +162,118 @@ impl OutputNoteTracker { self.output_notes.into_iter().flatten().collect() } } + +#[cfg(test)] +mod tests { + use alloc::sync::Arc; + + use miden_lib::{account::wallets::BasicWallet, transaction::TransactionKernel}; + use miden_objects::{ + account::{Account, AccountBuilder}, + note::{Note, NoteInclusionProofs}, + testing::{account_id::AccountIdBuilder, note::NoteBuilder}, + }; + use rand::{rngs::SmallRng, SeedableRng}; + use vm_core::assert_matches; + use vm_processor::Digest; + + use super::*; + use crate::testing::MockProvenTxBuilder; + + fn mock_account_id(num: u8) -> AccountId { + AccountIdBuilder::new().build_with_rng(&mut SmallRng::from_seed([num; 32])) + } + + fn mock_wallet_account(num: u8) -> Account { + AccountBuilder::new([num; 32]) + .with_component(BasicWallet) + .build_existing() + .unwrap() + } + + pub fn mock_note(num: u8) -> Note { + let sender = mock_account_id(num); + NoteBuilder::new(sender, SmallRng::from_seed([num; 32])) + .build(&TransactionKernel::assembler().with_debug_mode(true)) + .unwrap() + } + + pub fn mock_output_note(num: u8) -> OutputNote { + OutputNote::Full(mock_note(num)) + } + + #[test] + fn duplicate_unauthenticated_input_notes() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + let account2 = mock_wallet_account(100); + + let note0 = mock_note(50); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .unauthenticated_notes(vec![note0.clone()]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) + .unauthenticated_notes(vec![note0.clone()]) + .build()?; + + let error = LocalBatchProver::prove(ProposedBatch::new( + [tx1, tx2].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + )) + .unwrap_err(); + + assert_matches!(error, BatchError::DuplicateUnauthenticatedNote(id) if id == note0.id()); + + Ok(()) + } + + #[test] + fn duplicate_output_notes() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + let account2 = mock_wallet_account(100); + + let note0 = mock_output_note(50); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .output_notes(vec![note0.clone()]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) + .output_notes(vec![note0.clone()]) + .build()?; + + let error = LocalBatchProver::prove(ProposedBatch::new( + [tx1, tx2].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + )) + .unwrap_err(); + + assert_matches!(error, BatchError::DuplicateOutputNote(id) if id == note0.id()); + + Ok(()) + } + + #[test] + fn note_created_and_consumed_in_same_batch() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + let account2 = mock_wallet_account(100); + + let note0 = mock_note(50); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .output_notes(vec![mock_output_note(0)]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) + .unauthenticated_notes(vec![note0.clone()]) + .build()?; + + LocalBatchProver::prove(ProposedBatch::new( + [tx1, tx2].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + ))?; + + Ok(()) + } +} diff --git a/crates/miden-batch-prover/src/testing/mod.rs b/crates/miden-batch-prover/src/testing/mod.rs index 8b1378917..2154b4fb2 100644 --- a/crates/miden-batch-prover/src/testing/mod.rs +++ b/crates/miden-batch-prover/src/testing/mod.rs @@ -1 +1,2 @@ - +mod proven_tx_builder; +pub use proven_tx_builder::MockProvenTxBuilder; diff --git a/crates/miden-batch-prover/src/testing/proven_tx_builder.rs b/crates/miden-batch-prover/src/testing/proven_tx_builder.rs new file mode 100644 index 000000000..d044d5cfa --- /dev/null +++ b/crates/miden-batch-prover/src/testing/proven_tx_builder.rs @@ -0,0 +1,92 @@ +use alloc::vec::Vec; + +use anyhow::Context; +use miden_objects::{ + account::AccountId, + block::BlockNumber, + note::{Note, Nullifier}, + transaction::{InputNote, OutputNote, ProvenTransaction, ProvenTransactionBuilder}, + vm::ExecutionProof, +}; +use vm_processor::Digest; +use winterfell::Proof; + +/// A builder to build mocked [`ProvenTransaction`]s. +pub struct MockProvenTxBuilder { + account_id: AccountId, + initial_account_commitment: Digest, + final_account_commitment: Digest, + expiration_block_num: BlockNumber, + output_notes: Option>, + input_notes: Option>, + nullifiers: Option>, +} + +impl MockProvenTxBuilder { + /// Creates a new builder for a transaction executed against the given account with its initial + /// and final state commitment. + pub fn with_account( + account_id: AccountId, + initial_account_commitment: Digest, + final_account_commitment: Digest, + ) -> Self { + Self { + account_id, + initial_account_commitment, + final_account_commitment, + expiration_block_num: BlockNumber::from(u32::MAX), + output_notes: None, + input_notes: None, + nullifiers: None, + } + } + + /// Adds unauthenticated notes to the transaction. + #[must_use] + pub fn unauthenticated_notes(mut self, notes: Vec) -> Self { + self.input_notes = Some(notes.into_iter().map(InputNote::unauthenticated).collect()); + + self + } + + /// Adds nullifiers to the transaction's input note commitment. + #[must_use] + pub fn nullifiers(mut self, nullifiers: Vec) -> Self { + self.nullifiers = Some(nullifiers); + + self + } + + /// Sets the transaction's expiration block number. + #[must_use] + pub fn expiration_block_num(mut self, expiration_block_num: BlockNumber) -> Self { + self.expiration_block_num = expiration_block_num; + + self + } + + /// Adds notes to the transaction's output notes. + #[must_use] + pub fn output_notes(mut self, notes: Vec) -> Self { + self.output_notes = Some(notes); + + self + } + + /// Builds the [`ProvenTransaction`] and returns potential errors. + pub fn build(self) -> anyhow::Result { + ProvenTransactionBuilder::new( + self.account_id, + self.initial_account_commitment, + self.final_account_commitment, + Digest::default(), + self.expiration_block_num, + ExecutionProof::new(Proof::new_dummy(), Default::default()), + ) + .add_input_notes(self.input_notes.unwrap_or_default()) + .add_input_notes(self.nullifiers.unwrap_or_default()) + .add_output_notes(self.output_notes.unwrap_or_default()) + .build() + .context("failed to build proven transaction") + } +} From 7e82a7c3cb9e9aa3aea5db98638db0a869a70d8d Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 27 Jan 2025 15:29:22 +0100 Subject: [PATCH 15/22] feat: Test tx ordering in batches --- crates/miden-batch-prover/Cargo.toml | 2 +- .../src/local_batch_prover.rs | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/crates/miden-batch-prover/Cargo.toml b/crates/miden-batch-prover/Cargo.toml index dee1df38c..76ead7659 100644 --- a/crates/miden-batch-prover/Cargo.toml +++ b/crates/miden-batch-prover/Cargo.toml @@ -21,7 +21,7 @@ bench = false [features] default = ["std"] -std = ["assembly/std", "miden-crypto/std", "miden-verifier/std", "vm-core/std", "vm-processor/std"] +std = ["assembly/std", "miden-objects/std", "miden-crypto/std", "miden-verifier/std", "vm-core/std", "vm-processor/std"] testing = ["dep:rand", "dep:miden-lib", "dep:winterfell", "dep:anyhow"] [dependencies] diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 14c6482ec..847f33970 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -276,4 +276,39 @@ mod tests { Ok(()) } + + #[test] + fn multiple_transactions_against_same_account() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .output_notes(vec![mock_output_note(0)]) + .build()?; + // Use some other hash as the final state commitment of tx2. + let final_state_commitment = mock_note(10).hash(); + let tx2 = MockProvenTxBuilder::with_account( + account1.id(), + account1.hash(), + final_state_commitment, + ) + .build()?; + + // Success: Transactions are correctly ordered. + LocalBatchProver::prove(ProposedBatch::new( + [tx1.clone(), tx2.clone()].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + ))?; + + // Error: Transactions are incorrectly ordered. + let error = LocalBatchProver::prove(ProposedBatch::new( + [tx2, tx1].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + )) + .unwrap_err(); + + assert_matches!(error, BatchError::AccountUpdateError { .. }); + + Ok(()) + } } From a7f1b5df452d1dfed9a572c8bc8abc31e7b94bbf Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 27 Jan 2025 15:35:10 +0100 Subject: [PATCH 16/22] feat: Add `BatchAccountUpdate` --- crates/miden-batch-prover/src/error.rs | 4 +- .../src/local_batch_prover.rs | 15 +- crates/miden-batch-prover/src/proven_batch.rs | 10 +- .../src/batch/batch_account_update.rs | 158 ++++++++++++++++++ crates/miden-objects/src/batch/mod.rs | 3 + crates/miden-objects/src/errors.rs | 4 +- crates/miden-objects/src/lib.rs | 4 +- 7 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 crates/miden-objects/src/batch/batch_account_update.rs diff --git a/crates/miden-batch-prover/src/error.rs b/crates/miden-batch-prover/src/error.rs index c9bce676b..1e07cae08 100644 --- a/crates/miden-batch-prover/src/error.rs +++ b/crates/miden-batch-prover/src/error.rs @@ -1,4 +1,4 @@ -use miden_objects::{account::AccountId, note::NoteId, AccountUpdateError}; +use miden_objects::{account::AccountId, note::NoteId, BatchAccountUpdateError}; use thiserror::Error; use vm_processor::Digest; @@ -21,6 +21,6 @@ pub enum BatchError { #[error("failed to merge transaction delta into account {account_id}")] AccountUpdateError { account_id: AccountId, - source: AccountUpdateError, + source: BatchAccountUpdateError, }, } diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 847f33970..65efab21d 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -4,8 +4,8 @@ use alloc::{ }; use miden_objects::{ - account::{AccountId, AccountUpdate}, - batch::{BatchId, BatchNoteTree}, + account::AccountId, + batch::{BatchAccountUpdate, BatchId, BatchNoteTree}, block::BlockNumber, note::{NoteHeader, NoteId}, transaction::{InputNoteCommitment, OutputNote, ProvenTransaction}, @@ -31,14 +31,21 @@ impl LocalBatchProver { let id = BatchId::compute(transactions.iter().map(|tx| tx.id())); // Populate batch output notes and updated accounts. - let mut updated_accounts = BTreeMap::::new(); + let mut updated_accounts = BTreeMap::::new(); let mut unauthenticated_input_notes = BTreeSet::new(); let mut batch_expiration_block_num = BlockNumber::from(u32::MAX); for tx in transactions { // Merge account updates so that state transitions A->B->C become A->C. match updated_accounts.entry(tx.account_id()) { Entry::Vacant(vacant) => { - vacant.insert(tx.account_update().clone()); + let batch_account_update = BatchAccountUpdate::new( + tx.account_id(), + tx.account_update().init_state_hash(), + tx.account_update().final_state_hash(), + vec![tx.id()], + tx.account_update().details().clone(), + ); + vacant.insert(batch_account_update); }, Entry::Occupied(occupied) => { // This returns an error if the transactions are not correctly ordered, e.g. if diff --git a/crates/miden-batch-prover/src/proven_batch.rs b/crates/miden-batch-prover/src/proven_batch.rs index ff17bc3c0..2b8bf6a1d 100644 --- a/crates/miden-batch-prover/src/proven_batch.rs +++ b/crates/miden-batch-prover/src/proven_batch.rs @@ -1,8 +1,8 @@ use alloc::{collections::BTreeMap, vec::Vec}; use miden_objects::{ - account::{AccountId, AccountUpdate}, - batch::{BatchId, BatchNoteTree}, + account::AccountId, + batch::{BatchAccountUpdate, BatchId, BatchNoteTree}, block::BlockNumber, transaction::{InputNoteCommitment, OutputNote}, }; @@ -11,7 +11,7 @@ use miden_objects::{ #[derive(Debug, Clone, PartialEq, Eq)] pub struct ProvenBatch { id: BatchId, - account_updates: BTreeMap, + account_updates: BTreeMap, input_notes: Vec, output_notes_smt: BatchNoteTree, output_notes: Vec, @@ -25,7 +25,7 @@ impl ProvenBatch { /// Creates a new [`ProvenBatch`] from the provided parts. pub fn new( id: BatchId, - account_updates: BTreeMap, + account_updates: BTreeMap, input_notes: Vec, output_notes_smt: BatchNoteTree, output_notes: Vec, @@ -64,7 +64,7 @@ impl ProvenBatch { /// `B`, and applying the second one results in state `C`. Then the returned update represents /// the state transition from `A` to `C`. // TODO: Check if we should return the map or an opaque iterator. - pub fn account_updates(&self) -> &BTreeMap { + pub fn account_updates(&self) -> &BTreeMap { &self.account_updates } diff --git a/crates/miden-objects/src/batch/batch_account_update.rs b/crates/miden-objects/src/batch/batch_account_update.rs new file mode 100644 index 000000000..5093c3c14 --- /dev/null +++ b/crates/miden-objects/src/batch/batch_account_update.rs @@ -0,0 +1,158 @@ +use alloc::vec::Vec; + +use vm_core::utils::{ByteReader, ByteWriter, Deserializable, Serializable}; +use vm_processor::{DeserializationError, Digest}; + +use crate::{ + account::{delta::AccountUpdateDetails, AccountId}, + errors::BatchAccountUpdateError, + transaction::{ProvenTransaction, TransactionId}, +}; + +// ACCOUNT UPDATE +// ================================================================================================ + +/// Represents the changes made to an account resulting from executing a batch of transactions. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BatchAccountUpdate { + /// ID of the updated account. + account_id: AccountId, + + /// Commitment to the state of the account before this update is applied. + /// + /// Equal to `Digest::default()` for new accounts. + initial_state_commitment: Digest, + + /// Commitment to the state of the account after this update is applied. + final_state_commitment: Digest, + + /// IDs of all transactions that updated the account. + transactions: Vec, + + /// A set of changes which can be applied to the previous account state (i.e. `initial_state`) + /// to get the new account state. For private accounts, this is set to + /// [`AccountUpdateDetails::Private`]. + details: AccountUpdateDetails, +} + +impl BatchAccountUpdate { + // CONSTANTS + // -------------------------------------------------------------------------------------------- + + /// The maximum allowed size of an account update in bytes. Set to 32 KiB. + pub const MAX_SIZE: u16 = 2u16.pow(15); + + // CONSTRUCTORS + // -------------------------------------------------------------------------------------------- + + /// Returns a new [`AccountUpdate`] instantiated from the provided parts. + pub const fn new( + account_id: AccountId, + initial_state_commitment: Digest, + final_state_commitment: Digest, + transactions: Vec, + details: AccountUpdateDetails, + ) -> Self { + Self { + account_id, + initial_state_commitment, + final_state_commitment, + transactions, + details, + } + } + + // PUBLIC ACCESSORS + // -------------------------------------------------------------------------------------------- + + /// Returns the ID of the updated account. + pub fn account_id(&self) -> AccountId { + self.account_id + } + + /// Commitment to the state of the account before this update is applied. + /// + /// This is equal to [`Digest::default()`] for new accounts. + pub fn initial_state_commitment(&self) -> Digest { + self.initial_state_commitment + } + + /// Commitment to the state of the account after this update is applied. + pub fn final_state_commitment(&self) -> Digest { + self.final_state_commitment + } + + /// Returns a slice of [`TransactionId`]s that updated this account's state. + /// + /// This slice is generally non-empty, but may be empty in the special case of an account update + /// representing an account created in the genesis. + pub fn transactions(&self) -> &[TransactionId] { + &self.transactions + } + + /// Returns the contained [`AccountUpdateDetails`]. + /// + /// This update can be used to build the new account state from the previous account state. + pub fn details(&self) -> &AccountUpdateDetails { + &self.details + } + + /// Returns `true` if the account update details are for a private account. + pub fn is_private(&self) -> bool { + self.details.is_private() + } + + // MUTATORS + // -------------------------------------------------------------------------------------------- + + /// Merges the transaction's update into this account update. + pub fn merge_proven_tx( + &mut self, + tx: &ProvenTransaction, + ) -> Result<(), BatchAccountUpdateError> { + if self.account_id != tx.account_id() { + return Err(BatchAccountUpdateError::AccountUpdateIdMismatch { + transaction: tx.id(), + expected_account_id: self.account_id, + actual_account_id: tx.account_id(), + }); + } + + if self.final_state_commitment != tx.account_update().init_state_hash() { + return Err(BatchAccountUpdateError::AccountUpdateInitialStateMismatch(tx.id())); + } + + self.details = self.details.clone().merge(tx.account_update().details().clone()).map_err( + |source_err| BatchAccountUpdateError::TransactionUpdateMergeError(tx.id(), source_err), + )?; + self.final_state_commitment = tx.account_update().final_state_hash(); + self.transactions.push(tx.id()); + + Ok(()) + } +} + +// SERIALIZATION +// ================================================================================================ + +impl Serializable for BatchAccountUpdate { + fn write_into(&self, target: &mut W) { + self.account_id.write_into(target); + self.initial_state_commitment.write_into(target); + self.final_state_commitment.write_into(target); + self.transactions.write_into(target); + self.details.write_into(target); + } +} + +impl Deserializable for BatchAccountUpdate { + fn read_from(source: &mut R) -> Result { + Ok(Self { + account_id: AccountId::read_from(source)?, + initial_state_commitment: Digest::read_from(source)?, + final_state_commitment: Digest::read_from(source)?, + transactions: >::read_from(source)?, + details: AccountUpdateDetails::read_from(source)?, + }) + } +} diff --git a/crates/miden-objects/src/batch/mod.rs b/crates/miden-objects/src/batch/mod.rs index 568a95b08..2c0d786fa 100644 --- a/crates/miden-objects/src/batch/mod.rs +++ b/crates/miden-objects/src/batch/mod.rs @@ -3,3 +3,6 @@ pub use note_tree::BatchNoteTree; mod batch_id; pub use batch_id::BatchId; + +mod batch_account_update; +pub use batch_account_update::BatchAccountUpdate; diff --git a/crates/miden-objects/src/errors.rs b/crates/miden-objects/src/errors.rs index 890eb9c08..38861ba69 100644 --- a/crates/miden-objects/src/errors.rs +++ b/crates/miden-objects/src/errors.rs @@ -183,11 +183,11 @@ pub enum AccountDeltaError { NotAFungibleFaucetId(AccountId), } -// ACCOUNT UPDATE ERROR +// BATCH ACCOUNT UPDATE ERROR // ================================================================================================ #[derive(Debug, Error)] -pub enum AccountUpdateError { +pub enum BatchAccountUpdateError { #[error("account update for account {expected_account_id} cannot be merged with update from transaction {transaction} which was executed against account {actual_account_id}")] AccountUpdateIdMismatch { transaction: TransactionId, diff --git a/crates/miden-objects/src/lib.rs b/crates/miden-objects/src/lib.rs index 7eb5534aa..07d4a7f67 100644 --- a/crates/miden-objects/src/lib.rs +++ b/crates/miden-objects/src/lib.rs @@ -24,8 +24,8 @@ mod errors; pub use constants::*; pub use errors::{ - AccountDeltaError, AccountError, AccountIdError, AccountUpdateError, AssetError, - AssetVaultError, BlockError, ChainMmrError, NoteError, ProvenTransactionError, + AccountDeltaError, AccountError, AccountIdError, AssetError, AssetVaultError, + BatchAccountUpdateError, BlockError, ChainMmrError, NoteError, ProvenTransactionError, TransactionInputError, TransactionOutputError, TransactionScriptError, }; pub use miden_crypto::hash::rpo::{Rpo256 as Hasher, RpoDigest as Digest}; From 8961ad0185dbe2a45bfccabb3c08ab5bf7a35f69 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Tue, 28 Jan 2025 08:56:27 +0100 Subject: [PATCH 17/22] chore: Extend test assertions --- .../src/local_batch_prover.rs | 18 ++++++++++++++++-- crates/miden-batch-prover/src/proven_batch.rs | 5 +++++ crates/miden-objects/src/batch/note_tree.rs | 5 +++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 65efab21d..5b09f35e6 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -78,6 +78,7 @@ impl LocalBatchProver { // a problem. let mut output_notes = BatchOutputNoteTracker::new(transactions.iter().map(AsRef::as_ref))?; let mut input_notes = vec![]; + for tx in transactions { for input_note in tx.input_notes().iter() { // Header is present only for unauthenticated input notes. @@ -269,18 +270,31 @@ mod tests { let note0 = mock_note(50); let tx1 = MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) - .output_notes(vec![mock_output_note(0)]) + .output_notes(vec![OutputNote::Full(note0.clone())]) .build()?; let tx2 = MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) .unauthenticated_notes(vec![note0.clone()]) .build()?; - LocalBatchProver::prove(ProposedBatch::new( + let batch = LocalBatchProver::prove(ProposedBatch::new( [tx1, tx2].into_iter().map(Arc::new).collect(), NoteInclusionProofs::default(), ))?; + assert_eq!(batch.account_updates().len(), 2); + assert_eq!(batch.input_notes().len(), 0); + assert_eq!(batch.output_notes().len(), 0); + assert_eq!(batch.output_notes_tree().num_leaves(), 0); + assert_eq!( + batch.account_updates().get(&account1.id()).unwrap().final_state_commitment(), + account1.hash() + ); + assert_eq!( + batch.account_updates().get(&account2.id()).unwrap().final_state_commitment(), + account2.hash() + ); + Ok(()) } diff --git a/crates/miden-batch-prover/src/proven_batch.rs b/crates/miden-batch-prover/src/proven_batch.rs index 2b8bf6a1d..a4d2fcc8f 100644 --- a/crates/miden-batch-prover/src/proven_batch.rs +++ b/crates/miden-batch-prover/src/proven_batch.rs @@ -68,6 +68,11 @@ impl ProvenBatch { &self.account_updates } + /// Returns the slice of [`InputNoteCommitment`]s of this batch. + pub fn input_notes(&self) -> &[InputNoteCommitment] { + &self.input_notes + } + /// Returns the output notes of the batch. /// /// This is the aggregation of all output notes by the transactions in the batch, except the diff --git a/crates/miden-objects/src/batch/note_tree.rs b/crates/miden-objects/src/batch/note_tree.rs index a0d0b5536..e6e98de31 100644 --- a/crates/miden-objects/src/batch/note_tree.rs +++ b/crates/miden-objects/src/batch/note_tree.rs @@ -35,4 +35,9 @@ impl BatchNoteTree { pub fn root(&self) -> RpoDigest { self.0.root() } + + /// Returns the number of non-empty leaves in this tree. + pub fn num_leaves(&self) -> usize { + self.0.num_leaves() + } } From a72e5b7012e287b39dda0b1bad8f522a88e6c7f4 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Tue, 28 Jan 2025 09:45:30 +0100 Subject: [PATCH 18/22] feat: Refactor and document batch output note tracker --- crates/miden-batch-prover/src/error.rs | 12 ++- .../src/local_batch_prover.rs | 95 ++++++++++++++----- 2 files changed, 81 insertions(+), 26 deletions(-) diff --git a/crates/miden-batch-prover/src/error.rs b/crates/miden-batch-prover/src/error.rs index 1e07cae08..8d32249b4 100644 --- a/crates/miden-batch-prover/src/error.rs +++ b/crates/miden-batch-prover/src/error.rs @@ -1,4 +1,6 @@ -use miden_objects::{account::AccountId, note::NoteId, BatchAccountUpdateError}; +use miden_objects::{ + account::AccountId, note::NoteId, transaction::TransactionId, BatchAccountUpdateError, +}; use thiserror::Error; use vm_processor::Digest; @@ -8,8 +10,12 @@ pub enum BatchError { #[error("duplicated unauthenticated transaction input note ID in the batch: {0}")] DuplicateUnauthenticatedNote(NoteId), - #[error("duplicated transaction output note ID in the batch: {0}")] - DuplicateOutputNote(NoteId), + #[error("transaction {second_transaction_id} outputs a note with id {note_id} that is also produced by the previous transaction {first_transaction_id} in the batch")] + DuplicateOutputNote { + note_id: NoteId, + first_transaction_id: TransactionId, + second_transaction_id: TransactionId, + }, #[error("note hashes mismatch for note {id}: (input: {input_hash}, output: {output_hash})")] NoteHashesMismatch { diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 5b09f35e6..3e9ae99db 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -8,7 +8,7 @@ use miden_objects::{ batch::{BatchAccountUpdate, BatchId, BatchNoteTree}, block::BlockNumber, note::{NoteHeader, NoteId}, - transaction::{InputNoteCommitment, OutputNote, ProvenTransaction}, + transaction::{InputNoteCommitment, OutputNote, ProvenTransaction, TransactionId}, }; use crate::{BatchError, ProposedBatch, ProvenBatch}; @@ -30,6 +30,10 @@ impl LocalBatchProver { let transactions = proposed_batch.transactions(); let id = BatchId::compute(transactions.iter().map(|tx| tx.id())); + // Aggregate individual tx-level account updates into a batch-level account update - one per + // account. + // -------------------------------------------------------------------------------------------- + // Populate batch output notes and updated accounts. let mut updated_accounts = BTreeMap::::new(); let mut unauthenticated_input_notes = BTreeSet::new(); @@ -69,6 +73,9 @@ impl LocalBatchProver { batch_expiration_block_num = batch_expiration_block_num.min(tx.expiration_block_num()); } + // Create input and output note set of the batch. + // -------------------------------------------------------------------------------------------- + // Remove all output notes from the batch output note set that are consumed by transactions. // // One thing to note: @@ -84,9 +91,12 @@ impl LocalBatchProver { // Header is present only for unauthenticated input notes. let input_note = match input_note.header() { Some(input_note_header) => { - // If a transaction consumes a note that is also created in this batch, the - // note is effectively erased from the overall output note set of the batch. if output_notes.remove_note(input_note_header)? { + // If a transaction consumes a note that is also created in this batch, + // it is removed from the set of output notes. + // We `continue` so that the input note is not added to the set of input + // notes of the batch. + // That way the note appears in neither input nor output set. continue; } @@ -127,47 +137,80 @@ impl LocalBatchProver { } } +/// A helper struct to track output notes. +/// Its main purpose is to check for duplicates and allow for removal of output notes that are +/// consumed in the same batch, so are not output notes of the batch. +/// +/// The approach for this is that the output note set is initialized to the union of all output +/// notes of the transactions in the batch. +/// Then (outside of this struct) all input notes of transactions in the batch which are also output +/// notes can be removed, as they are considered consumed within the batch and will not be visible +/// as created or consumed notes for the batch. #[derive(Debug)] struct BatchOutputNoteTracker { - output_notes: Vec>, - output_note_index: BTreeMap, + /// An index from [`NoteId`]s to the transaction that creates the note and the note itself. + /// The transaction ID is tracked to produce better errors when a duplicate note is + /// encountered. + output_notes: BTreeMap, } impl BatchOutputNoteTracker { + /// Constructs a new output note tracker from the given transactions. + /// + /// # Errors + /// + /// Returns an error if: + /// - any output note is produced more than once (by the same or different transactions). fn new<'a>(txs: impl Iterator) -> Result { - let mut output_notes = vec![]; - let mut output_note_index = BTreeMap::new(); + let mut output_notes = BTreeMap::new(); for tx in txs { for note in tx.output_notes().iter() { - if output_note_index.insert(note.id(), output_notes.len()).is_some() { - return Err(BatchError::DuplicateOutputNote(note.id())); + if let Some((first_transaction_id, _)) = + output_notes.insert(note.id(), (tx.id(), note.clone())) + { + return Err(BatchError::DuplicateOutputNote { + note_id: note.id(), + first_transaction_id, + second_transaction_id: tx.id(), + }); } - output_notes.push(Some(note.clone())); } } - Ok(Self { output_notes, output_note_index }) + Ok(Self { output_notes }) } + /// Attempts to remove the given input note from the output note set. + /// + /// Returns `true` if the given note existed in the output note set and was removed from it, + /// `false` otherwise. + /// + /// # Errors + /// + /// Returns an error if: + /// - the given note has a corresponding note in the output note set with the same [`NoteId`] + /// but their hashes differ (i.e. their metadata is different). pub fn remove_note(&mut self, input_note_header: &NoteHeader) -> Result { let id = input_note_header.id(); - if let Some(note_index) = self.output_note_index.remove(&id) { - if let Some(output_note) = core::mem::take(&mut self.output_notes[note_index]) { - let input_hash = input_note_header.hash(); - let output_hash = output_note.hash(); - if output_hash != input_hash { - return Err(BatchError::NoteHashesMismatch { id, input_hash, output_hash }); - } - - return Ok(true); + if let Some((_, output_note)) = self.output_notes.remove(&id) { + // Check if the notes with the same ID have differing hashes. + // This could happen if the metadata of the notes is different, which we consider an + // error. + let input_hash = input_note_header.hash(); + let output_hash = output_note.hash(); + if output_hash != input_hash { + return Err(BatchError::NoteHashesMismatch { id, input_hash, output_hash }); } + + return Ok(true); } Ok(false) } + /// Consumes the tracker and returns a [`Vec`] of output notes. pub fn into_notes(self) -> Vec { - self.output_notes.into_iter().flatten().collect() + self.output_notes.into_iter().map(|(_, (_, output_note))| output_note).collect() } } @@ -252,12 +295,18 @@ mod tests { .build()?; let error = LocalBatchProver::prove(ProposedBatch::new( - [tx1, tx2].into_iter().map(Arc::new).collect(), + [tx1.clone(), tx2.clone()].into_iter().map(Arc::new).collect(), NoteInclusionProofs::default(), )) .unwrap_err(); - assert_matches!(error, BatchError::DuplicateOutputNote(id) if id == note0.id()); + assert_matches!(error, BatchError::DuplicateOutputNote { + note_id, + first_transaction_id, + second_transaction_id + } if note_id == note0.id() && + first_transaction_id == tx1.id() && + second_transaction_id == tx2.id()); Ok(()) } From 89630b0da20bdade2b2080cd4250c146c8da0967 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Tue, 28 Jan 2025 11:16:04 +0100 Subject: [PATCH 19/22] feat: Add input/output notes commitment test --- .../src/local_batch_prover.rs | 108 ++++++++++++++++-- 1 file changed, 99 insertions(+), 9 deletions(-) diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 3e9ae99db..3918def70 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -208,7 +208,7 @@ impl BatchOutputNoteTracker { Ok(false) } - /// Consumes the tracker and returns a [`Vec`] of output notes. + /// Consumes the tracker and returns a [`Vec`] of output notes sorted by [`NoteId`]. pub fn into_notes(self) -> Vec { self.output_notes.into_iter().map(|(_, (_, output_note))| output_note).collect() } @@ -223,6 +223,8 @@ mod tests { account::{Account, AccountBuilder}, note::{Note, NoteInclusionProofs}, testing::{account_id::AccountIdBuilder, note::NoteBuilder}, + transaction::InputNote, + BatchAccountUpdateError, }; use rand::{rngs::SmallRng, SeedableRng}; use vm_core::assert_matches; @@ -253,6 +255,8 @@ mod tests { OutputNote::Full(mock_note(num)) } + /// Tests that an error is returned if the same unauthenticated input note appears multiple + /// times in different transactions. #[test] fn duplicate_unauthenticated_input_notes() -> anyhow::Result<()> { let account1 = mock_wallet_account(10); @@ -279,6 +283,8 @@ mod tests { Ok(()) } + /// Tests that an error is returned if the same output note appears multiple times in different + /// transactions. #[test] fn duplicate_output_notes() -> anyhow::Result<()> { let account1 = mock_wallet_account(10); @@ -311,6 +317,8 @@ mod tests { Ok(()) } + /// Tests that a note created and consumed in the same batch are erased from the input and + /// output note commitments. #[test] fn note_created_and_consumed_in_same_batch() -> anyhow::Result<()> { let account1 = mock_wallet_account(10); @@ -347,15 +355,23 @@ mod tests { Ok(()) } + /// Test that multiple transactions against the same account 1) can be correctly executed and + /// 2) that an error is returned if they are incorrectly ordered. #[test] fn multiple_transactions_against_same_account() -> anyhow::Result<()> { let account1 = mock_wallet_account(10); - let tx1 = - MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) - .output_notes(vec![mock_output_note(0)]) - .build()?; - // Use some other hash as the final state commitment of tx2. + // Use some random hash as the initial state commitment of tx1. + let initial_state_commitment = Digest::default(); + let tx1 = MockProvenTxBuilder::with_account( + account1.id(), + initial_state_commitment, + account1.hash(), + ) + .output_notes(vec![mock_output_note(0)]) + .build()?; + + // Use some random hash as the final state commitment of tx2. let final_state_commitment = mock_note(10).hash(); let tx2 = MockProvenTxBuilder::with_account( account1.id(), @@ -365,19 +381,93 @@ mod tests { .build()?; // Success: Transactions are correctly ordered. - LocalBatchProver::prove(ProposedBatch::new( + let batch = LocalBatchProver::prove(ProposedBatch::new( [tx1.clone(), tx2.clone()].into_iter().map(Arc::new).collect(), NoteInclusionProofs::default(), ))?; + assert_eq!(batch.account_updates().len(), 1); + // Assert that initial state commitment from tx1 is used and the final state commitment from + // tx2. + assert_eq!( + batch.account_updates().get(&account1.id()).unwrap().initial_state_commitment(), + initial_state_commitment + ); + assert_eq!( + batch.account_updates().get(&account1.id()).unwrap().final_state_commitment(), + final_state_commitment + ); + // Error: Transactions are incorrectly ordered. let error = LocalBatchProver::prove(ProposedBatch::new( - [tx2, tx1].into_iter().map(Arc::new).collect(), + [tx2.clone(), tx1.clone()].into_iter().map(Arc::new).collect(), NoteInclusionProofs::default(), )) .unwrap_err(); - assert_matches!(error, BatchError::AccountUpdateError { .. }); + assert_matches!( + error, + BatchError::AccountUpdateError { + source: BatchAccountUpdateError::AccountUpdateInitialStateMismatch(tx_id), + .. + } if tx_id == tx1.id() + ); + + Ok(()) + } + + /// Tests that the input and outputs notes commitment is correctly computed. + /// - Notes created and consumed in the same batch are erased from these commitments. + /// - The input note commitment is sorted by the order in which the notes appeared in the batch. + /// - The output note commitment is sorted by [`NoteId`]. + #[test] + fn input_and_output_notes_commitment() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + let account2 = mock_wallet_account(100); + + let note0 = mock_output_note(50); + let note1 = mock_note(60); + let note2 = mock_output_note(70); + let note3 = mock_output_note(80); + let note4 = mock_note(90); + let note5 = mock_note(100); + + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .unauthenticated_notes(vec![note1.clone(), note5.clone()]) + .output_notes(vec![note0.clone()]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) + .unauthenticated_notes(vec![note4.clone()]) + .output_notes(vec![OutputNote::Full(note1.clone()), note2.clone(), note3.clone()]) + .build()?; + + let batch = LocalBatchProver::prove(ProposedBatch::new( + [tx1.clone(), tx2.clone()].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + ))?; + + // We expecte note1 to be erased from the input/output notes as it is created and consumed + // in the batch. + let mut expected_output_notes = [note0, note2, note3]; + // We expect a vector sorted by NoteId. + expected_output_notes.sort_unstable_by_key(OutputNote::id); + + assert_eq!(batch.output_notes().len(), 3); + assert_eq!(batch.output_notes(), expected_output_notes); + + assert_eq!(batch.output_notes_tree().num_leaves(), 3); + + // Input notes are sorted by the order in which they appeared in the batch. + assert_eq!(batch.input_notes().len(), 2); + assert_eq!( + batch.input_notes(), + &[ + InputNoteCommitment::from(&InputNote::unauthenticated(note5)), + InputNoteCommitment::from(&InputNote::unauthenticated(note4)), + ] + ); Ok(()) } From ad52eb1764cbfe5946681eea5d45a3e5ee4656e2 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Tue, 28 Jan 2025 11:42:58 +0100 Subject: [PATCH 20/22] feat: Remove `BlockNumber::from_usize` --- crates/miden-objects/src/block/block_number.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/miden-objects/src/block/block_number.rs b/crates/miden-objects/src/block/block_number.rs index 851ed4f00..656af8049 100644 --- a/crates/miden-objects/src/block/block_number.rs +++ b/crates/miden-objects/src/block/block_number.rs @@ -40,11 +40,6 @@ impl BlockNumber { BlockNumber((epoch as u32) << BlockNumber::EPOCH_LENGTH_EXPONENT) } - /// Creates a `BlockNumber` from a `usize`. - pub fn from_usize(value: usize) -> Self { - BlockNumber(value as u32) - } - /// Returns the epoch to which this block number belongs. pub const fn block_epoch(&self) -> u16 { (self.0 >> BlockNumber::EPOCH_LENGTH_EXPONENT) as u16 From dc57f9d6ec413123bbfaca5c00e669ea3ac4503c Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Tue, 28 Jan 2025 13:01:01 +0100 Subject: [PATCH 21/22] feat: Check for duplicate input notes --- crates/miden-batch-prover/src/error.rs | 15 ++- .../src/local_batch_prover.rs | 119 +++++++++++++++--- .../src/testing/proven_tx_builder.rs | 18 ++- 3 files changed, 132 insertions(+), 20 deletions(-) diff --git a/crates/miden-batch-prover/src/error.rs b/crates/miden-batch-prover/src/error.rs index 8d32249b4..fc362f648 100644 --- a/crates/miden-batch-prover/src/error.rs +++ b/crates/miden-batch-prover/src/error.rs @@ -1,5 +1,8 @@ use miden_objects::{ - account::AccountId, note::NoteId, transaction::TransactionId, BatchAccountUpdateError, + account::AccountId, + note::{NoteId, Nullifier}, + transaction::TransactionId, + BatchAccountUpdateError, }; use thiserror::Error; use vm_processor::Digest; @@ -7,10 +10,14 @@ use vm_processor::Digest; /// Error encountered while building a batch. #[derive(Debug, Error)] pub enum BatchError { - #[error("duplicated unauthenticated transaction input note ID in the batch: {0}")] - DuplicateUnauthenticatedNote(NoteId), + #[error("transaction {second_transaction_id} consumes the note with nullifier {note_nullifier} that is also consumed by another transaction {first_transaction_id} in the batch")] + DuplicateInputNote { + note_nullifier: Nullifier, + first_transaction_id: TransactionId, + second_transaction_id: TransactionId, + }, - #[error("transaction {second_transaction_id} outputs a note with id {note_id} that is also produced by the previous transaction {first_transaction_id} in the batch")] + #[error("transaction {second_transaction_id} creates the note with id {note_id} that is also created by another transaction {first_transaction_id} in the batch")] DuplicateOutputNote { note_id: NoteId, first_transaction_id: TransactionId, diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 3918def70..354785201 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -1,5 +1,5 @@ use alloc::{ - collections::{btree_map::Entry, BTreeMap, BTreeSet}, + collections::{btree_map::Entry, BTreeMap}, vec::Vec, }; @@ -7,7 +7,7 @@ use miden_objects::{ account::AccountId, batch::{BatchAccountUpdate, BatchId, BatchNoteTree}, block::BlockNumber, - note::{NoteHeader, NoteId}, + note::{NoteHeader, NoteId, Nullifier}, transaction::{InputNoteCommitment, OutputNote, ProvenTransaction, TransactionId}, }; @@ -36,7 +36,6 @@ impl LocalBatchProver { // Populate batch output notes and updated accounts. let mut updated_accounts = BTreeMap::::new(); - let mut unauthenticated_input_notes = BTreeSet::new(); let mut batch_expiration_block_num = BlockNumber::from(u32::MAX); for tx in transactions { // Merge account updates so that state transitions A->B->C become A->C. @@ -60,19 +59,32 @@ impl LocalBatchProver { }, }; - // Check unauthenticated input notes for duplicates: - for note in tx.get_unauthenticated_notes() { - let id = note.id(); - if !unauthenticated_input_notes.insert(id) { - return Err(BatchError::DuplicateUnauthenticatedNote(id)); - } - } - // The expiration block of the batch is the minimum of all transaction's expiration // block. batch_expiration_block_num = batch_expiration_block_num.min(tx.expiration_block_num()); } + // Check for duplicates in input notes. + // -------------------------------------------------------------------------------------------- + + // Check for duplicate input notes both within a transaction and across transactions. + // This also includes authenticated notes, as the transaction kernel doesn't check for + // duplicates. + let mut input_note_map = BTreeMap::new(); + + for tx in transactions.iter() { + for note in tx.input_notes() { + let nullifier = note.nullifier(); + if let Some(first_transaction_id) = input_note_map.insert(nullifier, tx.id()) { + return Err(BatchError::DuplicateInputNote { + note_nullifier: nullifier, + first_transaction_id, + second_transaction_id: tx.id(), + }); + } + } + } + // Create input and output note set of the batch. // -------------------------------------------------------------------------------------------- @@ -81,8 +93,7 @@ impl LocalBatchProver { // One thing to note: // This still allows transaction `A` to consume an unauthenticated note `x` and output note // `y` and for transaction `B` to consume an unauthenticated note `y` and output - // note `x` (i.e., have a circular dependency between transactions), but this is not - // a problem. + // note `x` (i.e., have a circular dependency between transactions). let mut output_notes = BatchOutputNoteTracker::new(transactions.iter().map(AsRef::as_ref))?; let mut input_notes = vec![]; @@ -107,6 +118,7 @@ impl LocalBatchProver { .note_inclusion_proofs() .contains_note(&input_note_header.id()) { + // Erase the note header from the input note. InputNoteCommitment::from(input_note.nullifier()) } else { input_note.clone() @@ -273,12 +285,89 @@ mod tests { .build()?; let error = LocalBatchProver::prove(ProposedBatch::new( - [tx1, tx2].into_iter().map(Arc::new).collect(), + [tx1.clone(), tx2.clone()].into_iter().map(Arc::new).collect(), NoteInclusionProofs::default(), )) .unwrap_err(); - assert_matches!(error, BatchError::DuplicateUnauthenticatedNote(id) if id == note0.id()); + assert_matches!(error, BatchError::DuplicateInputNote { + note_nullifier, + first_transaction_id, + second_transaction_id + } if note_nullifier == note0.nullifier() && + first_transaction_id == tx1.id() && + second_transaction_id == tx2.id() + ); + + Ok(()) + } + + /// Tests that an error is returned if the same authenticated input note appears multiple + /// times in different transactions. + #[test] + fn duplicate_authenticated_input_notes() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + let account2 = mock_wallet_account(100); + + let note0 = mock_note(50); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .authenticated_notes(vec![note0.clone()]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) + .authenticated_notes(vec![note0.clone()]) + .build()?; + + let error = LocalBatchProver::prove(ProposedBatch::new( + [tx1.clone(), tx2.clone()].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + )) + .unwrap_err(); + + assert_matches!(error, BatchError::DuplicateInputNote { + note_nullifier, + first_transaction_id, + second_transaction_id + } if note_nullifier == note0.nullifier() && + first_transaction_id == tx1.id() && + second_transaction_id == tx2.id() + ); + + Ok(()) + } + + /// Tests that an error is returned if the same input note appears multiple times in different + /// transactions as an unauthenticated or authenticated note. + #[test] + fn duplicate_mixed_input_notes() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + let account2 = mock_wallet_account(100); + + let note0 = mock_note(50); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .unauthenticated_notes(vec![note0.clone()]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) + .authenticated_notes(vec![note0.clone()]) + .build()?; + + let error = LocalBatchProver::prove(ProposedBatch::new( + [tx1.clone(), tx2.clone()].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + )) + .unwrap_err(); + + assert_matches!(error, BatchError::DuplicateInputNote { + note_nullifier, + first_transaction_id, + second_transaction_id + } if note_nullifier == note0.nullifier() && + first_transaction_id == tx1.id() && + second_transaction_id == tx2.id() + ); Ok(()) } diff --git a/crates/miden-batch-prover/src/testing/proven_tx_builder.rs b/crates/miden-batch-prover/src/testing/proven_tx_builder.rs index d044d5cfa..ed2a19a5b 100644 --- a/crates/miden-batch-prover/src/testing/proven_tx_builder.rs +++ b/crates/miden-batch-prover/src/testing/proven_tx_builder.rs @@ -1,10 +1,11 @@ use alloc::vec::Vec; use anyhow::Context; +use miden_crypto::merkle::MerklePath; use miden_objects::{ account::AccountId, block::BlockNumber, - note::{Note, Nullifier}, + note::{Note, NoteInclusionProof, Nullifier}, transaction::{InputNote, OutputNote, ProvenTransaction, ProvenTransactionBuilder}, vm::ExecutionProof, }; @@ -41,6 +42,21 @@ impl MockProvenTxBuilder { } } + /// Adds unauthenticated notes to the transaction. + #[must_use] + pub fn authenticated_notes(mut self, notes: Vec) -> Self { + let mock_proof = + NoteInclusionProof::new(BlockNumber::from(0), 0, MerklePath::new(vec![])).unwrap(); + self.input_notes = Some( + notes + .into_iter() + .map(|note| InputNote::authenticated(note, mock_proof.clone())) + .collect(), + ); + + self + } + /// Adds unauthenticated notes to the transaction. #[must_use] pub fn unauthenticated_notes(mut self, notes: Vec) -> Self { From dc38f1f0efccef355ab58c3e4e834a0be260b5c4 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Tue, 28 Jan 2025 14:21:16 +0100 Subject: [PATCH 22/22] feat: Add unauthenticated/authenticated scenario tests --- crates/miden-batch-prover/src/error.rs | 4 + .../src/local_batch_prover.rs | 96 ++++++++++++++++++- crates/miden-batch-prover/src/proven_batch.rs | 2 +- .../src/note/authentication_info.rs | 7 ++ 4 files changed, 105 insertions(+), 4 deletions(-) diff --git a/crates/miden-batch-prover/src/error.rs b/crates/miden-batch-prover/src/error.rs index fc362f648..b1602c406 100644 --- a/crates/miden-batch-prover/src/error.rs +++ b/crates/miden-batch-prover/src/error.rs @@ -1,5 +1,6 @@ use miden_objects::{ account::AccountId, + block::BlockNumber, note::{NoteId, Nullifier}, transaction::TransactionId, BatchAccountUpdateError, @@ -36,4 +37,7 @@ pub enum BatchError { account_id: AccountId, source: BatchAccountUpdateError, }, + + #[error("unauthenticated input note with id {note_id} for which an inclusion proof was provided was not created in block {block_num}")] + UnauthenticatedNoteAuthenticationFailed { note_id: NoteId, block_num: BlockNumber }, } diff --git a/crates/miden-batch-prover/src/local_batch_prover.rs b/crates/miden-batch-prover/src/local_batch_prover.rs index 354785201..b6655031b 100644 --- a/crates/miden-batch-prover/src/local_batch_prover.rs +++ b/crates/miden-batch-prover/src/local_batch_prover.rs @@ -6,8 +6,8 @@ use alloc::{ use miden_objects::{ account::AccountId, batch::{BatchAccountUpdate, BatchId, BatchNoteTree}, - block::BlockNumber, - note::{NoteHeader, NoteId, Nullifier}, + block::{BlockHeader, BlockNumber}, + note::{NoteHeader, NoteId, NoteInclusionProof}, transaction::{InputNoteCommitment, OutputNote, ProvenTransaction, TransactionId}, }; @@ -118,6 +118,7 @@ impl LocalBatchProver { .note_inclusion_proofs() .contains_note(&input_note_header.id()) { + // authenticate_unauthenticated_note // Erase the note header from the input note. InputNoteCommitment::from(input_note.nullifier()) } else { @@ -226,14 +227,35 @@ impl BatchOutputNoteTracker { } } +/// Validates whether the provided unauthenticated note belongs to the note tree of the specified +/// block header. +// TODO: Remove. +#[allow(dead_code)] +fn authenticate_unauthenticated_note( + note_header: &NoteHeader, + proof: &NoteInclusionProof, + block_header: &BlockHeader, +) -> Result<(), BatchError> { + let note_index = proof.location().node_index_in_block().into(); + let note_hash = note_header.hash(); + proof + .note_path() + .verify(note_index, note_hash, &block_header.note_root()) + .map_err(|_| BatchError::UnauthenticatedNoteAuthenticationFailed { + note_id: note_header.id(), + block_num: proof.location().block_num(), + }) +} + #[cfg(test)] mod tests { use alloc::sync::Arc; + use miden_crypto::merkle::MerklePath; use miden_lib::{account::wallets::BasicWallet, transaction::TransactionKernel}; use miden_objects::{ account::{Account, AccountBuilder}, - note::{Note, NoteInclusionProofs}, + note::{Note, NoteInclusionProof, NoteInclusionProofs}, testing::{account_id::AccountIdBuilder, note::NoteBuilder}, transaction::InputNote, BatchAccountUpdateError, @@ -267,6 +289,10 @@ mod tests { OutputNote::Full(mock_note(num)) } + pub fn mock_proof(node_index: u16) -> NoteInclusionProof { + NoteInclusionProof::new(BlockNumber::from(0), node_index, MerklePath::new(vec![])).unwrap() + } + /// Tests that an error is returned if the same unauthenticated input note appears multiple /// times in different transactions. #[test] @@ -444,6 +470,70 @@ mod tests { Ok(()) } + /// Test that an authenticated input note that is also created in the same batch does not error + /// and instead is marked as consumed. + /// - This requires a nullifier collision on the input and output note which is very unlikely in + /// practice. + /// - This makes the created note unspendable as its nullifier is added to the nullifier tree. + /// - The batch kernel cannot return an error in this case as it can't detect this condition due + /// to only having the nullifier for authenticated input notes _but_ not having the nullifier + /// for private output notes. + /// - We test this to ensure the kernel does something reasonable in this case and it is not an + /// attack vector. + #[test] + fn authenticated_note_created_in_same_batch() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + let account2 = mock_wallet_account(100); + + let note0 = mock_note(50); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .output_notes(vec![OutputNote::Full(note0.clone())]) + .build()?; + let tx2 = + MockProvenTxBuilder::with_account(account2.id(), Digest::default(), account2.hash()) + .authenticated_notes(vec![note0.clone()]) + .build()?; + + let batch = LocalBatchProver::prove(ProposedBatch::new( + [tx1, tx2].into_iter().map(Arc::new).collect(), + NoteInclusionProofs::default(), + ))?; + + assert_eq!(batch.input_notes().len(), 1); + assert_eq!(batch.output_notes().len(), 1); + assert_eq!(batch.output_notes_tree().num_leaves(), 1); + + Ok(()) + } + + /// Test that an unauthenticated input note for which a proof exists is converted into an + /// authenticated one and becomes part of the batch's input note commitment. + #[test] + fn unauthenticated_note_converted_authenticated() -> anyhow::Result<()> { + let account1 = mock_wallet_account(10); + + let note0 = mock_note(150); + let tx1 = + MockProvenTxBuilder::with_account(account1.id(), Digest::default(), account1.hash()) + .unauthenticated_notes(vec![note0.clone()]) + .build()?; + + let mock_note_inclusion_proofs = + NoteInclusionProofs::new(vec![], BTreeMap::from([(note0.id(), mock_proof(0))])); + + let batch = LocalBatchProver::prove(ProposedBatch::new( + [tx1].into_iter().map(Arc::new).collect(), + mock_note_inclusion_proofs, + ))?; + + // We expect the unauthenticated input note to have become an authenticated one, + // meaning it is part of the input note commitment. + assert_eq!(batch.input_notes().len(), 1); + + Ok(()) + } + /// Test that multiple transactions against the same account 1) can be correctly executed and /// 2) that an error is returned if they are incorrectly ordered. #[test] diff --git a/crates/miden-batch-prover/src/proven_batch.rs b/crates/miden-batch-prover/src/proven_batch.rs index a4d2fcc8f..e91582171 100644 --- a/crates/miden-batch-prover/src/proven_batch.rs +++ b/crates/miden-batch-prover/src/proven_batch.rs @@ -7,7 +7,7 @@ use miden_objects::{ transaction::{InputNoteCommitment, OutputNote}, }; -// TODO: Document. +/// A transaction batch with an execution proof. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ProvenBatch { id: BatchId, diff --git a/crates/miden-objects/src/note/authentication_info.rs b/crates/miden-objects/src/note/authentication_info.rs index 6cd12d51b..b3e24b7a4 100644 --- a/crates/miden-objects/src/note/authentication_info.rs +++ b/crates/miden-objects/src/note/authentication_info.rs @@ -16,6 +16,13 @@ pub struct NoteInclusionProofs { } impl NoteInclusionProofs { + pub fn new( + block_proofs: Vec, + note_proofs: BTreeMap, + ) -> Self { + Self { block_proofs, note_proofs } + } + pub fn block_proofs(&self) -> &[BlockInclusionProof] { &self.block_proofs }