change(consensus): Remove Sprout and Sapling parameter download task and debug_skip_preload config (#7844)

* removes groth16 download task

* updates docs.

* Adds new config to test configs

* fixes compatibility with past configs

* uses inner config to deserialize removed field for compatibility

* update docs

* updates latest config

* Applies suggestions from code review

* Avoid duplicating hard-coded default values

---------

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

book/src/user/mining-testnet-s-nomp.md

@@ -20,7 +20,6 @@ These fixes disable mining pool operator payments and miner payments: they just
     ```console
     [consensus]
     checkpoint_sync = true
-    debug_skip_parameter_preload = false
 
     [mempool]
     eviction_memory_time = '1h'

book/src/user/startup.md

@@ -129,10 +129,9 @@ Zebra also starts ongoing tasks to batch verify signatures and proofs.
 
 ```
 zebrad::commands::start: initializing verifiers
-init{config=Config { debug_skip_parameter_preload: false, ... } ... }: zebra_consensus::primitives::groth16::params: checking and loading Zcash Sapling and Sprout parameters
+init{config=Config { ... } ... }: zebra_consensus::primitives::groth16::params: checking and loading Zcash Sapling and Sprout parameters
 init{config=Config { checkpoint_sync: true, ... } ... }: zebra_consensus::chain: initializing chain verifier tip=None max_checkpoint_height=Height(1644839)
 ...
-init{config=Config { debug_skip_parameter_preload: false, ... } ... }: zebra_consensus::chain: Groth16 pre-download and check task finished
 ```
 
 ### Initialize Transaction Mempool

zebra-consensus/src/config.rs

@@ -5,7 +5,12 @@ use serde::{Deserialize, Serialize};
 /// Configuration for parallel semantic verification:
 /// <https://zebra.zfnd.org/dev/rfcs/0002-parallel-verification.html#definitions>
 #[derive(Clone, Debug, Deserialize, Serialize)]
-#[serde(deny_unknown_fields, default)]
+#[serde(
+    deny_unknown_fields,
+    default,
+    from = "InnerConfig",
+    into = "InnerConfig"
+)]
 pub struct Config {
     /// Should Zebra make sure that it follows the consensus chain while syncing?
     /// This is a developer-only option.
@@ -30,9 +35,40 @@ pub struct Config {
     /// For security reasons, this option might be deprecated or ignored in a future Zebra
     /// release.
     pub checkpoint_sync: bool,
+}
 
-    /// Skip the pre-download of Groth16 parameters if this option is true.
+impl From<InnerConfig> for Config {
-    pub debug_skip_parameter_preload: bool,
+    fn from(
+        InnerConfig {
+            checkpoint_sync, ..
+        }: InnerConfig,
+    ) -> Self {
+        Self { checkpoint_sync }
+    }
+}
+
+impl From<Config> for InnerConfig {
+    fn from(Config { checkpoint_sync }: Config) -> Self {
+        Self {
+            checkpoint_sync,
+            _debug_skip_parameter_preload: false,
+        }
+    }
+}
+
+/// Inner consensus configuration for backwards compatibility with older `zebrad.toml` files,
+/// which contain fields that have been removed.
+///
+/// Rust API callers should use [`Config`].
+#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
+#[serde(deny_unknown_fields, default)]
+pub struct InnerConfig {
+    /// See [`Config`] for more details.
+    pub checkpoint_sync: bool,
+
+    #[serde(skip_serializing, rename = "debug_skip_parameter_preload")]
+    /// Unused config field for backwards compatibility.
+    pub _debug_skip_parameter_preload: bool,
 }
 
 // we like our default configs to be explicit
@@ -42,7 +78,15 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             checkpoint_sync: true,
-            debug_skip_parameter_preload: false,
+        }
+    }
+}
+
+impl Default for InnerConfig {
+    fn default() -> Self {
+        Self {
+            checkpoint_sync: Config::default().checkpoint_sync,
+            _debug_skip_parameter_preload: false,
         }
     }
 }

zebra-consensus/src/router.rs

@@ -192,12 +192,10 @@ where
     }
 }
 
-/// Initialize block and transaction verification services,
+/// Initialize block and transaction verification services.
-/// and pre-download Groth16 parameters if requested by the `debug_skip_parameter_preload`
-/// config parameter and if the download is not already started.
 ///
 /// Returns a block verifier, transaction verifier,
-/// the Groth16 parameter download task [`JoinHandle`],
+/// a [`BackgroundTaskHandles`] with the state checkpoint verify task,
 /// and the maximum configured checkpoint verification height.
 ///
 /// The consensus configuration is specified by `config`, and the Zcash network
@@ -210,12 +208,6 @@ where
 /// The transaction verification service asynchronously performs semantic verification
 /// checks. Transactions that pass semantic verification return an `Ok` result to the caller.
 ///
-/// Pre-downloads the Sapling and Sprout Groth16 parameters if needed,
-/// checks they were downloaded correctly, and loads them into Zebra.
-/// (The transaction verifier automatically downloads the parameters on first use.
-/// But the parameter downloads can take around 10 minutes.
-/// So we pre-download the parameters, to avoid verification timeouts.)
-///
 /// This function should only be called once for a particular state service.
 ///
 /// Dropped requests are cancelled on a best-effort basis, but may continue to be processed.
@@ -230,7 +222,6 @@ pub async fn init<S>(
     config: Config,
     network: Network,
     mut state_service: S,
-    debug_skip_parameter_preload: bool,
 ) -> (
     Buffer<BoxService<Request, block::Hash, RouterError>, Request>,
     Buffer<
@@ -244,24 +235,9 @@ where
     S: Service<zs::Request, Response = zs::Response, Error = BoxError> + Send + Clone + 'static,
     S::Future: Send + 'static,
 {
-    // Give other tasks priority before spawning the download and checkpoint tasks.
+    // Give other tasks priority before spawning the checkpoint task.
     tokio::task::yield_now().await;
 
-    // Pre-download Groth16 parameters in a separate thread.
-
-    // The parameter download thread must be launched before initializing any verifiers.
-    // Otherwise, the download might happen on the startup thread.
-    let span = Span::current();
-    let groth16_download_handle = tokio::task::spawn_blocking(move || {
-        span.in_scope(|| {
-            if !debug_skip_parameter_preload {
-                // The lazy static initializer does the download, if needed,
-                // and the file hash checks.
-                lazy_static::initialize(&crate::groth16::GROTH16_PARAMETERS);
-            }
-        })
-    });
-
     // Make sure the state contains the known best chain checkpoints, in a separate thread.
 
     let checkpoint_state_service = state_service.clone();
@@ -381,7 +357,6 @@ where
     let router = Buffer::new(BoxService::new(router), VERIFIER_BUFFER_BOUND);
 
     let task_handles = BackgroundTaskHandles {
-        groth16_download_handle,
         state_checkpoint_verify_handle,
     };
 
@@ -408,10 +383,6 @@ pub fn init_checkpoint_list(config: Config, network: Network) -> (CheckpointList
 /// The background task handles for `zebra-consensus` verifier initialization.
 #[derive(Debug)]
 pub struct BackgroundTaskHandles {
-    /// A handle to the Groth16 parameter download task.
-    /// Finishes when the parameters are downloaded and their checksums verified.
-    pub groth16_download_handle: JoinHandle<()>,
-
     /// A handle to the state checkpoint verify task.
     /// Finishes when all the checkpoints are verified, or when the state tip is reached.
     pub state_checkpoint_verify_handle: JoinHandle<()>,

zebra-consensus/src/router/tests.rs

@@ -71,7 +71,7 @@ async fn verifiers_from_network(
         _transaction_verifier,
         _groth16_download_handle,
         _max_checkpoint_height,
-    ) = crate::router::init(Config::default(), network, state_service.clone(), true).await;
+    ) = crate::router::init(Config::default(), network, state_service.clone()).await;
 
     // We can drop the download task handle here, because:
     // - if the download task fails, the tests will panic, and
@@ -144,12 +144,10 @@ static STATE_VERIFY_TRANSCRIPT_GENESIS: Lazy<
 async fn verify_checkpoint_test() -> Result<(), Report> {
     verify_checkpoint(Config {
         checkpoint_sync: true,
-        debug_skip_parameter_preload: true,
     })
     .await?;
     verify_checkpoint(Config {
         checkpoint_sync: false,
-        debug_skip_parameter_preload: true,
     })
     .await?;
 
@@ -174,7 +172,7 @@ async fn verify_checkpoint(config: Config) -> Result<(), Report> {
         _transaction_verifier,
         _groth16_download_handle,
         _max_checkpoint_height,
-    ) = super::init(config.clone(), network, zs::init_test(network), true).await;
+    ) = super::init(config.clone(), network, zs::init_test(network)).await;
 
     // Add a timeout layer
     let block_verifier_router =

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

@@ -86,13 +86,8 @@ pub async fn test_responses<State, ReadState>(
         _transaction_verifier,
         _parameter_download_task_handle,
         _max_checkpoint_height,
-    ) = zebra_consensus::router::init(
+    ) = zebra_consensus::router::init(zebra_consensus::Config::default(), network, state.clone())
-        zebra_consensus::Config::default(),
+        .await;
-        network,
-        state.clone(),
-        true,
-    )
-    .await;
 
     let mut mock_sync_status = MockSyncStatus::default();
     mock_sync_status.set_is_close_to_tip(true);

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

@@ -874,13 +874,8 @@ async fn rpc_getblockcount() {
         _transaction_verifier,
         _parameter_download_task_handle,
         _max_checkpoint_height,
-    ) = zebra_consensus::router::init(
+    ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone())
-        zebra_consensus::Config::default(),
+        .await;
-        Mainnet,
-        state.clone(),
-        true,
-    )
-    .await;
 
     // Init RPC
     let get_block_template_rpc = GetBlockTemplateRpcImpl::new(
@@ -924,13 +919,8 @@ async fn rpc_getblockcount_empty_state() {
         _transaction_verifier,
         _parameter_download_task_handle,
         _max_checkpoint_height,
-    ) = zebra_consensus::router::init(
+    ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone())
-        zebra_consensus::Config::default(),
+        .await;
-        Mainnet,
-        state.clone(),
-        true,
-    )
-    .await;
 
     // Init RPC
     let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
@@ -976,13 +966,8 @@ async fn rpc_getpeerinfo() {
         _transaction_verifier,
         _parameter_download_task_handle,
         _max_checkpoint_height,
-    ) = zebra_consensus::router::init(
+    ) = zebra_consensus::router::init(zebra_consensus::Config::default(), network, state.clone())
-        zebra_consensus::Config::default(),
+        .await;
-        network,
-        state.clone(),
-        true,
-    )
-    .await;
 
     let mock_peer_address = zebra_network::types::MetaAddr::new_initial_peer(
         std::net::SocketAddr::new(
@@ -1051,13 +1036,8 @@ async fn rpc_getblockhash() {
         _transaction_verifier,
         _parameter_download_task_handle,
         _max_checkpoint_height,
-    ) = zebra_consensus::router::init(
+    ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone())
-        zebra_consensus::Config::default(),
+        .await;
-        Mainnet,
-        state.clone(),
-        true,
-    )
-    .await;
 
     // Init RPC
     let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
@@ -1536,13 +1516,8 @@ async fn rpc_submitblock_errors() {
         _transaction_verifier,
         _parameter_download_task_handle,
         _max_checkpoint_height,
-    ) = zebra_consensus::router::init(
+    ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone())
-        zebra_consensus::Config::default(),
+        .await;
-        Mainnet,
-        state.clone(),
-        true,
-    )
-    .await;
 
     // Init RPC
     let get_block_template_rpc = GetBlockTemplateRpcImpl::new(

zebrad/src/commands/start.rs

@@ -160,7 +160,6 @@ impl StartCmd {
                 config.consensus.clone(),
                 config.network.network,
                 state.clone(),
-                config.consensus.debug_skip_parameter_preload,
             )
             .await;
 
@@ -303,13 +302,9 @@ impl StartCmd {
 
         // startup tasks
         let BackgroundTaskHandles {
-            mut groth16_download_handle,
             mut state_checkpoint_verify_handle,
         } = consensus_task_handles;
 
-        let groth16_download_handle_fused = (&mut groth16_download_handle).fuse();
-        pin!(groth16_download_handle_fused);
-
         let state_checkpoint_verify_handle_fused = (&mut state_checkpoint_verify_handle).fuse();
         pin!(state_checkpoint_verify_handle_fused);
 
@@ -371,19 +366,6 @@ impl StartCmd {
                     .expect("unexpected panic in the end of support task")
                     .map(|_| info!("end of support task exited")),
 
-
-                // Unlike other tasks, we expect the download task to finish while Zebra is running.
-                groth16_download_result = &mut groth16_download_handle_fused => {
-                    groth16_download_result
-                        .unwrap_or_else(|_| panic!(
-                            "unexpected panic in the Groth16 pre-download and check task. {}",
-                            zebra_consensus::groth16::Groth16Parameters::failure_hint())
-                        );
-
-                    exit_when_task_finishes = false;
-                    Ok(())
-                }
-
                 // We also expect the state checkpoint verify task to finish.
                 state_checkpoint_verify_result = &mut state_checkpoint_verify_handle_fused => {
                     state_checkpoint_verify_result
@@ -430,7 +412,6 @@ impl StartCmd {
         end_of_support_task_handle.abort();
 
         // startup tasks
-        groth16_download_handle.abort();
         state_checkpoint_verify_handle.abort();
         old_databases_task_handle.abort();
 

zebrad/src/components/inbound/tests/fake_peer_set.rs

@@ -787,13 +787,8 @@ async fn setup(
 
     // Download task panics and timeouts are propagated to the tests that use Groth16 verifiers.
     let (block_verifier, _transaction_verifier, _groth16_download_handle, _max_checkpoint_height) =
-        zebra_consensus::router::init(
+        zebra_consensus::router::init(consensus_config.clone(), network, state_service.clone())
-            consensus_config.clone(),
+            .await;
-            network,
-            state_service.clone(),
-            true,
-        )
-        .await;
 
     let mut peer_set = MockService::build()
         .with_max_request_delay(MAX_PEER_SET_REQUEST_DELAY)

zebrad/tests/acceptance.rs

@@ -1167,8 +1167,6 @@ fn create_cached_database(network: Network) -> Result<()> {
     create_cached_database_height(
         network,
         height,
-        // We don't need the ZK parameters, we're only using checkpoints
-        true,
         // Use checkpoints to increase sync performance while caching the database
         true,
         // Check that we're still using checkpoints when we finish the cached sync
@@ -1185,8 +1183,6 @@ fn sync_past_mandatory_checkpoint(network: Network) -> Result<()> {
     create_cached_database_height(
         network,
         height.unwrap(),
-        // We need the ZK parameters for full validation
-        false,
         // Test full validation by turning checkpoints off
         false,
         // Check that we're doing full validation when we finish the cached sync
@@ -1216,8 +1212,6 @@ fn full_sync_test(network: Network, timeout_argument_name: &str) -> Result<()> {
             network,
             // Just keep going until we reach the chain tip
             block::Height::MAX,
-            // We need the ZK parameters for full validation
-            false,
             // Use the checkpoints to sync quickly, then do full validation until the chain tip
             true,
             // Finish when we reach the chain tip

zebrad/tests/common/config.rs

@@ -51,10 +51,7 @@ pub fn default_test_config(net: Network) -> Result<ZebradConfig> {
         ..mempool::Config::default()
     };
 
-    let consensus = zebra_consensus::Config {
+    let consensus = zebra_consensus::Config::default();
-        debug_skip_parameter_preload: true,
-        ..zebra_consensus::Config::default()
-    };
 
     let force_use_color = !matches!(
         env::var("ZEBRA_FORCE_USE_COLOR"),

zebrad/tests/common/configs/v1.4.0.toml

@@ -0,0 +1,73 @@
+# Default configuration for zebrad.
+#
+# This file can be used as a skeleton for custom configs.
+#
+# Unspecified fields use default values. Optional fields are Some(field) if the
+# field is present and None if it is absent.
+#
+# This file is generated as an example using zebrad's current defaults.
+# You should set only the config options you want to keep, and delete the rest.
+# Only a subset of fields are present in the skeleton, since optional values
+# whose default is None are omitted.
+#
+# The config format (including a complete list of sections and fields) is
+# documented here:
+# https://doc.zebra.zfnd.org/zebrad/config/struct.ZebradConfig.html
+#
+# zebrad attempts to load configs in the following order:
+#
+# 1. The -c flag on the command line, e.g., `zebrad -c myconfig.toml start`;
+# 2. The file `zebrad.toml` in the users's preference directory (platform-dependent);
+# 3. The default config.
+
+[consensus]
+checkpoint_sync = true
+
+[mempool]
+eviction_memory_time = "1h"
+tx_cost_limit = 80000000
+
+[metrics]
+
+[mining]
+debug_like_zcashd = true
+
+[network]
+cache_dir = true
+crawl_new_peer_interval = "1m 1s"
+initial_mainnet_peers = [
+    "dnsseed.z.cash:8233",
+    "dnsseed.str4d.xyz:8233",
+    "mainnet.seeder.zfnd.org:8233",
+    "mainnet.is.yolo.money:8233",
+]
+initial_testnet_peers = [
+    "dnsseed.testnet.z.cash:18233",
+    "testnet.seeder.zfnd.org:18233",
+    "testnet.is.yolo.money:18233",
+]
+listen_addr = "0.0.0.0:8233"
+max_connections_per_ip = 1
+network = "Mainnet"
+peerset_initial_target_size = 25
+
+[rpc]
+debug_force_finished_sync = false
+parallel_cpu_threads = 0
+
+[state]
+cache_dir = "cache_dir"
+delete_old_database = true
+ephemeral = false
+
+[sync]
+checkpoint_verify_concurrency_limit = 1000
+download_concurrency_limit = 50
+full_verify_concurrency_limit = 20
+parallel_cpu_threads = 0
+
+[tracing]
+buffer_limit = 128000
+force_use_color = false
+use_color = true
+use_journald = false

zebrad/tests/common/sync.rs

@@ -164,10 +164,6 @@ impl MempoolBehavior {
 ///
 /// If `reuse_tempdir` is supplied, use it as the test's temporary directory.
 ///
-/// If `height` is higher than the mandatory checkpoint,
-/// configures `zebrad` to preload the Zcash parameters.
-/// If it is lower, skips the parameter preload.
-///
 /// Configures `zebrad` to debug-enable the mempool based on `mempool_behavior`,
 /// then check the logs for the expected `mempool_behavior`.
 ///
@@ -211,11 +207,6 @@ pub fn sync_until(
     config.mempool.debug_enable_at_height = mempool_behavior.enable_at_height();
     config.consensus.checkpoint_sync = checkpoint_sync;
 
-    // Download the parameters at launch, if we're going to need them later.
-    if height > network.mandatory_checkpoint_height() {
-        config.consensus.debug_skip_parameter_preload = false;
-    }
-
     // Use the default lookahead limit if we're syncing lots of blocks.
     // (Most tests use a smaller limit to minimise redundant block downloads.)
     if height > MIN_HEIGHT_FOR_DEFAULT_LOOKAHEAD {
@@ -356,9 +347,6 @@ pub fn cached_mandatory_checkpoint_test_config(network: Network) -> Result<Zebra
 ///
 /// # Test Settings
 ///
-/// If `debug_skip_parameter_preload` is true, configures `zebrad` to preload the Zcash parameters.
-/// If it is false, skips the parameter preload.
-///
 /// If `checkpoint_sync` is true, configures `zebrad` to use as many checkpoints as possible.
 /// If it is false, only use the mandatory checkpoints.
 ///
@@ -379,7 +367,6 @@ pub fn cached_mandatory_checkpoint_test_config(network: Network) -> Result<Zebra
 pub fn create_cached_database_height(
     network: Network,
     height: Height,
-    debug_skip_parameter_preload: bool,
     checkpoint_sync: bool,
     stop_regex: &str,
 ) -> Result<()> {
@@ -389,7 +376,6 @@ pub fn create_cached_database_height(
     let mut config = cached_mandatory_checkpoint_test_config(network)?;
     // TODO: add convenience methods?
     config.state.debug_stop_at_height = Some(height.0);
-    config.consensus.debug_skip_parameter_preload = debug_skip_parameter_preload;
     config.consensus.checkpoint_sync = checkpoint_sync;
 
     let dir = PathBuf::from("/zebrad-cache");

zebrad/tests/common/test_type.rs

@@ -191,10 +191,6 @@ impl TestType {
             Err(error) => return Some(Err(error)),
         };
 
-        // We want to preload the consensus parameters,
-        // except when we're doing the quick empty state test
-        config.consensus.debug_skip_parameter_preload = !self.needs_zebra_cached_state();
-
         // We want to run multi-threaded RPCs, if we're using them
         if self.launches_lightwalletd() {
             // Automatically runs one thread per available CPU core