- Returns early when there is no Heartwood activation height when creating or updating HistoryTree

- Updates `check::legacy_chain()` to accept and compare an optional NU5 activation height
- Replaces `.map()` with `.filter_map()` in `NetworkUpgrade::target_spacings()`
- Updates some `expect()` messages when unwrapping return value of `NetworkUpgrade.activation_height()`
- Adds TODOs for updating `.mandatory_checkpoint_height()` method
- Removes TODO for calling `.expect()` when getting NU5 activation height
- Updates `Network.sapling_activation_height()` to return None instead of panicking when missing Sapling activation height
- Updates calls to `.sapling_activation_height()` to return errors or use `Height(1)` at the minimum birthday height when there's no Sapling activation height
This commit is contained in:
Arya 2024-04-23 22:17:08 -04:00
parent 7a756f73c9
commit 301900435b
29 changed files with 95 additions and 60 deletions

View File

@ -260,8 +260,9 @@ impl Default for LedgerState {
let default_override = LedgerStateOverride::default();
let most_recent_nu = NetworkUpgrade::current(&default_network, Height::MAX);
let most_recent_activation_height =
most_recent_nu.activation_height(&default_network).unwrap();
let most_recent_activation_height = most_recent_nu
.activation_height(&default_network)
.expect("should have an activation height in default networks");
LedgerState {
height: most_recent_activation_height,
@ -467,8 +468,10 @@ impl Block {
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);
.expect("should have an activation height in default networks");
let nu5_height = NetworkUpgrade::Nu5
.activation_height(&current.network)
.expect("should have an activation height in default networks");
match current_height.cmp(&heartwood_height) {
std::cmp::Ordering::Less => {
@ -491,7 +494,7 @@ impl Block {
Some(tree) => tree.hash().unwrap_or_else(|| [0u8; 32].into()),
None => [0u8; 32].into(),
};
if nu5_height.is_some() && current_height >= nu5_height.unwrap() {
if current_height >= nu5_height {
// From zebra-state/src/service/check.rs
let auth_data_root = block.auth_data_root();
let hash_block_commitments =
@ -696,7 +699,11 @@ impl Arbitrary for Commitment {
fn arbitrary_with(_args: ()) -> Self::Strategy {
(any::<[u8; 32]>(), any::<Network>(), any::<Height>())
.prop_map(|(commitment_bytes, network, block_height)| {
if block_height == Heartwood.activation_height(&network).unwrap() {
if block_height
== Heartwood
.activation_height(&network)
.expect("should have an activation height in default networks")
{
Commitment::ChainHistoryActivationReserved
} else {
Commitment::from_bytes(commitment_bytes, &network, block_height)

View File

@ -223,7 +223,7 @@ fn block_test_vectors_height(network: Network) {
if height
>= Sapling
.activation_height(&network)
.expect("sapling activation height is set")
.expect("sapling activation height is set on default network")
.0
{
assert!(

View File

@ -25,7 +25,7 @@ proptest! {
let (chain_tip, mock_chain_tip_sender) = MockChainTip::new();
let blossom_activation_height = NetworkUpgrade::Blossom
.activation_height(&network)
.expect("Blossom activation height is missing");
.expect("Blossom activation height is missing on default network");
block_heights.sort();
let current_height = block_heights[0];

View File

@ -428,9 +428,11 @@ impl HistoryTree {
sapling_root: &sapling::tree::Root,
orchard_root: &orchard::tree::Root,
) -> Result<Self, HistoryTreeError> {
let heartwood_height = NetworkUpgrade::Heartwood
.activation_height(network)
.expect("Heartwood height is known");
let Some(heartwood_height) = NetworkUpgrade::Heartwood.activation_height(network) else {
// Return early if there is no Heartwood activation height.
return Ok(HistoryTree(None));
};
match block
.coinbase_height()
.expect("must have height")

View File

@ -35,7 +35,10 @@ fn push_and_prune_for_network_upgrade(
) -> Result<()> {
let (blocks, sapling_roots) = network.block_sapling_roots_map();
let height = network_upgrade.activation_height(&network).unwrap().0;
let height = network_upgrade
.activation_height(&network)
.expect("network upgrade activation height is missing on default network")
.0;
// Load first block (activation block of the given network upgrade)
let first_block = Arc::new(
@ -118,7 +121,10 @@ fn upgrade() -> Result<()> {
fn upgrade_for_network_upgrade(network: Network, network_upgrade: NetworkUpgrade) -> Result<()> {
let (blocks, sapling_roots) = network.block_sapling_roots_map();
let height = network_upgrade.activation_height(&network).unwrap().0;
let height = network_upgrade
.activation_height(&network)
.expect("network upgrade activation height is missing on default network")
.0;
// Load previous block (the block before the activation block of the given network upgrade)
let block_prev = Arc::new(

View File

@ -267,6 +267,12 @@ impl Network {
//
// See the `ZIP_212_GRACE_PERIOD_DURATION` documentation for more information.
// TODO:
// - Support constructing pre-Canopy coinbase tx and block templates and return `Height::MAX` instead of panicking
// when Canopy activation height is `None` (#8434)
// - Add semantic block validation during the ZIP-212 grace period and update this method to return the lesser of
// `canopy_activation + ZIP_212_GRACE_PERIOD_DURATION` or the NU5 activation height. (#8430)
let canopy_activation = NetworkUpgrade::Canopy
.activation_height(self)
.expect("Canopy activation height must be present for both networks");
@ -296,10 +302,8 @@ impl Network {
}
/// Returns the Sapling activation height for this network.
pub fn sapling_activation_height(&self) -> Height {
super::NetworkUpgrade::Sapling
.activation_height(self)
.expect("Sapling activation height needs to be set")
pub fn sapling_activation_height(&self) -> Option<Height> {
super::NetworkUpgrade::Sapling.activation_height(self)
}
}

View File

@ -19,7 +19,7 @@ proptest! {
let canopy_activation = NetworkUpgrade::Canopy
.activation_height(&network)
.expect("Canopy activation height is set");
.expect("Canopy activation height is set on default networks");
let grace_period_end_height = (canopy_activation + ZIP_212_GRACE_PERIOD_DURATION)
.expect("ZIP-212 grace period ends in a valid block height");

View File

@ -112,7 +112,7 @@ fn activates_network_upgrades_correctly() {
let genesis_activation_height = NetworkUpgrade::Genesis
.activation_height(&network)
.expect("must return an activation height");
.expect("must always return an activation height for Genesis network upgrade");
assert_eq!(
genesis_activation_height,
@ -123,7 +123,7 @@ fn activates_network_upgrades_correctly() {
for nu in NETWORK_UPGRADES_IN_ORDER.into_iter().skip(1) {
let activation_height = nu
.activation_height(&network)
.expect("must return an activation height");
.expect("must return an activation height for all network upgrades when there's an NU5 activation height");
assert_eq!(
activation_height, Height(expected_activation_height),

View File

@ -402,14 +402,10 @@ impl NetworkUpgrade {
),
]
.into_iter()
.map(move |(upgrade, spacing_seconds)| {
let activation_height = upgrade
.activation_height(network)
.expect("missing activation height for target spacing change");
.filter_map(move |(upgrade, spacing_seconds)| {
let activation_height = upgrade.activation_height(network)?;
let target_spacing = Duration::seconds(spacing_seconds);
(activation_height, target_spacing)
Some((activation_height, target_spacing))
})
}

View File

@ -127,7 +127,7 @@ fn activation_consistent(network: Network) {
for &network_upgrade in network_upgrades {
let height = network_upgrade
.activation_height(&network)
.expect("activations must have a height");
.expect("activations must have a height on default networks");
assert!(NetworkUpgrade::is_activation_height(&network, height));
if height > block::Height(0) {

View File

@ -21,7 +21,10 @@ fn tree() -> Result<()> {
fn tree_for_network_upgrade(network: &Network, network_upgrade: NetworkUpgrade) -> Result<()> {
let (blocks, sapling_roots) = network.block_sapling_roots_map();
let height = network_upgrade.activation_height(network).unwrap().0;
let height = network_upgrade
.activation_height(network)
.expect("must have activation height on default network")
.0;
// Load Block 0 (activation block of the given network upgrade)
let block0 = Arc::new(

View File

@ -62,7 +62,7 @@ fn incremental_roots_with_blocks_for_network(network: Network) -> Result<()> {
let height = NetworkUpgrade::Sapling
.activation_height(&network)
.unwrap()
.expect("must have activation height on default network")
.0;
// Build empty note commitment tree

View File

@ -92,7 +92,7 @@ fn incremental_roots_with_blocks_for_network(network: Network) -> Result<()> {
// Load the Genesis height.
let genesis_height = NetworkUpgrade::Genesis
.activation_height(&network)
.unwrap()
.expect("must have activation height on default network")
.0;
// Load the Genesis block.

View File

@ -352,7 +352,7 @@ fn fake_v5_round_trip_for_network(network: Network) {
let overwinter_activation_height = NetworkUpgrade::Overwinter
.activation_height(&network)
.expect("a valid height")
.expect("must have activation height on default network")
.0;
// skip blocks that are before overwinter as they will not have a valid consensus branch id
@ -500,7 +500,7 @@ fn fake_v5_librustzcash_round_trip_for_network(network: Network) {
let overwinter_activation_height = NetworkUpgrade::Overwinter
.activation_height(&network)
.expect("a valid height")
.expect("must have activation height on default network")
.0;
let nu5_activation_height = NetworkUpgrade::Nu5

View File

@ -155,7 +155,7 @@ pub fn subsidy_is_valid(block: &Block, network: &Network) -> Result<(), BlockErr
let canopy_activation_height = NetworkUpgrade::Canopy
.activation_height(network)
.expect("Canopy activation height is known");
.expect("Canopy activation height is required for semantic block validation");
if height < SLOW_START_INTERVAL {
unreachable!(

View File

@ -25,7 +25,9 @@ pub fn funding_stream_values(
height: Height,
network: &Network,
) -> Result<HashMap<FundingStreamReceiver, Amount<NonNegative>>, Error> {
let canopy_height = Canopy.activation_height(network).unwrap();
let canopy_height = Canopy
.activation_height(network)
.expect("requires Canopy activation height");
let mut results = HashMap::new();
if height >= canopy_height {

View File

@ -230,7 +230,7 @@ impl ParameterSubsidy for Network {
match self {
Network::Mainnet => NetworkUpgrade::Canopy
.activation_height(self)
.expect("canopy activation height should be available"),
.expect("Canopy activation height should be available"),
// TODO: Check what zcashd does here, consider adding a field to `testnet::Parameters` to make this configurable.
Network::Testnet(_params) => FIRST_HALVING_TESTNET,
}

View File

@ -356,7 +356,6 @@ pub fn coinbase_expiry_height(
) -> Result<(), TransactionError> {
let expiry_height = coinbase.expiry_height();
// TODO: replace `if let` with `expect` after NU5 mainnet activation
if let Some(nu5_activation_height) = NetworkUpgrade::Nu5.activation_height(network) {
// # Consensus
//

View File

@ -88,7 +88,9 @@ async fn test_mocked_rpc_response_data_for_network(network: Network, random_port
.expect_request_that(|req| matches!(req, ScanRequest::Info))
.await
.respond(ScanResponse::Info {
min_sapling_birthday_height: network.sapling_activation_height(),
min_sapling_birthday_height: network
.sapling_activation_height()
.expect("must have Sapling activation height for default network"),
})
});
}

View File

@ -57,7 +57,10 @@ async fn test_mocked_getinfo_for_network(
// test the response
assert_eq!(
get_info_response.into_inner().min_sapling_birthday_height,
network.sapling_activation_height().0,
network
.sapling_activation_height()
.expect("must have Sapling activation height for default network")
.0,
"get_info response min sapling height should match network sapling activation height"
);
}
@ -396,7 +399,9 @@ async fn call_get_info(
.expect_request_that(|req| matches!(req, ScanRequest::Info))
.await
.respond(ScanResponse::Info {
min_sapling_birthday_height: network.sapling_activation_height(),
min_sapling_birthday_height: network
.sapling_activation_height()
.expect("must have Sapling activation height for default network"),
})
});

View File

@ -95,7 +95,10 @@ impl Service<Request> for ScanService {
return async move {
Ok(Response::Info {
min_sapling_birthday_height: db.network().sapling_activation_height(),
min_sapling_birthday_height: db
.network()
.sapling_activation_height()
.ok_or("missing Sapling activation height")?,
})
}
.boxed();

View File

@ -78,7 +78,9 @@ impl ScanTask {
let mut new_keys = HashMap::new();
let mut new_result_senders = HashMap::new();
let mut new_result_receivers = Vec::new();
let sapling_activation_height = network.sapling_activation_height();
let sapling_activation_height = network.sapling_activation_height().ok_or(eyre!(
"must have Sapling activation height for default network"
))?;
loop {
let cmd = match cmd_receiver.try_recv() {

View File

@ -79,7 +79,9 @@ pub async fn start(
mut cmd_receiver: tokio::sync::mpsc::Receiver<ScanTaskCommand>,
) -> Result<(), Report> {
let network = storage.network();
let sapling_activation_height = network.sapling_activation_height();
let sapling_activation_height = network
.sapling_activation_height()
.ok_or(eyre!("missing Sapling activation height"))?;
info!(?network, "starting scan task");

View File

@ -6,7 +6,7 @@ use crate::{
scan::{get_min_height, scan_height_and_store_results, wait_for_height, State, CHECK_INTERVAL},
storage::Storage,
};
use color_eyre::eyre::Report;
use color_eyre::eyre::{eyre, Report};
use tokio::{
sync::{mpsc::Sender, watch},
task::JoinHandle,
@ -88,7 +88,10 @@ pub async fn scan_range(
storage: Storage,
subscribed_keys_receiver: watch::Receiver<Arc<HashMap<String, Sender<ScanResult>>>>,
) -> Result<(), Report> {
let sapling_activation_height = storage.network().sapling_activation_height();
let sapling_activation_height = storage
.network()
.sapling_activation_height()
.ok_or(eyre!("missing Sapling activation height"))?;
// Do not scan and notify if we are below sapling activation height.
wait_for_height(
sapling_activation_height,

View File

@ -213,7 +213,10 @@ impl Storage {
sapling_key: &SaplingScanningKey,
birthday_height: Option<Height>,
) {
let min_birthday_height = self.network().sapling_activation_height();
let min_birthday_height = self
.network()
.sapling_activation_height()
.unwrap_or(Height(1));
// The birthday height must be at least the minimum height for that pool.
let birthday_height = birthday_height

View File

@ -170,7 +170,9 @@ fn scanning_fake_blocks_store_key_and_results() -> Result<()> {
.expect("height is stored")
.next()
.expect("height is not maximum"),
network.sapling_activation_height()
network
.sapling_activation_height()
.expect("should have Sapling activation height on default networks")
);
let nf = Nullifier([7; 32]);

View File

@ -423,9 +423,7 @@ impl StateService {
let timer = CodeTimer::start();
if let Some(tip) = state.best_tip() {
let nu5_activation_height = NetworkUpgrade::Nu5
.activation_height(network)
.expect("NU5 activation height is set");
let nu5_activation_height = NetworkUpgrade::Nu5.activation_height(network);
if let Err(error) = check::legacy_chain(
nu5_activation_height,

View File

@ -306,7 +306,7 @@ fn difficulty_threshold_and_time_are_valid(
/// `max_legacy_chain_blocks` should be [`MAX_LEGACY_CHAIN_BLOCKS`](crate::constants::MAX_LEGACY_CHAIN_BLOCKS).
/// They are only changed from the defaults for testing.
pub(crate) fn legacy_chain<I>(
nu5_activation_height: block::Height,
nu5_activation_height: Option<block::Height>,
ancestors: I,
network: &Network,
max_legacy_chain_blocks: usize,
@ -323,11 +323,7 @@ where
//
// If the cached tip is close to NU5 activation, but there aren't any V5 transactions in the
// chain yet, we could reach MAX_BLOCKS_TO_CHECK in Canopy, and incorrectly return an error.
if block
.coinbase_height()
.expect("valid blocks have coinbase heights")
< nu5_activation_height
{
if block.coinbase_height() < nu5_activation_height {
return Ok(());
}

View File

@ -306,7 +306,7 @@ proptest! {
fn some_block_less_than_network_upgrade(
(network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, UNDER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy)
) {
let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), &network, TEST_LEGACY_CHAIN_LIMIT)
let response = crate::service::check::legacy_chain(Some(nu_activation_height), chain.into_iter().rev(), &network, TEST_LEGACY_CHAIN_LIMIT)
.map_err(|error| error.to_string());
prop_assert_eq!(response, Ok(()));
@ -323,7 +323,7 @@ proptest! {
.coinbase_height()
.expect("chain contains valid blocks");
let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), &network, TEST_LEGACY_CHAIN_LIMIT)
let response = crate::service::check::legacy_chain(Some(nu_activation_height), chain.into_iter().rev(), &network, TEST_LEGACY_CHAIN_LIMIT)
.map_err(|error| error.to_string());
prop_assert_eq!(
@ -360,7 +360,7 @@ proptest! {
);
let response = crate::service::check::legacy_chain(
nu_activation_height,
Some(nu_activation_height),
chain.clone().into_iter().rev(),
&network,
TEST_LEGACY_CHAIN_LIMIT,
@ -380,7 +380,7 @@ proptest! {
fn at_least_one_transaction_with_valid_network_upgrade(
(network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, true, UNDER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy)
) {
let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), &network, TEST_LEGACY_CHAIN_LIMIT)
let response = crate::service::check::legacy_chain(Some(nu_activation_height), chain.into_iter().rev(), &network, TEST_LEGACY_CHAIN_LIMIT)
.map_err(|error| error.to_string());
prop_assert_eq!(response, Ok(()));