diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index 6e3ef56c4..7d51547ae 100644 --- a/zebra-chain/src/block.rs +++ b/zebra-chain/src/block.rs @@ -35,7 +35,7 @@ pub use commitment::{ }; pub use hash::Hash; pub use header::{BlockTimeError, CountedHeader, Header, ZCASH_BLOCK_VERSION}; -pub use height::Height; +pub use height::{Height, HeightDiff}; pub use serialize::{SerializedBlock, MAX_BLOCK_BYTES}; #[cfg(any(test, feature = "proptest-impl"))] diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index c7c968c4a..68c501ddc 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -6,9 +6,9 @@ use crate::serialization::SerializationError; /// The length of the chain back to the genesis block. /// -/// Block heights can't be added, but they can be *subtracted*, -/// to get a difference of block heights, represented as an `i32`, -/// and height differences can be added to block heights to get new heights. +/// Two [`Height`]s can't be added, but they can be *subtracted* to get their difference, +/// represented as an [`HeightDiff`]. This difference can then be added to or subtracted from a +/// [`Height`]. Note the similarity with `chrono::DateTime` and `chrono::Duration`. /// /// # Invariants /// @@ -64,70 +64,71 @@ impl Height { pub const MAX_EXPIRY_HEIGHT: Height = Height(499_999_999); } -impl Add for Height { - type Output = Option; +/// A difference between two [`Height`]s, possibly negative. +/// +/// This can represent the difference between any height values, +/// even if they are outside the valid height range (for example, in buggy RPC code). +pub type HeightDiff = i64; - fn add(self, rhs: Height) -> Option { - // We know that both values are positive integers. Therefore, the result is - // positive, and we can skip the conversions. The checked_add is required, - // because the result may overflow. - let height = self.0.checked_add(rhs.0)?; - let height = Height(height); +impl TryFrom for Height { + type Error = &'static str; - if height <= Height::MAX && height >= Height::MIN { - Some(height) + /// Checks that the `height` is within the valid [`Height`] range. + fn try_from(height: u32) -> Result { + // Check the bounds. + if Height::MIN.0 <= height && height <= Height::MAX.0 { + Ok(Height(height)) } else { - None + Err("heights must be less than or equal to Height::MAX") } } } -impl Sub for Height { - type Output = i32; - - /// Panics if the inputs or result are outside the valid i32 range. - fn sub(self, rhs: Height) -> i32 { - // We construct heights from integers without any checks, - // so the inputs or result could be out of range. - let lhs = i32::try_from(self.0) - .expect("out of range input `self`: inputs should be valid Heights"); - let rhs = - i32::try_from(rhs.0).expect("out of range input `rhs`: inputs should be valid Heights"); - lhs.checked_sub(rhs) - .expect("out of range result: valid input heights should yield a valid result") - } -} - // We don't implement Add or Sub, because they cause type inference issues for integer constants. -impl Add for Height { - type Output = Option; +impl Sub for Height { + type Output = HeightDiff; - fn add(self, rhs: i32) -> Option { - // Because we construct heights from integers without any checks, - // the input values could be outside the valid range for i32. - let lhs = i32::try_from(self.0).ok()?; - let result = lhs.checked_add(rhs)?; - let result = u32::try_from(result).ok()?; - match result { - h if (Height(h) <= Height::MAX && Height(h) >= Height::MIN) => Some(Height(h)), - _ => None, - } + /// Subtract two heights, returning the result, which can be negative. + /// Since [`HeightDiff`] is `i64` and [`Height`] is `u32`, the result is always correct. + fn sub(self, rhs: Height) -> Self::Output { + // All these conversions are exact, and the subtraction can't overflow or underflow. + let lhs = HeightDiff::from(self.0); + let rhs = HeightDiff::from(rhs.0); + + lhs - rhs } } -impl Sub for Height { +impl Sub for Height { + type Output = Option; + + /// Subtract a height difference from a height, returning `None` if the resulting height is + /// outside the valid `Height` range (this also checks the result is non-negative). + fn sub(self, rhs: HeightDiff) -> Option { + // We need to convert the height to [`i64`] so we can subtract negative [`HeightDiff`]s. + let lhs = HeightDiff::from(self.0); + let res = lhs - rhs; + + // Check the bounds. + let res = u32::try_from(res).ok()?; + Height::try_from(res).ok() + } +} + +impl Add for Height { type Output = Option; - fn sub(self, rhs: i32) -> Option { - // These checks are required, see above for details. - let lhs = i32::try_from(self.0).ok()?; - let result = lhs.checked_sub(rhs)?; - let result = u32::try_from(result).ok()?; - match result { - h if (Height(h) <= Height::MAX && Height(h) >= Height::MIN) => Some(Height(h)), - _ => None, - } + /// Add a height difference to a height, returning `None` if the resulting height is outside + /// the valid `Height` range (this also checks the result is non-negative). + fn add(self, rhs: HeightDiff) -> Option { + // We need to convert the height to [`i64`] so we can add negative [`HeightDiff`]s. + let lhs = i64::from(self.0); + let res = lhs + rhs; + + // Check the bounds. + let res = u32::try_from(res).ok()?; + Height::try_from(res).ok() } } @@ -136,22 +137,22 @@ fn operator_tests() { let _init_guard = zebra_test::init(); // Elementary checks. - assert_eq!(Some(Height(2)), Height(1) + Height(1)); - assert_eq!(None, Height::MAX + Height(1)); + assert_eq!(Some(Height(2)), Height(1) + 1); + assert_eq!(None, Height::MAX + 1); let height = Height(u32::pow(2, 31) - 2); assert!(height < Height::MAX); - let max_height = (height + Height(1)).expect("this addition should produce the max height"); + let max_height = (height + 1).expect("this addition should produce the max height"); assert!(height < max_height); assert!(max_height <= Height::MAX); assert_eq!(Height::MAX, max_height); - assert_eq!(None, max_height + Height(1)); + assert_eq!(None, max_height + 1); // Bad heights aren't caught at compile-time or runtime, until we add or subtract - assert_eq!(None, Height(Height::MAX_AS_U32 + 1) + Height(0)); - assert_eq!(None, Height(i32::MAX as u32) + Height(1)); - assert_eq!(None, Height(u32::MAX) + Height(0)); + assert_eq!(None, Height(Height::MAX_AS_U32 + 1) + 0); + assert_eq!(None, Height(i32::MAX as u32) + 1); + assert_eq!(None, Height(u32::MAX) + 0); assert_eq!(Some(Height(2)), Height(1) + 1); assert_eq!(None, Height::MAX + 1); @@ -162,14 +163,14 @@ fn operator_tests() { assert_eq!(None, Height(0) + -1); assert_eq!(Some(Height(Height::MAX_AS_U32 - 1)), Height::MAX + -1); - // Bad heights aren't caught at compile-time or runtime, until we add or subtract - // `+ 0` would also cause an error here, but it triggers a spurious clippy lint + // Bad heights aren't caught at compile-time or runtime, until we add or subtract, + // and the result is invalid assert_eq!(None, Height(Height::MAX_AS_U32 + 1) + 1); assert_eq!(None, Height(i32::MAX as u32) + 1); assert_eq!(None, Height(u32::MAX) + 1); // Adding negative numbers - assert_eq!(None, Height(i32::MAX as u32 + 1) + -1); + assert_eq!(Some(Height::MAX), Height(i32::MAX as u32 + 1) + -1); assert_eq!(None, Height(u32::MAX) + -1); assert_eq!(Some(Height(1)), Height(2) - 1); @@ -182,8 +183,9 @@ fn operator_tests() { assert_eq!(Some(Height::MAX), Height(Height::MAX_AS_U32 - 1) - -1); assert_eq!(None, Height::MAX - -1); - // Bad heights aren't caught at compile-time or runtime, until we add or subtract - assert_eq!(None, Height(i32::MAX as u32 + 1) - 1); + // Bad heights aren't caught at compile-time or runtime, until we add or subtract, + // and the result is invalid + assert_eq!(Some(Height::MAX), Height(i32::MAX as u32 + 1) - 1); assert_eq!(None, Height(u32::MAX) - 1); // Subtracting negative numbers @@ -191,13 +193,12 @@ fn operator_tests() { assert_eq!(None, Height(i32::MAX as u32) - -1); assert_eq!(None, Height(u32::MAX) - -1); - // Sub panics on out of range errors - assert_eq!(1, Height(2) - Height(1)); - assert_eq!(0, Height(1) - Height(1)); + assert_eq!(1, (Height(2) - Height(1))); + assert_eq!(0, (Height(1) - Height(1))); assert_eq!(-1, Height(0) - Height(1)); assert_eq!(-5, Height(2) - Height(7)); - assert_eq!(Height::MAX_AS_U32 as i32, Height::MAX - Height(0)); - assert_eq!(1, Height::MAX - Height(Height::MAX_AS_U32 - 1)); + assert_eq!(Height::MAX.0 as HeightDiff, (Height::MAX - Height(0))); + assert_eq!(1, (Height::MAX - Height(Height::MAX_AS_U32 - 1))); assert_eq!(-1, Height(Height::MAX_AS_U32 - 1) - Height::MAX); - assert_eq!(-(Height::MAX_AS_U32 as i32), Height(0) - Height::MAX); + assert_eq!(-(Height::MAX_AS_U32 as HeightDiff), Height(0) - Height::MAX); } diff --git a/zebra-chain/src/chain_tip.rs b/zebra-chain/src/chain_tip.rs index c24e2652e..e2d55a7da 100644 --- a/zebra-chain/src/chain_tip.rs +++ b/zebra-chain/src/chain_tip.rs @@ -95,26 +95,31 @@ pub trait ChainTip { Some(estimator.estimate_height_at(now)) } - /// Return an estimate of how many blocks there are ahead of Zebra's best chain tip - /// until the network chain tip, and Zebra's best chain tip height. + /// Return an estimate of how many blocks there are ahead of Zebra's best chain tip until the + /// network chain tip, and Zebra's best chain tip height. + /// + /// The first element in the returned tuple is the estimate. + /// The second element in the returned tuple is the current best chain tip. /// /// The estimate is calculated based on the current local time, the block time of the best tip /// and the height of the best tip. /// - /// This estimate may be negative if the current local time is behind the chain tip block's timestamp. + /// This estimate may be negative if the current local time is behind the chain tip block's + /// timestamp. + /// + /// Returns `None` if the state is empty. fn estimate_distance_to_network_chain_tip( &self, network: Network, - ) -> Option<(i32, block::Height)> { + ) -> Option<(block::HeightDiff, block::Height)> { let (current_height, current_block_time) = self.best_tip_height_and_block_time()?; let estimator = NetworkChainTipHeightEstimator::new(current_block_time, current_height, network); - Some(( - estimator.estimate_height_at(Utc::now()) - current_height, - current_height, - )) + let distance_to_tip = estimator.estimate_height_at(Utc::now()) - current_height; + + Some((distance_to_tip, current_height)) } } diff --git a/zebra-chain/src/chain_tip/mock.rs b/zebra-chain/src/chain_tip/mock.rs index c318c407b..980af3656 100644 --- a/zebra-chain/src/chain_tip/mock.rs +++ b/zebra-chain/src/chain_tip/mock.rs @@ -27,7 +27,7 @@ pub struct MockChainTipSender { best_tip_block_time: watch::Sender>>, /// A sender that sets the `estimate_distance_to_network_chain_tip` of a [`MockChainTip`]. - estimated_distance_to_network_chain_tip: watch::Sender>, + estimated_distance_to_network_chain_tip: watch::Sender>, } /// A mock [`ChainTip`] implementation that allows setting the `best_tip_height` externally. @@ -43,7 +43,7 @@ pub struct MockChainTip { best_tip_block_time: watch::Receiver>>, /// A mocked `estimate_distance_to_network_chain_tip` value set by the [`MockChainTipSender`]. - estimated_distance_to_network_chain_tip: watch::Receiver>, + estimated_distance_to_network_chain_tip: watch::Receiver>, } impl MockChainTip { @@ -112,7 +112,7 @@ impl ChainTip for MockChainTip { fn estimate_distance_to_network_chain_tip( &self, _network: Network, - ) -> Option<(i32, block::Height)> { + ) -> Option<(block::HeightDiff, block::Height)> { self.estimated_distance_to_network_chain_tip .borrow() .and_then(|estimated_distance| { @@ -179,7 +179,10 @@ impl MockChainTipSender { } /// Send a new estimated distance to network chain tip to the [`MockChainTip`]. - pub fn send_estimated_distance_to_network_chain_tip(&self, distance: impl Into>) { + pub fn send_estimated_distance_to_network_chain_tip( + &self, + distance: impl Into>, + ) { self.estimated_distance_to_network_chain_tip .send(distance.into()) .expect("attempt to send a best tip height to a dropped `MockChainTip`"); diff --git a/zebra-chain/src/chain_tip/network_chain_tip_height_estimator.rs b/zebra-chain/src/chain_tip/network_chain_tip_height_estimator.rs index 5d4e1f592..1c5e1cf08 100644 --- a/zebra-chain/src/chain_tip/network_chain_tip_height_estimator.rs +++ b/zebra-chain/src/chain_tip/network_chain_tip_height_estimator.rs @@ -5,7 +5,7 @@ use std::vec; use chrono::{DateTime, Duration, Utc}; use crate::{ - block, + block::{self, HeightDiff}, parameters::{Network, NetworkUpgrade}, }; @@ -87,12 +87,11 @@ impl NetworkChainTipHeightEstimator { /// The amount of blocks advanced is then used to extrapolate the amount to advance the /// `current_block_time`. fn estimate_up_to(&mut self, max_height: block::Height) { - let remaining_blocks = i64::from(max_height - self.current_height); + let remaining_blocks = max_height - self.current_height; if remaining_blocks > 0 { let target_spacing_seconds = self.current_target_spacing.num_seconds(); let time_to_activation = Duration::seconds(remaining_blocks * target_spacing_seconds); - self.current_block_time += time_to_activation; self.current_height = max_height; } @@ -119,21 +118,25 @@ impl NetworkChainTipHeightEstimator { time_difference_seconds -= 1; } - let block_difference = i32::try_from( - // Euclidean division is used so that the number is rounded towards negative infinity, - // so that fractionary values always round down to the previous height when going back - // in time (i.e., when the dividend is negative). This works because the divisor (the - // target spacing) is always positive. - time_difference_seconds.div_euclid(self.current_target_spacing.num_seconds()), - ) - .expect("time difference is too large"); + // Euclidean division is used so that the number is rounded towards negative infinity, + // so that fractionary values always round down to the previous height when going back + // in time (i.e., when the dividend is negative). This works because the divisor (the + // target spacing) is always positive. + let block_difference: HeightDiff = + time_difference_seconds.div_euclid(self.current_target_spacing.num_seconds()); - if -(block_difference as i64) > self.current_height.0 as i64 { + let current_height_as_diff = HeightDiff::from(self.current_height.0); + + if let Some(height_estimate) = self.current_height + block_difference { + height_estimate + } else if current_height_as_diff + block_difference < 0 { // Gracefully handle attempting to estimate a block before genesis. This can happen if // the local time is set incorrectly to a time too far in the past. block::Height(0) } else { - (self.current_height + block_difference).expect("block difference is too large") + // Gracefully handle attempting to estimate a block at a very large height. This can + // happen if the local time is set incorrectly to a time too far in the future. + block::Height::MAX } } } diff --git a/zebra-chain/src/chain_tip/tests/prop.rs b/zebra-chain/src/chain_tip/tests/prop.rs index 1543e9c12..1cbc7a537 100644 --- a/zebra-chain/src/chain_tip/tests/prop.rs +++ b/zebra-chain/src/chain_tip/tests/prop.rs @@ -71,10 +71,13 @@ fn estimate_time_difference( active_network_upgrade: NetworkUpgrade, ) -> Duration { let spacing_seconds = active_network_upgrade.target_spacing().num_seconds(); + let height_difference = end_height - start_height; - let height_difference = i64::from(end_height - start_height); - - Duration::seconds(height_difference * spacing_seconds) + if height_difference > 0 { + Duration::seconds(height_difference * spacing_seconds) + } else { + Duration::zero() + } } /// Use `displacement` to get a displacement duration between zero and the target spacing of the diff --git a/zebra-chain/src/parameters/network.rs b/zebra-chain/src/parameters/network.rs index d23abe944..6c6319ce7 100644 --- a/zebra-chain/src/parameters/network.rs +++ b/zebra-chain/src/parameters/network.rs @@ -4,7 +4,10 @@ use std::{fmt, str::FromStr}; use thiserror::Error; -use crate::{block::Height, parameters::NetworkUpgrade::Canopy}; +use crate::{ + block::{Height, HeightDiff}, + parameters::NetworkUpgrade::Canopy, +}; #[cfg(any(test, feature = "proptest-impl"))] use proptest_derive::Arbitrary; @@ -46,7 +49,7 @@ mod tests; /// period. Therefore Zebra must validate those blocks during the grace period using checkpoints. /// Therefore the mandatory checkpoint height ([`Network::mandatory_checkpoint_height`]) must be /// after the grace period. -const ZIP_212_GRACE_PERIOD_DURATION: i32 = 32_256; +const ZIP_212_GRACE_PERIOD_DURATION: HeightDiff = 32_256; /// An enum describing the possible network choices. #[derive(Copy, Clone, Debug, Default, Eq, PartialEq, Hash, Serialize, Deserialize)] diff --git a/zebra-consensus/src/block/subsidy/funding_streams.rs b/zebra-consensus/src/block/subsidy/funding_streams.rs index d661223c5..977b14a56 100644 --- a/zebra-consensus/src/block/subsidy/funding_streams.rs +++ b/zebra-consensus/src/block/subsidy/funding_streams.rs @@ -69,13 +69,23 @@ pub fn height_for_first_halving(network: Network) -> Height { /// /// [7.10]: https://zips.z.cash/protocol/protocol.pdf#fundingstreams fn funding_stream_address_period(height: Height, network: Network) -> u32 { - // - Spec equation: `address_period = floor((height - height_for_halving(1) - post_blossom_halving_interval)/funding_stream_address_change_interval)`: - // https://zips.z.cash/protocol/protocol.pdf#fundingstreams - // - In Rust, "integer division rounds towards zero": - // https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators + // Spec equation: `address_period = floor((height - (height_for_halving(1) - post_blossom_halving_interval))/funding_stream_address_change_interval)`, + // + // + // Note that the brackets make it so the post blossom halving interval is added to the total. + // + // In Rust, "integer division rounds towards zero": + // // This is the same as `floor()`, because these numbers are all positive. - (height.0 + (POST_BLOSSOM_HALVING_INTERVAL.0) - (height_for_first_halving(network).0)) - / (FUNDING_STREAM_ADDRESS_CHANGE_INTERVAL.0) + + let height_after_first_halving = height - height_for_first_halving(network); + + let address_period = (height_after_first_halving + POST_BLOSSOM_HALVING_INTERVAL) + / FUNDING_STREAM_ADDRESS_CHANGE_INTERVAL; + + address_period + .try_into() + .expect("all values are positive and smaller than the input height") } /// Returns the position in the address slice for each funding stream diff --git a/zebra-consensus/src/block/subsidy/general.rs b/zebra-consensus/src/block/subsidy/general.rs index e959a7929..b3cb02317 100644 --- a/zebra-consensus/src/block/subsidy/general.rs +++ b/zebra-consensus/src/block/subsidy/general.rs @@ -6,7 +6,7 @@ use std::{collections::HashSet, convert::TryFrom}; use zebra_chain::{ amount::{Amount, Error, NonNegative}, - block::Height, + block::{Height, HeightDiff}, parameters::{Network, NetworkUpgrade::*}, transaction::Transaction, }; @@ -27,35 +27,37 @@ pub fn halving_divisor(height: Height, network: Network) -> Option { if height < SLOW_START_SHIFT { unreachable!( - "unsupported block height: checkpoints should handle blocks below {:?}", - SLOW_START_SHIFT + "unsupported block height {height:?}: checkpoints should handle blocks below {SLOW_START_SHIFT:?}", ) } else if height < blossom_height { - let pre_blossom_height: u32 = (height - SLOW_START_SHIFT) - .try_into() - .expect("height is above slow start, and the difference fits in u32"); - let halving_shift = pre_blossom_height / PRE_BLOSSOM_HALVING_INTERVAL.0; + let pre_blossom_height = height - SLOW_START_SHIFT; + let halving_shift = pre_blossom_height / PRE_BLOSSOM_HALVING_INTERVAL; let halving_div = 1u64 - .checked_shl(halving_shift) + .checked_shl( + halving_shift + .try_into() + .expect("already checked for negatives"), + ) .expect("pre-blossom heights produce small shifts"); Some(halving_div) } else { - let pre_blossom_height: u32 = (blossom_height - SLOW_START_SHIFT) - .try_into() - .expect("blossom height is above slow start, and the difference fits in u32"); - let scaled_pre_blossom_height = pre_blossom_height * BLOSSOM_POW_TARGET_SPACING_RATIO; + let pre_blossom_height = blossom_height - SLOW_START_SHIFT; + let scaled_pre_blossom_height = pre_blossom_height + * HeightDiff::try_from(BLOSSOM_POW_TARGET_SPACING_RATIO).expect("constant is positive"); - let post_blossom_height: u32 = (height - blossom_height) - .try_into() - .expect("height is above blossom, and the difference fits in u32"); + let post_blossom_height = height - blossom_height; let halving_shift = - (scaled_pre_blossom_height + post_blossom_height) / POST_BLOSSOM_HALVING_INTERVAL.0; + (scaled_pre_blossom_height + post_blossom_height) / POST_BLOSSOM_HALVING_INTERVAL; // Some far-future shifts can be more than 63 bits - 1u64.checked_shl(halving_shift) + 1u64.checked_shl( + halving_shift + .try_into() + .expect("already checked for negatives"), + ) } } @@ -77,8 +79,7 @@ pub fn block_subsidy(height: Height, network: Network) -> Result Height(1_116_000), }; + assert_eq!( + 1, + halving_divisor((SLOW_START_INTERVAL + 1).unwrap(), network).unwrap() + ); assert_eq!( 1, halving_divisor((blossom_height - 1).unwrap(), network).unwrap() @@ -163,7 +168,7 @@ mod test { assert_eq!( 8, halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 2)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 2)).unwrap(), network ) .unwrap() @@ -172,7 +177,7 @@ mod test { assert_eq!( 1024, halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 9)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 9)).unwrap(), network ) .unwrap() @@ -180,7 +185,7 @@ mod test { assert_eq!( 1024 * 1024, halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 19)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 19)).unwrap(), network ) .unwrap() @@ -188,7 +193,7 @@ mod test { assert_eq!( 1024 * 1024 * 1024, halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 29)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 29)).unwrap(), network ) .unwrap() @@ -196,7 +201,7 @@ mod test { assert_eq!( 1024 * 1024 * 1024 * 1024, halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 39)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 39)).unwrap(), network ) .unwrap() @@ -206,7 +211,7 @@ mod test { assert_eq!( (i64::MAX as u64 + 1), halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 62)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 62)).unwrap(), network ) .unwrap(), @@ -216,7 +221,7 @@ mod test { assert_eq!( None, halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 63)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 63)).unwrap(), network, ), ); @@ -224,7 +229,7 @@ mod test { assert_eq!( None, halving_divisor( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 64)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 64)).unwrap(), network, ), ); @@ -264,6 +269,10 @@ mod test { // After slow-start mining and before Blossom the block subsidy is 12.5 ZEC // https://z.cash/support/faq/#what-is-slow-start-mining + assert_eq!( + Amount::try_from(1_250_000_000), + block_subsidy((SLOW_START_INTERVAL + 1).unwrap(), network) + ); assert_eq!( Amount::try_from(1_250_000_000), block_subsidy((blossom_height - 1).unwrap(), network) @@ -298,7 +307,7 @@ mod test { assert_eq!( Amount::try_from(4_882_812), block_subsidy( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 6)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 6)).unwrap(), network ) ); @@ -308,7 +317,7 @@ mod test { assert_eq!( Amount::try_from(1), block_subsidy( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 28)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 28)).unwrap(), network ) ); @@ -318,20 +327,71 @@ mod test { assert_eq!( Amount::try_from(0), block_subsidy( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 29)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 29)).unwrap(), network ) ); - // The largest possible divisor assert_eq!( Amount::try_from(0), block_subsidy( - (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL.0 as i32 * 62)).unwrap(), + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 39)).unwrap(), network ) ); + assert_eq!( + Amount::try_from(0), + block_subsidy( + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 49)).unwrap(), + network + ) + ); + + assert_eq!( + Amount::try_from(0), + block_subsidy( + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 59)).unwrap(), + network + ) + ); + + // The largest possible integer divisor + assert_eq!( + Amount::try_from(0), + block_subsidy( + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 62)).unwrap(), + network + ) + ); + + // Other large divisors which should also result in zero + assert_eq!( + Amount::try_from(0), + block_subsidy( + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 63)).unwrap(), + network + ) + ); + + assert_eq!( + Amount::try_from(0), + block_subsidy( + (first_halving_height + (POST_BLOSSOM_HALVING_INTERVAL * 64)).unwrap(), + network + ) + ); + + assert_eq!( + Amount::try_from(0), + block_subsidy(Height(Height::MAX_AS_U32 / 4), network) + ); + assert_eq!( + Amount::try_from(0), + block_subsidy(Height(Height::MAX_AS_U32 / 2), network) + ); + assert_eq!(Amount::try_from(0), block_subsidy(Height::MAX, network)); + Ok(()) } } diff --git a/zebra-consensus/src/checkpoint/list/tests.rs b/zebra-consensus/src/checkpoint/list/tests.rs index 29bc16928..60b4ec9ef 100644 --- a/zebra-consensus/src/checkpoint/list/tests.rs +++ b/zebra-consensus/src/checkpoint/list/tests.rs @@ -1,15 +1,15 @@ //! Tests for CheckpointList -use super::*; - use std::sync::Arc; -use zebra_chain::parameters::{Network, Network::*}; use zebra_chain::{ - block::{self, Block}, + block::{self, Block, HeightDiff}, + parameters::{Network, Network::*}, serialization::ZcashDeserialize, }; +use super::*; + /// Make a checkpoint list containing only the genesis block #[test] fn checkpoint_list_genesis() -> Result<(), BoxError> { @@ -290,7 +290,8 @@ fn checkpoint_list_hard_coded_max_gap(network: Network) -> Result<(), BoxError> assert_eq!(heights.next(), Some(&previous_height)); for height in heights { - let height_limit = (previous_height + (crate::MAX_CHECKPOINT_HEIGHT_GAP as i32)).unwrap(); + let height_limit = + (previous_height + (crate::MAX_CHECKPOINT_HEIGHT_GAP as HeightDiff)).unwrap(); assert!( height <= &height_limit, "Checkpoint gaps must be within MAX_CHECKPOINT_HEIGHT_GAP" diff --git a/zebra-consensus/src/parameters/subsidy.rs b/zebra-consensus/src/parameters/subsidy.rs index a6b0d3808..ba8f910d8 100644 --- a/zebra-consensus/src/parameters/subsidy.rs +++ b/zebra-consensus/src/parameters/subsidy.rs @@ -4,7 +4,11 @@ use std::collections::HashMap; use lazy_static::lazy_static; -use zebra_chain::{amount::COIN, block::Height, parameters::Network}; +use zebra_chain::{ + amount::COIN, + block::{Height, HeightDiff}, + parameters::Network, +}; /// An initial period from Genesis to this Height where the block subsidy is gradually incremented. [What is slow-start mining][slow-mining] /// @@ -33,11 +37,11 @@ pub const BLOSSOM_POW_TARGET_SPACING_RATIO: u32 = 2; /// Halving is at about every 4 years, before Blossom block time is 150 seconds. /// /// `(60 * 60 * 24 * 365 * 4) / 150 = 840960` -pub const PRE_BLOSSOM_HALVING_INTERVAL: Height = Height(840_000); +pub const PRE_BLOSSOM_HALVING_INTERVAL: HeightDiff = 840_000; /// After Blossom the block time is reduced to 75 seconds but halving period should remain around 4 years. -pub const POST_BLOSSOM_HALVING_INTERVAL: Height = - Height(PRE_BLOSSOM_HALVING_INTERVAL.0 * BLOSSOM_POW_TARGET_SPACING_RATIO); +pub const POST_BLOSSOM_HALVING_INTERVAL: HeightDiff = + PRE_BLOSSOM_HALVING_INTERVAL * (BLOSSOM_POW_TARGET_SPACING_RATIO as HeightDiff); /// The first halving height in the testnet is at block height `1_116_000` /// as specified in [protocol specification §7.10.1][7.10.1] @@ -133,8 +137,7 @@ lazy_static! { /// as described in [protocol specification §7.10.1][7.10.1]. /// /// [7.10.1]: https://zips.z.cash/protocol/protocol.pdf#zip214fundingstreams -pub const FUNDING_STREAM_ADDRESS_CHANGE_INTERVAL: Height = - Height(POST_BLOSSOM_HALVING_INTERVAL.0 / 48); +pub const FUNDING_STREAM_ADDRESS_CHANGE_INTERVAL: HeightDiff = POST_BLOSSOM_HALVING_INTERVAL / 48; /// Number of addresses for each funding stream in the Mainnet. /// In the spec ([protocol specification §7.10][7.10]) this is defined as: `fs.addressindex(fs.endheight - 1)` diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs b/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs index 8658f9299..8b6b9174a 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs @@ -2,6 +2,7 @@ use jsonrpc_core::ErrorCode; +use zebra_chain::block; use zebra_consensus::FundingStreamReceiver::{self, *}; /// When long polling, the amount of time we wait between mempool queries. @@ -42,7 +43,7 @@ pub const GET_BLOCK_TEMPLATE_CAPABILITIES_FIELD: &[&str] = &["proposal"]; /// > and clock time varies between nodes. /// > /// > -pub const MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP: i32 = 100; +pub const MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP: block::HeightDiff = 100; /// The RPC error code used by `zcashd` for when it's still downloading initial blocks. /// diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs index 16d4691d8..f7ba6adfd 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs @@ -173,7 +173,7 @@ where || estimated_distance_to_chain_tip > MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP { tracing::info!( - estimated_distance_to_chain_tip, + ?estimated_distance_to_chain_tip, ?local_tip_height, "Zebra has not synced to the chain tip. \ Hint: check your network connection, clock, and time zone settings." @@ -183,7 +183,7 @@ where code: NOT_SYNCED_ERROR_CODE, message: format!( "Zebra has not synced to the chain tip, \ - estimated distance: {estimated_distance_to_chain_tip}, \ + estimated distance: {estimated_distance_to_chain_tip:?}, \ local tip: {local_tip_height:?}. \ Hint: check your network connection, clock, and time zone settings." ), diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index b91fe29d7..166d93dd6 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -15,6 +15,8 @@ pub use zebra_chain::transparent::MIN_TRANSPARENT_COINBASE_MATURITY; /// early non-finalized blocks, or finalized blocks. But if that chain becomes /// the best chain, all non-finalized blocks past the [`MAX_BLOCK_REORG_HEIGHT`] /// will be finalized. This includes all mature coinbase outputs. +// +// TODO: change to HeightDiff pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRANSPARENT_COINBASE_MATURITY - 1; /// The database format version, incremented each time the database format changes. diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 0fe50c838..ea4a63b6a 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -42,7 +42,7 @@ use tracing::{instrument, Instrument, Span}; use tower::buffer::Buffer; use zebra_chain::{ - block::{self, CountedHeader}, + block::{self, CountedHeader, HeightDiff}, diagnostic::CodeTimer, parameters::{Network, NetworkUpgrade}, }; @@ -391,7 +391,8 @@ impl StateService { ); let full_verifier_utxo_lookahead = max_checkpoint_height - - i32::try_from(checkpoint_verify_concurrency_limit).expect("fits in i32"); + - HeightDiff::try_from(checkpoint_verify_concurrency_limit) + .expect("fits in HeightDiff"); let full_verifier_utxo_lookahead = full_verifier_utxo_lookahead.expect("unexpected negative height"); diff --git a/zebra-state/src/service/check/utxo.rs b/zebra-state/src/service/check/utxo.rs index 24242c642..d29e5bca0 100644 --- a/zebra-state/src/service/check/utxo.rs +++ b/zebra-state/src/service/check/utxo.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use zebra_chain::{ - amount, block, + amount, transparent::{self, utxos_from_ordered_utxos, CoinbaseSpendRestriction::*}, }; @@ -200,7 +200,7 @@ pub fn transparent_coinbase_spend( match spend_restriction { OnlyShieldedOutputs { spend_height } => { let min_spend_height = - utxo.utxo.height + block::Height(MIN_TRANSPARENT_COINBASE_MATURITY); + utxo.utxo.height + MIN_TRANSPARENT_COINBASE_MATURITY.try_into().unwrap(); let min_spend_height = min_spend_height.expect("valid UTXOs have coinbase heights far below Height::MAX"); if spend_height >= min_spend_height { diff --git a/zebrad/src/components/inbound/downloads.rs b/zebrad/src/components/inbound/downloads.rs index a8315b758..aa8b2cf6c 100644 --- a/zebrad/src/components/inbound/downloads.rs +++ b/zebrad/src/components/inbound/downloads.rs @@ -2,7 +2,6 @@ use std::{ collections::HashMap, - convert::TryFrom, pin::Pin, task::{Context, Poll}, }; @@ -17,7 +16,10 @@ use tokio::{sync::oneshot, task::JoinHandle}; use tower::{Service, ServiceExt}; use tracing_futures::Instrument; -use zebra_chain::{block, chain_tip::ChainTip}; +use zebra_chain::{ + block::{self, HeightDiff}, + chain_tip::ChainTip, +}; use zebra_network as zn; use zebra_state as zs; @@ -281,7 +283,8 @@ where let tip_height = latest_chain_tip.best_tip_height(); let max_lookahead_height = if let Some(tip_height) = tip_height { - let lookahead = i32::try_from(full_verify_concurrency_limit).expect("fits in i32"); + let lookahead = HeightDiff::try_from(full_verify_concurrency_limit) + .expect("fits in HeightDiff"); (tip_height + lookahead).expect("tip is much lower than Height::MAX") } else { let genesis_lookahead = diff --git a/zebrad/src/components/sync.rs b/zebrad/src/components/sync.rs index e74be2420..c0cc3889e 100644 --- a/zebrad/src/components/sync.rs +++ b/zebrad/src/components/sync.rs @@ -15,7 +15,7 @@ use tower::{ }; use zebra_chain::{ - block::{self, Height}, + block::{self, Height, HeightDiff}, chain_tip::ChainTip, parameters::genesis_hash, }; @@ -165,7 +165,7 @@ const FINAL_CHECKPOINT_BLOCK_VERIFY_TIMEOUT: Duration = Duration::from_secs(2 * /// The number of blocks after the final checkpoint that get the shorter timeout. /// /// We've only seen this error on the first few blocks after the final checkpoint. -const FINAL_CHECKPOINT_BLOCK_VERIFY_TIMEOUT_LIMIT: i32 = 100; +const FINAL_CHECKPOINT_BLOCK_VERIFY_TIMEOUT_LIMIT: HeightDiff = 100; /// Controls how long we wait to restart syncing after finishing a sync run. /// diff --git a/zebrad/src/components/sync/downloads.rs b/zebrad/src/components/sync/downloads.rs index 4b6b5f002..fde200f7f 100644 --- a/zebrad/src/components/sync/downloads.rs +++ b/zebrad/src/components/sync/downloads.rs @@ -24,7 +24,7 @@ use tower::{hedge, Service, ServiceExt}; use tracing_futures::Instrument; use zebra_chain::{ - block::{self, Height}, + block::{self, Height, HeightDiff}, chain_tip::ChainTip, }; use zebra_network as zn; @@ -56,7 +56,7 @@ pub const VERIFICATION_PIPELINE_SCALING_MULTIPLIER: usize = 2; /// The maximum height difference between Zebra's state tip and a downloaded block. /// Blocks higher than this will get dropped and return an error. -pub const VERIFICATION_PIPELINE_DROP_LIMIT: i32 = 50_000; +pub const VERIFICATION_PIPELINE_DROP_LIMIT: HeightDiff = 50_000; #[derive(Copy, Clone, Debug)] pub(super) struct AlwaysHedge; @@ -388,10 +388,10 @@ where let (lookahead_drop_height, lookahead_pause_height, lookahead_reset_height) = if let Some(tip_height) = tip_height { // Scale the height limit with the lookahead limit, // so users with low capacity or under DoS can reduce them both. - let lookahead_pause = i32::try_from( + let lookahead_pause = HeightDiff::try_from( lookahead_limit + lookahead_limit * VERIFICATION_PIPELINE_SCALING_MULTIPLIER, ) - .expect("fits in i32"); + .expect("fits in HeightDiff"); ((tip_height + VERIFICATION_PIPELINE_DROP_LIMIT).expect("tip is much lower than Height::MAX"), diff --git a/zebrad/src/components/sync/progress.rs b/zebrad/src/components/sync/progress.rs index b11202b6b..113bb36d5 100644 --- a/zebrad/src/components/sync/progress.rs +++ b/zebrad/src/components/sync/progress.rs @@ -6,7 +6,7 @@ use chrono::Utc; use num_integer::div_ceil; use zebra_chain::{ - block::Height, + block::{Height, HeightDiff}, chain_sync_status::ChainSyncStatus, chain_tip::ChainTip, fmt::humantime_seconds, @@ -23,14 +23,14 @@ const LOG_INTERVAL: Duration = Duration::from_secs(60); /// The number of blocks we consider to be close to the tip. /// /// Most chain forks are 1-7 blocks long. -const MAX_CLOSE_TO_TIP_BLOCKS: i32 = 1; +const MAX_CLOSE_TO_TIP_BLOCKS: HeightDiff = 1; /// Skip slow sync warnings when we are this close to the tip. /// /// In testing, we've seen warnings around 30 blocks. /// /// TODO: replace with `MAX_CLOSE_TO_TIP_BLOCKS` after fixing slow syncing near tip (#3375) -const MIN_SYNC_WARNING_BLOCKS: i32 = 60; +const MIN_SYNC_WARNING_BLOCKS: HeightDiff = 60; /// The number of fractional digits in sync percentages. const SYNC_PERCENT_FRAC_DIGITS: usize = 3; @@ -49,6 +49,8 @@ const SYNC_PERCENT_FRAC_DIGITS: usize = 3; /// /// We might add tests that sync from a cached tip state, /// so we only allow a few extra blocks here. +// +// TODO: change to HeightDiff? const MIN_BLOCKS_MINED_AFTER_CHECKPOINT_UPDATE: u32 = 10; /// Logs Zebra's estimated progress towards the chain tip every minute or so. @@ -67,9 +69,7 @@ pub async fn show_block_chain_progress( // and the automated tests for that update. let min_after_checkpoint_blocks = MAX_BLOCK_REORG_HEIGHT + MIN_BLOCKS_MINED_AFTER_CHECKPOINT_UPDATE; - let min_after_checkpoint_blocks: i32 = min_after_checkpoint_blocks - .try_into() - .expect("constant fits in i32"); + let min_after_checkpoint_blocks: HeightDiff = min_after_checkpoint_blocks.into(); // The minimum height of the valid best chain, based on: // - the hard-coded checkpoint height, @@ -128,7 +128,10 @@ pub async fn show_block_chain_progress( frac = SYNC_PERCENT_FRAC_DIGITS, ); - let remaining_sync_blocks = estimated_height - current_height; + let mut remaining_sync_blocks = estimated_height - current_height; + if remaining_sync_blocks < 0 { + remaining_sync_blocks = 0; + } // Work out how long it has been since the state height has increased. //