Fix interstitial sprout anchors check

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<OutLocation>`            | Up/Del  |
-| `tx_by_transparent_addr_loc`   | `FirstOutLocation`     | `AtLeastOne<TransactionLocation>`    | 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<OutLocation>`           | Up/Del  |
+| `tx_by_transparent_addr_loc`   | `FirstOutLocation`     | `AtLeastOne<TransactionLocation>`   | 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
-
-- 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.
+- 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.
+
+- `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.
 

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.

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;
 
@@ -55,46 +55,67 @@ pub(crate) fn anchors_refer_to_earlier_treestates(
             // > output treestate of any prior JoinSplit description in the same transaction.
             //
             // https://zips.z.cash/protocol/protocol.pdf#joinsplit
-            let mut interstitial_roots: HashSet<sprout::tree::Root> = HashSet::new();
-
-            let mut interstitial_note_commitment_tree = parent_chain.sprout_note_commitment_tree();
+            let mut interstitial_roots: 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.
                 //
+                // The anchor is checked and the matching tree is obtained, which
+                // is used to create the interstitial tree state for this JoinSplit:
+                //
+                // > 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.
+                //
+                // https://zips.z.cash/protocol/protocol.pdf#joinsplit
+                //
                 // Note that [`interstitial_roots`] is always empty in the first
                 // iteration of the loop. This is because:
                 //
                 // > "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")
-                }
+                //
+                // https://zips.z.cash/protocol/protocol.pdf#joinsplit
+                let input_tree = interstitial_roots
+                    .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_roots.insert(input_tree.root(), input_tree);
 
                 tracing::debug!(?joinsplit.anchor, "observed sprout anchor");
             }

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<sprout::tree::NoteCommitmentTree> {
+        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

zebra-state/src/service/non_finalized_state/chain.rs

@@ -63,6 +63,10 @@ pub struct Chain {
     pub(crate) sprout_anchors: HashMultiSet<sprout::tree::Root>,
     /// The Sprout anchors created by each block in `blocks`.
     pub(crate) sprout_anchors_by_height: BTreeMap<block::Height, sprout::tree::Root>,
+    /// The Sprout note commitment tree for each anchor.
+    /// This is required for interstitial states.
+    pub(crate) sprout_trees_by_anchor:
+        HashMap<sprout::tree::Root, sprout::tree::NoteCommitmentTree>,
     /// The Sapling anchors created by `blocks`.
     pub(crate) sapling_anchors: HashMultiSet<sapling::tree::Root>,
     /// 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<ContextuallyValidBlock> 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<ContextuallyValidBlock> 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

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