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.

Sign up for GitHub

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] Cache fee verification #2580

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from
Open

[Perf] Cache fee verification #2580

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc476e6
Expand is_partially_verified usage to also cache Fee verification
vicsn Nov 28, 2024
8bf4bca
Check partially_verified_transactions cache in tests
vicsn Nov 28, 2024
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
84 changes: 71 additions & 13 deletions synthesizer/src/vm/verify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,16 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {

lap!(timer, "Check for duplicate elements");

// First, verify the fee.
self.check_fee(transaction, rejected_id)?;

// Construct the transaction checksum.
let checksum = Data::<Transaction<N>>::Buffer(transaction.to_bytes_le()?.into()).to_checksum::<N>()?;

// Check if the transaction exists in the partially-verified cache.
let is_partially_verified =
self.partially_verified_transactions.read().peek(&(transaction.id())) == Some(&checksum);

// Verify the fee.
self.check_fee(transaction, rejected_id, is_partially_verified)?;

// Next, verify the deployment or execution.
match transaction {
Transaction::Deploy(id, owner, deployment, _) => {
Expand Down Expand Up @@ -202,7 +202,12 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {

/// Verifies the `fee` in the given transaction. On failure, returns an error.
#[inline]
pub fn check_fee(&self, transaction: &Transaction<N>, rejected_id: Option<Field<N>>) -> Result<()> {
pub fn check_fee(
&self,
transaction: &Transaction<N>,
rejected_id: Option<Field<N>>,
is_partially_verified: bool,
) -> Result<()> {
match transaction {
Transaction::Deploy(id, _, deployment, fee) => {
// Ensure the rejected ID is not present.
Expand All @@ -218,7 +223,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
bail!("Transaction '{id}' has an insufficient base fee (deployment) - requires {cost} microcredits")
}
// Verify the fee.
self.check_fee_internal(fee, deployment_id)?;
self.check_fee_internal(fee, deployment_id, is_partially_verified)?;
}
Transaction::Execute(id, execution, fee) => {
// Ensure the rejected ID is not present.
Expand Down Expand Up @@ -253,7 +258,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
ensure!(*fee.base_amount()? == 0, "Transaction '{id}' has a non-zero base fee (execution)");
}
// Verify the fee.
self.check_fee_internal(fee, execution_id)?;
self.check_fee_internal(fee, execution_id, is_partially_verified)?;
} else {
// Ensure the fee can be safely skipped.
ensure!(!is_fee_required, "Transaction '{id}' is missing a fee (execution)");
Expand All @@ -265,7 +270,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
Transaction::Fee(id, fee) => {
// Verify the fee.
match rejected_id {
Some(rejected_id) => self.check_fee_internal(fee, rejected_id)?,
Some(rejected_id) => self.check_fee_internal(fee, rejected_id, is_partially_verified)?,
None => bail!("Transaction '{id}' is missing a rejected ID (fee)"),
}
}
Expand Down Expand Up @@ -339,15 +344,23 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
/// Note: This is an internal check only. To ensure all components of the fee are checked,
/// use `VM::check_fee` instead.
#[inline]
fn check_fee_internal(&self, fee: &Fee<N>, deployment_or_execution_id: Field<N>) -> Result<()> {
fn check_fee_internal(
&self,
fee: &Fee<N>,
deployment_or_execution_id: Field<N>,
is_partially_verified: bool,
) -> Result<()> {
let timer = timer!("VM::check_fee");

// Ensure the fee does not exceed the limit.
let fee_amount = fee.amount()?;
ensure!(*fee_amount <= N::MAX_FEE, "Fee verification failed: fee exceeds the maximum limit");

// Verify the fee.
let verification = self.process.read().verify_fee(fee, deployment_or_execution_id);
// Verify the fee, if it has not been partially-verified before.
let verification = match is_partially_verified {
true => Ok(()),
false => self.process.read().verify_fee(fee, deployment_or_execution_id),
};
lap!(timer, "Verify the fee");

// TODO (howardwu): This check is technically insufficient. Consider moving this upstream
Expand Down Expand Up @@ -408,16 +421,22 @@ mod tests {
let deployment_transaction = crate::vm::test_helpers::sample_deployment_transaction(rng);
// Ensure the transaction verifies.
vm.check_transaction(&deployment_transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&deployment_transaction.id()).is_some());

// Fetch an execution transaction.
let execution_transaction = crate::vm::test_helpers::sample_execution_transaction_with_private_fee(rng);
// Ensure the transaction verifies.
vm.check_transaction(&execution_transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&execution_transaction.id()).is_some());

// Fetch an execution transaction.
let execution_transaction = crate::vm::test_helpers::sample_execution_transaction_with_public_fee(rng);
// Ensure the transaction verifies.
vm.check_transaction(&execution_transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&execution_transaction.id()).is_some());
}

#[test]
Expand All @@ -433,11 +452,15 @@ mod tests {

// Ensure the deployment is valid.
vm.check_deployment_internal(&deployment, rng).unwrap();
// Ensure the partially_verified_transactions cache is not updated.
assert!(vm.partially_verified_transactions.read().peek(&deployment.to_deployment_id().unwrap()).is_none());

// Ensure that deserialization doesn't break the transaction verification.
let serialized_deployment = deployment.to_string();
let deployment_transaction: Deployment<CurrentNetwork> = serde_json::from_str(&serialized_deployment).unwrap();
vm.check_deployment_internal(&deployment_transaction, rng).unwrap();
// Ensure the partially_verified_transactions cache is not updated.
assert!(vm.partially_verified_transactions.read().peek(&deployment.to_deployment_id().unwrap()).is_none());
}

#[test]
Expand All @@ -458,12 +481,23 @@ mod tests {
assert!(execution.proof().is_some());
// Verify the execution.
vm.check_execution_internal(&execution, false).unwrap();
// Ensure the partially_verified_transactions cache is not updated.
assert!(
vm.partially_verified_transactions.read().peek(&execution.to_execution_id().unwrap()).is_none()
);

// Ensure that deserialization doesn't break the transaction verification.
let serialized_execution = execution.to_string();
let recovered_execution: Execution<CurrentNetwork> =
serde_json::from_str(&serialized_execution).unwrap();
vm.check_execution_internal(&recovered_execution, false).unwrap();
// Ensure the partially_verified_transactions cache is not updated.
assert!(
vm.partially_verified_transactions
.read()
.peek(&recovered_execution.to_execution_id().unwrap())
.is_none()
);
}
_ => panic!("Expected an execution transaction"),
}
Expand All @@ -489,12 +523,16 @@ mod tests {
// Ensure the proof exists.
assert!(fee.proof().is_some());
// Verify the fee.
vm.check_fee_internal(&fee, execution_id).unwrap();
vm.check_fee_internal(&fee, execution_id, false).unwrap();
// Ensure the partially_verified_transactions cache is not updated.
assert!(vm.partially_verified_transactions.read().peek(&execution_id).is_none());

// Ensure that deserialization doesn't break the transaction verification.
let serialized_fee = fee.to_string();
let recovered_fee: Fee<CurrentNetwork> = serde_json::from_str(&serialized_fee).unwrap();
vm.check_fee_internal(&recovered_fee, execution_id).unwrap();
vm.check_fee_internal(&recovered_fee, execution_id, false).unwrap();
// Ensure the partially_verified_transactions cache is not updated.
assert!(vm.partially_verified_transactions.read().peek(&execution_id).is_none());
}
_ => panic!("Expected an execution with a fee"),
}
Expand All @@ -515,14 +553,20 @@ mod tests {
// Fetch a valid execution transaction with a private fee.
let valid_transaction = crate::vm::test_helpers::sample_execution_transaction_with_private_fee(rng);
vm.check_transaction(&valid_transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&valid_transaction.id()).is_some());

// Fetch a valid execution transaction with a public fee.
let valid_transaction = crate::vm::test_helpers::sample_execution_transaction_with_public_fee(rng);
vm.check_transaction(&valid_transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&valid_transaction.id()).is_some());

// Fetch an valid execution transaction with no fee.
// Fetch a valid execution transaction with no fee.
let valid_transaction = crate::vm::test_helpers::sample_execution_transaction_without_fee(rng);
vm.check_transaction(&valid_transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&valid_transaction.id()).is_some());
}

