From 4aeabd0b5272607b730e0bd283c88a49294301cc Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Tue, 18 Jan 2022 17:18:49 -0300 Subject: [PATCH] Fix interstitial sprout anchors check (#3283) * Fix interstitial Sprout anchors check * Update state docs; add sprout_trees_by_anchor to comparisons * Update book/src/dev/rfcs/0005-state-updates.md Co-authored-by: Marek * Rename `interstitial_roots` to `interstitial_trees` * Document consensus rules * Refactor the docs * Improve the docs for consensus rules * Update reference to cached state * Update zebra-state/src/service/check/anchors.rs Co-authored-by: Deirdre Connolly * Fix formatting Co-authored-by: Marek Co-authored-by: Deirdre Connolly Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .github/workflows/test.yml | 2 +- book/src/dev/rfcs/0005-state-updates.md | 72 +++++---- zebra-state/src/constants.rs | 2 +- zebra-state/src/service/check/anchors.rs | 147 +++++++++++------- zebra-state/src/service/finalized_state.rs | 21 ++- .../src/service/non_finalized_state/chain.rs | 20 ++- .../service/non_finalized_state/tests/prop.rs | 1 + 7 files changed, 162 insertions(+), 103 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2812e2287..dea0cc0df 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -54,7 +54,7 @@ jobs: --container-image rust:buster \ --container-mount-disk mount-path='/mainnet',name="zebrad-cache-$SHORT_SHA-mainnet-canopy" \ --container-restart-policy never \ - --create-disk name="zebrad-cache-$SHORT_SHA-mainnet-canopy",image=zebrad-cache-13c6a826-mainnet-canopy \ + --create-disk name="zebrad-cache-$SHORT_SHA-mainnet-canopy",image=zebrad-cache-1558f3378-mainnet-canopy \ --machine-type n2-standard-8 \ --service-account cos-vm@zealous-zebra.iam.gserviceaccount.com \ --scopes cloud-platform \ diff --git a/book/src/dev/rfcs/0005-state-updates.md b/book/src/dev/rfcs/0005-state-updates.md index d47829463..5866eb136 100644 --- a/book/src/dev/rfcs/0005-state-updates.md +++ b/book/src/dev/rfcs/0005-state-updates.md @@ -600,29 +600,29 @@ order on byte strings is the numeric ordering). We use the following rocksdb column families: -| Column Family | Keys | Values | Updates | -|--------------------------------|------------------------|--------------------------------------|---------| -| `hash_by_height` | `block::Height` | `block::Hash` | Never | -| `height_tx_count_by_hash` | `block::Hash` | `BlockTransactionCount` | Never | -| `block_header_by_height` | `block::Height` | `block::Header` | Never | -| `tx_by_location` | `TransactionLocation` | `Transaction` | Never | -| `hash_by_tx` | `TransactionLocation` | `transaction::Hash` | Never | -| `tx_by_hash` | `transaction::Hash` | `TransactionLocation` | Never | -| `utxo_by_outpoint` | `OutLocation` | `transparent::Output` | Delete | -| `balance_by_transparent_addr` | `transparent::Address` | `Amount \|\| FirstOutLocation` | Update | -| `utxo_by_transparent_addr_loc` | `FirstOutLocation` | `AtLeastOne` | Up/Del | -| `tx_by_transparent_addr_loc` | `FirstOutLocation` | `AtLeastOne` | Append | -| `sprout_nullifiers` | `sprout::Nullifier` | `()` | Never | -| `sprout_anchors` | `sprout::tree::Root` | `()` | Never | -| `sprout_note_commitment_tree` | `block::Height` | `sprout::tree::NoteCommitmentTree` | Delete | -| `sapling_nullifiers` | `sapling::Nullifier` | `()` | Never | -| `sapling_anchors` | `sapling::tree::Root` | `()` | Never | -| `sapling_note_commitment_tree` | `block::Height` | `sapling::tree::NoteCommitmentTree` | Delete | -| `orchard_nullifiers` | `orchard::Nullifier` | `()` | Never | -| `orchard_anchors` | `orchard::tree::Root` | `()` | Never | -| `orchard_note_commitment_tree` | `block::Height` | `orchard::tree::NoteCommitmentTree` | Delete | -| `history_tree` | `block::Height` | `NonEmptyHistoryTree` | Delete | -| `tip_chain_value_pool` | `()` | `ValueBalance` | Update | +| Column Family | Keys | Values | Updates | +| ------------------------------ | ---------------------- | ----------------------------------- | ------- | +| `hash_by_height` | `block::Height` | `block::Hash` | Never | +| `height_tx_count_by_hash` | `block::Hash` | `BlockTransactionCount` | Never | +| `block_header_by_height` | `block::Height` | `block::Header` | Never | +| `tx_by_location` | `TransactionLocation` | `Transaction` | Never | +| `hash_by_tx` | `TransactionLocation` | `transaction::Hash` | Never | +| `tx_by_hash` | `transaction::Hash` | `TransactionLocation` | Never | +| `utxo_by_outpoint` | `OutLocation` | `transparent::Output` | Delete | +| `balance_by_transparent_addr` | `transparent::Address` | `Amount \|\| FirstOutLocation` | Update | +| `utxo_by_transparent_addr_loc` | `FirstOutLocation` | `AtLeastOne` | Up/Del | +| `tx_by_transparent_addr_loc` | `FirstOutLocation` | `AtLeastOne` | Append | +| `sprout_nullifiers` | `sprout::Nullifier` | `()` | Never | +| `sprout_anchors` | `sprout::tree::Root` | `sprout::tree::NoteCommitmentTree` | Never | +| `sprout_note_commitment_tree` | `block::Height` | `sprout::tree::NoteCommitmentTree` | Delete | +| `sapling_nullifiers` | `sapling::Nullifier` | `()` | Never | +| `sapling_anchors` | `sapling::tree::Root` | `()` | Never | +| `sapling_note_commitment_tree` | `block::Height` | `sapling::tree::NoteCommitmentTree` | Delete | +| `orchard_nullifiers` | `orchard::Nullifier` | `()` | Never | +| `orchard_anchors` | `orchard::tree::Root` | `()` | Never | +| `orchard_note_commitment_tree` | `block::Height` | `orchard::tree::NoteCommitmentTree` | Delete | +| `history_tree` | `block::Height` | `NonEmptyHistoryTree` | Delete | +| `tip_chain_value_pool` | `()` | `ValueBalance` | Update | Zcash structures are encoded using `ZcashSerialize`/`ZcashDeserialize`. Other structures are encoded using `IntoDisk`/`FromDisk`. @@ -753,15 +753,25 @@ So they should not be used for consensus-critical checks. It also includes the `TransactionLocation` from the `FirstOutLocation`. (This duplicate data is small, and helps simplify the code.) -- Each incremental tree consists of nodes for a small number of peaks. - Peaks are written once, then deleted when they are no longer required. - New incremental tree nodes can be added each time the finalized tip changes, - and unused nodes can be deleted. - We only keep the nodes needed for the incremental tree for the finalized tip. - TODO: update this description based on the incremental merkle tree code +- Each `*_note_commitment_tree` stores the note commitment tree state + at the tip of the finalized state, for the specific pool. There is always + a single entry for those; they are indexed by height just to make testing + and debugging easier (so for each block committed, the old tree is + deleted and a new one is inserted by its new height). Each tree is stored + as a "Merkle tree frontier" which is basically a (logarithmic) subset of + the Merkle tree nodes as required to insert new items. -- The history tree indexes its peaks using blocks since the last network upgrade. - But we map those peak indexes to heights, to make testing and debugging easier. +- `history_tree` stores the ZIP-221 history tree state at the tip of the finalized + state. There is always a single entry for it; it is indexed by height just + to make testing and debugging easier. The tree is stored as the set of "peaks" + of the "Merkle mountain range" tree structure, which is what is required to + insert new items. + +- Each `*_anchors` stores the anchor (the root of a Merkle tree) of the note commitment + tree of a certain block. We only use the keys since we just need the set of anchors, + regardless of where they come from. The exception is `sprout_anchors` which also maps + the anchor to the matching note commitment tree. This is required to support interstitial + treestates, which are unique to Sprout. - The value pools are only stored for the finalized tip. diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index 615fa88e8..2f30bbcae 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -18,7 +18,7 @@ pub use zebra_chain::transparent::MIN_TRANSPARENT_COINBASE_MATURITY; pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRANSPARENT_COINBASE_MATURITY - 1; /// The database format version, incremented each time the database format changes. -pub const DATABASE_FORMAT_VERSION: u32 = 11; +pub const DATABASE_FORMAT_VERSION: u32 = 12; /// The maximum number of blocks to check for NU5 transactions, /// before we assume we are on a pre-NU5 legacy chain. diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index c139b6129..ebba88887 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -1,7 +1,7 @@ //! Checks for whether cited anchors are previously-computed note commitment //! tree roots. -use std::collections::HashSet; +use std::collections::HashMap; use zebra_chain::sprout; @@ -10,34 +10,14 @@ use crate::{ PreparedBlock, ValidateContextError, }; -/// Check that the Sprout, Sapling, and Orchard anchors specified by +/// Checks that the Sprout, Sapling, and Orchard anchors specified by /// transactions in this block have been computed previously within the context -/// of its parent chain. We do not check any anchors in checkpointed blocks, which avoids -/// JoinSplits +/// of its parent chain. We do not check any anchors in checkpointed blocks, +/// which avoids JoinSplits /// /// Sprout anchors may refer to some earlier block's final treestate (like -/// Sapling and Orchard do exclusively) _or_ to the interstisial output +/// Sapling and Orchard do exclusively) _or_ to the interstitial output /// treestate of any prior `JoinSplit` _within the same transaction_. -/// -/// > For the first JoinSplit description of a transaction, the anchor MUST be -/// > the output Sprout treestate of a previous block.[^sprout] -/// -/// > The anchor of each JoinSplit description in a transaction MUST refer to -/// > either some earlier block’s final Sprout treestate, or to the interstitial -/// > output treestate of any prior JoinSplit description in the same transaction.[^sprout] -/// -/// > The anchor of each Spend description MUST refer to some earlier -/// > block’s final Sapling treestate. The anchor is encoded separately in -/// > each Spend description for v4 transactions, or encoded once and -/// > shared between all Spend descriptions in a v5 transaction.[^sapling] -/// -/// > The anchorOrchard field of the transaction, whenever it exists (i.e. when -/// > there are any Action descriptions), MUST refer to some earlier block’s -/// > final Orchard treestate.[^orchard] -/// -/// [^sprout]: -/// [^sapling]: -/// [^orchard]: #[tracing::instrument(skip(finalized_state, parent_chain, prepared))] pub(crate) fn anchors_refer_to_earlier_treestates( finalized_state: &FinalizedState, @@ -46,55 +26,80 @@ pub(crate) fn anchors_refer_to_earlier_treestates( ) -> Result<(), ValidateContextError> { for transaction in prepared.block.transactions.iter() { // Sprout JoinSplits, with interstitial treestates to check as well. - // - // The FIRST JOINSPLIT in a transaction MUST refer to the output treestate - // of a previous block. if transaction.has_sprout_joinsplit_data() { - // > The anchor of each JoinSplit description in a transaction MUST refer to - // > either some earlier block’s final Sprout treestate, or to the interstitial - // > output treestate of any prior JoinSplit description in the same transaction. - // - // https://zips.z.cash/protocol/protocol.pdf#joinsplit - let mut interstitial_roots: HashSet = HashSet::new(); - - let mut interstitial_note_commitment_tree = parent_chain.sprout_note_commitment_tree(); + let mut interstitial_trees: HashMap< + sprout::tree::Root, + sprout::tree::NoteCommitmentTree, + > = HashMap::new(); for joinsplit in transaction.sprout_groth16_joinsplits() { - // Check all anchor sets, including the one for interstitial anchors. + // Check all anchor sets, including the one for interstitial + // anchors. // - // Note that [`interstitial_roots`] is always empty in the first - // iteration of the loop. This is because: + // The anchor is checked and the matching tree is obtained, + // which is used to create the interstitial tree state for this + // JoinSplit: // - // > "The anchor of each JoinSplit description in a transaction - // > MUST refer to [...] to the interstitial output treestate of - // > any **prior** JoinSplit description in the same transaction." - if !parent_chain.sprout_anchors.contains(&joinsplit.anchor) - && !finalized_state.contains_sprout_anchor(&joinsplit.anchor) - && (!interstitial_roots.contains(&joinsplit.anchor)) - { - // TODO: This check fails near: - // - mainnet block 1_047_908 - // with anchor 019c435cd1e8aca9a4165f7e126ac6e548952439d50213f4d15c546df9d49b61 - // - testnet block 1_057_737 - // with anchor 3ad623811ffa4fe8498b23f3d6bb4e086dca32269afef6c8e572fd9ee6d0c0ea - // - // Restore after finding the cause and fixing it. - // return Err(ValidateContextError::UnknownSproutAnchor { - // anchor: joinsplit.anchor, - // }); - tracing::warn!(?joinsplit.anchor, ?prepared.height, ?prepared.hash, "failed to find sprout anchor") - } + // > For each JoinSplit description in a transaction, an + // > interstitial output treestate is constructed which adds the + // > note commitments and nullifiers specified in that JoinSplit + // > description to the input treestate referred to by its + // > anchor. This interstitial output treestate is available for + // > use as the anchor of subsequent JoinSplit descriptions in + // > the same transaction. + // + // + // + // # Consensus + // + // > The anchor of each JoinSplit description in a transaction + // > MUST refer to either some earlier block’s final Sprout + // > treestate, or to the interstitial output treestate of any + // > prior JoinSplit description in the same transaction. + // + // > For the first JoinSplit description of a transaction, the + // > anchor MUST be the output Sprout treestate of a previous + // > block. + // + // + // + // Note that in order to satisfy the latter consensus rule above, + // [`interstitial_trees`] is always empty in the first iteration + // of the loop. + let input_tree = interstitial_trees + .get(&joinsplit.anchor) + .cloned() + .or_else(|| { + parent_chain + .sprout_trees_by_anchor + .get(&joinsplit.anchor) + .cloned() + .or_else(|| { + finalized_state + .sprout_note_commitment_tree_by_anchor(&joinsplit.anchor) + }) + }); + + let mut input_tree = match input_tree { + Some(tree) => tree, + None => { + tracing::warn!(?joinsplit.anchor, ?prepared.height, ?prepared.hash, "failed to find sprout anchor"); + return Err(ValidateContextError::UnknownSproutAnchor { + anchor: joinsplit.anchor, + }); + } + }; tracing::debug!(?joinsplit.anchor, "validated sprout anchor"); // Add new anchors to the interstitial note commitment tree. for cm in joinsplit.commitments { - interstitial_note_commitment_tree + input_tree .append(cm) .expect("note commitment should be appendable to the tree"); } - interstitial_roots.insert(interstitial_note_commitment_tree.root()); + interstitial_trees.insert(input_tree.root(), input_tree); tracing::debug!(?joinsplit.anchor, "observed sprout anchor"); } @@ -103,6 +108,20 @@ pub(crate) fn anchors_refer_to_earlier_treestates( // Sapling Spends // // MUST refer to some earlier block’s final Sapling treestate. + // + // # Consensus + // + // > The anchor of each Spend description MUST refer to some earlier + // > block’s final Sapling treestate. The anchor is encoded separately + // > in each Spend description for v4 transactions, or encoded once and + // > shared between all Spend descriptions in a v5 transaction. + // + // + // + // This rule is also implemented in + // [`zebra_chain::sapling::shielded_data`]. + // + // The "earlier treestate" check is implemented here. if transaction.has_sapling_shielded_data() { for anchor in transaction.sapling_anchors() { tracing::debug!(?anchor, "observed sapling anchor"); @@ -120,6 +139,14 @@ pub(crate) fn anchors_refer_to_earlier_treestates( // Orchard Actions // // MUST refer to some earlier block’s final Orchard treestate. + // + // # Consensus + // + // > The anchorOrchard field of the transaction, whenever it exists + // > (i.e. when there are any Action descriptions), MUST refer to some + // > earlier block’s final Orchard treestate. + // + // if let Some(orchard_shielded_data) = transaction.orchard_shielded_data() { tracing::debug!(?orchard_shielded_data.shared_anchor, "observed orchard anchor"); diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 81e00e5eb..66b620d55 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -438,7 +438,7 @@ impl FinalizedState { // Compute the new anchors and index them // Note: if the root hasn't changed, we write the same value again. - batch.zs_insert(sprout_anchors, sprout_root, ()); + batch.zs_insert(sprout_anchors, sprout_root, &sprout_note_commitment_tree); batch.zs_insert(sapling_anchors, sapling_root, ()); batch.zs_insert(orchard_anchors, orchard_root, ()); @@ -632,6 +632,7 @@ impl FinalizedState { } /// Returns `true` if the finalized state contains `sprout_anchor`. + #[allow(unused)] pub fn contains_sprout_anchor(&self, sprout_anchor: &sprout::tree::Root) -> bool { let sprout_anchors = self.db.cf_handle("sprout_anchors").unwrap(); self.db.zs_contains(sprout_anchors, &sprout_anchor) @@ -684,6 +685,18 @@ impl FinalizedState { .expect("Sprout note commitment tree must exist if there is a finalized tip") } + /// Returns the Sprout note commitment tree matching the given anchor. + /// + /// This is used for interstitial tree building, which is unique to Sprout. + pub fn sprout_note_commitment_tree_by_anchor( + &self, + sprout_anchor: &sprout::tree::Root, + ) -> Option { + let sprout_anchors = self.db.cf_handle("sprout_anchors").unwrap(); + + self.db.zs_get(sprout_anchors, sprout_anchor) + } + /// Returns the Sapling note commitment tree of the finalized tip /// or the empty tree if the state is empty. pub fn sapling_note_commitment_tree(&self) -> sapling::tree::NoteCommitmentTree { @@ -797,7 +810,11 @@ impl FinalizedState { for transaction in block.transactions.iter() { // Sprout for joinsplit in transaction.sprout_groth16_joinsplits() { - batch.zs_insert(sprout_anchors, joinsplit.anchor, ()); + batch.zs_insert( + sprout_anchors, + joinsplit.anchor, + sprout::tree::NoteCommitmentTree::default(), + ); } // Sapling diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index a775eaeb4..d2d8c84a9 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -63,6 +63,10 @@ pub struct Chain { pub(crate) sprout_anchors: HashMultiSet, /// The Sprout anchors created by each block in `blocks`. pub(crate) sprout_anchors_by_height: BTreeMap, + /// The Sprout note commitment tree for each anchor. + /// This is required for interstitial states. + pub(crate) sprout_trees_by_anchor: + HashMap, /// The Sapling anchors created by `blocks`. pub(crate) sapling_anchors: HashMultiSet, /// The Sapling anchors created by each block in `blocks`. @@ -117,6 +121,7 @@ impl Chain { spent_utxos: Default::default(), sprout_anchors: HashMultiSet::new(), sprout_anchors_by_height: Default::default(), + sprout_trees_by_anchor: Default::default(), sapling_anchors: HashMultiSet::new(), sapling_anchors_by_height: Default::default(), orchard_anchors: HashMultiSet::new(), @@ -155,6 +160,7 @@ impl Chain { // note commitment trees self.sprout_note_commitment_tree.root() == other.sprout_note_commitment_tree.root() && + self.sprout_trees_by_anchor == other.sprout_trees_by_anchor && self.sapling_note_commitment_tree.root() == other.sapling_note_commitment_tree.root() && self.orchard_note_commitment_tree.root() == other.orchard_note_commitment_tree.root() && @@ -383,6 +389,7 @@ impl Chain { sprout_anchors: self.sprout_anchors.clone(), sapling_anchors: self.sapling_anchors.clone(), orchard_anchors: self.orchard_anchors.clone(), + sprout_trees_by_anchor: self.sprout_trees_by_anchor.clone(), sprout_anchors_by_height: self.sprout_anchors_by_height.clone(), sapling_anchors_by_height: self.sapling_anchors_by_height.clone(), orchard_anchors_by_height: self.orchard_anchors_by_height.clone(), @@ -394,14 +401,6 @@ impl Chain { chain_value_pools: self.chain_value_pools, } } - - /// Returns a clone of the Sprout note commitment tree for this chain. - /// - /// Useful when calculating interstitial note commitment trees for each JoinSplit in a Sprout - /// shielded transaction. - pub fn sprout_note_commitment_tree(&self) -> sprout::tree::NoteCommitmentTree { - self.sprout_note_commitment_tree.clone() - } } /// The revert position being performed on a chain. @@ -528,6 +527,8 @@ impl UpdateWith for Chain { let sprout_root = self.sprout_note_commitment_tree.root(); self.sprout_anchors.insert(sprout_root); self.sprout_anchors_by_height.insert(height, sprout_root); + self.sprout_trees_by_anchor + .insert(sprout_root, self.sprout_note_commitment_tree.clone()); let sapling_root = self.sapling_note_commitment_tree.root(); self.sapling_anchors.insert(sapling_root); self.sapling_anchors_by_height.insert(height, sapling_root); @@ -643,6 +644,9 @@ impl UpdateWith for Chain { self.sprout_anchors.remove(&anchor), "Sprout anchor must be present if block was added to chain" ); + if !self.sprout_anchors.contains(&anchor) { + self.sprout_trees_by_anchor.remove(&anchor); + } let anchor = self .sapling_anchors_by_height diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index 48203103c..f8603ea2e 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -607,6 +607,7 @@ fn different_blocks_different_chains() -> Result<()> { // anchors chain1.sprout_anchors = chain2.sprout_anchors.clone(); chain1.sprout_anchors_by_height = chain2.sprout_anchors_by_height.clone(); + chain1.sprout_trees_by_anchor = chain2.sprout_trees_by_anchor.clone(); chain1.sapling_anchors = chain2.sapling_anchors.clone(); chain1.sapling_anchors_by_height = chain2.sapling_anchors_by_height.clone(); chain1.orchard_anchors = chain2.orchard_anchors.clone();