feat(block-producer): arc transaction data (#530)

This makes `AuthenticatedTransaction` much cheaper to clone and move around.

Author: Mirko-von-Leipzig

crates/block-producer/src/batch_builder/mod.rs

@@ -9,6 +9,7 @@ use std::{
 
 use miden_objects::{
     accounts::AccountId,
+    assembly::SourceManager,
     notes::NoteId,
     transaction::{OutputNote, TransactionId},
     Digest,
@@ -278,8 +279,13 @@ impl WorkerPool {
             async move {
                 tracing::debug!("Begin proving batch.");
 
-                let transactions =
-                    transactions.into_iter().map(AuthenticatedTransaction::into_raw).collect();
+                // TODO: This is a deep clone which can be avoided by change batch building to using
+                // refs or arcs.
+                let transactions = transactions
+                    .iter()
+                    .map(AuthenticatedTransaction::raw_proven_transaction)
+                    .cloned()
+                    .collect();
 
                 tokio::time::sleep(simulated_proof_time).await;
                 if failed {

crates/block-producer/src/domain/transaction.rs

@@ -1,4 +1,4 @@
-use std::collections::BTreeSet;
+use std::{collections::BTreeSet, sync::Arc};
 
 use miden_objects::{
     accounts::AccountId,
@@ -14,10 +14,12 @@ use crate::{errors::VerifyTxError, mempool::BlockNumber, store::TransactionInput
 /// Authentication ensures that all nullifiers are unspent, and additionally authenticates some
 /// previously unauthenticated input notes.
 ///
+/// This struct is cheap to clone as it uses an Arc for the heavy data.
+///
 /// Note that this is of course only valid for the chain height of the authentication.
 #[derive(Clone, Debug, PartialEq)]
 pub struct AuthenticatedTransaction {
-    inner: ProvenTransaction,
+    inner: Arc<ProvenTransaction>,
     /// The account state provided by the store [inputs](TransactionInputs).
     ///
     /// This does not necessarily have to match the transaction's initial state
@@ -62,7 +64,7 @@ impl AuthenticatedTransaction {
             .collect();
 
         Ok(AuthenticatedTransaction {
-            inner: tx,
+            inner: Arc::new(tx),
             notes_authenticated_by_store: authenticated_notes,
             authentication_height: BlockNumber::new(inputs.current_block_height),
             store_account_state: inputs.account_hash,
@@ -107,8 +109,8 @@ impl AuthenticatedTransaction {
             .filter(|note_id| !self.notes_authenticated_by_store.contains(note_id))
     }
 
-    pub fn into_raw(self) -> ProvenTransaction {
-        self.inner
+    pub fn raw_proven_transaction(&self) -> &ProvenTransaction {
+        &self.inner
     }
 }
 
@@ -117,18 +119,24 @@ impl AuthenticatedTransaction {
     //! Builder methods intended for easier test setup.
 
     /// Short-hand for `Self::new` where the input's are setup to match the transaction's initial
-    /// account state.
+    /// account state. This covers the account's initial state and nullifiers being set to unspent.
     pub fn from_inner(inner: ProvenTransaction) -> Self {
         let store_account_state = match inner.account_update().init_state_hash() {
             zero if zero == Digest::default() => None,
             non_zero => Some(non_zero),
         };
-        Self {
-            inner,
-            store_account_state,
-            notes_authenticated_by_store: Default::default(),
-            authentication_height: Default::default(),
-        }
+        let inputs = TransactionInputs {
+            account_id: inner.account_id(),
+            account_hash: store_account_state,
+            nullifiers: inner.get_nullifiers().map(|nullifier| (nullifier, None)).collect(),
+            missing_unauthenticated_notes: inner
+                .get_unauthenticated_notes()
+                .map(|header| header.id())
+                .collect(),
+            current_block_height: Default::default(),
+        };
+        // SAFETY: nullifiers were set to None aka are definitely unspent.
+        Self::new(inner, inputs).unwrap()
     }
 
     /// Overrides the authentication height with the given value.

crates/block-producer/src/mempool/dependency_graph.rs

@@ -66,7 +66,7 @@ impl<K, V> Default for DependencyGraph<K, V> {
     }
 }
 
-impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
+impl<K: Ord + Copy, V: Clone> DependencyGraph<K, V> {
     /// Inserts a new node into the graph.
     ///
     /// # Errors
@@ -82,19 +82,19 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
         let missing_parents = parents
             .iter()
             .filter(|parent| !self.vertices.contains_key(parent))
-            .cloned()
+            .copied()
             .collect::<BTreeSet<_>>();
         if !missing_parents.is_empty() {
             return Err(GraphError::MissingParents(missing_parents));
         }
 
         // Inform parents of their new child.
         for parent in &parents {
-            self.children.entry(parent.clone()).or_default().insert(key.clone());
+            self.children.entry(*parent).or_default().insert(key);
         }
-        self.vertices.insert(key.clone(), value);
-        self.parents.insert(key.clone(), parents);
-        self.children.insert(key.clone(), Default::default());
+        self.vertices.insert(key, value);
+        self.parents.insert(key, parents);
+        self.children.insert(key, Default::default());
 
         self.try_make_root(key);
 
@@ -115,12 +115,12 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
         let missing_nodes = keys
             .iter()
             .filter(|key| !self.vertices.contains_key(key))
-            .cloned()
+            .copied()
             .collect::<BTreeSet<_>>();
         if !missing_nodes.is_empty() {
             return Err(GraphError::UnknownNodes(missing_nodes));
         }
-        let unprocessed = keys.difference(&self.processed).cloned().collect::<BTreeSet<_>>();
+        let unprocessed = keys.difference(&self.processed).copied().collect::<BTreeSet<_>>();
         if !unprocessed.is_empty() {
             return Err(GraphError::UnprocessedNodes(unprocessed));
         }
@@ -137,7 +137,7 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
                 .map(|children| children.difference(&reverted))
                 .into_iter()
                 .flatten()
-                .cloned();
+                .copied();
 
             to_revert.extend(unprocessed_children);
 
@@ -176,13 +176,13 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
         let missing_nodes = keys
             .iter()
             .filter(|key| !self.vertices.contains_key(key))
-            .cloned()
+            .copied()
             .collect::<BTreeSet<_>>();
         if !missing_nodes.is_empty() {
             return Err(GraphError::UnknownNodes(missing_nodes));
         }
 
-        let unprocessed = keys.difference(&self.processed).cloned().collect::<BTreeSet<_>>();
+        let unprocessed = keys.difference(&self.processed).copied().collect::<BTreeSet<_>>();
         if !unprocessed.is_empty() {
             return Err(GraphError::UnprocessedNodes(unprocessed));
         }
@@ -193,7 +193,7 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
             .flat_map(|key| self.parents.get(key))
             .flatten()
             .filter(|parent| !keys.contains(parent))
-            .cloned()
+            .copied()
             .collect::<BTreeSet<_>>();
         if !dangling.is_empty() {
             return Err(GraphError::DanglingNodes(dangling));
@@ -236,7 +236,7 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
         let missing_nodes = keys
             .iter()
             .filter(|key| !self.vertices.contains_key(key))
-            .cloned()
+            .copied()
             .collect::<BTreeSet<_>>();
         if !missing_nodes.is_empty() {
             return Err(GraphError::UnknownNodes(missing_nodes));
@@ -251,15 +251,15 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
                 .vertices
                 .remove(&key)
                 .expect("Node was checked in precondition and must therefore exist");
-            removed.insert(key.clone(), value);
+            removed.insert(key, value);
 
             self.processed.remove(&key);
             self.roots.remove(&key);
 
             // Children must also be purged. Take care not to visit them twice which is
             // possible since children can have multiple purged parents.
             let unvisited_children = self.children.remove(&key).unwrap_or_default();
-            let unvisited_children = unvisited_children.difference(&visited).cloned();
+            let unvisited_children = unvisited_children.difference(&visited);
             to_remove.extend(unvisited_children);
 
             // Inform parents that this child no longer exists.
@@ -315,7 +315,7 @@ impl<K: Ord + Clone, V: Clone> DependencyGraph<K, V> {
             return Err(GraphError::NotARootNode(key));
         }
 
-        self.processed.insert(key.clone());
+        self.processed.insert(key);
 
         self.children
             .get(&key)
@@ -374,7 +374,7 @@ mod tests {
 
         /// Calls process_root until all nodes have been processed.
         fn process_all(&mut self) {
-            while let Some(root) = self.roots().first().cloned() {
+            while let Some(root) = self.roots().first().copied() {
                 /// SAFETY: this is definitely a root since we just took it from there :)
                 self.process_root(root);
             }