ZIP-221/244 auth data commitment validation in checkpoint verifier (#…

…2633)

* Add validation of ZIP-221 and ZIP-244 commitments

* Apply suggestions from code review

Co-authored-by: teor <[email protected]>

* Add auth commitment check in the finalized state

* Reset the verifier when comitting to state fails

* Add explanation comment

* Add test with fake activation heights

* Add generate_valid_commitments flag

* Enable fake activation heights using env var instead of feature

* Also update initial_tip_hash; refactor into progress_from_tip()

* Improve comments

* Add fake activation heights test to CI

* Fix bug that caused commitment trees to not match when generating partial arbitrary chains

* Add ChainHistoryBlockTxAuthCommitmentHash::from_commitments to organize and deduplicate code

* Remove stale comment, improve readability

* Allow overriding with PROPTEST_CASES

* partial_chain_strategy(): don't update note commitment trees when not needed; add comment

Co-authored-by: teor <[email protected]>

.github/workflows/ci.yml

@@ -69,6 +69,15 @@ jobs:
         with:
           command: test
           args: --verbose --all
+
+      - name: Run tests with fake activation heights
+        uses: actions-rs/[email protected]
+        env:
+          TEST_FAKE_ACTIVATION_HEIGHTS:
+        with:
+          command: test
+          args: --verbose --all -- with_fake_activation_heights
+
       # Explicitly run any tests that are usually #[ignored]
 
       - name: Run zebrad large sync tests

zebra-chain/build.rs

@@ -0,0 +1,9 @@
+use std::env;
+
+fn main() {
+    let use_fake_heights = env::var_os("TEST_FAKE_ACTIVATION_HEIGHTS").is_some();
+    println!("cargo:rerun-if-env-changed=TEST_FAKE_ACTIVATION_HEIGHTS");
+    if use_fake_heights {
+        println!("cargo:rustc-cfg=test_fake_activation_heights");
+    }
+}

zebra-chain/src/block.rs

@@ -16,7 +16,9 @@ pub mod tests;
 
 use std::{collections::HashMap, convert::TryInto, fmt, ops::Neg};
 
-pub use commitment::{ChainHistoryMmrRootHash, Commitment, CommitmentError};
+pub use commitment::{
+    ChainHistoryBlockTxAuthCommitmentHash, ChainHistoryMmrRootHash, Commitment, CommitmentError,
+};
 pub use hash::Hash;
 pub use header::{BlockTimeError, CountedHeader, Header};
 pub use height::Height;

zebra-chain/src/block/arbitrary.rs

