change(rpc): Return an error from getblocktemplate RPC if Zebra is still syncing lots of blocks (#5769)

* updates MAX_ESTIMATED_DISTANCE_TO_NETWORK_TIP

* adds ChainSyncStatus trait

* adds sync_status to GetBlockTemplateRpcs

Returns an error from getblocktemplate rpc if !is_close_to_tip

adds MockSyncStatus for tests

* hide MockSyncStatus behind feature flag

* moves is_close_to_tip call into async block

adds tests

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

Co-authored-by: teor <teor@riseup.net>

* uses SeqCst ordering for atomic bool ops.

uses a constant for not synced error code.

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

zebra-chain/src/chain_sync_status.rs

@@ -0,0 +1,32 @@
+//! Defines method signatures for checking if the synchronizer is likely close to the network chain tip.
+
+use std::sync::{
+    atomic::{AtomicBool, Ordering},
+    Arc,
+};
+
+/// An interface for checking if the synchronization is likely close to the network chain tip.
+pub trait ChainSyncStatus {
+    /// Check if the synchronization is likely close to the network chain tip.
+    fn is_close_to_tip(&self) -> bool;
+}
+
+/// A mock [`ChainSyncStatus`] implementation that allows setting the status externally.
+#[derive(Clone, Default)]
+pub struct MockSyncStatus {
+    is_close_to_tip: Arc<AtomicBool>,
+}
+
+impl MockSyncStatus {
+    /// Sets mock sync status determining the return value of `is_close_to_tip()`
+    pub fn set_is_close_to_tip(&mut self, is_close_to_tip: bool) {
+        self.is_close_to_tip
+            .store(is_close_to_tip, Ordering::SeqCst);
+    }
+}
+
+impl ChainSyncStatus for MockSyncStatus {
+    fn is_close_to_tip(&self) -> bool {
+        self.is_close_to_tip.load(Ordering::SeqCst)
+    }
+}

zebra-chain/src/lib.rs

@@ -20,6 +20,7 @@ extern crate tracing;
 
 pub mod amount;
 pub mod block;
+pub mod chain_sync_status;
 pub mod chain_tip;
 pub mod diagnostic;
 pub mod fmt;

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -15,6 +15,7 @@ use zebra_chain::{
         merkle::{self, AuthDataRoot},
         Block, MAX_BLOCK_BYTES, ZCASH_BLOCK_VERSION,
     },
+    chain_sync_status::ChainSyncStatus,
     chain_tip::ChainTip,
     parameters::Network,
     serialization::ZcashDeserializeInto,
@@ -44,10 +45,19 @@ pub mod constants;
 pub(crate) mod types;
 pub(crate) mod zip317;
 
-/// The max estimated distance to the chain tip for the getblocktemplate method
+/// The max estimated distance to the chain tip for the getblocktemplate method.
-// Set to 30 in case the local time is a little ahead.
+///
-// TODO: Replace this with SyncStatus
+/// Allows the same clock skew as the Zcash network, which is 100 blocks, based on the standard rule:
-const MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP: i32 = 30;
+/// > A full validator MUST NOT accept blocks with nTime more than two hours in the future
+/// > according to its clock. This is not strictly a consensus rule because it is nondeterministic,
+/// > and clock time varies between nodes.
+const MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP: i32 = 100;
+
+/// The RPC error code used by `zcashd` for when it's still downloading initial blocks.
+///
+/// `s-nomp` mining pool expects error code `-10` when the node is not synced:
+/// <https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L142>
+pub const NOT_SYNCED_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-10);
 
 /// getblocktemplate RPC method signatures.
 #[rpc(server)]
@@ -123,7 +133,7 @@ pub trait GetBlockTemplateRpc {
 }
 
 /// RPC method implementations.
-pub struct GetBlockTemplateRpcImpl<Mempool, State, Tip, ChainVerifier>
+pub struct GetBlockTemplateRpcImpl<Mempool, State, Tip, ChainVerifier, SyncStatus>
 where
     Mempool: Service<
         mempool::Request,
@@ -140,6 +150,7 @@ where
         + Send
         + Sync
         + 'static,