#[test]
Expand Down Expand Up @@ -630,6 +674,8 @@ mod tests {

// Verify.
vm.check_transaction(&transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&transaction.id()).is_some());
}

#[test]
Expand Down Expand Up @@ -679,6 +725,8 @@ function compute:
// Fetch a valid execution transaction with a public fee.
let valid_transaction = crate::vm::test_helpers::sample_execution_transaction_with_public_fee(rng);
vm.check_transaction(&valid_transaction, None, rng).unwrap();
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&valid_transaction.id()).is_some());

// Mutate the execution transaction by inserting a Field::Zero as an output.
let execution = valid_transaction.execution().unwrap();
Expand Down Expand Up @@ -730,6 +778,8 @@ function compute:

// Ensure that the mutated transaction fails verification due to an extra output.
assert!(vm.check_transaction(&mutated_transaction, None, rng).is_err());
// Ensure the partially_verified_transactions cache is not updated.
assert!(vm.partially_verified_transactions.read().peek(&mutated_transaction.id()).is_none());
}

#[cfg(feature = "test")]
Expand Down Expand Up @@ -760,6 +810,8 @@ function compute:
minimum_credits_transfer_public_fee,
);
assert!(vm.check_transaction(&fee_too_low_transaction, None, rng).is_err());
// Ensure the partially_verified_transactions cache is not updated.
assert!(vm.partially_verified_transactions.read().peek(&fee_too_low_transaction.id()).is_none());

// Try to submit a tx with the old fee before the migration block height
let old_valid_transaction = crate::vm::test_helpers::create_new_transaction_with_different_fee(
Expand All @@ -768,6 +820,8 @@ function compute:
old_minimum_credits_transfer_public_fee,
);
assert!(vm.check_transaction(&old_valid_transaction, None, rng).is_ok());
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&old_valid_transaction.id()).is_some());

// Update the VM to the migration block height
let private_key = test_helpers::sample_genesis_private_key(rng);
Expand Down Expand Up @@ -811,6 +865,8 @@ function compute:
old_minimum_credits_transfer_public_fee,
);
assert!(vm.check_transaction(&fee_too_high_transaction, None, rng).is_ok());
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&fee_too_high_transaction.id()).is_some());

// Try to submit a tx with the new fee after the migration block height
let valid_transaction = crate::vm::test_helpers::create_new_transaction_with_different_fee(
Expand All @@ -819,5 +875,7 @@ function compute:
minimum_credits_transfer_public_fee,
);
assert!(vm.check_transaction(&valid_transaction, None, rng).is_ok());
// Ensure the partially_verified_transactions cache is updated.
assert!(vm.partially_verified_transactions.read().peek(&valid_transaction.id()).is_some());
}
}