@@ -11,6 +11,7 @@ use crate::{
     amount::NonNegative,
     block,
     fmt::SummaryDebug,
+    history_tree::HistoryTree,
     parameters::{
         Network,
         NetworkUpgrade::{self, *},
@@ -375,10 +376,15 @@ impl Block {
     /// `check_transparent_coinbase_spend` is used to check if
     /// transparent coinbase UTXOs are valid, before using them in blocks.
     /// Use [`allow_all_transparent_coinbase_spends`] to disable this check.
+    ///
+    /// `generate_valid_commitments` specifies if the generated blocks
+    /// should have valid commitments. This makes it much slower so it's better
+    /// to enable only when needed.
     pub fn partial_chain_strategy<F, T, E>(
         mut current: LedgerState,
         count: usize,
         check_transparent_coinbase_spend: F,
+        generate_valid_commitments: bool,
     ) -> BoxedStrategy<SummaryDebug<Vec<Arc<Self>>>>
     where
         F: Fn(
@@ -402,6 +408,15 @@ impl Block {
             let mut previous_block_hash = None;
             let mut utxos = HashMap::new();
             let mut chain_value_pools = ValueBalance::zero();
+            let mut sapling_tree = sapling::tree::NoteCommitmentTree::default();
+            let mut orchard_tree = orchard::tree::NoteCommitmentTree::default();
+            // The history tree usually takes care of "creating itself". But this
+            // only works when blocks are pushed into it starting from genesis
+            // (or at least pre-Heartwood, where the tree is not required).
+            // However, this strategy can generate blocks from an arbitrary height,
+            // so we must wait for the first block to create the history tree from it.
+            // This is why `Option` is used here.
+            let mut history_tree: Option<HistoryTree> = None;
 
             for (height, block) in vec.iter_mut() {
                 // fixup the previous block hash
@@ -419,16 +434,81 @@ impl Block {
                         &mut utxos,
                         check_transparent_coinbase_spend,
                     ) {
+                        // The FinalizedState does not update the note commitment trees with the genesis block,
+                        // because it doesn't need to (the trees are not used at that point) and updating them
+                        // would be awkward since the genesis block is handled separatedly there.
+                        // This forces us to skip the genesis block here too in order to able to use
+                        // this to test the finalized state.
+                        if generate_valid_commitments && *height != Height(0) {
+                            for sapling_note_commitment in transaction.sapling_note_commitments() {
+                                sapling_tree.append(*sapling_note_commitment).unwrap();
+                            }
+                            for orchard_note_commitment in transaction.orchard_note_commitments() {
+                                orchard_tree.append(*orchard_note_commitment).unwrap();
+                            }
+                        }
                         new_transactions.push(Arc::new(transaction));
                     }
                 }
 
                 // delete invalid transactions
                 block.transactions = new_transactions;
 
-                // TODO: if needed, fixup after modifying the block:
-                // - history and authorizing data commitments
-                // - the transaction merkle root
+                // fix commitment (must be done after finishing changing the block)
+                if generate_valid_commitments {
+                    let current_height = block.coinbase_height().unwrap();
+                    let heartwood_height = NetworkUpgrade::Heartwood
+                        .activation_height(current.network)
+                        .unwrap();
+                    let nu5_height = NetworkUpgrade::Nu5.activation_height(current.network);
+                    match current_height.cmp(&heartwood_height) {
+                        std::cmp::Ordering::Less => {}
+                        std::cmp::Ordering::Equal => {
+                            block.header.commitment_bytes = [0u8; 32];
+                        }
+                        std::cmp::Ordering::Greater => {
+                            let history_tree_root = match &history_tree {
+                                Some(tree) => tree.hash().unwrap_or_else(|| [0u8; 32].into()),
+                                None => [0u8; 32].into(),
+                            };
+                            if nu5_height.is_some() && current_height >= nu5_height.unwrap() {
+                                // From zebra-state/src/service/check.rs
+                                let auth_data_root = block.auth_data_root();
+                                let hash_block_commitments =
+                                    ChainHistoryBlockTxAuthCommitmentHash::from_commitments(
+                                        &history_tree_root,
+                                        &auth_data_root,
+                                    );
+                                block.header.commitment_bytes = hash_block_commitments.into();
+                            } else {
+                                block.header.commitment_bytes = history_tree_root.into();
+                            }
+                        }
+                    }
+                    // update history tree for the next block
+                    if history_tree.is_none() {
+                        history_tree = Some(
+                            HistoryTree::from_block(
+                                current.network,
+                                Arc::new(block.clone()),
+                                &sapling_tree.root(),
+                                &orchard_tree.root(),
+                            )
+                            .unwrap(),
+                        );
+                    } else {
+                        history_tree
+                            .as_mut()
+                            .unwrap()
+                            .push(
+                                current.network,
+                                Arc::new(block.clone()),
+                                sapling_tree.root(),
+                                orchard_tree.root(),
+                            )
+                            .unwrap();
+                    }
+                }
 
                 // now that we've made all the changes, calculate our block hash,
                 // so the next block can use it

zebra-chain/src/block/commitment.rs

@@ -2,10 +2,13 @@
 
 use thiserror::Error;
 
+use std::convert::TryInto;
+
 use crate::parameters::{Network, NetworkUpgrade, NetworkUpgrade::*};
 use crate::sapling;
 
 use super::super::block;
+use super::merkle::AuthDataRoot;
 
 /// Zcash blocks contain different kinds of commitments to their contents,
 /// depending on the network and height.
@@ -161,11 +164,6 @@ impl From<ChainHistoryMmrRootHash> for [u8; 32] {
 /// - the transaction authorising data in this block.
 ///
 /// Introduced in NU5.
-//
-// TODO:
-//    - add auth data type
-//    - add a method for hashing chain history and auth data together
-//    - move to a separate file
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)]
 pub struct ChainHistoryBlockTxAuthCommitmentHash([u8; 32]);
 
@@ -181,6 +179,40 @@ impl From<ChainHistoryBlockTxAuthCommitmentHash> for [u8; 32] {
     }
 }
 
+impl ChainHistoryBlockTxAuthCommitmentHash {
+    /// Compute the block commitment from the history tree root and the
+    /// authorization data root, as specified in [ZIP-244].
+    ///
+    /// `history_tree_root` is the root of the history tree up to and including
+    /// the *previous* block.
+    /// `auth_data_root` is the root of the Merkle tree of authorizing data
+    /// commmitments of each transaction in the *current* block.
+    ///
+    ///  [ZIP-244]: https://zips.z.cash/zip-0244#block-header-changes
+    pub fn from_commitments(
+        history_tree_root: &ChainHistoryMmrRootHash,
+        auth_data_root: &AuthDataRoot,
+    ) -> Self {
+        // > The value of this hash [hashBlockCommitments] is the BLAKE2b-256 hash personalized
+        // > by the string "ZcashBlockCommit" of the following elements:
+        // >   hashLightClientRoot (as described in ZIP 221)
+        // >   hashAuthDataRoot    (as described below)
+        // >   terminator          [0u8;32]
+        let hash_block_commitments: [u8; 32] = blake2b_simd::Params::new()
+            .hash_length(32)
+            .personal(b"ZcashBlockCommit")
+            .to_state()
+            .update(&<[u8; 32]>::from(*history_tree_root)[..])
+            .update(&<[u8; 32]>::from(*auth_data_root))
+            .update(&[0u8; 32])
+            .finalize()
+            .as_bytes()
+            .try_into()
+            .expect("32 byte array");
+        Self(hash_block_commitments)
+    }
+}
+
 /// Errors that can occur when checking RootHash consensus rules.
 ///
 /// Each error variant corresponds to a consensus rule, so enumerating

zebra-chain/src/block/tests/prop.rs

@@ -173,6 +173,7 @@ fn genesis_partial_chain_strategy() -> Result<()> {
             init,
             PREVOUTS_CHAIN_HEIGHT,
             allow_all_transparent_coinbase_spends,
+            false,
         )
     });
 
@@ -222,6 +223,7 @@ fn arbitrary_height_partial_chain_strategy() -> Result<()> {
                 init,
                 PREVOUTS_CHAIN_HEIGHT,
                 allow_all_transparent_coinbase_spends,
+                false,
             )
         });
 