+    SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
 {
     // TODO: Add the other fields from the [`Rpc`] struct as-needed
 
@@ -166,9 +177,13 @@ where
 
     /// The chain verifier, used for submitting blocks.
     chain_verifier: ChainVerifier,
+
+    /// The chain sync status, used for checking if Zebra is likely close to the network chain tip.
+    sync_status: SyncStatus,
 }
 
-impl<Mempool, State, Tip, ChainVerifier> GetBlockTemplateRpcImpl<Mempool, State, Tip, ChainVerifier>
+impl<Mempool, State, Tip, ChainVerifier, SyncStatus>
+    GetBlockTemplateRpcImpl<Mempool, State, Tip, ChainVerifier, SyncStatus>
 where
     Mempool: Service<
             mempool::Request,
@@ -189,6 +204,7 @@ where
         + Send
         + Sync
         + 'static,
+    SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
 {
     /// Create a new instance of the handler for getblocktemplate RPCs.
     pub fn new(
@@ -198,6 +214,7 @@ where
         state: State,
         latest_chain_tip: Tip,
         chain_verifier: ChainVerifier,
+        sync_status: SyncStatus,
     ) -> Self {
         Self {
             network,
@@ -206,12 +223,13 @@ where
             state,
             latest_chain_tip,
             chain_verifier,
+            sync_status,
         }
     }
 }
 
-impl<Mempool, State, Tip, ChainVerifier> GetBlockTemplateRpc
+impl<Mempool, State, Tip, ChainVerifier, SyncStatus> GetBlockTemplateRpc
-    for GetBlockTemplateRpcImpl<Mempool, State, Tip, ChainVerifier>
+    for GetBlockTemplateRpcImpl<Mempool, State, Tip, ChainVerifier, SyncStatus>
 where
     Mempool: Service<
             mempool::Request,
@@ -235,6 +253,7 @@ where
         + Sync
         + 'static,
     <ChainVerifier as Service<Arc<Block>>>::Future: Send,
+    SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
 {
     fn get_block_count(&self) -> Result<u32> {
         best_chain_tip_height(&self.latest_chain_tip).map(|height| height.0)
@@ -279,6 +298,7 @@ where
 
         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();
 
         // Since this is a very large RPC, we use separate functions for each group of fields.
@@ -301,7 +321,7 @@ where
                     data: None,
                 })?;
 
-            if estimated_distance_to_chain_tip > MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP {
+            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,
                     ?tip_height,
@@ -309,9 +329,7 @@ where
                 );
 
                 return Err(Error {
-                    // Return error code -10 (https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L140)
+                    code: NOT_SYNCED_ERROR_CODE,
-                    // TODO: Confirm that this is the expected error code for !synced
-                    code: ErrorCode::ServerError(-10),
                     message: format!("Zebra has not synced to the chain tip, estimated distance: {estimated_distance_to_chain_tip}"),
                     data: None,
                 });

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

