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

[Perf] Avoid tx root derivation during block generation #2577

Open
wants to merge 5 commits into
base: staging
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions console/program/src/state_path/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ pub type TransactionsTree<N> = BHPMerkleTree<N, TRANSACTIONS_DEPTH>;
/// The Merkle path for a transaction in a block.
pub type TransactionsPath<N> = MerklePath<N, TRANSACTIONS_DEPTH>;

/// The Merkle tree for the execution.
pub type ExecutionTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
/// The Merkle tree for the deployment.
pub type DeploymentTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
/// The Merkle tree for the transaction.
pub type TransactionTree<N> = BHPMerkleTree<N, TRANSACTION_DEPTH>;
/// The Merkle path for a function or transition in the transaction.
Expand Down
5 changes: 3 additions & 2 deletions ledger/block/src/transaction/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ impl<N: Network> ToBytes for Transaction<N> {
1u8.write_le(&mut writer)?;

// Write the transaction.
// We don't write the deployment or execution id, which are recomputed when creating the transaction.
match self {
Self::Deploy(id, owner, deployment, fee) => {
Self::Deploy(id, _, owner, deployment, fee) => {
// Write the variant.
0u8.write_le(&mut writer)?;
// Write the ID.
Expand All @@ -109,7 +110,7 @@ impl<N: Network> ToBytes for Transaction<N> {
// Write the fee.
fee.write_le(&mut writer)
}
Self::Execute(id, execution, fee) => {
Self::Execute(id, _, execution, fee) => {
// Write the variant.
1u8.write_le(&mut writer)?;
// Write the ID.
Expand Down
12 changes: 11 additions & 1 deletion ledger/block/src/transaction/deployment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,16 @@ impl<N: Network> Deployment<N> {
Ok(u64::try_from(self.to_bytes_le()?.len())?)
}

/// Returns the number of program functions in the deployment.
pub fn len(&self) -> usize {
Copy link
Collaborator

@ljedrz ljedrz Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is len making it obvious that this refers to the number of functions in the program? otherwise num_functions might be a better pick; the same comment applies to is_empty

Copy link
Collaborator Author

@vicsn vicsn Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed its a bit of an odd naming without context, though it is not unnatural. The Deployment contains just one vector which equals the number of functions/verifying_keys/certificates.

I mainly did this to have consistent naming with Execution::len, where it refers to the number of transitions. This PR then uses both deployment.len() and execution.len() to determine at which "index" to insert an additional Fee transition.

I'd be keen to hear more reviewer's thoughts on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I'd opt for a more precise naming for clarity. But this is fine as well.

self.program.functions().len()
}

/// Returns `true` if the deployment is empty.
pub fn is_empty(&self) -> bool {
self.program.functions().is_empty()
}

/// Returns the edition.
pub const fn edition(&self) -> u16 {
self.edition
Expand Down Expand Up @@ -161,7 +171,7 @@ impl<N: Network> Deployment<N> {

/// Returns the deployment ID.
pub fn to_deployment_id(&self) -> Result<Field<N>> {
Ok(*Transaction::deployment_tree(self, None)?.root())
Ok(*Transaction::deployment_tree(self)?.root())
}
}

Expand Down
4 changes: 2 additions & 2 deletions ledger/block/src/transaction/execution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl<N: Network> Execution<N> {

/// Returns the execution ID.
pub fn to_execution_id(&self) -> Result<Field<N>> {
Ok(*Transaction::execution_tree(self, &None)?.root())
Ok(*Transaction::execution_tree(self)?.root())
}
}

Expand Down Expand Up @@ -157,6 +157,6 @@ pub mod test_helpers {
// Retrieve a transaction.
let transaction = block.transactions().iter().next().unwrap().deref().clone();
// Retrieve the execution.
if let Transaction::Execute(_, execution, _) = transaction { execution } else { unreachable!() }
if let Transaction::Execute(_, _, execution, _) = transaction { execution } else { unreachable!() }
}
}
96 changes: 49 additions & 47 deletions ledger/block/src/transaction/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl<N: Network> Transaction<N> {
/// Returns the Merkle leaf for the given ID of a function or transition in the transaction.
pub fn to_leaf(&self, id: &Field<N>) -> Result<TransactionLeaf<N>> {
match self {
Self::Deploy(_, _, deployment, fee) => {
Self::Deploy(_, _, _, deployment, fee) => {
// Check if the ID is the transition ID for the fee.
if *id == **fee.id() {
// Return the transaction leaf.
Expand All @@ -48,7 +48,7 @@ impl<N: Network> Transaction<N> {
// Error if the function hash was not found.
bail!("Function hash not found in deployment transaction");
}
Self::Execute(_, execution, fee) => {
Self::Execute(_, _, execution, fee) => {
// Check if the ID is the transition ID for the fee.
if let Some(fee) = fee {
if *id == **fee.id() {
Expand Down Expand Up @@ -92,9 +92,19 @@ impl<N: Network> Transaction<N> {
pub fn to_tree(&self) -> Result<TransactionTree<N>> {
match self {
// Compute the deployment tree.
Transaction::Deploy(_, _, deployment, fee) => Self::deployment_tree(deployment, Some(fee)),
Transaction::Deploy(_, _, _, deployment, fee) => {
let deployment_tree = Self::deployment_tree(deployment)?;
Self::transaction_tree(deployment_tree, deployment.len(), fee)
}
// Compute the execution tree.
Transaction::Execute(_, execution, fee) => Self::execution_tree(execution, fee),
Transaction::Execute(_, _, execution, fee) => {
let execution_tree = Self::execution_tree(execution)?;
if let Some(fee) = fee {
Ok(Transaction::transaction_tree(execution_tree, execution.len(), fee)?)
} else {
Ok(execution_tree)
}
}
// Compute the fee tree.
Transaction::Fee(_, fee) => Self::fee_tree(fee),
}
Expand All @@ -103,76 +113,68 @@ impl<N: Network> Transaction<N> {

impl<N: Network> Transaction<N> {
/// Returns the Merkle tree for the given deployment.
pub fn deployment_tree(deployment: &Deployment<N>, fee: Option<&Fee<N>>) -> Result<TransactionTree<N>> {
pub fn deployment_tree(deployment: &Deployment<N>) -> Result<DeploymentTree<N>> {
// Ensure the number of leaves is within the Merkle tree size.
Self::check_deployment_size(deployment)?;
// Retrieve the program.
let program = deployment.program();
// Prepare the leaves.
let leaves = program.functions().values().enumerate().map(|(index, function)| {
// Construct the transaction leaf.
Ok(TransactionLeaf::new_deployment(
u16::try_from(index)?,
N::hash_bhp1024(&to_bits_le![program.id(), function.to_bytes_le()?])?,
)
.to_bits_le())
});
// If the fee is present, add it to the leaves.
let leaves = match fee {
Some(fee) => {
let leaves = program
.functions()
.values()
.enumerate()
.map(|(index, function)| {
// Construct the transaction leaf.
let leaf = TransactionLeaf::new_fee(
u16::try_from(program.functions().len())?, // The last index.
**fee.transition_id(),
Ok(TransactionLeaf::new_deployment(
u16::try_from(index)?,
N::hash_bhp1024(&to_bits_le![program.id(), function.to_bytes_le()?])?,
)
.to_bits_le();
// Add the leaf to the leaves.
leaves.chain([Ok(leaf)].into_iter()).collect::<Result<Vec<_>>>()?
}
None => leaves.collect::<Result<Vec<_>>>()?,
};
.to_bits_le())
})
.collect::<Result<Vec<_>>>()?;
// Compute the deployment tree.
N::merkle_tree_bhp::<TRANSACTION_DEPTH>(&leaves)
}

/// Returns the Merkle tree for the given execution.
pub fn execution_tree(execution: &Execution<N>, fee: &Option<Fee<N>>) -> Result<TransactionTree<N>> {
Self::transitions_tree(execution.transitions(), fee)
pub fn execution_tree(execution: &Execution<N>) -> Result<ExecutionTree<N>> {
Self::transitions_tree(execution.transitions())
}

/// Returns the Merkle tree for the given transitions.
pub fn transitions_tree<'a>(
transitions: impl ExactSizeIterator<Item = &'a Transition<N>>,
fee: &Option<Fee<N>>,
) -> Result<TransactionTree<N>> {
) -> Result<ExecutionTree<N>> {
// Retrieve the number of transitions.
let num_transitions = transitions.len();
// Ensure the number of leaves is within the Merkle tree size.
Self::check_execution_size(num_transitions)?;
// Prepare the leaves.
let leaves = transitions.enumerate().map(|(index, transition)| {
// Construct the transaction leaf.
Ok::<_, Error>(TransactionLeaf::new_execution(u16::try_from(index)?, **transition.id()).to_bits_le())
});
// If the fee is present, add it to the leaves.
let leaves = match fee {
Some(fee) => {
let leaves = transitions
.enumerate()
.map(|(index, transition)| {
// Construct the transaction leaf.
let leaf = TransactionLeaf::new_fee(
u16::try_from(num_transitions)?, // The last index.
**fee.transition_id(),
)
.to_bits_le();
// Add the leaf to the leaves.
leaves.chain([Ok(leaf)].into_iter()).collect::<Result<Vec<_>, _>>()?
}
None => leaves.collect::<Result<Vec<_>, _>>()?,
};

Ok::<_, Error>(TransactionLeaf::new_execution(u16::try_from(index)?, **transition.id()).to_bits_le())
})
.collect::<Result<Vec<_>, _>>()?;
// Compute the execution tree.
N::merkle_tree_bhp::<TRANSACTION_DEPTH>(&leaves)
}

/// Returns the Merkle tree for the given 1. transaction or deployment tree and 2. fee.
pub fn transaction_tree(
mut transaction_tree: TransactionTree<N>,
fee_index: usize,
fee: &Fee<N>,
) -> Result<TransactionTree<N>> {
// Construct the transaction leaf.
let leaf = TransactionLeaf::new_fee(u16::try_from(fee_index)?, **fee.transition_id()).to_bits_le();
// Compute the updated transaction tree.
transaction_tree.append(&[leaf])?;

Ok(transaction_tree)
}

/// Returns the Merkle tree for the given fee.
pub fn fee_tree(fee: &Fee<N>) -> Result<TransactionTree<N>> {
// Construct the transaction leaf.
Expand Down
Loading