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 3 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
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
10 changes: 10 additions & 0 deletions 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
Contributor 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.

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
2 changes: 1 addition & 1 deletion ledger/block/src/transaction/execution/mod.rs
Original file line number Diff line number Diff line change
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!() }
}
}
22 changes: 18 additions & 4 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,9 @@ 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) => Self::deployment_tree(deployment, Some(fee)),
// Compute the execution tree.
Transaction::Execute(_, execution, fee) => Self::execution_tree(execution, fee),
Transaction::Execute(_, _, execution, fee) => Self::execution_tree(execution, fee),
// Compute the fee tree.
Transaction::Fee(_, fee) => Self::fee_tree(fee),
}
Expand Down Expand Up @@ -173,6 +173,20 @@ impl<N: Network> Transaction<N> {
N::merkle_tree_bhp::<TRANSACTION_DEPTH>(&leaves)
}

/// Returns the Merkle tree for the given execution tree and fee.
pub fn transaction_tree(
mut execution_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 transaction tree.
execution_tree.append(&[leaf])?;

Ok(execution_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
115 changes: 83 additions & 32 deletions ledger/block/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,15 @@ use console::{
types::{Field, Group, U64},
};

type DeploymentID<N> = Field<N>;
type ExecutionID<N> = Field<N>;

#[derive(Clone, PartialEq, Eq)]
pub enum Transaction<N: Network> {
/// The deploy transaction publishes an Aleo program to the network.
Deploy(N::TransactionID, ProgramOwner<N>, Box<Deployment<N>>, Fee<N>),
Deploy(N::TransactionID, DeploymentID<N>, ProgramOwner<N>, Box<Deployment<N>>, Fee<N>),
/// The execute transaction represents a call to an Aleo program.
Execute(N::TransactionID, Execution<N>, Option<Fee<N>>),
Execute(N::TransactionID, ExecutionID<N>, Execution<N>, Option<Fee<N>>),
/// The fee transaction represents a fee paid to the network, used for rejected transactions.
Fee(N::TransactionID, Fee<N>),
}
Expand All @@ -49,24 +52,36 @@ impl<N: Network> Transaction<N> {
pub fn from_deployment(owner: ProgramOwner<N>, deployment: Deployment<N>, fee: Fee<N>) -> Result<Self> {
// Ensure the transaction is not empty.
ensure!(!deployment.program().functions().is_empty(), "Attempted to create an empty deployment transaction");
// Compute the transaction ID.
let id = *Self::deployment_tree(&deployment, Some(&fee))?.root();
// Compute the deployment tree.
let deployment_tree = Self::deployment_tree(&deployment, None)?;
// Compute the deployment ID.
let deployment_id = deployment.to_deployment_id()?;
let deployment_id = *deployment_tree.root();
// Compute the transaction ID
let transaction_id = *Self::transaction_tree(deployment_tree, deployment.len(), &fee)?.root();
// Ensure the owner signed the correct transaction ID.
ensure!(owner.verify(deployment_id), "Attempted to create a deployment transaction with an invalid owner");
// Construct the deployment transaction.
Ok(Self::Deploy(id.into(), owner, Box::new(deployment), fee))
Ok(Self::Deploy(transaction_id.into(), deployment_id, owner, Box::new(deployment), fee))
}

/// Initializes a new execution transaction.
pub fn from_execution(execution: Execution<N>, fee: Option<Fee<N>>) -> Result<Self> {
// Ensure the transaction is not empty.
ensure!(!execution.is_empty(), "Attempted to create an empty execution transaction");
// Compute the transaction ID.
let id = *Self::execution_tree(&execution, &fee)?.root();
// Compute the execution tree.
let execution_tree = Self::execution_tree(&execution, &None)?;
// Compute the execution ID.
let execution_id = *execution_tree.root();
// Compute the transaction ID
let transaction_id = match &fee {
Some(fee) => {
// Compute the root of the transacton tree.
*Self::transaction_tree(execution_tree, execution.len(), fee)?.root()
}
None => execution_id,
};
// Construct the execution transaction.
Ok(Self::Execute(id.into(), execution, fee))
Ok(Self::Execute(transaction_id.into(), execution_id, execution, fee))
}

/// Initializes a new fee transaction.
Expand Down Expand Up @@ -106,7 +121,7 @@ impl<N: Network> Transaction<N> {
pub fn contains_split(&self) -> bool {
match self {
// Case 1 - The transaction contains a transition that calls 'credits.aleo/split'.
Transaction::Execute(_, execution, _) => execution.transitions().any(|transition| transition.is_split()),
Transaction::Execute(_, _, execution, _) => execution.transitions().any(|transition| transition.is_split()),
// Otherwise, return 'false'.
_ => false,
}
Expand All @@ -118,7 +133,7 @@ impl<N: Network> Transaction<N> {
#[inline]
pub fn owner(&self) -> Option<&ProgramOwner<N>> {
match self {
Self::Deploy(_, owner, _, _) => Some(owner),
Self::Deploy(_, _, owner, _, _) => Some(owner),
_ => None,
}
}
Expand All @@ -127,7 +142,7 @@ impl<N: Network> Transaction<N> {
#[inline]
pub fn deployment(&self) -> Option<&Deployment<N>> {
match self {
Self::Deploy(_, _, deployment, _) => Some(deployment.as_ref()),
Self::Deploy(_, _, _, deployment, _) => Some(deployment.as_ref()),
_ => None,
}
}
Expand All @@ -136,7 +151,7 @@ impl<N: Network> Transaction<N> {
#[inline]
pub fn execution(&self) -> Option<&Execution<N>> {
match self {
Self::Execute(_, execution, _) => Some(execution),
Self::Execute(_, _, execution, _) => Some(execution),
_ => None,
}
}
Expand Down Expand Up @@ -186,38 +201,38 @@ impl<N: Network> Transaction<N> {
/// Returns the transaction total fee.
pub fn fee_amount(&self) -> Result<U64<N>> {
match self {
Self::Deploy(_, _, _, fee) => fee.amount(),
Self::Execute(_, _, Some(fee)) => fee.amount(),
Self::Execute(_, _, None) => Ok(U64::zero()),
Self::Deploy(_, _, _, _, fee) => fee.amount(),
Self::Execute(_, _, _, Some(fee)) => fee.amount(),
Self::Execute(_, _, _, None) => Ok(U64::zero()),
Self::Fee(_, fee) => fee.amount(),
}
}

/// Returns the transaction base fee.
pub fn base_fee_amount(&self) -> Result<U64<N>> {
match self {
Self::Deploy(_, _, _, fee) => fee.base_amount(),
Self::Execute(_, _, Some(fee)) => fee.base_amount(),
Self::Execute(_, _, None) => Ok(U64::zero()),
Self::Deploy(_, _, _, _, fee) => fee.base_amount(),
Self::Execute(_, _, _, Some(fee)) => fee.base_amount(),
Self::Execute(_, _, _, None) => Ok(U64::zero()),
Self::Fee(_, fee) => fee.base_amount(),
}
}

/// Returns the transaction priority fee.
pub fn priority_fee_amount(&self) -> Result<U64<N>> {
match self {
Self::Deploy(_, _, _, fee) => fee.priority_amount(),
Self::Execute(_, _, Some(fee)) => fee.priority_amount(),
Self::Execute(_, _, None) => Ok(U64::zero()),
Self::Deploy(_, _, _, _, fee) => fee.priority_amount(),
Self::Execute(_, _, _, Some(fee)) => fee.priority_amount(),
Self::Execute(_, _, _, None) => Ok(U64::zero()),
Self::Fee(_, fee) => fee.priority_amount(),
}
}

/// Returns the fee transition.
pub fn fee_transition(&self) -> Option<Fee<N>> {
match self {
Self::Deploy(_, _, _, fee) => Some(fee.clone()),
Self::Execute(_, _, fee) => fee.clone(),
Self::Deploy(_, _, _, _, fee) => Some(fee.clone()),
Self::Execute(_, _, _, fee) => fee.clone(),
Self::Fee(_, fee) => Some(fee.clone()),
}
}
Expand All @@ -228,9 +243,9 @@ impl<N: Network> Transaction<N> {
pub fn contains_transition(&self, transition_id: &N::TransitionID) -> bool {
match self {
// Check the fee.
Self::Deploy(_, _, _, fee) => fee.id() == transition_id,
Self::Deploy(_, _, _, _, fee) => fee.id() == transition_id,
// Check the execution and fee.
Self::Execute(_, execution, fee) => {
Self::Execute(_, _, execution, fee) => {
execution.contains_transition(transition_id)
|| fee.as_ref().map_or(false, |fee| fee.id() == transition_id)
}
Expand All @@ -255,12 +270,12 @@ impl<N: Network> Transaction<N> {
pub fn find_transition(&self, transition_id: &N::TransitionID) -> Option<&Transition<N>> {
match self {
// Check the fee.
Self::Deploy(_, _, _, fee) => match fee.id() == transition_id {
Self::Deploy(_, _, _, _, fee) => match fee.id() == transition_id {
true => Some(fee.transition()),
false => None,
},
// Check the execution and fee.
Self::Execute(_, execution, fee) => execution.get_transition(transition_id).or_else(|| {
Self::Execute(_, _, execution, fee) => execution.get_transition(transition_id).or_else(|| {
fee.as_ref().and_then(|fee| match fee.id() == transition_id {
true => Some(fee.transition()),
false => None,
Expand Down Expand Up @@ -299,8 +314,8 @@ impl<N: Network> Transaction<N> {
/// Returns an iterator over all transitions.
pub fn transitions(&self) -> impl '_ + DoubleEndedIterator<Item = &Transition<N>> {
match self {
Self::Deploy(_, _, _, fee) => IterWrap::Deploy(Some(fee.transition()).into_iter()),
Self::Execute(_, execution, fee) => {
Self::Deploy(_, _, _, _, fee) => IterWrap::Deploy(Some(fee.transition()).into_iter()),
Self::Execute(_, _, execution, fee) => {
IterWrap::Execute(execution.transitions().chain(fee.as_ref().map(|fee| fee.transition())))
}
Self::Fee(_, fee) => IterWrap::Fee(Some(fee.transition()).into_iter()),
Expand Down Expand Up @@ -366,8 +381,8 @@ impl<N: Network> Transaction<N> {
/// Returns a consuming iterator over all transitions.
pub fn into_transitions(self) -> impl DoubleEndedIterator<Item = Transition<N>> {
match self {
Self::Deploy(_, _, _, fee) => IterWrap::Deploy(Some(fee.into_transition()).into_iter()),
Self::Execute(_, execution, fee) => {
Self::Deploy(_, _, _, _, fee) => IterWrap::Deploy(Some(fee.into_transition()).into_iter()),
Self::Execute(_, _, execution, fee) => {
IterWrap::Execute(execution.into_transitions().chain(fee.map(|fee| fee.into_transition())))
}
Self::Fee(_, fee) => IterWrap::Fee(Some(fee.into_transition()).into_iter()),
Expand Down Expand Up @@ -470,3 +485,39 @@ pub mod test_helpers {
Transaction::from_fee(fee).unwrap()
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_transaction_id() -> Result<()> {
let rng = &mut TestRng::default();

// Transaction IDs are created using `transaction_tree`.
for expected in [
crate::transaction::test_helpers::sample_deployment_transaction(true, rng),
crate::transaction::test_helpers::sample_deployment_transaction(false, rng),
crate::transaction::test_helpers::sample_execution_transaction_with_fee(true, rng),
crate::transaction::test_helpers::sample_execution_transaction_with_fee(false, rng),
]
.into_iter()
{
match expected {
// Compare against transaction IDs created using `deployment_tree`.
Transaction::Deploy(_, id, _, deployment, fee) => {
let computed_id = *Transaction::deployment_tree(&deployment, Some(&fee))?.root();
assert_eq!(computed_id, id);
}
// Compare against transaction IDs created using `execution_tree`.
Transaction::Execute(_, id, execution, fee) => {
let computed_id = *Transaction::execution_tree(&execution, &fee)?.root();
assert_eq!(computed_id, id);
}
_ => panic!("Unexpected test case."),
};
}

Ok(())
}
}
5 changes: 3 additions & 2 deletions ledger/block/src/transaction/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ impl<N: Network> Serialize for Transaction<N> {
/// Serializes the transaction to a JSON-string or buffer.
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
match serializer.is_human_readable() {
// We don't write the deployment or execution id, which are recomputed when creating the Transaction.
true => match self {
Self::Deploy(id, owner, deployment, fee) => {
Self::Deploy(id, _, owner, deployment, fee) => {
let mut transaction = serializer.serialize_struct("Transaction", 5)?;
transaction.serialize_field("type", "deploy")?;
transaction.serialize_field("id", &id)?;
Expand All @@ -29,7 +30,7 @@ impl<N: Network> Serialize for Transaction<N> {
transaction.serialize_field("fee", &fee)?;
transaction.end()
}
Self::Execute(id, execution, fee) => {
Self::Execute(id, _, execution, fee) => {
let mut transaction = serializer.serialize_struct("Transaction", 3 + fee.is_some() as usize)?;
transaction.serialize_field("type", "execute")?;
transaction.serialize_field("id", &id)?;
Expand Down
2 changes: 1 addition & 1 deletion ledger/block/src/transactions/confirmed/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl<N: Network> ConfirmedTransaction<N> {
) -> Result<Self> {
// Retrieve the program and fee from the deployment transaction, and ensure the transaction is a deploy transaction.
let (program, fee) = match &transaction {
Transaction::Deploy(_, _, deployment, fee) => (deployment.program(), fee),
Transaction::Deploy(_, _, _, deployment, fee) => (deployment.program(), fee),
Transaction::Execute(..) | Transaction::Fee(..) => {
bail!("Transaction '{}' is not a deploy transaction", transaction.id())
}
Expand Down
4 changes: 2 additions & 2 deletions ledger/block/src/transactions/rejected/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub mod test_helpers {
pub(crate) fn sample_rejected_deployment(is_fee_private: bool, rng: &mut TestRng) -> Rejected<CurrentNetwork> {
// Sample a deploy transaction.
let deployment = match crate::transaction::test_helpers::sample_deployment_transaction(is_fee_private, rng) {
Transaction::Deploy(_, _, deployment, _) => (*deployment).clone(),
Transaction::Deploy(_, _, _, deployment, _) => (*deployment).clone(),
_ => unreachable!(),
};

Expand All @@ -121,7 +121,7 @@ pub mod test_helpers {
// Sample an execute transaction.
let execution =
match crate::transaction::test_helpers::sample_execution_transaction_with_fee(is_fee_private, rng) {
Transaction::Execute(_, execution, _) => execution,
Transaction::Execute(_, _, execution, _) => execution,
_ => unreachable!(),
};

Expand Down
2 changes: 1 addition & 1 deletion ledger/block/src/transition/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ pub mod test_helpers {

/// Samples a random transition.
pub(crate) fn sample_transition(rng: &mut TestRng) -> Transition<CurrentNetwork> {
if let Transaction::Execute(_, execution, _) =
if let Transaction::Execute(_, _, execution, _) =
crate::transaction::test_helpers::sample_execution_transaction_with_fee(true, rng)
{
execution.into_transitions().next().unwrap()
Expand Down
2 changes: 1 addition & 1 deletion ledger/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ finalize failed_assert:
assert_eq!(next_block.transactions().len(), 1);
let confirmed_transaction = next_block.transactions().iter().next().unwrap();
assert!(confirmed_transaction.is_rejected());
if let Transaction::Execute(_, execution, fee) = failed_assert_transaction {
if let Transaction::Execute(_, _, execution, fee) = failed_assert_transaction {
let fee_transaction = Transaction::from_fee(fee.unwrap()).unwrap();
let expected_confirmed_transaction =
ConfirmedTransaction::RejectedExecute(0, fee_transaction, Rejected::new_execution(execution), vec![]);
Expand Down
Loading