zebra-chain/src/parameters/network_upgrade.rs

@@ -48,6 +48,7 @@ pub enum NetworkUpgrade {
 ///
 /// This is actually a bijective map, but it is const, so we use a vector, and
 /// do the uniqueness check in the unit tests.
+#[cfg(not(test_fake_activation_heights))]
 pub(crate) const MAINNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[
     (block::Height(0), Genesis),
     (block::Height(1), BeforeOverwinter),
@@ -59,10 +60,23 @@ pub(crate) const MAINNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)]
     // TODO: Add Nu5 mainnet activation height
 ];
 
+#[cfg(test_fake_activation_heights)]
+pub(crate) const MAINNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[
+    (block::Height(0), Genesis),
+    (block::Height(5), BeforeOverwinter),
+    (block::Height(10), Overwinter),
+    (block::Height(15), Sapling),
+    (block::Height(20), Blossom),
+    (block::Height(25), Heartwood),
+    (block::Height(30), Canopy),
+    (block::Height(35), Nu5),
+];
+
 /// Testnet network upgrade activation heights.
 ///
 /// This is actually a bijective map, but it is const, so we use a vector, and
 /// do the uniqueness check in the unit tests.
+#[cfg(not(test_fake_activation_heights))]
 pub(crate) const TESTNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[
     (block::Height(0), Genesis),
     (block::Height(1), BeforeOverwinter),
@@ -74,6 +88,18 @@ pub(crate) const TESTNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)]
     // TODO: Add Nu5 testnet activation height
 ];
 
+#[cfg(test_fake_activation_heights)]
+pub(crate) const TESTNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[
+    (block::Height(0), Genesis),
+    (block::Height(5), BeforeOverwinter),
+    (block::Height(10), Overwinter),
+    (block::Height(15), Sapling),
+    (block::Height(20), Blossom),
+    (block::Height(25), Heartwood),
+    (block::Height(30), Canopy),
+    (block::Height(35), Nu5),
+];
+
 /// The Consensus Branch Id, used to bind transactions and blocks to a
 /// particular network upgrade.
 #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]