fix(rpc): Mine standard and minimum difficulty blocks on testnet (#5747)

* Mine both standard and min difficulty blocks on testnet

* Add a POW_ADJUSTMENT_BLOCK_SPAN and fix an incorrect assertion

* Split the testnet adjustment into its own function

* Clarify a panic message

* Fix comments

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -393,7 +393,7 @@ where
 
                 size_limit: MAX_BLOCK_BYTES,
 
-                cur_time: chain_info.current_system_time.timestamp(),
+                cur_time: chain_info.cur_time.timestamp(),
 
                 bits: format!("{:#010x}", chain_info.expected_difficulty.to_value())
                     .drain(2..)

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs

@@ -151,7 +151,7 @@ pub async fn test_responses<State, ReadState>(
             .respond(ReadResponse::ChainInfo(Some(GetBlockTemplateChainInfo {
                 expected_difficulty: CompactDifficulty::from(ExpandedDifficulty::from(U256::one())),
                 tip: (fake_tip_height, fake_tip_hash),
-                current_system_time: fake_cur_time,
+                cur_time: fake_cur_time,
                 min_time: fake_min_time,
                 max_time: fake_max_time,
             })));

zebra-rpc/src/methods/tests/vectors.rs

@@ -849,7 +849,7 @@ async fn rpc_getblocktemplate() {
             .respond(ReadResponse::ChainInfo(Some(GetBlockTemplateChainInfo {
                 expected_difficulty: CompactDifficulty::from(ExpandedDifficulty::from(U256::one())),
                 tip: (fake_tip_height, fake_tip_hash),
-                current_system_time: fake_cur_time,
+                cur_time: fake_cur_time,
                 min_time: fake_min_time,
                 max_time: fake_max_time,
             })));

zebra-state/src/response.rs

@@ -147,7 +147,7 @@ pub struct GetBlockTemplateChainInfo {
     pub expected_difficulty: CompactDifficulty,
 
     /// The current system time, adjusted to fit within `min_time` and `max_time`.
-    pub current_system_time: chrono::DateTime<chrono::Utc>,
+    pub cur_time: chrono::DateTime<chrono::Utc>,
 
     /// The mininimum time the miner can use in this block.
     pub min_time: chrono::DateTime<chrono::Utc>,

zebra-state/src/service/check.rs

@@ -7,15 +7,14 @@ use chrono::Duration;
 use zebra_chain::{
     block::{self, Block, ChainHistoryBlockTxAuthCommitmentHash, CommitmentError},
     history_tree::HistoryTree,
-    parameters::POW_AVERAGING_WINDOW,
     parameters::{Network, NetworkUpgrade},
     work::difficulty::CompactDifficulty,
 };
 
 use crate::{
     service::{
-        block_iter::any_ancestor_blocks, finalized_state::FinalizedState,
+        block_iter::any_ancestor_blocks, check::difficulty::POW_ADJUSTMENT_BLOCK_SPAN,
-        non_finalized_state::NonFinalizedState,
+        finalized_state::FinalizedState, non_finalized_state::NonFinalizedState,
     },
     BoxError, PreparedBlock, ValidateContextError,
 };
@@ -35,7 +34,7 @@ pub(crate) mod utxo;
 #[cfg(test)]
 mod tests;
 
-pub(crate) use difficulty::{AdjustedDifficulty, POW_MEDIAN_BLOCK_SPAN};
+pub(crate) use difficulty::AdjustedDifficulty;
 
 /// Check that the `prepared` block is contextually valid for `network`, based
 /// on the `finalized_tip_height` and `relevant_chain`.
@@ -48,8 +47,7 @@ pub(crate) use difficulty::{AdjustedDifficulty, POW_MEDIAN_BLOCK_SPAN};
 ///
 /// # Panics
 ///
-/// If the state contains less than 28
+/// If the state contains less than 28 ([`POW_ADJUSTMENT_BLOCK_SPAN`]) blocks.
-/// (`POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN`) blocks.
 #[tracing::instrument(skip(prepared, finalized_tip_height, relevant_chain))]
 pub(crate) fn block_is_valid_for_recent_chain<C>(
     prepared: &PreparedBlock,
@@ -66,11 +64,9 @@ where
         .expect("finalized state must contain at least one block to do contextual validation");
     check::block_is_not_orphaned(finalized_tip_height, prepared.height)?;
 
-    // The maximum number of blocks used by contextual checks
-    const MAX_CONTEXT_BLOCKS: usize = POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN;
     let relevant_chain: Vec<_> = relevant_chain
         .into_iter()
-        .take(MAX_CONTEXT_BLOCKS)
+        .take(POW_ADJUSTMENT_BLOCK_SPAN)
         .collect();
 
     let parent_block = relevant_chain
@@ -84,14 +80,14 @@ where
 
     // skip this check during tests if we don't have enough blocks in the chain
     #[cfg(test)]
-    if relevant_chain.len() < POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN {
+    if relevant_chain.len() < POW_ADJUSTMENT_BLOCK_SPAN {
         return Ok(());
     }
     // process_queued also checks the chain length, so we can skip this assertion during testing
     // (tests that want to check this code should use the correct number of blocks)
     assert_eq!(
         relevant_chain.len(),
-        POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN,
+        POW_ADJUSTMENT_BLOCK_SPAN,
         "state must contain enough blocks to do proof of work contextual validation, \
          and validation must receive the exact number of required blocks"
     );

zebra-state/src/service/check/difficulty.rs

@@ -5,10 +5,10 @@
 //!  * the Testnet minimum difficulty adjustment from ZIPs 205 and 208, and
 //!  * `median-time-past`.
 
-use chrono::{DateTime, Duration, Utc};
-
 use std::{cmp::max, cmp::min, convert::TryInto};
 
+use chrono::{DateTime, Duration, Utc};
+
 use zebra_chain::{
     block,
     block::Block,
@@ -24,6 +24,11 @@ use zebra_chain::{
 /// `PoWMedianBlockSpan` in the Zcash specification.
 pub const POW_MEDIAN_BLOCK_SPAN: usize = 11;
 
+/// The overall block span used for adjusting Zcash block difficulty.
+///
+/// `PoWAveragingWindow + PoWMedianBlockSpan` in the Zcash specification.
+pub const POW_ADJUSTMENT_BLOCK_SPAN: usize = POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN;
+
 /// The damping factor for median timespan variance.
 ///
 /// `PoWDampingFactor` in the Zcash specification.
@@ -59,15 +64,14 @@ pub(crate) struct AdjustedDifficulty {
     /// The `header.difficulty_threshold`s from the previous
     /// `PoWAveragingWindow + PoWMedianBlockSpan` (28) blocks, in reverse height
     /// order.
-    relevant_difficulty_thresholds:
+    relevant_difficulty_thresholds: [CompactDifficulty; POW_ADJUSTMENT_BLOCK_SPAN],
-        [CompactDifficulty; POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN],
     /// The `header.time`s from the previous
     /// `PoWAveragingWindow + PoWMedianBlockSpan` (28) blocks, in reverse height
     /// order.
     ///
     /// Only the first and last `PoWMedianBlockSpan` times are used. Times
     /// `11..=16` are ignored.
-    relevant_times: [DateTime<Utc>; POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN],
+    relevant_times: [DateTime<Utc>; POW_ADJUSTMENT_BLOCK_SPAN],
 }
 
 impl AdjustedDifficulty {
@@ -131,7 +135,7 @@ impl AdjustedDifficulty {
 
         let (relevant_difficulty_thresholds, relevant_times) = context
             .into_iter()
-            .take(POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN)
+            .take(POW_ADJUSTMENT_BLOCK_SPAN)
             .unzip::<_, _, Vec<_>, Vec<_>>();
 
         let relevant_difficulty_thresholds = relevant_difficulty_thresholds

zebra-state/src/service/read/difficulty.rs

@@ -6,15 +6,15 @@ use chrono::{DateTime, Duration, TimeZone, Utc};
 
 use zebra_chain::{
     block::{Block, Hash, Height},
-    parameters::{Network, NetworkUpgrade, POW_AVERAGING_WINDOW},
+    parameters::{Network, NetworkUpgrade, POST_BLOSSOM_POW_TARGET_SPACING},
-    work::difficulty::{CompactDifficulty, ExpandedDifficulty},
+    work::difficulty::CompactDifficulty,
 };
 
 use crate::{
     service::{
         any_ancestor_blocks,
         check::{
-            difficulty::{BLOCK_MAX_TIME_SINCE_MEDIAN, POW_MEDIAN_BLOCK_SPAN},
+            difficulty::{BLOCK_MAX_TIME_SINCE_MEDIAN, POW_ADJUSTMENT_BLOCK_SPAN},
             AdjustedDifficulty,
         },
         finalized_state::ZebraDb,
@@ -23,12 +23,11 @@ use crate::{
     GetBlockTemplateChainInfo,
 };
 
-/// Returns :
+/// Returns the [`GetBlockTemplateChainInfo`] for the current best chain.
-/// - The `CompactDifficulty`, for the current best chain.
-/// - The current system time.
-/// - The minimum time for a next block.
 ///
-/// Panic if we don't have enough blocks in the state.
+/// # Panics
+///
+/// If we don't have enough blocks in the state.
 pub fn difficulty_and_time_info(
     non_finalized_state: &NonFinalizedState,
     db: &ZebraDb,
@@ -39,6 +38,9 @@ pub fn difficulty_and_time_info(
     difficulty_and_time(relevant_chain, tip, network)
 }
 
+/// Returns the [`GetBlockTemplateChainInfo`] for the current best chain.
+///
+/// See [`difficulty_and_time_info()`] for details.
 fn difficulty_and_time<C>(
     relevant_chain: C,
     tip: (Height, Hash),
@@ -49,11 +51,9 @@ where
     C::Item: Borrow<Block>,
     C::IntoIter: ExactSizeIterator,
 {
-    const MAX_CONTEXT_BLOCKS: usize = POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN;
-
     let relevant_chain: Vec<_> = relevant_chain
         .into_iter()
-        .take(MAX_CONTEXT_BLOCKS)
+        .take(POW_ADJUSTMENT_BLOCK_SPAN)
         .collect();
 
     let relevant_data: Vec<(CompactDifficulty, DateTime<Utc>)> = relevant_chain
@@ -70,27 +70,23 @@ where
     // So this will never happen in production code.
     assert_eq!(
         relevant_data.len(),
-        MAX_CONTEXT_BLOCKS,
+        POW_ADJUSTMENT_BLOCK_SPAN,
         "getblocktemplate RPC called with a near-empty state: should have returned an error",
     );
 
-    let current_system_time = chrono::Utc::now();
+    let cur_time = chrono::Utc::now();
 
-    // Get the median-time-past, which doesn't depend on the current system time.
+    // Get the median-time-past, which doesn't depend on the time or the previous block height.
     //
     // TODO: split out median-time-past into its own struct?
-    let median_time_past = AdjustedDifficulty::new_from_header_time(
+    let median_time_past =
-        current_system_time,
+        AdjustedDifficulty::new_from_header_time(cur_time, tip.0, network, relevant_data.clone())
-        tip.0,
+            .median_time_past();
-        network,
-        relevant_data.clone(),
-    )
-    .median_time_past();
 
     // > For each block other than the genesis block , nTime MUST be strictly greater than
     // > the median-time-past of that block.
     // https://zips.z.cash/protocol/protocol.pdf#blockheader
-    let mut min_time = median_time_past
+    let min_time = median_time_past
         .checked_add_signed(Duration::seconds(1))
         .expect("median time plus a small constant is far below i64::MAX");
 
@@ -102,92 +98,102 @@ where
         .checked_add_signed(Duration::seconds(BLOCK_MAX_TIME_SINCE_MEDIAN))
         .expect("median time plus a small constant is far below i64::MAX");
 
-    let current_system_time = current_system_time
+    let cur_time = cur_time
         .timestamp()
         .clamp(min_time.timestamp(), max_time.timestamp());
 
-    let mut current_system_time = Utc.timestamp_opt(current_system_time, 0).single().expect(
+    let cur_time = Utc.timestamp_opt(cur_time, 0).single().expect(
         "clamping a timestamp between two valid times can't make it invalid, and \
              UTC never has ambiguous time zone conversions",
     );
 
     // Now that we have a valid time, get the difficulty for that time.
-    let mut difficulty_adjustment = AdjustedDifficulty::new_from_header_time(
+    let difficulty_adjustment = AdjustedDifficulty::new_from_header_time(
-        current_system_time,
+        cur_time,
         tip.0,
         network,
         relevant_data.iter().cloned(),
     );
 
-    // On testnet, changing the block time can also change the difficulty,
+    let mut result = GetBlockTemplateChainInfo {
-    // due to the minimum difficulty consensus rule:
-    // > if the block time of a block at height height ≥ 299188
-    // > is greater than 6 * PoWTargetSpacing(height) seconds after that of the preceding block,
-    // > then the block is a minimum-difficulty block.
-    //
-    // In this case, we adjust the min_time and cur_time to the first minimum difficulty time.
-    //
-    // In rare cases, this could make some testnet miners produce invalid blocks,
-    // if they use the full 90 minute time gap in the consensus rules.
-    // (The getblocktemplate RPC reference doesn't have a max_time field,
-    // so there is no standard way of telling miners that the max_time is smaller.)
-    //
-    // But that's better than obscure failures caused by changing the time a small amount,
-    // if that moves the block from standard to minimum difficulty.
-    if network == Network::Testnet {
-        let max_time_difficulty_adjustment = AdjustedDifficulty::new_from_header_time(
-            max_time,
-            tip.0,
-            network,
-            relevant_data.iter().cloned(),
-        );
-
-        // The max time is a minimum difficulty block,
-        // so the time range could have different difficulties.
-        if max_time_difficulty_adjustment.expected_difficulty_threshold()
-            == ExpandedDifficulty::target_difficulty_limit(Network::Testnet).to_compact()
-        {
-            let min_time_difficulty_adjustment = AdjustedDifficulty::new_from_header_time(
-                min_time,
-                tip.0,
-                network,
-                relevant_data.iter().cloned(),
-            );
-
-            // Part of the valid range has a different difficulty.
-            // So we need to find the minimum time that is also a minimum difficulty block.
-            // This is the valid range for miners.
-            if min_time_difficulty_adjustment.expected_difficulty_threshold()
-                != max_time_difficulty_adjustment.expected_difficulty_threshold()
-            {
-                let preceding_block_time = relevant_data.last().expect("has at least one block").1;
-                let minimum_difficulty_spacing =
-                    NetworkUpgrade::minimum_difficulty_spacing_for_height(network, tip.0)
-                        .expect("just checked the minimum difficulty rule is active");
-
-                // The first minimum difficulty time is strictly greater than the spacing.
-                min_time = preceding_block_time + minimum_difficulty_spacing + Duration::seconds(1);
-
-                // Update the difficulty and times to match
-                if current_system_time < min_time {
-                    current_system_time = min_time;
-                }
-
-                difficulty_adjustment = AdjustedDifficulty::new_from_header_time(
-                    current_system_time,
-                    tip.0,
-                    network,
-                    relevant_data,
-                );
-            }
-        }
-    }
-
-    GetBlockTemplateChainInfo {
         tip,
         expected_difficulty: difficulty_adjustment.expected_difficulty_threshold(),
         min_time,
-        current_system_time,
+        cur_time,
         max_time,
+    };
+
+    adjust_difficulty_and_time_for_testnet(&mut result, network, tip.0, relevant_data);
+
+    result
+}
+
+/// Adjust the difficulty and time for the testnet minimum difficulty rule.
+fn adjust_difficulty_and_time_for_testnet(
+    result: &mut GetBlockTemplateChainInfo,
+    network: Network,
+    previous_block_height: Height,
+    relevant_data: Vec<(CompactDifficulty, DateTime<Utc>)>,
+) {
+    if network == Network::Mainnet {
+        return;
+    }
+
+    // On testnet, changing the block time can also change the difficulty,
+    // due to the minimum difficulty consensus rule:
+    // > if the block time of a block at height `height ≥ 299188`
+    // > is greater than 6 * PoWTargetSpacing(height) seconds after that of the preceding block,
+    // > then the block is a minimum-difficulty block.
+    //
+    // The max time is always a minimum difficulty block, because the minimum difficulty
+    // gap is 7.5 minutes, but the maximum gap is 90 minutes. This means that testnet blocks
+    // have two valid time ranges with different difficulties:
+    // * 1s - 7m30s: standard difficulty
+    // * 7m31s - 90m: minimum difficulty
+    //
+    // In rare cases, this could make some testnet miners produce invalid blocks,
+    // if they use the full 90 minute time gap in the consensus rules.
+    // (The zcashd getblocktemplate RPC reference doesn't have a max_time field,
+    // so there is no standard way of telling miners that the max_time is smaller.)
+    //
+    // So Zebra adjusts the min or max times to produce a valid time range for the difficulty.
+    // There is still a small chance that miners will produce an invalid block, if they are
+    // just below the max time, and don't check it.
+
+    let previous_block_time = relevant_data.last().expect("has at least one block").1;
+    let minimum_difficulty_spacing =
+        NetworkUpgrade::minimum_difficulty_spacing_for_height(network, previous_block_height)
+            .expect("just checked testnet, and the RPC returns an error for low heights");
+
+    // The first minimum difficulty time is strictly greater than the spacing.
+    let std_difficulty_max_time = previous_block_time + minimum_difficulty_spacing;
+    let min_difficulty_min_time = std_difficulty_max_time + Duration::seconds(1);
+
+    // If a miner is likely to find a block with the cur_time and standard difficulty
+    //
+    // We don't need to undo the clamping here:
+    // - if cur_time is clamped to min_time, then we're more likely to have a minimum
+    //    difficulty block, which makes mining easier;
+    // - if cur_time gets clamped to max_time, this is already a minimum difficulty block.
+    if result.cur_time + Duration::seconds(POST_BLOSSOM_POW_TARGET_SPACING * 2)
+        <= std_difficulty_max_time
+    {
+        // Standard difficulty: the max time needs to exclude min difficulty blocks
+        result.max_time = std_difficulty_max_time;
+    } else {
+        // Minimum difficulty: the min and cur time need to exclude min difficulty blocks
+        result.min_time = min_difficulty_min_time;
+        if result.cur_time < min_difficulty_min_time {
+            result.cur_time = min_difficulty_min_time;
+        }
+
+        // And then the difficulty needs to be updated for cur_time
+        result.expected_difficulty = AdjustedDifficulty::new_from_header_time(
+            result.cur_time,
+            previous_block_height,
+            network,
+            relevant_data.iter().cloned(),
+        )
+        .expected_difficulty_threshold();
     }
 }