diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index 0bbd4d14fe2..2762bbbbc61 100644 --- a/zebra-chain/src/block.rs +++ b/zebra-chain/src/block.rs @@ -16,10 +16,9 @@ mod tests; use std::fmt; -pub use commitment::Commitment; +pub use commitment::{Commitment, CommitmentError}; pub use hash::Hash; -pub use header::BlockTimeError; -pub use header::{CountedHeader, Header}; +pub use header::{BlockTimeError, CountedHeader, Header}; pub use height::Height; use serde::{Deserialize, Serialize}; @@ -63,15 +62,20 @@ impl Block { Hash::from(self) } - /// Get the parsed root hash for this block. + /// Get the parsed block [`Commitment`] for this block. /// - /// The interpretation of the root hash depends on the + /// The interpretation of the commitment depends on the /// configured `network`, and this block's height. /// - /// Returns None if this block does not have a block height. - pub fn commitment(&self, network: Network) -> Option { - self.coinbase_height() - .map(|height| Commitment::from_bytes(self.header.commitment_bytes, network, height)) + /// Returns an error if this block does not have a block height, + /// or if the commitment value is structurally invalid. + pub fn commitment(&self, network: Network) -> Result { + match self.coinbase_height() { + None => Err(CommitmentError::MissingBlockHeight { + block_hash: self.hash(), + }), + Some(height) => Commitment::from_bytes(self.header.commitment_bytes, network, height), + } } } diff --git a/zebra-chain/src/block/arbitrary.rs b/zebra-chain/src/block/arbitrary.rs index f50d1f9347d..a9993a516ee 100644 --- a/zebra-chain/src/block/arbitrary.rs +++ b/zebra-chain/src/block/arbitrary.rs @@ -53,7 +53,16 @@ impl Arbitrary for Commitment { fn arbitrary_with(_args: ()) -> Self::Strategy { (any::<[u8; 32]>(), any::(), any::()) .prop_map(|(commitment_bytes, network, block_height)| { - Commitment::from_bytes(commitment_bytes, network, block_height) + match Commitment::from_bytes(commitment_bytes, network, block_height) { + Ok(commitment) => commitment, + // just fix up the reserved values when they fail + Err(_) => Commitment::from_bytes( + super::commitment::RESERVED_BYTES, + network, + block_height, + ) + .expect("from_bytes only fails due to reserved bytes"), + } }) .boxed() } diff --git a/zebra-chain/src/block/commitment.rs b/zebra-chain/src/block/commitment.rs index de250357275..ebf972c403c 100644 --- a/zebra-chain/src/block/commitment.rs +++ b/zebra-chain/src/block/commitment.rs @@ -1,36 +1,55 @@ //! The Commitment enum, used for the corresponding block header field. +use thiserror::Error; + use crate::parameters::{Network, NetworkUpgrade, NetworkUpgrade::*}; -use crate::sapling::tree::Root; +use crate::sapling; -use super::Height; +use super::super::block; -/// Zcash blocks contain different kinds of root hashes, depending on the network upgrade. +/// Zcash blocks contain different kinds of commitments to their contents, +/// depending on the network and height. /// -/// The `BlockHeader.commitment_bytes` field is interpreted differently, -/// based on the current block height. The interpretation changes at or after -/// network upgrades. +/// The `Header.commitment_bytes` field is interpreted differently, based on the +/// network and height. The interpretation changes in the network upgrade +/// activation block, or in the block immediately after network upgrade +/// activation. #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] pub enum Commitment { /// [Pre-Sapling] Reserved field. /// - /// All zeroes. - PreSaplingReserved([u8; 32]), + /// The value of this field MUST be all zeroes. + /// + /// This field is verified in `Commitment::from_bytes`. + PreSaplingReserved, /// [Sapling and Blossom] The final Sapling treestate of this block. /// /// The root LEBS2OSP256(rt) of the Sapling note commitment tree /// corresponding to the final Sapling treestate of this block. - FinalSaplingRoot(Root), + /// + /// Subsequent `Commitment` variants also commit to the `FinalSaplingRoot`, + /// via their `EarliestSaplingRoot` and `LatestSaplingRoot` fields. + /// + /// TODO: this field is verified during semantic verification + /// + /// since Zebra checkpoints on Canopy, we don't need to validate this + /// field, but since it's included in the ChainHistoryRoot, we are + /// already calculating it, so we might as well validate it + FinalSaplingRoot(sapling::tree::Root), /// [Heartwood activation block] Reserved field. /// - /// All zeroes. This MUST NOT be interpreted as a root hash. + /// The value of this field MUST be all zeroes. + /// + /// This MUST NOT be interpreted as a root hash. /// See ZIP-221 for details. - ChainHistoryActivationReserved([u8; 32]), + /// + /// This field is verified in `Commitment::from_bytes`. + ChainHistoryActivationReserved, - /// [After Heartwood activation block] The root of a Merkle Mountain - /// Range chain history tree. + /// [(Heartwood activation block + 1) to Canopy] The root of a Merkle + /// Mountain Range chain history tree. /// /// This root hash commits to various features of the chain's history, /// including the Sapling commitment tree. This commitment supports the @@ -39,23 +58,65 @@ pub enum Commitment { /// The commitment in each block covers the chain history from the most /// recent network upgrade, through to the previous block. In particular, /// an activation block commits to the entire previous network upgrade, and - /// the block after activation commits only to the activation block. + /// the block after activation commits only to the activation block. (And + /// therefore transitively to all previous network upgrades covered by a + /// chain history hash in their activation block, via the previous block + /// hash field.) + /// + /// TODO: this field is verified during semantic verification ChainHistoryRoot(ChainHistoryMmrRootHash), + + /// [NU5 activation onwards] A commitment to: + /// - the chain history Merkle Mountain Range tree, and + /// - the auth data merkle tree covering this block. + /// + /// The chain history Merkle Mountain Range tree commits to the previous + /// block and all ancestors in the current network upgrade. The auth data + /// merkle tree commits to this block. + /// + /// This commitment supports the FlyClient protocol and non-malleable + /// transaction IDs. See ZIP-221 and ZIP-244 for details. + /// + /// See also the [`ChainHistoryRoot`] variant. + /// + /// TODO: this field is verified during semantic verification + // + // TODO: Do block commitments activate at NU5 activation, or (NU5 + 1)? + // https://github.com/zcash/zips/pull/474 + BlockCommitments(BlockCommitmentsHash), } +/// The required value of reserved `Commitment`s. +pub(crate) const RESERVED_BYTES: [u8; 32] = [0; 32]; + impl Commitment { /// Returns `bytes` as the Commitment variant for `network` and `height`. - pub(super) fn from_bytes(bytes: [u8; 32], network: Network, height: Height) -> Commitment { + pub(super) fn from_bytes( + bytes: [u8; 32], + network: Network, + height: block::Height, + ) -> Result { use Commitment::*; + use CommitmentError::*; match NetworkUpgrade::current(network, height) { - Genesis | BeforeOverwinter | Overwinter => PreSaplingReserved(bytes), - Sapling | Blossom => FinalSaplingRoot(Root(bytes)), + Genesis | BeforeOverwinter | Overwinter => { + if bytes == RESERVED_BYTES { + Ok(PreSaplingReserved) + } else { + Err(InvalidPreSaplingReserved { actual: bytes }) + } + } + Sapling | Blossom => Ok(FinalSaplingRoot(sapling::tree::Root(bytes))), Heartwood if Some(height) == Heartwood.activation_height(network) => { - ChainHistoryActivationReserved(bytes) + if bytes == RESERVED_BYTES { + Ok(ChainHistoryActivationReserved) + } else { + Err(InvalidChainHistoryActivationReserved { actual: bytes }) + } } - Heartwood | Canopy => ChainHistoryRoot(ChainHistoryMmrRootHash(bytes)), - Nu5 => unimplemented!("Nu5 uses hashAuthDataRoot as specified in ZIP-244"), + Heartwood | Canopy => Ok(ChainHistoryRoot(ChainHistoryMmrRootHash(bytes))), + Nu5 => Ok(BlockCommitments(BlockCommitmentsHash(bytes))), } } @@ -65,10 +126,11 @@ impl Commitment { use Commitment::*; match self { - PreSaplingReserved(b) => b, - FinalSaplingRoot(v) => v.0, - ChainHistoryActivationReserved(b) => b, - ChainHistoryRoot(v) => v.0, + PreSaplingReserved => RESERVED_BYTES, + FinalSaplingRoot(hash) => hash.0, + ChainHistoryActivationReserved => RESERVED_BYTES, + ChainHistoryRoot(hash) => hash.0, + BlockCommitments(hash) => hash.0, } } } @@ -76,7 +138,60 @@ impl Commitment { /// The root hash of a Merkle Mountain Range chain history tree. // TODO: // - add methods for maintaining the MMR peaks, and calculating the root -// hash from the current set of peaks. -// - move to a separate file. +// hash from the current set of peaks +// - move to a separate file #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct ChainHistoryMmrRootHash([u8; 32]); + +/// The Block Commitments for a block. As of NU5, these cover: +/// - the chain history tree for all ancestors in the current network upgrade, +/// and +/// - the transaction authorising data in this block. +// +// 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 BlockCommitmentsHash([u8; 32]); + +/// Errors that can occur when checking RootHash consensus rules. +/// +/// Each error variant corresponds to a consensus rule, so enumerating +/// all possible verification failures enumerates the consensus rules we +/// implement, and ensures that we don't reject blocks or transactions +/// for a non-enumerated reason. +#[allow(dead_code)] +#[derive(Error, Debug, PartialEq)] +pub enum CommitmentError { + #[error("invalid pre-Sapling reserved committment: expected all zeroes, actual: {actual:?}")] + InvalidPreSaplingReserved { + // TODO: are these fields a security risk? If so, open a ticket to remove + // similar fields across Zebra + actual: [u8; 32], + }, + + #[error("invalid final sapling root: expected {expected:?}, actual: {actual:?}")] + InvalidFinalSaplingRoot { + expected: [u8; 32], + actual: [u8; 32], + }, + + #[error("invalid chain history activation reserved block committment: expected all zeroes, actual: {actual:?}")] + InvalidChainHistoryActivationReserved { actual: [u8; 32] }, + + #[error("invalid chain history root: expected {expected:?}, actual: {actual:?}")] + InvalidChainHistoryRoot { + expected: [u8; 32], + actual: [u8; 32], + }, + + #[error("invalid block commitment: expected {expected:?}, actual: {actual:?}")] + InvalidBlockCommitment { + expected: [u8; 32], + actual: [u8; 32], + }, + + #[error("missing required block height: block commitments can't be parsed without a block height, block hash: {block_hash:?}")] + MissingBlockHeight { block_hash: block::Hash }, +} diff --git a/zebra-chain/src/block/header.rs b/zebra-chain/src/block/header.rs index 8ab3492127b..4d903498d05 100644 --- a/zebra-chain/src/block/header.rs +++ b/zebra-chain/src/block/header.rs @@ -40,12 +40,14 @@ pub struct Header { /// valid. pub merkle_root: merkle::Root, - /// Some kind of root hash. + /// Zcash blocks contain different kinds of commitments to their contents, + /// depending on the network and height. /// - /// Unfortunately, the interpretation of this field was changed without - /// incrementing the version, so it cannot be parsed without the block height - /// and network. Use [`Block::commitment`](super::Block::commitment) to get the - /// parsed [`Commitment`](super::Commitment). + /// The interpretation of this field has been changed multiple times, without + /// incrementing the block [`version`]. Therefore, this field cannot be + /// parsed without the network and height. Use + /// [`Block::commitment`](super::Block::commitment) to get the parsed + /// [`Commitment`](super::Commitment). pub commitment_bytes: [u8; 32], /// The block timestamp is a Unix epoch time (UTC) when the miner diff --git a/zebra-chain/src/block/tests/prop.rs b/zebra-chain/src/block/tests/prop.rs index f2006186914..3cf47b13093 100644 --- a/zebra-chain/src/block/tests/prop.rs +++ b/zebra-chain/src/block/tests/prop.rs @@ -47,10 +47,13 @@ proptest! { ) { zebra_test::init(); - let commitment = Commitment::from_bytes(bytes, network, block_height); - let other_bytes = commitment.to_bytes(); + // just skip the test if the bytes don't parse, because there's nothing + // to compare with + if let Ok(commitment) = Commitment::from_bytes(bytes, network, block_height) { + let other_bytes = commitment.to_bytes(); - prop_assert_eq![bytes, other_bytes]; + prop_assert_eq![bytes, other_bytes]; + } } } @@ -69,13 +72,11 @@ proptest! { let bytes = block.zcash_serialize_to_vec()?; let bytes = &mut bytes.as_slice(); - // Check the root hash + // Check the block commitment let commitment = block.commitment(network); - if let Some(commitment) = commitment { + if let Ok(commitment) = commitment { let commitment_bytes = commitment.to_bytes(); prop_assert_eq![block.header.commitment_bytes, commitment_bytes]; - } else { - prop_assert_eq![block.coinbase_height(), None]; } // Check the block size limit