From d5f31e7f190b1f406683b8cfeb63402cd17335a2 Mon Sep 17 00:00:00 2001 From: Marek Date: Wed, 15 Mar 2023 15:41:18 +0100 Subject: [PATCH 01/20] Unify the `impl`s of `Sub` and `Add` for `Height` --- zebra-chain/src/block/height.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index c7c968c4ab1..bd6a40ce70a 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -83,18 +83,19 @@ impl Add for Height { } 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") + type Output = Option; + + fn sub(self, rhs: Height) -> Self::Output { + // Perform the subtraction. + let result = self.0.checked_sub(rhs.0)?; + let height = Height(result); + + // Check the bounds. + if Height::MIN <= height && height <= Height::MAX { + Some(height) + } else { + None + } } } From ca9574fc68c07344cc17d2e00886f24d7f7cfa33 Mon Sep 17 00:00:00 2001 From: Marek Date: Wed, 15 Mar 2023 15:46:46 +0100 Subject: [PATCH 02/20] Adjust tests for `Height` subtraction --- zebra-chain/src/block/height.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index bd6a40ce70a..6866072d3ca 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -192,13 +192,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(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!(-1, Height(Height::MAX_AS_U32 - 1) - Height::MAX); - assert_eq!(-(Height::MAX_AS_U32 as i32), Height(0) - Height::MAX); + assert_eq!(1, (Height(2) - Height(1)).unwrap().0); + assert_eq!(0, (Height(1) - Height(1)).unwrap().0); + assert_eq!(None, Height(0) - Height(1)); + assert_eq!(None, Height(2) - Height(7)); + assert_eq!(Height::MAX, (Height::MAX - Height(0)).unwrap()); + assert_eq!(1, (Height::MAX - Height(Height::MAX_AS_U32 - 1)).unwrap().0); + assert_eq!(None, Height(Height::MAX_AS_U32 - 1) - Height::MAX); + assert_eq!(None, Height(0) - Height::MAX); } From 3ba92d57409cff01c81c770be1a835361149b63b Mon Sep 17 00:00:00 2001 From: Marek Date: Wed, 15 Mar 2023 15:47:56 +0100 Subject: [PATCH 03/20] Use `Height` instead of `i32` --- zebra-chain/src/chain_tip.rs | 19 ++++++---- zebra-chain/src/chain_tip/mock.rs | 11 ++++-- .../network_chain_tip_height_estimator.rs | 5 ++- zebra-chain/src/chain_tip/tests/prop.rs | 2 +- zebra-consensus/src/block/subsidy/general.rs | 38 +++++++++---------- .../get_block_template_rpcs/constants.rs | 3 +- .../get_block_template.rs | 4 +- zebrad/src/components/sync/progress.rs | 7 ++-- 8 files changed, 48 insertions(+), 41 deletions(-) diff --git a/zebra-chain/src/chain_tip.rs b/zebra-chain/src/chain_tip.rs index c24e2652eab..f10b1ddc1cf 100644 --- a/zebra-chain/src/chain_tip.rs +++ b/zebra-chain/src/chain_tip.rs @@ -95,26 +95,29 @@ 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 [`None`] if the current local time is behind the chain tip block's + /// timestamp. fn estimate_distance_to_network_chain_tip( &self, network: Network, - ) -> Option<(i32, block::Height)> { + ) -> Option<(block::Height, 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; + + distance_to_tip.map(|distance_to_tip| (distance_to_tip, current_height)) } } diff --git a/zebra-chain/src/chain_tip/mock.rs b/zebra-chain/src/chain_tip/mock.rs index c318c407bdb..2d318130791 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::Height, 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 5d4e1f59237..247842bf246 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 @@ -87,9 +87,10 @@ 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 { + if let Some(remaining_blocks) = remaining_blocks { + let remaining_blocks = i64::from(remaining_blocks.0); let target_spacing_seconds = self.current_target_spacing.num_seconds(); let time_to_activation = Duration::seconds(remaining_blocks * target_spacing_seconds); diff --git a/zebra-chain/src/chain_tip/tests/prop.rs b/zebra-chain/src/chain_tip/tests/prop.rs index 1543e9c12cf..660e399bbe5 100644 --- a/zebra-chain/src/chain_tip/tests/prop.rs +++ b/zebra-chain/src/chain_tip/tests/prop.rs @@ -72,7 +72,7 @@ fn estimate_time_difference( ) -> Duration { let spacing_seconds = active_network_upgrade.target_spacing().num_seconds(); - let height_difference = i64::from(end_height - start_height); + let height_difference = (end_height - start_height).map_or(0, |height| height.0.into()); Duration::seconds(height_difference * spacing_seconds) } diff --git a/zebra-consensus/src/block/subsidy/general.rs b/zebra-consensus/src/block/subsidy/general.rs index e959a7929d7..14587e39314 100644 --- a/zebra-consensus/src/block/subsidy/general.rs +++ b/zebra-consensus/src/block/subsidy/general.rs @@ -25,37 +25,35 @@ pub fn halving_divisor(height: Height, network: Network) -> Option { .activation_height(network) .expect("blossom activation height should be available"); - if height < SLOW_START_SHIFT { + let slow_start_shift_diff = height - SLOW_START_SHIFT; + let slow_start_shift_diff = slow_start_shift_diff.unwrap_or_else(|| { unreachable!( "unsupported block 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 halving_div = 1u64 - .checked_shl(halving_shift) - .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"); + if let Some(post_blossom_height) = height - blossom_height { + // The Blossom height is way above `SLOW_START_SHIFT`. + let pre_blossom_height = (blossom_height - SLOW_START_SHIFT) + .expect("Blossom height must be above SlowStartShift.") + .0; let scaled_pre_blossom_height = pre_blossom_height * BLOSSOM_POW_TARGET_SPACING_RATIO; - let post_blossom_height: u32 = (height - blossom_height) - .try_into() - .expect("height is above blossom, and the difference fits in u32"); - let halving_shift = - (scaled_pre_blossom_height + post_blossom_height) / POST_BLOSSOM_HALVING_INTERVAL.0; + (scaled_pre_blossom_height + post_blossom_height.0) / POST_BLOSSOM_HALVING_INTERVAL.0; // Some far-future shifts can be more than 63 bits 1u64.checked_shl(halving_shift) + } else { + let pre_blossom_height = slow_start_shift_diff.0; + let halving_shift = pre_blossom_height / PRE_BLOSSOM_HALVING_INTERVAL.0; + + let halving_div = 1u64 + .checked_shl(halving_shift) + .expect("pre-blossom heights produce small shifts"); + + Some(halving_div) } } 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 8658f9299a1..ef2f2232f89 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::Height = block::Height(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 16d4691d864..f7ba6adfd60 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/zebrad/src/components/sync/progress.rs b/zebrad/src/components/sync/progress.rs index b11202b6b1c..c52f01d0367 100644 --- a/zebrad/src/components/sync/progress.rs +++ b/zebrad/src/components/sync/progress.rs @@ -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: u32 = 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: u32 = 60; /// The number of fractional digits in sync percentages. const SYNC_PERCENT_FRAC_DIGITS: usize = 3; @@ -128,7 +128,8 @@ pub async fn show_block_chain_progress( frac = SYNC_PERCENT_FRAC_DIGITS, ); - let remaining_sync_blocks = estimated_height - current_height; + let remaining_sync_blocks = + (estimated_height - current_height).map_or(0, |height| height.0); // Work out how long it has been since the state height has increased. // From f97b104c2f290a46b956869066665e0bb2678ad2 Mon Sep 17 00:00:00 2001 From: Marek Date: Wed, 15 Mar 2023 22:42:55 +0100 Subject: [PATCH 04/20] Use `block:Height` in RPC tests --- .../methods/tests/snapshot/get_block_template_rpcs.rs | 4 ++-- zebra-rpc/src/methods/tests/vectors.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs b/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs index 48fcfd697e3..995f906b9d2 100644 --- a/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs @@ -12,7 +12,7 @@ use insta::Settings; use tower::{buffer::Buffer, Service}; use zebra_chain::{ - block::Hash, + block::{self, Hash}, chain_sync_status::MockSyncStatus, chain_tip::mock::MockChainTip, parameters::{Network, NetworkUpgrade}, @@ -123,7 +123,7 @@ pub async fn test_responses( let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new(); mock_chain_tip_sender.send_best_tip_height(fake_tip_height); mock_chain_tip_sender.send_best_tip_hash(fake_tip_hash); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(block::Height(0))); let mock_address_book = MockAddressBookPeers::new(vec![MetaAddr::new_initial_peer(SocketAddr::new( diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 50641ddc5ea..7ed95661d4a 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -1148,7 +1148,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new(); mock_chain_tip_sender.send_best_tip_height(fake_tip_height); mock_chain_tip_sender.send_best_tip_hash(fake_tip_hash); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(0))); // Init RPC let get_block_template_rpc = GetBlockTemplateRpcImpl::new( @@ -1251,7 +1251,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { mempool.expect_no_requests().await; - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(200)); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(200))); let get_block_template_sync_error = get_block_template_rpc .get_block_template(None) .await @@ -1264,7 +1264,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { mock_sync_status.set_is_close_to_tip(false); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(0))); let get_block_template_sync_error = get_block_template_rpc .get_block_template(None) .await @@ -1275,7 +1275,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { ErrorCode::ServerError(-10) ); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(200)); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(200))); let get_block_template_sync_error = get_block_template_rpc .get_block_template(None) .await @@ -1543,7 +1543,7 @@ async fn rpc_getdifficulty() { let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new(); mock_chain_tip_sender.send_best_tip_height(fake_tip_height); mock_chain_tip_sender.send_best_tip_hash(fake_tip_hash); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(0))); // Init RPC let get_block_template_rpc = GetBlockTemplateRpcImpl::new( From e6dbc734074bb47e85a224c53a69478e3a59f6fa Mon Sep 17 00:00:00 2001 From: Marek Date: Thu, 16 Mar 2023 23:22:14 +0100 Subject: [PATCH 05/20] Use `let .. else` statement Co-authored-by: Arya --- zebra-consensus/src/block/subsidy/general.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zebra-consensus/src/block/subsidy/general.rs b/zebra-consensus/src/block/subsidy/general.rs index 14587e39314..ab9db53f8ac 100644 --- a/zebra-consensus/src/block/subsidy/general.rs +++ b/zebra-consensus/src/block/subsidy/general.rs @@ -25,8 +25,7 @@ pub fn halving_divisor(height: Height, network: Network) -> Option { .activation_height(network) .expect("blossom activation height should be available"); - let slow_start_shift_diff = height - SLOW_START_SHIFT; - let slow_start_shift_diff = slow_start_shift_diff.unwrap_or_else(|| { + let Some(slow_start_shift_diff) = height - SLOW_START_SHIFT else { unreachable!( "unsupported block height: checkpoints should handle blocks below {:?}", SLOW_START_SHIFT From 5e4bf7698a1a465f6b099bd7901464d4d8ce7b35 Mon Sep 17 00:00:00 2001 From: Arya Date: Thu, 16 Mar 2023 22:28:46 -0400 Subject: [PATCH 06/20] Update zebra-consensus/src/block/subsidy/general.rs --- zebra-consensus/src/block/subsidy/general.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-consensus/src/block/subsidy/general.rs b/zebra-consensus/src/block/subsidy/general.rs index ab9db53f8ac..870b9621853 100644 --- a/zebra-consensus/src/block/subsidy/general.rs +++ b/zebra-consensus/src/block/subsidy/general.rs @@ -30,7 +30,7 @@ pub fn halving_divisor(height: Height, network: Network) -> Option { "unsupported block height: checkpoints should handle blocks below {:?}", SLOW_START_SHIFT ) - }); + }; if let Some(post_blossom_height) = height - blossom_height { // The Blossom height is way above `SLOW_START_SHIFT`. From 9dd463e05e70f32855dd3f2d154831eb25a06c0d Mon Sep 17 00:00:00 2001 From: Marek Date: Thu, 23 Mar 2023 19:45:27 +0100 Subject: [PATCH 07/20] Refactor the handling of height differences --- zebra-chain/src/block.rs | 2 +- zebra-chain/src/block/height.rs | 122 +++++++++--------- zebra-chain/src/chain_tip.rs | 4 +- zebra-chain/src/chain_tip/mock.rs | 8 +- .../network_chain_tip_height_estimator.rs | 21 +-- zebra-chain/src/chain_tip/tests/prop.rs | 2 +- zebra-consensus/src/block/check.rs | 2 +- .../src/block/subsidy/funding_streams.rs | 2 +- zebra-consensus/src/block/subsidy/general.rs | 76 +++++------ zebra-consensus/src/block/tests.rs | 2 +- zebra-consensus/src/parameters/subsidy.rs | 18 ++- .../get_block_template_rpcs/constants.rs | 2 +- .../tests/snapshot/get_block_template_rpcs.rs | 4 +- zebra-rpc/src/methods/tests/vectors.rs | 10 +- zebra-state/src/service/check/utxo.rs | 4 +- zebrad/src/components/sync/progress.rs | 6 +- 16 files changed, 144 insertions(+), 141 deletions(-) diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index 6e3ef56c451..7d51547ae92 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 6866072d3ca..5e491263363 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,31 +64,41 @@ 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. +pub type HeightDiff = i32; - 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 HeightDiff { +// type Error = TryFromIntError; - if height <= Height::MAX && height >= Height::MIN { - Some(height) - } else { - None - } - } -} +// fn try_from(height: Height) -> Result { +// HeightDiff::try_from(height.0) +// } +// } impl Sub for Height { - type Output = Option; + type Output = Option; fn sub(self, rhs: Height) -> Self::Output { - // Perform the subtraction. - let result = self.0.checked_sub(rhs.0)?; - let height = Height(result); + // We convert the heights from [`u32`] to [`i32`] because `u32::checked_sub` returns + // [`None`] for negative results. We must check the conversion since it's possible to + // construct heights outside the valid range for [`i32`]. + let lhs = i32::try_from(self.0).ok()?; + let rhs = i32::try_from(rhs.0).ok()?; + lhs.checked_sub(rhs) + } +} + +impl Sub for Height { + type Output = Option; + + fn sub(self, rhs: HeightDiff) -> Option { + // We need to convert the height to [`i32`] so we can subtract negative [`HeightDiff`]s. We + // must check the conversion since it's possible to construct heights outside the valid + // range for [`i32`]. + let lhs = i32::try_from(self.0).ok()?; + let res = lhs.checked_sub(rhs)?; + let res = u32::try_from(res).ok()?; + let height = Height(res); // Check the bounds. if Height::MIN <= height && height <= Height::MAX { @@ -99,35 +109,23 @@ impl Sub for Height { } } -// We don't implement Add or Sub, because they cause type inference issues for integer constants. - -impl Add for Height { +impl Add for Height { type Output = Option; - 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. + fn add(self, rhs: HeightDiff) -> Option { + // We need to convert the height to [`i32`] so we can subtract negative [`HeightDiff`]s. We + // must check the conversion since it's possible to construct heights 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, - } - } -} + let res = lhs.checked_add(rhs)?; + let res = u32::try_from(res).ok()?; + let height = Height(res); -impl Sub 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, + // Check the bounds. + if Height::MIN <= height && height <= Height::MAX { + Some(height) + } else { + None } } } @@ -137,22 +135,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); @@ -192,12 +190,12 @@ fn operator_tests() { assert_eq!(None, Height(i32::MAX as u32) - -1); assert_eq!(None, Height(u32::MAX) - -1); - assert_eq!(1, (Height(2) - Height(1)).unwrap().0); - assert_eq!(0, (Height(1) - Height(1)).unwrap().0); - assert_eq!(None, Height(0) - Height(1)); - assert_eq!(None, Height(2) - Height(7)); - assert_eq!(Height::MAX, (Height::MAX - Height(0)).unwrap()); - assert_eq!(1, (Height::MAX - Height(Height::MAX_AS_U32 - 1)).unwrap().0); - assert_eq!(None, Height(Height::MAX_AS_U32 - 1) - Height::MAX); - assert_eq!(None, Height(0) - Height::MAX); + assert_eq!(1, (Height(2) - Height(1)).unwrap()); + assert_eq!(0, (Height(1) - Height(1)).unwrap()); + assert_eq!(Some(-1), Height(0) - Height(1)); + assert_eq!(Some(-5), Height(2) - Height(7)); + assert_eq!(Height::MAX, (Height::MAX - 0).unwrap()); + assert_eq!(1, (Height::MAX - Height(Height::MAX_AS_U32 - 1)).unwrap()); + assert_eq!(Some(-1), Height(Height::MAX_AS_U32 - 1) - Height::MAX); + assert_eq!(Some(-(Height::MAX_AS_U32 as i32)), Height(0) - Height::MAX); } diff --git a/zebra-chain/src/chain_tip.rs b/zebra-chain/src/chain_tip.rs index f10b1ddc1cf..67dbef96e33 100644 --- a/zebra-chain/src/chain_tip.rs +++ b/zebra-chain/src/chain_tip.rs @@ -104,12 +104,12 @@ pub trait ChainTip { /// 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 [`None`] if the current local time is behind the chain tip block's + /// This estimate may be negative if the current local time is behind the chain tip block's /// timestamp. fn estimate_distance_to_network_chain_tip( &self, network: Network, - ) -> Option<(block::Height, block::Height)> { + ) -> Option<(block::HeightDiff, block::Height)> { let (current_height, current_block_time) = self.best_tip_height_and_block_time()?; let estimator = diff --git a/zebra-chain/src/chain_tip/mock.rs b/zebra-chain/src/chain_tip/mock.rs index 2d318130791..980af365673 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<(block::Height, block::Height)> { + ) -> Option<(block::HeightDiff, block::Height)> { self.estimated_distance_to_network_chain_tip .borrow() .and_then(|estimated_distance| { @@ -181,7 +181,7 @@ 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>, + distance: impl Into>, ) { self.estimated_distance_to_network_chain_tip .send(distance.into()) 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 247842bf246..7e0ea6ef8e0 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 @@ -87,16 +87,17 @@ 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 = max_height - self.current_height; - - if let Some(remaining_blocks) = remaining_blocks { - let remaining_blocks = i64::from(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; - } + (max_height - self.current_height).and_then(|remaining_blocks| { + remaining_blocks.is_positive().then(|| { + let remaining_blocks = i64::from(remaining_blocks); + 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; + }) + }); } /// Calculate an estimate for the chain height using the `current_target_spacing`. diff --git a/zebra-chain/src/chain_tip/tests/prop.rs b/zebra-chain/src/chain_tip/tests/prop.rs index 660e399bbe5..8e8ee7ae32f 100644 --- a/zebra-chain/src/chain_tip/tests/prop.rs +++ b/zebra-chain/src/chain_tip/tests/prop.rs @@ -72,7 +72,7 @@ fn estimate_time_difference( ) -> Duration { let spacing_seconds = active_network_upgrade.target_spacing().num_seconds(); - let height_difference = (end_height - start_height).map_or(0, |height| height.0.into()); + let height_difference = (end_height - start_height).map_or(0, |height| height.into()); Duration::seconds(height_difference * spacing_seconds) } diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs index ddd3dbefa63..15f8e883bcd 100644 --- a/zebra-consensus/src/block/check.rs +++ b/zebra-consensus/src/block/check.rs @@ -154,7 +154,7 @@ pub fn subsidy_is_valid(block: &Block, network: Network) -> Result<(), BlockErro .activation_height(network) .expect("Canopy activation height is known"); - if height < SLOW_START_INTERVAL { + if (height - SLOW_START_INTERVAL).is_none() { unreachable!( "unsupported block height: callers should handle blocks below {:?}", SLOW_START_INTERVAL diff --git a/zebra-consensus/src/block/subsidy/funding_streams.rs b/zebra-consensus/src/block/subsidy/funding_streams.rs index d661223c515..4e56ab1940a 100644 --- a/zebra-consensus/src/block/subsidy/funding_streams.rs +++ b/zebra-consensus/src/block/subsidy/funding_streams.rs @@ -74,7 +74,7 @@ fn funding_stream_address_period(height: Height, network: Network) -> u32 { // - In Rust, "integer division rounds towards zero": // https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators // 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)) + (height.0 + (POST_BLOSSOM_HALVING_INTERVAL as u32) - (height_for_first_halving(network).0)) / (FUNDING_STREAM_ADDRESS_CHANGE_INTERVAL.0) } diff --git a/zebra-consensus/src/block/subsidy/general.rs b/zebra-consensus/src/block/subsidy/general.rs index 14587e39314..3648f4ba384 100644 --- a/zebra-consensus/src/block/subsidy/general.rs +++ b/zebra-consensus/src/block/subsidy/general.rs @@ -25,36 +25,36 @@ pub fn halving_divisor(height: Height, network: Network) -> Option { .activation_height(network) .expect("blossom activation height should be available"); - let slow_start_shift_diff = height - SLOW_START_SHIFT; - let slow_start_shift_diff = slow_start_shift_diff.unwrap_or_else(|| { + let Some(height_after_slow_start_shift) = height - SLOW_START_SHIFT else { unreachable!( "unsupported block height: checkpoints should handle blocks below {:?}", SLOW_START_SHIFT ) - }); - - if let Some(post_blossom_height) = height - blossom_height { - // The Blossom height is way above `SLOW_START_SHIFT`. - let pre_blossom_height = (blossom_height - SLOW_START_SHIFT) - .expect("Blossom height must be above SlowStartShift.") - .0; - let scaled_pre_blossom_height = pre_blossom_height * BLOSSOM_POW_TARGET_SPACING_RATIO; - - let halving_shift = - (scaled_pre_blossom_height + post_blossom_height.0) / POST_BLOSSOM_HALVING_INTERVAL.0; - - // Some far-future shifts can be more than 63 bits - 1u64.checked_shl(halving_shift) - } else { - let pre_blossom_height = slow_start_shift_diff.0; - let halving_shift = pre_blossom_height / PRE_BLOSSOM_HALVING_INTERVAL.0; - - let halving_div = 1u64 - .checked_shl(halving_shift) - .expect("pre-blossom heights produce small shifts"); + }; - Some(halving_div) - } + let post_blossom_height_diff = (height - blossom_height)?; + + post_blossom_height_diff + .is_negative() + .then(|| { + let pre_blossom_height = height_after_slow_start_shift.0; + let halving_shift = pre_blossom_height / (PRE_BLOSSOM_HALVING_INTERVAL as u32); + + 1u64.checked_shl(halving_shift) + .expect("pre-blossom heights produce small shifts") + }) + .or_else(|| { + let pre_blossom_height = (blossom_height - SLOW_START_SHIFT) + .expect("Blossom height must be above SlowStartShift.") + .0; + let scaled_pre_blossom_height = pre_blossom_height * BLOSSOM_POW_TARGET_SPACING_RATIO; + + let halving_shift = (scaled_pre_blossom_height + (post_blossom_height_diff as u32)) + / (POST_BLOSSOM_HALVING_INTERVAL as u32); + + // Some far-future shifts can be more than 63 bits + 1u64.checked_shl(halving_shift) + }) } /// `BlockSubsidy(height)` as described in [protocol specification §7.8][7.8] @@ -73,7 +73,7 @@ pub fn block_subsidy(height: Height, network: Network) -> Result Result<(), Report> { }; for (&height, block) in block_iter { - if Height(height) > SLOW_START_SHIFT { + if (Height(height) - SLOW_START_SHIFT).is_some() { let block = Block::zcash_deserialize(&block[..]).expect("block should deserialize"); // fake the miner fee to a big amount diff --git a/zebra-consensus/src/parameters/subsidy.rs b/zebra-consensus/src/parameters/subsidy.rs index a6b0d3808c6..b62b6a64ce6 100644 --- a/zebra-consensus/src/parameters/subsidy.rs +++ b/zebra-consensus/src/parameters/subsidy.rs @@ -4,19 +4,23 @@ 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] /// /// [slow-mining]: https://z.cash/support/faq/#what-is-slow-start-mining -pub const SLOW_START_INTERVAL: Height = Height(20_000); +pub const SLOW_START_INTERVAL: HeightDiff = 20_000; /// `SlowStartShift()` as described in [protocol specification §7.8][7.8] /// /// [7.8]: https://zips.z.cash/protocol/protocol.pdf#subsidies /// /// This calculation is exact, because `SLOW_START_INTERVAL` is divisible by 2. -pub const SLOW_START_SHIFT: Height = Height(SLOW_START_INTERVAL.0 / 2); +pub const SLOW_START_SHIFT: HeightDiff = SLOW_START_INTERVAL / 2; /// The largest block subsidy, used before the first halving. /// @@ -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 i32); /// 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] @@ -134,7 +138,7 @@ lazy_static! { /// /// [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); + Height(POST_BLOSSOM_HALVING_INTERVAL as u32 / 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 ef2f2232f89..8b6b9174a58 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs @@ -43,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: block::Height = block::Height(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/tests/snapshot/get_block_template_rpcs.rs b/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs index 995f906b9d2..48fcfd697e3 100644 --- a/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs @@ -12,7 +12,7 @@ use insta::Settings; use tower::{buffer::Buffer, Service}; use zebra_chain::{ - block::{self, Hash}, + block::Hash, chain_sync_status::MockSyncStatus, chain_tip::mock::MockChainTip, parameters::{Network, NetworkUpgrade}, @@ -123,7 +123,7 @@ pub async fn test_responses( let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new(); mock_chain_tip_sender.send_best_tip_height(fake_tip_height); mock_chain_tip_sender.send_best_tip_hash(fake_tip_hash); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(block::Height(0))); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); let mock_address_book = MockAddressBookPeers::new(vec![MetaAddr::new_initial_peer(SocketAddr::new( diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 7ed95661d4a..50641ddc5ea 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -1148,7 +1148,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new(); mock_chain_tip_sender.send_best_tip_height(fake_tip_height); mock_chain_tip_sender.send_best_tip_hash(fake_tip_hash); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(0))); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); // Init RPC let get_block_template_rpc = GetBlockTemplateRpcImpl::new( @@ -1251,7 +1251,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { mempool.expect_no_requests().await; - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(200))); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(200)); let get_block_template_sync_error = get_block_template_rpc .get_block_template(None) .await @@ -1264,7 +1264,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { mock_sync_status.set_is_close_to_tip(false); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(0))); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); let get_block_template_sync_error = get_block_template_rpc .get_block_template(None) .await @@ -1275,7 +1275,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) { ErrorCode::ServerError(-10) ); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(200))); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(200)); let get_block_template_sync_error = get_block_template_rpc .get_block_template(None) .await @@ -1543,7 +1543,7 @@ async fn rpc_getdifficulty() { let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new(); mock_chain_tip_sender.send_best_tip_height(fake_tip_height); mock_chain_tip_sender.send_best_tip_hash(fake_tip_hash); - mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(Height(0))); + mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0)); // Init RPC let get_block_template_rpc = GetBlockTemplateRpcImpl::new( diff --git a/zebra-state/src/service/check/utxo.rs b/zebra-state/src/service/check/utxo.rs index 24242c64256..d29e5bca078 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/sync/progress.rs b/zebrad/src/components/sync/progress.rs index c52f01d0367..4efbaf165fa 100644 --- a/zebrad/src/components/sync/progress.rs +++ b/zebrad/src/components/sync/progress.rs @@ -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: u32 = 1; +const MAX_CLOSE_TO_TIP_BLOCKS: i32 = 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: u32 = 60; +const MIN_SYNC_WARNING_BLOCKS: i32 = 60; /// The number of fractional digits in sync percentages. const SYNC_PERCENT_FRAC_DIGITS: usize = 3; @@ -129,7 +129,7 @@ pub async fn show_block_chain_progress( ); let remaining_sync_blocks = - (estimated_height - current_height).map_or(0, |height| height.0); + (estimated_height - current_height).map_or(0, |height| height); // Work out how long it has been since the state height has increased. // From 665ecad41922c2bd60b751c7919d915ddc4c7d99 Mon Sep 17 00:00:00 2001 From: Marek Date: Thu, 23 Mar 2023 19:53:10 +0100 Subject: [PATCH 08/20] Remove a redundant comment --- zebra-chain/src/block/height.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index 5e491263363..09ed72be9e3 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -67,14 +67,6 @@ impl Height { /// A difference between two [`Height`]s, possibly negative. pub type HeightDiff = i32; -// impl TryFrom for HeightDiff { -// type Error = TryFromIntError; - -// fn try_from(height: Height) -> Result { -// HeightDiff::try_from(height.0) -// } -// } - impl Sub for Height { type Output = Option; From 028bd509b1f086c456761d91593a968cdfabafec Mon Sep 17 00:00:00 2001 From: Marek Date: Thu, 23 Mar 2023 20:40:38 +0100 Subject: [PATCH 09/20] Update zebrad/src/components/sync/progress.rs Co-authored-by: Arya --- zebrad/src/components/sync/progress.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebrad/src/components/sync/progress.rs b/zebrad/src/components/sync/progress.rs index 4efbaf165fa..ba0dc062c22 100644 --- a/zebrad/src/components/sync/progress.rs +++ b/zebrad/src/components/sync/progress.rs @@ -129,7 +129,7 @@ pub async fn show_block_chain_progress( ); let remaining_sync_blocks = - (estimated_height - current_height).map_or(0, |height| height); + (estimated_height - current_height).unwrap_or(0); // Work out how long it has been since the state height has increased. // From 3f4b473d00ab85daf3f12dbac2690db8b81d8bc5 Mon Sep 17 00:00:00 2001 From: Marek Date: Thu, 23 Mar 2023 23:52:22 +0100 Subject: [PATCH 10/20] Update progress.rs --- zebrad/src/components/sync/progress.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zebrad/src/components/sync/progress.rs b/zebrad/src/components/sync/progress.rs index ba0dc062c22..8befd233484 100644 --- a/zebrad/src/components/sync/progress.rs +++ b/zebrad/src/components/sync/progress.rs @@ -128,8 +128,7 @@ pub async fn show_block_chain_progress( frac = SYNC_PERCENT_FRAC_DIGITS, ); - let remaining_sync_blocks = - (estimated_height - current_height).unwrap_or(0); + let remaining_sync_blocks = (estimated_height - current_height).unwrap_or(0); // Work out how long it has been since the state height has increased. // From 53115ab5a38af817a60a07ec9639c4f0c7dca196 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 13:11:33 +1000 Subject: [PATCH 11/20] impl TryFrom for Height --- zebra-chain/src/block/height.rs | 39 +++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index 09ed72be9e3..c90d127a86a 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -67,9 +67,26 @@ impl Height { /// A difference between two [`Height`]s, possibly negative. pub type HeightDiff = i32; +impl TryFrom for Height { + type Error = &'static str; + + /// 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 { + Err("heights must be less than or equal to Height::MAX") + } + } +} + impl Sub for Height { type Output = Option; + /// Subtract two heights, returning `None` if: + /// - either height is outside the valid `i32` range ([`Height::MAX`] is currently [`i32::MAX`]) + /// - the result is outside the valid `i32` range fn sub(self, rhs: Height) -> Self::Output { // We convert the heights from [`u32`] to [`i32`] because `u32::checked_sub` returns // [`None`] for negative results. We must check the conversion since it's possible to @@ -83,6 +100,10 @@ impl Sub for Height { impl Sub for Height { type Output = Option; + /// Subtract a height difference from a height, returning `None` if: + /// - the height is outside the valid `i32` range ([`Height::MAX`] is currently [`i32::MAX`]) + /// - 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 [`i32`] so we can subtract negative [`HeightDiff`]s. We // must check the conversion since it's possible to construct heights outside the valid @@ -90,20 +111,19 @@ impl Sub for Height { let lhs = i32::try_from(self.0).ok()?; let res = lhs.checked_sub(rhs)?; let res = u32::try_from(res).ok()?; - let height = Height(res); // Check the bounds. - if Height::MIN <= height && height <= Height::MAX { - Some(height) - } else { - None - } + Height::try_from(res).ok() } } impl Add for Height { type Output = Option; + /// Add a height difference to a height, returning `None` if: + /// - the height is outside the valid `i32` range ([`Height::MAX`] is currently [`i32::MAX`]) + /// - 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 [`i32`] so we can subtract negative [`HeightDiff`]s. We // must check the conversion since it's possible to construct heights outside the valid @@ -111,14 +131,9 @@ impl Add for Height { let lhs = i32::try_from(self.0).ok()?; let res = lhs.checked_add(rhs)?; let res = u32::try_from(res).ok()?; - let height = Height(res); // Check the bounds. - if Height::MIN <= height && height <= Height::MAX { - Some(height) - } else { - None - } + Height::try_from(res).ok() } } From 06d79145070cb642df5f7101318f3822544f9894 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 13:14:23 +1000 Subject: [PATCH 12/20] Make some test assertions clearer --- zebra-chain/src/block/height.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index c90d127a86a..3ac818055f1 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -197,12 +197,12 @@ fn operator_tests() { assert_eq!(None, Height(i32::MAX as u32) - -1); assert_eq!(None, Height(u32::MAX) - -1); - assert_eq!(1, (Height(2) - Height(1)).unwrap()); - assert_eq!(0, (Height(1) - Height(1)).unwrap()); + assert_eq!(Some(1), (Height(2) - Height(1))); + assert_eq!(Some(0), (Height(1) - Height(1))); assert_eq!(Some(-1), Height(0) - Height(1)); assert_eq!(Some(-5), Height(2) - Height(7)); - assert_eq!(Height::MAX, (Height::MAX - 0).unwrap()); - assert_eq!(1, (Height::MAX - Height(Height::MAX_AS_U32 - 1)).unwrap()); + assert_eq!(Some(Height::MAX), (Height::MAX - 0)); + assert_eq!(Some(1), (Height::MAX - Height(Height::MAX_AS_U32 - 1))); assert_eq!(Some(-1), Height(Height::MAX_AS_U32 - 1) - Height::MAX); assert_eq!(Some(-(Height::MAX_AS_U32 as i32)), Height(0) - Height::MAX); } From 976316ae903124e512a9dd357eaf6edd2b64caac Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 13:16:20 +1000 Subject: [PATCH 13/20] Refactor estimate_up_to() --- .../network_chain_tip_height_estimator.rs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) 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 7e0ea6ef8e0..7c4bae6b5fa 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 @@ -87,17 +87,15 @@ 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) { - (max_height - self.current_height).and_then(|remaining_blocks| { - remaining_blocks.is_positive().then(|| { - let remaining_blocks = i64::from(remaining_blocks); - 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; - }) - }); + if let Some(remaining_blocks) = + (max_height - self.current_height).filter(|r| r.is_positive()) + { + let remaining_blocks = i64::from(remaining_blocks); + 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; + } } /// Calculate an estimate for the chain height using the `current_target_spacing`. From f834ae1c6c760458f6822c60c6cb65aefcc81691 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 13:17:52 +1000 Subject: [PATCH 14/20] Restore a comment that was accidentally removed --- zebra-chain/src/block/height.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index 3ac818055f1..4af86ff9c38 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -81,6 +81,8 @@ impl TryFrom for Height { } } +// We don't implement Add or Sub, because they cause type inference issues for integer constants. + impl Sub for Height { type Output = Option; From f1d0921956c82ccf1cf93db37033e4f339fde579 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 13:20:42 +1000 Subject: [PATCH 15/20] Document when estimate_distance_to_network_chain_tip() returns None --- zebra-chain/src/chain_tip.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zebra-chain/src/chain_tip.rs b/zebra-chain/src/chain_tip.rs index 67dbef96e33..dfc6f7e9776 100644 --- a/zebra-chain/src/chain_tip.rs +++ b/zebra-chain/src/chain_tip.rs @@ -106,6 +106,10 @@ pub trait ChainTip { /// /// 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, or + /// - the distance to the tip is outside the valid `i32` range (currently unreachable). fn estimate_distance_to_network_chain_tip( &self, network: Network, From a765be43bfe129d614ae75d2c77ad8239af90368 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 15:18:58 +1000 Subject: [PATCH 16/20] Change HeightDiff to i64 and make Height.sub(Height) return HeightDiff (no Option) --- zebra-chain/src/block/height.rs | 71 +++++++++----------- zebra-chain/src/parameters/network.rs | 7 +- zebra-consensus/src/checkpoint/list/tests.rs | 11 +-- zebra-consensus/src/parameters/subsidy.rs | 2 +- zebra-state/src/constants.rs | 2 + zebra-state/src/service.rs | 5 +- zebrad/src/components/inbound/downloads.rs | 9 ++- zebrad/src/components/sync.rs | 4 +- zebrad/src/components/sync/downloads.rs | 8 +-- zebrad/src/components/sync/progress.rs | 12 ++-- 10 files changed, 67 insertions(+), 64 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index 4af86ff9c38..c08234ddf1a 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -65,7 +65,10 @@ impl Height { } /// A difference between two [`Height`]s, possibly negative. -pub type HeightDiff = i32; +/// +/// 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; impl TryFrom for Height { type Error = &'static str; @@ -84,37 +87,31 @@ impl TryFrom for Height { // We don't implement Add or Sub, because they cause type inference issues for integer constants. impl Sub for Height { - type Output = Option; + type Output = HeightDiff; - /// Subtract two heights, returning `None` if: - /// - either height is outside the valid `i32` range ([`Height::MAX`] is currently [`i32::MAX`]) - /// - the result is outside the valid `i32` range + /// 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 { - // We convert the heights from [`u32`] to [`i32`] because `u32::checked_sub` returns - // [`None`] for negative results. We must check the conversion since it's possible to - // construct heights outside the valid range for [`i32`]. - let lhs = i32::try_from(self.0).ok()?; - let rhs = i32::try_from(rhs.0).ok()?; - lhs.checked_sub(rhs) + // 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 { type Output = Option; - /// Subtract a height difference from a height, returning `None` if: - /// - the height is outside the valid `i32` range ([`Height::MAX`] is currently [`i32::MAX`]) - /// - the resulting height is outside the valid `Height` range, - /// this also checks the result is non-negative + /// 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 [`i32`] so we can subtract negative [`HeightDiff`]s. We - // must check the conversion since it's possible to construct heights outside the valid - // range for [`i32`]. - let lhs = i32::try_from(self.0).ok()?; - let res = lhs.checked_sub(rhs)?; - let res = u32::try_from(res).ok()?; + // 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() } } @@ -122,19 +119,15 @@ impl Sub for Height { impl Add for Height { type Output = Option; - /// Add a height difference to a height, returning `None` if: - /// - the height is outside the valid `i32` range ([`Height::MAX`] is currently [`i32::MAX`]) - /// - the resulting height is outside the valid `Height` range, - /// this also checks the result is non-negative + /// 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 [`i32`] so we can subtract negative [`HeightDiff`]s. We - // must check the conversion since it's possible to construct heights outside the valid - // range for [`i32`]. - let lhs = i32::try_from(self.0).ok()?; - let res = lhs.checked_add(rhs)?; - let res = u32::try_from(res).ok()?; + // 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() } } @@ -199,12 +192,12 @@ fn operator_tests() { assert_eq!(None, Height(i32::MAX as u32) - -1); assert_eq!(None, Height(u32::MAX) - -1); - assert_eq!(Some(1), (Height(2) - Height(1))); - assert_eq!(Some(0), (Height(1) - Height(1))); - assert_eq!(Some(-1), Height(0) - Height(1)); - assert_eq!(Some(-5), Height(2) - Height(7)); - assert_eq!(Some(Height::MAX), (Height::MAX - 0)); - assert_eq!(Some(1), (Height::MAX - Height(Height::MAX_AS_U32 - 1))); - assert_eq!(Some(-1), Height(Height::MAX_AS_U32 - 1) - Height::MAX); - assert_eq!(Some(-(Height::MAX_AS_U32 as i32)), Height(0) - Height::MAX); + 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.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 HeightDiff), Height(0) - Height::MAX); } diff --git a/zebra-chain/src/parameters/network.rs b/zebra-chain/src/parameters/network.rs index d23abe94417..6c6319ce765 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/checkpoint/list/tests.rs b/zebra-consensus/src/checkpoint/list/tests.rs index 29bc16928d1..60b4ec9ef99 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 b62b6a64ce6..cca22955339 100644 --- a/zebra-consensus/src/parameters/subsidy.rs +++ b/zebra-consensus/src/parameters/subsidy.rs @@ -41,7 +41,7 @@ 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: HeightDiff = - PRE_BLOSSOM_HALVING_INTERVAL * (BLOSSOM_POW_TARGET_SPACING_RATIO as i32); + 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] diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index b91fe29d755..166d93dd6a3 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 0fe50c8380c..ea4a63b6aa5 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/zebrad/src/components/inbound/downloads.rs b/zebrad/src/components/inbound/downloads.rs index a8315b75812..aa8b2cf6c25 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 e74be242077..c0cc3889e56 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 4b6b5f00256..fde200f7fc8 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 8befd233484..840436c1c1b 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, From 4ad832e2e49426fb1fcbfbce0292b214f1dabc2b Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 15:26:23 +1000 Subject: [PATCH 17/20] Update chain tip estimates for HeightDiff i64 --- zebra-chain/src/chain_tip.rs | 6 ++-- .../network_chain_tip_height_estimator.rs | 33 ++++++++++--------- zebra-chain/src/chain_tip/tests/prop.rs | 9 +++-- zebrad/src/components/sync/progress.rs | 5 ++- 4 files changed, 30 insertions(+), 23 deletions(-) diff --git a/zebra-chain/src/chain_tip.rs b/zebra-chain/src/chain_tip.rs index dfc6f7e9776..e2d55a7dab6 100644 --- a/zebra-chain/src/chain_tip.rs +++ b/zebra-chain/src/chain_tip.rs @@ -107,9 +107,7 @@ pub trait ChainTip { /// 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, or - /// - the distance to the tip is outside the valid `i32` range (currently unreachable). + /// Returns `None` if the state is empty. fn estimate_distance_to_network_chain_tip( &self, network: Network, @@ -121,7 +119,7 @@ pub trait ChainTip { let distance_to_tip = estimator.estimate_height_at(Utc::now()) - current_height; - distance_to_tip.map(|distance_to_tip| (distance_to_tip, current_height)) + Some((distance_to_tip, current_height)) } } 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 7c4bae6b5fa..1c5e1cf08c5 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,10 +87,9 @@ 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) { - if let Some(remaining_blocks) = - (max_height - self.current_height).filter(|r| r.is_positive()) - { - let remaining_blocks = i64::from(remaining_blocks); + 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; @@ -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()); + + let current_height_as_diff = HeightDiff::from(self.current_height.0); - if -(block_difference as i64) > self.current_height.0 as i64 { + 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 8e8ee7ae32f..1cbc7a5375d 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 = (end_height - start_height).map_or(0, |height| height.into()); - - 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/zebrad/src/components/sync/progress.rs b/zebrad/src/components/sync/progress.rs index 840436c1c1b..113bb36d546 100644 --- a/zebrad/src/components/sync/progress.rs +++ b/zebrad/src/components/sync/progress.rs @@ -128,7 +128,10 @@ pub async fn show_block_chain_progress( frac = SYNC_PERCENT_FRAC_DIGITS, ); - let remaining_sync_blocks = (estimated_height - current_height).unwrap_or(0); + 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. // From 155ce35d1d806e62c793efb9e06140ed869d4c71 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 15:27:03 +1000 Subject: [PATCH 18/20] Update subsidy for HeightDiff i64 --- zebra-consensus/src/block/check.rs | 2 +- .../src/block/subsidy/funding_streams.rs | 10 +- zebra-consensus/src/block/subsidy/general.rs | 126 +++++++++++++----- zebra-consensus/src/block/tests.rs | 2 +- zebra-consensus/src/parameters/subsidy.rs | 7 +- 5 files changed, 107 insertions(+), 40 deletions(-) diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs index 15f8e883bcd..ddd3dbefa63 100644 --- a/zebra-consensus/src/block/check.rs +++ b/zebra-consensus/src/block/check.rs @@ -154,7 +154,7 @@ pub fn subsidy_is_valid(block: &Block, network: Network) -> Result<(), BlockErro .activation_height(network) .expect("Canopy activation height is known"); - if (height - SLOW_START_INTERVAL).is_none() { + if height < SLOW_START_INTERVAL { unreachable!( "unsupported block height: callers should handle blocks below {:?}", SLOW_START_INTERVAL diff --git a/zebra-consensus/src/block/subsidy/funding_streams.rs b/zebra-consensus/src/block/subsidy/funding_streams.rs index 4e56ab1940a..4d1ab9a6cc4 100644 --- a/zebra-consensus/src/block/subsidy/funding_streams.rs +++ b/zebra-consensus/src/block/subsidy/funding_streams.rs @@ -74,8 +74,14 @@ fn funding_stream_address_period(height: Height, network: Network) -> u32 { // - In Rust, "integer division rounds towards zero": // https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators // This is the same as `floor()`, because these numbers are all positive. - (height.0 + (POST_BLOSSOM_HALVING_INTERVAL as u32) - (height_for_first_halving(network).0)) - / (FUNDING_STREAM_ADDRESS_CHANGE_INTERVAL.0) + let height_above_first_halving = height - height_for_first_halving(network); + + let address_period = (height_above_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 3648f4ba384..b3cb0231734 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, }; @@ -25,36 +25,40 @@ pub fn halving_divisor(height: Height, network: Network) -> Option { .activation_height(network) .expect("blossom activation height should be available"); - let Some(height_after_slow_start_shift) = height - SLOW_START_SHIFT else { + 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 = height - SLOW_START_SHIFT; + let halving_shift = pre_blossom_height / PRE_BLOSSOM_HALVING_INTERVAL; + + let halving_div = 1u64 + .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 = 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 = height - blossom_height; - let post_blossom_height_diff = (height - blossom_height)?; - - post_blossom_height_diff - .is_negative() - .then(|| { - let pre_blossom_height = height_after_slow_start_shift.0; - let halving_shift = pre_blossom_height / (PRE_BLOSSOM_HALVING_INTERVAL as u32); - - 1u64.checked_shl(halving_shift) - .expect("pre-blossom heights produce small shifts") - }) - .or_else(|| { - let pre_blossom_height = (blossom_height - SLOW_START_SHIFT) - .expect("Blossom height must be above SlowStartShift.") - .0; - let scaled_pre_blossom_height = pre_blossom_height * BLOSSOM_POW_TARGET_SPACING_RATIO; - - let halving_shift = (scaled_pre_blossom_height + (post_blossom_height_diff as u32)) - / (POST_BLOSSOM_HALVING_INTERVAL as u32); - - // Some far-future shifts can be more than 63 bits - 1u64.checked_shl(halving_shift) - }) + let halving_shift = + (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 + .try_into() + .expect("already checked for negatives"), + ) + } } /// `BlockSubsidy(height)` as described in [protocol specification §7.8][7.8] @@ -73,10 +77,9 @@ 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() @@ -262,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) @@ -321,7 +332,31 @@ mod test { ) ); - // The largest possible divisor + assert_eq!( + Amount::try_from(0), + block_subsidy( + (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( @@ -330,6 +365,33 @@ mod test { ) ); + // 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/block/tests.rs b/zebra-consensus/src/block/tests.rs index cd73f944d5b..bad6ab40630 100644 --- a/zebra-consensus/src/block/tests.rs +++ b/zebra-consensus/src/block/tests.rs @@ -485,7 +485,7 @@ fn miner_fees_validation_for_network(network: Network) -> Result<(), Report> { }; for (&height, block) in block_iter { - if (Height(height) - SLOW_START_SHIFT).is_some() { + if Height(height) > SLOW_START_SHIFT { let block = Block::zcash_deserialize(&block[..]).expect("block should deserialize"); // fake the miner fee to a big amount diff --git a/zebra-consensus/src/parameters/subsidy.rs b/zebra-consensus/src/parameters/subsidy.rs index cca22955339..ba8f910d8e3 100644 --- a/zebra-consensus/src/parameters/subsidy.rs +++ b/zebra-consensus/src/parameters/subsidy.rs @@ -13,14 +13,14 @@ use zebra_chain::{ /// An initial period from Genesis to this Height where the block subsidy is gradually incremented. [What is slow-start mining][slow-mining] /// /// [slow-mining]: https://z.cash/support/faq/#what-is-slow-start-mining -pub const SLOW_START_INTERVAL: HeightDiff = 20_000; +pub const SLOW_START_INTERVAL: Height = Height(20_000); /// `SlowStartShift()` as described in [protocol specification §7.8][7.8] /// /// [7.8]: https://zips.z.cash/protocol/protocol.pdf#subsidies /// /// This calculation is exact, because `SLOW_START_INTERVAL` is divisible by 2. -pub const SLOW_START_SHIFT: HeightDiff = SLOW_START_INTERVAL / 2; +pub const SLOW_START_SHIFT: Height = Height(SLOW_START_INTERVAL.0 / 2); /// The largest block subsidy, used before the first halving. /// @@ -137,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 as u32 / 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)` From 7ac8b89d139b638801a71ea76f5de553e344b45d Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 15:32:59 +1000 Subject: [PATCH 19/20] Fix some height calculation test edge cases --- zebra-chain/src/block/height.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index c08234ddf1a..68c501ddc56 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -163,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); @@ -183,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 From 3238412dca26955d25ef4c566660d5941f82e010 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 28 Mar 2023 15:53:22 +1000 Subject: [PATCH 20/20] Fix the funding stream interval calculation --- .../src/block/subsidy/funding_streams.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/zebra-consensus/src/block/subsidy/funding_streams.rs b/zebra-consensus/src/block/subsidy/funding_streams.rs index 4d1ab9a6cc4..977b14a56d8 100644 --- a/zebra-consensus/src/block/subsidy/funding_streams.rs +++ b/zebra-consensus/src/block/subsidy/funding_streams.rs @@ -69,14 +69,18 @@ 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. - let height_above_first_halving = height - height_for_first_halving(network); - let address_period = (height_above_first_halving - POST_BLOSSOM_HALVING_INTERVAL) + 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