zecfoundation
/
zebra
mirror of https://github.com/ZcashFoundation/zebra.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

refactor: address comments from #3415: Prepare for changes in ZIP-244 (#3446) 

* refactor: address comments from #3415

* Shorter `if let` match

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
Conrado Gouvea 2022-02-01 03:24:08 -03:00 committed by GitHub
parent 07f120a21a
commit 494b7dc9f4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 85 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
zebra-chain/src/transaction/tests/vectors.rs
View File

@ -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<transparent::Output> {
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(),

5
zebra-consensus/src/block/tests.rs
View File

@ -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),

5
zebra-consensus/src/chain.rs
View File

@ -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

10
zebra-consensus/src/script.rs
View File

@ -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)]

53
zebra-consensus/src/transaction.rs
View File

@ -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();

66
zebra-consensus/src/transaction/tests.rs
View File

@ -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

5
zebra-consensus/src/transaction/tests/prop.rs
View File

@ -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

17
zebra-script/src/lib.rs
View File

@ -89,23 +89,23 @@ impl CachedFfiTransaction {
transaction: Arc<Transaction>,
all_previous_outputs: Vec<transparent::Output>,
) -> 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