diff --git a/crates/sc-consensus-nakamoto/src/lib.rs b/crates/sc-consensus-nakamoto/src/lib.rs index 8fa8f4d0baf51..5dfab521d98bc 100644 --- a/crates/sc-consensus-nakamoto/src/lib.rs +++ b/crates/sc-consensus-nakamoto/src/lib.rs @@ -13,6 +13,7 @@ pub use block_import::{ insert_bitcoin_block_hash_mapping, BitcoinBlockImport, BitcoinBlockImporter, ImportConfig, ImportStatus, }; +pub use chain_params::ChainParams; pub use import_queue::{ bitcoin_import_queue, BlockImportQueue, ImportBlocks, ImportManyBlocksResult, }; diff --git a/crates/sc-consensus-nakamoto/src/verification.rs b/crates/sc-consensus-nakamoto/src/verification.rs index eeb681d6ccc98..b77ab55440449 100644 --- a/crates/sc-consensus-nakamoto/src/verification.rs +++ b/crates/sc-consensus-nakamoto/src/verification.rs @@ -189,7 +189,7 @@ where ) -> Result<(), Error> { match self.block_verification { BlockVerification::Full => { - let lock_time_cutoff = self.header_verifier.verify_header(&block.header)?; + let lock_time_cutoff = self.header_verifier.verify(&block.header)?; if block_number >= self.chain_params.segwit_height && !block.check_witness_commitment() @@ -205,7 +205,7 @@ where self.verify_transactions(block_number, block, txids, lock_time_cutoff)?; } BlockVerification::HeaderOnly => { - self.header_verifier.verify_header(&block.header)?; + self.header_verifier.verify(&block.header)?; } BlockVerification::None => {} } diff --git a/crates/sc-consensus-nakamoto/src/verification/header_verify.rs b/crates/sc-consensus-nakamoto/src/verification/header_verify.rs index 1d2f738ef2d1d..6767574659694 100644 --- a/crates/sc-consensus-nakamoto/src/verification/header_verify.rs +++ b/crates/sc-consensus-nakamoto/src/verification/header_verify.rs @@ -71,7 +71,7 @@ where /// - The time must be greater than the median time of the last 11 blocks. /// /// - pub fn verify_header(&self, header: &BitcoinHeader) -> Result { + pub fn verify(&self, header: &BitcoinHeader) -> Result { let prev_block_hash = header.prev_blockhash; let prev_block_header = self.client.block_header(prev_block_hash).ok_or( diff --git a/crates/subcoin-network/src/block_downloader/headers_first.rs b/crates/subcoin-network/src/block_downloader/headers_first.rs index 2b69c1a5eb855..4630cec3faf64 100644 --- a/crates/subcoin-network/src/block_downloader/headers_first.rs +++ b/crates/subcoin-network/src/block_downloader/headers_first.rs @@ -381,7 +381,7 @@ where best_queued_number = self.download_manager.best_queued_number, requested_blocks_count = get_data_msg.len(), downloaded_headers = self.downloaded_headers.len(), - "Headers from {start} to {end} downloaded, requesting blocks", + "Downloaded headers from {start} to {end}, requesting blocks", ); self.download_state = @@ -397,7 +397,14 @@ where .collect::>>(); let total_batches = batches.len(); - let initial_batch = batches.pop_front().expect("Batch must not be empty; qed"); + + let Some(initial_batch) = batches.pop_front() else { + tracing::warn!( + ?total_batches, + "Download batches is empty, failed to start new block download" + ); + return SyncAction::None; + }; let get_data_msg = prepare_ordered_block_data_request(initial_batch.clone(), &self.downloaded_headers); diff --git a/crates/subcoin-node/src/cli.rs b/crates/subcoin-node/src/cli.rs index b63eeefe97d84..248df4dc76384 100644 --- a/crates/subcoin-node/src/cli.rs +++ b/crates/subcoin-node/src/cli.rs @@ -181,7 +181,7 @@ pub fn run() -> sc_cli::Result<()> { task_manager, import_queue, .. - } = subcoin_service::new_partial(&config)?; + } = subcoin_service::new_partial(&config, bitcoin::Network::Bitcoin)?; Ok((cmd.run(client, import_queue), task_manager)) }) } @@ -192,7 +192,7 @@ pub fn run() -> sc_cli::Result<()> { client, task_manager, .. - } = subcoin_service::new_partial(&config)?; + } = subcoin_service::new_partial(&config, bitcoin::Network::Bitcoin)?; Ok((cmd.run(client, config.database), task_manager)) }) } @@ -203,7 +203,7 @@ pub fn run() -> sc_cli::Result<()> { client, task_manager, .. - } = subcoin_service::new_partial(&config)?; + } = subcoin_service::new_partial(&config, bitcoin::Network::Bitcoin)?; let run_cmd = async move { tracing::info!("Exporting raw state..."); @@ -235,7 +235,7 @@ pub fn run() -> sc_cli::Result<()> { task_manager, backend, .. - } = subcoin_service::new_partial(&config)?; + } = subcoin_service::new_partial(&config, bitcoin::Network::Bitcoin)?; Ok((cmd.run(client, backend, None), task_manager)) }) } @@ -251,7 +251,7 @@ pub fn run() -> sc_cli::Result<()> { } BenchmarkCmd::Block(cmd) => { let PartialComponents { client, .. } = - subcoin_service::new_partial(&config)?; + subcoin_service::new_partial(&config, bitcoin::Network::Bitcoin)?; cmd.run(client) } #[cfg(not(feature = "runtime-benchmarks"))] @@ -263,7 +263,7 @@ pub fn run() -> sc_cli::Result<()> { BenchmarkCmd::Storage(cmd) => { let PartialComponents { client, backend, .. - } = subcoin_service::new_partial(&config)?; + } = subcoin_service::new_partial(&config, bitcoin::Network::Bitcoin)?; let db = backend.expose_db(); let storage = backend.expose_storage(); diff --git a/crates/subcoin-node/src/commands/run.rs b/crates/subcoin-node/src/commands/run.rs index 37ce374734560..1be64d5810023 100644 --- a/crates/subcoin-node/src/commands/run.rs +++ b/crates/subcoin-node/src/commands/run.rs @@ -97,7 +97,7 @@ impl RunCmd { storage_monitor: sc_storage_monitor::StorageMonitorParams, ) -> sc_cli::Result { let block_execution_strategy = run.common_params.block_execution_strategy(); - let network = run.common_params.bitcoin_network(); + let bitcoin_network = run.common_params.bitcoin_network(); let import_config = run.common_params.import_config(); let no_finalizer = run.no_finalizer; let major_sync_confirmation_depth = run.major_sync_confirmation_depth; @@ -107,11 +107,10 @@ impl RunCmd { backend, mut task_manager, block_executor, - keystore_container, telemetry, .. } = subcoin_service::new_node(subcoin_service::SubcoinConfiguration { - network, + network: bitcoin_network, config: &config, block_execution_strategy, no_hardware_benchmarks, @@ -141,7 +140,7 @@ impl RunCmd { let (subcoin_networking, subcoin_network_handle) = subcoin_network::Network::new( client.clone(), - run.subcoin_network_params(network), + run.subcoin_network_params(bitcoin_network), import_queue, spawn_handle.clone(), config.prometheus_registry().cloned(), @@ -174,7 +173,7 @@ impl RunCmd { client.clone(), backend, &mut task_manager, - keystore_container.keystore(), + bitcoin_network, telemetry, )? } @@ -184,7 +183,7 @@ impl RunCmd { client.clone(), backend, &mut task_manager, - keystore_container.keystore(), + bitcoin_network, telemetry, )? } diff --git a/crates/subcoin-service/src/lib.rs b/crates/subcoin-service/src/lib.rs index 9b50a67d493a9..a8625835c36e6 100644 --- a/crates/subcoin-service/src/lib.rs +++ b/crates/subcoin-service/src/lib.rs @@ -15,7 +15,7 @@ use genesis_block_builder::GenesisBlockBuilder; use sc_client_api::{AuxStore, BlockchainEvents, Finalizer, HeaderBackend}; use sc_consensus::import_queue::BasicQueue; use sc_consensus::{BlockImportParams, Verifier}; -use sc_consensus_nakamoto::{BlockExecutionStrategy, BlockExecutor}; +use sc_consensus_nakamoto::{BlockExecutionStrategy, BlockExecutor, ChainParams, HeaderVerifier}; use sc_executor::NativeElseWasmExecutor; use sc_network_sync::SyncingService; use sc_service::config::PrometheusConfig; @@ -28,7 +28,6 @@ use sc_utils::mpsc::TracingUnboundedSender; use sp_consensus::SyncOracle; use sp_core::traits::SpawnNamed; use sp_core::Encode; -use sp_keystore::KeystorePtr; use sp_runtime::traits::{Block as BlockT, CheckedSub, Header as HeaderT}; use std::ops::Deref; use std::path::Path; @@ -272,7 +271,7 @@ pub fn start_substrate_network( client: Arc, _backend: Arc, task_manager: &mut TaskManager, - _keystore: KeystorePtr, + bitcoin_network: bitcoin::Network, mut telemetry: Option, ) -> Result where @@ -294,7 +293,7 @@ where ); let import_queue = BasicQueue::new( - SubstrateImportQueueVerifier, + SubstrateImportQueueVerifier::new(client.clone(), bitcoin_network), Box::new(client.clone()), None, &task_manager.spawn_essential_handle(), @@ -389,7 +388,7 @@ pub async fn finalize_confirmed_blocks( Backend: sc_client_api::backend::Backend + 'static, { // Use `every_import_notification_stream()` so that we can receive the notifications even when - // major syncing. + // the Substrate network is major syncing. let mut block_import_stream = client.every_import_notification_stream(); while let Some(notification) = block_import_stream.next().await { @@ -397,7 +396,7 @@ pub async fn finalize_confirmed_blocks( .number(notification.hash) .ok() .flatten() - .expect("Imported Block must be available; qed"); + .expect("Imported block must be available; qed"); let Some(confirmed_block_number) = block_number.checked_sub(&confirmation_depth.into()) else { @@ -411,7 +410,8 @@ pub async fn finalize_confirmed_blocks( } if subcoin_networking_is_major_syncing.load(Ordering::Relaxed) { - // During major sync, finalize every `major_sync_confirmation_depth` block to avoid race conditions: + // During the major sync of Subcoin networking, we choose to finalize every `major_sync_confirmation_depth` + // block to avoid race conditions: // >Safety violation: attempted to revert finalized block... if confirmed_block_number < finalized_number + major_sync_confirmation_depth.into() { continue; @@ -419,6 +419,11 @@ pub async fn finalize_confirmed_blocks( } if let Some(sync_service) = substrate_sync_service.as_ref() { + // State sync relies on the finalized block notification to progress + // Substrate chain sync component relies on the finalized block notification to + // initiate the state sync, do not attempt to finalize the block when the queued blocks + // are not empty, so that the state sync can be started when the last finalized block + // notification is sent. if sync_service.is_major_syncing() && sync_service.num_queued_blocks().await.unwrap_or(0) > 0 { @@ -450,6 +455,8 @@ pub async fn finalize_confirmed_blocks( || substrate_sync_service .map(|sync_service| sync_service.is_major_syncing()) .unwrap_or(false); + + // Only print the log when not major syncing to not clutter the logs. if !is_major_syncing { tracing::info!("✅ Successfully finalized block: {block_to_finalize}"); } @@ -477,7 +484,10 @@ type PartialComponents = sc_service::PartialComponents< >; /// Creates a partial node, for the chain ops commands. -pub fn new_partial(config: &Configuration) -> Result { +pub fn new_partial( + config: &Configuration, + bitcoin_network: bitcoin::Network, +) -> Result { let telemetry = config .telemetry_endpoints .clone() @@ -517,7 +527,7 @@ pub fn new_partial(config: &Configuration) -> Result Result { + btc_header_verifier: HeaderVerifier, +} + +impl SubstrateImportQueueVerifier { + /// Constructs a new instance of [`SubstrateImportQueueVerifier`]. + pub fn new(client: Arc, network: bitcoin::Network) -> Self { + Self { + btc_header_verifier: HeaderVerifier::new(client, ChainParams::new(network)), + } + } +} #[async_trait::async_trait] -impl Verifier for SubstrateImportQueueVerifier { +impl Verifier for SubstrateImportQueueVerifier +where + Block: BlockT, + Client: HeaderBackend + AuxStore, +{ async fn verify( &self, mut block_import_params: BlockImportParams, ) -> Result, String> { - // TODO: Verify header. + let substrate_header = &block_import_params.header; + + let btc_header = + subcoin_primitives::extract_bitcoin_block_header::(substrate_header) + .map_err(|err| format!("Failed to extract bitcoin header: {err:?}"))?; + + self.btc_header_verifier + .verify(&btc_header) + .map_err(|err| format!("Invalid header: {err:?}"))?; block_import_params.fork_choice = Some(sc_consensus::ForkChoiceStrategy::LongestChain); let bitcoin_block_hash = - subcoin_primitives::extract_bitcoin_block_hash::(&block_import_params.header) + subcoin_primitives::extract_bitcoin_block_hash::(substrate_header) .map_err(|err| format!("Failed to extract bitcoin block hash: {err:?}"))?; - let substrate_block_hash = block_import_params.header.hash(); + let substrate_block_hash = substrate_header.hash(); sc_consensus_nakamoto::insert_bitcoin_block_hash_mapping::( &mut block_import_params,