Applies suggestions from code review

zebra-consensus/src/block/check.rs

@@ -247,6 +247,7 @@ pub fn miner_fees_are_valid(
     if if should_allow_unclaimed_subsidy {
         left > right
     } else {
+        // TODO: Document the need for claiming all subsidy once the ZIP for NU6 is out.
         left != right
     } {
         Err(SubsidyError::InvalidMinerFees)?;

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

@@ -32,10 +32,20 @@ pub fn funding_stream_values(
     let mut results = HashMap::new();
 
     if current_network_upgrade >= Canopy {
-        let range = FUNDING_STREAM_HEIGHT_RANGES.get(&network.kind()).unwrap();
+        let is_pre_nu6 = current_network_upgrade < Nu6;
+        let range = if is_pre_nu6 {
+            PRE_NU6_FUNDING_STREAM_HEIGHT_RANGES
+                .get(&network.kind())
+                .unwrap()
+        } else {
+            POST_NU6_FUNDING_STREAM_HEIGHT_RANGES
+                .get(&network.kind())
+                .unwrap()
+        };
+
         if range.contains(&height) {
             let block_subsidy = block_subsidy(height, network)?;
-            let funding_stream_numerators = if current_network_upgrade < Nu6 {
+            let funding_stream_numerators = if is_pre_nu6 {
                 PRE_NU6_FUNDING_STREAM_RECEIVER_NUMERATORS.iter()
             } else {
                 POST_NU6_FUNDING_STREAM_RECEIVER_NUMERATORS.iter()
@@ -87,17 +97,21 @@ fn funding_stream_address_period(height: Height, network: &Network) -> u32 {
 /// [7.10]: https://zips.z.cash/protocol/protocol.pdf#fundingstreams
 fn funding_stream_address_index(height: Height, network: &Network) -> usize {
     let num_addresses = network.num_funding_streams();
+    let is_pre_nu6 = NetworkUpgrade::current(network, height) < Nu6;
+    let range = if is_pre_nu6 {
+        PRE_NU6_FUNDING_STREAM_HEIGHT_RANGES
+            .get(&network.kind())
+            .unwrap()
+    } else {
+        POST_NU6_FUNDING_STREAM_HEIGHT_RANGES
+            .get(&network.kind())
+            .unwrap()
+    };
 
     let index = 1u32
         .checked_add(funding_stream_address_period(height, network))
         .expect("no overflow should happen in this sum")
-        .checked_sub(funding_stream_address_period(
-            FUNDING_STREAM_HEIGHT_RANGES
-                .get(&network.kind())
-                .unwrap()
-                .start,
-            network,
-        ))
+        .checked_sub(funding_stream_address_period(range.start, network))
         .expect("no overflow should happen in this sub") as usize;
 
     assert!(index > 0 && index <= num_addresses);

zebra-consensus/src/block/subsidy/funding_streams/tests.rs

@@ -43,16 +43,25 @@ fn test_funding_stream_values() -> Result<(), Report> {
         hash_map
     );
 
-    // funding stream period is ending
-    let range = FUNDING_STREAM_HEIGHT_RANGES.get(&network.kind()).unwrap();
-    let end = range.end;
-    let last = end - 1;
+    for range in [
+        PRE_NU6_FUNDING_STREAM_HEIGHT_RANGES
+            .get(&network.kind())
+            .unwrap(),
+        // TODO: Uncomment this once NU6 activation height is added on Mainnet
+        // POST_NU6_FUNDING_STREAM_HEIGHT_RANGES
+        //     .get(&network.kind())
+        //     .unwrap(),
+    ] {
+        // funding stream period is ending
+        let end = range.end;
+        let last = end - 1;
 
-    assert_eq!(
-        funding_stream_values(last.unwrap(), network).unwrap(),
-        hash_map
-    );
-    assert!(funding_stream_values(end, network)?.is_empty());
+        assert_eq!(
+            funding_stream_values(last.unwrap(), network).unwrap(),
+            hash_map
+        );
+        assert!(funding_stream_values(end, network)?.is_empty());
+    }
 
     Ok(())
 }

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

@@ -119,22 +119,20 @@ fn lockbox_input_value(network: &Network, height: Height) -> Amount<NonNegative>
         .get(&FundingStreamReceiver::Deferred)
         .expect("we expect a lockbox funding stream after NU5");
 
-    let funding_stream_height_range = FUNDING_STREAM_HEIGHT_RANGES
+    let funding_stream_height_range = POST_NU6_FUNDING_STREAM_HEIGHT_RANGES
         .get(&network.kind())
         .expect("must have funding stream height range on all networks");
 
     // `min(height, last_height_with_deferred_pool_contribution) - (nu6_activation_height - 1)`,
-    // funding stream height range end bound is not incremented since it's an exclusive end bound
-    let num_blocks_with_lockbox_output = height
-        .next()
-        .expect("should be a valid height")
-        .min(funding_stream_height_range.end)
-        - nu6_activation_height;
-
-    (deferred_amount_per_block
-        * u64::try_from(num_blocks_with_lockbox_output)
-            .expect("num blocks with lockbox funding stream should fit in u64"))
-    .expect("lockbox input value should fit in Amount")
+    // We decrement NU6 activation height since it's an inclusive lower bound.
+    // Funding stream height range end bound is not incremented since it's an exclusive end bound
+    let num_blocks_with_lockbox_output = (height.0 + 1)
+        .min(funding_stream_height_range.end.0)
+        .checked_sub(nu6_activation_height.0)
+        .unwrap_or_default();
+
+    (deferred_amount_per_block * num_blocks_with_lockbox_output.into())
+        .expect("lockbox input value should fit in Amount")
 }
 
 /// Returns all output amounts in `Transaction`.

zebra-consensus/src/parameters/subsidy.rs

@@ -106,12 +106,12 @@ lazy_static! {
         hash_map
     };
 
-    /// Start and end Heights for funding streams
+    /// Start and end Heights for pre-NU6 funding streams
     /// as described in [protocol specification §7.10.1][7.10.1].
     ///
     /// [7.10.1]: https://zips.z.cash/protocol/protocol.pdf#zip214fundingstreams
     // TODO: Move the value here to a field on `testnet::Parameters` (#8367)
-    pub static ref FUNDING_STREAM_HEIGHT_RANGES: HashMap<NetworkKind, std::ops::Range<Height>> = {
+    pub static ref PRE_NU6_FUNDING_STREAM_HEIGHT_RANGES: HashMap<NetworkKind, std::ops::Range<Height>> = {
         let mut hash_map = HashMap::new();
         // TODO: Adjust these values once a proposal is selected
         hash_map.insert(NetworkKind::Mainnet, Height(1_046_400)..Height(2_726_400));
@@ -120,6 +120,21 @@ lazy_static! {
         hash_map
     };
 
+    /// Start and end Heights for post-NU6 funding streams
+    /// as described in [protocol specification §7.10.1][7.10.1].
+    ///
+    /// [7.10.1]: https://zips.z.cash/protocol/protocol.pdf#zip214fundingstreams
+    // TODO: Move the value here to a field on `testnet::Parameters` (#8367)
+    pub static ref POST_NU6_FUNDING_STREAM_HEIGHT_RANGES: HashMap<NetworkKind, std::ops::Range<Height>> = {
+        let mut hash_map = HashMap::new();
+        // TODO: Adjust these values once a proposal is selected
+        hash_map.insert(NetworkKind::Mainnet, Height(2_726_400)..Height(3_146_400));
+        hash_map.insert(NetworkKind::Testnet, Height(2_942_000)..Height(3_362_000));
+        hash_map.insert(NetworkKind::Regtest, Height(2_942_000)..Height(3_362_000));
+        hash_map
+    };
+
+
     /// Convenient storage for all addresses, for all receivers and networks
     pub static ref FUNDING_STREAM_ADDRESSES: HashMap<NetworkKind, HashMap<FundingStreamReceiver, Vec<String>>> = {
         let mut addresses_by_network = HashMap::with_capacity(2);