Skip to content

Commit

Permalink
change(consensus): Remove Sprout and Sapling parameter download task …
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
arya2 and teor2345 authored Oct 27, 2023
1 parent d145526 commit 0a3790b
Show file tree
Hide file tree
Showing 14 changed files with 142 additions and 139 deletions.
1 change: 0 additions & 1 deletion book/src/user/mining-testnet-s-nomp.md
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
3 changes: 1 addition & 2 deletions book/src/user/startup.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 48 additions & 4 deletions zebra-consensus/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
}

impl From<InnerConfig> for Config {
fn from(
InnerConfig {
checkpoint_sync, ..
}: InnerConfig,
) -> Self {
Self { checkpoint_sync }
}
}

/// Skip the pre-download of Groth16 parameters if this option is true.
pub debug_skip_parameter_preload: bool,
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
Expand All @@ -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,
}
}
}
35 changes: 3 additions & 32 deletions zebra-consensus/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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<
Expand All @@ -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();
Expand Down Expand Up @@ -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,
};

Expand All @@ -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<()>,
Expand Down
6 changes: 2 additions & 4 deletions zebra-consensus/src/router/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?;

Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::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);
Expand Down
45 changes: 10 additions & 35 deletions zebra-rpc/src/methods/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
19 changes: 0 additions & 19 deletions zebrad/src/commands/start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ impl StartCmd {
config.consensus.clone(),
config.network.network,
state.clone(),
config.consensus.debug_skip_parameter_preload,
)
.await;

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand Down
9 changes: 2 additions & 7 deletions zebrad/src/components/inbound/tests/fake_peer_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 0 additions & 6 deletions zebrad/tests/acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 0a3790b

Please sign in to comment.