Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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