@@ -12,6 +12,7 @@ use tower::{buffer::Buffer, Service};
 
 use zebra_chain::{
     block::Hash,
+    chain_sync_status::MockSyncStatus,
     chain_tip::mock::MockChainTip,
     parameters::{Network, NetworkUpgrade},
     serialization::ZcashDeserializeInto,
@@ -77,6 +78,9 @@ pub async fn test_responses<State, ReadState>(
     )
     .await;
 
+    let mut mock_sync_status = MockSyncStatus::default();
+    mock_sync_status.set_is_close_to_tip(true);
+
     let mining_config = get_block_template_rpcs::config::Config {
         miner_address: Some(transparent::Address::from_script_hash(network, [0xad; 20])),
     };
@@ -107,6 +111,7 @@ pub async fn test_responses<State, ReadState>(
         read_state,
         mock_chain_tip.clone(),
         chain_verifier.clone(),
+        mock_sync_status.clone(),
     );
 
     // `getblockcount`
@@ -138,6 +143,7 @@ pub async fn test_responses<State, ReadState>(
         new_read_state.clone(),
         mock_chain_tip,
         chain_verifier,
+        mock_sync_status,
     );
 
     // `getblocktemplate`

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

@@ -21,6 +21,9 @@ 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();
@@ -658,6 +661,7 @@ async fn rpc_getblockcount() {
         read_state,
         latest_chain_tip.clone(),
         chain_verifier,
+        MockSyncStatus::default(),
     );
 
     // Get the tip height using RPC method `get_block_count`
@@ -703,6 +707,7 @@ async fn rpc_getblockcount_empty_state() {
         read_state,
         latest_chain_tip.clone(),
         chain_verifier,
+        MockSyncStatus::default(),
     );
 
     // Get the tip height using RPC method `get_block_count
@@ -754,6 +759,7 @@ async fn rpc_getblockhash() {
         read_state,
         latest_chain_tip.clone(),
         tower::ServiceBuilder::new().service(chain_verifier),
+        MockSyncStatus::default(),
     );
 
     // Query the hashes using positive indexes
@@ -809,6 +815,9 @@ async fn rpc_getblocktemplate() {
     let read_state = MockService::build().for_unit_tests();
     let chain_verifier = MockService::build().for_unit_tests();
 
+    let mut mock_sync_status = MockSyncStatus::default();
+    mock_sync_status.set_is_close_to_tip(true);
+
     let mining_config = get_block_template_rpcs::config::Config {
         miner_address: Some(transparent::Address::from_script_hash(Mainnet, [0x7e; 20])),
     };
@@ -837,7 +846,8 @@ async fn rpc_getblocktemplate() {
         Buffer::new(mempool.clone(), 1),
         read_state.clone(),
         mock_chain_tip,
-        tower::ServiceBuilder::new().service(chain_verifier),
+        chain_verifier,
+        mock_sync_status.clone(),
     );
 
     // Fake the ChainInfo response
@@ -909,7 +919,7 @@ async fn rpc_getblocktemplate() {
 
     mempool.expect_no_requests().await;
 
-    mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(100));
+    mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(200));
     let get_block_template_sync_error = get_block_template_rpc
         .get_block_template()
         .await
@@ -919,6 +929,30 @@ async fn rpc_getblocktemplate() {
         get_block_template_sync_error.code,
         ErrorCode::ServerError(-10)
     );
+
+    mock_sync_status.set_is_close_to_tip(false);
+
+    mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(0));
+    let get_block_template_sync_error = get_block_template_rpc
+        .get_block_template()
+        .await
+        .expect_err("needs an error when syncer is not close to tip");
+
+    assert_eq!(
+        get_block_template_sync_error.code,
+        ErrorCode::ServerError(-10)
+    );
+
+    mock_chain_tip_sender.send_estimated_distance_to_network_chain_tip(Some(200));
+    let get_block_template_sync_error = get_block_template_rpc
+        .get_block_template()
+        .await
+        .expect_err("needs an error when syncer is not close to tip or estimated distance to network chain tip is far");
+
+    assert_eq!(
+        get_block_template_sync_error.code,
+        ErrorCode::ServerError(-10)
+    );
 }
 
 #[cfg(feature = "getblocktemplate-rpcs")]
@@ -958,7 +992,8 @@ async fn rpc_submitblock_errors() {
         Buffer::new(mempool.clone(), 1),
         read_state,
         latest_chain_tip.clone(),
-        tower::ServiceBuilder::new().service(chain_verifier),
+        chain_verifier,
+        MockSyncStatus::default(),
     );
 
     // Try to submit pre-populated blocks and assert that it responds with duplicate.

zebra-rpc/src/server.rs

@@ -18,6 +18,7 @@ use tracing::{Instrument, *};
 
 use zebra_chain::{
     block::{self, Block},
+    chain_sync_status::ChainSyncStatus,
     chain_tip::ChainTip,
     parameters::Network,
 };
