From 0a3790b73e4e4e62da7e375f8b2074dffadb86c0 Mon Sep 17 00:00:00 2001 From: Arya Date: Fri, 27 Oct 2023 02:12:57 -0400 Subject: [PATCH] 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 --- book/src/user/mining-testnet-s-nomp.md | 1 - book/src/user/startup.md | 3 +- zebra-consensus/src/config.rs | 52 ++++++++++++- zebra-consensus/src/router.rs | 35 +-------- zebra-consensus/src/router/tests.rs | 6 +- .../tests/snapshot/get_block_template_rpcs.rs | 9 +-- zebra-rpc/src/methods/tests/vectors.rs | 45 +++--------- zebrad/src/commands/start.rs | 19 ----- .../components/inbound/tests/fake_peer_set.rs | 9 +-- zebrad/tests/acceptance.rs | 6 -- zebrad/tests/common/config.rs | 5 +- zebrad/tests/common/configs/v1.4.0.toml | 73 +++++++++++++++++++ zebrad/tests/common/sync.rs | 14 ---- zebrad/tests/common/test_type.rs | 4 - 14 files changed, 142 insertions(+), 139 deletions(-) create mode 100644 zebrad/tests/common/configs/v1.4.0.toml diff --git a/book/src/user/mining-testnet-s-nomp.md b/book/src/user/mining-testnet-s-nomp.md index fbffdb14d..61548daba 100644 --- a/book/src/user/mining-testnet-s-nomp.md +++ b/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' diff --git a/book/src/user/startup.md b/book/src/user/startup.md index d7e1faf7a..ffb41dbe7 100644 --- a/book/src/user/startup.md +++ b/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 diff --git a/zebra-consensus/src/config.rs b/zebra-consensus/src/config.rs index 8bad913b8..c2cc914ba 100644 --- a/zebra-consensus/src/config.rs +++ b/zebra-consensus/src/config.rs @@ -5,7 +5,12 @@ use serde::{Deserialize, Serialize}; /// Configuration for parallel semantic verification: /// #[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. - pub debug_skip_parameter_preload: bool, +impl From for Config { + fn from( + InnerConfig { + checkpoint_sync, .. + }: InnerConfig, + ) -> Self { + Self { checkpoint_sync } + } +} + +impl From 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, } } } diff --git a/zebra-consensus/src/router.rs b/zebra-consensus/src/router.rs index 28fac00c0..179dc487d 100644 --- a/zebra-consensus/src/router.rs +++ b/zebra-consensus/src/router.rs @@ -192,12 +192,10 @@ where } } -/// 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. +/// Initialize block and transaction verification services. /// /// 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( config: Config, network: Network, mut state_service: S, - debug_skip_parameter_preload: bool, ) -> ( Buffer, Request>, Buffer< @@ -244,24 +235,9 @@ where S: Service + 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<()>, diff --git a/zebra-consensus/src/router/tests.rs b/zebra-consensus/src/router/tests.rs index eb2abf1b2..c438a79f8 100644 --- a/zebra-consensus/src/router/tests.rs +++ b/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 = 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 index 258159cb3..29dd8c577 100644 --- a/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs @@ -86,13 +86,8 @@ pub async fn test_responses( _transaction_verifier, _parameter_download_task_handle, _max_checkpoint_height, - ) = zebra_consensus::router::init( - zebra_consensus::Config::default(), - network, - state.clone(), - true, - ) - .await; + ) = zebra_consensus::router::init(zebra_consensus::Config::default(), network, state.clone()) + .await; let mut mock_sync_status = MockSyncStatus::default(); mock_sync_status.set_is_close_to_tip(true); diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 68f08c184..408cf53af 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/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::Config::default(), - Mainnet, - state.clone(), - true, - ) - .await; + ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone()) + .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::Config::default(), - Mainnet, - state.clone(), - true, - ) - .await; + ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone()) + .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::Config::default(), - network, - state.clone(), - true, - ) - .await; + ) = zebra_consensus::router::init(zebra_consensus::Config::default(), network, state.clone()) + .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::Config::default(), - Mainnet, - state.clone(), - true, - ) - .await; + ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone()) + .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::Config::default(), - Mainnet, - state.clone(), - true, - ) - .await; + ) = zebra_consensus::router::init(zebra_consensus::Config::default(), Mainnet, state.clone()) + .await; // Init RPC let get_block_template_rpc = GetBlockTemplateRpcImpl::new( diff --git a/zebrad/src/commands/start.rs b/zebrad/src/commands/start.rs index 3f44525b2..2248f3ff0 100644 --- a/zebrad/src/commands/start.rs +++ b/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(); diff --git a/zebrad/src/components/inbound/tests/fake_peer_set.rs b/zebrad/src/components/inbound/tests/fake_peer_set.rs index 1a383bb90..783686afa 100644 --- a/zebrad/src/components/inbound/tests/fake_peer_set.rs +++ b/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( - consensus_config.clone(), - network, - state_service.clone(), - true, - ) - .await; + zebra_consensus::router::init(consensus_config.clone(), network, state_service.clone()) + .await; let mut peer_set = MockService::build() .with_max_request_delay(MAX_PEER_SET_REQUEST_DELAY) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index c51bdc989..c6d3097a1 100644 --- a/zebrad/tests/acceptance.rs +++ b/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 diff --git a/zebrad/tests/common/config.rs b/zebrad/tests/common/config.rs index 3abf83d7e..dd2d4544b 100644 --- a/zebrad/tests/common/config.rs +++ b/zebrad/tests/common/config.rs @@ -51,10 +51,7 @@ pub fn default_test_config(net: Network) -> Result { ..mempool::Config::default() }; - let consensus = zebra_consensus::Config { - debug_skip_parameter_preload: true, - ..zebra_consensus::Config::default() - }; + let consensus = zebra_consensus::Config::default(); let force_use_color = !matches!( env::var("ZEBRA_FORCE_USE_COLOR"), diff --git a/zebrad/tests/common/configs/v1.4.0.toml b/zebrad/tests/common/configs/v1.4.0.toml new file mode 100644 index 000000000..0879f87c4 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/zebrad/tests/common/sync.rs b/zebrad/tests/common/sync.rs index 599a10c4b..6f0b5c32c 100644 --- a/zebrad/tests/common/sync.rs +++ b/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 Result 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"); diff --git a/zebrad/tests/common/test_type.rs b/zebrad/tests/common/test_type.rs index ddd7386c1..bb4291c3d 100644 --- a/zebrad/tests/common/test_type.rs +++ b/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