Skip to content

Commit

Permalink
Block verifier now passes transaction ids to the tx verifier
Browse files Browse the repository at this point in the history
  • Loading branch information
arya2 committed Oct 19, 2024
1 parent ee23d3d commit 3633bb6
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 11 deletions.
5 changes: 4 additions & 1 deletion zebra-consensus/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,15 @@ where
let known_outpoint_hashes: Arc<HashSet<transaction::Hash>> =
Arc::new(known_utxos.keys().map(|outpoint| outpoint.hash).collect());

for transaction in &block.transactions {
for (&transaction_hash, transaction) in
transaction_hashes.iter().zip(block.transactions.iter())
{
let rsp = transaction_verifier
.ready()
.await
.expect("transaction verifier is always ready")
.call(tx::Request::Block {
transaction_hash,
transaction: transaction.clone(),
known_outpoint_hashes: known_outpoint_hashes.clone(),
known_utxos: known_utxos.clone(),
Expand Down
32 changes: 22 additions & 10 deletions zebra-consensus/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ where
pub enum Request {
/// Verify the supplied transaction as part of a block.
Block {
/// The transaction hash.
transaction_hash: transaction::Hash,
/// The transaction itself.
transaction: Arc<Transaction>,
/// Set of transaction hashes that create new transparent outputs.
Expand Down Expand Up @@ -261,6 +263,16 @@ impl Request {
}
}

/// The mined transaction ID for the transaction in this request.
pub fn tx_mined_id(&self) -> transaction::Hash {
match self {
Request::Block {
transaction_hash, ..
} => *transaction_hash,
Request::Mempool { transaction, .. } => transaction.id.mined_id(),
}
}

/// The set of additional known unspent transaction outputs that's in this request.
pub fn known_utxos(&self) -> Arc<HashMap<transparent::OutPoint, transparent::OrderedUtxo>> {
match self {
Expand Down Expand Up @@ -390,16 +402,14 @@ where
async move {
tracing::trace!(?tx_id, ?req, "got tx verify request");

if !req.is_mempool() {
if let Some(result) = Self::try_find_verified_unmined_tx(&req, mempool.clone()).await {
let verified_tx = result?;
if let Some(result) = Self::try_find_verified_unmined_tx(&req, mempool.clone()).await {
let verified_tx = result?;

return Ok(Response::Block {
tx_id: verified_tx.transaction.id,
miner_fee: Some(verified_tx.miner_fee),
legacy_sigop_count: verified_tx.legacy_sigop_count
});
}
return Ok(Response::Block {
tx_id: verified_tx.transaction.id,
miner_fee: Some(verified_tx.miner_fee),
legacy_sigop_count: verified_tx.legacy_sigop_count
});
}

// Do quick checks first
Expand Down Expand Up @@ -650,7 +660,7 @@ where

let mut mempool = mempool?;
let known_outpoint_hashes = req.known_outpoint_hashes();
let tx_id = req.transaction().hash();
let tx_id = req.tx_mined_id();

let mempool::Response::TransactionWithDeps {
transaction,
Expand All @@ -666,6 +676,8 @@ where
panic!("unexpected response to TransactionWithDepsByMinedId request");
};

// Note: This does not verify that the spends are in order, this should be
// done during contextual validation in zebra-state.
let has_all_tx_deps = dependencies
.into_iter()
.all(|dependency_id| known_outpoint_hashes.contains(&dependency_id));
Expand Down
32 changes: 32 additions & 0 deletions zebra-consensus/src/transaction/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,7 @@ async fn v5_transaction_is_rejected_before_nu5_activation() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1046,6 +1047,7 @@ fn v5_transaction_is_accepted_after_nu5_activation_for_network(network: Network)

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1102,6 +1104,7 @@ async fn v4_transaction_with_transparent_transfer_is_accepted() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1147,6 +1150,7 @@ async fn v4_transaction_with_last_valid_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1193,6 +1197,7 @@ async fn v4_coinbase_transaction_with_low_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1241,6 +1246,7 @@ async fn v4_transaction_with_too_low_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1292,6 +1298,7 @@ async fn v4_transaction_with_exceeding_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1346,6 +1353,7 @@ async fn v4_coinbase_transaction_with_exceeding_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1398,6 +1406,7 @@ async fn v4_coinbase_transaction_is_accepted() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1454,6 +1463,7 @@ async fn v4_transaction_with_transparent_transfer_is_rejected_by_the_script() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1510,6 +1520,7 @@ async fn v4_transaction_with_conflicting_transparent_spend_is_rejected() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1582,6 +1593,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_inside_joinsplit_is_rejected

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1659,6 +1671,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_across_joinsplits_is_rejecte

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1719,6 +1732,7 @@ async fn v5_transaction_with_transparent_transfer_is_accepted() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1766,6 +1780,7 @@ async fn v5_transaction_with_last_valid_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1812,6 +1827,7 @@ async fn v5_coinbase_transaction_expiry_height() {
let result = verifier
.clone()
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand All @@ -1834,6 +1850,7 @@ async fn v5_coinbase_transaction_expiry_height() {
let result = verifier
.clone()
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(new_transaction.clone()),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1864,6 +1881,7 @@ async fn v5_coinbase_transaction_expiry_height() {
let result = verifier
.clone()
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(new_transaction.clone()),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1896,6 +1914,7 @@ async fn v5_coinbase_transaction_expiry_height() {
let result = verifier
.clone()
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(new_transaction.clone()),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1946,6 +1965,7 @@ async fn v5_transaction_with_too_low_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -1998,6 +2018,7 @@ async fn v5_transaction_with_exceeding_expiry_height() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction.clone()),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2053,6 +2074,7 @@ async fn v5_coinbase_transaction_is_accepted() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2111,6 +2133,7 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2169,6 +2192,7 @@ async fn v5_transaction_with_conflicting_transparent_spend_is_rejected() {

let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2214,6 +2238,7 @@ fn v4_with_signed_sprout_transfer_is_accepted() {
// Test the transaction verifier
let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction,
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2304,6 +2329,7 @@ async fn v4_with_joinsplit_is_rejected_for_modification(
let result = verifier
.clone()
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: transaction.clone(),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2351,6 +2377,7 @@ fn v4_with_sapling_spends() {
// Test the transaction verifier
let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction,
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2394,6 +2421,7 @@ fn v4_with_duplicate_sapling_spends() {
// Test the transaction verifier
let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction,
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2439,6 +2467,7 @@ fn v4_with_sapling_outputs_and_no_spends() {
// Test the transaction verifier
let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction,
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2488,6 +2517,7 @@ fn v5_with_sapling_spends() {
// Test the transaction verifier
let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2532,6 +2562,7 @@ fn v5_with_duplicate_sapling_spends() {
// Test the transaction verifier
let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down Expand Up @@ -2595,6 +2626,7 @@ fn v5_with_duplicate_orchard_action() {
// Test the transaction verifier
let result = verifier
.oneshot(Request::Block {
transaction_hash: transaction.hash(),
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down
2 changes: 2 additions & 0 deletions zebra-consensus/src/transaction/tests/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,11 +455,13 @@ fn validate(
tower::service_fn(|_| async { unreachable!("State service should not be called") });
let verifier = transaction::Verifier::new_for_tests(&network, state_service);
let verifier = Buffer::new(verifier, 10);
let transaction_hash = transaction.hash();

// Test the transaction verifier
verifier
.clone()
.oneshot(transaction::Request::Block {
transaction_hash,
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
known_outpoint_hashes: Arc::new(HashSet::new()),
Expand Down

0 comments on commit 3633bb6

Please sign in to comment.