Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 22 commits into from
Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a39dbc4
Add validation of ZIP-221 and ZIP-244 commitments
conradoplg Aug 11, 2021
d497a2a
Apply suggestions from code review
conradoplg Aug 12, 2021
1fab6c9
Add auth commitment check in the finalized state
conradoplg Aug 13, 2021
da154f1
Merge remote-tracking branch 'origin/main' into zip221-validation-che…
conradoplg Aug 17, 2021
9b7b0a1
Reset the verifier when comitting to state fails
conradoplg Aug 18, 2021
96e9502
Add explanation comment
conradoplg Aug 18, 2021
1001d62
Merge remote-tracking branch 'origin/main' into zip221-validation-che…
conradoplg Aug 18, 2021
a3ff5b2
Add test with fake activation heights
conradoplg Aug 13, 2021
05978e3
Add generate_valid_commitments flag
conradoplg Aug 19, 2021
0dd9557
Enable fake activation heights using env var instead of feature
conradoplg Aug 19, 2021
80768e1
Also update initial_tip_hash; refactor into progress_from_tip()
conradoplg Aug 19, 2021
931ff39
Merge remote-tracking branch 'origin/main' into zip221-validation-che…
conradoplg Aug 19, 2021
9e823e2
Improve comments
conradoplg Aug 19, 2021
8e2968f
Add fake activation heights test to CI
conradoplg Aug 19, 2021
2c4b3df
Merge branch 'main' into zip221-validation-checkpoint
conradoplg Aug 19, 2021
e2e3b6b
Fix bug that caused commitment trees to not match when generating par…
conradoplg Aug 20, 2021
32a53cd
Add ChainHistoryBlockTxAuthCommitmentHash::from_commitments to organi…
conradoplg Aug 20, 2021
e3d1907
Remove stale comment, improve readability
conradoplg Aug 20, 2021
c0abcde
Allow overriding with PROPTEST_CASES
conradoplg Aug 20, 2021
c24a354
Merge remote-tracking branch 'origin/main' into zip221-validation-che…
conradoplg Aug 20, 2021
f684422
partial_chain_strategy(): don't update note commitment trees when not…
conradoplg Aug 20, 2021
ef48109
Merge branch 'main' into zip221-validation-checkpoint
conradoplg Aug 23, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions zebra-chain/build.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
4 changes: 3 additions & 1 deletion zebra-chain/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
86 changes: 83 additions & 3 deletions zebra-chain/src/block/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::{
amount::NonNegative,
block,
fmt::SummaryDebug,
history_tree::HistoryTree,
parameters::{
Network,
NetworkUpgrade::{self, *},
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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
Expand Down
42 changes: 37 additions & 5 deletions zebra-chain/src/block/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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]);

Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions zebra-chain/src/block/tests/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ fn genesis_partial_chain_strategy() -> Result<()> {
init,
PREVOUTS_CHAIN_HEIGHT,
allow_all_transparent_coinbase_spends,
false,
)
});

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

Expand Down
26 changes: 26 additions & 0 deletions zebra-chain/src/parameters/network_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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)]
Expand Down
Loading