@@ -71,7 +72,7 @@ impl RpcServer {
     //
     // TODO: put some of the configs or services in their own struct?
     #[allow(clippy::too_many_arguments)]
-    pub fn spawn<Version, Mempool, State, Tip, ChainVerifier>(
+    pub fn spawn<Version, Mempool, State, Tip, ChainVerifier, SyncStatus>(
         config: Config,
         #[cfg(feature = "getblocktemplate-rpcs")]
         mining_config: get_block_template_rpcs::config::Config,
@@ -83,6 +84,8 @@ impl RpcServer {
         state: State,
         #[cfg_attr(not(feature = "getblocktemplate-rpcs"), allow(unused_variables))]
         chain_verifier: ChainVerifier,
+        #[cfg_attr(not(feature = "getblocktemplate-rpcs"), allow(unused_variables))]
+        sync_status: SyncStatus,
         latest_chain_tip: Tip,
         network: Network,
     ) -> (JoinHandle<()>, JoinHandle<()>, Option<Self>)
@@ -110,6 +113,7 @@ impl RpcServer {
             + Sync
             + 'static,
         <ChainVerifier as Service<Arc<Block>>>::Future: Send,
+        SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
     {
         if let Some(listen_addr) = config.listen_addr {
             info!("Trying to open RPC endpoint at {}...", listen_addr,);
@@ -144,6 +148,7 @@ impl RpcServer {
                     state.clone(),
                     latest_chain_tip.clone(),
                     chain_verifier,
+                    sync_status,
                 );
 
                 io.extend_with(get_block_template_rpc_impl.to_delegate());

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

@@ -8,7 +8,9 @@ use std::{
 use futures::FutureExt;
 use tower::buffer::Buffer;
 
-use zebra_chain::{chain_tip::NoChainTip, parameters::Network::*};
+use zebra_chain::{
+    chain_sync_status::MockSyncStatus, chain_tip::NoChainTip, parameters::Network::*,
+};
 use zebra_node_services::BoxError;
 
 use zebra_test::mock_service::MockService;
@@ -58,6 +60,7 @@ fn rpc_server_spawn(parallel_cpu_threads: bool) {
             Buffer::new(mempool.clone(), 1),
             Buffer::new(state.clone(), 1),
             Buffer::new(chain_verifier.clone(), 1),
+            MockSyncStatus::default(),
             NoChainTip,
             Mainnet,
         );
@@ -142,6 +145,7 @@ fn rpc_server_spawn_unallocated_port(parallel_cpu_threads: bool, do_shutdown: bo
             Buffer::new(mempool.clone(), 1),
             Buffer::new(state.clone(), 1),
             Buffer::new(chain_verifier.clone(), 1),
+            MockSyncStatus::default(),
             NoChainTip,
             Mainnet,
         );
@@ -220,6 +224,7 @@ fn rpc_server_spawn_port_conflict() {
                 Buffer::new(mempool.clone(), 1),
                 Buffer::new(state.clone(), 1),
                 Buffer::new(chain_verifier.clone(), 1),
+                MockSyncStatus::default(),
                 NoChainTip,
                 Mainnet,
             );
@@ -235,6 +240,7 @@ fn rpc_server_spawn_port_conflict() {
             Buffer::new(mempool.clone(), 1),
             Buffer::new(state.clone(), 1),
             Buffer::new(chain_verifier.clone(), 1),
+            MockSyncStatus::default(),
             NoChainTip,
             Mainnet,
         );
@@ -324,6 +330,7 @@ fn rpc_server_spawn_port_conflict_parallel_auto() {
                 Buffer::new(mempool.clone(), 1),
                 Buffer::new(state.clone(), 1),
                 Buffer::new(chain_verifier.clone(), 1),
+                MockSyncStatus::default(),
                 NoChainTip,
                 Mainnet,
             );
@@ -339,6 +346,7 @@ fn rpc_server_spawn_port_conflict_parallel_auto() {
             Buffer::new(mempool.clone(), 1),
             Buffer::new(state.clone(), 1),
             Buffer::new(chain_verifier.clone(), 1),
+            MockSyncStatus::default(),
             NoChainTip,
             Mainnet,
         );

zebrad/src/commands/start.rs

@@ -185,6 +185,7 @@ impl StartCmd {
             mempool.clone(),
             read_only_state_service,
             chain_verifier.clone(),
+            sync_status.clone(),
             latest_chain_tip.clone(),
             config.network.network,
         );

zebrad/src/components/mempool.rs

@@ -30,7 +30,10 @@ use futures::{future::FutureExt, stream::Stream};
 use tokio::sync::watch;
 use tower::{buffer::Buffer, timeout::Timeout, util::BoxService, Service};
 
-use zebra_chain::{block::Height, chain_tip::ChainTip, transaction::UnminedTxId};
+use zebra_chain::{
+    block::Height, chain_sync_status::ChainSyncStatus, chain_tip::ChainTip,
+    transaction::UnminedTxId,
+};
 use zebra_consensus::{error::TransactionError, transaction};
 use zebra_network as zn;
 use zebra_node_services::mempool::{Request, Response};

zebrad/src/components/mempool/crawler/tests/prop.rs

@@ -8,7 +8,9 @@ use proptest::{
 };
 use tokio::time;
 
-use zebra_chain::{parameters::Network, transaction::UnminedTxId};
+use zebra_chain::{
+    chain_sync_status::ChainSyncStatus, parameters::Network, transaction::UnminedTxId,
+};
 use zebra_network as zn;
 use zebra_node_services::mempool::Gossip;
 use zebra_state::ChainTipSender;

zebrad/src/components/sync/progress.rs

@@ -7,6 +7,7 @@ use num_integer::div_ceil;
 
 use zebra_chain::{
     block::Height,
+    chain_sync_status::ChainSyncStatus,
     chain_tip::ChainTip,
     fmt::humantime_seconds,
     parameters::{Network, NetworkUpgrade, POST_BLOSSOM_POW_TARGET_SPACING},

zebrad/src/components/sync/status.rs

@@ -1,4 +1,5 @@
 use tokio::sync::watch;
+use zebra_chain::chain_sync_status::ChainSyncStatus;
 
 use super::RecentSyncLengths;
 
@@ -43,8 +44,28 @@ impl SyncStatus {
         Ok(())
     }
 
+    /// Feed the given [`RecentSyncLengths`] it order to make the matching
+    /// [`SyncStatus`] report that it's close to the tip.
+    #[cfg(test)]
+    pub(crate) fn sync_close_to_tip(recent_syncs: &mut RecentSyncLengths) {
+        for _ in 0..RecentSyncLengths::MAX_RECENT_LENGTHS {
+            recent_syncs.push_extend_tips_length(1);
+        }
+    }
+
+    /// Feed the given [`RecentSyncLengths`] it order to make the matching
+    /// [`SyncStatus`] report that it's not close to the tip.
+    #[cfg(test)]
+    pub(crate) fn sync_far_from_tip(recent_syncs: &mut RecentSyncLengths) {
+        for _ in 0..RecentSyncLengths::MAX_RECENT_LENGTHS {
+            recent_syncs.push_extend_tips_length(Self::MIN_DIST_FROM_TIP * 10);
+        }
+    }
+}
+
+impl ChainSyncStatus for SyncStatus {
     /// Check if the synchronization is likely close to the chain tip.
-    pub fn is_close_to_tip(&self) -> bool {
+    fn is_close_to_tip(&self) -> bool {
         let sync_lengths = self.latest_sync_length.borrow();
 
         // Return early if sync_lengths is empty.
@@ -66,22 +87,4 @@ impl SyncStatus {
         // average sync length falls below the threshold.
         avg < Self::MIN_DIST_FROM_TIP
     }
-
-    /// Feed the given [`RecentSyncLengths`] it order to make the matching
-    /// [`SyncStatus`] report that it's close to the tip.
-    #[cfg(test)]
-    pub(crate) fn sync_close_to_tip(recent_syncs: &mut RecentSyncLengths) {
-        for _ in 0..RecentSyncLengths::MAX_RECENT_LENGTHS {
-            recent_syncs.push_extend_tips_length(1);
-        }
-    }
-
-    /// Feed the given [`RecentSyncLengths`] it order to make the matching
-    /// [`SyncStatus`] report that it's not close to the tip.
-    #[cfg(test)]
-    pub(crate) fn sync_far_from_tip(recent_syncs: &mut RecentSyncLengths) {
-        for _ in 0..RecentSyncLengths::MAX_RECENT_LENGTHS {
-            recent_syncs.push_extend_tips_length(Self::MIN_DIST_FROM_TIP * 10);
-        }
-    }
 }

zebrad/src/components/sync/status/tests.rs

@@ -3,6 +3,7 @@ use std::{env, sync::Arc, time::Duration};
 use futures::{select, FutureExt};
 use proptest::prelude::*;
 use tokio::{sync::Semaphore, time::timeout};
+use zebra_chain::chain_sync_status::ChainSyncStatus;
 
 use super::{super::RecentSyncLengths, SyncStatus};