diff --git a/zebra-chain/src/transparent.rs b/zebra-chain/src/transparent.rs index 7395a2ba..7534651e 100644 --- a/zebra-chain/src/transparent.rs +++ b/zebra-chain/src/transparent.rs @@ -9,7 +9,7 @@ mod utxo; pub use address::Address; pub use script::Script; -pub use utxo::{new_outputs, Utxo}; +pub use utxo::{new_ordered_outputs, new_outputs, utxos_from_ordered_utxos, OrderedUtxo, Utxo}; #[cfg(any(test, feature = "proptest-impl"))] use proptest_derive::Arbitrary; diff --git a/zebra-chain/src/transparent/utxo.rs b/zebra-chain/src/transparent/utxo.rs index dbab18c1..cb652d50 100644 --- a/zebra-chain/src/transparent/utxo.rs +++ b/zebra-chain/src/transparent/utxo.rs @@ -22,32 +22,94 @@ pub struct Utxo { pub from_coinbase: bool, } -/// Compute an index of newly created transparent outputs, given a block and a +/// A [`Utxo`], and the index of its transaction within its block. +/// +/// This extra index is used to check that spends come after outputs, +/// when a new output and its spend are both in the same block. +/// +/// The extra index is only used during block verification, +/// so it does not need to be sent to the state. +#[derive(Clone, Debug, PartialEq, Eq)] +#[cfg_attr( + any(test, feature = "proptest-impl"), + derive(proptest_derive::Arbitrary) +)] +pub struct OrderedUtxo { + /// An unspent transaction output. + pub utxo: Utxo, + /// The index of the transaction that created the output, in the block at `height`. + /// + /// Used to make sure that transaction can only spend outputs + /// that were created earlier in the chain. + /// + /// Note: this is different from `OutPoint.index`, + /// which is the index of the output in its transaction. + pub tx_index_in_block: usize, +} + +impl OrderedUtxo { + /// Create a new ordered UTXO from its fields. + pub fn new( + output: transparent::Output, + height: block::Height, + from_coinbase: bool, + tx_index_in_block: usize, + ) -> OrderedUtxo { + let utxo = Utxo { + output, + height, + from_coinbase, + }; + + OrderedUtxo { + utxo, + tx_index_in_block, + } + } +} + +/// Compute an index of [`Utxo`]s, given an index of [`OrderedUtxo`]s. +pub fn utxos_from_ordered_utxos( + ordered_utxos: HashMap, +) -> HashMap { + ordered_utxos + .into_iter() + .map(|(out_point, ordered_utxo)| (out_point, ordered_utxo.utxo)) + .collect() +} + +/// Compute an index of newly created [`Utxo`]s, given a block and a /// list of precomputed transaction hashes. pub fn new_outputs( block: &Block, transaction_hashes: &[transaction::Hash], ) -> HashMap { - let mut new_outputs = HashMap::default(); + utxos_from_ordered_utxos(new_ordered_outputs(block, transaction_hashes)) +} + +/// Compute an index of newly created [`OrderedUtxo`]s, given a block and a +/// list of precomputed transaction hashes. +pub fn new_ordered_outputs( + block: &Block, + transaction_hashes: &[transaction::Hash], +) -> HashMap { + let mut new_ordered_outputs = HashMap::default(); let height = block.coinbase_height().expect("block has coinbase height"); - for (transaction, hash) in block + for (tx_index_in_block, (transaction, hash)) in block .transactions .iter() .zip(transaction_hashes.iter().cloned()) + .enumerate() { let from_coinbase = transaction.is_coinbase(); for (index, output) in transaction.outputs().iter().cloned().enumerate() { let index = index as u32; - new_outputs.insert( + new_ordered_outputs.insert( transparent::OutPoint { hash, index }, - Utxo { - output, - height, - from_coinbase, - }, + OrderedUtxo::new(output, height, from_coinbase, tx_index_in_block), ); } } - new_outputs + new_ordered_outputs } diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index bca718ab..f8fc0fd4 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -174,7 +174,10 @@ where let mut async_checks = FuturesUnordered::new(); - let known_utxos = Arc::new(transparent::new_outputs(&block, &transaction_hashes)); + let known_utxos = Arc::new(transparent::new_ordered_outputs( + &block, + &transaction_hashes, + )); for transaction in &block.transactions { let rsp = transaction_verifier .ready_and() diff --git a/zebra-consensus/src/script.rs b/zebra-consensus/src/script.rs index d71c3b94..006f4ae3 100644 --- a/zebra-consensus/src/script.rs +++ b/zebra-consensus/src/script.rs @@ -58,7 +58,7 @@ pub struct Request { /// A set of additional UTXOs known in the context of this verification request. /// /// This allows specifying additional UTXOs that are not already known to the chain state. - pub known_utxos: Arc>, + pub known_utxos: Arc>, /// The network upgrade active in the context of this verification request. /// /// Because the consensus branch ID changes with each network upgrade, @@ -110,7 +110,7 @@ where tracing::trace!("awaiting outpoint lookup"); let utxo = if let Some(output) = known_utxos.get(&outpoint) { tracing::trace!("UXTO in known_utxos, discarding query"); - output.clone() + output.utxo.clone() } else if let zebra_state::Response::Utxo(utxo) = query.await? { utxo } else { diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index 022e282b..f4d77221 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -70,7 +70,7 @@ pub enum Request { /// The transaction itself. transaction: Arc, /// Additional UTXOs which are known at the time of verification. - known_utxos: Arc>, + known_utxos: Arc>, /// The height of the block containing this transaction. height: block::Height, }, @@ -100,7 +100,7 @@ impl Request { } /// The set of additional known unspent transaction outputs that's in this request. - pub fn known_utxos(&self) -> Arc> { + pub fn known_utxos(&self) -> Arc> { match self { Request::Block { known_utxos, .. } => known_utxos.clone(), Request::Mempool { .. } => HashMap::new().into(), diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 0583fcbb..1b72f05f 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -822,23 +822,27 @@ fn v5_with_sapling_spends() { /// First, this creates a fake unspent transaction output from a fake transaction included in the /// specified `previous_utxo_height` block height. This fake [`Utxo`] also contains a simple script /// that can either accept or reject any spend attempt, depending on if `script_should_succeed` is -/// `true` or `false`. +/// `true` or `false`. Since the `tx_index_in_block` is irrelevant for blocks that have already +/// been verified, it is set to `1`. /// /// Then, a [`transparent::Input::PrevOut`] is created that attempts to spend the previously created fake -/// UTXO. A new UTXO is created with the [`transparent::Output`] resulting from the spend. +/// UTXO to a new [`transparent::Output`]. /// /// Finally, the initial fake UTXO is placed in a `known_utxos` [`HashMap`] so that it can be /// retrieved during verification. /// /// The function then returns the generated transparent input and output, as well as the /// `known_utxos` map. +/// +/// Note: `known_utxos` is only intended to be used for UTXOs within the same block, +/// so future verification changes might break this mocking function. fn mock_transparent_transfer( previous_utxo_height: block::Height, script_should_succeed: bool, ) -> ( transparent::Input, transparent::Output, - HashMap, + HashMap, ) { // A script with a single opcode that accepts the transaction (pushes true on the stack) let accepting_script = transparent::Script::new(&[1, 1]); @@ -862,11 +866,8 @@ fn mock_transparent_transfer( lock_script, }; - let previous_utxo = transparent::Utxo { - output: previous_output, - height: previous_utxo_height, - from_coinbase: false, - }; + let previous_utxo = + transparent::OrderedUtxo::new(previous_output, previous_utxo_height, false, 1); // Use the `previous_outpoint` as input let input = transparent::Input::PrevOut { diff --git a/zebra-state/src/lib.rs b/zebra-state/src/lib.rs index 499393a6..e3f08dab 100644 --- a/zebra-state/src/lib.rs +++ b/zebra-state/src/lib.rs @@ -35,3 +35,5 @@ pub use error::{BoxError, CloneError, CommitBlockError, ValidateContextError}; pub use request::{FinalizedBlock, HashOrHeight, PreparedBlock, Request}; pub use response::Response; pub use service::init; + +pub(crate) use request::ContextuallyValidBlock; diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 0a0949cc..c66e5eb2 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -68,10 +68,14 @@ pub struct PreparedBlock { /// New transparent outputs created in this block, indexed by /// [`Outpoint`](transparent::Outpoint). /// + /// Each output is tagged with its transaction index in the block. + /// (The outputs of earlier transactions in a block can be spent by later + /// transactions.) + /// /// Note: although these transparent outputs are newly created, they may not /// be unspent, since a later transaction in a block can spend outputs of an /// earlier transaction. - pub new_outputs: HashMap, + pub new_outputs: HashMap, /// A precomputed list of the hashes of the transactions in this block. pub transaction_hashes: Vec, // TODO: add these parameters when we can compute anchors. @@ -79,6 +83,19 @@ pub struct PreparedBlock { // sapling_anchor: sapling::tree::Root, } +/// A contextually validated block, ready to be committed directly to the finalized state with +/// no checks, if it becomes the root of the best non-finalized chain. +/// +/// Used by the state service and non-finalized [`Chain`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ContextuallyValidBlock { + pub(crate) block: Arc, + pub(crate) hash: block::Hash, + pub(crate) height: block::Height, + pub(crate) new_outputs: HashMap, + pub(crate) transaction_hashes: Vec, +} + /// A finalized block, ready to be committed directly to the finalized state with /// no checks. /// @@ -90,14 +107,7 @@ pub struct FinalizedBlock { pub(crate) block: Arc, pub(crate) hash: block::Hash, pub(crate) height: block::Height, - /// New transparent outputs created in this block, indexed by - /// [`Outpoint`](transparent::Outpoint). - /// - /// Note: although these transparent outputs are newly created, they may not - /// be unspent, since a later transaction in a block can spend outputs of an - /// earlier transaction. pub(crate) new_outputs: HashMap, - /// A precomputed list of the hashes of the transactions in this block. pub(crate) transaction_hashes: Vec, } @@ -127,7 +137,7 @@ impl From> for FinalizedBlock { } } -impl From for FinalizedBlock { +impl From for ContextuallyValidBlock { fn from(prepared: PreparedBlock) -> Self { let PreparedBlock { block, @@ -136,6 +146,25 @@ impl From for FinalizedBlock { new_outputs, transaction_hashes, } = prepared; + Self { + block, + hash, + height, + new_outputs: transparent::utxos_from_ordered_utxos(new_outputs), + transaction_hashes, + } + } +} + +impl From for FinalizedBlock { + fn from(contextually_valid: ContextuallyValidBlock) -> Self { + let ContextuallyValidBlock { + block, + hash, + height, + new_outputs, + transaction_hashes, + } = contextually_valid; Self { block, hash, diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index e975a623..2442884e 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -246,6 +246,9 @@ impl StateService { /// Check that the prepared block is contextually valid for the configured /// network, based on the committed finalized and non-finalized state. + /// + /// Note: some additional contextual validity checks are performed by the + /// non-finalized [`Chain`]. fn check_contextual_validity( &mut self, prepared: &PreparedBlock, @@ -631,7 +634,8 @@ impl Service for StateService { Request::CommitBlock(prepared) => { metrics::counter!("state.requests", 1, "type" => "commit_block"); - self.pending_utxos.check_against(&prepared.new_outputs); + self.pending_utxos + .check_against_ordered(&prepared.new_outputs); let rsp_rx = self.queue_and_commit_non_finalized(prepared); async move { diff --git a/zebra-state/src/service/finalized_state/tests/prop.rs b/zebra-state/src/service/finalized_state/tests/prop.rs index 821d4254..93209b51 100644 --- a/zebra-state/src/service/finalized_state/tests/prop.rs +++ b/zebra-state/src/service/finalized_state/tests/prop.rs @@ -9,6 +9,7 @@ use crate::{ arbitrary::PreparedChain, finalized_state::{FinalizedBlock, FinalizedState}, }, + ContextuallyValidBlock, }; const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 32; @@ -26,7 +27,7 @@ fn blocks_with_v5_transactions() -> Result<()> { // use `count` to minimize test failures, so they are easier to diagnose for block in chain.iter().take(count) { let hash = state.commit_finalized_direct( - FinalizedBlock::from(block.clone()), + FinalizedBlock::from(ContextuallyValidBlock::from(block.clone())), "blocks_with_v5_transactions test" ); prop_assert_eq!(Some(height), state.finalized_tip_height()); diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index eefa915a..68f7a67f 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -222,8 +222,8 @@ impl NonFinalizedState { /// `transparent::OutPoint` if it is present in any chain. pub fn any_utxo(&self, outpoint: &transparent::OutPoint) -> Option { for chain in self.chain_set.iter().rev() { - if let Some(output) = chain.created_utxos.get(outpoint) { - return Some(output.clone()); + if let Some(utxo) = chain.created_utxos.get(outpoint) { + return Some(utxo.clone()); } } diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 0b62abc5..2a29f613 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -11,25 +11,44 @@ use zebra_chain::{ transaction::Transaction::*, transparent, work::difficulty::PartialCumulativeWork, }; -use crate::{service::check, PreparedBlock, ValidateContextError}; +use crate::{service::check, ContextuallyValidBlock, PreparedBlock, ValidateContextError}; #[derive(Debug, Default, Clone)] pub struct Chain { - pub blocks: BTreeMap, + /// The contextually valid blocks which form this non-finalized partial chain, in height order. + pub(crate) blocks: BTreeMap, + + /// An index of block heights for each block hash in `blocks`. pub height_by_hash: HashMap, + /// An index of block heights and transaction indexes for each transaction hash in `blocks`. pub tx_by_hash: HashMap, - pub created_utxos: HashMap, + /// The [`Utxo`]s created by `blocks`. + /// + /// Note that these UTXOs may not be unspent. + /// Outputs can be spent by later transactions or blocks in the chain. + pub(super) created_utxos: HashMap, + /// The [`OutPoint`]s spent by `blocks`, + /// including those created by earlier transactions or blocks in the chain. pub(super) spent_utxos: HashSet, + /// The sprout anchors created by `blocks`. + /// + /// TODO: does this include intersitial anchors? pub(super) sprout_anchors: HashSet, + /// The sapling anchors created by `blocks`. pub(super) sapling_anchors: HashSet, + /// The orchard anchors created by `blocks`. pub(super) orchard_anchors: HashSet, + /// The sprout nullifiers revealed by `blocks`. pub(super) sprout_nullifiers: HashSet, + /// The sapling nullifiers revealed by `blocks`. pub(super) sapling_nullifiers: HashSet, + /// The orchard nullifiers revealed by `blocks`. pub(super) orchard_nullifiers: HashSet, + /// The cumulative work represented by this partial non-finalized chain. pub(super) partial_cumulative_work: PartialCumulativeWork, } @@ -76,6 +95,9 @@ impl Chain { /// If the block is invalid, drop this chain and return an error. #[instrument(level = "debug", skip(self, block), fields(block = %block.block))] pub fn push(mut self, block: PreparedBlock) -> Result { + // the block isn't contextually valid until `update_chain_state_with` returns success + let block = ContextuallyValidBlock::from(block); + // update cumulative data members self.update_chain_state_with(&block)?; tracing::debug!(block = %block.block, "adding block to chain"); @@ -85,7 +107,7 @@ impl Chain { /// Remove the lowest height block of the non-finalized portion of a chain. #[instrument(level = "debug", skip(self))] - pub fn pop_root(&mut self) -> PreparedBlock { + pub(crate) fn pop_root(&mut self) -> ContextuallyValidBlock { let block_height = self.lowest_height(); // remove the lowest height block from self.blocks @@ -180,16 +202,18 @@ trait UpdateWith { fn revert_chain_state_with(&mut self, _: &T); } -impl UpdateWith for Chain { +impl UpdateWith for Chain { + #[instrument(skip(self, contextually_valid), fields(block = %contextually_valid.block))] fn update_chain_state_with( &mut self, - prepared: &PreparedBlock, + contextually_valid: &ContextuallyValidBlock, ) -> Result<(), ValidateContextError> { - let (block, hash, height, transaction_hashes) = ( - prepared.block.as_ref(), - prepared.hash, - prepared.height, - &prepared.transaction_hashes, + let (block, hash, height, new_outputs, transaction_hashes) = ( + contextually_valid.block.as_ref(), + contextually_valid.hash, + contextually_valid.height, + &contextually_valid.new_outputs, + &contextually_valid.transaction_hashes, ); // add hash to height_by_hash @@ -254,7 +278,7 @@ impl UpdateWith for Chain { ); // add the utxos this produced - self.update_chain_state_with(&prepared.new_outputs)?; + self.update_chain_state_with(new_outputs)?; // add the utxos this consumed self.update_chain_state_with(inputs)?; @@ -268,12 +292,13 @@ impl UpdateWith for Chain { Ok(()) } - #[instrument(skip(self, prepared), fields(block = %prepared.block))] - fn revert_chain_state_with(&mut self, prepared: &PreparedBlock) { - let (block, hash, transaction_hashes) = ( - prepared.block.as_ref(), - prepared.hash, - &prepared.transaction_hashes, + #[instrument(skip(self, contextually_valid), fields(block = %contextually_valid.block))] + fn revert_chain_state_with(&mut self, contextually_valid: &ContextuallyValidBlock) { + let (block, hash, new_outputs, transaction_hashes) = ( + contextually_valid.block.as_ref(), + contextually_valid.hash, + &contextually_valid.new_outputs, + &contextually_valid.transaction_hashes, ); // remove the blocks hash from `height_by_hash` @@ -331,7 +356,7 @@ impl UpdateWith for Chain { ); // remove the utxos this produced - self.revert_chain_state_with(&prepared.new_outputs); + self.revert_chain_state_with(new_outputs); // remove the utxos this consumed self.revert_chain_state_with(inputs); diff --git a/zebra-state/src/service/non_finalized_state/queued_blocks.rs b/zebra-state/src/service/non_finalized_state/queued_blocks.rs index 5fe2d4bd..b60a0d2e 100644 --- a/zebra-state/src/service/non_finalized_state/queued_blocks.rs +++ b/zebra-state/src/service/non_finalized_state/queued_blocks.rs @@ -33,8 +33,9 @@ impl QueuedBlocks { let parent_hash = new.0.block.header.previous_block_hash; // Track known UTXOs in queued blocks. - for (outpoint, output) in new.0.new_outputs.iter() { - self.known_utxos.insert(*outpoint, output.clone()); + for (outpoint, ordered_utxo) in new.0.new_outputs.iter() { + self.known_utxos + .insert(*outpoint, ordered_utxo.utxo.clone()); } let replaced = self.blocks.insert(new_hash, new); diff --git a/zebra-state/src/service/pending_utxos.rs b/zebra-state/src/service/pending_utxos.rs index 6fbb71cf..e7d38007 100644 --- a/zebra-state/src/service/pending_utxos.rs +++ b/zebra-state/src/service/pending_utxos.rs @@ -46,13 +46,20 @@ impl PendingUtxos { } } - /// Check the list of pending UTXO requests against the supplied UTXO index. + /// Check the list of pending UTXO requests against the supplied [`OrderedUtxo`] index. + pub fn check_against_ordered( + &mut self, + ordered_utxos: &HashMap, + ) { + for (outpoint, ordered_utxo) in ordered_utxos.iter() { + self.respond(outpoint, ordered_utxo.utxo.clone()) + } + } + + /// Check the list of pending UTXO requests against the supplied [`Utxo`] index. pub fn check_against(&mut self, utxos: &HashMap) { for (outpoint, utxo) in utxos.iter() { - if let Some(sender) = self.0.remove(outpoint) { - tracing::trace!(?outpoint, "found pending UTXO"); - let _ = sender.send(utxo.clone()); - } + self.respond(outpoint, utxo.clone()) } } diff --git a/zebra-state/src/tests.rs b/zebra-state/src/tests.rs index 224d454b..b2e78a4d 100644 --- a/zebra-state/src/tests.rs +++ b/zebra-state/src/tests.rs @@ -21,7 +21,7 @@ impl Prepare for Arc { let hash = block.hash(); let height = block.coinbase_height().unwrap(); let transaction_hashes: Vec<_> = block.transactions.iter().map(|tx| tx.hash()).collect(); - let new_outputs = transparent::new_outputs(&block, transaction_hashes.as_slice()); + let new_outputs = transparent::new_ordered_outputs(&block, transaction_hashes.as_slice()); PreparedBlock { block,