diff --git a/zebra-chain/src/transaction/tests/vectors.rs b/zebra-chain/src/transaction/tests/vectors.rs index bef3cdf28..a71619b4a 100644 --- a/zebra-chain/src/transaction/tests/vectors.rs +++ b/zebra-chain/src/transaction/tests/vectors.rs @@ -32,6 +32,16 @@ lazy_static! { }; } +/// Build a mock output list for pre-V5 transactions, with (index+1) +/// copies of `output`, which is used to computed the sighash. +/// +/// Pre-V5, the entire output list is not used; only the output for the +/// given index is read. Therefore, we just need a list where `array[index]` +/// is the given `output`. +fn mock_pre_v5_output_list(output: transparent::Output, index: usize) -> Vec { + iter::repeat(output).take(index + 1).collect() +} + #[test] fn transactionhash_struct_from_str_roundtrip() { zebra_test::init(); @@ -612,7 +622,7 @@ fn test_vec143_1() -> Result<()> { &transaction, HashType::ALL, NetworkUpgrade::Overwinter, - Default::default(), + &[], None, ); @@ -641,13 +651,12 @@ fn test_vec143_2() -> Result<()> { let lock_script = Script::new(&hex::decode("53")?); let input_ind = 1; let output = transparent::Output { value, lock_script }; - let all_previous_outputs = vec![output.clone(), output]; + let all_previous_outputs = mock_pre_v5_output_list(output, input_ind); let hasher = SigHasher::new( &transaction, HashType::SINGLE, NetworkUpgrade::Overwinter, - // Pre-V5, only the matching output matters, so just use clones for the rest &all_previous_outputs, Some(input_ind), ); @@ -678,7 +687,7 @@ fn test_vec243_1() -> Result<()> { &transaction, HashType::ALL, NetworkUpgrade::Sapling, - Default::default(), + &[], None, ); @@ -717,13 +726,12 @@ fn test_vec243_2() -> Result<()> { let lock_script = Script::new(&[]); let input_ind = 1; let output = transparent::Output { value, lock_script }; - let all_previous_outputs = vec![output.clone(), output]; + let all_previous_outputs = mock_pre_v5_output_list(output, input_ind); let hasher = SigHasher::new( &transaction, HashType::NONE, NetworkUpgrade::Sapling, - // Pre-V5, only the matching output matters, so just use clones for the rest &all_previous_outputs, Some(input_ind), ); @@ -743,13 +751,13 @@ fn test_vec243_2() -> Result<()> { let lock_script = Script::new(&[]); let prevout = transparent::Output { value, lock_script }; let index = input_ind as usize; + let all_previous_outputs = mock_pre_v5_output_list(prevout, input_ind); let alt_sighash = crate::primitives::zcash_primitives::sighash( &transaction, HashType::NONE, NetworkUpgrade::Sapling, - // Pre-V5, only the matching output matters, so just use clones for the rest - &[prevout.clone(), prevout], + &all_previous_outputs, Some(index), ); let result = hex::encode(alt_sighash); @@ -826,9 +834,8 @@ fn zip143_sighash() -> Result<()> { ), None => (None, None), }; - // Pre-V5, only the matching output matters, so just use clones for the rest let all_previous_outputs: Vec<_> = match output { - Some(output) => (0..=input_index.unwrap()).map(|_| output.clone()).collect(), + Some(output) => mock_pre_v5_output_list(output, input_index.unwrap()), None => vec![], }; let result = hex::encode( @@ -863,9 +870,8 @@ fn zip243_sighash() -> Result<()> { ), None => (None, None), }; - // Pre-V5, only the matching output matters, so just use clones for the rest let all_previous_outputs: Vec<_> = match output { - Some(output) => (0..=input_index.unwrap()).map(|_| output.clone()).collect(), + Some(output) => mock_pre_v5_output_list(output, input_index.unwrap()), None => vec![], }; let result = hex::encode( @@ -908,9 +914,8 @@ fn zip244_sighash() -> Result<()> { ), None => (None, None), }; - // Pre-V5, only the matching output matters, so just use clones for the rest let all_previous_outputs: Vec<_> = match output { - Some(output) => (0..=input_index.unwrap()).map(|_| output.clone()).collect(), + Some(output) => mock_pre_v5_output_list(output, input_index.unwrap()), None => vec![], }; let result = hex::encode(transaction.sighash( @@ -955,8 +960,7 @@ fn binding_signatures_for_network(network: Network) { .. } => { if let Some(sapling_shielded_data) = sapling_shielded_data { - let shielded_sighash = - tx.sighash(upgrade, HashType::ALL, Default::default(), None); + let shielded_sighash = tx.sighash(upgrade, HashType::ALL, &[], None); let bvk = redjubjub::VerificationKey::try_from( sapling_shielded_data.binding_verification_key(), @@ -975,8 +979,7 @@ fn binding_signatures_for_network(network: Network) { .. } => { if let Some(sapling_shielded_data) = sapling_shielded_data { - let shielded_sighash = - tx.sighash(upgrade, HashType::ALL, Default::default(), None); + let shielded_sighash = tx.sighash(upgrade, HashType::ALL, &[], None); let bvk = redjubjub::VerificationKey::try_from( sapling_shielded_data.binding_verification_key(), diff --git a/zebra-consensus/src/block/tests.rs b/zebra-consensus/src/block/tests.rs index c75c3cd7c..cc7332b68 100644 --- a/zebra-consensus/src/block/tests.rs +++ b/zebra-consensus/src/block/tests.rs @@ -22,7 +22,7 @@ use zebra_chain::{ use zebra_script::CachedFfiTransaction; use zebra_test::transcript::{ExpectedTranscriptError, Transcript}; -use crate::{parameters::SLOW_START_SHIFT, script, transaction}; +use crate::{parameters::SLOW_START_SHIFT, transaction}; use super::*; @@ -125,8 +125,7 @@ async fn check_transcripts() -> Result<(), Report> { let network = Network::Mainnet; let state_service = zebra_state::init_test(network); - let script = script::Verifier::new(); - let transaction = transaction::Verifier::new(network, state_service.clone(), script); + let transaction = transaction::Verifier::new(network, state_service.clone()); let transaction = Buffer::new(BoxService::new(transaction), 1); let block_verifier = Buffer::new( BlockVerifier::new(network, state_service.clone(), transaction), diff --git a/zebra-consensus/src/chain.rs b/zebra-consensus/src/chain.rs index 675da08f8..c244184e6 100644 --- a/zebra-consensus/src/chain.rs +++ b/zebra-consensus/src/chain.rs @@ -38,7 +38,7 @@ use crate::{ block::VerifyBlockError, checkpoint::{CheckpointList, CheckpointVerifier, VerifyCheckpointError}, error::TransactionError, - script, transaction, BoxError, Config, + transaction, BoxError, Config, }; #[cfg(test)] @@ -229,8 +229,7 @@ where // transaction verification - let script = script::Verifier::new(); - let transaction = transaction::Verifier::new(network, state_service.clone(), script); + let transaction = transaction::Verifier::new(network, state_service.clone()); let transaction = Buffer::new(BoxService::new(transaction), VERIFIER_BUFFER_BOUND); // block verification diff --git a/zebra-consensus/src/script.rs b/zebra-consensus/src/script.rs index d0e6efa07..5adbc18c1 100644 --- a/zebra-consensus/src/script.rs +++ b/zebra-consensus/src/script.rs @@ -18,14 +18,8 @@ use crate::BoxError; /// The asynchronous script verification design is documented in [RFC4]. /// /// [RFC4]: https://zebra.zfnd.org/dev/rfcs/0004-asynchronous-script-verification.html -#[derive(Debug, Clone, Default)] -pub struct Verifier {} - -impl Verifier { - pub fn new() -> Self { - Self {} - } -} +#[derive(Debug, Clone, Default, Copy, PartialEq, Eq)] +pub struct Verifier; /// A script verification request. #[derive(Debug)] diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index 46b3b8ee4..2a8c8490c 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -13,7 +13,7 @@ use std::{ use chrono::{DateTime, Utc}; use futures::{ stream::{FuturesUnordered, StreamExt}, - FutureExt, TryFutureExt, + FutureExt, }; use tower::{timeout::Timeout, Service, ServiceExt}; use tracing::Instrument; @@ -73,11 +73,11 @@ where ZS::Future: Send + 'static, { /// Create a new transaction verifier. - pub fn new(network: Network, state: ZS, script_verifier: script::Verifier) -> Self { + pub fn new(network: Network, state: ZS) -> Self { Self { network, state: Timeout::new(state, UTXO_LOOKUP_TIMEOUT), - script_verifier, + script_verifier: script::Verifier::default(), } } } @@ -294,7 +294,7 @@ where // TODO: break up each chunk into its own method fn call(&mut self, req: Request) -> Self::Future { - let script_verifier = self.script_verifier.clone(); + let script_verifier = self.script_verifier; let network = self.network; let state = self.state.clone(); @@ -457,31 +457,26 @@ where let mut spent_utxos = HashMap::new(); let mut spent_outputs = Vec::new(); for input in inputs { - match input { - transparent::Input::PrevOut { - outpoint, - unlock_script: _, - sequence: _, - } => { - tracing::trace!("awaiting outpoint lookup"); - let utxo = if let Some(output) = known_utxos.get(outpoint) { - tracing::trace!("UXTO in known_utxos, discarding query"); - output.utxo.clone() + if let transparent::Input::PrevOut { outpoint, .. } = input { + tracing::trace!("awaiting outpoint lookup"); + let utxo = if let Some(output) = known_utxos.get(outpoint) { + tracing::trace!("UXTO in known_utxos, discarding query"); + output.utxo.clone() + } else { + let query = state + .clone() + .oneshot(zebra_state::Request::AwaitUtxo(*outpoint)); + if let zebra_state::Response::Utxo(utxo) = query.await? { + utxo } else { - let query = state - .clone() - .oneshot(zebra_state::Request::AwaitUtxo(*outpoint)); - if let zebra_state::Response::Utxo(utxo) = query.await? { - utxo - } else { - unreachable!("AwaitUtxo always responds with Utxo") - } - }; - tracing::trace!(?utxo, "got UTXO"); - spent_outputs.push(utxo.output.clone()); - spent_utxos.insert(*outpoint, utxo); - } - transparent::Input::Coinbase { .. } => continue, + unreachable!("AwaitUtxo always responds with Utxo") + } + }; + tracing::trace!(?utxo, "got UTXO"); + spent_outputs.push(utxo.output.clone()); + spent_utxos.insert(*outpoint, utxo); + } else { + continue; } } Ok((spent_utxos, spent_outputs)) @@ -689,7 +684,7 @@ where input_index, }; - script_verifier.clone().oneshot(request).map_ok(|_r| {}) + script_verifier.oneshot(request) }) .collect(); diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index aed7d682b..eb7a37944 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -28,7 +28,7 @@ use zebra_chain::{ use super::{check, Request, Verifier}; -use crate::{error::TransactionError, script}; +use crate::error::TransactionError; use color_eyre::eyre::Report; #[cfg(test)] @@ -249,8 +249,7 @@ async fn v5_transaction_is_rejected_before_nu5_activation() { for (network, blocks) in networks { let state_service = service_fn(|_| async { unreachable!("Service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let transaction = fake_v5_transactions_for_network(network, blocks) .rev() @@ -303,8 +302,7 @@ async fn v5_transaction_is_accepted_after_nu5_activation_for_network(network: Ne }; let state_service = service_fn(|_| async { unreachable!("Service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let transaction = fake_v5_transactions_for_network(network, blocks) .rev() @@ -365,8 +363,7 @@ async fn v4_transaction_with_transparent_transfer_is_accepted() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -412,8 +409,7 @@ async fn v4_coinbase_transaction_is_accepted() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -463,8 +459,7 @@ async fn v4_transaction_with_transparent_transfer_is_rejected_by_the_script() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -514,8 +509,7 @@ async fn v4_transaction_with_conflicting_transparent_spend_is_rejected() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -569,7 +563,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_inside_joinsplit_is_rejected }; // Sign the transaction - let sighash = transaction.sighash(network_upgrade, HashType::ALL, &Vec::new(), None); + let sighash = transaction.sighash(network_upgrade, HashType::ALL, &[], None); match &mut transaction { Transaction::V4 { @@ -581,8 +575,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_inside_joinsplit_is_rejected let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -641,7 +634,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_across_joinsplits_is_rejecte }; // Sign the transaction - let sighash = transaction.sighash(network_upgrade, HashType::ALL, &Vec::new(), None); + let sighash = transaction.sighash(network_upgrade, HashType::ALL, &[], None); match &mut transaction { Transaction::V4 { @@ -653,8 +646,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_across_joinsplits_is_rejecte let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -708,8 +700,7 @@ async fn v5_transaction_with_transparent_transfer_is_accepted() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -758,8 +749,7 @@ async fn v5_coinbase_transaction_is_accepted() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -811,8 +801,7 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -864,8 +853,7 @@ async fn v5_transaction_with_conflicting_transparent_spend_is_rejected() { let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); let result = verifier .oneshot(Request::Block { @@ -910,8 +898,7 @@ fn v4_with_signed_sprout_transfer_is_accepted() { // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier @@ -984,8 +971,7 @@ async fn v4_with_joinsplit_is_rejected_for_modification( // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier @@ -1022,8 +1008,7 @@ fn v4_with_sapling_spends() { // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier @@ -1067,8 +1052,7 @@ fn v4_with_duplicate_sapling_spends() { // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier @@ -1114,8 +1098,7 @@ fn v4_with_sapling_outputs_and_no_spends() { // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier @@ -1162,8 +1145,7 @@ fn v5_with_sapling_spends() { // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier @@ -1210,8 +1192,7 @@ fn v5_with_duplicate_sapling_spends() { // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier @@ -1274,8 +1255,7 @@ fn v5_with_duplicate_orchard_action() { // Initialize the verifier let state_service = service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = Verifier::new(network, state_service, script_verifier); + let verifier = Verifier::new(network, state_service); // Test the transaction verifier let result = verifier diff --git a/zebra-consensus/src/transaction/tests/prop.rs b/zebra-consensus/src/transaction/tests/prop.rs index f9ca93e57..c2c3d4bcf 100644 --- a/zebra-consensus/src/transaction/tests/prop.rs +++ b/zebra-consensus/src/transaction/tests/prop.rs @@ -13,7 +13,7 @@ use zebra_chain::{ }; use super::mock_transparent_transfer; -use crate::{error::TransactionError, script, transaction}; +use crate::{error::TransactionError, transaction}; /// The maximum number of transparent inputs to include in a mock transaction. const MAX_TRANSPARENT_INPUTS: usize = 10; @@ -441,8 +441,7 @@ fn validate( // Initialize the verifier let state_service = tower::service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(); - let verifier = transaction::Verifier::new(network, state_service, script_verifier); + let verifier = transaction::Verifier::new(network, state_service); // Test the transaction verifier verifier diff --git a/zebra-script/src/lib.rs b/zebra-script/src/lib.rs index 86e96d265..14566281f 100644 --- a/zebra-script/src/lib.rs +++ b/zebra-script/src/lib.rs @@ -89,23 +89,23 @@ impl CachedFfiTransaction { transaction: Arc, all_previous_outputs: Vec, ) -> Self { - let tx_to = transaction + let tx_to_serialized = transaction .zcash_serialize_to_vec() .expect("serialization into a vec is infallible"); - let tx_to_ptr = tx_to.as_ptr(); - let tx_to_len = tx_to + let tx_to_serialized_ptr = tx_to_serialized.as_ptr(); + let tx_to_serialized_len = tx_to_serialized .len() .try_into() .expect("serialized transaction lengths are much less than u32::MAX"); let mut err = 0; - let serialized_all_previous_outputs = all_previous_outputs + let all_previous_outputs_serialized = all_previous_outputs .zcash_serialize_to_vec() .expect("serialization into a vec is infallible"); // TODO: pass to zcash_script after API update - let _all_previous_outputs_ptr = serialized_all_previous_outputs.as_ptr(); - let _all_previous_outputs_len: u32 = serialized_all_previous_outputs + let _all_previous_outputs_serialized_ptr = all_previous_outputs_serialized.as_ptr(); + let _all_previous_outputs_serialized_len: u32 = all_previous_outputs_serialized .len() .try_into() .expect("serialized transaction lengths are much less than u32::MAX"); @@ -115,7 +115,8 @@ impl CachedFfiTransaction { // the `all_previous_outputs_*` fields are created from a valid Rust `Vec` let precomputed = unsafe { zcash_script::zcash_script_new_precomputed_tx( - tx_to_ptr, tx_to_len, + tx_to_serialized_ptr, + tx_to_serialized_len, // all_previous_outputs_ptr, // all_previous_outputs_len, &mut err, @@ -150,7 +151,7 @@ impl CachedFfiTransaction { } /// Verify if the script in the input at `input_index` of a transaction correctly - /// spends the matching `transparent::Output` it refers to, with the `ConsensusBranchId` + /// spends the matching [`transparent::Output`] it refers to, with the [`ConsensusBranchId`] /// of the block containing the transaction. pub fn is_valid(&self, branch_id: ConsensusBranchId, input_index: usize) -> Result<(), Error> { let previous_output = self