change(rpc): Refactor get block template RPC into stages (#5837)

* Add some TODOs

* Move and rename height_from_signed_int()

* Move get_block_template() support functions to a submodule

* Fix incorrect P2SH comments and logs

* Split initial checks into their own functions

* Split state fetch into its own function, do some cleanup

* Move get_block_template_opts to get_block_template::parameters

* Fix and simplify test imports

* Rename block_height to next_block_height

* Rename to chain_tip_and_local_time to make it clear what it contains

* Split fetching mempool transactions out, include them in long poll id

* Refactor coinbase generation

* Split default root calculation into a separate function

* Use DateTime32 for getblocktemplate times

* Use typed difficulty fields rather than strings

* Split out a generate coinbase and roots function

* Move GetBlockTemplate construction into a method

* Document what happens to unusual difficulty values

* Clean up some TODOs

* fastmod check_address check_miner_address

* cargo fmt --all

* Fix an incorrect panic message

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

zebra-chain/src/block/hash.rs

@@ -1,9 +1,6 @@
 use std::{fmt, io, sync::Arc};
 
 use hex::{FromHex, ToHex};
-
-#[cfg(any(test, feature = "proptest-impl"))]
-use proptest_derive::Arbitrary;
 use serde::{Deserialize, Serialize};
 
 use crate::serialization::{
@@ -12,6 +9,9 @@ use crate::serialization::{
 
 use super::Header;
 
+#[cfg(any(test, feature = "proptest-impl"))]
+use proptest_derive::Arbitrary;
+
 /// A hash of a block, used to identify blocks and link blocks into a chain. ⛓️
 ///
 /// Technically, this is the (SHA256d) hash of a block *header*, but since the

zebra-chain/src/lib.rs

@@ -40,3 +40,10 @@ pub mod work;
 
 #[cfg(any(test, feature = "proptest-impl"))]
 pub use block::LedgerState;
+
+/// Error type alias to make working with generic errors easier.
+///
+/// Note: the 'static lifetime bound means that the *type* cannot have any
+/// non-'static lifetimes, (e.g., when a type contains a borrow and is
+/// parameterized by 'a), *not* that the object itself has 'static lifetime.
+pub type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;

zebra-chain/src/parameters/network_upgrade.rs

@@ -197,7 +197,7 @@ pub(crate) const CONSENSUS_BRANCH_IDS: &[(NetworkUpgrade, ConsensusBranchId)] =
 const PRE_BLOSSOM_POW_TARGET_SPACING: i64 = 150;
 
 /// The target block spacing after Blossom activation.
-pub const POST_BLOSSOM_POW_TARGET_SPACING: i64 = 75;
+pub const POST_BLOSSOM_POW_TARGET_SPACING: u32 = 75;
 
 /// The averaging window for difficulty threshold arithmetic mean calculations.
 ///
@@ -337,7 +337,7 @@ impl NetworkUpgrade {
     pub fn target_spacing(&self) -> Duration {
         let spacing_seconds = match self {
             Genesis | BeforeOverwinter | Overwinter | Sapling => PRE_BLOSSOM_POW_TARGET_SPACING,
-            Blossom | Heartwood | Canopy | Nu5 => POST_BLOSSOM_POW_TARGET_SPACING,
+            Blossom | Heartwood | Canopy | Nu5 => POST_BLOSSOM_POW_TARGET_SPACING.into(),
         };
 
         Duration::seconds(spacing_seconds)
@@ -354,7 +354,10 @@ impl NetworkUpgrade {
     pub fn target_spacings(network: Network) -> impl Iterator<Item = (block::Height, Duration)> {
         [
             (NetworkUpgrade::Genesis, PRE_BLOSSOM_POW_TARGET_SPACING),
-            (NetworkUpgrade::Blossom, POST_BLOSSOM_POW_TARGET_SPACING),
+            (
+                NetworkUpgrade::Blossom,
+                POST_BLOSSOM_POW_TARGET_SPACING.into(),
+            ),
         ]
         .into_iter()
         .map(move |(upgrade, spacing_seconds)| {

zebra-chain/src/serialization/date_time.rs

@@ -1,10 +1,6 @@
 //! DateTime types with specific serialization invariants.
 
-use std::{
-    convert::{TryFrom, TryInto},
-    fmt,
-    num::TryFromIntError,
-};
+use std::{fmt, num::TryFromIntError};
 
 use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use chrono::{TimeZone, Utc};
@@ -12,7 +8,8 @@ use chrono::{TimeZone, Utc};
 use super::{SerializationError, ZcashDeserialize, ZcashSerialize};
 
 /// A date and time, represented by a 32-bit number of seconds since the UNIX epoch.
-#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
+#[serde(transparent)]
 pub struct DateTime32 {
     timestamp: u32,
 }

zebra-chain/src/work/difficulty.rs

@@ -9,19 +9,18 @@
 //! The block work is used to find the chain with the greatest total work. Each
 //! block's work value depends on the fixed threshold in the block header, not
 //! the actual work represented by the block header hash.
-#![allow(clippy::unit_arg)]
-
-use crate::{block, parameters::Network};
 
 use std::{
     cmp::{Ordering, PartialEq, PartialOrd},
     fmt,
     iter::Sum,
-    ops::Add,
-    ops::Div,
-    ops::Mul,
+    ops::{Add, Div, Mul},
 };
 
+use hex::{FromHex, ToHex};
+
+use crate::{block, parameters::Network, BoxError};
+
 pub use crate::work::u256::U256;
 
 #[cfg(any(test, feature = "proptest-impl"))]
@@ -60,19 +59,6 @@ mod tests;
 #[derive(Clone, Copy, Eq, PartialEq, Serialize, Deserialize)]
 pub struct CompactDifficulty(pub(crate) u32);
 
-impl fmt::Debug for CompactDifficulty {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        // There isn't a standard way to show different representations of the
-        // same value
-        f.debug_tuple("CompactDifficulty")
-            // Use hex, because it's a float
-            .field(&format_args!("{:#010x}", self.0))
-            // Use expanded difficulty, for bitwise difficulty comparisons
-            .field(&format_args!("{:?}", self.to_expanded()))
-            .finish()
-    }
-}
-
 /// An invalid CompactDifficulty value, for testing.
 pub const INVALID_COMPACT_DIFFICULTY: CompactDifficulty = CompactDifficulty(u32::MAX);
 
@@ -101,28 +87,6 @@ pub const INVALID_COMPACT_DIFFICULTY: CompactDifficulty = CompactDifficulty(u32:
 #[derive(Clone, Copy, Eq, PartialEq, Ord, PartialOrd)]
 pub struct ExpandedDifficulty(U256);
 
-impl fmt::Debug for ExpandedDifficulty {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let mut buf = [0; 32];
-        // Use the same byte order as block::Hash
-        self.0.to_big_endian(&mut buf);
-        f.debug_tuple("ExpandedDifficulty")
-            .field(&hex::encode(buf))
-            .finish()
-    }
-}
-
-#[cfg(feature = "getblocktemplate-rpcs")]
-impl fmt::Display for ExpandedDifficulty {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let mut buf = [0; 32];
-        // Use the same byte order as block::Hash
-        self.0.to_big_endian(&mut buf);
-
-        f.write_str(&hex::encode(buf))
-    }
-}
-
 /// A 128-bit unsigned "Work" value.
 ///
 /// Used to calculate the total work for each chain of blocks.
@@ -269,10 +233,81 @@ impl CompactDifficulty {
         Work::try_from(expanded).ok()
     }
 
-    #[cfg(feature = "getblocktemplate-rpcs")]
-    /// Returns the raw inner value.
-    pub fn to_value(&self) -> u32 {
-        self.0
+    /// Return the difficulty bytes in big-endian byte-order.
+    ///
+    /// Zebra displays difficulties in big-endian byte-order,
+    /// following the u256 convention set by Bitcoin and zcashd.
+    pub fn bytes_in_display_order(&self) -> [u8; 4] {
+        self.0.to_be_bytes()
+    }
+
+    /// Convert bytes in big-endian byte-order into a [`CompactDifficulty`].
+    ///
+    /// Zebra displays difficulties in big-endian byte-order,
+    /// following the u256 convention set by Bitcoin and zcashd.
+    ///
+    /// Returns an error if the difficulty value is invalid.
+    pub fn from_bytes_in_display_order(
+        bytes_in_display_order: &[u8; 4],
+    ) -> Result<CompactDifficulty, BoxError> {
+        let internal_byte_order = u32::from_be_bytes(*bytes_in_display_order);
+
+        let difficulty = CompactDifficulty(internal_byte_order);
+
+        if difficulty.to_expanded().is_none() {
+            return Err("invalid difficulty value".into());
+        }
+
+        Ok(difficulty)
+    }
+}
+
+impl fmt::Debug for CompactDifficulty {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        // There isn't a standard way to show different representations of the
+        // same value
+        f.debug_tuple("CompactDifficulty")
+            // Use hex, because it's a float
+            .field(&format_args!("{:#010x}", self.0))
+            // Use expanded difficulty, for bitwise difficulty comparisons
+            .field(&format_args!("{:?}", self.to_expanded()))
+            .finish()
+    }
+}
+
+impl fmt::Display for CompactDifficulty {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.write_str(&self.encode_hex::<String>())
+    }
+}
+
+impl ToHex for &CompactDifficulty {
+    fn encode_hex<T: FromIterator<char>>(&self) -> T {
+        self.bytes_in_display_order().encode_hex()
+    }
+
+    fn encode_hex_upper<T: FromIterator<char>>(&self) -> T {
+        self.bytes_in_display_order().encode_hex_upper()
+    }
+}
+
+impl ToHex for CompactDifficulty {
+    fn encode_hex<T: FromIterator<char>>(&self) -> T {
+        (&self).encode_hex()
+    }
+
+    fn encode_hex_upper<T: FromIterator<char>>(&self) -> T {
+        (&self).encode_hex_upper()
+    }
+}
+
+impl FromHex for CompactDifficulty {
+    type Error = BoxError;
+
+    fn from_hex<T: AsRef<[u8]>>(hex: T) -> Result<Self, Self::Error> {
+        let bytes_in_display_order = <[u8; 4]>::from_hex(hex)?;
+
+        CompactDifficulty::from_bytes_in_display_order(&bytes_in_display_order)
     }
 }
 
@@ -401,6 +436,78 @@ impl ExpandedDifficulty {
             unreachable!("converted CompactDifficulty values must be valid")
         }
     }
+
+    /// Return the difficulty bytes in big-endian byte-order,
+    /// suitable for printing out byte by byte.
+    ///
+    /// Zebra displays difficulties in big-endian byte-order,
+    /// following the u256 convention set by Bitcoin and zcashd.
+    pub fn bytes_in_display_order(&self) -> [u8; 32] {
+        let mut reversed_bytes = [0; 32];
+        self.0.to_big_endian(&mut reversed_bytes);
+
+        reversed_bytes
+    }
+
+    /// Convert bytes in big-endian byte-order into an [`ExpandedDifficulty`].
+    ///
+    /// Zebra displays difficulties in big-endian byte-order,
+    /// following the u256 convention set by Bitcoin and zcashd.
+    ///
+    /// Preserves the exact difficulty value represented by the bytes,
+    /// even if it can't be generated from a [`CompactDifficulty`].
+    /// This means a round-trip conversion to [`CompactDifficulty`] can be lossy.
+    pub fn from_bytes_in_display_order(bytes_in_display_order: &[u8; 32]) -> ExpandedDifficulty {
+        let internal_byte_order = U256::from_big_endian(bytes_in_display_order);
+
+        ExpandedDifficulty(internal_byte_order)
+    }
+}
+
+impl fmt::Display for ExpandedDifficulty {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.write_str(&self.encode_hex::<String>())
+    }
+}
+
+impl fmt::Debug for ExpandedDifficulty {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_tuple("ExpandedDifficulty")
+            .field(&self.encode_hex::<String>())
+            .finish()
+    }
+}
+
+impl ToHex for &ExpandedDifficulty {
+    fn encode_hex<T: FromIterator<char>>(&self) -> T {
+        self.bytes_in_display_order().encode_hex()
+    }
+
+    fn encode_hex_upper<T: FromIterator<char>>(&self) -> T {
+        self.bytes_in_display_order().encode_hex_upper()
+    }
+}
+
+impl ToHex for ExpandedDifficulty {
+    fn encode_hex<T: FromIterator<char>>(&self) -> T {
+        (&self).encode_hex()
+    }
+
+    fn encode_hex_upper<T: FromIterator<char>>(&self) -> T {
+        (&self).encode_hex_upper()
+    }
+}
+
+impl FromHex for ExpandedDifficulty {
+    type Error = <[u8; 32] as FromHex>::Error;
+
+    fn from_hex<T: AsRef<[u8]>>(hex: T) -> Result<Self, Self::Error> {
+        let bytes_in_display_order = <[u8; 32]>::from_hex(hex)?;
+
+        Ok(ExpandedDifficulty::from_bytes_in_display_order(
+            &bytes_in_display_order,
+        ))
+    }
 }
 
 impl From<U256> for ExpandedDifficulty {

zebra-rpc/src/methods.rs

@@ -501,7 +501,8 @@ where
         .boxed()
     }
 
-    // TODO: use HexData to handle transaction data, and a generic error constructor (#5548)
+    // TODO: use HexData or GetRawTransaction::Bytes to handle the transaction data argument
+    //       use a generic error constructor (#5548)
     fn send_raw_transaction(
         &self,
         raw_transaction_hex: String,
@@ -556,7 +557,11 @@ where
         .boxed()
     }
 
-    // TODO: use a generic error constructor (#5548)
+    // TODO:
+    // - use a generic error constructor (#5548)
+    // - use `height_from_signed_int()` to handle negative heights
+    //   (this might be better in the state request, because it needs the state height)
+    // - create a function that handles block hashes or heights, and use it in `z_get_treestate()`
     fn get_block(&self, height: String, verbosity: u8) -> BoxFuture<Result<GetBlock>> {
         let mut state = self.state.clone();
 
@@ -644,6 +649,7 @@ where
         async move {
             let request = mempool::Request::TransactionIds;
 
+            // `zcashd` doesn't check if it is synced to the tip here, so we don't either.
             let response = mempool
                 .ready()
                 .and_then(|service| service.call(request))
@@ -673,7 +679,8 @@ where
         .boxed()
     }
 
-    // TODO: use HexData to handle the transaction ID, and a generic error constructor (#5548)
+    // TODO: use HexData or SentTransactionHash to handle the transaction ID
+    //       use a generic error constructor (#5548)
     fn get_raw_transaction(
         &self,
         txid_hex: String,
@@ -757,7 +764,11 @@ where
         .boxed()
     }
 
-    // TODO: use a generic error constructor (#5548)
+    // TODO:
+    // - use a generic error constructor (#5548)
+    // - use `height_from_signed_int()` to handle negative heights
+    //   (this might be better in the state request, because it needs the state height)
+    // - create a function that handles block hashes or heights, and use it in `get_block()`
     fn z_get_treestate(&self, hash_or_height: String) -> BoxFuture<Result<GetTreestate>> {
         let mut state = self.state.clone();
 
@@ -1346,3 +1357,49 @@ fn check_height_range(start: Height, end: Height, chain_height: Height) -> Resul
 
     Ok(())
 }
+
+/// Given a potentially negative index, find the corresponding `Height`.
+///
+/// This function is used to parse the integer index argument of `get_block_hash`.
+/// This is based on zcashd's implementation:
+/// <https://github.com/zcash/zcash/blob/c267c3ee26510a974554f227d40a89e3ceb5bb4d/src/rpc/blockchain.cpp#L589-L618>
+//
+// TODO: also use this function in `get_block` and `z_get_treestate`
+#[allow(dead_code)]
+pub fn height_from_signed_int(index: i32, tip_height: Height) -> Result<Height> {
+    if index >= 0 {
+        let height = index.try_into().expect("Positive i32 always fits in u32");
+        if height > tip_height.0 {
+            return Err(Error::invalid_params(
+                "Provided index is greater than the current tip",
+            ));
+        }
+        Ok(Height(height))
+    } else {
+        // `index + 1` can't overflow, because `index` is always negative here.
+        let height = i32::try_from(tip_height.0)
+            .expect("tip height fits in i32, because Height::MAX fits in i32")
+            .checked_add(index + 1);
+
+        let sanitized_height = match height {
+            None => return Err(Error::invalid_params("Provided index is not valid")),
+            Some(h) => {
+                if h < 0 {
+                    return Err(Error::invalid_params(
+                        "Provided negative index ends up with a negative height",
+                    ));
+                }
+                let h: u32 = h.try_into().expect("Positive i32 always fits in u32");
+                if h > tip_height.0 {
+                    return Err(Error::invalid_params(
+                        "Provided index is greater than the current tip",
+                    ));
+                }
+
+                h
+            }
+        };
+
+        Ok(Height(sanitized_height))
+    }
+}

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -1,6 +1,6 @@
 //! RPC methods related to mining only available with `getblocktemplate-rpcs` rust feature.
 
-use std::{iter, sync::Arc};
+use std::sync::Arc;
 
 use futures::{FutureExt, TryFutureExt};
 use jsonrpc_core::{self, BoxFuture, Error, ErrorCode, Result};
@@ -8,48 +8,37 @@ use jsonrpc_derive::rpc;
 use tower::{buffer::Buffer, Service, ServiceExt};
 
 use zebra_chain::{
-    amount::{self, Amount, NegativeOrZero, NonNegative},
-    block::{
-        self,
-        merkle::{self, AuthDataRoot},
-        Block, ChainHistoryBlockTxAuthCommitmentHash, Height, MAX_BLOCK_BYTES, ZCASH_BLOCK_VERSION,
-    },
+    block::{self, Block, Height},
     chain_sync_status::ChainSyncStatus,
     chain_tip::ChainTip,
     parameters::Network,
     serialization::ZcashDeserializeInto,
-    transaction::{Transaction, UnminedTx, VerifiedUnminedTx},
     transparent,
 };
-
-use zebra_consensus::{
-    funding_stream_address, funding_stream_values, miner_subsidy, VerifyChainError,
-    MAX_BLOCK_SIGOPS,
-};
-
+use zebra_consensus::VerifyChainError;
 use zebra_node_services::mempool;
-
 use zebra_state::{ReadRequest, ReadResponse};
 
 use crate::methods::{
     best_chain_tip_height,
     get_block_template_rpcs::{
-        constants::{
-            DEFAULT_SOLUTION_RATE_WINDOW_SIZE, GET_BLOCK_TEMPLATE_CAPABILITIES_FIELD,
-            GET_BLOCK_TEMPLATE_MUTABLE_FIELD, GET_BLOCK_TEMPLATE_NONCE_RANGE_FIELD,
-            MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP, NOT_SYNCED_ERROR_CODE,
+        constants::DEFAULT_SOLUTION_RATE_WINDOW_SIZE,
+        get_block_template::{
+            check_block_template_parameters, check_miner_address, check_synced_to_tip,
+            fetch_mempool_transactions, fetch_state_tip_and_local_time,
+            generate_coinbase_and_roots,
         },
         types::{
-            default_roots::DefaultRoots, get_block_template::GetBlockTemplate,
-            get_block_template_opts::GetBlockTemplateRequestMode, hex_data::HexData,
-            long_poll::LongPollInput, submit_block, transaction::TransactionTemplate,
+            get_block_template::GetBlockTemplate, get_mining_info, hex_data::HexData,
+            long_poll::LongPollInput, submit_block,
         },
     },
-    GetBlockHash, MISSING_BLOCK_ERROR_CODE,
+    height_from_signed_int, GetBlockHash, MISSING_BLOCK_ERROR_CODE,
 };
 
 pub mod config;
 pub mod constants;
+pub mod get_block_template;
 pub mod types;
 pub mod zip317;
 
@@ -109,7 +98,7 @@ pub trait GetBlockTemplateRpc {
     #[rpc(name = "getblocktemplate")]
     fn get_block_template(
         &self,
-        options: Option<types::get_block_template_opts::JsonParameters>,
+        parameters: Option<get_block_template::JsonParameters>,
     ) -> BoxFuture<Result<GetBlockTemplate>>;
 
     /// Submits block to the node to be validated and committed.
@@ -125,14 +114,14 @@ pub trait GetBlockTemplateRpc {
     fn submit_block(
         &self,
         hex_data: HexData,
-        _options: Option<submit_block::JsonParameters>,
+        _parameters: Option<submit_block::JsonParameters>,
     ) -> BoxFuture<Result<submit_block::Response>>;
 
     /// Returns mining-related information.
     ///
     /// zcashd reference: [`getmininginfo`](https://zcash.github.io/rpc/getmininginfo.html)
     #[rpc(name = "getmininginfo")]
-    fn get_mining_info(&self) -> BoxFuture<Result<types::get_mining_info::Response>>;
+    fn get_mining_info(&self) -> BoxFuture<Result<get_mining_info::Response>>;
 
     /// Returns the estimated network solutions per second based on the last `num_blocks` before `height`.
     /// If `num_blocks` is not supplied, uses 120 blocks.
@@ -181,8 +170,6 @@ where
         + 'static,
     SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
 {
-    // TODO: Add the other fields from the [`Rpc`] struct as-needed
-
     // Configuration
     //
     /// The configured network for this RPC service.
@@ -190,7 +177,7 @@ where
 
     /// The configured miner address for this RPC service.
     ///
-    /// Zebra currently only supports single-signature P2SH transparent addresses.
+    /// Zebra currently only supports transparent addresses.
     miner_address: Option<transparent::Address>,
 
     // Services
@@ -294,9 +281,10 @@ where
         let latest_chain_tip = self.latest_chain_tip.clone();
 
         async move {
+            // TODO: look up this height as part of the state request?
             let tip_height = best_chain_tip_height(&latest_chain_tip)?;
 
-            let height = get_height_from_int(index, tip_height)?;
+            let height = height_from_signed_int(index, tip_height)?;
 
             let request = zebra_state::ReadRequest::BestChainBlockHash(height);
             let response = state
@@ -325,168 +313,105 @@ where
     // TODO: use HexData to handle block proposal data, and a generic error constructor (#5548)
     fn get_block_template(
         &self,
-        options: Option<types::get_block_template_opts::JsonParameters>,
+        parameters: Option<get_block_template::JsonParameters>,
     ) -> BoxFuture<Result<GetBlockTemplate>> {
+        // Clone Config
         let network = self.network;
         let miner_address = self.miner_address;
 
+        // Clone Services
         let mempool = self.mempool.clone();
         let latest_chain_tip = self.latest_chain_tip.clone();
         let sync_status = self.sync_status.clone();
-        let mut state = self.state.clone();
+        let state = self.state.clone();
 
-        // Since this is a very large RPC, we use separate functions for each group of fields.
+        // To implement long polling correctly, we split this RPC into multiple phases.
         async move {
-            if let Some(options) = options {
-                if options.data.is_some() || options.mode == GetBlockTemplateRequestMode::Proposal {
-                    return Err(Error {
-                        code: ErrorCode::InvalidParams,
-                        message: "\"proposal\" mode is currently unsupported by Zebra".to_string(),
-                        data: None,
-                    })
-                }
+            // - One-off checks
+
+            // Check config and parameters.
+            // These checks always have the same result during long polling.
+            let miner_address = check_miner_address(miner_address)?;
+
+            if let Some(parameters) = parameters {
+                check_block_template_parameters(parameters)?;
             }
 
-            let miner_address = miner_address.ok_or_else(|| Error {
-                code: ErrorCode::ServerError(0),
-                message: "configure mining.miner_address in zebrad.toml \
-                          with a transparent P2SH address"
-                    .to_string(),
-                data: None,
-            })?;
+            // - Checks and fetches that can change during long polling
 
-            // The tip estimate may not be the same as the one coming from the state
-            // but this is ok for an estimate
-            let (estimated_distance_to_chain_tip, local_tip_height) = latest_chain_tip
-                .estimate_distance_to_network_chain_tip(network)
-                .ok_or_else(|| Error {
-                    code: ErrorCode::ServerError(0),
-                    message: "No Chain tip available yet".to_string(),
-                    data: None,
-                })?;
+            // Check if we are synced to the tip.
+            // The result of this check can change during long polling.
+            check_synced_to_tip(network, latest_chain_tip, sync_status)?;
 
-            if !sync_status.is_close_to_tip() || estimated_distance_to_chain_tip > MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP {
-                tracing::info!(
-                    estimated_distance_to_chain_tip,
-                    ?local_tip_height,
-                    "Zebra has not synced to the chain tip"
-                );
+            // Fetch the state data and local time for the block template:
+            // - if the tip block hash changes, we must return from long polling,
+            // - if the local clock changes on testnet, we might return from long polling
+            //
+            // We also return after 90 minutes on mainnet, even if we have the same response.
+            let chain_tip_and_local_time = fetch_state_tip_and_local_time(state).await?;
 
-                return Err(Error {
-                    code: NOT_SYNCED_ERROR_CODE,
-                    message: format!("Zebra has not synced to the chain tip, estimated distance: {estimated_distance_to_chain_tip}"),
-                    data: None,
-                });
-            }
+            // Fetch the mempool data for the block template:
+            // - if the mempool transactions change, we might return from long polling.
+            let mempool_txs = fetch_mempool_transactions(mempool).await?;
 
-            // Calling state with `ChainInfo` request for relevant chain data
-            let request = ReadRequest::ChainInfo;
-            let response = state
-                .ready()
-                .and_then(|service| service.call(request))
-                .await
-                .map_err(|error| Error {
-                    code: ErrorCode::ServerError(0),
-                    message: error.to_string(),
-                                data: None,
-                            })?;
+            // - Long poll ID calculation
+            //
+            // TODO: check if the client passed the same long poll id,
+            //       if they did, wait until the inputs change,
+            //       or the max time, or Zebra is no longer synced to the tip.
 
-            let chain_info = match response {
-                ReadResponse::ChainInfo(chain_info) => chain_info,
-                _ => unreachable!("we should always have enough state data here to get a `GetBlockTemplateChainInfo`"),
-            };
+            let long_poll_id = LongPollInput::new(
+                chain_tip_and_local_time.tip_height,
+                chain_tip_and_local_time.tip_hash,
+                chain_tip_and_local_time.max_time,
+                mempool_txs.iter().map(|tx| tx.transaction.id),
+            )
+            .into();
 
-            // Get the tip data from the state call
-            let block_height = (chain_info.tip_height + 1).expect("tip is far below Height::MAX");
+            // - Processing fetched data to create a transaction template
+            //
+            // Apart from random weighted transaction selection,
+            // the template only depends on the previously fetched data.
+            // This processing never fails.
 
-            // Use a fake coinbase transaction to break the dependency between transaction
-            // selection, the miner fee, and the fee payment in the coinbase transaction.
-            let fake_coinbase_tx = fake_coinbase_transaction(network, block_height, miner_address);
-            let mempool_txs = zip317::select_mempool_transactions(fake_coinbase_tx, mempool).await?;
+            // Calculate the next block height.
+            let next_block_height =
+                (chain_tip_and_local_time.tip_height + 1).expect("tip is far below Height::MAX");
 
-            let miner_fee = miner_fee(&mempool_txs);
+            // Randomly select some mempool transactions.
+            //
+            // TODO: sort these transactions to match zcashd's order, to make testing easier.
+            let mempool_txs = zip317::select_mempool_transactions(
+                network,
+                next_block_height,
+                miner_address,
+                mempool_txs,
+            )
+            .await;
 
-            let outputs =
-                standard_coinbase_outputs(network, block_height, miner_address, miner_fee);
-            let coinbase_tx = Transaction::new_v5_coinbase(network, block_height, outputs).into();
+            // - After this point, the template only depends on the previously fetched data.
 
-            let (merkle_root, auth_data_root) =
-                calculate_transaction_roots(&coinbase_tx, &mempool_txs);
-
-            let history_tree = chain_info.history_tree;
-            // TODO: move expensive cryptography to a rayon thread?
-            let chain_history_root = history_tree.hash().expect("history tree can't be empty");
-
-            // TODO: move expensive cryptography to a rayon thread?
-            let block_commitments_hash = ChainHistoryBlockTxAuthCommitmentHash::from_commitments(
-                &chain_history_root,
-                &auth_data_root,
+            // Generate the coinbase transaction and default roots
+            //
+            // TODO: move expensive root, hash, and tree cryptography to a rayon thread?
+            let (coinbase_txn, default_roots) = generate_coinbase_and_roots(
+                network,
+                next_block_height,
+                miner_address,
+                &mempool_txs,
+                chain_tip_and_local_time.history_tree.clone(),
             );
 
-            // TODO: use the entire mempool content via a watch channel,
-            //       rather than just the randomly selected transactions
-            let long_poll_id = LongPollInput::new(
-                chain_info.tip_height,
-                chain_info.tip_hash,
-                chain_info.max_time,
-                mempool_txs.iter().map(|tx| tx.transaction.id),
-            ).into();
-
-            // Convert into TransactionTemplates
-            let mempool_txs = mempool_txs.iter().map(Into::into).collect();
-
-            let capabilities: Vec<String> = GET_BLOCK_TEMPLATE_CAPABILITIES_FIELD.iter().map(ToString::to_string).collect();
-            let mutable: Vec<String> = GET_BLOCK_TEMPLATE_MUTABLE_FIELD.iter().map(ToString::to_string).collect();
-
-            Ok(GetBlockTemplate {
-                capabilities,
-
-                version: ZCASH_BLOCK_VERSION,
-
-                previous_block_hash: GetBlockHash(chain_info.tip_hash),
-                block_commitments_hash,
-                light_client_root_hash: block_commitments_hash,
-                final_sapling_root_hash: block_commitments_hash,
-                default_roots: DefaultRoots {
-                    merkle_root,
-                    chain_history_root,
-                    auth_data_root,
-                    block_commitments_hash,
-                },
-
-                transactions: mempool_txs,
-
-                coinbase_txn: TransactionTemplate::from_coinbase(&coinbase_tx, miner_fee),
-
+            let response = GetBlockTemplate::new(
+                next_block_height,
+                &chain_tip_and_local_time,
                 long_poll_id,
+                coinbase_txn,
+                &mempool_txs,
+                default_roots,
+            );
 
-                target: format!(
-                    "{}",
-                    chain_info.expected_difficulty
-                        .to_expanded()
-                        .expect("state always returns a valid difficulty value")
-                ),
-
-                min_time: chain_info.min_time.timestamp(),
-
-                mutable,
-
-                nonce_range: GET_BLOCK_TEMPLATE_NONCE_RANGE_FIELD.to_string(),
-
-                sigop_limit: MAX_BLOCK_SIGOPS,
-
-                size_limit: MAX_BLOCK_BYTES,
-
-                cur_time: chain_info.cur_time.timestamp(),
-
-                bits: format!("{:#010x}", chain_info.expected_difficulty.to_value())
-                    .drain(2..)
-                    .collect(),
-
-                height: block_height.0,
-
-                max_time: chain_info.max_time.timestamp(),
-            })
+            Ok(response)
         }
         .boxed()
     }
@@ -494,7 +419,7 @@ where
     fn submit_block(
         &self,
         HexData(block_bytes): HexData,
-        _options: Option<submit_block::JsonParameters>,
+        _parameters: Option<submit_block::JsonParameters>,
     ) -> BoxFuture<Result<submit_block::Response>> {
         let mut chain_verifier = self.chain_verifier.clone();
 
@@ -563,11 +488,11 @@ where
         .boxed()
     }
 
-    fn get_mining_info(&self) -> BoxFuture<Result<types::get_mining_info::Response>> {
+    fn get_mining_info(&self) -> BoxFuture<Result<get_mining_info::Response>> {
         let network = self.network;
         let solution_rate_fut = self.get_network_sol_ps(None, None);
         async move {
-            Ok(types::get_mining_info::Response::new(
+            Ok(get_mining_info::Response::new(
                 network,
                 solution_rate_fut.await?,
             ))
@@ -616,141 +541,4 @@ where
     }
 }
 
-// get_block_template support methods
-
-/// Returns the total miner fee for `mempool_txs`.
-pub fn miner_fee(mempool_txs: &[VerifiedUnminedTx]) -> Amount<NonNegative> {
-    let miner_fee: amount::Result<Amount<NonNegative>> =
-        mempool_txs.iter().map(|tx| tx.miner_fee).sum();
-
-    miner_fee.expect(
-        "invalid selected transactions: \
-         fees in a valid block can not be more than MAX_MONEY",
-    )
-}
-
-/// Returns the standard funding stream and miner reward transparent output scripts
-/// for `network`, `height` and `miner_fee`.
-///
-/// Only works for post-Canopy heights.
-pub fn standard_coinbase_outputs(
-    network: Network,
-    height: Height,
-    miner_address: transparent::Address,
-    miner_fee: Amount<NonNegative>,
-) -> Vec<(Amount<NonNegative>, transparent::Script)> {
-    let funding_streams = funding_stream_values(height, network)
-        .expect("funding stream value calculations are valid for reasonable chain heights");
-
-    let mut funding_streams: Vec<(Amount<NonNegative>, transparent::Address)> = funding_streams
-        .iter()
-        .map(|(receiver, amount)| (*amount, funding_stream_address(height, network, *receiver)))
-        .collect();
-    // The HashMap returns funding streams in an arbitrary order,
-    // but Zebra's snapshot tests expect the same order every time.
-    funding_streams.sort_by_key(|(amount, _address)| *amount);
-
-    let miner_reward = miner_subsidy(height, network)
-        .expect("reward calculations are valid for reasonable chain heights")
-        + miner_fee;
-    let miner_reward =
-        miner_reward.expect("reward calculations are valid for reasonable chain heights");
-
-    let mut coinbase_outputs = funding_streams;
-    coinbase_outputs.push((miner_reward, miner_address));
-
-    coinbase_outputs
-        .iter()
-        .map(|(amount, address)| (*amount, address.create_script_from_address()))
-        .collect()
-}
-
-/// Returns a fake coinbase transaction that can be used during transaction selection.
-///
-/// This avoids a data dependency loop involving the selected transactions, the miner fee,
-/// and the coinbase transaction.
-///
-/// This transaction's serialized size and sigops must be at least as large as the real coinbase
-/// transaction with the correct height and fee.
-fn fake_coinbase_transaction(
-    network: Network,
-    block_height: Height,
-    miner_address: transparent::Address,
-) -> TransactionTemplate<NegativeOrZero> {
-    // Block heights are encoded as variable-length (script) and `u32` (lock time, expiry height).
-    // They can also change the `u32` consensus branch id.
-    // We use the template height here, which has the correct byte length.
-    // https://zips.z.cash/protocol/protocol.pdf#txnconsensus
-    // https://github.com/zcash/zips/blob/main/zip-0203.rst#changes-for-nu5
-    //
-    // Transparent amounts are encoded as `i64`,
-    // so one zat has the same size as the real amount:
-    // https://developer.bitcoin.org/reference/transactions.html#txout-a-transaction-output
-    let miner_fee = 1.try_into().expect("amount is valid and non-negative");
-
-    let outputs = standard_coinbase_outputs(network, block_height, miner_address, miner_fee);
-    let coinbase_tx = Transaction::new_v5_coinbase(network, block_height, outputs).into();
-
-    TransactionTemplate::from_coinbase(&coinbase_tx, miner_fee)
-}
-
-/// Returns the transaction effecting and authorizing roots
-/// for `coinbase_tx` and `mempool_txs`.
-//
-// TODO: should this be spawned into a cryptographic operations pool?
-//       (it would only matter if there were a lot of small transactions in a block)
-pub fn calculate_transaction_roots(
-    coinbase_tx: &UnminedTx,
-    mempool_txs: &[VerifiedUnminedTx],
-) -> (merkle::Root, AuthDataRoot) {
-    let block_transactions =
-        || iter::once(coinbase_tx).chain(mempool_txs.iter().map(|tx| &tx.transaction));
-
-    let merkle_root = block_transactions().cloned().collect();
-    let auth_data_root = block_transactions().cloned().collect();
-
-    (merkle_root, auth_data_root)
-}
-
-// get_block_hash support methods
-
-/// Given a potentially negative index, find the corresponding `Height`.
-///
-/// This function is used to parse the integer index argument of `get_block_hash`.
-fn get_height_from_int(index: i32, tip_height: Height) -> Result<Height> {
-    if index >= 0 {
-        let height = index.try_into().expect("Positive i32 always fits in u32");
-        if height > tip_height.0 {
-            return Err(Error::invalid_params(
-                "Provided index is greater than the current tip",
-            ));
-        }
-        Ok(Height(height))
-    } else {
-        // `index + 1` can't overflow, because `index` is always negative here.
-        let height = i32::try_from(tip_height.0)
-            .expect("tip height fits in i32, because Height::MAX fits in i32")
-            .checked_add(index + 1);
-
-        let sanitized_height = match height {
-            None => return Err(Error::invalid_params("Provided index is not valid")),
-            Some(h) => {
-                if h < 0 {
-                    return Err(Error::invalid_params(
-                        "Provided negative index ends up with a negative height",
-                    ));
-                }
-                let h: u32 = h.try_into().expect("Positive i32 always fits in u32");
-                if h > tip_height.0 {
-                    return Err(Error::invalid_params(
-                        "Provided index is greater than the current tip",
-                    ));
-                }
-
-                h
-            }
-        };
-
-        Ok(Height(sanitized_height))
-    }
-}
+// Put support functions in a submodule, to keep this file small.

zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs

@@ -0,0 +1,305 @@
+//! Support functions for the `get_block_template()` RPC.
+
+use std::{iter, sync::Arc};
+
+use jsonrpc_core::{Error, ErrorCode, Result};
+use tower::{Service, ServiceExt};
+
+use zebra_chain::{
+    amount::{self, Amount, NegativeOrZero, NonNegative},
+    block::{
+        merkle::{self, AuthDataRoot},
+        ChainHistoryBlockTxAuthCommitmentHash, Height,
+    },
+    chain_sync_status::ChainSyncStatus,
+    chain_tip::ChainTip,
+    parameters::Network,
+    transaction::{Transaction, UnminedTx, VerifiedUnminedTx},
+    transparent,
+};
+use zebra_consensus::{funding_stream_address, funding_stream_values, miner_subsidy};
+use zebra_node_services::mempool;
+use zebra_state::GetBlockTemplateChainInfo;
+
+use crate::methods::get_block_template_rpcs::{
+    constants::{MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP, NOT_SYNCED_ERROR_CODE},
+    types::{default_roots::DefaultRoots, get_block_template, transaction::TransactionTemplate},
+};
+
+pub use crate::methods::get_block_template_rpcs::types::get_block_template::*;
+
+// - Parameter checks
+
+/// Returns an error if the get block template RPC `parameters` are invalid.
+pub fn check_block_template_parameters(
+    parameters: get_block_template::JsonParameters,
+) -> Result<()> {
+    if parameters.data.is_some() || parameters.mode == GetBlockTemplateRequestMode::Proposal {
+        return Err(Error {
+            code: ErrorCode::InvalidParams,
+            message: "\"proposal\" mode is currently unsupported by Zebra".to_string(),
+            data: None,
+        });
+    }
+
+    Ok(())
+}
+
+/// Returns the miner address, or an error if it is invalid.
+pub fn check_miner_address(
+    miner_address: Option<transparent::Address>,
+) -> Result<transparent::Address> {
+    miner_address.ok_or_else(|| Error {
+        code: ErrorCode::ServerError(0),
+        message: "configure mining.miner_address in zebrad.toml \
+                  with a transparent address"
+            .to_string(),
+        data: None,
+    })
+}
+
+// - State and syncer checks
+
+/// Returns an error if Zebra is not synced to the consensus chain tip.
+/// This error might be incorrect if the local clock is skewed.
+pub fn check_synced_to_tip<Tip, SyncStatus>(
+    network: Network,
+    latest_chain_tip: Tip,
+    sync_status: SyncStatus,
+) -> Result<()>
+where
+    Tip: ChainTip + Clone + Send + Sync + 'static,
+    SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
+{
+    // The tip estimate may not be the same as the one coming from the state
+    // but this is ok for an estimate
+    let (estimated_distance_to_chain_tip, local_tip_height) = latest_chain_tip
+        .estimate_distance_to_network_chain_tip(network)
+        .ok_or_else(|| Error {
+            code: ErrorCode::ServerError(0),
+            message: "No Chain tip available yet".to_string(),
+            data: None,
+        })?;
+
+    if !sync_status.is_close_to_tip()
+        || estimated_distance_to_chain_tip > MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP
+    {
+        tracing::info!(
+            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."
+        );
+
+        return Err(Error {
+            code: NOT_SYNCED_ERROR_CODE,
+            message: format!(
+                "Zebra has not synced to the chain tip, \
+                 estimated distance: {estimated_distance_to_chain_tip}, \
+                 local tip: {local_tip_height:?}. \
+                 Hint: check your network connection, clock, and time zone settings."
+            ),
+            data: None,
+        });
+    }
+
+    Ok(())
+}
+
+// - State and mempool data fetches
+
+/// Returns the state data for the block template.
+///
+/// You should call `check_synced_to_tip()` before calling this function.
+/// If the state does not have enough blocks, returns an error.
+pub async fn fetch_state_tip_and_local_time<State>(
+    state: State,
+) -> Result<GetBlockTemplateChainInfo>
+where
+    State: Service<
+            zebra_state::ReadRequest,
+            Response = zebra_state::ReadResponse,
+            Error = zebra_state::BoxError,
+        > + Clone
+        + Send
+        + Sync
+        + 'static,
+{
+    let request = zebra_state::ReadRequest::ChainInfo;
+    let response = state
+        .oneshot(request.clone())
+        .await
+        .map_err(|error| Error {
+            code: ErrorCode::ServerError(0),
+            message: error.to_string(),
+            data: None,
+        })?;
+
+    let chain_info = match response {
+        zebra_state::ReadResponse::ChainInfo(chain_info) => chain_info,
+        _ => unreachable!("incorrect response to {request:?}"),
+    };
+
+    Ok(chain_info)
+}
+
+/// Returns the transactions that are currently in `mempool`.
+///
+/// You should call `check_synced_to_tip()` before calling this function.
+/// If the mempool is inactive because Zebra is not synced to the tip, returns no transactions.
+pub async fn fetch_mempool_transactions<Mempool>(mempool: Mempool) -> Result<Vec<VerifiedUnminedTx>>
+where
+    Mempool: Service<
+            mempool::Request,
+            Response = mempool::Response,
+            Error = zebra_node_services::BoxError,
+        > + 'static,
+    Mempool::Future: Send,
+{
+    let response = mempool
+        .oneshot(mempool::Request::FullTransactions)
+        .await
+        .map_err(|error| Error {
+            code: ErrorCode::ServerError(0),
+            message: error.to_string(),
+            data: None,
+        })?;
+
+    if let mempool::Response::FullTransactions(transactions) = response {
+        Ok(transactions)
+    } else {
+        unreachable!("unmatched response to a mempool::FullTransactions request")
+    }
+}
+
+// - Response processing
+
+/// Generates and returns the coinbase transaction and default roots.
+pub fn generate_coinbase_and_roots(
+    network: Network,
+    height: Height,
+    miner_address: transparent::Address,
+    mempool_txs: &[VerifiedUnminedTx],
+    history_tree: Arc<zebra_chain::history_tree::HistoryTree>,
+) -> (TransactionTemplate<NegativeOrZero>, DefaultRoots) {
+    // Generate the coinbase transaction
+    let miner_fee = calculate_miner_fee(mempool_txs);
+    let coinbase_txn = generate_coinbase_transaction(network, height, miner_address, miner_fee);
+
+    // Calculate block default roots
+    //
+    // TODO: move expensive root, hash, and tree cryptography to a rayon thread?
+    let default_roots = calculate_default_root_hashes(&coinbase_txn, mempool_txs, history_tree);
+
+    let coinbase_txn = TransactionTemplate::from_coinbase(&coinbase_txn, miner_fee);
+
+    (coinbase_txn, default_roots)
+}
+
+// - Coinbase transaction processing
+
+/// Returns a coinbase transaction for the supplied parameters.
+pub fn generate_coinbase_transaction(
+    network: Network,
+    height: Height,
+    miner_address: transparent::Address,
+    miner_fee: Amount<NonNegative>,
+) -> UnminedTx {
+    let outputs = standard_coinbase_outputs(network, height, miner_address, miner_fee);
+
+    Transaction::new_v5_coinbase(network, height, outputs).into()
+}
+
+/// Returns the total miner fee for `mempool_txs`.
+pub fn calculate_miner_fee(mempool_txs: &[VerifiedUnminedTx]) -> Amount<NonNegative> {
+    let miner_fee: amount::Result<Amount<NonNegative>> =
+        mempool_txs.iter().map(|tx| tx.miner_fee).sum();
+
+    miner_fee.expect(
+        "invalid selected transactions: \
+         fees in a valid block can not be more than MAX_MONEY",
+    )
+}
+
+/// Returns the standard funding stream and miner reward transparent output scripts
+/// for `network`, `height` and `miner_fee`.
+///
+/// Only works for post-Canopy heights.
+pub fn standard_coinbase_outputs(
+    network: Network,
+    height: Height,
+    miner_address: transparent::Address,
+    miner_fee: Amount<NonNegative>,
+) -> Vec<(Amount<NonNegative>, transparent::Script)> {
+    let funding_streams = funding_stream_values(height, network)
+        .expect("funding stream value calculations are valid for reasonable chain heights");
+
+    let mut funding_streams: Vec<(Amount<NonNegative>, transparent::Address)> = funding_streams
+        .iter()
+        .map(|(receiver, amount)| (*amount, funding_stream_address(height, network, *receiver)))
+        .collect();
+    // The HashMap returns funding streams in an arbitrary order,
+    // but Zebra's snapshot tests expect the same order every time.
+    funding_streams.sort_by_key(|(amount, _address)| *amount);
+
+    let miner_reward = miner_subsidy(height, network)
+        .expect("reward calculations are valid for reasonable chain heights")
+        + miner_fee;
+    let miner_reward =
+        miner_reward.expect("reward calculations are valid for reasonable chain heights");
+
+    let mut coinbase_outputs = funding_streams;
+    coinbase_outputs.push((miner_reward, miner_address));
+
+    coinbase_outputs
+        .iter()
+        .map(|(amount, address)| (*amount, address.create_script_from_address()))
+        .collect()
+}
+
+// - Transaction roots processing
+
+/// Returns the default block roots for the supplied coinbase and mempool transactions,
+/// and the supplied history tree.
+///
+/// This function runs expensive cryptographic operations.
+pub fn calculate_default_root_hashes(
+    coinbase_txn: &UnminedTx,
+    mempool_txs: &[VerifiedUnminedTx],
+    history_tree: Arc<zebra_chain::history_tree::HistoryTree>,
+) -> DefaultRoots {
+    let (merkle_root, auth_data_root) = calculate_transaction_roots(coinbase_txn, mempool_txs);
+
+    let history_tree = history_tree;
+    let chain_history_root = history_tree.hash().expect("history tree can't be empty");
+
+    let block_commitments_hash = ChainHistoryBlockTxAuthCommitmentHash::from_commitments(
+        &chain_history_root,
+        &auth_data_root,
+    );
+
+    DefaultRoots {
+        merkle_root,
+        chain_history_root,
+        auth_data_root,
+        block_commitments_hash,
+    }
+}
+
+/// Returns the transaction effecting and authorizing roots
+/// for `coinbase_txn` and `mempool_txs`, which are used in the block header.
+//
+// TODO: should this be spawned into a cryptographic operations pool?
+//       (it would only matter if there were a lot of small transactions in a block)
+pub fn calculate_transaction_roots(
+    coinbase_txn: &UnminedTx,
+    mempool_txs: &[VerifiedUnminedTx],
+) -> (merkle::Root, AuthDataRoot) {
+    let block_transactions =
+        || iter::once(coinbase_txn).chain(mempool_txs.iter().map(|tx| &tx.transaction));
+
+    let merkle_root = block_transactions().cloned().collect();
+    let auth_data_root = block_transactions().cloned().collect();
+
+    (merkle_root, auth_data_root)
+}

zebra-rpc/src/methods/get_block_template_rpcs/types.rs

@@ -2,7 +2,6 @@
 
 pub mod default_roots;
 pub mod get_block_template;
-pub mod get_block_template_opts;
 pub mod get_mining_info;
 pub mod hex_data;
 pub mod long_poll;

zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs

@@ -1,15 +1,33 @@
 //! The `GetBlockTempate` type is the output of the `getblocktemplate` RPC method.
 
-use zebra_chain::{amount, block::ChainHistoryBlockTxAuthCommitmentHash};
+use zebra_chain::{
+    amount,
+    block::{ChainHistoryBlockTxAuthCommitmentHash, Height, MAX_BLOCK_BYTES, ZCASH_BLOCK_VERSION},
+    serialization::DateTime32,
+    transaction::VerifiedUnminedTx,
+    work::difficulty::{CompactDifficulty, ExpandedDifficulty},
+};
+use zebra_consensus::MAX_BLOCK_SIGOPS;
+use zebra_state::GetBlockTemplateChainInfo;
 
 use crate::methods::{
-    get_block_template_rpcs::types::{
-        default_roots::DefaultRoots, long_poll::LongPollId, transaction::TransactionTemplate,
+    get_block_template_rpcs::{
+        constants::{
+            GET_BLOCK_TEMPLATE_CAPABILITIES_FIELD, GET_BLOCK_TEMPLATE_MUTABLE_FIELD,
+            GET_BLOCK_TEMPLATE_NONCE_RANGE_FIELD,
+        },
+        types::{
+            default_roots::DefaultRoots, long_poll::LongPollId, transaction::TransactionTemplate,
+        },
     },
     GetBlockHash,
 };
 
-/// Documentation to be added after we document all the individual fields.
+pub mod parameters;
+
+pub use parameters::*;
+
+/// A serialized `getblocktemplate` RPC response.
 #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
 pub struct GetBlockTemplate {
     /// The getblocktemplate RPC capabilities supported by Zebra.
@@ -59,8 +77,6 @@ pub struct GetBlockTemplate {
     pub default_roots: DefaultRoots,
 
     /// The non-coinbase transactions selected for this block template.
-    ///
-    /// TODO: select these transactions using ZIP-317 (#5473)
     pub transactions: Vec<TransactionTemplate<amount::NonNegative>>,
 
     /// The coinbase transaction generated from `transactions` and `height`.
@@ -72,16 +88,15 @@ pub struct GetBlockTemplate {
     pub long_poll_id: LongPollId,
 
     /// The expected difficulty for the new block displayed in expanded form.
-    // TODO: use ExpandedDifficulty type.
-    pub target: String,
+    #[serde(with = "hex")]
+    pub target: ExpandedDifficulty,
 
     /// > 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>
     #[serde(rename = "mintime")]
-    // TODO: use DateTime32 type?
-    pub min_time: i64,
+    pub min_time: DateTime32,
 
     /// Hardcoded list of block fields the miner is allowed to change.
     pub mutable: Vec<String>,
@@ -102,16 +117,15 @@ pub struct GetBlockTemplate {
     /// > note this is not necessarily the system clock, and must fall within the mintime/maxtime rules
     ///
     /// <https://en.bitcoin.it/wiki/BIP_0022#Block_Template_Request>
-    // TODO: use DateTime32 type?
     #[serde(rename = "curtime")]
-    pub cur_time: i64,
+    pub cur_time: DateTime32,
 
     /// The expected difficulty for the new block displayed in compact form.
-    // TODO: use CompactDifficulty type.
-    pub bits: String,
+    #[serde(with = "hex")]
+    pub bits: CompactDifficulty,
 
     /// The height of the next block in the best chain.
-    // TODO: use Height type?
+    // Optional TODO: use Height type, but check that deserialized heights are within Height::MAX
     pub height: u32,
 
     /// > the maximum time allowed
@@ -128,6 +142,76 @@ pub struct GetBlockTemplate {
     /// Some miners don't check the maximum time. This can cause invalid blocks after network downtime,
     /// a significant drop in the hash rate, or after the testnet minimum difficulty interval.
     #[serde(rename = "maxtime")]
-    // TODO: use DateTime32 type?
-    pub max_time: i64,
+    pub max_time: DateTime32,
+}
+
+impl GetBlockTemplate {
+    /// Returns a new [`GetBlockTemplate`] struct, based on the supplied arguments and defaults.
+    ///
+    /// The result of this method only depends on the supplied arguments and constants.
+    pub fn new(
+        next_block_height: Height,
+        chain_tip_and_local_time: &GetBlockTemplateChainInfo,
+        long_poll_id: LongPollId,
+        coinbase_txn: TransactionTemplate<amount::NegativeOrZero>,
+        mempool_txs: &[VerifiedUnminedTx],
+        default_roots: DefaultRoots,
+    ) -> Self {
+        // Convert transactions into TransactionTemplates
+        let mempool_txs = mempool_txs.iter().map(Into::into).collect();
+
+        // Convert difficulty
+        let target = chain_tip_and_local_time
+            .expected_difficulty
+            .to_expanded()
+            .expect("state always returns a valid difficulty value");
+
+        // Convert default values
+        let capabilities: Vec<String> = GET_BLOCK_TEMPLATE_CAPABILITIES_FIELD
+            .iter()
+            .map(ToString::to_string)
+            .collect();
+        let mutable: Vec<String> = GET_BLOCK_TEMPLATE_MUTABLE_FIELD
+            .iter()
+            .map(ToString::to_string)
+            .collect();
+
+        GetBlockTemplate {
+            capabilities,
+
+            version: ZCASH_BLOCK_VERSION,
+
+            previous_block_hash: GetBlockHash(chain_tip_and_local_time.tip_hash),
+            block_commitments_hash: default_roots.block_commitments_hash,
+            light_client_root_hash: default_roots.block_commitments_hash,
+            final_sapling_root_hash: default_roots.block_commitments_hash,
+            default_roots,
+
+            transactions: mempool_txs,
+
+            coinbase_txn,
+
+            long_poll_id,
+
+            target,
+
+            min_time: chain_tip_and_local_time.min_time,
+
+            mutable,
+
+            nonce_range: GET_BLOCK_TEMPLATE_NONCE_RANGE_FIELD.to_string(),
+
+            sigop_limit: MAX_BLOCK_SIGOPS,
+
+            size_limit: MAX_BLOCK_BYTES,
+
+            cur_time: chain_tip_and_local_time.cur_time,
+
+            bits: chain_tip_and_local_time.expected_difficulty,
+
+            height: next_block_height.0,
+
+            max_time: chain_tip_and_local_time.max_time,
+        }
+    }
 }

zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template/parameters.rs

@@ -1,6 +1,6 @@
 //! Parameter types for the `getblocktemplate` RPC.
 
-use super::{hex_data::HexData, long_poll::LongPollId};
+use crate::methods::get_block_template_rpcs::types::{hex_data::HexData, long_poll::LongPollId};
 
 /// Defines whether the RPC method should generate a block template or attempt to validate a block proposal.
 /// `Proposal` mode is currently unsupported and will return an error.
@@ -52,11 +52,12 @@ pub enum GetBlockTemplateCapability {
 
     /// Unknown capability to fill in for mutations.
     // TODO: Fill out valid mutations capabilities.
+    //       The set of possible capabilities is open-ended, so we need to keep UnknownCapability.
     #[serde(other)]
     UnknownCapability,
 }
 
-/// Optional argument `jsonrequestobject` for `getblocktemplate` RPC request.
+/// Optional parameter `jsonrequestobject` for `getblocktemplate` RPC request.
 ///
 /// The `data` field must be provided in `proposal` mode, and must be omitted in `template` mode.
 /// All other fields are optional.
@@ -78,7 +79,6 @@ pub struct JsonParameters {
     pub data: Option<HexData>,
 
     /// A list of client-side supported capability features
-    // TODO: Fill out valid mutations capabilities.
     #[serde(default)]
     pub capabilities: Vec<GetBlockTemplateCapability>,
 

zebra-rpc/src/methods/get_block_template_rpcs/types/long_poll.rs

@@ -5,11 +5,11 @@
 
 use std::{str::FromStr, sync::Arc};
 
-use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
 
 use zebra_chain::{
     block::{self, Height},
+    serialization::DateTime32,
     transaction::{self, UnminedTxId},
 };
 use zebra_node_services::BoxError;
@@ -48,7 +48,7 @@ pub struct LongPollInput {
     ///
     /// Ideally, a new template should be provided at least one target block interval before
     /// the max time. This avoids wasted work.
-    pub max_time: DateTime<Utc>,
+    pub max_time: DateTime32,
 
     // Fields that allow old work:
     //
@@ -68,7 +68,7 @@ impl LongPollInput {
     pub fn new(
         tip_height: Height,
         tip_hash: block::Hash,
-        max_time: DateTime<Utc>,
+        max_time: DateTime32,
         mempool_tx_ids: impl IntoIterator<Item = UnminedTxId>,
     ) -> Self {
         let mempool_transaction_mined_ids =
@@ -177,10 +177,10 @@ impl From<LongPollInput> for LongPollId {
 
             tip_hash_checksum,
 
+            max_timestamp: input.max_time.timestamp(),
+
             // It's ok to do wrapping conversions here,
             // because long polling checks are probabilistic.
-            max_timestamp: input.max_time.timestamp() as u32,
-
             mempool_transaction_count: input.mempool_transaction_mined_ids.len() as u32,
 
             mempool_transaction_content_checksum,

zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs

@@ -6,18 +6,23 @@
 //! > when computing `size_target`, since there is no consensus requirement for this to be
 //! > exactly the same between implementations.
 
-use jsonrpc_core::{Error, ErrorCode, Result};
 use rand::{
     distributions::{Distribution, WeightedIndex},
     prelude::thread_rng,
 };
-use tower::{Service, ServiceExt};
 
-use zebra_chain::{amount::NegativeOrZero, block::MAX_BLOCK_BYTES, transaction::VerifiedUnminedTx};
+use zebra_chain::{
+    amount::NegativeOrZero,
+    block::{Height, MAX_BLOCK_BYTES},
+    parameters::Network,
+    transaction::{Transaction, VerifiedUnminedTx},
+    transparent,
+};
 use zebra_consensus::MAX_BLOCK_SIGOPS;
-use zebra_node_services::mempool;
 
-use super::types::transaction::TransactionTemplate;
+use crate::methods::get_block_template_rpcs::{
+    get_block_template::standard_coinbase_outputs, types::transaction::TransactionTemplate,
+};
 
 /// The ZIP-317 recommended limit on the number of unpaid actions per block.
 /// `block_unpaid_action_limit` in ZIP-317.
@@ -30,24 +35,20 @@ pub const BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT: u32 = 50;
 /// as the real coinbase transaction. (The real coinbase transaction depends on the total
 /// fees from the transactions returned by this function.)
 ///
-/// Returns selected transactions from the `mempool`, or an error if the mempool has failed.
+/// Returns selected transactions from `mempool_txs`.
 ///
 /// [ZIP-317]: https://zips.z.cash/zip-0317#block-production
-pub async fn select_mempool_transactions<Mempool>(
-    fake_coinbase_tx: TransactionTemplate<NegativeOrZero>,
-    mempool: Mempool,
-) -> Result<Vec<VerifiedUnminedTx>>
-where
-    Mempool: Service<
-            mempool::Request,
-            Response = mempool::Response,
-            Error = zebra_node_services::BoxError,
-        > + 'static,
-    Mempool::Future: Send,
-{
-    // Setup the transaction lists.
-    let mempool_txs = fetch_mempool_transactions(mempool).await?;
+pub async fn select_mempool_transactions(
+    network: Network,
+    next_block_height: Height,
+    miner_address: transparent::Address,
+    mempool_txs: Vec<VerifiedUnminedTx>,
+) -> Vec<VerifiedUnminedTx> {
+    // Use a fake coinbase transaction to break the dependency between transaction
+    // selection, the miner fee, and the fee payment in the coinbase transaction.
+    let fake_coinbase_tx = fake_coinbase_transaction(network, next_block_height, miner_address);
 
+    // Setup the transaction lists.
     let (conventional_fee_txs, low_fee_txs): (Vec<_>, Vec<_>) = mempool_txs
         .into_iter()
         .partition(VerifiedUnminedTx::pays_conventional_fee);
@@ -94,33 +95,36 @@ where
         );
     }
 
-    Ok(selected_txs)
+    selected_txs
 }
 
-/// Fetch the transactions that are currently in `mempool`.
-async fn fetch_mempool_transactions<Mempool>(mempool: Mempool) -> Result<Vec<VerifiedUnminedTx>>
-where
-    Mempool: Service<
-            mempool::Request,
-            Response = mempool::Response,
-            Error = zebra_node_services::BoxError,
-        > + 'static,
-    Mempool::Future: Send,
-{
-    let response = mempool
-        .oneshot(mempool::Request::FullTransactions)
-        .await
-        .map_err(|error| Error {
-            code: ErrorCode::ServerError(0),
-            message: error.to_string(),
-            data: None,
-        })?;
+/// Returns a fake coinbase transaction that can be used during transaction selection.
+///
+/// This avoids a data dependency loop involving the selected transactions, the miner fee,
+/// and the coinbase transaction.
+///
+/// This transaction's serialized size and sigops must be at least as large as the real coinbase
+/// transaction with the correct height and fee.
+pub fn fake_coinbase_transaction(
+    network: Network,
+    block_height: Height,
+    miner_address: transparent::Address,
+) -> TransactionTemplate<NegativeOrZero> {
+    // Block heights are encoded as variable-length (script) and `u32` (lock time, expiry height).
+    // They can also change the `u32` consensus branch id.
+    // We use the template height here, which has the correct byte length.
+    // https://zips.z.cash/protocol/protocol.pdf#txnconsensus
+    // https://github.com/zcash/zips/blob/main/zip-0203.rst#changes-for-nu5
+    //
+    // Transparent amounts are encoded as `i64`,
+    // so one zat has the same size as the real amount:
+    // https://developer.bitcoin.org/reference/transactions.html#txout-a-transaction-output
+    let miner_fee = 1.try_into().expect("amount is valid and non-negative");
 
-    if let mempool::Response::FullTransactions(transactions) = response {
-        Ok(transactions)
-    } else {
-        unreachable!("unmatched response to a mempool::FullTransactions request")
-    }
+    let outputs = standard_coinbase_outputs(network, block_height, miner_address, miner_fee);
+    let coinbase_tx = Transaction::new_v5_coinbase(network, block_height, outputs).into();
+
+    TransactionTemplate::from_coinbase(&coinbase_tx, miner_fee)
 }
 
 /// Returns a fee-weighted index and the total weight of `transactions`.

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

@@ -5,7 +5,6 @@
 //! cargo insta test --review --features getblocktemplate-rpcs --delete-unreferenced-snapshots
 //! ```
 
-use chrono::{TimeZone, Utc};
 use hex::FromHex;
 use insta::Settings;
 use tower::{buffer::Buffer, Service};
@@ -15,7 +14,7 @@ use zebra_chain::{
     chain_sync_status::MockSyncStatus,
     chain_tip::mock::MockChainTip,
     parameters::{Network, NetworkUpgrade},
-    serialization::ZcashDeserializeInto,
+    serialization::{DateTime32, ZcashDeserializeInto},
     transaction::Transaction,
     transparent,
     work::difficulty::{CompactDifficulty, ExpandedDifficulty, U256},
@@ -95,11 +94,11 @@ pub async fn test_responses<State, ReadState>(
         Hash::from_hex("0000000000d723156d9b65ffcf4984da7a19675ed7e2f06d9e5d5188af087bf8").unwrap();
 
     //  nu5 block time + 1
-    let fake_min_time = Utc.timestamp_opt(1654008606, 0).unwrap();
+    let fake_min_time = DateTime32::from(1654008606);
     // nu5 block time + 12
-    let fake_cur_time = Utc.timestamp_opt(1654008617, 0).unwrap();
+    let fake_cur_time = DateTime32::from(1654008617);
     // nu5 block time + 123
-    let fake_max_time = Utc.timestamp_opt(1654008728, 0).unwrap();
+    let fake_max_time = DateTime32::from(1654008728);
 
     let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new();
     mock_chain_tip_sender.send_best_tip_height(fake_tip_height);

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

@@ -21,9 +21,6 @@ use zebra_test::mock_service::MockService;
 
 use super::super::*;
 
-#[cfg(feature = "getblocktemplate-rpcs")]
-use zebra_chain::chain_sync_status::MockSyncStatus;
-
 #[tokio::test(flavor = "multi_thread")]
 async fn rpc_getinfo() {
     let _init_guard = zebra_test::init();
@@ -618,6 +615,8 @@ async fn rpc_getaddressutxos_response() {
 #[tokio::test(flavor = "multi_thread")]
 #[cfg(feature = "getblocktemplate-rpcs")]
 async fn rpc_getblockcount() {
+    use zebra_chain::chain_sync_status::MockSyncStatus;
+
     let _init_guard = zebra_test::init();
 
     // Create a continuous chain of mainnet blocks from genesis
@@ -651,7 +650,7 @@ async fn rpc_getblockcount() {
     .await;
 
     // Init RPC
-    let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+    let get_block_template_rpc = GetBlockTemplateRpcImpl::new(
         Mainnet,
         Default::default(),
         Buffer::new(mempool.clone(), 1),
@@ -675,6 +674,8 @@ async fn rpc_getblockcount() {
 #[cfg(feature = "getblocktemplate-rpcs")]
 #[tokio::test(flavor = "multi_thread")]
 async fn rpc_getblockcount_empty_state() {
+    use zebra_chain::chain_sync_status::MockSyncStatus;
+
     let _init_guard = zebra_test::init();
 
     // Get a mempool handle
@@ -722,6 +723,8 @@ async fn rpc_getblockcount_empty_state() {
 #[cfg(feature = "getblocktemplate-rpcs")]
 #[tokio::test(flavor = "multi_thread")]
 async fn rpc_getblockhash() {
+    use zebra_chain::chain_sync_status::MockSyncStatus;
+
     let _init_guard = zebra_test::init();
 
     // Create a continuous chain of mainnet blocks from genesis
@@ -788,6 +791,8 @@ async fn rpc_getblockhash() {
 #[cfg(feature = "getblocktemplate-rpcs")]
 #[tokio::test(flavor = "multi_thread")]
 async fn rpc_getmininginfo() {
+    use zebra_chain::chain_sync_status::MockSyncStatus;
+
     let _init_guard = zebra_test::init();
 
     // Create a continuous chain of mainnet blocks from genesis
@@ -820,6 +825,8 @@ async fn rpc_getmininginfo() {
 #[cfg(feature = "getblocktemplate-rpcs")]
 #[tokio::test(flavor = "multi_thread")]
 async fn rpc_getnetworksolps() {
+    use zebra_chain::chain_sync_status::MockSyncStatus;
+
     let _init_guard = zebra_test::init();
 
     // Create a continuous chain of mainnet blocks from genesis
@@ -887,12 +894,12 @@ async fn rpc_getblocktemplate() {
 async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
     use std::panic;
 
-    use chrono::TimeZone;
-
     use zebra_chain::{
         amount::NonNegative,
         block::{Hash, MAX_BLOCK_BYTES, ZCASH_BLOCK_VERSION},
+        chain_sync_status::MockSyncStatus,
         chain_tip::mock::MockChainTip,
+        serialization::DateTime32,
         work::difficulty::{CompactDifficulty, ExpandedDifficulty, U256},
     };
     use zebra_consensus::MAX_BLOCK_SIGOPS;
@@ -900,11 +907,13 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
 
     use crate::methods::{
         get_block_template_rpcs::{
+            config::Config,
             constants::{
                 GET_BLOCK_TEMPLATE_CAPABILITIES_FIELD, GET_BLOCK_TEMPLATE_MUTABLE_FIELD,
                 GET_BLOCK_TEMPLATE_NONCE_RANGE_FIELD,
             },
-            types::long_poll::LONG_POLL_ID_LENGTH,
+            get_block_template::{self, GetBlockTemplateRequestMode},
+            types::{hex_data::HexData, long_poll::LONG_POLL_ID_LENGTH},
         },
         tests::utils::fake_history_tree,
     };
@@ -924,7 +933,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
         true => Some(transparent::Address::from_pub_key_hash(Mainnet, [0x7e; 20])),
     };
 
-    let mining_config = get_block_template_rpcs::config::Config { miner_address };
+    let mining_config = Config { miner_address };
 
     // nu5 block height
     let fake_tip_height = NetworkUpgrade::Nu5.activation_height(Mainnet).unwrap();
@@ -932,11 +941,11 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
     let fake_tip_hash =
         Hash::from_hex("0000000000d723156d9b65ffcf4984da7a19675ed7e2f06d9e5d5188af087bf8").unwrap();
     //  nu5 block time + 1
-    let fake_min_time = Utc.timestamp_opt(1654008606, 0).unwrap();
+    let fake_min_time = DateTime32::from(1654008606);
     // nu5 block time + 12
-    let fake_cur_time = Utc.timestamp_opt(1654008617, 0).unwrap();
+    let fake_cur_time = DateTime32::from(1654008617);
     // nu5 block time + 123
-    let fake_max_time = Utc.timestamp_opt(1654008728, 0).unwrap();
+    let fake_max_time = DateTime32::from(1654008728);
 
     let (mock_chain_tip, mock_chain_tip_sender) = MockChainTip::new();
     mock_chain_tip_sender.send_best_tip_height(fake_tip_height);
@@ -944,7 +953,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
     mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0));
 
     // Init RPC
-    let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+    let get_block_template_rpc = GetBlockTemplateRpcImpl::new(
         Mainnet,
         mining_config,
         Buffer::new(mempool.clone(), 1),
@@ -996,9 +1005,12 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
     assert!(get_block_template.transactions.is_empty());
     assert_eq!(
         get_block_template.target,
-        "0000000000000000000000000000000000000000000000000000000000000001"
+        ExpandedDifficulty::from_hex(
+            "0000000000000000000000000000000000000000000000000000000000000001"
+        )
+        .expect("test vector is valid")
     );
-    assert_eq!(get_block_template.min_time, fake_min_time.timestamp());
+    assert_eq!(get_block_template.min_time, fake_min_time);
     assert_eq!(
         get_block_template.mutable,
         GET_BLOCK_TEMPLATE_MUTABLE_FIELD.to_vec()
@@ -1009,10 +1021,13 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
     );
     assert_eq!(get_block_template.sigop_limit, MAX_BLOCK_SIGOPS);
     assert_eq!(get_block_template.size_limit, MAX_BLOCK_BYTES);
-    assert_eq!(get_block_template.cur_time, fake_cur_time.timestamp());
-    assert_eq!(get_block_template.bits, "01010000");
+    assert_eq!(get_block_template.cur_time, fake_cur_time);
+    assert_eq!(
+        get_block_template.bits,
+        CompactDifficulty::from_hex("01010000").expect("test vector is valid")
+    );
     assert_eq!(get_block_template.height, 1687105); // nu5 height
-    assert_eq!(get_block_template.max_time, fake_max_time.timestamp());
+    assert_eq!(get_block_template.max_time, fake_max_time);
 
     // Coinbase transaction checks.
     assert!(get_block_template.coinbase_txn.required);
@@ -1069,8 +1084,8 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
     );
 
     let get_block_template_sync_error = get_block_template_rpc
-        .get_block_template(Some(get_block_template_rpcs::types::get_block_template_opts::JsonParameters {
-            mode: get_block_template_rpcs::types::get_block_template_opts::GetBlockTemplateRequestMode::Proposal,
+        .get_block_template(Some(get_block_template::JsonParameters {
+            mode: GetBlockTemplateRequestMode::Proposal,
             ..Default::default()
         }))
         .await
@@ -1079,12 +1094,10 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
     assert_eq!(get_block_template_sync_error.code, ErrorCode::InvalidParams);
 
     let get_block_template_sync_error = get_block_template_rpc
-        .get_block_template(Some(
-            get_block_template_rpcs::types::get_block_template_opts::JsonParameters {
-                data: Some(get_block_template_rpcs::types::hex_data::HexData("".into())),
-                ..Default::default()
-            },
-        ))
+        .get_block_template(Some(get_block_template::JsonParameters {
+            data: Some(HexData("".into())),
+            ..Default::default()
+        }))
         .await
         .expect_err("needs an error when passing in block data");
 
@@ -1092,18 +1105,16 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
 
     // The long poll id is valid, so it returns a state error instead
     let get_block_template_sync_error = get_block_template_rpc
-        .get_block_template(Some(
-            get_block_template_rpcs::types::get_block_template_opts::JsonParameters {
-                // This must parse as a LongPollId.
-                // It must be the correct length and have hex/decimal digits.
-                longpollid: Some(
-                    "0".repeat(LONG_POLL_ID_LENGTH)
-                        .parse()
-                        .expect("unexpected invalid LongPollId"),
-                ),
-                ..Default::default()
-            },
-        ))
+        .get_block_template(Some(get_block_template::JsonParameters {
+            // This must parse as a LongPollId.
+            // It must be the correct length and have hex/decimal digits.
+            longpollid: Some(
+                "0".repeat(LONG_POLL_ID_LENGTH)
+                    .parse()
+                    .expect("unexpected invalid LongPollId"),
+            ),
+            ..Default::default()
+        }))
         .await
         .expect_err("needs an error when the state is empty");
 
@@ -1116,6 +1127,10 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
 #[cfg(feature = "getblocktemplate-rpcs")]
 #[tokio::test(flavor = "multi_thread")]
 async fn rpc_submitblock_errors() {
+    use zebra_chain::chain_sync_status::MockSyncStatus;
+
+    use crate::methods::get_block_template_rpcs::types::{hex_data::HexData, submit_block};
+
     let _init_guard = zebra_test::init();
 
     // Create a continuous chain of mainnet blocks from genesis
@@ -1144,7 +1159,7 @@ async fn rpc_submitblock_errors() {
     .await;
 
     // Init RPC
-    let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+    let get_block_template_rpc = GetBlockTemplateRpcImpl::new(
         Mainnet,
         Default::default(),
         Buffer::new(mempool.clone(), 1),
@@ -1157,30 +1172,25 @@ async fn rpc_submitblock_errors() {
     // Try to submit pre-populated blocks and assert that it responds with duplicate.
     for (_height, &block_bytes) in zebra_test::vectors::CONTINUOUS_MAINNET_BLOCKS.iter() {
         let submit_block_response = get_block_template_rpc
-            .submit_block(
-                get_block_template_rpcs::types::hex_data::HexData(block_bytes.into()),
-                None,
-            )
+            .submit_block(HexData(block_bytes.into()), None)
             .await;
 
         assert_eq!(
             submit_block_response,
-            Ok(get_block_template_rpcs::types::submit_block::ErrorResponse::Duplicate.into())
+            Ok(submit_block::ErrorResponse::Duplicate.into())
         );
     }
 
     let submit_block_response = get_block_template_rpc
         .submit_block(
-            get_block_template_rpcs::types::hex_data::HexData(
-                zebra_test::vectors::BAD_BLOCK_MAINNET_202_BYTES.to_vec(),
-            ),
+            HexData(zebra_test::vectors::BAD_BLOCK_MAINNET_202_BYTES.to_vec()),
             None,
         )
         .await;
 
     assert_eq!(
         submit_block_response,
-        Ok(get_block_template_rpcs::types::submit_block::ErrorResponse::Rejected.into())
+        Ok(submit_block::ErrorResponse::Rejected.into())
     );
 
     mempool.expect_no_requests().await;

zebra-state/src/response.rs

@@ -11,7 +11,7 @@ use zebra_chain::{
 };
 
 #[cfg(feature = "getblocktemplate-rpcs")]
-use zebra_chain::work::difficulty::CompactDifficulty;
+use zebra_chain::{serialization::DateTime32, work::difficulty::CompactDifficulty};
 
 // Allow *only* these unused imports, so that rustdoc link resolution
 // will work with inline links.
@@ -139,32 +139,44 @@ pub enum ReadResponse {
     SolutionRate(Option<u128>),
 }
 
-#[cfg(feature = "getblocktemplate-rpcs")]
 /// A structure with the information needed from the state to build a `getblocktemplate` RPC response.
+#[cfg(feature = "getblocktemplate-rpcs")]
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct GetBlockTemplateChainInfo {
-    /// The current state tip height.
-    /// The block template OAfor the candidate block is the next block after this block.
-    pub tip_height: block::Height,
-
+    // Data fetched directly from the state tip.
+    //
     /// The current state tip height.
     /// The block template for the candidate block has this hash as the previous block hash.
     pub tip_hash: block::Hash,
 
-    /// The expected difficulty of the candidate block.
-    pub expected_difficulty: CompactDifficulty,
-
-    /// The current system time, adjusted to fit within `min_time` and `max_time`.
-    pub cur_time: chrono::DateTime<chrono::Utc>,
-
-    /// The mininimum time the miner can use in this block.
-    pub min_time: chrono::DateTime<chrono::Utc>,
-
-    /// The maximum time the miner can use in this block.
-    pub max_time: chrono::DateTime<chrono::Utc>,
+    /// The current state tip height.
+    /// The block template OAfor the candidate block is the next block after this block.
+    /// Depends on the `tip_hash`.
+    pub tip_height: block::Height,
 
     /// The history tree of the current best chain.
+    /// Depends on the `tip_hash`.
     pub history_tree: Arc<zebra_chain::history_tree::HistoryTree>,
+
+    // Data derived from the state tip and recent blocks.
+    //
+    /// The expected difficulty of the candidate block.
+    /// Depends on the `tip_hash`.
+    pub expected_difficulty: CompactDifficulty,
+
+    // Data derived from the state tip and recent blocks, and the current local clock.
+    //
+    /// The current system time, adjusted to fit within `min_time` and `max_time`.
+    /// Depends on the local clock and the `tip_hash`.
+    pub cur_time: DateTime32,
+
+    /// The mininimum time the miner can use in this block.
+    /// Depends on the `tip_hash`, and the local clock on testnet.
+    pub min_time: DateTime32,
+
+    /// The maximum time the miner can use in this block.
+    /// Depends on the `tip_hash`, and the local clock on testnet.
+    pub max_time: DateTime32,
 }
 
 /// Conversion from read-only [`ReadResponse`]s to read-write [`Response`]s.

zebra-state/src/service/check.rs

@@ -243,7 +243,7 @@ fn difficulty_threshold_is_valid(
     let network = difficulty_adjustment.network();
     let median_time_past = difficulty_adjustment.median_time_past();
     let block_time_max =
-        median_time_past + Duration::seconds(difficulty::BLOCK_MAX_TIME_SINCE_MEDIAN);
+        median_time_past + Duration::seconds(difficulty::BLOCK_MAX_TIME_SINCE_MEDIAN.into());
 
     // # Consensus
     //

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

@@ -48,7 +48,7 @@ pub const POW_MAX_ADJUST_DOWN_PERCENT: i32 = 32;
 /// and the block's `time` field.
 ///
 /// Part of the block header consensus rules in the Zcash specification.
-pub const BLOCK_MAX_TIME_SINCE_MEDIAN: i64 = 90 * 60;
+pub const BLOCK_MAX_TIME_SINCE_MEDIAN: u32 = 90 * 60;
 
 /// Contains the context needed to calculate the adjusted difficulty for a block.
 pub(crate) struct AdjustedDifficulty {

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

@@ -2,12 +2,13 @@
 
 use std::sync::Arc;
 
-use chrono::{DateTime, Duration, TimeZone, Utc};
+use chrono::{DateTime, Utc};
 
 use zebra_chain::{
     block::{self, Block, Hash, Height},
     history_tree::HistoryTree,
     parameters::{Network, NetworkUpgrade, POST_BLOSSOM_POW_TARGET_SPACING},
+    serialization::{DateTime32, Duration32},
     work::difficulty::{CompactDifficulty, PartialCumulativeWork},
 };
 
@@ -185,14 +186,14 @@ fn difficulty_time_and_history_tree(
         .map(|block| (block.header.difficulty_threshold, block.header.time))
         .collect();
 
-    let cur_time = chrono::Utc::now();
+    let cur_time = DateTime32::now();
 
     // Get the median-time-past, which doesn't depend on the time or the previous block height.
     // `context` will always have the correct length, because this function takes an array.
     //
     // TODO: split out median-time-past into its own struct?
     let median_time_past = AdjustedDifficulty::new_from_header_time(
-        cur_time,
+        cur_time.into(),
         tip_height,
         network,
         relevant_data.clone(),
@@ -202,43 +203,39 @@ fn difficulty_time_and_history_tree(
     // > 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 median_time_past =
+        DateTime32::try_from(median_time_past).expect("valid blocks have in-range times");
+
     let min_time = median_time_past
-        .checked_add_signed(Duration::seconds(1))
-        .expect("median time plus a small constant is far below i64::MAX");
+        .checked_add(Duration32::from_seconds(1))
+        .expect("a valid block time plus a small constant is in-range");
 
     // > For each block at block height 2 or greater on Mainnet, or block height 653606 or greater on Testnet, nTime
     // > MUST be less than or equal to the median-time-past of that block plus 90 * 60 seconds.
     //
     // We ignore the height as we are checkpointing on Canopy or higher in Mainnet and Testnet.
     let max_time = median_time_past
-        .checked_add_signed(Duration::seconds(BLOCK_MAX_TIME_SINCE_MEDIAN))
-        .expect("median time plus a small constant is far below i64::MAX");
+        .checked_add(Duration32::from_seconds(BLOCK_MAX_TIME_SINCE_MEDIAN))
+        .expect("a valid block time plus a small constant is in-range");
 
-    let cur_time = cur_time
-        .timestamp()
-        .clamp(min_time.timestamp(), max_time.timestamp());
-
-    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",
-    );
+    let cur_time = cur_time.clamp(min_time, max_time);
 
     // Now that we have a valid time, get the difficulty for that time.
     let difficulty_adjustment = AdjustedDifficulty::new_from_header_time(
-        cur_time,
+        cur_time.into(),
         tip_height,
         network,
         relevant_data.iter().cloned(),
     );
 
     let mut result = GetBlockTemplateChainInfo {
-        tip_height,
         tip_hash,
-        expected_difficulty: difficulty_adjustment.expected_difficulty_threshold(),
-        min_time,
-        cur_time,
-        max_time,
+        tip_height,
         history_tree,
+        expected_difficulty: difficulty_adjustment.expected_difficulty_threshold(),
+        cur_time,
+        min_time,
+        max_time,
     };
 
     adjust_difficulty_and_time_for_testnet(&mut result, network, tip_height, relevant_data);
@@ -279,13 +276,24 @@ fn adjust_difficulty_and_time_for_testnet(
     // 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 previous_block_time: DateTime32 = previous_block_time
+        .try_into()
+        .expect("valid blocks have in-range times");
+
     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");
+    let minimum_difficulty_spacing: Duration32 = minimum_difficulty_spacing
+        .try_into()
+        .expect("small positive values are in-range");
 
     // 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);
+    let std_difficulty_max_time = previous_block_time
+        .checked_add(minimum_difficulty_spacing)
+        .expect("a valid block time plus a small constant is in-range");
+    let min_difficulty_min_time = std_difficulty_max_time
+        .checked_add(Duration32::from_seconds(1))
+        .expect("a valid block time plus a small constant is in-range");
 
     // If a miner is likely to find a block with the cur_time and standard difficulty
     //
@@ -293,9 +301,13 @@ fn adjust_difficulty_and_time_for_testnet(
     // - 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
-    {
+    let local_std_difficulty_limit = std_difficulty_max_time
+        .checked_sub(Duration32::from_seconds(
+            POST_BLOSSOM_POW_TARGET_SPACING * 2,
+        ))
+        .expect("a valid block time minus a small constant is in-range");
+
+    if result.cur_time <= local_std_difficulty_limit {
         // Standard difficulty: the max time needs to exclude min difficulty blocks
         result.max_time = std_difficulty_max_time;
     } else {
@@ -307,7 +319,7 @@ fn adjust_difficulty_and_time_for_testnet(
 
         // And then the difficulty needs to be updated for cur_time
         result.expected_difficulty = AdjustedDifficulty::new_from_header_time(
-            result.cur_time,
+            result.cur_time.into(),
             previous_block_height,
             network,
             relevant_data.iter().cloned(),

zebrad/src/components/sync/progress.rs

@@ -13,6 +13,7 @@ use zebra_chain::{
     parameters::{Network, NetworkUpgrade, POST_BLOSSOM_POW_TARGET_SPACING},
 };
 use zebra_consensus::CheckpointList;
+use zebra_state::MAX_BLOCK_REORG_HEIGHT;
 
 use crate::components::sync::SyncStatus;
 
@@ -48,7 +49,7 @@ 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.
-const MIN_BLOCKS_MINED_AFTER_CHECKPOINT_UPDATE: i32 = 10;
+const MIN_BLOCKS_MINED_AFTER_CHECKPOINT_UPDATE: u32 = 10;
 
 /// Logs Zebra's estimated progress towards the chain tip every minute or so.
 ///
@@ -64,9 +65,11 @@ pub async fn show_block_chain_progress(
     // - the non-finalized state limit, and
     // - the minimum number of extra blocks mined between a checkpoint update,
     //   and the automated tests for that update.
-    let min_after_checkpoint_blocks = i32::try_from(zebra_state::MAX_BLOCK_REORG_HEIGHT)
-        .expect("constant fits in i32")
-        + MIN_BLOCKS_MINED_AFTER_CHECKPOINT_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");
 
     // The minimum height of the valid best chain, based on:
     // - the hard-coded checkpoint height,
@@ -179,9 +182,8 @@ pub async fn show_block_chain_progress(
             } else if is_syncer_stopped && current_height <= after_checkpoint_height {
                 // We've stopped syncing blocks,
                 // but we're below the minimum height estimated from our checkpoints.
-                let min_minutes_after_checkpoint_update: i64 = div_ceil(
-                    i64::from(MIN_BLOCKS_MINED_AFTER_CHECKPOINT_UPDATE)
-                        * POST_BLOSSOM_POW_TARGET_SPACING,
+                let min_minutes_after_checkpoint_update = div_ceil(
+                    MIN_BLOCKS_MINED_AFTER_CHECKPOINT_UPDATE * POST_BLOSSOM_POW_TARGET_SPACING,
                     60,
                 );