From 7551aaeb2b3b0d7891a726e7c2e95cbed37894f1 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Wed, 23 Nov 2022 18:53:19 +0100 Subject: [PATCH 01/28] add backend --- bin/node/src/service.rs | 4 ++- finality-aleph/src/justification/handler.rs | 21 +++++------ finality-aleph/src/justification/requester.rs | 36 +++++++++++-------- finality-aleph/src/lib.rs | 3 +- finality-aleph/src/nodes/mod.rs | 16 +++++---- finality-aleph/src/nodes/nonvalidator_node.rs | 6 ++-- finality-aleph/src/nodes/validator_node.rs | 6 ++-- 7 files changed, 52 insertions(+), 40 deletions(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index 48549f4df0..a2088b08de 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -13,7 +13,7 @@ use finality_aleph::{ }; use futures::channel::mpsc; use log::warn; -use sc_client_api::ExecutorProvider; +use sc_client_api::{Backend, ExecutorProvider}; use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams}; use sc_network::NetworkService; use sc_service::{ @@ -283,6 +283,7 @@ pub fn new_authority( let backoff_authoring_blocks: Option<()> = None; let prometheus_registry = config.prometheus_registry().cloned(); + let blockchain_backend = backend.blockchain(); let (_rpc_handlers, network, network_starter) = setup( config, backend, @@ -348,6 +349,7 @@ pub fn new_authority( let aleph_config = AlephConfig { network, client, + blockchain_backend, select_chain, session_period, millisecs_per_block, diff --git a/finality-aleph/src/justification/handler.rs b/finality-aleph/src/justification/handler.rs index cf2b752549..09a08e9e76 100644 --- a/finality-aleph/src/justification/handler.rs +++ b/finality-aleph/src/justification/handler.rs @@ -1,12 +1,9 @@ -use std::{ - sync::Arc, - time::{Duration, Instant}, -}; +use std::time::{Duration, Instant}; use futures::{channel::mpsc, Stream, StreamExt}; use futures_timer::Delay; use log::{debug, error}; -use sc_client_api::HeaderBackend; +use sc_client_api::backend::Backend; use sp_api::BlockT; use sp_runtime::traits::Header; use tokio::time::timeout; @@ -20,36 +17,36 @@ use crate::{ network, Metrics, STATUS_REPORT_INTERVAL, }; -pub struct JustificationHandler +pub struct JustificationHandler where B: BlockT, V: Verifier, RB: network::RequestBlocks + 'static, - C: HeaderBackend + Send + Sync + 'static, S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, + BB: Backend + Send + Sync + 'static, { session_info_provider: SI, - block_requester: BlockRequester, + block_requester: BlockRequester, verifier_timeout: Duration, notification_timeout: Duration, } -impl JustificationHandler +impl JustificationHandler where B: BlockT, V: Verifier, RB: network::RequestBlocks + 'static, - C: HeaderBackend + Send + Sync + 'static, S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, + BB: Backend + Send + Sync + 'static, { pub fn new( session_info_provider: SI, block_requester: RB, - client: Arc, + backend: BB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -59,7 +56,7 @@ where session_info_provider, block_requester: BlockRequester::new( block_requester, - client, + backend, finalizer, justification_request_scheduler, metrics, diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 1e140f8c8d..603fdd0fd0 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -1,8 +1,8 @@ -use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; +use std::{fmt, marker::PhantomData, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; -use sc_client_api::HeaderBackend; +use sc_client_api::{backend::Backend, HeaderBackend}; use sp_api::{BlockId, BlockT, NumberFor}; use sp_runtime::traits::Header; @@ -72,17 +72,16 @@ impl fmt::Display for JustificationRequestStatus { } } -pub struct BlockRequester +pub struct BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, - C: HeaderBackend + Send + Sync + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, { block_requester: RB, - client: Arc, + backend: BB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -91,18 +90,18 @@ where _phantom: PhantomData, } -impl BlockRequester +impl BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, - C: HeaderBackend + Send + Sync + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, + BB: Backend, { pub fn new( block_requester: RB, - client: Arc, + backend: BB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -110,7 +109,7 @@ where ) -> Self { BlockRequester { block_requester, - client, + backend, finalizer, justification_request_scheduler, metrics, @@ -172,10 +171,17 @@ where pub fn request_justification(&mut self, num: NumberFor) { match self.justification_request_scheduler.schedule_action() { SchedulerActions::Request => { - let num = if num > self.client.info().best_number - && self.client.info().best_number > self.min_allowed_delay - { - self.client.info().best_number - self.min_allowed_delay + let finalized_hash = self.backend.blockchain().info().finalized_hash; + + let best_number = self.backend.blockchain().info().best_number; + let finalized_number = self.backend.blockchain().info().finalized_number; + let delay = if self.min_allowed_delay < (best_number - finalized_number) { + self.min_allowed_delay + } else { + best_number - finalized_number + }; + let num = if num > best_number && best_number > self.min_allowed_delay { + best_number - delay } else { num }; @@ -183,7 +189,7 @@ where debug!(target: "aleph-justification", "Trying to request block {:?}", num); self.request_status.save_block_number(num); - if let Ok(Some(header)) = self.client.header(BlockId::Number(num)) { + if let Ok(Some(header)) = self.backend.blockchain().header(BlockId::Number(num)) { self.request_status.insert_hash(header.hash()); debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", num, header.hash()); self.justification_request_scheduler.on_request_sent(); @@ -202,6 +208,6 @@ where } pub fn finalized_number(&self) -> NumberFor { - self.client.info().finalized_number + self.backend.blockchain().info().finalized_number } } diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs index dab38bbbe9..bd62d9914e 100644 --- a/finality-aleph/src/lib.rs +++ b/finality-aleph/src/lib.rs @@ -242,9 +242,10 @@ impl From<(H, N)> for HashNum { pub type BlockHashNum = HashNum<::Hash, NumberFor>; -pub struct AlephConfig { +pub struct AlephConfig { pub network: Arc>, pub client: Arc, + pub backend: BB, pub select_chain: SC, pub spawn_handle: SpawnTaskHandle, pub keystore: Arc, diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index d9e6d52e74..1da7b2bf36 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -7,7 +7,7 @@ use aleph_primitives::{AuthorityId, SessionAuthorityData}; use codec::Encode; use log::warn; pub use nonvalidator_node::run_nonvalidator_node; -use sc_client_api::Backend; +use sc_client_api::backend::Backend; use sc_network::{ExHashT, NetworkService}; use sp_runtime::{ traits::{Block, Header, NumberFor}, @@ -83,9 +83,10 @@ impl Verifier for JustificationVerifier { } } -struct JustificationParams { +struct JustificationParams { pub network: Arc>, pub client: Arc, + pub backend: BB, pub justification_rx: mpsc::UnboundedReceiver>, pub metrics: Option::Hash>>, pub session_period: SessionPeriod, @@ -126,8 +127,8 @@ impl SessionInfoProvider for SessionInfoProv } } -fn setup_justification_handler( - just_params: JustificationParams, +fn setup_justification_handler( + just_params: JustificationParams, ) -> ( UnboundedSender>, impl Future, @@ -135,13 +136,14 @@ fn setup_justification_handler( where B: Block, H: ExHashT, - C: crate::ClientForAleph + Send + Sync + 'static, + C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, - BE: Backend + 'static, + BB: Backend + 'static, { let JustificationParams { network, client, + backend, justification_rx, metrics, session_period, @@ -152,7 +154,7 @@ where let handler = JustificationHandler::new( SessionInfoProviderImpl::new(session_map, session_period), network, - client.clone(), + backend, AlephFinalizer::new(client), JustificationRequestSchedulerImpl::new(&session_period, &millisecs_per_block, MAX_ATTEMPTS), metrics, diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs index e70c03f6b3..487fc5714a 100644 --- a/finality-aleph/src/nodes/nonvalidator_node.rs +++ b/finality-aleph/src/nodes/nonvalidator_node.rs @@ -1,5 +1,5 @@ use log::{debug, error}; -use sc_client_api::Backend; +use sc_client_api::backend::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_runtime::traits::Block; @@ -10,7 +10,7 @@ use crate::{ AlephConfig, }; -pub async fn run_nonvalidator_node(aleph_config: AlephConfig) +pub async fn run_nonvalidator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, @@ -22,6 +22,7 @@ where let AlephConfig { network, client, + backend, metrics, session_period, millisecs_per_block, @@ -42,6 +43,7 @@ where justification_rx, network, client, + backend, metrics, session_period, millisecs_per_block, diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs index 6576ac2449..690e0dfc18 100644 --- a/finality-aleph/src/nodes/validator_node.rs +++ b/finality-aleph/src/nodes/validator_node.rs @@ -3,7 +3,7 @@ use std::{marker::PhantomData, sync::Arc}; use bip39::{Language, Mnemonic, MnemonicType}; use futures::channel::oneshot; use log::{debug, error}; -use sc_client_api::Backend; +use sc_client_api::backend::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_keystore::CryptoStore; @@ -37,7 +37,7 @@ pub async fn new_pen(mnemonic: &str, keystore: Arc) -> Authorit .expect("we just generated this key so everything should work") } -pub async fn run_validator_node(aleph_config: AlephConfig) +pub async fn run_validator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, @@ -49,6 +49,7 @@ where let AlephConfig { network, client, + backend, select_chain, spawn_handle, keystore, @@ -106,6 +107,7 @@ where justification_rx, network: network.clone(), client: client.clone(), + backend, metrics: metrics.clone(), session_period, millisecs_per_block, From d38352bd5e85799cc522806b67bd8c90929316a2 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Wed, 23 Nov 2022 21:14:30 +0100 Subject: [PATCH 02/28] request justification from forks --- bin/node/src/service.rs | 10 +- finality-aleph/src/justification/handler.rs | 14 +-- finality-aleph/src/justification/requester.rs | 109 ++++++++++++------ finality-aleph/src/lib.rs | 6 +- finality-aleph/src/nodes/mod.rs | 14 +-- finality-aleph/src/nodes/nonvalidator_node.rs | 2 +- finality-aleph/src/nodes/validator_node.rs | 2 +- 7 files changed, 99 insertions(+), 58 deletions(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index a2088b08de..742d0632d9 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -13,7 +13,7 @@ use finality_aleph::{ }; use futures::channel::mpsc; use log::warn; -use sc_client_api::{Backend, ExecutorProvider}; +use sc_client_api::{ExecutorProvider}; use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams}; use sc_network::NetworkService; use sc_service::{ @@ -283,10 +283,9 @@ pub fn new_authority( let backoff_authoring_blocks: Option<()> = None; let prometheus_registry = config.prometheus_registry().cloned(); - let blockchain_backend = backend.blockchain(); let (_rpc_handlers, network, network_starter) = setup( config, - backend, + backend.clone(), &keystore_container, import_queue, transaction_pool.clone(), @@ -349,7 +348,7 @@ pub fn new_authority( let aleph_config = AlephConfig { network, client, - blockchain_backend, + backend, select_chain, session_period, millisecs_per_block, @@ -398,7 +397,7 @@ pub fn new_full( let (_rpc_handlers, network, network_starter) = setup( config, - backend, + backend.clone(), &keystore_container, import_queue, transaction_pool, @@ -425,6 +424,7 @@ pub fn new_full( let aleph_config = AlephConfig { network, client, + backend, select_chain, session_period, millisecs_per_block, diff --git a/finality-aleph/src/justification/handler.rs b/finality-aleph/src/justification/handler.rs index 09a08e9e76..8bf0502ef5 100644 --- a/finality-aleph/src/justification/handler.rs +++ b/finality-aleph/src/justification/handler.rs @@ -1,4 +1,4 @@ -use std::time::{Duration, Instant}; +use std::{sync::Arc,time::{Duration, Instant}}; use futures::{channel::mpsc, Stream, StreamExt}; use futures_timer::Delay; @@ -17,7 +17,7 @@ use crate::{ network, Metrics, STATUS_REPORT_INTERVAL, }; -pub struct JustificationHandler +pub struct JustificationHandler where B: BlockT, V: Verifier, @@ -25,15 +25,15 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BB: Backend + Send + Sync + 'static, + BE: Backend + Send + Sync + 'static, { session_info_provider: SI, - block_requester: BlockRequester, + block_requester: BlockRequester, verifier_timeout: Duration, notification_timeout: Duration, } -impl JustificationHandler +impl JustificationHandler where B: BlockT, V: Verifier, @@ -41,12 +41,12 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BB: Backend + Send + Sync + 'static, + BE: Backend + Send + Sync + 'static, { pub fn new( session_info_provider: SI, block_requester: RB, - backend: BB, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 603fdd0fd0..f6be965c5a 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -1,8 +1,8 @@ -use std::{fmt, marker::PhantomData, time::Instant}; +use std::{sync::Arc, fmt, marker::PhantomData, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; -use sc_client_api::{backend::Backend, HeaderBackend}; +use sc_client_api::{backend::Backend, blockchain::Backend as _, HeaderBackend}; use sp_api::{BlockId, BlockT, NumberFor}; use sp_runtime::traits::Header; @@ -72,7 +72,7 @@ impl fmt::Display for JustificationRequestStatus { } } -pub struct BlockRequester +pub struct BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, @@ -81,7 +81,7 @@ where V: Verifier, { block_requester: RB, - backend: BB, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -90,18 +90,18 @@ where _phantom: PhantomData, } -impl BlockRequester +impl BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - BB: Backend, + BE: Backend, { pub fn new( block_requester: RB, - backend: BB, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -171,33 +171,9 @@ where pub fn request_justification(&mut self, num: NumberFor) { match self.justification_request_scheduler.schedule_action() { SchedulerActions::Request => { - let finalized_hash = self.backend.blockchain().info().finalized_hash; - - let best_number = self.backend.blockchain().info().best_number; - let finalized_number = self.backend.blockchain().info().finalized_number; - let delay = if self.min_allowed_delay < (best_number - finalized_number) { - self.min_allowed_delay - } else { - best_number - finalized_number - }; - let num = if num > best_number && best_number > self.min_allowed_delay { - best_number - delay - } else { - num - }; - - debug!(target: "aleph-justification", "Trying to request block {:?}", num); - self.request_status.save_block_number(num); - - if let Ok(Some(header)) = self.backend.blockchain().header(BlockId::Number(num)) { - self.request_status.insert_hash(header.hash()); - debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", num, header.hash()); - self.justification_request_scheduler.on_request_sent(); - self.block_requester - .request_justification(&header.hash(), *header.number()); - } else { - debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", num); - } + self.leaves(num) + .into_iter() + .for_each(|hash| self.request(hash)); } SchedulerActions::ClearQueue => { debug!(target: "aleph-justification", "Clearing justification request queue"); @@ -210,4 +186,69 @@ where pub fn finalized_number(&self) -> NumberFor { self.backend.blockchain().info().finalized_number } + + fn request(&mut self, header: ::Header) { + let number = *header.number(); + let hash = header.hash(); + + debug!(target: "aleph-justification", "Trying to request block {:?} {:?}", number, hash); + self.request_status.save_block_number(number); + + self.request_status.insert_hash(hash); + debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", number, hash); + self.justification_request_scheduler.on_request_sent(); + self.block_requester.request_justification(&hash, number); + } + + fn leaves(&self, limit: NumberFor) -> Vec<::Header> { + let blockchain_backend = self.backend.blockchain(); + let finalized_hash = blockchain_backend.info().finalized_hash; + let finalized_number = blockchain_backend.info().finalized_number; + + let ancestor = |hash: ::Hash| { + blockchain_backend + .header(BlockId::Hash(hash)) + .ok() + .flatten() + .map(|header| { + let number = *header.number(); + let mut ancestor = header; + let mut delay = if self.min_allowed_delay < (number - finalized_number) { + self.min_allowed_delay + } else { + number - finalized_number + }; + while delay > 0u32.into() { + let ancestor_hash = *ancestor.parent_hash(); + if let Ok(Some(header)) = + blockchain_backend.header(BlockId::Hash(ancestor_hash)) + { + ancestor = header; + delay -= 1u32.into(); + } else { + break; + } + } + ancestor + }) + }; + + let leaves = { + let lock = self.backend.get_import_lock(); + blockchain_backend + .children(finalized_hash) + .unwrap_or_default() + .iter() + .filter_map(|hash| { + blockchain_backend + .best_containing(*hash, Some(limit), lock) + .ok() + .flatten() + .and_then(ancestor) + }) + .collect() + }; + + leaves + } } diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs index bd62d9914e..66cd780176 100644 --- a/finality-aleph/src/lib.rs +++ b/finality-aleph/src/lib.rs @@ -8,7 +8,7 @@ use futures::{ channel::{mpsc, oneshot}, Future, }; -use sc_client_api::{backend::Backend, BlockchainEvents, Finalizer, LockImportRun, TransactionFor}; +use sc_client_api::{Backend, BlockchainEvents, Finalizer, LockImportRun, TransactionFor}; use sc_consensus::BlockImport; use sc_network::{ExHashT, NetworkService}; use sc_service::SpawnTaskHandle; @@ -242,10 +242,10 @@ impl From<(H, N)> for HashNum { pub type BlockHashNum = HashNum<::Hash, NumberFor>; -pub struct AlephConfig { +pub struct AlephConfig { pub network: Arc>, pub client: Arc, - pub backend: BB, + pub backend: Arc, pub select_chain: SC, pub spawn_handle: SpawnTaskHandle, pub keystore: Arc, diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index 1da7b2bf36..07c44fd60a 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -7,7 +7,7 @@ use aleph_primitives::{AuthorityId, SessionAuthorityData}; use codec::Encode; use log::warn; pub use nonvalidator_node::run_nonvalidator_node; -use sc_client_api::backend::Backend; +use sc_client_api::Backend; use sc_network::{ExHashT, NetworkService}; use sp_runtime::{ traits::{Block, Header, NumberFor}, @@ -83,10 +83,10 @@ impl Verifier for JustificationVerifier { } } -struct JustificationParams { +struct JustificationParams { pub network: Arc>, pub client: Arc, - pub backend: BB, + pub backend: Arc, pub justification_rx: mpsc::UnboundedReceiver>, pub metrics: Option::Hash>>, pub session_period: SessionPeriod, @@ -127,8 +127,8 @@ impl SessionInfoProvider for SessionInfoProv } } -fn setup_justification_handler( - just_params: JustificationParams, +fn setup_justification_handler( + just_params: JustificationParams, ) -> ( UnboundedSender>, impl Future, @@ -136,9 +136,9 @@ fn setup_justification_handler( where B: Block, H: ExHashT, - C: crate::ClientForAleph + Send + Sync + 'static, + C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, - BB: Backend + 'static, + BE: Backend + 'static, { let JustificationParams { network, diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs index 487fc5714a..b949d17f9c 100644 --- a/finality-aleph/src/nodes/nonvalidator_node.rs +++ b/finality-aleph/src/nodes/nonvalidator_node.rs @@ -1,5 +1,5 @@ use log::{debug, error}; -use sc_client_api::backend::Backend; +use sc_client_api::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_runtime::traits::Block; diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs index 690e0dfc18..90d9b05bf1 100644 --- a/finality-aleph/src/nodes/validator_node.rs +++ b/finality-aleph/src/nodes/validator_node.rs @@ -3,7 +3,7 @@ use std::{marker::PhantomData, sync::Arc}; use bip39::{Language, Mnemonic, MnemonicType}; use futures::channel::oneshot; use log::{debug, error}; -use sc_client_api::backend::Backend; +use sc_client_api::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_keystore::CryptoStore; From 561a8e2fa97f1776fa4b329f036ecdd12fa25ee4 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 11:00:22 +0100 Subject: [PATCH 03/28] change requesting logic --- finality-aleph/src/justification/handler.rs | 8 +- finality-aleph/src/justification/mod.rs | 18 +-- finality-aleph/src/justification/requester.rs | 109 ++++++++---------- 3 files changed, 55 insertions(+), 80 deletions(-) diff --git a/finality-aleph/src/justification/handler.rs b/finality-aleph/src/justification/handler.rs index 8bf0502ef5..f8ac7207b5 100644 --- a/finality-aleph/src/justification/handler.rs +++ b/finality-aleph/src/justification/handler.rs @@ -1,4 +1,7 @@ -use std::{sync::Arc,time::{Duration, Instant}}; +use std::{ + sync::Arc, + time::{Duration, Instant}, +}; use futures::{channel::mpsc, Stream, StreamExt}; use futures_timer::Delay; @@ -50,7 +53,7 @@ where finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, - justification_handler_config: JustificationHandlerConfig, + justification_handler_config: JustificationHandlerConfig, ) -> Self { Self { session_info_provider, @@ -60,7 +63,6 @@ where finalizer, justification_request_scheduler, metrics, - justification_handler_config.min_allowed_delay, ), verifier_timeout: justification_handler_config.verifier_timeout, notification_timeout: justification_handler_config.notification_timeout, diff --git a/finality-aleph/src/justification/mod.rs b/finality-aleph/src/justification/mod.rs index fcc16bf9b5..2efeb6b34b 100644 --- a/finality-aleph/src/justification/mod.rs +++ b/finality-aleph/src/justification/mod.rs @@ -55,36 +55,28 @@ pub struct JustificationNotification { } #[derive(Clone)] -pub struct JustificationHandlerConfig { +pub struct JustificationHandlerConfig { /// How long should we wait when the session verifier is not yet available. verifier_timeout: Duration, /// How long should we wait for any notification. notification_timeout: Duration, - ///Distance (in amount of blocks) between the best and the block we want to request justification - min_allowed_delay: NumberFor, } -impl Default for JustificationHandlerConfig { +impl Default for JustificationHandlerConfig { fn default() -> Self { Self { verifier_timeout: Duration::from_millis(500), - notification_timeout: Duration::from_millis(1000), - min_allowed_delay: 3u32.into(), + notification_timeout: Duration::from_millis(800), } } } #[cfg(test)] -impl JustificationHandlerConfig { - pub fn new( - verifier_timeout: Duration, - notification_timeout: Duration, - min_allowed_delay: NumberFor, - ) -> Self { +impl JustificationHandlerConfig { + pub fn new(verifier_timeout: Duration, notification_timeout: Duration) -> Self { Self { verifier_timeout, notification_timeout, - min_allowed_delay, } } } diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index f6be965c5a..004b97760e 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -1,4 +1,4 @@ -use std::{sync::Arc, fmt, marker::PhantomData, time::Instant}; +use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; @@ -79,13 +79,13 @@ where S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, + BE: Backend, { block_requester: RB, backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, - min_allowed_delay: NumberFor, request_status: JustificationRequestStatus, _phantom: PhantomData, } @@ -105,7 +105,6 @@ where finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, - min_allowed_delay: NumberFor, ) -> Self { BlockRequester { block_requester, @@ -113,7 +112,6 @@ where finalizer, justification_request_scheduler, metrics, - min_allowed_delay, request_status: JustificationRequestStatus::new(), _phantom: PhantomData, } @@ -171,7 +169,7 @@ where pub fn request_justification(&mut self, num: NumberFor) { match self.justification_request_scheduler.schedule_action() { SchedulerActions::Request => { - self.leaves(num) + self.request_targets(num) .into_iter() .for_each(|hash| self.request(hash)); } @@ -187,68 +185,51 @@ where self.backend.blockchain().info().finalized_number } - fn request(&mut self, header: ::Header) { - let number = *header.number(); - let hash = header.hash(); - - debug!(target: "aleph-justification", "Trying to request block {:?} {:?}", number, hash); - self.request_status.save_block_number(number); - - self.request_status.insert_hash(hash); - debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", number, hash); - self.justification_request_scheduler.on_request_sent(); - self.block_requester.request_justification(&hash, number); + fn request(&mut self, hash: ::Hash) { + if let Ok(Some(header)) = self.backend.blockchain().header(BlockId::Hash(hash)) { + let number = *header.number(); + debug!(target: "aleph-justification", "Trying to request block {:?}", number); + self.request_status.save_block_number(number); + self.request_status.insert_hash(hash); + debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", number, header.hash()); + self.justification_request_scheduler.on_request_sent(); + self.block_requester.request_justification(&hash, number); + } else { + debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", hash); + } } - fn leaves(&self, limit: NumberFor) -> Vec<::Header> { + // We request justifications for all the children of last finalized block and a justification + // for a block of number num on longest branch. + // Assuming that we request at the same pace that finalization is progressing, the former ensures + // that we are up to date with finalization. On the other hand, the former enables fast catch up + // if we are behind. + // We don't remove the child that it's on the same branch as best since a fork may happen + // somewhere in between them. + fn request_targets(&self, mut num: NumberFor) -> Vec<::Hash> { let blockchain_backend = self.backend.blockchain(); - let finalized_hash = blockchain_backend.info().finalized_hash; - let finalized_number = blockchain_backend.info().finalized_number; - - let ancestor = |hash: ::Hash| { - blockchain_backend - .header(BlockId::Hash(hash)) - .ok() - .flatten() - .map(|header| { - let number = *header.number(); - let mut ancestor = header; - let mut delay = if self.min_allowed_delay < (number - finalized_number) { - self.min_allowed_delay - } else { - number - finalized_number - }; - while delay > 0u32.into() { - let ancestor_hash = *ancestor.parent_hash(); - if let Ok(Some(header)) = - blockchain_backend.header(BlockId::Hash(ancestor_hash)) - { - ancestor = header; - delay -= 1u32.into(); - } else { - break; - } - } - ancestor - }) - }; - - let leaves = { - let lock = self.backend.get_import_lock(); - blockchain_backend - .children(finalized_hash) - .unwrap_or_default() - .iter() - .filter_map(|hash| { - blockchain_backend - .best_containing(*hash, Some(limit), lock) - .ok() - .flatten() - .and_then(ancestor) - }) - .collect() - }; + let blockchain_info = blockchain_backend.info(); + let finalized_hash = blockchain_info.finalized_hash; + + let mut targets = blockchain_backend + .children(finalized_hash) + .unwrap_or_default(); + let best_number = blockchain_info.best_number; + if best_number < num { + num = best_number; + } + match blockchain_backend.hash(num) { + Ok(Some(hash)) => { + targets.push(hash); + } + Ok(None) => { + debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", num); + } + Err(err) => { + debug!(target: "aleph-justification", "Cancelling request, because fetching block {:?} failed {:?}.", num, err); + } + } - leaves + targets } } From 5a1319068b38e3a17b4e58b6b1d21d41eada3d5f Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 11:24:11 +0100 Subject: [PATCH 04/28] Request penultimate block --- finality-aleph/src/justification/requester.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 004b97760e..cad08a74a8 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -216,7 +216,8 @@ where .unwrap_or_default(); let best_number = blockchain_info.best_number; if best_number < num { - num = best_number; + // most probably block best_number is not yet finalized + num = best_number - 1; } match blockchain_backend.hash(num) { Ok(Some(hash)) => { From 8228585a997d81e83a2fdcb0e522d1d05f5369a7 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 11:58:21 +0100 Subject: [PATCH 05/28] review changes --- finality-aleph/src/justification/mod.rs | 1 + finality-aleph/src/justification/requester.rs | 14 ++++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/finality-aleph/src/justification/mod.rs b/finality-aleph/src/justification/mod.rs index 2efeb6b34b..f5cde9d497 100644 --- a/finality-aleph/src/justification/mod.rs +++ b/finality-aleph/src/justification/mod.rs @@ -66,6 +66,7 @@ impl Default for JustificationHandlerConfig { fn default() -> Self { Self { verifier_timeout: Duration::from_millis(500), + // request justifications slightly more frequently than they're created notification_timeout: Duration::from_millis(800), } } diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index cad08a74a8..a99c3afe8d 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -4,7 +4,7 @@ use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; use sc_client_api::{backend::Backend, blockchain::Backend as _, HeaderBackend}; use sp_api::{BlockId, BlockT, NumberFor}; -use sp_runtime::traits::Header; +use sp_runtime::traits::{Header, One}; use crate::{ finalization::BlockFinalizer, @@ -206,7 +206,7 @@ where // if we are behind. // We don't remove the child that it's on the same branch as best since a fork may happen // somewhere in between them. - fn request_targets(&self, mut num: NumberFor) -> Vec<::Hash> { + fn request_targets(&self, mut top_wanted: NumberFor) -> Vec<::Hash> { let blockchain_backend = self.backend.blockchain(); let blockchain_info = blockchain_backend.info(); let finalized_hash = blockchain_info.finalized_hash; @@ -215,13 +215,15 @@ where .children(finalized_hash) .unwrap_or_default(); let best_number = blockchain_info.best_number; - if best_number < num { + if best_number <= top_wanted { // most probably block best_number is not yet finalized - num = best_number - 1; + top_wanted = best_number - NumberFor::::one(); } - match blockchain_backend.hash(num) { + match blockchain_backend.hash(top_wanted) { Ok(Some(hash)) => { - targets.push(hash); + if !targets.contains(&hash) { + targets.push(hash); + } } Ok(None) => { debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", num); From 8e1249abdc810595a8ee7d00111ea6dfd28f962d Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 11:59:56 +0100 Subject: [PATCH 06/28] fix rename leftover --- finality-aleph/src/justification/requester.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index a99c3afe8d..5b7e01b591 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -226,10 +226,10 @@ where } } Ok(None) => { - debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", num); + debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", top_wanted); } Err(err) => { - debug!(target: "aleph-justification", "Cancelling request, because fetching block {:?} failed {:?}.", num, err); + debug!(target: "aleph-justification", "Cancelling request, because fetching block {:?} failed {:?}.", top_wanted, err); } } From abf26055b33cde4770d60f34a0ed88439adebcce Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 12:35:29 +0100 Subject: [PATCH 07/28] simplify backend dependecy --- finality-aleph/src/justification/handler.rs | 14 +++++----- finality-aleph/src/justification/requester.rs | 27 +++++++++---------- finality-aleph/src/nodes/mod.rs | 10 ++++--- finality-aleph/src/nodes/nonvalidator_node.rs | 4 ++- finality-aleph/src/nodes/validator_node.rs | 4 ++- 5 files changed, 31 insertions(+), 28 deletions(-) diff --git a/finality-aleph/src/justification/handler.rs b/finality-aleph/src/justification/handler.rs index f8ac7207b5..8528039b23 100644 --- a/finality-aleph/src/justification/handler.rs +++ b/finality-aleph/src/justification/handler.rs @@ -6,7 +6,7 @@ use std::{ use futures::{channel::mpsc, Stream, StreamExt}; use futures_timer::Delay; use log::{debug, error}; -use sc_client_api::backend::Backend; +use sc_client_api::blockchain::Backend as BlockchainBackend; use sp_api::BlockT; use sp_runtime::traits::Header; use tokio::time::timeout; @@ -20,7 +20,7 @@ use crate::{ network, Metrics, STATUS_REPORT_INTERVAL, }; -pub struct JustificationHandler +pub struct JustificationHandler where B: BlockT, V: Verifier, @@ -28,15 +28,15 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BE: Backend + Send + Sync + 'static, + BB: BlockchainBackend + Send + Sync + 'static, { session_info_provider: SI, - block_requester: BlockRequester, + block_requester: BlockRequester, verifier_timeout: Duration, notification_timeout: Duration, } -impl JustificationHandler +impl JustificationHandler where B: BlockT, V: Verifier, @@ -44,12 +44,12 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BE: Backend + Send + Sync + 'static, + BB: BlockchainBackend + Send + Sync + 'static, { pub fn new( session_info_provider: SI, block_requester: RB, - backend: Arc, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 5b7e01b591..58fe215019 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -2,7 +2,7 @@ use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; -use sc_client_api::{backend::Backend, blockchain::Backend as _, HeaderBackend}; +use sc_client_api::{blockchain::Backend as BlockchainBackend, HeaderBackend}; use sp_api::{BlockId, BlockT, NumberFor}; use sp_runtime::traits::{Header, One}; @@ -72,17 +72,17 @@ impl fmt::Display for JustificationRequestStatus { } } -pub struct BlockRequester +pub struct BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - BE: Backend, + BB: BlockchainBackend, { block_requester: RB, - backend: Arc, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -90,18 +90,18 @@ where _phantom: PhantomData, } -impl BlockRequester +impl BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - BE: Backend, + BB: BlockchainBackend, { pub fn new( block_requester: RB, - backend: Arc, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -182,11 +182,11 @@ where } pub fn finalized_number(&self) -> NumberFor { - self.backend.blockchain().info().finalized_number + self.backend.info().finalized_number } fn request(&mut self, hash: ::Hash) { - if let Ok(Some(header)) = self.backend.blockchain().header(BlockId::Hash(hash)) { + if let Ok(Some(header)) = self.backend.header(BlockId::Hash(hash)) { let number = *header.number(); debug!(target: "aleph-justification", "Trying to request block {:?}", number); self.request_status.save_block_number(number); @@ -207,19 +207,16 @@ where // We don't remove the child that it's on the same branch as best since a fork may happen // somewhere in between them. fn request_targets(&self, mut top_wanted: NumberFor) -> Vec<::Hash> { - let blockchain_backend = self.backend.blockchain(); - let blockchain_info = blockchain_backend.info(); + let blockchain_info = self.backend.info(); let finalized_hash = blockchain_info.finalized_hash; - let mut targets = blockchain_backend - .children(finalized_hash) - .unwrap_or_default(); + let mut targets = self.backend.children(finalized_hash).unwrap_or_default(); let best_number = blockchain_info.best_number; if best_number <= top_wanted { // most probably block best_number is not yet finalized top_wanted = best_number - NumberFor::::one(); } - match blockchain_backend.hash(top_wanted) { + match self.backend.hash(top_wanted) { Ok(Some(hash)) => { if !targets.contains(&hash) { targets.push(hash); diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index 07c44fd60a..5827c81570 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -7,6 +7,7 @@ use aleph_primitives::{AuthorityId, SessionAuthorityData}; use codec::Encode; use log::warn; pub use nonvalidator_node::run_nonvalidator_node; +use sc_client_api::blockchain::Backend as BlockchainBackend; use sc_client_api::Backend; use sc_network::{ExHashT, NetworkService}; use sp_runtime::{ @@ -83,10 +84,10 @@ impl Verifier for JustificationVerifier { } } -struct JustificationParams { +struct JustificationParams { pub network: Arc>, pub client: Arc, - pub backend: Arc, + pub backend: Arc, pub justification_rx: mpsc::UnboundedReceiver>, pub metrics: Option::Hash>>, pub session_period: SessionPeriod, @@ -127,8 +128,8 @@ impl SessionInfoProvider for SessionInfoProv } } -fn setup_justification_handler( - just_params: JustificationParams, +fn setup_justification_handler( + just_params: JustificationParams, ) -> ( UnboundedSender>, impl Future, @@ -138,6 +139,7 @@ where H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, + BB: BlockchainBackend + 'static, BE: Backend + 'static, { let JustificationParams { diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs index b949d17f9c..dceaae4f5f 100644 --- a/finality-aleph/src/nodes/nonvalidator_node.rs +++ b/finality-aleph/src/nodes/nonvalidator_node.rs @@ -1,4 +1,5 @@ use log::{debug, error}; +use sc_client_api::blockchain::Backend as BlockchainBackend; use sc_client_api::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; @@ -10,12 +11,13 @@ use crate::{ AlephConfig, }; -pub async fn run_nonvalidator_node(aleph_config: AlephConfig) +pub async fn run_nonvalidator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, + BB: BlockchainBackend + 'static, BE: Backend + 'static, SC: SelectChain + 'static, { diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs index 90d9b05bf1..82b4a14c8d 100644 --- a/finality-aleph/src/nodes/validator_node.rs +++ b/finality-aleph/src/nodes/validator_node.rs @@ -3,6 +3,7 @@ use std::{marker::PhantomData, sync::Arc}; use bip39::{Language, Mnemonic, MnemonicType}; use futures::channel::oneshot; use log::{debug, error}; +use sc_client_api::blockchain::Backend as BlockchainBackend; use sc_client_api::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; @@ -37,12 +38,13 @@ pub async fn new_pen(mnemonic: &str, keystore: Arc) -> Authorit .expect("we just generated this key so everything should work") } -pub async fn run_validator_node(aleph_config: AlephConfig) +pub async fn run_validator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, + BB: BlockchainBackend + 'static, BE: Backend + 'static, SC: SelectChain + 'static, { From 7b3953971f492e6bc5d1d01a3a887cfb0f4ad758 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 14:13:22 +0100 Subject: [PATCH 08/28] Rework justification request status --- bin/node/src/service.rs | 12 +- finality-aleph/src/justification/requester.rs | 119 ++++++++++++------ finality-aleph/src/nodes/mod.rs | 3 +- finality-aleph/src/nodes/nonvalidator_node.rs | 3 +- finality-aleph/src/nodes/validator_node.rs | 3 +- .../mocks/justification_handler_config.rs | 3 +- 6 files changed, 93 insertions(+), 50 deletions(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index 742d0632d9..8d93a25186 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -13,7 +13,7 @@ use finality_aleph::{ }; use futures::channel::mpsc; use log::warn; -use sc_client_api::{ExecutorProvider}; +use sc_client_api::ExecutorProvider; use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams}; use sc_network::NetworkService; use sc_service::{ @@ -283,9 +283,10 @@ pub fn new_authority( let backoff_authoring_blocks: Option<()> = None; let prometheus_registry = config.prometheus_registry().cloned(); + let blockchain_backed = backend.blockchain(); let (_rpc_handlers, network, network_starter) = setup( config, - backend.clone(), + backend, &keystore_container, import_queue, transaction_pool.clone(), @@ -348,7 +349,7 @@ pub fn new_authority( let aleph_config = AlephConfig { network, client, - backend, + blockchain_backend, select_chain, session_period, millisecs_per_block, @@ -395,9 +396,10 @@ pub fn new_full( .path(), ); + let blockchain_backed = backend.blockchain(); let (_rpc_handlers, network, network_starter) = setup( config, - backend.clone(), + backend, &keystore_container, import_queue, transaction_pool, @@ -424,7 +426,7 @@ pub fn new_full( let aleph_config = AlephConfig { network, client, - backend, + blockchain_backend, select_chain, session_period, millisecs_per_block, diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 58fe215019..3aa6da88a5 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -2,7 +2,7 @@ use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; -use sc_client_api::{blockchain::Backend as BlockchainBackend, HeaderBackend}; +use sc_client_api::blockchain::Backend as BlockchainBackend; use sp_api::{BlockId, BlockT, NumberFor}; use sp_runtime::traits::{Header, One}; @@ -23,7 +23,10 @@ const REPORT_THRESHOLD: u32 = 2; pub struct JustificationRequestStatus { block_number: Option>, block_hash: Option, - tries: u32, + block_tries: u32, + parent: Option, + n_childred: Option, + children_tries: u32, report_threshold: u32, } @@ -32,41 +35,69 @@ impl JustificationRequestStatus { Self { block_number: None, block_hash: None, - tries: 0, + block_tries: 0, + parent: None, + n_childred: None, + children_tries: 0, report_threshold: REPORT_THRESHOLD, } } - fn save_block_number(&mut self, num: NumberFor) { - if Some(num) == self.block_number { - self.tries += 1; + fn save_children(&mut self, hash: B::Hash, n_childred: usize) { + if self.parent == Some(hash) { + self.children_tries += 1; } else { - self.block_number = Some(num); - self.block_hash = None; - self.tries = 1; + self.parent = Some(hash); + self.n_childred = Some(n_childred); + self.children_tries = 1; } } - fn insert_hash(&mut self, hash: B::Hash) { - self.block_hash = Some(hash); + fn save_block(&mut self, num: NumberFor, hash: B::Hash) { + if self.block_number == Some(num) { + self.block_tries += 1; + } else { + self.block_hash = Some(hash); + self.block_number = Some(num); + self.block_hash = None; + self.block_tries = 1; + } } fn should_report(&self) -> bool { - self.tries >= self.report_threshold + self.block_tries >= self.report_threshold || self.children_tries >= self.report_threshold } } impl fmt::Display for JustificationRequestStatus { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if let Some(n) = self.block_number { - let mut status = format!("tries - {}; requested block number - {}; ", self.tries, n); - if let Some(header) = self.block_hash { - status.push_str(&format!("hash - {}; ", header)); - } else { - status.push_str("hash - unknown; "); + if self.block_tries >= self.report_threshold { + if let Some(n) = self.block_number { + let mut status = format!( + "tries - {}; requested block number - {}; ", + self.block_tries, n + ); + if let Some(header) = self.block_hash { + status.push_str(&format!("hash - {}; ", header)); + } else { + status.push_str("hash - unknown; "); + } + + write!(f, "{}", status)?; } + } + if self.children_tries >= self.report_threshold { + if let Some(parent) = self.parent { + let n = self + .n_childred + .expect("Children number is saved with parent."); - write!(f, "{}", status)?; + write!( + f, + "tries - {}; requested {} children of finalized block {}; ", + self.children_tries, n, parent + )?; + } } Ok(()) } @@ -185,18 +216,13 @@ where self.backend.info().finalized_number } - fn request(&mut self, hash: ::Hash) { - if let Ok(Some(header)) = self.backend.header(BlockId::Hash(hash)) { - let number = *header.number(); - debug!(target: "aleph-justification", "Trying to request block {:?}", number); - self.request_status.save_block_number(number); - self.request_status.insert_hash(hash); - debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", number, header.hash()); - self.justification_request_scheduler.on_request_sent(); - self.block_requester.request_justification(&hash, number); - } else { - debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", hash); - } + fn request(&mut self, header: ::Header) { + let number = *header.number(); + let hash = header.hash(); + debug!(target: "aleph-justification", + "We have block {:?} with hash {:?}. Requesting justification.", number, hash); + self.justification_request_scheduler.on_request_sent(); + self.block_requester.request_justification(&hash, number); } // We request justifications for all the children of last finalized block and a justification @@ -206,20 +232,39 @@ where // if we are behind. // We don't remove the child that it's on the same branch as best since a fork may happen // somewhere in between them. - fn request_targets(&self, mut top_wanted: NumberFor) -> Vec<::Hash> { + fn request_targets(&mut self, mut top_wanted: NumberFor) -> Vec<::Header> { let blockchain_info = self.backend.info(); let finalized_hash = blockchain_info.finalized_hash; - let mut targets = self.backend.children(finalized_hash).unwrap_or_default(); + let mut targets = self + .backend + .children(finalized_hash) + .unwrap_or_default() + .into_iter() + .filter_map(|hash| { + if let Ok(Some(header)) = self.backend.header(BlockId::Hash(hash)) { + Some(header) + } else { + debug!(target: "aleph-justification", + "Cancelling request for child {:?} of {:?}: no block.", hash, finalized_hash); + None + } + }) + .collect::>(); + self.request_status + .save_children(finalized_hash, targets.len()); + let best_number = blockchain_info.best_number; if best_number <= top_wanted { // most probably block best_number is not yet finalized top_wanted = best_number - NumberFor::::one(); } - match self.backend.hash(top_wanted) { - Ok(Some(hash)) => { - if !targets.contains(&hash) { - targets.push(hash); + match self.backend.header(BlockId::Number(top_wanted)) { + Ok(Some(header)) => { + if !targets.contains(&header) { + self.request_status + .save_block(*header.number(), header.hash()); + targets.push(header); } } Ok(None) => { diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index 5827c81570..a20859d4c2 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -7,8 +7,7 @@ use aleph_primitives::{AuthorityId, SessionAuthorityData}; use codec::Encode; use log::warn; pub use nonvalidator_node::run_nonvalidator_node; -use sc_client_api::blockchain::Backend as BlockchainBackend; -use sc_client_api::Backend; +use sc_client_api::{blockchain::Backend as BlockchainBackend, Backend}; use sc_network::{ExHashT, NetworkService}; use sp_runtime::{ traits::{Block, Header, NumberFor}, diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs index dceaae4f5f..38e807048a 100644 --- a/finality-aleph/src/nodes/nonvalidator_node.rs +++ b/finality-aleph/src/nodes/nonvalidator_node.rs @@ -1,6 +1,5 @@ use log::{debug, error}; -use sc_client_api::blockchain::Backend as BlockchainBackend; -use sc_client_api::Backend; +use sc_client_api::{blockchain::Backend as BlockchainBackend, Backend}; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_runtime::traits::Block; diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs index 82b4a14c8d..f9f7918ad8 100644 --- a/finality-aleph/src/nodes/validator_node.rs +++ b/finality-aleph/src/nodes/validator_node.rs @@ -3,8 +3,7 @@ use std::{marker::PhantomData, sync::Arc}; use bip39::{Language, Mnemonic, MnemonicType}; use futures::channel::oneshot; use log::{debug, error}; -use sc_client_api::blockchain::Backend as BlockchainBackend; -use sc_client_api::Backend; +use sc_client_api::{blockchain::Backend as BlockchainBackend, Backend}; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_keystore::CryptoStore; diff --git a/finality-aleph/src/testing/mocks/justification_handler_config.rs b/finality-aleph/src/testing/mocks/justification_handler_config.rs index 49dee9543e..5bea9fc419 100644 --- a/finality-aleph/src/testing/mocks/justification_handler_config.rs +++ b/finality-aleph/src/testing/mocks/justification_handler_config.rs @@ -58,12 +58,11 @@ impl JustificationRequestScheduler for JustificationRequestSchedulerImpl { const DEFAULT_VERIFIER_TIMEOUT_MS: u64 = 10u64; const DEFAULT_NOTIFICATION_TIMEOUT_MS: u64 = 10u64; -impl JustificationHandlerConfig { +impl JustificationHandlerConfig { pub fn test() -> Self { JustificationHandlerConfig::new( Duration::from_millis(DEFAULT_VERIFIER_TIMEOUT_MS), Duration::from_millis(DEFAULT_NOTIFICATION_TIMEOUT_MS), - 3u32.into(), ) } } From 754987305b34e1223d6e5027e753ccabf93d0446 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 14:35:46 +0100 Subject: [PATCH 09/28] Revert "simplify backend dependecy" This reverts commit abf26055b33cde4770d60f34a0ed88439adebcce. --- finality-aleph/src/justification/handler.rs | 14 +++--- finality-aleph/src/justification/requester.rs | 44 +++++++------------ finality-aleph/src/nodes/mod.rs | 11 +++-- finality-aleph/src/nodes/nonvalidator_node.rs | 5 +-- finality-aleph/src/nodes/validator_node.rs | 5 +-- 5 files changed, 31 insertions(+), 48 deletions(-) diff --git a/finality-aleph/src/justification/handler.rs b/finality-aleph/src/justification/handler.rs index 8528039b23..f8ac7207b5 100644 --- a/finality-aleph/src/justification/handler.rs +++ b/finality-aleph/src/justification/handler.rs @@ -6,7 +6,7 @@ use std::{ use futures::{channel::mpsc, Stream, StreamExt}; use futures_timer::Delay; use log::{debug, error}; -use sc_client_api::blockchain::Backend as BlockchainBackend; +use sc_client_api::backend::Backend; use sp_api::BlockT; use sp_runtime::traits::Header; use tokio::time::timeout; @@ -20,7 +20,7 @@ use crate::{ network, Metrics, STATUS_REPORT_INTERVAL, }; -pub struct JustificationHandler +pub struct JustificationHandler where B: BlockT, V: Verifier, @@ -28,15 +28,15 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BB: BlockchainBackend + Send + Sync + 'static, + BE: Backend + Send + Sync + 'static, { session_info_provider: SI, - block_requester: BlockRequester, + block_requester: BlockRequester, verifier_timeout: Duration, notification_timeout: Duration, } -impl JustificationHandler +impl JustificationHandler where B: BlockT, V: Verifier, @@ -44,12 +44,12 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BB: BlockchainBackend + Send + Sync + 'static, + BE: Backend + Send + Sync + 'static, { pub fn new( session_info_provider: SI, block_requester: RB, - backend: Arc, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 3aa6da88a5..e814cb0555 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -2,7 +2,7 @@ use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; -use sc_client_api::blockchain::Backend as BlockchainBackend; +use sc_client_api::{backend::Backend, blockchain::Backend as _, HeaderBackend}; use sp_api::{BlockId, BlockT, NumberFor}; use sp_runtime::traits::{Header, One}; @@ -103,17 +103,17 @@ impl fmt::Display for JustificationRequestStatus { } } -pub struct BlockRequester +pub struct BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - BB: BlockchainBackend, + BE: Backend, { block_requester: RB, - backend: Arc, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -121,18 +121,18 @@ where _phantom: PhantomData, } -impl BlockRequester +impl BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - BB: BlockchainBackend, + BE: Backend, { pub fn new( block_requester: RB, - backend: Arc, + backend: Arc, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -202,7 +202,7 @@ where SchedulerActions::Request => { self.request_targets(num) .into_iter() - .for_each(|hash| self.request(hash)); + .for_each(|header| self.request(header)); } SchedulerActions::ClearQueue => { debug!(target: "aleph-justification", "Clearing justification request queue"); @@ -213,7 +213,7 @@ where } pub fn finalized_number(&self) -> NumberFor { - self.backend.info().finalized_number + self.backend.blockchain().info().finalized_number } fn request(&mut self, header: ::Header) { @@ -232,34 +232,20 @@ where // if we are behind. // We don't remove the child that it's on the same branch as best since a fork may happen // somewhere in between them. - fn request_targets(&mut self, mut top_wanted: NumberFor) -> Vec<::Header> { - let blockchain_info = self.backend.info(); + fn request_targets(&self, mut top_wanted: NumberFor) -> Vec<::Header> { + let blockchain_backend = self.backend.blockchain(); + let blockchain_info = blockchain_backend.info(); let finalized_hash = blockchain_info.finalized_hash; - let mut targets = self - .backend + let mut targets = blockchain_backend .children(finalized_hash) - .unwrap_or_default() - .into_iter() - .filter_map(|hash| { - if let Ok(Some(header)) = self.backend.header(BlockId::Hash(hash)) { - Some(header) - } else { - debug!(target: "aleph-justification", - "Cancelling request for child {:?} of {:?}: no block.", hash, finalized_hash); - None - } - }) - .collect::>(); - self.request_status - .save_children(finalized_hash, targets.len()); - + .unwrap_or_default(); let best_number = blockchain_info.best_number; if best_number <= top_wanted { // most probably block best_number is not yet finalized top_wanted = best_number - NumberFor::::one(); } - match self.backend.header(BlockId::Number(top_wanted)) { + match blockchain_backend.header(BlockId::Number(top_wanted)) { Ok(Some(header)) => { if !targets.contains(&header) { self.request_status diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index a20859d4c2..d02e46a262 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -7,7 +7,7 @@ use aleph_primitives::{AuthorityId, SessionAuthorityData}; use codec::Encode; use log::warn; pub use nonvalidator_node::run_nonvalidator_node; -use sc_client_api::{blockchain::Backend as BlockchainBackend, Backend}; +use sc_client_api::{Backend}; use sc_network::{ExHashT, NetworkService}; use sp_runtime::{ traits::{Block, Header, NumberFor}, @@ -83,10 +83,10 @@ impl Verifier for JustificationVerifier { } } -struct JustificationParams { +struct JustificationParams { pub network: Arc>, pub client: Arc, - pub backend: Arc, + pub backend: Arc, pub justification_rx: mpsc::UnboundedReceiver>, pub metrics: Option::Hash>>, pub session_period: SessionPeriod, @@ -127,8 +127,8 @@ impl SessionInfoProvider for SessionInfoProv } } -fn setup_justification_handler( - just_params: JustificationParams, +fn setup_justification_handler( + just_params: JustificationParams, ) -> ( UnboundedSender>, impl Future, @@ -138,7 +138,6 @@ where H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, - BB: BlockchainBackend + 'static, BE: Backend + 'static, { let JustificationParams { diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs index 38e807048a..b949d17f9c 100644 --- a/finality-aleph/src/nodes/nonvalidator_node.rs +++ b/finality-aleph/src/nodes/nonvalidator_node.rs @@ -1,5 +1,5 @@ use log::{debug, error}; -use sc_client_api::{blockchain::Backend as BlockchainBackend, Backend}; +use sc_client_api::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_runtime::traits::Block; @@ -10,13 +10,12 @@ use crate::{ AlephConfig, }; -pub async fn run_nonvalidator_node(aleph_config: AlephConfig) +pub async fn run_nonvalidator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, - BB: BlockchainBackend + 'static, BE: Backend + 'static, SC: SelectChain + 'static, { diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs index f9f7918ad8..90d9b05bf1 100644 --- a/finality-aleph/src/nodes/validator_node.rs +++ b/finality-aleph/src/nodes/validator_node.rs @@ -3,7 +3,7 @@ use std::{marker::PhantomData, sync::Arc}; use bip39::{Language, Mnemonic, MnemonicType}; use futures::channel::oneshot; use log::{debug, error}; -use sc_client_api::{blockchain::Backend as BlockchainBackend, Backend}; +use sc_client_api::Backend; use sc_network::ExHashT; use sp_consensus::SelectChain; use sp_keystore::CryptoStore; @@ -37,13 +37,12 @@ pub async fn new_pen(mnemonic: &str, keystore: Arc) -> Authorit .expect("we just generated this key so everything should work") } -pub async fn run_validator_node(aleph_config: AlephConfig) +pub async fn run_validator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, - BB: BlockchainBackend + 'static, BE: Backend + 'static, SC: SelectChain + 'static, { From 5e844eb6f8d9e3a89ff5bfc53662c2d648d9cff2 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 14:39:45 +0100 Subject: [PATCH 10/28] fix revert --- finality-aleph/src/justification/requester.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index e814cb0555..b3ba1b73da 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -232,14 +232,27 @@ where // if we are behind. // We don't remove the child that it's on the same branch as best since a fork may happen // somewhere in between them. - fn request_targets(&self, mut top_wanted: NumberFor) -> Vec<::Header> { + fn request_targets(&mut self, mut top_wanted: NumberFor) -> Vec<::Header> { let blockchain_backend = self.backend.blockchain(); let blockchain_info = blockchain_backend.info(); let finalized_hash = blockchain_info.finalized_hash; let mut targets = blockchain_backend .children(finalized_hash) - .unwrap_or_default(); + .unwrap_or_default() + .into_iter() + .filter_map(|hash| { + if let Ok(Some(header)) = blockchain_backend.header(BlockId::Hash(hash)) { + Some(header) + } else { + debug!(target: "aleph-justification", + "Cancelling request for child {:?} of {:?}: no block.", hash, finalized_hash); + None + } + }) + .collect::>(); + self.request_status + .save_children(finalized_hash, targets.len()); let best_number = blockchain_info.best_number; if best_number <= top_wanted { // most probably block best_number is not yet finalized From ec833e6c1c00cda2f3977194954f18382e143a34 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 14:42:34 +0100 Subject: [PATCH 11/28] fix service --- bin/node/src/service.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index 8d93a25186..0873c8e034 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -283,10 +283,9 @@ pub fn new_authority( let backoff_authoring_blocks: Option<()> = None; let prometheus_registry = config.prometheus_registry().cloned(); - let blockchain_backed = backend.blockchain(); let (_rpc_handlers, network, network_starter) = setup( config, - backend, + backend.clone(), &keystore_container, import_queue, transaction_pool.clone(), @@ -349,7 +348,7 @@ pub fn new_authority( let aleph_config = AlephConfig { network, client, - blockchain_backend, + backend, select_chain, session_period, millisecs_per_block, @@ -396,10 +395,9 @@ pub fn new_full( .path(), ); - let blockchain_backed = backend.blockchain(); let (_rpc_handlers, network, network_starter) = setup( config, - backend, + backend.clone(), &keystore_container, import_queue, transaction_pool, @@ -426,7 +424,7 @@ pub fn new_full( let aleph_config = AlephConfig { network, client, - blockchain_backend, + backend, select_chain, session_period, millisecs_per_block, From 2afbab789fc051ebd6f2debe766c393874fec2d7 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 14:46:37 +0100 Subject: [PATCH 12/28] warn about missing blocks --- finality-aleph/src/justification/requester.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index b3ba1b73da..c7e0c0b3fd 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -245,7 +245,7 @@ where if let Ok(Some(header)) = blockchain_backend.header(BlockId::Hash(hash)) { Some(header) } else { - debug!(target: "aleph-justification", + warn!(target: "aleph-justification", "Cancelling request for child {:?} of {:?}: no block.", hash, finalized_hash); None } @@ -267,10 +267,10 @@ where } } Ok(None) => { - debug!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", top_wanted); + warn!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", top_wanted); } Err(err) => { - debug!(target: "aleph-justification", "Cancelling request, because fetching block {:?} failed {:?}.", top_wanted, err); + warn!(target: "aleph-justification", "Cancelling request, because fetching block {:?} failed {:?}.", top_wanted, err); } } From 2a802215ccd94b0c946b7323aec90eecb5f3805b Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 14:49:27 +0100 Subject: [PATCH 13/28] improve comment --- finality-aleph/src/justification/requester.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index c7e0c0b3fd..0870f48aaf 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -228,9 +228,9 @@ where // We request justifications for all the children of last finalized block and a justification // for a block of number num on longest branch. // Assuming that we request at the same pace that finalization is progressing, the former ensures - // that we are up to date with finalization. On the other hand, the former enables fast catch up + // that we are up to date with finalization. On the other hand, the latter enables fast catch up // if we are behind. - // We don't remove the child that it's on the same branch as best since a fork may happen + // We also request the child that it's on the same branch as top_wanted since a fork may happen // somewhere in between them. fn request_targets(&mut self, mut top_wanted: NumberFor) -> Vec<::Header> { let blockchain_backend = self.backend.blockchain(); From 9686b68f7cd393abe6b043d5ca9dc00467d85571 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 21:58:25 +0100 Subject: [PATCH 14/28] Review changes --- finality-aleph/src/justification/requester.rs | 81 +++++++------------ finality-aleph/src/nodes/mod.rs | 2 +- 2 files changed, 32 insertions(+), 51 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 0870f48aaf..80f3c2dfdc 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -1,4 +1,4 @@ -use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; +use std::{fmt, iter, marker::PhantomData, sync::Arc, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; @@ -13,7 +13,7 @@ use crate::{ JustificationRequestScheduler, Verifier, }, metrics::Checkpoint, - network, Metrics, + network, BlockHashNum, Metrics, }; /// Threshold for how many tries are needed so that JustificationRequestStatus is logged @@ -21,11 +21,10 @@ const REPORT_THRESHOLD: u32 = 2; /// This structure is created for keeping and reporting status of BlockRequester pub struct JustificationRequestStatus { - block_number: Option>, - block_hash: Option, + block_hash_number: Option>, block_tries: u32, parent: Option, - n_childred: Option, + n_childred: usize, children_tries: u32, report_threshold: u32, } @@ -33,11 +32,10 @@ pub struct JustificationRequestStatus { impl JustificationRequestStatus { fn new() -> Self { Self { - block_number: None, - block_hash: None, + block_hash_number: None, block_tries: 0, parent: None, - n_childred: None, + n_childred: 0, children_tries: 0, report_threshold: REPORT_THRESHOLD, } @@ -48,18 +46,16 @@ impl JustificationRequestStatus { self.children_tries += 1; } else { self.parent = Some(hash); - self.n_childred = Some(n_childred); + self.n_childred = n_childred; self.children_tries = 1; } } - fn save_block(&mut self, num: NumberFor, hash: B::Hash) { - if self.block_number == Some(num) { + fn save_block(&mut self, hn: BlockHashNum) { + if self.block_hash_number == Some(hn.clone()) { self.block_tries += 1; } else { - self.block_hash = Some(hash); - self.block_number = Some(num); - self.block_hash = None; + self.block_hash_number = Some(hn); self.block_tries = 1; } } @@ -72,30 +68,20 @@ impl JustificationRequestStatus { impl fmt::Display for JustificationRequestStatus { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if self.block_tries >= self.report_threshold { - if let Some(n) = self.block_number { - let mut status = format!( - "tries - {}; requested block number - {}; ", - self.block_tries, n - ); - if let Some(header) = self.block_hash { - status.push_str(&format!("hash - {}; ", header)); - } else { - status.push_str("hash - unknown; "); - } - - write!(f, "{}", status)?; + if let Some(hn) = &self.block_hash_number { + write!( + f, + "tries - {}; requested block number - {}; hash - {};", + self.block_tries, hn.num, hn.hash, + )?; } } if self.children_tries >= self.report_threshold { if let Some(parent) = self.parent { - let n = self - .n_childred - .expect("Children number is saved with parent."); - write!( f, "tries - {}; requested {} children of finalized block {}; ", - self.children_tries, n, parent + self.children_tries, self.n_childred, parent )?; } } @@ -216,13 +202,11 @@ where self.backend.blockchain().info().finalized_number } - fn request(&mut self, header: ::Header) { - let number = *header.number(); - let hash = header.hash(); + fn request(&mut self, hn: BlockHashNum) { debug!(target: "aleph-justification", - "We have block {:?} with hash {:?}. Requesting justification.", number, hash); + "We have block {:?} with hash {:?}. Requesting justification.", hn.num, hn.hash); self.justification_request_scheduler.on_request_sent(); - self.block_requester.request_justification(&hash, number); + self.block_requester.request_justification(&hn.hash, hn.num); } // We request justifications for all the children of last finalized block and a justification @@ -232,24 +216,18 @@ where // if we are behind. // We also request the child that it's on the same branch as top_wanted since a fork may happen // somewhere in between them. - fn request_targets(&mut self, mut top_wanted: NumberFor) -> Vec<::Header> { + fn request_targets(&mut self, mut top_wanted: NumberFor) -> Vec> { let blockchain_backend = self.backend.blockchain(); let blockchain_info = blockchain_backend.info(); let finalized_hash = blockchain_info.finalized_hash; + let finalized_number = blockchain_info.finalized_number; let mut targets = blockchain_backend .children(finalized_hash) .unwrap_or_default() .into_iter() - .filter_map(|hash| { - if let Ok(Some(header)) = blockchain_backend.header(BlockId::Hash(hash)) { - Some(header) - } else { - warn!(target: "aleph-justification", - "Cancelling request for child {:?} of {:?}: no block.", hash, finalized_hash); - None - } - }) + .zip(iter::once(finalized_number)) + .map(Into::into) .collect::>(); self.request_status .save_children(finalized_hash, targets.len()); @@ -258,12 +236,15 @@ where // most probably block best_number is not yet finalized top_wanted = best_number - NumberFor::::one(); } + match blockchain_backend.header(BlockId::Number(top_wanted)) { Ok(Some(header)) => { - if !targets.contains(&header) { - self.request_status - .save_block(*header.number(), header.hash()); - targets.push(header); + // We know that top_wanted >= finalized_number, and if top_wanted == finalized_number + 1, + // then hash(top_wanted) is among targets already. + if top_wanted > finalized_number + NumberFor::::one() { + let hn: BlockHashNum = (header.hash(), *header.number()).into(); + targets.push(hn.clone()); + self.request_status.save_block(hn); } } Ok(None) => { diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index d02e46a262..07c44fd60a 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -7,7 +7,7 @@ use aleph_primitives::{AuthorityId, SessionAuthorityData}; use codec::Encode; use log::warn; pub use nonvalidator_node::run_nonvalidator_node; -use sc_client_api::{Backend}; +use sc_client_api::Backend; use sc_network::{ExHashT, NetworkService}; use sp_runtime::{ traits::{Block, Header, NumberFor}, From edf58053c4d12dfbf68b00bb6f4163e1129f5ffa Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 22:12:51 +0100 Subject: [PATCH 15/28] improve comment --- finality-aleph/src/justification/requester.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 80f3c2dfdc..0f35086db3 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -24,7 +24,7 @@ pub struct JustificationRequestStatus { block_hash_number: Option>, block_tries: u32, parent: Option, - n_childred: usize, + n_children: usize, children_tries: u32, report_threshold: u32, } @@ -35,18 +35,18 @@ impl JustificationRequestStatus { block_hash_number: None, block_tries: 0, parent: None, - n_childred: 0, + n_children: 0, children_tries: 0, report_threshold: REPORT_THRESHOLD, } } - fn save_children(&mut self, hash: B::Hash, n_childred: usize) { - if self.parent == Some(hash) { + fn save_children(&mut self, hash: B::Hash, n_children: usize) { + if self.parent == Some(hash) && self.n_children >= n_children { self.children_tries += 1; } else { self.parent = Some(hash); - self.n_childred = n_childred; + self.n_children = n_children; self.children_tries = 1; } } @@ -81,7 +81,7 @@ impl fmt::Display for JustificationRequestStatus { write!( f, "tries - {}; requested {} children of finalized block {}; ", - self.children_tries, self.n_childred, parent + self.children_tries, self.n_children, parent )?; } } @@ -231,16 +231,17 @@ where .collect::>(); self.request_status .save_children(finalized_hash, targets.len()); + let best_number = blockchain_info.best_number; if best_number <= top_wanted { // most probably block best_number is not yet finalized top_wanted = best_number - NumberFor::::one(); } - match blockchain_backend.header(BlockId::Number(top_wanted)) { Ok(Some(header)) => { - // We know that top_wanted >= finalized_number, and if top_wanted == finalized_number + 1, - // then hash(top_wanted) is among targets already. + // We know that top_wanted >= finalized_number, so + // - if top_wanted == finalized_number, then we don't want to request it + // - if top_wanted == finalized_number + 1, then hash(top_wanted) is among targets already. if top_wanted > finalized_number + NumberFor::::one() { let hn: BlockHashNum = (header.hash(), *header.number()).into(); targets.push(hn.clone()); From f6cfc69f2bddd53ed39930b8fba88f8e6fd4cf66 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 22:30:14 +0100 Subject: [PATCH 16/28] split and simplify request logic --- finality-aleph/src/justification/requester.rs | 79 +++++++++++-------- 1 file changed, 46 insertions(+), 33 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 0f35086db3..7207db0dfe 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -2,7 +2,11 @@ use std::{fmt, iter, marker::PhantomData, sync::Arc, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; -use sc_client_api::{backend::Backend, blockchain::Backend as _, HeaderBackend}; +use sc_client_api::{ + backend::Backend, + blockchain::{Backend as _, Info}, + HeaderBackend, +}; use sp_api::{BlockId, BlockT, NumberFor}; use sp_runtime::traits::{Header, One}; @@ -183,12 +187,12 @@ where } } - pub fn request_justification(&mut self, num: NumberFor) { + pub fn request_justification(&mut self, wanted: NumberFor) { match self.justification_request_scheduler.schedule_action() { SchedulerActions::Request => { - self.request_targets(num) - .into_iter() - .for_each(|header| self.request(header)); + let info = self.backend.blockchain().info(); + self.request_children(&info); + self.request_wanted(wanted, &info); } SchedulerActions::ClearQueue => { debug!(target: "aleph-justification", "Clearing justification request queue"); @@ -202,51 +206,62 @@ where self.backend.blockchain().info().finalized_number } - fn request(&mut self, hn: BlockHashNum) { + fn do_request(&mut self, hn: BlockHashNum) { debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", hn.num, hn.hash); self.justification_request_scheduler.on_request_sent(); self.block_requester.request_justification(&hn.hash, hn.num); } - // We request justifications for all the children of last finalized block and a justification - // for a block of number num on longest branch. - // Assuming that we request at the same pace that finalization is progressing, the former ensures - // that we are up to date with finalization. On the other hand, the latter enables fast catch up - // if we are behind. + // We request justifications for all the children of last finalized block. + // Assuming that we request at the same pace that finalization is progressing, it ensures + // that we are up to date with finalization. // We also request the child that it's on the same branch as top_wanted since a fork may happen // somewhere in between them. - fn request_targets(&mut self, mut top_wanted: NumberFor) -> Vec> { - let blockchain_backend = self.backend.blockchain(); - let blockchain_info = blockchain_backend.info(); - let finalized_hash = blockchain_info.finalized_hash; - let finalized_number = blockchain_info.finalized_number; + fn request_children(&mut self, info: &Info) { + let finalized_hash = info.finalized_hash; + let finalized_number = info.finalized_number; - let mut targets = blockchain_backend + let children = self + .backend + .blockchain() .children(finalized_hash) - .unwrap_or_default() + .unwrap_or_default(); + + if !children.is_empty() { + self.request_status + .save_children(finalized_hash, children.len()); + } + + children .into_iter() .zip(iter::once(finalized_number)) .map(Into::into) - .collect::>(); - self.request_status - .save_children(finalized_hash, targets.len()); + .for_each(|hn| self.do_request(hn)); + } - let best_number = blockchain_info.best_number; + // This request is important in the case when we are far behind and want to catch up. + fn request_wanted(&mut self, mut top_wanted: NumberFor, info: &Info) { + let best_number = info.best_number; if best_number <= top_wanted { // most probably block best_number is not yet finalized top_wanted = best_number - NumberFor::::one(); } - match blockchain_backend.header(BlockId::Number(top_wanted)) { + let finalized_number = info.finalized_number; + // We know that top_wanted >= finalized_number, so + // - if top_wanted == finalized_number, then we don't want to request it + // - if top_wanted == finalized_number + 1, then we already requested it + if top_wanted > finalized_number + NumberFor::::one() { + return; + } + match self + .backend + .blockchain() + .header(BlockId::Number(top_wanted)) + { Ok(Some(header)) => { - // We know that top_wanted >= finalized_number, so - // - if top_wanted == finalized_number, then we don't want to request it - // - if top_wanted == finalized_number + 1, then hash(top_wanted) is among targets already. - if top_wanted > finalized_number + NumberFor::::one() { - let hn: BlockHashNum = (header.hash(), *header.number()).into(); - targets.push(hn.clone()); - self.request_status.save_block(hn); - } + self.request_status + .save_block((header.hash(), *header.number()).into()); } Ok(None) => { warn!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", top_wanted); @@ -255,7 +270,5 @@ where warn!(target: "aleph-justification", "Cancelling request, because fetching block {:?} failed {:?}.", top_wanted, err); } } - - targets } } From 0be21c0b74def796b31efc401d6a39dfce948b9e Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 22:38:45 +0100 Subject: [PATCH 17/28] Do do_request wanted --- finality-aleph/src/justification/requester.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 7207db0dfe..ac8c2b651b 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -206,7 +206,7 @@ where self.backend.blockchain().info().finalized_number } - fn do_request(&mut self, hn: BlockHashNum) { + fn do_request(&mut self, hn: &BlockHashNum) { debug!(target: "aleph-justification", "We have block {:?} with hash {:?}. Requesting justification.", hn.num, hn.hash); self.justification_request_scheduler.on_request_sent(); @@ -222,9 +222,7 @@ where let finalized_hash = info.finalized_hash; let finalized_number = info.finalized_number; - let children = self - .backend - .blockchain() + let children = self.backend.blockchain() .children(finalized_hash) .unwrap_or_default(); @@ -237,7 +235,7 @@ where .into_iter() .zip(iter::once(finalized_number)) .map(Into::into) - .for_each(|hn| self.do_request(hn)); + .for_each(|hn| self.do_request(&hn)); } // This request is important in the case when we are far behind and want to catch up. @@ -254,14 +252,11 @@ where if top_wanted > finalized_number + NumberFor::::one() { return; } - match self - .backend - .blockchain() - .header(BlockId::Number(top_wanted)) - { + match self.backend.blockchain().header(BlockId::Number(top_wanted)) { Ok(Some(header)) => { - self.request_status - .save_block((header.hash(), *header.number()).into()); + let hn: BlockHashNum = (header.hash(), *header.number()).into(); + self.do_request(&hn); + self.request_status.save_block(hn); } Ok(None) => { warn!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", top_wanted); From 48928789f7b912143d209a26d07434ffda869d98 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 22:48:38 +0100 Subject: [PATCH 18/28] Clearer? --- finality-aleph/src/justification/requester.rs | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index ac8c2b651b..b6a94ce7d2 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -1,4 +1,4 @@ -use std::{fmt, iter, marker::PhantomData, sync::Arc, time::Instant}; +use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; @@ -206,11 +206,11 @@ where self.backend.blockchain().info().finalized_number } - fn do_request(&mut self, hn: &BlockHashNum) { + fn do_request(&mut self, hash: &::Hash, num: NumberFor) { debug!(target: "aleph-justification", - "We have block {:?} with hash {:?}. Requesting justification.", hn.num, hn.hash); + "We have block {:?} with hash {:?}. Requesting justification.", num, hash); self.justification_request_scheduler.on_request_sent(); - self.block_requester.request_justification(&hn.hash, hn.num); + self.block_requester.request_justification(hash, num); } // We request justifications for all the children of last finalized block. @@ -222,7 +222,9 @@ where let finalized_hash = info.finalized_hash; let finalized_number = info.finalized_number; - let children = self.backend.blockchain() + let children = self + .backend + .blockchain() .children(finalized_hash) .unwrap_or_default(); @@ -231,11 +233,9 @@ where .save_children(finalized_hash, children.len()); } - children - .into_iter() - .zip(iter::once(finalized_number)) - .map(Into::into) - .for_each(|hn| self.do_request(&hn)); + for child in &children { + self.do_request(child, finalized_number); + } } // This request is important in the case when we are far behind and want to catch up. @@ -252,11 +252,16 @@ where if top_wanted > finalized_number + NumberFor::::one() { return; } - match self.backend.blockchain().header(BlockId::Number(top_wanted)) { + match self + .backend + .blockchain() + .header(BlockId::Number(top_wanted)) + { Ok(Some(header)) => { - let hn: BlockHashNum = (header.hash(), *header.number()).into(); - self.do_request(&hn); - self.request_status.save_block(hn); + let hash = header.hash(); + let num = *header.number(); + self.do_request(&hash, num); + self.request_status.save_block((hash, num).into()); } Ok(None) => { warn!(target: "aleph-justification", "Cancelling request, because we don't have block {:?}.", top_wanted); From 5bde7e8fcfb23d892db4d2cb0e50dc4538054a89 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Thu, 24 Nov 2022 22:54:28 +0100 Subject: [PATCH 19/28] fix inequality after moving surrounding block --- finality-aleph/src/justification/requester.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index b6a94ce7d2..78de2d8589 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -249,7 +249,7 @@ where // We know that top_wanted >= finalized_number, so // - if top_wanted == finalized_number, then we don't want to request it // - if top_wanted == finalized_number + 1, then we already requested it - if top_wanted > finalized_number + NumberFor::::one() { + if top_wanted <= finalized_number + NumberFor::::one() { return; } match self From 3d9d923d683eb9fcdc5539fe012085fc857ce1a2 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Fri, 25 Nov 2022 12:05:00 +0100 Subject: [PATCH 20/28] Always count child tries --- finality-aleph/src/justification/requester.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 78de2d8589..f580f16d00 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -46,13 +46,13 @@ impl JustificationRequestStatus { } fn save_children(&mut self, hash: B::Hash, n_children: usize) { - if self.parent == Some(hash) && self.n_children >= n_children { + if self.parent == Some(hash) { self.children_tries += 1; } else { self.parent = Some(hash); - self.n_children = n_children; self.children_tries = 1; } + self.n_children = n_children; } fn save_block(&mut self, hn: BlockHashNum) { From 72571851f8f98a787ebbb064f903349d0a821342 Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Fri, 25 Nov 2022 16:13:45 +0100 Subject: [PATCH 21/28] impl BB --- bin/node/src/service.rs | 121 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 119 insertions(+), 2 deletions(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index 0873c8e034..470b7addde 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -6,7 +6,11 @@ use std::{ }; use aleph_primitives::AlephSessionApi; -use aleph_runtime::{self, opaque::Block, RuntimeApi, MAX_BLOCK_SIZE}; +use aleph_runtime::{ + self, + opaque::{Block, Header}, + Hash, RuntimeApi, MAX_BLOCK_SIZE, +}; use finality_aleph::{ run_nonvalidator_node, run_validator_node, AlephBlockImport, AlephConfig, JustificationNotification, Metrics, MillisecsPerBlock, Protocol, SessionPeriod, @@ -21,7 +25,7 @@ use sc_service::{ TFullClient, TaskManager, }; use sc_telemetry::{Telemetry, TelemetryWorker}; -use sp_api::ProvideRuntimeApi; +use sp_api::{NumberFor, ProvideRuntimeApi}; use sp_consensus_aura::sr25519::AuthorityPair as AuraPair; use sp_runtime::{ generic::BlockId, @@ -447,3 +451,116 @@ pub fn new_full( network_starter.start_network(); Ok(task_manager) } + +struct BlockchainBackend { + backend: FullBackend, +} + +use sc_client_api::Backend as _; +use sp_blockchain::CachedHeaderMetadata; + +impl sp_blockchain::HeaderBackend for BlockchainBackend { + fn header(&self, id: BlockId) -> sp_blockchain::Result> { + self.backend.blockchain().header(id) + } + fn info(&self) -> sp_blockchain::Info { + self.backend.blockchain().info() + } + fn status(&self, id: BlockId) -> sp_blockchain::Result { + self.backend.blockchain().status(id) + } + fn number( + &self, + hash: Hash, + ) -> sp_blockchain::Result::Header as HeaderT>::Number>> { + self.backend.blockchain().number(hash) + } + fn hash(&self, number: NumberFor) -> sp_blockchain::Result> { + self.backend.blockchain().hash(number) + } + fn block_hash_from_id(&self, id: &BlockId) -> sp_blockchain::Result> { + self.backend.blockchain().block_hash_from_id(id) + } + fn block_number_from_id( + &self, + id: &BlockId, + ) -> sp_blockchain::Result>> { + self.backend.blockchain().block_number_from_id(id) + } + fn expect_header(&self, id: BlockId) -> sp_blockchain::Result
{ + self.backend.blockchain().expect_header(id) + } + fn expect_block_number_from_id( + &self, + id: &BlockId, + ) -> sp_blockchain::Result> { + self.backend.blockchain().expect_block_number_from_id(id) + } + fn expect_block_hash_from_id(&self, id: &BlockId) -> sp_blockchain::Result { + self.backend.blockchain().expect_block_hash_from_id(id) + } +} + +impl sp_blockchain::HeaderMetadata for BlockchainBackend { + type Error = sp_blockchain::Error; + + fn header_metadata(&self, hash: Hash) -> sp_blockchain::Result> { + self.backend.blockchain().header_metadata(hash) + } + fn insert_header_metadata(&self, hash: Hash, header_metadata: CachedHeaderMetadata) { + self.backend + .blockchain() + .insert_header_metadata(hash, header_metadata) + } + fn remove_header_metadata(&self, hash: Hash) { + self.backend.blockchain().remove_header_metadata(hash) + } +} + +impl sp_blockchain::Backend for BlockchainBackend { + fn body( + &self, + id: BlockId, + ) -> sp_blockchain::Result::Extrinsic>>> { + self.backend.blockchain().body(id) + } + fn justifications( + &self, + id: BlockId, + ) -> sp_blockchain::Result> { + self.backend.blockchain().justifications(id) + } + fn last_finalized(&self) -> sp_blockchain::Result { + self.backend.blockchain().last_finalized() + } + fn leaves(&self) -> sp_blockchain::Result> { + self.backend.blockchain().leaves() + } + fn displaced_leaves_after_finalizing( + &self, + block_number: NumberFor, + ) -> sp_blockchain::Result> { + self.backend + .blockchain() + .displaced_leaves_after_finalizing(block_number) + } + fn children(&self, parent_hash: Hash) -> sp_blockchain::Result> { + self.backend.blockchain().children(parent_hash) + } + fn indexed_transaction(&self, hash: &Hash) -> sp_blockchain::Result>> { + self.backend.blockchain().indexed_transaction(hash) + } + fn has_indexed_transaction(&self, hash: &Hash) -> sp_blockchain::Result { + self.backend.blockchain().has_indexed_transaction(hash) + } + fn block_indexed_body( + &self, + id: BlockId, + ) -> sp_blockchain::Result>>> { + self.backend.blockchain().block_indexed_body(id) + } +} + +unsafe impl Send for BlockchainBackend {} + +unsafe impl Sync for BlockchainBackend {} From 23fdf9fcca48002fdef85ad20bc4ff03e7a1a22e Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Fri, 25 Nov 2022 17:25:57 +0100 Subject: [PATCH 22/28] prepare for tests --- bin/node/src/service.rs | 131 ++---------------- finality-aleph/src/justification/handler.rs | 22 ++- finality-aleph/src/justification/requester.rs | 38 ++--- finality-aleph/src/lib.rs | 9 +- finality-aleph/src/nodes/mod.rs | 15 +- finality-aleph/src/nodes/nonvalidator_node.rs | 12 +- finality-aleph/src/nodes/validator_node.rs | 9 +- 7 files changed, 69 insertions(+), 167 deletions(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index 470b7addde..e9bd5b3dec 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -6,18 +6,14 @@ use std::{ }; use aleph_primitives::AlephSessionApi; -use aleph_runtime::{ - self, - opaque::{Block, Header}, - Hash, RuntimeApi, MAX_BLOCK_SIZE, -}; +use aleph_runtime::{self, opaque::Block, RuntimeApi, MAX_BLOCK_SIZE}; use finality_aleph::{ run_nonvalidator_node, run_validator_node, AlephBlockImport, AlephConfig, JustificationNotification, Metrics, MillisecsPerBlock, Protocol, SessionPeriod, }; use futures::channel::mpsc; use log::warn; -use sc_client_api::ExecutorProvider; +use sc_client_api::{Backend as _, ExecutorProvider}; use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams}; use sc_network::NetworkService; use sc_service::{ @@ -25,7 +21,7 @@ use sc_service::{ TFullClient, TaskManager, }; use sc_telemetry::{Telemetry, TelemetryWorker}; -use sp_api::{NumberFor, ProvideRuntimeApi}; +use sp_api::ProvideRuntimeApi; use sp_consensus_aura::sr25519::AuthorityPair as AuraPair; use sp_runtime::{ generic::BlockId, @@ -349,10 +345,11 @@ pub fn new_authority( if aleph_config.external_addresses().is_empty() { panic!("Cannot run a validator node without external addresses, stopping."); } + let get_blockchain_backend = GetBlockchainBackendImpl { backend }; let aleph_config = AlephConfig { network, client, - backend, + get_blockchain_backend, select_chain, session_period, millisecs_per_block, @@ -425,10 +422,11 @@ pub fn new_full( .unwrap(), ); + let get_blockchain_backend = GetBlockchainBackendImpl { backend }; let aleph_config = AlephConfig { network, client, - backend, + get_blockchain_backend, select_chain, session_period, millisecs_per_block, @@ -452,115 +450,12 @@ pub fn new_full( Ok(task_manager) } -struct BlockchainBackend { - backend: FullBackend, -} - -use sc_client_api::Backend as _; -use sp_blockchain::CachedHeaderMetadata; - -impl sp_blockchain::HeaderBackend for BlockchainBackend { - fn header(&self, id: BlockId) -> sp_blockchain::Result> { - self.backend.blockchain().header(id) - } - fn info(&self) -> sp_blockchain::Info { - self.backend.blockchain().info() - } - fn status(&self, id: BlockId) -> sp_blockchain::Result { - self.backend.blockchain().status(id) - } - fn number( - &self, - hash: Hash, - ) -> sp_blockchain::Result::Header as HeaderT>::Number>> { - self.backend.blockchain().number(hash) - } - fn hash(&self, number: NumberFor) -> sp_blockchain::Result> { - self.backend.blockchain().hash(number) - } - fn block_hash_from_id(&self, id: &BlockId) -> sp_blockchain::Result> { - self.backend.blockchain().block_hash_from_id(id) - } - fn block_number_from_id( - &self, - id: &BlockId, - ) -> sp_blockchain::Result>> { - self.backend.blockchain().block_number_from_id(id) - } - fn expect_header(&self, id: BlockId) -> sp_blockchain::Result
{ - self.backend.blockchain().expect_header(id) - } - fn expect_block_number_from_id( - &self, - id: &BlockId, - ) -> sp_blockchain::Result> { - self.backend.blockchain().expect_block_number_from_id(id) - } - fn expect_block_hash_from_id(&self, id: &BlockId) -> sp_blockchain::Result { - self.backend.blockchain().expect_block_hash_from_id(id) - } -} - -impl sp_blockchain::HeaderMetadata for BlockchainBackend { - type Error = sp_blockchain::Error; - - fn header_metadata(&self, hash: Hash) -> sp_blockchain::Result> { - self.backend.blockchain().header_metadata(hash) - } - fn insert_header_metadata(&self, hash: Hash, header_metadata: CachedHeaderMetadata) { - self.backend - .blockchain() - .insert_header_metadata(hash, header_metadata) - } - fn remove_header_metadata(&self, hash: Hash) { - self.backend.blockchain().remove_header_metadata(hash) - } +struct GetBlockchainBackendImpl { + backend: Arc, } - -impl sp_blockchain::Backend for BlockchainBackend { - fn body( - &self, - id: BlockId, - ) -> sp_blockchain::Result::Extrinsic>>> { - self.backend.blockchain().body(id) - } - fn justifications( - &self, - id: BlockId, - ) -> sp_blockchain::Result> { - self.backend.blockchain().justifications(id) - } - fn last_finalized(&self) -> sp_blockchain::Result { - self.backend.blockchain().last_finalized() - } - fn leaves(&self) -> sp_blockchain::Result> { - self.backend.blockchain().leaves() - } - fn displaced_leaves_after_finalizing( - &self, - block_number: NumberFor, - ) -> sp_blockchain::Result> { - self.backend - .blockchain() - .displaced_leaves_after_finalizing(block_number) - } - fn children(&self, parent_hash: Hash) -> sp_blockchain::Result> { - self.backend.blockchain().children(parent_hash) - } - fn indexed_transaction(&self, hash: &Hash) -> sp_blockchain::Result>> { - self.backend.blockchain().indexed_transaction(hash) - } - fn has_indexed_transaction(&self, hash: &Hash) -> sp_blockchain::Result { - self.backend.blockchain().has_indexed_transaction(hash) - } - fn block_indexed_body( - &self, - id: BlockId, - ) -> sp_blockchain::Result>>> { - self.backend.blockchain().block_indexed_body(id) +impl finality_aleph::GetBlockchainBackend for GetBlockchainBackendImpl { + type BlockchainBackend = >::Blockchain; + fn blockchain(&self) -> &Self::BlockchainBackend { + self.backend.blockchain() } } - -unsafe impl Send for BlockchainBackend {} - -unsafe impl Sync for BlockchainBackend {} diff --git a/finality-aleph/src/justification/handler.rs b/finality-aleph/src/justification/handler.rs index f8ac7207b5..c623e6862d 100644 --- a/finality-aleph/src/justification/handler.rs +++ b/finality-aleph/src/justification/handler.rs @@ -1,12 +1,8 @@ -use std::{ - sync::Arc, - time::{Duration, Instant}, -}; +use std::time::{Duration, Instant}; use futures::{channel::mpsc, Stream, StreamExt}; use futures_timer::Delay; use log::{debug, error}; -use sc_client_api::backend::Backend; use sp_api::BlockT; use sp_runtime::traits::Header; use tokio::time::timeout; @@ -17,10 +13,10 @@ use crate::{ requester::BlockRequester, JustificationHandlerConfig, JustificationNotification, JustificationRequestScheduler, SessionInfo, SessionInfoProvider, Verifier, }, - network, Metrics, STATUS_REPORT_INTERVAL, + network, GetBlockchainBackend, Metrics, STATUS_REPORT_INTERVAL, }; -pub struct JustificationHandler +pub struct JustificationHandler where B: BlockT, V: Verifier, @@ -28,15 +24,15 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BE: Backend + Send + Sync + 'static, + GBB: GetBlockchainBackend + 'static, { session_info_provider: SI, - block_requester: BlockRequester, + block_requester: BlockRequester, verifier_timeout: Duration, notification_timeout: Duration, } -impl JustificationHandler +impl JustificationHandler where B: BlockT, V: Verifier, @@ -44,12 +40,12 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - BE: Backend + Send + Sync + 'static, + GBB: GetBlockchainBackend + 'static, { pub fn new( session_info_provider: SI, block_requester: RB, - backend: Arc, + get_blockchain_backend: GBB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -59,7 +55,7 @@ where session_info_provider, block_requester: BlockRequester::new( block_requester, - backend, + get_blockchain_backend, finalizer, justification_request_scheduler, metrics, diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index f580f16d00..0f8ffdb28b 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -1,14 +1,13 @@ -use std::{fmt, marker::PhantomData, sync::Arc, time::Instant}; +use std::{fmt, marker::PhantomData, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; use sc_client_api::{ - backend::Backend, - blockchain::{Backend as _, Info}, + blockchain::{Backend, Info}, HeaderBackend, }; use sp_api::{BlockId, BlockT, NumberFor}; -use sp_runtime::traits::{Header, One}; +use sp_runtime::traits::{Header, One, Saturating}; use crate::{ finalization::BlockFinalizer, @@ -17,7 +16,7 @@ use crate::{ JustificationRequestScheduler, Verifier, }, metrics::Checkpoint, - network, BlockHashNum, Metrics, + network, BlockHashNum, GetBlockchainBackend, Metrics, }; /// Threshold for how many tries are needed so that JustificationRequestStatus is logged @@ -93,17 +92,17 @@ impl fmt::Display for JustificationRequestStatus { } } -pub struct BlockRequester +pub struct BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - BE: Backend, + GBB: GetBlockchainBackend + 'static, { block_requester: RB, - backend: Arc, + get_blockchain_backend: GBB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -111,25 +110,25 @@ where _phantom: PhantomData, } -impl BlockRequester +impl BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - BE: Backend, + GBB: GetBlockchainBackend + 'static, { pub fn new( block_requester: RB, - backend: Arc, + get_blockchain_backend: GBB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, ) -> Self { BlockRequester { block_requester, - backend, + get_blockchain_backend, finalizer, justification_request_scheduler, metrics, @@ -190,7 +189,7 @@ where pub fn request_justification(&mut self, wanted: NumberFor) { match self.justification_request_scheduler.schedule_action() { SchedulerActions::Request => { - let info = self.backend.blockchain().info(); + let info = self.get_blockchain_backend.blockchain().info(); self.request_children(&info); self.request_wanted(wanted, &info); } @@ -203,7 +202,10 @@ where } pub fn finalized_number(&self) -> NumberFor { - self.backend.blockchain().info().finalized_number + self.get_blockchain_backend + .blockchain() + .info() + .finalized_number } fn do_request(&mut self, hash: &::Hash, num: NumberFor) { @@ -223,7 +225,7 @@ where let finalized_number = info.finalized_number; let children = self - .backend + .get_blockchain_backend .blockchain() .children(finalized_hash) .unwrap_or_default(); @@ -234,7 +236,7 @@ where } for child in &children { - self.do_request(child, finalized_number); + self.do_request(child, finalized_number + NumberFor::::one()); } } @@ -243,7 +245,7 @@ where let best_number = info.best_number; if best_number <= top_wanted { // most probably block best_number is not yet finalized - top_wanted = best_number - NumberFor::::one(); + top_wanted = best_number.saturating_sub(NumberFor::::one()); } let finalized_number = info.finalized_number; // We know that top_wanted >= finalized_number, so @@ -253,7 +255,7 @@ where return; } match self - .backend + .get_blockchain_backend .blockchain() .header(BlockId::Number(top_wanted)) { diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs index 66cd780176..eaf512f4aa 100644 --- a/finality-aleph/src/lib.rs +++ b/finality-aleph/src/lib.rs @@ -242,10 +242,10 @@ impl From<(H, N)> for HashNum { pub type BlockHashNum = HashNum<::Hash, NumberFor>; -pub struct AlephConfig { +pub struct AlephConfig { pub network: Arc>, pub client: Arc, - pub backend: Arc, + pub get_blockchain_backend: GBB, pub select_chain: SC, pub spawn_handle: SpawnTaskHandle, pub keystore: Arc, @@ -258,3 +258,8 @@ pub struct AlephConfig { pub external_addresses: Vec, pub validator_port: u16, } + +pub trait GetBlockchainBackend { + type BlockchainBackend: sp_blockchain::Backend; + fn blockchain(&self) -> &Self::BlockchainBackend; +} diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index 07c44fd60a..542746955d 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -26,7 +26,7 @@ use crate::{ mpsc::UnboundedSender, session_id_from_block_num, session_map::ReadOnlySessionMap, - JustificationNotification, Metrics, MillisecsPerBlock, SessionPeriod, + GetBlockchainBackend, JustificationNotification, Metrics, MillisecsPerBlock, SessionPeriod, }; #[cfg(test)] @@ -83,10 +83,10 @@ impl Verifier for JustificationVerifier { } } -struct JustificationParams { +struct JustificationParams { pub network: Arc>, pub client: Arc, - pub backend: Arc, + pub get_blockchain_backend: GBB, pub justification_rx: mpsc::UnboundedReceiver>, pub metrics: Option::Hash>>, pub session_period: SessionPeriod, @@ -127,8 +127,8 @@ impl SessionInfoProvider for SessionInfoProv } } -fn setup_justification_handler( - just_params: JustificationParams, +fn setup_justification_handler( + just_params: JustificationParams, ) -> ( UnboundedSender>, impl Future, @@ -139,11 +139,12 @@ where C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, BE: Backend + 'static, + GBB: GetBlockchainBackend + 'static + Send, { let JustificationParams { network, client, - backend, + get_blockchain_backend, justification_rx, metrics, session_period, @@ -154,7 +155,7 @@ where let handler = JustificationHandler::new( SessionInfoProviderImpl::new(session_map, session_period), network, - backend, + get_blockchain_backend, AlephFinalizer::new(client), JustificationRequestSchedulerImpl::new(&session_period, &millisecs_per_block, MAX_ATTEMPTS), metrics, diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs index b949d17f9c..c03fb01def 100644 --- a/finality-aleph/src/nodes/nonvalidator_node.rs +++ b/finality-aleph/src/nodes/nonvalidator_node.rs @@ -7,22 +7,24 @@ use sp_runtime::traits::Block; use crate::{ nodes::{setup_justification_handler, JustificationParams}, session_map::{AuthorityProviderImpl, FinalityNotificatorImpl, SessionMapUpdater}, - AlephConfig, + AlephConfig, GetBlockchainBackend, }; -pub async fn run_nonvalidator_node(aleph_config: AlephConfig) -where +pub async fn run_nonvalidator_node( + aleph_config: AlephConfig, +) where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, BE: Backend + 'static, + GBB: GetBlockchainBackend + Send + 'static, SC: SelectChain + 'static, { let AlephConfig { network, client, - backend, + get_blockchain_backend, metrics, session_period, millisecs_per_block, @@ -43,7 +45,7 @@ where justification_rx, network, client, - backend, + get_blockchain_backend, metrics, session_period, millisecs_per_block, diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs index 90d9b05bf1..8bc76b3219 100644 --- a/finality-aleph/src/nodes/validator_node.rs +++ b/finality-aleph/src/nodes/validator_node.rs @@ -24,7 +24,7 @@ use crate::{ session_map::{AuthorityProviderImpl, FinalityNotificatorImpl, SessionMapUpdater}, tcp_network::new_tcp_network, validator_network::{Service, KEY_TYPE}, - AlephConfig, + AlephConfig, GetBlockchainBackend, }; pub async fn new_pen(mnemonic: &str, keystore: Arc) -> AuthorityPen { @@ -37,19 +37,20 @@ pub async fn new_pen(mnemonic: &str, keystore: Arc) -> Authorit .expect("we just generated this key so everything should work") } -pub async fn run_validator_node(aleph_config: AlephConfig) +pub async fn run_validator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, BE: Backend + 'static, + GBB: GetBlockchainBackend + Send + 'static, SC: SelectChain + 'static, { let AlephConfig { network, client, - backend, + get_blockchain_backend, select_chain, spawn_handle, keystore, @@ -107,7 +108,7 @@ where justification_rx, network: network.clone(), client: client.clone(), - backend, + get_blockchain_backend, metrics: metrics.clone(), session_period, millisecs_per_block, From 1606eea02a9fbcffb5f57520693075c17515198e Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Fri, 25 Nov 2022 18:55:20 +0100 Subject: [PATCH 23/28] Fix tests --- finality-aleph/src/testing/justification.rs | 57 ++--- finality-aleph/src/testing/mocks/backend.rs | 202 ++++++++++++++++++ .../src/testing/mocks/header_backend.rs | 109 ---------- .../mocks/justification_handler_config.rs | 2 +- finality-aleph/src/testing/mocks/mod.rs | 4 +- 5 files changed, 235 insertions(+), 139 deletions(-) create mode 100644 finality-aleph/src/testing/mocks/backend.rs delete mode 100644 finality-aleph/src/testing/mocks/header_backend.rs diff --git a/finality-aleph/src/testing/justification.rs b/finality-aleph/src/testing/justification.rs index 5777d4e72d..1be9fdf51e 100644 --- a/finality-aleph/src/testing/justification.rs +++ b/finality-aleph/src/testing/justification.rs @@ -12,9 +12,9 @@ use AcceptancePolicy::*; use crate::{ justification::{AlephJustification, JustificationHandler, JustificationHandlerConfig}, testing::mocks::{ - create_block, AcceptancePolicy, Client, JustificationRequestSchedulerImpl, - MockedBlockFinalizer, MockedBlockRequester, SessionInfoProviderImpl, TBlock, - VerifierWrapper, + create_block, AcceptancePolicy, Backend, GetBlockchainBackendMock, + JustificationRequestSchedulerImpl, MockedBlockFinalizer, MockedBlockRequester, + SessionInfoProviderImpl, TBlock, VerifierWrapper, }, JustificationNotification, SessionPeriod, SignatureSet, }; @@ -26,15 +26,15 @@ type TJustHandler = JustificationHandler< TBlock, VerifierWrapper, MockedBlockRequester, - Client, JustificationRequestSchedulerImpl, SessionInfoProviderImpl, MockedBlockFinalizer, + GetBlockchainBackendMock, >; type Sender = UnboundedSender>; type Environment = ( TJustHandler, - Client, + Arc, MockedBlockRequester, MockedBlockFinalizer, JustificationRequestSchedulerImpl, @@ -67,7 +67,10 @@ fn prepare_env( verification_policy: AcceptancePolicy, request_policy: AcceptancePolicy, ) -> Environment { - let client = Client::new(finalization_height); + let backend = Arc::new(Backend::new(finalization_height)); + let get_blockchain_backend = GetBlockchainBackendMock { + backend: backend.clone(), + }; let info_provider = SessionInfoProviderImpl::new(SESSION_PERIOD, verification_policy); let finalizer = MockedBlockFinalizer::new(); let requester = MockedBlockRequester::new(); @@ -77,7 +80,7 @@ fn prepare_env( let justification_handler = JustificationHandler::new( info_provider, requester.clone(), - Arc::new(client.clone()), + get_blockchain_backend, finalizer.clone(), justification_request_scheduler.clone(), None, @@ -86,7 +89,7 @@ fn prepare_env( ( justification_handler, - client, + backend, requester, finalizer, justification_request_scheduler, @@ -125,19 +128,19 @@ where S: FnOnce( Sender, Sender, - Client, + Arc, MockedBlockRequester, MockedBlockFinalizer, JustificationRequestSchedulerImpl, ) -> F, { - let (justification_handler, client, requester, finalizer, justification_request_scheduler) = + let (justification_handler, backend, requester, finalizer, justification_request_scheduler) = env; let (handle_run, auth_just_tx, imp_just_tx) = run_justification_handler(justification_handler); scenario( auth_just_tx.clone(), imp_just_tx.clone(), - client, + backend, requester, finalizer, justification_request_scheduler, @@ -186,8 +189,8 @@ async fn expect_not_requested( async fn leads_to_finalization_when_appropriate_justification_comes() { run_test( prepare_env(FINALIZED_HEIGHT, AlwaysAccept, AlwaysReject), - |_, imp_just_tx, client, _, finalizer, justification_request_scheduler| async move { - let block = client.next_block_to_finalize(); + |_, imp_just_tx, backend, _, finalizer, justification_request_scheduler| async move { + let block = backend.next_block_to_finalize(); let message = create_justification_notification_for(block.clone()); imp_just_tx.unbounded_send(message).unwrap(); expect_finalized(&finalizer, &justification_request_scheduler, block).await; @@ -201,8 +204,8 @@ async fn waits_for_verifier_before_finalizing() { let verification_policy = FromSequence(RefCell::new(VecDeque::from(vec![false, false, true]))); run_test( prepare_env(FINALIZED_HEIGHT, verification_policy, AlwaysReject), - |_, imp_just_tx, client, _, finalizer, justification_request_scheduler| async move { - let block = client.next_block_to_finalize(); + |_, imp_just_tx, backend, _, finalizer, justification_request_scheduler| async move { + let block = backend.next_block_to_finalize(); let message = create_justification_notification_for(block.clone()); imp_just_tx.unbounded_send(message.clone()).unwrap(); @@ -222,8 +225,8 @@ async fn waits_for_verifier_before_finalizing() { async fn keeps_finalizing_block_if_not_finalized_yet() { run_test( prepare_env(FINALIZED_HEIGHT, AlwaysAccept, AlwaysReject), - |auth_just_tx, imp_just_tx, client, _, finalizer, justification_request_scheduler| async move { - let block = client.next_block_to_finalize(); + |auth_just_tx, imp_just_tx, backend, _, finalizer, justification_request_scheduler| async move { + let block = backend.next_block_to_finalize(); let message = create_justification_notification_for(block.clone()); imp_just_tx.unbounded_send(message.clone()).unwrap(); @@ -240,8 +243,8 @@ async fn keeps_finalizing_block_if_not_finalized_yet() { async fn ignores_notifications_for_old_blocks() { run_test( prepare_env(FINALIZED_HEIGHT, AlwaysAccept, AlwaysReject), - |_, imp_just_tx, client, _, finalizer, justification_request_scheduler| async move { - let block = client.get_block(BlockId::Number(1u64)).unwrap(); + |_, imp_just_tx, backend, _, finalizer, justification_request_scheduler| async move { + let block = backend.get_block(BlockId::Number(1u64)).unwrap(); let message = create_justification_notification_for(block); imp_just_tx.unbounded_send(message).unwrap(); expect_not_finalized(&finalizer, &justification_request_scheduler).await; @@ -268,8 +271,8 @@ async fn ignores_notifications_from_future_session() { async fn does_not_buffer_notifications_from_future_session() { run_test( prepare_env((SESSION_PERIOD.0 - 2) as u64, AlwaysAccept, AlwaysReject), - |_, imp_just_tx, client, _, finalizer, justification_request_scheduler| async move { - let current_block = client.next_block_to_finalize(); + |_, imp_just_tx, backend, _, finalizer, justification_request_scheduler| async move { + let current_block = backend.next_block_to_finalize(); let future_block = create_block(current_block.hash(), SESSION_PERIOD.0 as u64); let message = create_justification_notification_for(future_block); @@ -290,8 +293,8 @@ async fn does_not_buffer_notifications_from_future_session() { async fn requests_for_session_ending_justification() { run_test( prepare_env((SESSION_PERIOD.0 - 2) as u64, AlwaysReject, AlwaysAccept), - |_, imp_just_tx, client, requester, _, justification_request_scheduler| async move { - let last_block = client.next_block_to_finalize(); + |_, imp_just_tx, backend, requester, _, justification_request_scheduler| async move { + let last_block = backend.next_block_to_finalize(); // doesn't need any notification passed to keep asking expect_requested( @@ -321,14 +324,14 @@ async fn requests_for_session_ending_justification() { async fn does_not_request_for_session_ending_justification_too_often() { run_test( prepare_env((SESSION_PERIOD.0 - 2) as u64, AlwaysReject, AlwaysReject), - |_, _, client, requester, _, justification_request_scheduler| async move { + |_, _, backend, requester, _, justification_request_scheduler| async move { expect_not_requested(&requester, &justification_request_scheduler).await; justification_request_scheduler.update_policy(AlwaysAccept); expect_requested( &requester, &justification_request_scheduler, - client.next_block_to_finalize(), + backend.next_block_to_finalize(), ) .await; @@ -343,10 +346,10 @@ async fn does_not_request_for_session_ending_justification_too_often() { async fn does_not_request_nor_finalize_when_verifier_is_not_available() { run_test( prepare_env((SESSION_PERIOD.0 - 2) as u64, Unavailable, AlwaysAccept), - |_, imp_just_tx, client, requester, finalizer, justification_request_scheduler| async move { + |_, imp_just_tx, backend, requester, finalizer, justification_request_scheduler| async move { expect_not_requested(&requester, &justification_request_scheduler).await; - let block = client.next_block_to_finalize(); + let block = backend.next_block_to_finalize(); imp_just_tx .unbounded_send(create_justification_notification_for(block)) .unwrap(); diff --git a/finality-aleph/src/testing/mocks/backend.rs b/finality-aleph/src/testing/mocks/backend.rs new file mode 100644 index 0000000000..a83c6405bd --- /dev/null +++ b/finality-aleph/src/testing/mocks/backend.rs @@ -0,0 +1,202 @@ +use sp_api::BlockId; +use sp_blockchain::{ + Backend as BlockchainBackend, BlockStatus, HeaderBackend, HeaderMetadata, Info, +}; +use sp_runtime::traits::Block; +use std::sync::Arc; + +use crate::{ + testing::mocks::{TBlock, THash, THeader, TNumber}, + GetBlockchainBackend, +}; + +#[derive(Clone)] +pub(crate) struct Backend { + blocks: Vec, + next_block_to_finalize: TBlock, +} + +pub(crate) fn create_block(parent_hash: THash, number: TNumber) -> TBlock { + TBlock { + header: THeader { + parent_hash, + number, + state_root: Default::default(), + extrinsics_root: Default::default(), + digest: Default::default(), + }, + extrinsics: vec![], + } +} + +const GENESIS_HASH: [u8; 32] = [0u8; 32]; + +impl Backend { + pub(crate) fn new(finalized_height: u64) -> Self { + let mut blocks: Vec = vec![]; + + for n in 1u64..=finalized_height { + let parent_hash = match n { + 1 => GENESIS_HASH.into(), + _ => blocks.last().unwrap().header.hash(), + }; + blocks.push(create_block(parent_hash, n)); + } + + let next_block_to_finalize = + create_block(blocks.last().unwrap().hash(), finalized_height + 1); + + Backend { + blocks, + next_block_to_finalize, + } + } + + pub(crate) fn next_block_to_finalize(&self) -> TBlock { + self.next_block_to_finalize.clone() + } + + pub(crate) fn get_block(&self, id: BlockId) -> Option { + match id { + BlockId::Hash(h) => { + if self.next_block_to_finalize.hash() == h { + Some(self.next_block_to_finalize.clone()) + } else { + self.blocks.iter().find(|b| b.header.hash().eq(&h)).cloned() + } + } + BlockId::Number(n) => { + if self.next_block_to_finalize.header.number == n { + Some(self.next_block_to_finalize.clone()) + } else { + self.blocks.get((n - 1) as usize).cloned() + } + } + } + } +} + +impl HeaderBackend for Backend { + fn header(&self, id: BlockId) -> sp_blockchain::Result> { + Ok(self.get_block(id).map(|b| b.header)) + } + + fn info(&self) -> Info { + Info { + best_hash: self.next_block_to_finalize.hash(), + best_number: self.next_block_to_finalize.header.number, + finalized_hash: self.blocks.last().unwrap().hash(), + finalized_number: self.blocks.len() as u64, + genesis_hash: GENESIS_HASH.into(), + number_leaves: Default::default(), + finalized_state: None, + block_gap: None, + } + } + + fn status(&self, id: BlockId) -> sp_blockchain::Result { + Ok(match self.get_block(id) { + Some(_) => BlockStatus::InChain, + _ => BlockStatus::Unknown, + }) + } + + fn number(&self, hash: THash) -> sp_blockchain::Result> { + Ok(self.get_block(BlockId::hash(hash)).map(|b| b.header.number)) + } + + fn hash(&self, number: TNumber) -> sp_blockchain::Result> { + Ok(self.get_block(BlockId::Number(number)).map(|b| b.hash())) + } +} + +impl HeaderMetadata for Backend { + type Error = sp_blockchain::Error; + fn header_metadata( + &self, + _hash: ::Hash, + ) -> Result, Self::Error> { + Err(sp_blockchain::Error::Backend( + "Header metadata not implemented".into(), + )) + } + fn insert_header_metadata( + &self, + _hash: ::Hash, + _header_metadata: sp_blockchain::CachedHeaderMetadata, + ) { + } + fn remove_header_metadata(&self, _hash: ::Hash) {} +} + +impl BlockchainBackend for Backend { + fn body( + &self, + _id: BlockId, + ) -> sp_blockchain::Result::Extrinsic>>> { + Ok(None) + } + fn justifications( + &self, + _id: BlockId, + ) -> sp_blockchain::Result> { + Ok(None) + } + fn last_finalized(&self) -> sp_blockchain::Result<::Hash> { + Ok(self.info().finalized_hash) + } + fn leaves(&self) -> sp_blockchain::Result::Hash>> { + Ok(vec![self.next_block_to_finalize.hash()]) + } + fn displaced_leaves_after_finalizing( + &self, + _block_number: sp_api::NumberFor, + ) -> sp_blockchain::Result::Hash>> { + Ok(Vec::new()) + } + fn children(&self, parent_hash: THash) -> sp_blockchain::Result> { + let leaves = if self.next_block_to_finalize.hash() == parent_hash { + Vec::new() + } else if self + .blocks + .last() + .map(|b| b.hash()) + .unwrap() + .eq(&parent_hash) + { + vec![self.next_block_to_finalize.hash()] + } else { + self.blocks + .windows(2) + .flat_map(<&[TBlock; 2]>::try_from) + .find(|[parent, _]| parent.header.hash().eq(&parent_hash)) + .map(|[_, c]| vec![c.hash()]) + .unwrap_or_default() + }; + Ok(leaves) + } + fn indexed_transaction(&self, _hash: &THash) -> sp_blockchain::Result>> { + Ok(None) + } + fn block_indexed_body( + &self, + _id: BlockId, + ) -> sp_blockchain::Result>>> { + Ok(None) + } +} + +unsafe impl Send for Backend {} + +unsafe impl Sync for Backend {} + +pub(crate) struct GetBlockchainBackendMock { + pub(crate) backend: Arc, +} + +impl GetBlockchainBackend for GetBlockchainBackendMock { + type BlockchainBackend = Backend; + fn blockchain(&self) -> &Self::BlockchainBackend { + &self.backend + } +} diff --git a/finality-aleph/src/testing/mocks/header_backend.rs b/finality-aleph/src/testing/mocks/header_backend.rs deleted file mode 100644 index 4adaf2c8f2..0000000000 --- a/finality-aleph/src/testing/mocks/header_backend.rs +++ /dev/null @@ -1,109 +0,0 @@ -use sp_api::BlockId; -use sp_blockchain::{BlockStatus, HeaderBackend, Info}; -use sp_runtime::traits::Block; - -use crate::testing::mocks::{TBlock, THash, THeader, TNumber}; - -#[derive(Clone)] -pub(crate) struct Client { - blocks: Vec, - next_block_to_finalize: TBlock, -} - -pub(crate) fn create_block(parent_hash: THash, number: TNumber) -> TBlock { - TBlock { - header: THeader { - parent_hash, - number, - state_root: Default::default(), - extrinsics_root: Default::default(), - digest: Default::default(), - }, - extrinsics: vec![], - } -} - -const GENESIS_HASH: [u8; 32] = [0u8; 32]; - -impl Client { - pub(crate) fn new(finalized_height: u64) -> Self { - let mut blocks: Vec = vec![]; - - for n in 1u64..=finalized_height { - let parent_hash = match n { - 1 => GENESIS_HASH.into(), - _ => blocks.last().unwrap().header.hash(), - }; - blocks.push(create_block(parent_hash, n)); - } - - let next_block_to_finalize = - create_block(blocks.last().unwrap().hash(), finalized_height + 1); - - Client { - blocks, - next_block_to_finalize, - } - } - - pub(crate) fn next_block_to_finalize(&self) -> TBlock { - self.next_block_to_finalize.clone() - } - - pub(crate) fn get_block(&self, id: BlockId) -> Option { - match id { - BlockId::Hash(h) => { - if self.next_block_to_finalize.hash() == h { - Some(self.next_block_to_finalize.clone()) - } else { - self.blocks.iter().find(|b| b.header.hash().eq(&h)).cloned() - } - } - BlockId::Number(n) => { - if self.next_block_to_finalize.header.number == n { - Some(self.next_block_to_finalize.clone()) - } else { - self.blocks.get((n - 1) as usize).cloned() - } - } - } - } -} - -impl HeaderBackend for Client { - fn header(&self, id: BlockId) -> sp_blockchain::Result> { - Ok(self.get_block(id).map(|b| b.header)) - } - - fn info(&self) -> Info { - Info { - best_hash: self.next_block_to_finalize.hash(), - best_number: self.next_block_to_finalize.header.number, - finalized_hash: self.blocks.last().unwrap().hash(), - finalized_number: self.blocks.len() as u64, - genesis_hash: GENESIS_HASH.into(), - number_leaves: Default::default(), - finalized_state: None, - block_gap: None, - } - } - - fn status(&self, id: BlockId) -> sp_blockchain::Result { - Ok(match self.get_block(id) { - Some(_) => BlockStatus::InChain, - _ => BlockStatus::Unknown, - }) - } - - fn number(&self, hash: THash) -> sp_blockchain::Result> { - Ok(self.get_block(BlockId::hash(hash)).map(|b| b.header.number)) - } - - fn hash(&self, number: TNumber) -> sp_blockchain::Result> { - Ok(self.get_block(BlockId::Number(number)).map(|b| b.hash())) - } -} - -unsafe impl Send for Client {} - -unsafe impl Sync for Client {} diff --git a/finality-aleph/src/testing/mocks/justification_handler_config.rs b/finality-aleph/src/testing/mocks/justification_handler_config.rs index 5bea9fc419..6db2ec7f16 100644 --- a/finality-aleph/src/testing/mocks/justification_handler_config.rs +++ b/finality-aleph/src/testing/mocks/justification_handler_config.rs @@ -5,7 +5,7 @@ use std::{ use crate::{ justification::{JustificationHandlerConfig, JustificationRequestScheduler, SchedulerActions}, - testing::mocks::{single_action_mock::SingleActionMock, AcceptancePolicy, TBlock}, + testing::mocks::{single_action_mock::SingleActionMock, AcceptancePolicy}, }; #[derive(Clone)] diff --git a/finality-aleph/src/testing/mocks/mod.rs b/finality-aleph/src/testing/mocks/mod.rs index 3a95478e8d..bfb6278417 100644 --- a/finality-aleph/src/testing/mocks/mod.rs +++ b/finality-aleph/src/testing/mocks/mod.rs @@ -1,7 +1,7 @@ pub(crate) use acceptance_policy::AcceptancePolicy; +pub(crate) use backend::{create_block, Backend, GetBlockchainBackendMock}; pub(crate) use block_finalizer::MockedBlockFinalizer; pub(crate) use block_request::MockedBlockRequester; -pub(crate) use header_backend::{create_block, Client}; pub(crate) use justification_handler_config::JustificationRequestSchedulerImpl; pub(crate) use proposal::{ aleph_data_from_blocks, aleph_data_from_headers, unvalidated_proposal_from_headers, @@ -14,9 +14,9 @@ pub(crate) type THash = substrate_test_runtime::Hash; pub(crate) type TNumber = substrate_test_runtime::BlockNumber; mod acceptance_policy; +mod backend; mod block_finalizer; mod block_request; -mod header_backend; mod justification_handler_config; mod proposal; mod session_info; From 566fb28e9157fad22228f4c3cdc3620afaf2e21a Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Fri, 25 Nov 2022 19:15:35 +0100 Subject: [PATCH 24/28] bump version --- finality-aleph/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/finality-aleph/Cargo.toml b/finality-aleph/Cargo.toml index 89b014f34c..cb59ffb654 100644 --- a/finality-aleph/Cargo.toml +++ b/finality-aleph/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "finality-aleph" -version = "0.5.2" +version = "0.5.3" authors = ["Cardinal Cryptography"] edition = "2021" license = "Apache 2.0" From 9b12698962fab49dd6bb306769b584b311c26a8a Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Sun, 27 Nov 2022 23:39:37 +0100 Subject: [PATCH 25/28] fmt --- Cargo.lock | 2 +- finality-aleph/src/testing/mocks/backend.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e44f271687..8179605794 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1943,7 +1943,7 @@ dependencies = [ [[package]] name = "finality-aleph" -version = "0.5.2" +version = "0.5.3" dependencies = [ "aggregator 0.1.0", "aggregator 0.2.0", diff --git a/finality-aleph/src/testing/mocks/backend.rs b/finality-aleph/src/testing/mocks/backend.rs index a83c6405bd..b338e4438f 100644 --- a/finality-aleph/src/testing/mocks/backend.rs +++ b/finality-aleph/src/testing/mocks/backend.rs @@ -1,9 +1,10 @@ +use std::sync::Arc; + use sp_api::BlockId; use sp_blockchain::{ Backend as BlockchainBackend, BlockStatus, HeaderBackend, HeaderMetadata, Info, }; use sp_runtime::traits::Block; -use std::sync::Arc; use crate::{ testing::mocks::{TBlock, THash, THeader, TNumber}, From 3e86b2f6529c7ba3ad298ae88aad5e890f146c1d Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Mon, 28 Nov 2022 13:04:07 +0100 Subject: [PATCH 26/28] Simplify backend trait --- bin/node/src/service.rs | 38 ++++-- finality-aleph/src/justification/handler.rs | 16 +-- finality-aleph/src/justification/requester.rs | 40 ++---- finality-aleph/src/lib.rs | 14 +- finality-aleph/src/nodes/mod.rs | 16 +-- finality-aleph/src/nodes/nonvalidator_node.rs | 13 +- finality-aleph/src/nodes/validator_node.rs | 10 +- finality-aleph/src/testing/justification.rs | 19 ++- finality-aleph/src/testing/mocks/backend.rs | 123 +++--------------- finality-aleph/src/testing/mocks/mod.rs | 2 +- 10 files changed, 101 insertions(+), 190 deletions(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index 36f5e0179d..a34f6c6966 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -13,7 +13,7 @@ use finality_aleph::{ }; use futures::channel::mpsc; use log::warn; -use sc_client_api::Backend as _; +use sc_client_api::{Backend, HeaderBackend}; use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams}; use sc_network::NetworkService; use sc_service::{ @@ -22,6 +22,7 @@ use sc_service::{ }; use sc_telemetry::{Telemetry, TelemetryWorker}; use sp_api::ProvideRuntimeApi; +use sp_blockchain::Backend as _; use sp_consensus_aura::sr25519::AuthorityPair as AuraPair; use sp_runtime::{ generic::BlockId, @@ -34,7 +35,7 @@ type FullClient = sc_service::TFullClient; type FullBackend = sc_service::TFullBackend; type FullSelectChain = sc_consensus::LongestChain; -fn get_backup_path(aleph_config: &AlephCli, base_path: &Path) -> Option { +fn backup_path(aleph_config: &AlephCli, base_path: &Path) -> Option { if aleph_config.no_backup() { return None; } @@ -254,7 +255,7 @@ pub fn new_authority( other: (block_import, justification_tx, justification_rx, mut telemetry, metrics), } = new_partial(&config)?; - let backup_path = get_backup_path( + let backup_path = backup_path( &aleph_config, config .base_path @@ -340,11 +341,11 @@ pub fn new_authority( if aleph_config.external_addresses().is_empty() { panic!("Cannot run a validator node without external addresses, stopping."); } - let get_blockchain_backend = GetBlockchainBackendImpl { backend }; + let blockchain_backend = BlockchainBackendImpl { backend }; let aleph_config = AlephConfig { network, client, - get_blockchain_backend, + blockchain_backend, select_chain, session_period, millisecs_per_block, @@ -382,7 +383,7 @@ pub fn new_full( other: (_, justification_tx, justification_rx, mut telemetry, metrics), } = new_partial(&config)?; - let backup_path = get_backup_path( + let backup_path = backup_path( &aleph_config, config .base_path @@ -417,11 +418,11 @@ pub fn new_full( .unwrap(), ); - let get_blockchain_backend = GetBlockchainBackendImpl { backend }; + let blockchain_backend = BlockchainBackendImpl { backend }; let aleph_config = AlephConfig { network, client, - get_blockchain_backend, + blockchain_backend, select_chain, session_period, millisecs_per_block, @@ -445,12 +446,23 @@ pub fn new_full( Ok(task_manager) } -struct GetBlockchainBackendImpl { +struct BlockchainBackendImpl { backend: Arc, } -impl finality_aleph::GetBlockchainBackend for GetBlockchainBackendImpl { - type BlockchainBackend = >::Blockchain; - fn blockchain(&self) -> &Self::BlockchainBackend { - self.backend.blockchain() +impl finality_aleph::BlockchainBackend for BlockchainBackendImpl { + fn children(&self, parent_hash: ::Hash) -> Vec<::Hash> { + self.backend + .blockchain() + .children(parent_hash) + .unwrap_or_default() + } + fn info(&self) -> sp_blockchain::Info { + self.backend.blockchain().info() + } + fn header( + &self, + block_id: sp_api::BlockId, + ) -> sp_blockchain::Result::Header>> { + self.backend.blockchain().header(block_id) } } diff --git a/finality-aleph/src/justification/handler.rs b/finality-aleph/src/justification/handler.rs index c623e6862d..2c038fa0e2 100644 --- a/finality-aleph/src/justification/handler.rs +++ b/finality-aleph/src/justification/handler.rs @@ -13,10 +13,10 @@ use crate::{ requester::BlockRequester, JustificationHandlerConfig, JustificationNotification, JustificationRequestScheduler, SessionInfo, SessionInfoProvider, Verifier, }, - network, GetBlockchainBackend, Metrics, STATUS_REPORT_INTERVAL, + network, BlockchainBackend, Metrics, STATUS_REPORT_INTERVAL, }; -pub struct JustificationHandler +pub struct JustificationHandler where B: BlockT, V: Verifier, @@ -24,15 +24,15 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - GBB: GetBlockchainBackend + 'static, + BB: BlockchainBackend + 'static, { session_info_provider: SI, - block_requester: BlockRequester, + block_requester: BlockRequester, verifier_timeout: Duration, notification_timeout: Duration, } -impl JustificationHandler +impl JustificationHandler where B: BlockT, V: Verifier, @@ -40,12 +40,12 @@ where S: JustificationRequestScheduler, SI: SessionInfoProvider, F: BlockFinalizer, - GBB: GetBlockchainBackend + 'static, + BB: BlockchainBackend + 'static, { pub fn new( session_info_provider: SI, block_requester: RB, - get_blockchain_backend: GBB, + blockchain_backend: BB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -55,7 +55,7 @@ where session_info_provider, block_requester: BlockRequester::new( block_requester, - get_blockchain_backend, + blockchain_backend, finalizer, justification_request_scheduler, metrics, diff --git a/finality-aleph/src/justification/requester.rs b/finality-aleph/src/justification/requester.rs index 0f8ffdb28b..1bc4595df9 100644 --- a/finality-aleph/src/justification/requester.rs +++ b/finality-aleph/src/justification/requester.rs @@ -2,10 +2,7 @@ use std::{fmt, marker::PhantomData, time::Instant}; use aleph_primitives::ALEPH_ENGINE_ID; use log::{debug, error, info, warn}; -use sc_client_api::{ - blockchain::{Backend, Info}, - HeaderBackend, -}; +use sc_client_api::blockchain::Info; use sp_api::{BlockId, BlockT, NumberFor}; use sp_runtime::traits::{Header, One, Saturating}; @@ -16,7 +13,7 @@ use crate::{ JustificationRequestScheduler, Verifier, }, metrics::Checkpoint, - network, BlockHashNum, GetBlockchainBackend, Metrics, + network, BlockHashNum, BlockchainBackend, Metrics, }; /// Threshold for how many tries are needed so that JustificationRequestStatus is logged @@ -92,17 +89,17 @@ impl fmt::Display for JustificationRequestStatus { } } -pub struct BlockRequester +pub struct BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - GBB: GetBlockchainBackend + 'static, + BB: BlockchainBackend + 'static, { block_requester: RB, - get_blockchain_backend: GBB, + blockchain_backend: BB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, @@ -110,25 +107,25 @@ where _phantom: PhantomData, } -impl BlockRequester +impl BlockRequester where B: BlockT, RB: network::RequestBlocks + 'static, S: JustificationRequestScheduler, F: BlockFinalizer, V: Verifier, - GBB: GetBlockchainBackend + 'static, + BB: BlockchainBackend + 'static, { pub fn new( block_requester: RB, - get_blockchain_backend: GBB, + blockchain_backend: BB, finalizer: F, justification_request_scheduler: S, metrics: Option::Hash>>, ) -> Self { BlockRequester { block_requester, - get_blockchain_backend, + blockchain_backend, finalizer, justification_request_scheduler, metrics, @@ -189,7 +186,7 @@ where pub fn request_justification(&mut self, wanted: NumberFor) { match self.justification_request_scheduler.schedule_action() { SchedulerActions::Request => { - let info = self.get_blockchain_backend.blockchain().info(); + let info = self.blockchain_backend.info(); self.request_children(&info); self.request_wanted(wanted, &info); } @@ -202,10 +199,7 @@ where } pub fn finalized_number(&self) -> NumberFor { - self.get_blockchain_backend - .blockchain() - .info() - .finalized_number + self.blockchain_backend.info().finalized_number } fn do_request(&mut self, hash: &::Hash, num: NumberFor) { @@ -224,11 +218,7 @@ where let finalized_hash = info.finalized_hash; let finalized_number = info.finalized_number; - let children = self - .get_blockchain_backend - .blockchain() - .children(finalized_hash) - .unwrap_or_default(); + let children = self.blockchain_backend.children(finalized_hash); if !children.is_empty() { self.request_status @@ -254,11 +244,7 @@ where if top_wanted <= finalized_number + NumberFor::::one() { return; } - match self - .get_blockchain_backend - .blockchain() - .header(BlockId::Number(top_wanted)) - { + match self.blockchain_backend.header(BlockId::Number(top_wanted)) { Ok(Some(header)) => { let hash = header.hash(); let num = *header.number(); diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs index 224bc57aeb..7dc23e2bc9 100644 --- a/finality-aleph/src/lib.rs +++ b/finality-aleph/src/lib.rs @@ -243,10 +243,10 @@ impl From<(H, N)> for HashNum { pub type BlockHashNum = HashNum<::Hash, NumberFor>; -pub struct AlephConfig { +pub struct AlephConfig { pub network: Arc>, pub client: Arc, - pub get_blockchain_backend: GBB, + pub blockchain_backend: BB, pub select_chain: SC, pub spawn_handle: SpawnTaskHandle, pub keystore: Arc, @@ -260,7 +260,11 @@ pub struct AlephConfig { pub validator_port: u16, } -pub trait GetBlockchainBackend { - type BlockchainBackend: sp_blockchain::Backend; - fn blockchain(&self) -> &Self::BlockchainBackend; +pub trait BlockchainBackend { + fn children(&self, parent_hash: ::Hash) -> Vec<::Hash>; + fn info(&self) -> sp_blockchain::Info; + fn header( + &self, + block_id: sp_api::BlockId, + ) -> sp_blockchain::Result::Header>>; } diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs index 1c2e76b9dc..91e063284d 100644 --- a/finality-aleph/src/nodes/mod.rs +++ b/finality-aleph/src/nodes/mod.rs @@ -27,7 +27,7 @@ use crate::{ mpsc::UnboundedSender, session_id_from_block_num, session_map::ReadOnlySessionMap, - GetBlockchainBackend, JustificationNotification, Metrics, MillisecsPerBlock, SessionPeriod, + BlockchainBackend, JustificationNotification, Metrics, MillisecsPerBlock, SessionPeriod, }; #[cfg(test)] @@ -84,10 +84,10 @@ impl Verifier for JustificationVerifier { } } -struct JustificationParams { +struct JustificationParams { pub network: Arc>, pub client: Arc, - pub get_blockchain_backend: GBB, + pub blockchain_backend: BB, pub justification_rx: mpsc::UnboundedReceiver>, pub metrics: Option::Hash>>, pub session_period: SessionPeriod, @@ -128,8 +128,8 @@ impl SessionInfoProvider for SessionInfoProv } } -fn setup_justification_handler( - just_params: JustificationParams, +fn setup_justification_handler( + just_params: JustificationParams, ) -> ( UnboundedSender>, impl Future, @@ -140,12 +140,12 @@ where C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, BE: Backend + 'static, - GBB: GetBlockchainBackend + 'static + Send, + BB: BlockchainBackend + 'static + Send, { let JustificationParams { network, client, - get_blockchain_backend, + blockchain_backend, justification_rx, metrics, session_period, @@ -156,7 +156,7 @@ where let handler = JustificationHandler::new( SessionInfoProviderImpl::new(session_map, session_period), network, - get_blockchain_backend, + blockchain_backend, AlephFinalizer::new(client), JustificationRequestSchedulerImpl::new(&session_period, &millisecs_per_block, MAX_ATTEMPTS), metrics, diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs index 8cfe7c8fb0..a768f1cfd5 100644 --- a/finality-aleph/src/nodes/nonvalidator_node.rs +++ b/finality-aleph/src/nodes/nonvalidator_node.rs @@ -7,24 +7,23 @@ use sp_runtime::traits::Block; use crate::{ nodes::{setup_justification_handler, JustificationParams}, session_map::{AuthorityProviderImpl, FinalityNotificatorImpl, SessionMapUpdater}, - AlephConfig, GetBlockchainBackend, + AlephConfig, BlockchainBackend, }; -pub async fn run_nonvalidator_node( - aleph_config: AlephConfig, -) where +pub async fn run_nonvalidator_node(aleph_config: AlephConfig) +where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, BE: Backend + 'static, - GBB: GetBlockchainBackend + Send + 'static, + BB: BlockchainBackend + Send + 'static, SC: SelectChain + 'static, { let AlephConfig { network, client, - get_blockchain_backend, + blockchain_backend, metrics, session_period, millisecs_per_block, @@ -45,7 +44,7 @@ pub async fn run_nonvalidator_node( justification_rx, network, client, - get_blockchain_backend, + blockchain_backend, metrics, session_period, millisecs_per_block, diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs index 5bd15eddb7..ec06def4bb 100644 --- a/finality-aleph/src/nodes/validator_node.rs +++ b/finality-aleph/src/nodes/validator_node.rs @@ -24,7 +24,7 @@ use crate::{ session_map::{AuthorityProviderImpl, FinalityNotificatorImpl, SessionMapUpdater}, tcp_network::{new_tcp_network, KEY_TYPE}, validator_network::Service, - AlephConfig, GetBlockchainBackend, + AlephConfig, BlockchainBackend, }; pub async fn new_pen(mnemonic: &str, keystore: Arc) -> AuthorityPen { @@ -37,20 +37,20 @@ pub async fn new_pen(mnemonic: &str, keystore: Arc) -> Authorit .expect("we just generated this key so everything should work") } -pub async fn run_validator_node(aleph_config: AlephConfig) +pub async fn run_validator_node(aleph_config: AlephConfig) where B: Block, H: ExHashT, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: aleph_primitives::AlephSessionApi, BE: Backend + 'static, - GBB: GetBlockchainBackend + Send + 'static, + BB: BlockchainBackend + Send + 'static, SC: SelectChain + 'static, { let AlephConfig { network, client, - get_blockchain_backend, + blockchain_backend, select_chain, spawn_handle, keystore, @@ -108,7 +108,7 @@ where justification_rx, network: network.clone(), client: client.clone(), - get_blockchain_backend, + blockchain_backend, metrics: metrics.clone(), session_period, millisecs_per_block, diff --git a/finality-aleph/src/testing/justification.rs b/finality-aleph/src/testing/justification.rs index 1be9fdf51e..fe8d314806 100644 --- a/finality-aleph/src/testing/justification.rs +++ b/finality-aleph/src/testing/justification.rs @@ -1,4 +1,4 @@ -use std::{cell::RefCell, collections::VecDeque, sync::Arc, time::Duration}; +use std::{cell::RefCell, collections::VecDeque, time::Duration}; use futures::{ channel::mpsc::{unbounded, UnboundedSender}, @@ -12,11 +12,11 @@ use AcceptancePolicy::*; use crate::{ justification::{AlephJustification, JustificationHandler, JustificationHandlerConfig}, testing::mocks::{ - create_block, AcceptancePolicy, Backend, GetBlockchainBackendMock, + create_block, AcceptancePolicy, Backend, JustificationRequestSchedulerImpl, MockedBlockFinalizer, MockedBlockRequester, SessionInfoProviderImpl, TBlock, VerifierWrapper, }, - JustificationNotification, SessionPeriod, SignatureSet, + JustificationNotification, SessionPeriod, SignatureSet }; const SESSION_PERIOD: SessionPeriod = SessionPeriod(5u32); @@ -29,12 +29,12 @@ type TJustHandler = JustificationHandler< JustificationRequestSchedulerImpl, SessionInfoProviderImpl, MockedBlockFinalizer, - GetBlockchainBackendMock, + Backend, >; type Sender = UnboundedSender>; type Environment = ( TJustHandler, - Arc, + Backend, MockedBlockRequester, MockedBlockFinalizer, JustificationRequestSchedulerImpl, @@ -67,10 +67,7 @@ fn prepare_env( verification_policy: AcceptancePolicy, request_policy: AcceptancePolicy, ) -> Environment { - let backend = Arc::new(Backend::new(finalization_height)); - let get_blockchain_backend = GetBlockchainBackendMock { - backend: backend.clone(), - }; + let backend = Backend::new(finalization_height); let info_provider = SessionInfoProviderImpl::new(SESSION_PERIOD, verification_policy); let finalizer = MockedBlockFinalizer::new(); let requester = MockedBlockRequester::new(); @@ -80,7 +77,7 @@ fn prepare_env( let justification_handler = JustificationHandler::new( info_provider, requester.clone(), - get_blockchain_backend, + backend.clone(), finalizer.clone(), justification_request_scheduler.clone(), None, @@ -128,7 +125,7 @@ where S: FnOnce( Sender, Sender, - Arc, + Backend, MockedBlockRequester, MockedBlockFinalizer, JustificationRequestSchedulerImpl, diff --git a/finality-aleph/src/testing/mocks/backend.rs b/finality-aleph/src/testing/mocks/backend.rs index b338e4438f..a2365ad7b4 100644 --- a/finality-aleph/src/testing/mocks/backend.rs +++ b/finality-aleph/src/testing/mocks/backend.rs @@ -1,14 +1,10 @@ -use std::sync::Arc; - use sp_api::BlockId; -use sp_blockchain::{ - Backend as BlockchainBackend, BlockStatus, HeaderBackend, HeaderMetadata, Info, -}; +use sp_blockchain::Info; use sp_runtime::traits::Block; use crate::{ testing::mocks::{TBlock, THash, THeader, TNumber}, - GetBlockchainBackend, + BlockchainBackend, }; #[derive(Clone)] @@ -77,86 +73,9 @@ impl Backend { } } -impl HeaderBackend for Backend { - fn header(&self, id: BlockId) -> sp_blockchain::Result> { - Ok(self.get_block(id).map(|b| b.header)) - } - - fn info(&self) -> Info { - Info { - best_hash: self.next_block_to_finalize.hash(), - best_number: self.next_block_to_finalize.header.number, - finalized_hash: self.blocks.last().unwrap().hash(), - finalized_number: self.blocks.len() as u64, - genesis_hash: GENESIS_HASH.into(), - number_leaves: Default::default(), - finalized_state: None, - block_gap: None, - } - } - - fn status(&self, id: BlockId) -> sp_blockchain::Result { - Ok(match self.get_block(id) { - Some(_) => BlockStatus::InChain, - _ => BlockStatus::Unknown, - }) - } - - fn number(&self, hash: THash) -> sp_blockchain::Result> { - Ok(self.get_block(BlockId::hash(hash)).map(|b| b.header.number)) - } - - fn hash(&self, number: TNumber) -> sp_blockchain::Result> { - Ok(self.get_block(BlockId::Number(number)).map(|b| b.hash())) - } -} - -impl HeaderMetadata for Backend { - type Error = sp_blockchain::Error; - fn header_metadata( - &self, - _hash: ::Hash, - ) -> Result, Self::Error> { - Err(sp_blockchain::Error::Backend( - "Header metadata not implemented".into(), - )) - } - fn insert_header_metadata( - &self, - _hash: ::Hash, - _header_metadata: sp_blockchain::CachedHeaderMetadata, - ) { - } - fn remove_header_metadata(&self, _hash: ::Hash) {} -} - impl BlockchainBackend for Backend { - fn body( - &self, - _id: BlockId, - ) -> sp_blockchain::Result::Extrinsic>>> { - Ok(None) - } - fn justifications( - &self, - _id: BlockId, - ) -> sp_blockchain::Result> { - Ok(None) - } - fn last_finalized(&self) -> sp_blockchain::Result<::Hash> { - Ok(self.info().finalized_hash) - } - fn leaves(&self) -> sp_blockchain::Result::Hash>> { - Ok(vec![self.next_block_to_finalize.hash()]) - } - fn displaced_leaves_after_finalizing( - &self, - _block_number: sp_api::NumberFor, - ) -> sp_blockchain::Result::Hash>> { - Ok(Vec::new()) - } - fn children(&self, parent_hash: THash) -> sp_blockchain::Result> { - let leaves = if self.next_block_to_finalize.hash() == parent_hash { + fn children(&self, parent_hash: THash) -> Vec { + if self.next_block_to_finalize.hash() == parent_hash { Vec::new() } else if self .blocks @@ -173,31 +92,25 @@ impl BlockchainBackend for Backend { .find(|[parent, _]| parent.header.hash().eq(&parent_hash)) .map(|[_, c]| vec![c.hash()]) .unwrap_or_default() - }; - Ok(leaves) + } } - fn indexed_transaction(&self, _hash: &THash) -> sp_blockchain::Result>> { - Ok(None) + fn header(&self, id: BlockId) -> sp_blockchain::Result> { + Ok(self.get_block(id).map(|b| b.header)) } - fn block_indexed_body( - &self, - _id: BlockId, - ) -> sp_blockchain::Result>>> { - Ok(None) + fn info(&self) -> Info { + Info { + best_hash: self.next_block_to_finalize.hash(), + best_number: self.next_block_to_finalize.header.number, + finalized_hash: self.blocks.last().unwrap().hash(), + finalized_number: self.blocks.len() as u64, + genesis_hash: GENESIS_HASH.into(), + number_leaves: Default::default(), + finalized_state: None, + block_gap: None, + } } } unsafe impl Send for Backend {} unsafe impl Sync for Backend {} - -pub(crate) struct GetBlockchainBackendMock { - pub(crate) backend: Arc, -} - -impl GetBlockchainBackend for GetBlockchainBackendMock { - type BlockchainBackend = Backend; - fn blockchain(&self) -> &Self::BlockchainBackend { - &self.backend - } -} diff --git a/finality-aleph/src/testing/mocks/mod.rs b/finality-aleph/src/testing/mocks/mod.rs index bfb6278417..dee1ad4e89 100644 --- a/finality-aleph/src/testing/mocks/mod.rs +++ b/finality-aleph/src/testing/mocks/mod.rs @@ -1,5 +1,5 @@ pub(crate) use acceptance_policy::AcceptancePolicy; -pub(crate) use backend::{create_block, Backend, GetBlockchainBackendMock}; +pub(crate) use backend::{create_block, Backend}; pub(crate) use block_finalizer::MockedBlockFinalizer; pub(crate) use block_request::MockedBlockRequester; pub(crate) use justification_handler_config::JustificationRequestSchedulerImpl; From c33ce97b582b38dd92bb2416433f165b3930140c Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Mon, 28 Nov 2022 13:52:09 +0000 Subject: [PATCH 27/28] fmt --- finality-aleph/src/testing/justification.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/finality-aleph/src/testing/justification.rs b/finality-aleph/src/testing/justification.rs index fe8d314806..9cb3fd4373 100644 --- a/finality-aleph/src/testing/justification.rs +++ b/finality-aleph/src/testing/justification.rs @@ -12,11 +12,11 @@ use AcceptancePolicy::*; use crate::{ justification::{AlephJustification, JustificationHandler, JustificationHandlerConfig}, testing::mocks::{ - create_block, AcceptancePolicy, Backend, - JustificationRequestSchedulerImpl, MockedBlockFinalizer, MockedBlockRequester, - SessionInfoProviderImpl, TBlock, VerifierWrapper, + create_block, AcceptancePolicy, Backend, JustificationRequestSchedulerImpl, + MockedBlockFinalizer, MockedBlockRequester, SessionInfoProviderImpl, TBlock, + VerifierWrapper, }, - JustificationNotification, SessionPeriod, SignatureSet + JustificationNotification, SessionPeriod, SignatureSet, }; const SESSION_PERIOD: SessionPeriod = SessionPeriod(5u32); From ec88fadb376bc7fb2976fc219e7b217dcd8346fd Mon Sep 17 00:00:00 2001 From: Michal Swietek Date: Mon, 28 Nov 2022 14:16:02 +0000 Subject: [PATCH 28/28] Missing generic --- bin/node/src/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/src/service.rs b/bin/node/src/service.rs index a34f6c6966..561899ef35 100644 --- a/bin/node/src/service.rs +++ b/bin/node/src/service.rs @@ -461,7 +461,7 @@ impl finality_aleph::BlockchainBackend for BlockchainBackendImpl { } fn header( &self, - block_id: sp_api::BlockId, + block_id: sp_api::BlockId, ) -> sp_blockchain::Result::Header>> { self.backend.blockchain().header(block_id) }