fix(mining): Advertise mined blocks (#9176)

* add a channel for submit_block notifications to gossip task * fix tests and gossip logic * remove the network discriminant and add a test * clippy suggestions * fix unused variable * attempt to fix the conditional compilation issues * fix default * Suggestions for "fix(mining): Advertise mined blocks" (#9183) * refactor error conversions in GetBlockTemplateRpcImpl and rewords documentation * Replaces polling mined block receiver with a select * Skip checking that Zebra is likely synced to the network tip before returning block templates on Testnet. * fixes a clippy lint and a concurrency bug --------- Co-authored-by: Arya <[email protected]>
ZcashFoundation · Feb 5, 2025 · 343656c · 343656c
1 parent 6d01f05
commit 343656c
Show file tree

Hide file tree

Showing 13 changed files with 327 additions and 37 deletions.
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs
@@ -6,6 +6,7 @@ use futures::{future::OptionFuture, TryFutureExt};
 use jsonrpsee::core::{async_trait, RpcResult as Result};
 use jsonrpsee_proc_macros::rpc;
 use jsonrpsee_types::ErrorObject;
+use tokio::sync::watch;
 use tower::{Service, ServiceExt};
 
 use zcash_address::{unified::Encoding, TryFromAddress};
@@ -63,7 +64,10 @@ use crate::{
         hex_data::HexData,
         GetBlockHash,
     },
-    server::{self, error::MapError},
+    server::{
+        self,
+        error::{MapError, OkOrError},
+    },
 };
 
 pub mod constants;
@@ -375,6 +379,10 @@ pub struct GetBlockTemplateRpcImpl<
 
     /// Address book of peers, used for `getpeerinfo`.
     address_book: AddressBook,
+
+    /// A channel to send successful block submissions to the block gossip task,
+    /// so they can be advertised to peers.
+    mined_block_sender: watch::Sender<(block::Hash, block::Height)>,
 }
 
 impl<Mempool, State, Tip, BlockVerifierRouter, SyncStatus, AddressBook> Debug
@@ -465,6 +473,7 @@ where
         block_verifier_router: BlockVerifierRouter,
         sync_status: SyncStatus,
         address_book: AddressBook,
+        mined_block_sender: Option<watch::Sender<(block::Hash, block::Height)>>,
     ) -> Self {
         // Prevent loss of miner funds due to an unsupported or incorrect address type.
         if let Some(miner_address) = mining_config.miner_address.clone() {
@@ -527,6 +536,8 @@ where
             block_verifier_router,
             sync_status,
             address_book,
+            mined_block_sender: mined_block_sender
+                .unwrap_or(submit_block::SubmitBlockChannel::default().sender()),
         }
     }
 }
@@ -937,8 +948,7 @@ where
 
         let block_height = block
             .coinbase_height()
-            .map(|height| height.0.to_string())
-            .unwrap_or_else(|| "invalid coinbase height".to_string());
+            .ok_or_error(0, "coinbase height not found")?;
         let block_hash = block.hash();
 
         let block_verifier_router_response = block_verifier_router
@@ -957,6 +967,11 @@ where
             // The difference is important to miners, because they want to mine on the best chain.
             Ok(block_hash) => {
                 tracing::info!(?block_hash, ?block_height, "submit block accepted");
+
+                self.mined_block_sender
+                    .send((block_hash, block_height))
+                    .map_error_with_prefix(0, "failed to send mined block")?;
+
                 return Ok(submit_block::Response::Accepted);
             }
 

diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
@@ -161,10 +161,7 @@ where
     Tip: ChainTip + Clone + Send + Sync + 'static,
     SyncStatus: ChainSyncStatus + Clone + Send + Sync + 'static,
 {
-    // TODO:
-    // - Add a `disable_peers` field to `Network` to check instead of `disable_pow()` (#8361)
-    // - Check the field in `sync_status` so it applies to the mempool as well.
-    if network.disable_pow() {
+    if network.is_a_test_network() {
         return Ok(());
     }
 

diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/types/submit_block.rs b/zebra-rpc/src/methods/get_block_template_rpcs/types/submit_block.rs
@@ -1,5 +1,9 @@
 //! Parameter and response types for the `submitblock` RPC.
 
+use tokio::sync::watch;
+
+use zebra_chain::{block, parameters::GENESIS_PREVIOUS_BLOCK_HASH};
+
 // Allow doc links to these imports.
 #[allow(unused_imports)]
 use crate::methods::get_block_template_rpcs::GetBlockTemplate;
@@ -64,3 +68,35 @@ impl From<ErrorResponse> for Response {
         Self::ErrorResponse(error_response)
     }
 }
+
+/// A submit block channel, used to inform the gossip task about mined blocks.
+pub struct SubmitBlockChannel {
+    /// The channel sender
+    sender: watch::Sender<(block::Hash, block::Height)>,
+    /// The channel receiver
+    receiver: watch::Receiver<(block::Hash, block::Height)>,
+}
+
+impl SubmitBlockChannel {
+    /// Create a new submit block channel
+    pub fn new() -> Self {
+        let (sender, receiver) = watch::channel((GENESIS_PREVIOUS_BLOCK_HASH, block::Height::MIN));
+        Self { sender, receiver }
+    }
+
+    /// Get the channel sender
+    pub fn sender(&self) -> watch::Sender<(block::Hash, block::Height)> {
+        self.sender.clone()
+    }
+
+    /// Get the channel receiver
+    pub fn receiver(&self) -> watch::Receiver<(block::Hash, block::Height)> {
+        self.receiver.clone()
+    }
+}
+
+impl Default for SubmitBlockChannel {
+    fn default() -> Self {
+        Self::new()
+    }
+}
diff --git a/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs b/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
@@ -151,6 +151,7 @@ pub async fn test_responses<State, ReadState>(
         block_verifier_router.clone(),
         mock_sync_status.clone(),
         mock_address_book,
+        None,
     );
 
     if network.is_a_test_network() && !network.is_default_testnet() {
@@ -286,6 +287,7 @@ pub async fn test_responses<State, ReadState>(
         block_verifier_router,
         mock_sync_status.clone(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // Basic variant (default mode and no extra features)
@@ -395,6 +397,7 @@ pub async fn test_responses<State, ReadState>(
         mock_block_verifier_router.clone(),
         mock_sync_status,
         MockAddressBookPeers::default(),
+        None,
     );
 
     let get_block_template_fut = get_block_template_rpc_mock_state_verifier.get_block_template(

diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
@@ -6,13 +6,12 @@ use std::sync::Arc;
 use futures::FutureExt;
 use tower::buffer::Buffer;
 
-use zebra_chain::serialization::ZcashSerialize;
 use zebra_chain::{
     amount::Amount,
     block::Block,
     chain_tip::{mock::MockChainTip, NoChainTip},
     parameters::Network::*,
-    serialization::ZcashDeserializeInto,
+    serialization::{ZcashDeserializeInto, ZcashSerialize},
     transaction::UnminedTxId,
 };
 use zebra_node_services::BoxError;
@@ -1195,6 +1194,7 @@ async fn rpc_getblockcount() {
         block_verifier_router,
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // Get the tip height using RPC method `get_block_count`
@@ -1244,6 +1244,7 @@ async fn rpc_getblockcount_empty_state() {
         block_verifier_router,
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // Get the tip height using RPC method `get_block_count
@@ -1312,6 +1313,7 @@ async fn rpc_getpeerinfo() {
         block_verifier_router,
         MockSyncStatus::default(),
         mock_address_book,
+        None,
     );
 
     // Call `get_peer_info`
@@ -1372,6 +1374,7 @@ async fn rpc_getblockhash() {
         tower::ServiceBuilder::new().service(block_verifier_router),
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // Query the hashes using positive indexes
@@ -1428,6 +1431,7 @@ async fn rpc_getmininginfo() {
         MockService::build().for_unit_tests(),
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     get_block_template_rpc
@@ -1464,6 +1468,7 @@ async fn rpc_getnetworksolps() {
         MockService::build().for_unit_tests(),
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     let get_network_sol_ps_inputs = [
@@ -1595,6 +1600,7 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
         block_verifier_router,
         mock_sync_status.clone(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // Fake the ChainInfo response
@@ -1870,6 +1876,7 @@ async fn rpc_submitblock_errors() {
         block_verifier_router,
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // Try to submit pre-populated blocks and assert that it responds with duplicate.
@@ -1922,6 +1929,7 @@ async fn rpc_validateaddress() {
         MockService::build().for_unit_tests(),
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     let validate_address = get_block_template_rpc
@@ -1967,6 +1975,7 @@ async fn rpc_z_validateaddress() {
         MockService::build().for_unit_tests(),
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     let z_validate_address = get_block_template_rpc
@@ -2055,6 +2064,7 @@ async fn rpc_getdifficulty() {
         block_verifier_router,
         mock_sync_status.clone(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // Fake the ChainInfo response: smallest numeric difficulty
@@ -2176,6 +2186,7 @@ async fn rpc_z_listunifiedreceivers() {
         MockService::build().for_unit_tests(),
         MockSyncStatus::default(),
         MockAddressBookPeers::default(),
+        None,
     );
 
     // invalid address

diff --git a/zebra-rpc/src/server.rs b/zebra-rpc/src/server.rs
@@ -12,7 +12,7 @@ use std::{fmt, panic};
 use cookie::Cookie;
 use jsonrpsee::server::middleware::rpc::RpcServiceBuilder;
 use jsonrpsee::server::{Server, ServerHandle};
-use tokio::task::JoinHandle;
+use tokio::{sync::watch, task::JoinHandle};
 use tower::Service;
 use tracing::*;
 
@@ -120,6 +120,8 @@ impl RpcServer {
         address_book: AddressBook,
         latest_chain_tip: Tip,
         network: Network,
+        #[cfg_attr(not(feature = "getblocktemplate-rpcs"), allow(unused_variables))]
+        mined_block_sender: Option<watch::Sender<(block::Hash, block::Height)>>,
     ) -> Result<(ServerTask, JoinHandle<()>), tower::BoxError>
     where
         VersionString: ToString + Clone + Send + 'static,
@@ -170,6 +172,7 @@ impl RpcServer {
             block_verifier_router,
             sync_status,
             address_book,
+            mined_block_sender,
         );
 
         // Initialize the rpc methods with the zebra version

diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
@@ -69,6 +69,14 @@ pub(crate) trait MapError<T>: Sized {
     /// Maps errors to [`jsonrpsee_types::ErrorObjectOwned`] with a specific error code.
     fn map_error(self, code: impl Into<ErrorCode>) -> std::result::Result<T, ErrorObjectOwned>;
 
+    /// Maps errors to [`jsonrpsee_types::ErrorObjectOwned`] with a prefixed message and a specific error code.
+    #[cfg(feature = "getblocktemplate-rpcs")]
+    fn map_error_with_prefix(
+        self,
+        code: impl Into<ErrorCode>,
+        msg_prefix: impl ToString,
+    ) -> Result<T, ErrorObjectOwned>;
+
     /// Maps errors to [`jsonrpsee_types::ErrorObjectOwned`] with a [`LegacyCode::Misc`] error code.
     fn map_misc_error(self) -> std::result::Result<T, ErrorObjectOwned> {
         self.map_error(LegacyCode::Misc)
@@ -98,6 +106,21 @@ where
     fn map_error(self, code: impl Into<ErrorCode>) -> Result<T, ErrorObjectOwned> {
         self.map_err(|error| ErrorObject::owned(code.into().code(), error.to_string(), None::<()>))
     }
+
+    #[cfg(feature = "getblocktemplate-rpcs")]
+    fn map_error_with_prefix(
+        self,
+        code: impl Into<ErrorCode>,
+        msg_prefix: impl ToString,
+    ) -> Result<T, ErrorObjectOwned> {
+        self.map_err(|error| {
+            ErrorObject::owned(
+                code.into().code(),
+                format!("{}: {}", msg_prefix.to_string(), error.to_string()),
+                None::<()>,
+            )
+        })
+    }
 }
 
 impl<T> OkOrError<T> for Option<T> {

diff --git a/zebra-rpc/src/server/tests/vectors.rs b/zebra-rpc/src/server/tests/vectors.rs
@@ -56,6 +56,7 @@ async fn rpc_server_spawn() {
         MockAddressBookPeers::default(),
         NoChainTip,
         Mainnet,
+        None,
     );
 
     info!("spawned RPC server, checking services...");
@@ -115,6 +116,7 @@ async fn rpc_spawn_unallocated_port(do_shutdown: bool) {
         MockAddressBookPeers::default(),
         NoChainTip,
         Mainnet,
+        None,
     )
     .await
     .expect("");
@@ -170,6 +172,7 @@ async fn rpc_server_spawn_port_conflict() {
         MockAddressBookPeers::default(),
         NoChainTip,
         Mainnet,
+        None,
     )
     .await;
 
@@ -189,6 +192,7 @@ async fn rpc_server_spawn_port_conflict() {
         MockAddressBookPeers::default(),
         NoChainTip,
         Mainnet,
+        None,
     )
     .await;