change(chain): Refactor the handling of height differences

zebra-chain/src/block/height.rs

@@ -83,18 +83,19 @@ impl Add<Height> for Height {
 }
 
 impl Sub<Height> 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<Height>;
+
+    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
+        }
     }
 }
 
@@ -191,13 +192,12 @@ fn operator_tests() {
     assert_eq!(None, Height(i32::MAX as u32) - -1);
     assert_eq!(None, Height(u32::MAX) - -1);
 
-    // Sub<Height> 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);
 }

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))
     }
 }
 

zebra-chain/src/chain_tip/mock.rs

@@ -27,7 +27,7 @@ pub struct MockChainTipSender {
     best_tip_block_time: watch::Sender<Option<DateTime<Utc>>>,
 
     /// A sender that sets the `estimate_distance_to_network_chain_tip` of a [`MockChainTip`].
-    estimated_distance_to_network_chain_tip: watch::Sender<Option<i32>>,
+    estimated_distance_to_network_chain_tip: watch::Sender<Option<block::Height>>,
 }
 
 /// 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<Option<DateTime<Utc>>>,
 
     /// A mocked `estimate_distance_to_network_chain_tip` value set by the [`MockChainTipSender`].
-    estimated_distance_to_network_chain_tip: watch::Receiver<Option<i32>>,
+    estimated_distance_to_network_chain_tip: watch::Receiver<Option<block::Height>>,
 }
 
 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<Option<i32>>) {
+    pub fn send_estimated_distance_to_network_chain_tip(
+        &self,
+        distance: impl Into<Option<block::Height>>,
+    ) {
         self.estimated_distance_to_network_chain_tip
             .send(distance.into())
             .expect("attempt to send a best tip height to a dropped `MockChainTip`");

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);
 

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)
 }

zebra-consensus/src/block/subsidy/general.rs

@@ -25,37 +25,34 @@ pub fn halving_divisor(height: Height, network: Network) -> Option<u64> {
         .activation_height(network)
         .expect("blossom activation height should be available");
 
-    if height < SLOW_START_SHIFT {
+    let Some(slow_start_shift_diff) = height - SLOW_START_SHIFT 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)
     }
 }
 

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.
 /// >
 /// > <https://zips.z.cash/protocol/protocol.pdf#blockheader>
-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.
 ///

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."
             ),

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<State, ReadState>(
     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(

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(

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.
             //