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

change(rpc): Avoid re-verifying transactions in blocks if those transactions are in the mempool #8951

Draft
wants to merge 3 commits into
base: verify-orphaned-mempool-txs
Choose a base branch
from
Draft
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
13 changes: 11 additions & 2 deletions zebra-consensus/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//! verification, where it may be accepted or rejected.

use std::{
collections::HashSet,
future::Future,
pin::Pin,
sync::Arc,
Expand All @@ -25,7 +26,7 @@ use zebra_chain::{
amount::Amount,
block,
parameters::{subsidy::FundingStreamReceiver, Network},
transparent,
transaction, transparent,
work::equihash,
};
use zebra_state as zs;
Expand Down Expand Up @@ -232,13 +233,21 @@ where
&block,
&transaction_hashes,
));
for transaction in &block.transactions {

let known_outpoint_hashes: Arc<HashSet<transaction::Hash>> =
Arc::new(known_utxos.keys().map(|outpoint| outpoint.hash).collect());

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(),
height,
time: block.header.time,
Expand Down
82 changes: 81 additions & 1 deletion zebra-consensus/src/transaction.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Asynchronous verification of transactions.

use std::{
collections::HashMap,
collections::{HashMap, HashSet},
future::Future,
pin::Pin,
sync::Arc,
Expand Down Expand Up @@ -146,8 +146,12 @@ 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.
known_outpoint_hashes: Arc<HashSet<transaction::Hash>>,
/// Additional UTXOs which are known at the time of verification.
known_utxos: Arc<HashMap<transparent::OutPoint, transparent::OrderedUtxo>>,
/// The height of the block containing this transaction.
Expand Down Expand Up @@ -259,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 All @@ -267,6 +281,17 @@ impl Request {
}
}

/// The set of additional known [`transparent::OutPoint`]s of unspent transaction outputs that's in this request.
pub fn known_outpoint_hashes(&self) -> Arc<HashSet<transaction::Hash>> {
match self {
Request::Block {
known_outpoint_hashes,
..
} => known_outpoint_hashes.clone(),
Request::Mempool { .. } => HashSet::new().into(),
}
}

/// The height used to select the consensus rules for verifying this transaction.
pub fn height(&self) -> block::Height {
match self {
Expand Down Expand Up @@ -377,6 +402,16 @@ where
async move {
tracing::trace!(?tx_id, ?req, "got tx verify request");

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
});
}

// Do quick checks first
check::has_inputs_and_outputs(&tx)?;
check::has_enough_orchard_flags(&tx)?;
Expand Down Expand Up @@ -608,6 +643,51 @@ where
}
}

/// Attempts to find a transaction in the mempool by its transaction hash and checks
/// that all of its dependencies are available in the block.
///
/// Returns [`Some(Ok(VerifiedUnminedTx))`](VerifiedUnminedTx) if successful,
/// None if the transaction id was not found in the mempool,
/// or `Some(Err(TransparentInputNotFound))` if the transaction was found, but some of its
/// dependencies are missing in the block.
async fn try_find_verified_unmined_tx(
req: &Request,
mempool: Option<Timeout<Mempool>>,
) -> Option<Result<VerifiedUnminedTx, TransactionError>> {
if req.is_mempool() {
return None;
}

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

let mempool::Response::TransactionWithDeps {
transaction,
dependencies,
} = mempool
.oneshot(mempool::Request::TransactionWithDepsByMinedId(tx_id))
.await
.ok()?
else {
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));

let result = if has_all_tx_deps {
Ok(transaction)
} else {
Err(TransactionError::TransparentInputNotFound)
};

Some(result)
}

/// Wait for the UTXOs that are being spent by the given transaction.
///
/// `known_utxos` are additional UTXOs known at the time of validation (i.e.
Expand Down
Loading
Loading