Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions crates/block-producer/src/batch_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{

use miden_objects::{
accounts::AccountId,
assembly::SourceManager,
notes::NoteId,
transaction::{OutputNote, TransactionId},
Digest,
Expand Down Expand Up @@ -284,8 +285,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();
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved

tokio::time::sleep(simulated_proof_time).await;
if failed {
Expand Down
32 changes: 20 additions & 12 deletions crates/block-producer/src/domain/transaction.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::BTreeSet;
use std::{collections::BTreeSet, sync::Arc};

use miden_objects::{
accounts::AccountId,
Expand All @@ -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).
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved
///
/// This does not necessarily have to match the transaction's initial state
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
}

Expand All @@ -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.
Expand Down
34 changes: 17 additions & 17 deletions crates/block-producer/src/mempool/dependency_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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));
}
Expand All @@ -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);

Expand Down Expand Up @@ -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));
}
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down
Loading