diff --git a/finality-aleph/src/abft/current/mod.rs b/finality-aleph/src/abft/current/mod.rs index de87bbcf9b..b69ee986dd 100644 --- a/finality-aleph/src/abft/current/mod.rs +++ b/finality-aleph/src/abft/current/mod.rs @@ -17,6 +17,7 @@ use crate::{ common::{unit_creation_delay_fn, MAX_ROUNDS, SESSION_LEN_LOWER_BOUND_MS}, NetworkWrapper, }, + block::{Header as BlockHeader, HeaderVerifier, UnverifiedHeader}, crypto::Signature, data_io::{AlephData, OrderedDataInterpreter, SubstrateChainInfoProvider}, network::data::Network, @@ -28,23 +29,31 @@ use crate::{ CurrentNetworkData, Hasher, Keychain, NodeIndex, SessionId, SignatureSet, UnitCreationDelay, }; -pub fn run_member( +type WrappedNetwork = NetworkWrapper< + current_aleph_bft::NetworkData, Signature, SignatureSet>, + ADN, +>; + +pub fn run_member( subtask_common: TaskCommon, multikeychain: Keychain, config: Config, - network: NetworkWrapper< - current_aleph_bft::NetworkData>, - ADN, + network: WrappedNetwork, + data_provider: impl current_aleph_bft::DataProvider> + Send + 'static, + ordered_data_interpreter: OrderedDataInterpreter< + SubstrateChainInfoProvider, + B::Header, + V, >, - data_provider: impl current_aleph_bft::DataProvider + Send + 'static, - ordered_data_interpreter: OrderedDataInterpreter>, backup: ABFTBackup, ) -> Task where B: Block, - B::Header: Header, + B::Header: + Header + UnverifiedHeader + BlockHeader, C: HeaderBackend + Send + 'static, - ADN: Network + 'static, + ADN: Network> + 'static, + V: HeaderVerifier, { let TaskCommon { spawn_handle, diff --git a/finality-aleph/src/abft/current/network.rs b/finality-aleph/src/abft/current/network.rs index f28455b9c0..1d827d8aff 100644 --- a/finality-aleph/src/abft/current/network.rs +++ b/finality-aleph/src/abft/current/network.rs @@ -1,15 +1,16 @@ use crate::{ abft::SignatureSet, + block::UnverifiedHeader, crypto::Signature, data_io::{AlephData, AlephNetworkMessage}, Hasher, }; -pub type NetworkData = - current_aleph_bft::NetworkData>; +pub type NetworkData = + current_aleph_bft::NetworkData, Signature, SignatureSet>; -impl AlephNetworkMessage for NetworkData { - fn included_data(&self) -> Vec { +impl AlephNetworkMessage for NetworkData { + fn included_data(&self) -> Vec> { self.included_data() } } diff --git a/finality-aleph/src/abft/current/traits.rs b/finality-aleph/src/abft/current/traits.rs index da597ccc8d..4162baace0 100644 --- a/finality-aleph/src/abft/current/traits.rs +++ b/finality-aleph/src/abft/current/traits.rs @@ -1,18 +1,28 @@ //! Implementations and definitions of traits used in current abft -use crate::data_io::{AlephData, ChainInfoProvider, DataProvider, OrderedDataInterpreter}; +use crate::{ + block::{Header, HeaderVerifier, UnverifiedHeader}, + data_io::{AlephData, ChainInfoProvider, DataProvider, OrderedDataInterpreter}, +}; #[async_trait::async_trait] -impl current_aleph_bft::DataProvider for DataProvider { - async fn get_data(&mut self) -> Option { +impl current_aleph_bft::DataProvider> for DataProvider { + async fn get_data(&mut self) -> Option> { DataProvider::get_data(self).await } } -impl current_aleph_bft::FinalizationHandler for OrderedDataInterpreter +impl current_aleph_bft::FinalizationHandler> + for OrderedDataInterpreter where CIP: ChainInfoProvider, + H: Header, + V: HeaderVerifier, { - fn data_finalized(&mut self, data: AlephData, _creator: current_aleph_bft::NodeIndex) { + fn data_finalized( + &mut self, + data: AlephData, + _creator: current_aleph_bft::NodeIndex, + ) { OrderedDataInterpreter::data_finalized(self, data) } } diff --git a/finality-aleph/src/block/mock/backend.rs b/finality-aleph/src/block/mock/backend.rs index 8a782ef9d7..b2f15344c5 100644 --- a/finality-aleph/src/block/mock/backend.rs +++ b/finality-aleph/src/block/mock/backend.rs @@ -12,7 +12,7 @@ use crate::{ mock::{MockBlock, MockHeader, MockJustification, MockNotification}, Block, BlockImport, BlockStatus, ChainStatus, ChainStatusNotifier, EquivocationProof as EquivocationProofT, FinalizationStatus, Finalizer, Header, - Justification as JustificationT, VerifiedHeader, Verifier, + HeaderVerifier, Justification as JustificationT, JustificationVerifier, VerifiedHeader, }, nodes::VERIFIER_CACHE_SIZE, session::{SessionBoundaryInfo, SessionId}, @@ -414,8 +414,7 @@ impl Display for VerifierError { } } -impl Verifier for Backend { - type EquivocationProof = EquivocationProof; +impl JustificationVerifier for Backend { type Error = VerifierError; fn verify_justification( @@ -445,12 +444,17 @@ impl Verifier for Backend { false => Err(Self::Error::Justification), } } +} + +impl HeaderVerifier for Backend { + type EquivocationProof = EquivocationProof; + type Error = VerifierError; fn verify_header( &mut self, header: MockHeader, _just_created: bool, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { match (header.valid(), header.equivocated()) { (true, false) => Ok(VerifiedHeader { header, diff --git a/finality-aleph/src/block/mod.rs b/finality-aleph/src/block/mod.rs index 355c63bc42..a2ee9aa6c2 100644 --- a/finality-aleph/src/block/mod.rs +++ b/finality-aleph/src/block/mod.rs @@ -42,7 +42,7 @@ impl Display for BlockId { } /// The unverified header of a block, containing information about the parent relation. -pub trait UnverifiedHeader: Clone + Codec + Debug + Send + Sync + 'static { +pub trait UnverifiedHeader: Clone + Codec + Debug + Send + Sync + Eq + 'static { /// The identifier of this block. fn id(&self) -> BlockId; } @@ -82,6 +82,15 @@ pub trait Justification: Clone + Send + Sync + Debug + 'static { fn into_unverified(self) -> Self::Unverified; } +/// A verifier of justifications. +pub trait JustificationVerifier { + type Error: Display + Debug; + + /// Verifies the raw justification and returns a full justification if successful, otherwise an + /// error. + fn verify_justification(&mut self, justification: J::Unverified) -> Result; +} + pub type UnverifiedHeaderFor = <::Header as Header>::Unverified; pub trait EquivocationProof: Display { @@ -89,19 +98,15 @@ pub trait EquivocationProof: Display { fn are_we_equivocating(&self) -> bool; } -pub struct VerifiedHeader { - pub header: J::Header, +pub struct VerifiedHeader { + pub header: H, pub maybe_equivocation_proof: Option

, } -/// A verifier of justifications and headers. -pub trait Verifier { +/// A verifier of headers. +pub trait HeaderVerifier: Clone + Send + Sync + 'static { type EquivocationProof: EquivocationProof; - type Error: Display; - - /// Verifies the raw justification and returns a full justification if successful, otherwise an - /// error. - fn verify_justification(&mut self, justification: J::Unverified) -> Result; + type Error: Display + Debug; /// Verifies the raw header and returns a struct containing a full header and possibly /// an equivocation proof if successful, otherwise an error. @@ -109,9 +114,9 @@ pub trait Verifier { /// the `just_created` flag must be set to `true`. fn verify_header( &mut self, - header: UnverifiedHeaderFor, + header: H::Unverified, just_created: bool, - ) -> Result, Self::Error>; + ) -> Result, Self::Error>; } /// The block, including a header. diff --git a/finality-aleph/src/block/substrate/verification/cache.rs b/finality-aleph/src/block/substrate/verification/cache.rs index 3f2ae8a01a..de86c0d041 100644 --- a/finality-aleph/src/block/substrate/verification/cache.rs +++ b/finality-aleph/src/block/substrate/verification/cache.rs @@ -25,7 +25,7 @@ use crate::{ }, InnerJustification, Justification, }, - Header as HeaderT, VerifiedHeader, Verifier, + Header as HeaderT, HeaderVerifier, JustificationVerifier, VerifiedHeader, }, session::{SessionBoundaryInfo, SessionId}, session_map::AuthorityProvider, @@ -36,7 +36,7 @@ use crate::{ const HEADER_VERIFICATION_SLOT_OFFSET: u64 = 10; /// Ways in which a justification can fail verification. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Clone)] pub enum CacheError { UnknownAuthorities(SessionId), UnknownAuraAuthorities(SessionId), @@ -79,6 +79,7 @@ impl Display for CacheError { } } +#[derive(Clone)] struct CachedData { session_verifier: SessionVerifier, aura_authorities: Vec<(Option, AuraId)>, @@ -125,6 +126,7 @@ fn download_data( /// If the session is too new or ancient it will fail to return requested data. /// Highest session verifier this cache returns is for the session after the current finalization session. /// Lowest session verifier this cache returns is for `top_returned_session` - `cache_size`. +#[derive(Clone)] pub struct VerifierCache where AP: AuthorityProvider, @@ -339,12 +341,11 @@ where } } -impl Verifier for VerifierCache +impl JustificationVerifier for VerifierCache where AP: AuthorityProvider, FS: FinalizationInfo, { - type EquivocationProof = EquivocationProof; type Error = VerificationError; fn verify_justification( @@ -364,12 +365,21 @@ where }, } } +} + +impl HeaderVerifier

for VerifierCache +where + AP: AuthorityProvider, + FS: FinalizationInfo, +{ + type Error = VerificationError; + type EquivocationProof = EquivocationProof; fn verify_header( &mut self, mut header: Header, just_created: bool, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { // compare genesis header directly to the one we know if header.number().is_zero() { return match header == self.genesis_header { @@ -397,7 +407,10 @@ where #[cfg(test)] mod tests { - use std::{cell::Cell, collections::HashMap}; + use std::{ + collections::HashMap, + sync::{Arc, Mutex}, + }; use sp_runtime::testing::UintAuthorityId; @@ -415,19 +428,20 @@ mod tests { const SESSION_PERIOD: u32 = 30; const CACHE_SIZE: usize = 3; - type TestVerifierCache<'a> = - VerifierCache, MockHeader>; + type TestVerifierCache = VerifierCache; - struct MockFinalizationInfo<'a> { - finalized_number: &'a Cell, + #[derive(Clone)] + struct MockFinalizationInfo { + finalized_number: Arc>, } - impl<'a> FinalizationInfo for MockFinalizationInfo<'a> { + impl FinalizationInfo for MockFinalizationInfo { fn finalized_number(&self) -> BlockNumber { - self.finalized_number.get() + *self.finalized_number.lock().expect("mutex works") } } + #[derive(Clone)] struct MockAuthorityProvider { session_map: HashMap, aura_authority_map: HashMap>, @@ -499,7 +513,7 @@ mod tests { } } - fn setup_test(max_session_n: u32, finalized_number: &'_ Cell) -> TestVerifierCache<'_> { + fn setup_test(max_session_n: u32, finalized_number: Arc>) -> TestVerifierCache { let finalization_info = MockFinalizationInfo { finalized_number }; let authority_provider = MockAuthorityProvider::new(max_session_n); let genesis_header = MockHeader::random_parentless(0); @@ -513,8 +527,8 @@ mod tests { ) } - fn finalize_first_in_session(finalized_number: &Cell, session_id: u32) { - finalized_number.set(session_id * SESSION_PERIOD); + fn finalize_first_in_session(finalized_number: Arc>, session_id: u32) { + *finalized_number.lock().expect("mutex works") = session_id * SESSION_PERIOD; } fn session_verifier( @@ -533,28 +547,28 @@ mod tests { #[test] fn genesis_session() { - let finalized_number = Cell::new(0); + let finalized_number = Arc::new(Mutex::new(0)); - let mut verifier = setup_test(0, &finalized_number); + let mut verifier = setup_test(0, finalized_number); check_session_verifier(&mut verifier, 0); } #[test] fn normal_session() { - let finalized_number = Cell::new(0); + let finalized_number = Arc::new(Mutex::new(0)); - let mut verifier = setup_test(3, &finalized_number); + let mut verifier = setup_test(3, finalized_number.clone()); check_session_verifier(&mut verifier, 0); check_session_verifier(&mut verifier, 1); - finalize_first_in_session(&finalized_number, 1); + finalize_first_in_session(finalized_number.clone(), 1); check_session_verifier(&mut verifier, 0); check_session_verifier(&mut verifier, 1); check_session_verifier(&mut verifier, 2); - finalize_first_in_session(&finalized_number, 2); + finalize_first_in_session(finalized_number, 2); check_session_verifier(&mut verifier, 1); check_session_verifier(&mut verifier, 2); check_session_verifier(&mut verifier, 3); @@ -564,17 +578,17 @@ mod tests { fn prunes_old_sessions() { assert_eq!(CACHE_SIZE, 3); - let finalized_number = Cell::new(0); + let finalized_number = Arc::new(Mutex::new(0)); - let mut verifier = setup_test(4, &finalized_number); + let mut verifier = setup_test(4, finalized_number.clone()); check_session_verifier(&mut verifier, 0); check_session_verifier(&mut verifier, 1); - finalize_first_in_session(&finalized_number, 1); + finalize_first_in_session(finalized_number.clone(), 1); check_session_verifier(&mut verifier, 2); - finalize_first_in_session(&finalized_number, 2); + finalize_first_in_session(finalized_number.clone(), 2); check_session_verifier(&mut verifier, 3); // Should no longer have verifier for session 0 @@ -583,7 +597,7 @@ mod tests { Err(CacheError::SessionTooOld(SessionId(0), SessionId(1))) ); - finalize_first_in_session(&finalized_number, 3); + finalize_first_in_session(finalized_number, 3); check_session_verifier(&mut verifier, 4); // Should no longer have verifier for session 1 @@ -595,11 +609,11 @@ mod tests { #[test] fn session_from_future() { - let finalized_number = Cell::new(0); + let finalized_number = Arc::new(Mutex::new(0)); - let mut verifier = setup_test(3, &finalized_number); + let mut verifier = setup_test(3, finalized_number.clone()); - finalize_first_in_session(&finalized_number, 1); + finalize_first_in_session(finalized_number, 1); // Did not finalize first block in session 2 yet assert_eq!( @@ -610,15 +624,15 @@ mod tests { #[test] fn authority_provider_error() { - let finalized_number = Cell::new(0); - let mut verifier = setup_test(0, &finalized_number); + let finalized_number = Arc::new(Mutex::new(0)); + let mut verifier = setup_test(0, finalized_number.clone()); assert_eq!( session_verifier(&mut verifier, 1), Err(CacheError::UnknownAuthorities(SessionId(1))) ); - finalize_first_in_session(&finalized_number, 1); + finalize_first_in_session(finalized_number, 1); assert_eq!( session_verifier(&mut verifier, 2), diff --git a/finality-aleph/src/block/substrate/verification/mod.rs b/finality-aleph/src/block/substrate/verification/mod.rs index 20e8ffbebb..ade75e852b 100644 --- a/finality-aleph/src/block/substrate/verification/mod.rs +++ b/finality-aleph/src/block/substrate/verification/mod.rs @@ -23,20 +23,26 @@ pub use cache::VerifierCache; pub use verifier::SessionVerifier; /// Supplies finalized number. Will be unified together with other traits we used in A0-1839. -pub trait FinalizationInfo { +pub trait FinalizationInfo: Clone + Send + Sync + 'static { fn finalized_number(&self) -> BlockNumber; } /// Substrate specific implementation of `FinalizationInfo` pub struct SubstrateFinalizationInfo>(Arc); +impl> Clone for SubstrateFinalizationInfo { + fn clone(&self) -> Self { + SubstrateFinalizationInfo(self.0.clone()) + } +} + impl> SubstrateFinalizationInfo { pub fn new(client: Arc) -> Self { Self(client) } } -impl> FinalizationInfo for SubstrateFinalizationInfo { +impl + 'static> FinalizationInfo for SubstrateFinalizationInfo { fn finalized_number(&self) -> BlockNumber { self.0.info().finalized_number } diff --git a/finality-aleph/src/data_io/data_interpreter.rs b/finality-aleph/src/data_io/data_interpreter.rs index 522ecf9538..c56ddeb1c6 100644 --- a/finality-aleph/src/data_io/data_interpreter.rs +++ b/finality-aleph/src/data_io/data_interpreter.rs @@ -1,9 +1,10 @@ -use std::default::Default; +use std::{default::Default, marker::PhantomData}; use futures::channel::mpsc; use log::{debug, error, warn}; use crate::{ + block::{Header, HeaderVerifier}, data_io::{ chain_info::{AuxFinalizationChainInfoProvider, CachedChainInfoProvider}, proposal::ProposalStatus, @@ -20,14 +21,18 @@ type InterpretersChainInfoProvider = /// Takes as input ordered `AlephData` from `AlephBFT` and pushes blocks that should be finalized /// to an output channel. The other end of the channel is held by the aggregator whose goal is to /// create multisignatures under the finalized blocks. -pub struct OrderedDataInterpreter +pub struct OrderedDataInterpreter where CIP: ChainInfoProvider, + H: Header, + V: HeaderVerifier, { blocks_to_finalize_tx: mpsc::UnboundedSender, chain_info_provider: InterpretersChainInfoProvider, + verifier: V, last_finalized_by_aleph: BlockId, session_boundaries: SessionBoundaries, + _phantom: PhantomData, } fn get_last_block_prev_session( @@ -51,13 +56,16 @@ where } } -impl OrderedDataInterpreter +impl OrderedDataInterpreter where CIP: ChainInfoProvider, + H: Header, + V: HeaderVerifier, { pub fn new( blocks_to_finalize_tx: mpsc::UnboundedSender, mut chain_info: CIP, + verifier: V, session_boundaries: SessionBoundaries, ) -> Self { let last_finalized_by_aleph = @@ -72,6 +80,8 @@ where chain_info_provider, last_finalized_by_aleph, session_boundaries, + verifier, + _phantom: PhantomData, } } @@ -87,7 +97,10 @@ where self.blocks_to_finalize_tx.unbounded_send(block) } - pub fn blocks_to_finalize_from_data(&mut self, new_data: AlephData) -> Vec { + pub fn blocks_to_finalize_from_data( + &mut self, + new_data: AlephData, + ) -> Vec { let unvalidated_proposal = new_data.head_proposal; let proposal = match unvalidated_proposal.validate_bounds(&self.session_boundaries) { Ok(proposal) => proposal, @@ -101,7 +114,12 @@ where // analyzed for possible safety violations. use ProposalStatus::*; - let status = get_proposal_status(&mut self.chain_info_provider, &proposal, None); + let status = get_proposal_status( + &mut self.chain_info_provider, + &mut self.verifier, + &proposal, + None, + ); match status { Finalize(blocks) => blocks, Ignore => { @@ -116,7 +134,7 @@ where } } - pub fn data_finalized(&mut self, data: AlephData) { + pub fn data_finalized(&mut self, data: AlephData) { for block in self.blocks_to_finalize_from_data(data) { self.set_last_finalized(block.clone()); self.chain_info_provider() diff --git a/finality-aleph/src/data_io/data_provider.rs b/finality-aleph/src/data_io/data_provider.rs index d7f12bd6ed..70c2e8d3cd 100644 --- a/finality-aleph/src/data_io/data_provider.rs +++ b/finality-aleph/src/data_io/data_provider.rs @@ -12,6 +12,7 @@ use sp_runtime::{ use crate::{ aleph_primitives::{BlockHash, BlockNumber}, + block::UnverifiedHeader, data_io::{proposal::UnvalidatedAlephProposal, AlephData, MAX_DATA_BRANCH_LEN}, metrics::Checkpoint, party::manager::Runnable, @@ -56,16 +57,22 @@ where } } +pub enum ProposalPreparationError { + MissingHeader, + BestContradictsFinalized, +} + pub fn get_proposal( client: &C, best_block: BlockId, finalized_block: BlockId, -) -> Result +) -> Result>, ProposalPreparationError> where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader, C: HeaderBackend, { + use ProposalPreparationError::*; let mut curr_block = best_block; let mut branch = Vec::new(); while curr_block.number() > finalized_block.number() { @@ -77,17 +84,24 @@ where curr_block = get_parent(client, &curr_block).expect("block of num >= 1 must have a parent") } if curr_block.hash() == finalized_block.hash() { - let num_last = finalized_block.number() + ::saturated_from(branch.len()); - // The hashes in `branch` are ordered from top to bottom -- need to reverse. - branch.reverse(); - Ok(AlephData { - head_proposal: UnvalidatedAlephProposal::new(branch, num_last), - }) + let mut branch = branch.into_iter(); + let head_hash = match branch.next() { + Some(hash) => hash, + None => return Ok(None), + }; + let head = match client.header(head_hash) { + Ok(Some(header)) => header, + _ => return Err(MissingHeader), + }; + let tail: Vec<_> = branch.rev().collect(); + Ok(Some(AlephData { + head_proposal: UnvalidatedAlephProposal::new(head, tail), + })) } else { // By backtracking from the best block we reached a block conflicting with best finalized. // This is most likely a bug, or some extremely unlikely synchronization issue of the client. warn!(target: "aleph-data-store", "Error computing proposal. Conflicting blocks: {:?}, finalized {:?}", curr_block, finalized_block); - Err(()) + Err(BestContradictsFinalized) } } @@ -118,13 +132,13 @@ struct ChainInfo { pub struct ChainTracker where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader, C: HeaderBackend + 'static, SC: SelectChain + 'static, { select_chain: SC, client: Arc, - data_to_propose: Arc>>, + data_to_propose: Arc>>>, session_boundaries: SessionBoundaries, prev_chain_info: Option, config: ChainTrackerConfig, @@ -134,7 +148,7 @@ where impl ChainTracker where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader, C: HeaderBackend + 'static, SC: SelectChain + 'static, { @@ -144,7 +158,7 @@ where session_boundaries: SessionBoundaries, config: ChainTrackerConfig, metrics: TimingBlockMetrics, - ) -> (Self, DataProvider) { + ) -> (Self, DataProvider) { let data_to_propose = Arc::new(Mutex::new(None)); ( ChainTracker { @@ -195,11 +209,6 @@ where highest_finalized: finalized_block.clone(), }); - if best_block_in_session.number() == finalized_block.number() { - // We don't have anything to propose, we go ahead with an empty proposal. - *self.data_to_propose.lock() = None; - return; - } if best_block_in_session.number() < finalized_block.number() { // Because of the client synchronization, in extremely rare cases this could happen. warn!(target: "aleph-data-store", "Error updating data. best_block {:?} is lower than finalized {:?}.", best_block_in_session, finalized_block); @@ -211,7 +220,7 @@ where best_block_in_session.clone(), finalized_block, ) { - *self.data_to_propose.lock() = Some(proposal); + *self.data_to_propose.lock() = proposal; } } @@ -296,7 +305,7 @@ where impl Runnable for ChainTracker where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader, C: HeaderBackend + 'static, SC: SelectChain + 'static, { @@ -307,8 +316,8 @@ where /// Provides data to AlephBFT for ordering. #[derive(Clone)] -pub struct DataProvider { - data_to_propose: Arc>>, +pub struct DataProvider { + data_to_propose: Arc>>>, metrics: TimingBlockMetrics, } @@ -320,13 +329,13 @@ pub struct DataProvider { // then the node proposes `Empty`, otherwise the node proposes a branch extending from one block above // last finalized till `best_block` with the restriction that the branch must be truncated to length // at most MAX_DATA_BRANCH_LEN. -impl DataProvider { - pub async fn get_data(&mut self) -> Option { +impl DataProvider { + pub async fn get_data(&mut self) -> Option> { let data_to_propose = (*self.data_to_propose.lock()).take(); if let Some(data) = &data_to_propose { self.metrics.report_block_if_not_present( - *data.head_proposal.branch.last().unwrap(), + data.head_proposal.top_block().hash(), std::time::Instant::now(), Checkpoint::Proposed, ); @@ -351,7 +360,7 @@ mod tests { }, testing::{ client_chain_builder::ClientChainBuilder, - mocks::{aleph_data_from_blocks, TestClientBuilder, TestClientBuilderExt}, + mocks::{aleph_data_from_blocks, THeader, TestClientBuilder, TestClientBuilderExt}, }, SessionBoundaryInfo, SessionId, SessionPeriod, TimingBlockMetrics, }; @@ -365,7 +374,7 @@ mod tests { impl Future, oneshot::Sender<()>, ClientChainBuilder, - DataProvider, + DataProvider, ) { let (client, select_chain) = TestClientBuilder::new().build_with_longest_chain(); let client = Arc::new(client); @@ -406,7 +415,7 @@ mod tests { async fn run_test(scenario: S) where F: Future, - S: FnOnce(ClientChainBuilder, DataProvider) -> F, + S: FnOnce(ClientChainBuilder, DataProvider) -> F, { let (task_handle, exit, chain_builder, data_provider) = prepare_chain_tracker_test(); let chain_tracker_handle = tokio::spawn(task_handle); diff --git a/finality-aleph/src/data_io/data_store.rs b/finality-aleph/src/data_io/data_store.rs index 27e3bb4035..a6fdd07d09 100644 --- a/finality-aleph/src/data_io/data_store.rs +++ b/finality-aleph/src/data_io/data_store.rs @@ -22,6 +22,7 @@ use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use crate::{ aleph_primitives::{BlockHash, BlockNumber}, + block::{Header, HeaderVerifier, UnverifiedHeader}, data_io::{ chain_info::{CachedChainInfoProvider, ChainInfoProvider, SubstrateChainInfoProvider}, proposal::{AlephProposal, PendingProposalStatus, ProposalStatus}, @@ -65,13 +66,13 @@ impl PendingProposalInfo { } #[derive(PartialEq, Eq, Clone, Debug)] -pub struct PendingMessageInfo { +pub struct PendingMessageInfo> { message: M, // Data items that we still wait for - pending_proposals: HashSet, + pending_proposals: HashSet>, } -impl PendingMessageInfo { +impl> PendingMessageInfo { fn new(message: M) -> Self { PendingMessageInfo { message, @@ -145,13 +146,13 @@ impl Default for DataStoreConfig { /// This component is used for filtering available data for Aleph Network. /// It needs to be started by calling the run method. -pub struct DataStore +pub struct DataStore where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader + Header, C: HeaderBackend + BlockchainEvents + Send + Sync + 'static, RB: RequestBlocks + 'static, - Message: AlephNetworkMessage + Message: AlephNetworkMessage + std::fmt::Debug + Send + Sync @@ -159,15 +160,17 @@ where + parity_scale_codec::Codec + 'static, R: Receiver, + V: HeaderVerifier, { next_free_id: MessageId, - pending_proposals: HashMap, - event_triggers: HashMap>, + pending_proposals: HashMap, PendingProposalInfo>, + event_triggers: HashMap>>, // We use BtreeMap instead of HashMap to be able to fetch the Message with lowest MessageId // when pruning messages. - pending_messages: BTreeMap>, + pending_messages: BTreeMap>, chain_info_provider: CachedChainInfoProvider>, - available_proposals_cache: LruCache, + verifier: V, + available_proposals_cache: LruCache, ProposalStatus>, num_triggers_registered_since_last_pruning: usize, highest_finalized_num: BlockNumber, session_boundaries: SessionBoundaries, @@ -178,13 +181,13 @@ where messages_for_aleph: UnboundedSender, } -impl DataStore +impl DataStore where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader + Header, C: HeaderBackend + BlockchainEvents + Send + Sync + 'static, RB: RequestBlocks + 'static, - Message: AlephNetworkMessage + Message: AlephNetworkMessage + std::fmt::Debug + Send + Sync @@ -192,11 +195,13 @@ where + parity_scale_codec::Codec + 'static, R: Receiver, + V: HeaderVerifier, { /// Returns a struct to be run and a network that outputs messages filtered as appropriate pub fn new>( session_boundaries: SessionBoundaries, client: Arc, + verifier: V, block_requester: RB, config: DataStoreConfig, component_network: N, @@ -217,6 +222,7 @@ where event_triggers: HashMap::new(), pending_messages: BTreeMap::new(), chain_info_provider, + verifier, available_proposals_cache: LruCache::new(config.available_proposals_cache_capacity), num_triggers_registered_since_last_pruning: 0, highest_finalized_num, @@ -336,7 +342,11 @@ where } } - fn register_block_import_trigger(&mut self, proposal: &AlephProposal, block: &BlockId) { + fn register_block_import_trigger( + &mut self, + proposal: &AlephProposal, + block: &BlockId, + ) { self.num_triggers_registered_since_last_pruning += 1; self.event_triggers .entry(ChainEvent::Imported(block.clone())) @@ -344,7 +354,11 @@ where .insert(proposal.clone()); } - fn register_finality_trigger(&mut self, proposal: &AlephProposal, number: BlockNumber) { + fn register_finality_trigger( + &mut self, + proposal: &AlephProposal, + number: BlockNumber, + ) { self.num_triggers_registered_since_last_pruning += 1; if number > self.highest_finalized_num { self.event_triggers @@ -354,7 +368,7 @@ where } } - fn register_next_finality_trigger(&mut self, proposal: &AlephProposal) { + fn register_next_finality_trigger(&mut self, proposal: &AlephProposal) { if self.highest_finalized_num < proposal.number_below_branch() { self.register_finality_trigger(proposal, proposal.number_below_branch()); } else if self.highest_finalized_num < proposal.number_top_block() { @@ -392,7 +406,7 @@ where } } - fn on_proposal_available(&mut self, proposal: &AlephProposal) { + fn on_proposal_available(&mut self, proposal: &AlephProposal) { if let Some(proposal_info) = self.pending_proposals.remove(proposal) { for id in proposal_info.messages { self.remove_proposal_from_pending_message(proposal, id); @@ -402,7 +416,7 @@ where // Makes an availability check for `data` and updates its status. Outputs whether the bump resulted in // this proposal becoming available. - fn bump_proposal(&mut self, proposal: &AlephProposal) -> bool { + fn bump_proposal(&mut self, proposal: &AlephProposal) -> bool { // Some minor inefficiencies in HashMap access below because of borrow checker. let old_status = match self.pending_proposals.get(proposal) { None => { @@ -444,13 +458,18 @@ where // Outputs the current status of the proposal based on the `old_status` (for optimization). fn check_proposal_availability( &mut self, - proposal: &AlephProposal, + proposal: &AlephProposal, old_status: Option<&ProposalStatus>, ) -> ProposalStatus { if let Some(status) = self.available_proposals_cache.get(proposal) { return status.clone(); } - let status = get_proposal_status(&mut self.chain_info_provider, proposal, old_status); + let status = get_proposal_status( + &mut self.chain_info_provider, + &mut self.verifier, + proposal, + old_status, + ); match status { ProposalStatus::Finalize(_) | ProposalStatus::Ignore => { // We can cache only if the proposal is available. If it is pending, its @@ -472,8 +491,8 @@ where // If the proposal is available, message_info is not modified. fn add_message_proposal_dependency( &mut self, - proposal: &AlephProposal, - message_info: &mut PendingMessageInfo, + proposal: &AlephProposal, + message_info: &mut PendingMessageInfo, id: MessageId, ) { if !self.pending_proposals.contains_key(proposal) { @@ -530,7 +549,11 @@ where // This is called upon a proposal being available -- we remove it from the set of // proposals a message waits for. - fn remove_proposal_from_pending_message(&mut self, proposal: &AlephProposal, id: MessageId) { + fn remove_proposal_from_pending_message( + &mut self, + proposal: &AlephProposal, + id: MessageId, + ) { let mut message_info = match self.pending_messages.remove(&id) { Some(message_info) => message_info, None => { @@ -547,7 +570,11 @@ where } } - fn remove_message_id_from_pending_proposal(&mut self, proposal: &AlephProposal, id: MessageId) { + fn remove_message_id_from_pending_proposal( + &mut self, + proposal: &AlephProposal, + id: MessageId, + ) { if let Occupied(mut proposal_entry) = self.pending_proposals.entry(proposal.clone()) { let proposal_info = proposal_entry.get_mut(); proposal_info.messages.remove(&id); @@ -635,13 +662,13 @@ where } #[async_trait::async_trait] -impl Runnable for DataStore +impl Runnable for DataStore where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader + Header, C: HeaderBackend + BlockchainEvents + Send + Sync + 'static, RB: RequestBlocks + 'static, - Message: AlephNetworkMessage + Message: AlephNetworkMessage + std::fmt::Debug + Send + Sync @@ -649,6 +676,7 @@ where + parity_scale_codec::Codec + 'static, R: Receiver + 'static, + V: HeaderVerifier, { async fn run(mut self, exit: oneshot::Receiver<()>) { DataStore::run(&mut self, exit).await diff --git a/finality-aleph/src/data_io/legacy/data_provider.rs b/finality-aleph/src/data_io/legacy/data_provider.rs index abec3171ba..ae32a1d3e0 100644 --- a/finality-aleph/src/data_io/legacy/data_provider.rs +++ b/finality-aleph/src/data_io/legacy/data_provider.rs @@ -345,13 +345,14 @@ mod tests { use tokio::time::sleep; use crate::{ - data_io::{ + data_io::legacy::{ data_provider::{ChainTracker, ChainTrackerConfig}, + test::aleph_data_from_blocks, DataProvider, MAX_DATA_BRANCH_LEN, }, testing::{ client_chain_builder::ClientChainBuilder, - mocks::{aleph_data_from_blocks, TestClientBuilder, TestClientBuilderExt}, + mocks::{TestClientBuilder, TestClientBuilderExt}, }, SessionBoundaryInfo, SessionId, SessionPeriod, TimingBlockMetrics, }; diff --git a/finality-aleph/src/data_io/legacy/mod.rs b/finality-aleph/src/data_io/legacy/mod.rs index 7e8c0f3739..b021e534a1 100644 --- a/finality-aleph/src/data_io/legacy/mod.rs +++ b/finality-aleph/src/data_io/legacy/mod.rs @@ -1,4 +1,4 @@ -use std::{fmt::Debug, hash::Hash, num::NonZeroUsize}; +use std::{fmt::Debug, hash::Hash}; use parity_scale_codec::{Decode, Encode}; @@ -13,6 +13,8 @@ pub use data_provider::{ChainTracker, DataProvider}; pub use data_store::{DataStore, DataStoreConfig}; pub use proposal::UnvalidatedAlephProposal; +pub use super::ChainInfoCacheConfig; + // Maximum number of blocks above the last finalized allowed in an AlephBFT proposal. pub const MAX_DATA_BRANCH_LEN: usize = 7; @@ -28,15 +30,27 @@ pub trait AlephNetworkMessage: Clone + Debug { fn included_data(&self) -> Vec; } -#[derive(Clone, Debug)] -pub struct ChainInfoCacheConfig { - pub block_cache_capacity: NonZeroUsize, -} +#[cfg(test)] +mod test { + use crate::{ + data_io::legacy::{AlephData, UnvalidatedAlephProposal}, + testing::mocks::{TBlock, THeader}, + }; + + pub fn unvalidated_proposal_from_headers(headers: Vec) -> UnvalidatedAlephProposal { + let num = headers.last().unwrap().number; + let hashes = headers.into_iter().map(|header| header.hash()).collect(); + UnvalidatedAlephProposal::new(hashes, num) + } + + pub fn aleph_data_from_blocks(blocks: Vec) -> AlephData { + let headers = blocks.into_iter().map(|b| b.header).collect(); + aleph_data_from_headers(headers) + } -impl Default for ChainInfoCacheConfig { - fn default() -> ChainInfoCacheConfig { - ChainInfoCacheConfig { - block_cache_capacity: NonZeroUsize::new(2000).unwrap(), + pub fn aleph_data_from_headers(headers: Vec) -> AlephData { + AlephData { + head_proposal: unvalidated_proposal_from_headers(headers), } } } diff --git a/finality-aleph/src/data_io/legacy/status_provider.rs b/finality-aleph/src/data_io/legacy/status_provider.rs index 74cd923ed4..77a22a7eb0 100644 --- a/finality-aleph/src/data_io/legacy/status_provider.rs +++ b/finality-aleph/src/data_io/legacy/status_provider.rs @@ -156,20 +156,20 @@ mod tests { AuxFinalizationChainInfoProvider, CachedChainInfoProvider, SubstrateChainInfoProvider, }, - proposal::{ - AlephProposal, - PendingProposalStatus::*, - ProposalStatus::{self, *}, + legacy::{ + proposal::{ + AlephProposal, + PendingProposalStatus::*, + ProposalStatus::{self, *}, + }, + status_provider::get_proposal_status, + test::unvalidated_proposal_from_headers, + ChainInfoCacheConfig, MAX_DATA_BRANCH_LEN, }, - status_provider::get_proposal_status, - ChainInfoCacheConfig, MAX_DATA_BRANCH_LEN, }, testing::{ client_chain_builder::ClientChainBuilder, - mocks::{ - unvalidated_proposal_from_headers, TBlock, THeader, TestClient, TestClientBuilder, - TestClientBuilderExt, - }, + mocks::{TBlock, THeader, TestClient, TestClientBuilder, TestClientBuilderExt}, }, SessionBoundaryInfo, SessionId, SessionPeriod, }; diff --git a/finality-aleph/src/data_io/mod.rs b/finality-aleph/src/data_io/mod.rs index 1f03a45415..58dffb8f79 100644 --- a/finality-aleph/src/data_io/mod.rs +++ b/finality-aleph/src/data_io/mod.rs @@ -1,7 +1,13 @@ -use std::{fmt::Debug, hash::Hash, num::NonZeroUsize}; +use std::{ + fmt::Debug, + hash::{Hash, Hasher}, + num::NonZeroUsize, +}; use parity_scale_codec::{Decode, Encode}; +use crate::block::UnverifiedHeader; + mod chain_info; mod data_interpreter; mod data_provider; @@ -22,15 +28,21 @@ pub use proposal::UnvalidatedAlephProposal; pub const MAX_DATA_BRANCH_LEN: usize = 7; /// The data ordered by the Aleph consensus. -#[derive(Clone, Debug, Encode, Decode, Hash, PartialEq, Eq)] -pub struct AlephData { - pub head_proposal: UnvalidatedAlephProposal, +#[derive(Clone, Debug, Encode, Decode, PartialEq, Eq)] +pub struct AlephData { + pub head_proposal: UnvalidatedAlephProposal, +} + +impl Hash for AlephData { + fn hash(&self, state: &mut H) { + self.head_proposal.hash(state); + } } /// A trait allowing to check the data contained in an AlephBFT network message, for the purpose of /// data availability checks. -pub trait AlephNetworkMessage: Clone + Debug { - fn included_data(&self) -> Vec; +pub trait AlephNetworkMessage: Clone + Debug { + fn included_data(&self) -> Vec>; } #[derive(Clone, Debug)] diff --git a/finality-aleph/src/data_io/proposal.rs b/finality-aleph/src/data_io/proposal.rs index 84ce25705b..d8e4b96947 100644 --- a/finality-aleph/src/data_io/proposal.rs +++ b/finality-aleph/src/data_io/proposal.rs @@ -1,38 +1,48 @@ -use std::{cmp::max, hash::Hash, ops::Index}; +use std::{ + cmp::max, + hash::{Hash, Hasher}, + iter, +}; use parity_scale_codec::{Decode, Encode}; use sp_runtime::SaturatedConversion; use crate::{ aleph_primitives::{BlockHash, BlockNumber}, + block::UnverifiedHeader, data_io::MAX_DATA_BRANCH_LEN, BlockId, SessionBoundaries, }; /// Represents a proposal we obtain from another node. Note that since the proposal might come from /// a malicious node there is no guarantee that the block hashes in the proposal correspond to real blocks -/// and even if they do then they could not match the provided number. Moreover, the block number in the -/// proposal might be completely arbitrary and hence we perform initial validation of the block number and -/// the branch length before we transform it into a safer `AlephProposal` type that guarantees we will not -/// fail on any integer over- or underflows. -/// We expect that honest nodes create UnvalidatedAlephProposal {branch: [h_0, h_1, ..., h_n], number: num} objects -/// that represent an ascending sequence of blocks b_0, b_1, ..., b_n satisfying the following conditions: -/// 1) hash(b_i) = h_i for i = 0, 1, ..., n, +/// or encompass a branch within a session. Hence we perform initial validation of the block number and +/// the branch length before we transform it into a safer `AlephProposal` type that guarantees we +/// will not fail on any integer over- or underflows. We expect that honest nodes create +/// UnvalidatedAlephProposal {head: hd_n, tail: [h_0, h_1, ..., h_(n-1)]} objects that represent +/// an ascending sequence of blocks b_0, b_1, ..., b_n satisfying the following conditions: +/// 1) hash(b_i) = h_i for i = 0, 1, ..., n-1, /// 2) parent(b_{i+1}) = b_i for i = 0, 1, ..., (n-1), -/// 3) height(b_n) = num, +/// 3) header(b_n) = hd_n, /// 4) The parent of b_0 has been finalized (prior to creating this AlephData). /// Such an UnvalidatedAlephProposal object should be thought of as a proposal for block b_n to be finalized. /// We refer for to `DataProvider` for a precise description of honest nodes' algorithm of creating proposals. -#[derive(Clone, Debug, Encode, Decode, Hash, PartialEq, Eq)] -pub struct UnvalidatedAlephProposal { - pub branch: Vec, - pub number: BlockNumber, +#[derive(Clone, Debug, Encode, Decode, PartialEq, Eq)] +pub struct UnvalidatedAlephProposal { + head: UH, + tail: Vec, +} + +impl Hash for UnvalidatedAlephProposal { + fn hash(&self, state: &mut H) { + self.head.encode().hash(state); + self.tail.hash(state); + } } /// Represents possible invalid states as described in [UnvalidatedAlephProposal]. #[derive(Debug, PartialEq, Eq)] pub enum ValidationError { - BranchEmpty, BranchTooLong { branch_size: usize, }, @@ -48,38 +58,45 @@ pub enum ValidationError { }, } -impl UnvalidatedAlephProposal { - pub(crate) fn new(branch: Vec, block_number: BlockNumber) -> Self { - UnvalidatedAlephProposal { - branch, - number: block_number, - } +impl UnvalidatedAlephProposal { + pub fn new(head: UH, tail: Vec) -> Self { + UnvalidatedAlephProposal { head, tail } + } + + fn top_number(&self) -> BlockNumber { + self.top_block().number() + } + + /// Outputs the highest block in the branch. + pub fn top_block(&self) -> BlockId { + self.head.id() + } + + fn branch_len(&self) -> usize { + self.tail.len() + 1 } - pub(crate) fn validate_bounds( + pub fn validate_bounds( &self, session_boundaries: &SessionBoundaries, - ) -> Result { + ) -> Result, ValidationError> { use ValidationError::*; - if self.branch.len() > MAX_DATA_BRANCH_LEN { + if self.branch_len() > MAX_DATA_BRANCH_LEN { return Err(BranchTooLong { - branch_size: self.branch.len(), + branch_size: self.branch_len(), }); } - if self.branch.is_empty() { - return Err(BranchEmpty); - } - if self.number < ::saturated_from(self.branch.len()) { + if self.top_number() < ::saturated_from(self.branch_len()) { // Note that this also excludes branches starting at the genesis (0th) block. return Err(BlockNumberOutOfBounds { - branch_size: self.branch.len(), - block_number: self.number, + branch_size: self.branch_len(), + block_number: self.top_number(), }); } - let bottom_block = self.number - ::saturated_from(self.branch.len() - 1); - let top_block = self.number; + let bottom_block = self.top_number() - ::saturated_from(self.branch_len() - 1); + let top_block = self.top_number(); let session_start = session_boundaries.first_block(); let session_end = session_boundaries.last_block(); if session_start > bottom_block || top_block > session_end { @@ -92,81 +109,76 @@ impl UnvalidatedAlephProposal { } Ok(AlephProposal { - branch: self.branch.clone(), - number: self.number, + head: self.head.clone(), + tail: self.tail.clone(), }) } } /// A version of UnvalidatedAlephProposal that has been initially validated and fits /// within session bounds. -#[derive(Clone, Debug, Encode, Decode, Hash, PartialEq, Eq)] -pub struct AlephProposal { - branch: Vec, - number: BlockNumber, +#[derive(Clone, Debug, Encode, Decode, PartialEq, Eq)] +pub struct AlephProposal { + head: UH, + tail: Vec, } -impl Index for AlephProposal { - type Output = BlockHash; - fn index(&self, index: usize) -> &Self::Output { - &self.branch[index] +impl Hash for AlephProposal { + fn hash(&self, state: &mut H) { + self.head.encode().hash(state); + self.tail.hash(state); } } -impl AlephProposal { +impl AlephProposal { /// Outputs the length the branch. pub fn len(&self) -> usize { - self.branch.len() + self.tail.len() + 1 + } + + pub fn top_block_header(&self) -> UH { + self.head.clone() } /// Outputs the highest block in the branch. pub fn top_block(&self) -> BlockId { - ( - *self - .branch - .last() - .expect("cannot be empty for correct data"), - self.number_top_block(), - ) - .into() + self.top_block_header().id() } /// Outputs the lowest block in the branch. pub fn bottom_block(&self) -> BlockId { - // Assumes that the data is within bounds - ( - *self - .branch - .first() - .expect("cannot be empty for correct data"), - self.number_bottom_block(), - ) - .into() + match self.tail.first() { + Some(hash) => BlockId::new(*hash, self.number_bottom_block()), + None => self.top_block(), + } } /// Outputs the number one below the lowest block in the branch. pub fn number_below_branch(&self) -> BlockNumber { // Assumes that data is within bounds - self.number - ::saturated_from(self.branch.len()) + self.number_top_block() - ::saturated_from(self.len()) } /// Outputs the number of the lowest block in the branch. pub fn number_bottom_block(&self) -> BlockNumber { // Assumes that data is within bounds - self.number - ::saturated_from(self.branch.len() - 1) + self.number_top_block() - ::saturated_from(self.len() - 1) } /// Outputs the number of the highest block in the branch. pub fn number_top_block(&self) -> BlockNumber { - self.number + self.top_block().number() } /// Outputs the block corresponding to the number in the proposed branch in case num is /// between the lowest and highest block number of the branch. Otherwise returns None. pub fn block_at_num(&self, num: BlockNumber) -> Option { - if self.number_bottom_block() <= num && num <= self.number_top_block() { + if num == self.number_top_block() { + return Some(self.top_block()); + } + if self.number_bottom_block() <= num && num < self.number_top_block() { let ind: usize = (num - self.number_bottom_block()).saturated_into(); - return Some((self.branch[ind], num).into()); + return Some(BlockId::new(self.tail[ind], num)); } None } @@ -175,10 +187,11 @@ impl AlephProposal { /// empty, if it's too low the whole branch is returned. pub fn blocks_from_num(&self, num: BlockNumber) -> impl Iterator + '_ { let num = max(num, self.number_bottom_block()); - self.branch + self.tail .iter() - .skip((num - self.number_bottom_block()).saturated_into()) .cloned() + .chain(iter::once(self.head.id().hash())) + .skip((num - self.number_bottom_block()).saturated_into()) .zip(0u32..) .map(move |(hash, index)| (hash, num + index).into()) } @@ -200,34 +213,26 @@ pub enum ProposalStatus { #[cfg(test)] mod tests { - use sp_core::hash::H256; - use super::{UnvalidatedAlephProposal, ValidationError::*}; use crate::{ - aleph_primitives::BlockNumber, data_io::MAX_DATA_BRANCH_LEN, SessionBoundaryInfo, - SessionId, SessionPeriod, + block::{mock::MockHeader, Header}, + data_io::MAX_DATA_BRANCH_LEN, + BlockId, SessionBoundaryInfo, SessionId, SessionPeriod, }; - #[test] - fn proposal_with_empty_branch_is_invalid() { - let session_boundaries = - SessionBoundaryInfo::new(SessionPeriod(20)).boundaries_for_session(SessionId(1)); - let branch = vec![]; - let proposal = UnvalidatedAlephProposal::new(branch, session_boundaries.first_block()); - assert_eq!( - proposal.validate_bounds(&session_boundaries), - Err(BranchEmpty) - ); - } - #[test] fn too_long_proposal_is_invalid() { let session_boundaries = SessionBoundaryInfo::new(SessionPeriod(20)).boundaries_for_session(SessionId(1)); - let session_end = session_boundaries.last_block(); - let branch = vec![H256::default(); MAX_DATA_BRANCH_LEN + 1]; - let branch_size = branch.len(); - let proposal = UnvalidatedAlephProposal::new(branch, session_end); + let session_start = session_boundaries.first_block(); + let tail: Vec<_> = BlockId::new_random(session_start) + .random_branch() + .take(MAX_DATA_BRANCH_LEN) + .collect(); + let head = tail.last().unwrap().random_child(); + let tail = tail.into_iter().map(|header| header.id().hash()).collect(); + let proposal = UnvalidatedAlephProposal::new(head, tail); + let branch_size = MAX_DATA_BRANCH_LEN + 1; assert_eq!( proposal.validate_bounds(&session_boundaries), Err(BranchTooLong { branch_size }) @@ -240,9 +245,11 @@ mod tests { SessionBoundaryInfo::new(SessionPeriod(20)).boundaries_for_session(SessionId(1)); let session_start = session_boundaries.first_block(); let session_end = session_boundaries.last_block(); - let branch = vec![H256::default(); 2]; + let prev_session_block = BlockId::new_random(session_start - 1); + let head = prev_session_block.random_child(); + let tail = vec![prev_session_block.hash()]; - let proposal = UnvalidatedAlephProposal::new(branch.clone(), session_start); + let proposal = UnvalidatedAlephProposal::new(head, tail); assert_eq!( proposal.validate_bounds(&session_boundaries), Err(BlockOutsideSessionBoundaries { @@ -253,7 +260,10 @@ mod tests { }) ); - let proposal = UnvalidatedAlephProposal::new(branch, session_end + 1); + let last_session_block = BlockId::new_random(session_end); + let head = last_session_block.random_child(); + let tail = vec![last_session_block.hash()]; + let proposal = UnvalidatedAlephProposal::new(head, tail); assert_eq!( proposal.validate_bounds(&session_boundaries), Err(BlockOutsideSessionBoundaries { @@ -269,9 +279,10 @@ mod tests { fn proposal_starting_at_zero_block_is_invalid() { let session_boundaries = SessionBoundaryInfo::new(SessionPeriod(20)).boundaries_for_session(SessionId(0)); - let branch = vec![H256::default(); 2]; - - let proposal = UnvalidatedAlephProposal::new(branch, 1); + let genesis = MockHeader::genesis(); + let head = genesis.random_child(); + let tail = vec![genesis.id().hash()]; + let proposal = UnvalidatedAlephProposal::new(head, tail); assert_eq!( proposal.validate_bounds(&session_boundaries), Err(BlockNumberOutOfBounds { @@ -286,14 +297,18 @@ mod tests { let session_boundaries = SessionBoundaryInfo::new(SessionPeriod(20)).boundaries_for_session(SessionId(0)); - let branch = vec![H256::default(); MAX_DATA_BRANCH_LEN]; - let proposal = - UnvalidatedAlephProposal::new(branch, (MAX_DATA_BRANCH_LEN + 1) as BlockNumber); + let genesis = MockHeader::genesis(); + let branch: Vec<_> = genesis + .random_branch() + .take(MAX_DATA_BRANCH_LEN - 1) + .collect(); + let head = branch.last().unwrap().random_child(); + let tail = branch.iter().map(|header| header.id().hash()).collect(); + let proposal = UnvalidatedAlephProposal::new(head, tail); assert!(proposal.validate_bounds(&session_boundaries).is_ok()); - let branch = vec![H256::default(); 1]; - let proposal = - UnvalidatedAlephProposal::new(branch, (MAX_DATA_BRANCH_LEN + 1) as BlockNumber); + let head = branch.last().unwrap().random_child(); + let proposal = UnvalidatedAlephProposal::new(head, Vec::new()); assert!(proposal.validate_bounds(&session_boundaries).is_ok()); } } diff --git a/finality-aleph/src/data_io/status_provider.rs b/finality-aleph/src/data_io/status_provider.rs index af53e67ac6..cf195179ca 100644 --- a/finality-aleph/src/data_io/status_provider.rs +++ b/finality-aleph/src/data_io/status_provider.rs @@ -1,21 +1,25 @@ -use log::debug; +use log::{debug, warn}; use sp_runtime::SaturatedConversion; use crate::{ aleph_primitives::BlockNumber, + block::{Header, HeaderVerifier, UnverifiedHeader}, data_io::{ chain_info::ChainInfoProvider, proposal::{AlephProposal, PendingProposalStatus, ProposalStatus}, }, }; -pub fn get_proposal_status( +pub fn get_proposal_status( chain_info_provider: &mut CIP, - proposal: &AlephProposal, + header_verifier: &mut V, + proposal: &AlephProposal, old_status: Option<&ProposalStatus>, ) -> ProposalStatus where CIP: ChainInfoProvider, + H: Header, + V: HeaderVerifier, { use PendingProposalStatus::*; use ProposalStatus::*; @@ -33,7 +37,17 @@ where let old_status = match old_status { Some(status) => status, - None => &Pending(PendingTopBlock), + None => { + // Verify header here, so it happens at most once. Incorrect headers are equivalent to a broken branch, + // since we cannot depend on the blocks represented by them being ever acquired. + match header_verifier.verify_header(proposal.top_block_header(), false) { + Ok(_) => &Pending(PendingTopBlock), + Err(e) => { + warn!(target: "aleph-finality", "Invalid header in proposal: {}", e); + &Pending(TopBlockImportedButIncorrectBranch) + } + } + } }; match old_status { Pending(PendingTopBlock) => { @@ -82,16 +96,17 @@ where } } -fn is_hopeless_fork(chain_info_provider: &mut CIP, proposal: &AlephProposal) -> bool +fn is_hopeless_fork(chain_info_provider: &mut CIP, proposal: &AlephProposal) -> bool where CIP: ChainInfoProvider, + UH: UnverifiedHeader, { let bottom_num = proposal.number_bottom_block(); - for i in 0..proposal.len() { + for (i, id) in proposal.blocks_from_num(bottom_num).enumerate() { if let Ok(finalized_block) = chain_info_provider.get_finalized_at(bottom_num + ::saturated_from(i)) { - if finalized_block.hash() != proposal[i] { + if finalized_block.hash() != id.hash() { return true; } } else { @@ -102,9 +117,13 @@ where false } -fn is_ancestor_finalized(chain_info_provider: &mut CIP, proposal: &AlephProposal) -> bool +fn is_ancestor_finalized( + chain_info_provider: &mut CIP, + proposal: &AlephProposal, +) -> bool where CIP: ChainInfoProvider, + UH: UnverifiedHeader, { let bottom = proposal.bottom_block(); let parent_hash = if let Ok(hash) = chain_info_provider.get_parent_hash(&bottom) { @@ -122,17 +141,22 @@ where } // Checks that the subsequent blocks in the branch are in the parent-child relation, as required. -fn is_branch_ancestry_correct(chain_info_provider: &mut CIP, proposal: &AlephProposal) -> bool +fn is_branch_ancestry_correct( + chain_info_provider: &mut CIP, + proposal: &AlephProposal, +) -> bool where CIP: ChainInfoProvider, + UH: UnverifiedHeader, { let bottom_num = proposal.number_bottom_block(); - for i in 1..proposal.len() { - let curr_num = bottom_num + ::saturated_from(i); - let curr_block = proposal.block_at_num(curr_num).expect("is within bounds"); - match chain_info_provider.get_parent_hash(&curr_block) { + for (parent, current) in proposal + .blocks_from_num(bottom_num) + .zip(proposal.blocks_from_num(bottom_num + 1)) + { + match chain_info_provider.get_parent_hash(¤t) { Ok(parent_hash) => { - if parent_hash != proposal[i - 1] { + if parent_hash != parent.hash() { return false; } } @@ -168,7 +192,7 @@ mod tests { client_chain_builder::ClientChainBuilder, mocks::{ unvalidated_proposal_from_headers, TBlock, THeader, TestClient, TestClientBuilder, - TestClientBuilderExt, + TestClientBuilderExt, TestVerifier, }, }, SessionBoundaryInfo, SessionId, SessionPeriod, @@ -177,14 +201,14 @@ mod tests { // A large number only for the purpose of creating `AlephProposal`s const DUMMY_SESSION_LEN: u32 = 1_000_000; - fn proposal_from_headers(headers: Vec) -> AlephProposal { + fn proposal_from_headers(headers: Vec) -> AlephProposal { let unvalidated = unvalidated_proposal_from_headers(headers); let session_boundaries = SessionBoundaryInfo::new(SessionPeriod(DUMMY_SESSION_LEN)) .boundaries_for_session(SessionId(0)); unvalidated.validate_bounds(&session_boundaries).unwrap() } - fn proposal_from_blocks(blocks: Vec) -> AlephProposal { + fn proposal_from_blocks(blocks: Vec) -> AlephProposal { let headers = blocks.into_iter().map(|b| b.header().clone()).collect(); proposal_from_headers(headers) } @@ -221,15 +245,15 @@ mod tests { fn verify_proposal_status( cached_cip: &mut TestCachedChainInfo, aux_cip: &mut TestAuxChainInfo, - proposal: &AlephProposal, + proposal: &AlephProposal, correct_status: ProposalStatus, ) { - let status_a = get_proposal_status(aux_cip, proposal, None); + let status_a = get_proposal_status(aux_cip, &mut TestVerifier, proposal, None); assert_eq!( status_a, correct_status, "Aux chain info gives wrong status for proposal {proposal:?}" ); - let status_c = get_proposal_status(cached_cip, proposal, None); + let status_c = get_proposal_status(cached_cip, &mut TestVerifier, proposal, None); assert_eq!( status_c, correct_status, "Cached chain info gives wrong status for proposal {proposal:?}" diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs index 37aae329c5..5602cbe408 100644 --- a/finality-aleph/src/lib.rs +++ b/finality-aleph/src/lib.rs @@ -32,6 +32,7 @@ use crate::{ SignatureSet, SpawnHandle, CURRENT_VERSION, LEGACY_VERSION, }, aggregation::{CurrentRmcNetworkData, LegacyRmcNetworkData}, + block::UnverifiedHeader, compatibility::{Version, Versioned}, network::data::split::Split, session::{SessionBoundaries, SessionBoundaryInfo, SessionId}, @@ -106,13 +107,13 @@ pub struct MillisecsPerBlock(pub u64); pub struct UnitCreationDelay(pub u64); type LegacySplitData = Split; -type CurrentSplitData = Split; +type CurrentSplitData = Split, CurrentRmcNetworkData>; impl Versioned for LegacyNetworkData { const VERSION: Version = Version(LEGACY_VERSION); } -impl Versioned for CurrentNetworkData { +impl Versioned for CurrentNetworkData { const VERSION: Version = Version(CURRENT_VERSION); } @@ -164,7 +165,7 @@ impl Encode for VersionedEitherMes } } -type VersionedNetworkData = VersionedEitherMessage; +type VersionedNetworkData = VersionedEitherMessage>; #[derive(Debug, Display, Clone)] pub enum VersionedTryFromError { @@ -172,20 +173,20 @@ pub enum VersionedTryFromError { ExpectedOldGotNew, } -impl TryFrom for LegacySplitData { +impl TryFrom> for LegacySplitData { type Error = VersionedTryFromError; - fn try_from(value: VersionedNetworkData) -> Result { + fn try_from(value: VersionedNetworkData) -> Result { Ok(match value { VersionedEitherMessage::Left(data) => data, VersionedEitherMessage::Right(_) => return Err(ExpectedOldGotNew), }) } } -impl TryFrom for CurrentSplitData { +impl TryFrom> for CurrentSplitData { type Error = VersionedTryFromError; - fn try_from(value: VersionedNetworkData) -> Result { + fn try_from(value: VersionedNetworkData) -> Result { Ok(match value { VersionedEitherMessage::Left(_) => return Err(ExpectedNewGotOld), VersionedEitherMessage::Right(data) => data, @@ -193,14 +194,14 @@ impl TryFrom for CurrentSplitData { } } -impl From for VersionedNetworkData { +impl From for VersionedNetworkData { fn from(data: LegacySplitData) -> Self { VersionedEitherMessage::Left(data) } } -impl From for VersionedNetworkData { - fn from(data: CurrentSplitData) -> Self { +impl From> for VersionedNetworkData { + fn from(data: CurrentSplitData) -> Self { VersionedEitherMessage::Right(data) } } diff --git a/finality-aleph/src/nodes.rs b/finality-aleph/src/nodes.rs index 6df948f72c..8e192de921 100644 --- a/finality-aleph/src/nodes.rs +++ b/finality-aleph/src/nodes.rs @@ -180,11 +180,15 @@ where justification_rx, block_rx, ); - let (sync_service, justifications_for_sync, request_block) = - match SyncService::new(verifier, session_info.clone(), sync_io, registry.clone()) { - Ok(x) => x, - Err(e) => panic!("Failed to initialize Sync service: {e}"), - }; + let (sync_service, justifications_for_sync, request_block) = match SyncService::new( + verifier.clone(), + session_info.clone(), + sync_io, + registry.clone(), + ) { + Ok(x) => x, + Err(e) => panic!("Failed to initialize Sync service: {e}"), + }; let sync_task = async move { sync_service.run().await }; let validator_address_cache_updater = validator_address_cache_updater( @@ -228,6 +232,7 @@ where session_manager: NodeSessionManagerImpl::new( client, select_chain, + verifier, session_period, unit_creation_delay, justifications_for_sync, diff --git a/finality-aleph/src/party/manager/mod.rs b/finality-aleph/src/party/manager/mod.rs index b8a0e779f5..c17bd8886e 100644 --- a/finality-aleph/src/party/manager/mod.rs +++ b/finality-aleph/src/party/manager/mod.rs @@ -16,7 +16,10 @@ use crate::{ run_legacy_member, SpawnHandle, }, aleph_primitives::{AlephSessionApi, BlockHash, BlockNumber, KEY_TYPE}, - block::substrate::{Justification, JustificationTranslator}, + block::{ + substrate::{Justification, JustificationTranslator}, + Header, HeaderVerifier, UnverifiedHeader, + }, crypto::{AuthorityPen, AuthorityVerifier}, data_io::{ legacy::{ @@ -71,10 +74,10 @@ type CurrentNetworkType = SimpleNetwork< struct SubtasksParams where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader, C: crate::ClientForAleph + Send + Sync + 'static, BE: Backend + 'static, - N: Network + 'static, + N: Network> + 'static, JS: JustificationSubmissions + Send + Sync + Clone, { n_members: usize, @@ -92,19 +95,21 @@ where phantom: PhantomData, } -pub struct NodeSessionManagerImpl +pub struct NodeSessionManagerImpl where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader + Header, C: crate::ClientForAleph + Send + Sync + 'static, BE: Backend + 'static, SC: SelectChain + 'static, RB: RequestBlocks, - SM: SessionManager + 'static, + SM: SessionManager> + 'static, JS: JustificationSubmissions + Send + Sync + Clone, + V: HeaderVerifier, { client: Arc, select_chain: SC, + verifier: V, session_info: SessionBoundaryInfo, unit_creation_delay: UnitCreationDelay, justifications_for_sync: JS, @@ -117,22 +122,24 @@ where _phantom: PhantomData<(B, BE)>, } -impl NodeSessionManagerImpl +impl NodeSessionManagerImpl where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader + Header, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: crate::aleph_primitives::AlephSessionApi, BE: Backend + 'static, SC: SelectChain + 'static, RB: RequestBlocks, - SM: SessionManager, + SM: SessionManager>, JS: JustificationSubmissions + Send + Sync + Clone + 'static, + V: HeaderVerifier, { #[allow(clippy::too_many_arguments)] pub fn new( client: Arc, select_chain: SC, + verifier: V, session_period: SessionPeriod, unit_creation_delay: UnitCreationDelay, justifications_for_sync: JS, @@ -146,6 +153,7 @@ where Self { client, select_chain, + verifier, session_info: SessionBoundaryInfo::new(session_period), unit_creation_delay, justifications_for_sync, @@ -159,7 +167,7 @@ where } } - fn legacy_subtasks + 'static>( + fn legacy_subtasks> + 'static>( &self, params: SubtasksParams, ) -> Subtasks { @@ -228,7 +236,7 @@ where ) } - fn current_subtasks + 'static>( + fn current_subtasks> + 'static>( &self, params: SubtasksParams, ) -> Subtasks { @@ -257,6 +265,7 @@ where let ordered_data_interpreter = OrderedDataInterpreter::new( blocks_for_aggregator, chain_info, + self.verifier.clone(), session_boundaries.clone(), ); let consensus_config = @@ -268,6 +277,7 @@ where let (data_store, aleph_network) = DataStore::new( session_boundaries.clone(), self.client.clone(), + self.verifier.clone(), self.block_requester.clone(), Default::default(), unfiltered_aleph_network, @@ -407,18 +417,19 @@ where } #[async_trait] -impl NodeSessionManager - for NodeSessionManagerImpl +impl NodeSessionManager + for NodeSessionManagerImpl where B: BlockT, - B::Header: HeaderT, + B::Header: HeaderT + UnverifiedHeader + Header, C: crate::ClientForAleph + Send + Sync + 'static, C::Api: crate::aleph_primitives::AlephSessionApi, BE: Backend + 'static, SC: SelectChain + 'static, RB: RequestBlocks, - SM: SessionManager, + SM: SessionManager>, JS: JustificationSubmissions + Send + Sync + Clone + 'static, + V: HeaderVerifier, { type Error = SM::Error; diff --git a/finality-aleph/src/runtime_api.rs b/finality-aleph/src/runtime_api.rs index 5a377cddc5..960a35ed30 100644 --- a/finality-aleph/src/runtime_api.rs +++ b/finality-aleph/src/runtime_api.rs @@ -17,7 +17,7 @@ use crate::{ }; /// Trait handling connection between host code and runtime storage -pub trait RuntimeApi { +pub trait RuntimeApi: Clone + Send + Sync + 'static { type Error: Display; /// Returns aura authorities for the next session using state from block `at` fn next_aura_authorities(&self, at: BlockHash) @@ -26,7 +26,6 @@ pub trait RuntimeApi { type QueuedKeys = Vec<(AccountId, SessionKeys)>; -#[derive(Clone)] pub struct RuntimeApiImpl where C: ClientForAleph + Send + Sync + 'static, @@ -38,6 +37,18 @@ where _phantom: PhantomData<(B, BE)>, } +impl Clone for RuntimeApiImpl +where + C: ClientForAleph + Send + Sync + 'static, + C::Api: AlephSessionApi, + B: Block, + BE: Backend + 'static, +{ + fn clone(&self) -> Self { + RuntimeApiImpl::new(self.client.clone()) + } +} + impl RuntimeApiImpl where C: ClientForAleph + Send + Sync + 'static, diff --git a/finality-aleph/src/session_map.rs b/finality-aleph/src/session_map.rs index bf9619d318..c205f77c7d 100644 --- a/finality-aleph/src/session_map.rs +++ b/finality-aleph/src/session_map.rs @@ -24,7 +24,7 @@ const LOG_TARGET: &str = "aleph-session-updater"; type SessionMap = HashMap; type SessionSubscribers = HashMap>>; -pub trait AuthorityProvider { +pub trait AuthorityProvider: Clone + Send + Sync + 'static { /// returns authority data for block fn authority_data(&self, block_number: BlockNumber) -> Option; /// returns next session authority data where current session is for block @@ -51,6 +51,20 @@ where _phantom: PhantomData<(B, BE)>, } +impl Clone for AuthorityProviderImpl +where + C: ClientForAleph + Send + Sync + 'static, + C::Api: crate::aleph_primitives::AlephSessionApi + AuraApi, + B: Block, + B::Header: Header, + BE: Backend + 'static, + RA: RuntimeApi, +{ + fn clone(&self) -> Self { + AuthorityProviderImpl::new(self.client.clone(), self.api.clone()) + } +} + impl AuthorityProviderImpl where C: ClientForAleph + Send + Sync + 'static, @@ -426,6 +440,7 @@ mod tests { } } + #[derive(Clone)] struct MockProvider { pub session_map: HashMap, pub next_session_map: HashMap, diff --git a/finality-aleph/src/sync/handler/mod.rs b/finality-aleph/src/sync/handler/mod.rs index 572baa7941..411775fc20 100644 --- a/finality-aleph/src/sync/handler/mod.rs +++ b/finality-aleph/src/sync/handler/mod.rs @@ -8,8 +8,9 @@ use std::{ use crate::{ block::{ - Block, BlockImport, BlockStatus, ChainStatus, Finalizer, Header, Justification, - UnverifiedHeader, UnverifiedHeaderFor, UnverifiedJustification, VerifiedHeader, Verifier, + Block, BlockImport, BlockStatus, ChainStatus, Finalizer, Header, HeaderVerifier, + Justification, JustificationVerifier, UnverifiedHeader, UnverifiedHeaderFor, + UnverifiedJustification, VerifiedHeader, }, session::{SessionBoundaryInfo, SessionId}, sync::{ @@ -201,7 +202,7 @@ where B: Block>, I: PeerId, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, BI: BlockImport, { @@ -233,11 +234,11 @@ where type HandleStateOutput = ( HandleStateAction, - Option<>::EquivocationProof>, + Option<::Header>>::EquivocationProof>, ); type HandleOwnBlockOutput = ( Vec>, - Option<>::EquivocationProof>, + Option<::Header>>::EquivocationProof>, ); impl HandleStateAction @@ -261,16 +262,17 @@ where } /// What can go wrong when handling a piece of data. -#[derive(Clone, Debug)] +#[derive(Debug)] pub enum Error where J: Justification, B: Block>, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, { - Verifier(V::Error), + JustificationVerifier(>::Error), + HeaderVerifier(>::Error), ChainStatus(CS::Error), Finalizer(F::Error), Forest(ForestError), @@ -287,13 +289,14 @@ where J: Justification, B: Block>, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { use Error::*; match self { - Verifier(e) => write!(f, "verifier error: {e}"), + JustificationVerifier(e) => write!(f, "justification verifier error: {e}"), + HeaderVerifier(e) => write!(f, "header verifier error: {e}"), ChainStatus(e) => write!(f, "chain status error: {e}"), Finalizer(e) => write!(f, "finalized error: {e}"), Forest(e) => write!(f, "forest error: {e}"), @@ -319,7 +322,7 @@ where J: Justification, B: Block>, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, { fn from(e: ForestError) -> Self { @@ -332,7 +335,7 @@ where J: Justification, B: Block>, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, { fn from(e: TrySyncError) -> Self { @@ -349,7 +352,7 @@ where J: Justification, B: Block>, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, { fn from(e: RequestHandlerError) -> Self { @@ -363,7 +366,7 @@ where B: Block>, I: PeerId, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, BI: BlockImport, { @@ -376,7 +379,7 @@ where B: Block>, I: PeerId, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, BI: BlockImport, { @@ -465,14 +468,17 @@ where &mut self, block: B, own_block: bool, - ) -> Result>::EquivocationProof>, ::Error> { + ) -> Result< + Option<>::EquivocationProof>, + ::Error, + > { let VerifiedHeader { maybe_equivocation_proof, .. } = self .verifier .verify_header(block.header().clone(), own_block) - .map_err(Error::Verifier)?; + .map_err(Error::HeaderVerifier)?; self.block_importer.import_block(block); Ok(maybe_equivocation_proof) } @@ -558,7 +564,7 @@ where let justification = self .verifier .verify_justification(justification) - .map_err(Error::Verifier)?; + .map_err(Error::JustificationVerifier)?; let new_highest = self .forest .update_justification(justification, maybe_peer)?; @@ -639,7 +645,7 @@ where let h = match self .verifier .verify_header(h, false) - .map_err(Error::Verifier) + .map_err(Error::HeaderVerifier) { Ok(VerifiedHeader { header: h, @@ -734,7 +740,7 @@ where } = self .verifier .verify_header(state.favourite_block(), false) - .map_err(Error::Verifier)?; + .map_err(Error::HeaderVerifier)?; let action = match local_session.0.checked_sub(remote_session.0) { // remote session number larger than ours, we can try to import the justification None => HandleStateAction::maybe_extend( @@ -1202,7 +1208,7 @@ mod tests { } let (_, _, maybe_error) = handler.handle_request_response(response, 7); match maybe_error { - Some(Error::Verifier(_)) => (), + Some(Error::HeaderVerifier(_)) => (), e => panic!("should return Verifier error, {e:?}"), }; } @@ -1893,14 +1899,14 @@ mod tests { header, ); match handler.handle_state(state, peer) { - Err(Error::Verifier(_)) => (), + Err(Error::HeaderVerifier(_)) => (), e => panic!("should return Verifier error, {e:?}"), }; let mut header = MockHeader::random_parentless(1000).random_child(); header.invalidate(); let state = State::new(MockJustification::for_header(header.clone()), header); match handler.handle_state(state, peer) { - Err(Error::Verifier(_)) => (), + Err(Error::HeaderVerifier(_)) => (), e => panic!("should return Verifier error, {e:?}"), }; } diff --git a/finality-aleph/src/sync/service.rs b/finality-aleph/src/sync/service.rs index db63361469..26fcb42e57 100644 --- a/finality-aleph/src/sync/service.rs +++ b/finality-aleph/src/sync/service.rs @@ -7,8 +7,8 @@ use substrate_prometheus_endpoint::Registry; use crate::{ block::{ Block, BlockImport, ChainStatus, ChainStatusNotification, ChainStatusNotifier, - EquivocationProof, Finalizer, Justification, UnverifiedHeader, UnverifiedHeaderFor, - Verifier, + EquivocationProof, Finalizer, HeaderVerifier, Justification, JustificationVerifier, + UnverifiedHeader, UnverifiedHeaderFor, }, network::GossipNetwork, session::SessionBoundaryInfo, @@ -88,7 +88,7 @@ where N: GossipNetwork>, CE: ChainStatusNotifier, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, BI: BlockImport, { @@ -128,7 +128,7 @@ where N: GossipNetwork>, CE: ChainStatusNotifier, CS: ChainStatus, - V: Verifier, + V: JustificationVerifier + HeaderVerifier, F: Finalizer, BI: BlockImport, { @@ -340,9 +340,13 @@ where Err(e) => { self.metrics.report_event_error(Event::HandleState); match e { - HandlerError::Verifier(e) => debug!( + HandlerError::JustificationVerifier(e) => debug!( target: LOG_TARGET, - "Could not verify data in sync state from {:?}: {}.", peer, e + "Could not verify justification data in sync state from {:?}: {}.", peer, e + ), + HandlerError::HeaderVerifier(e) => debug!( + target: LOG_TARGET, + "Could not verify header data in sync state from {:?}: {}.", peer, e ), e => warn!( target: LOG_TARGET, @@ -371,10 +375,14 @@ where self.handler .handle_state_response(justification, maybe_justification, peer.clone()); match maybe_error { - Some(HandlerError::Verifier(e)) => debug!( + Some(HandlerError::JustificationVerifier(e)) => debug!( target: LOG_TARGET, "Could not verify justification in sync state from {:?}: {}.", peer, e ), + Some(HandlerError::HeaderVerifier(e)) => debug!( + target: LOG_TARGET, + "Could not verify header in sync state from {:?}: {}.", peer, e + ), Some(e) => warn!( target: LOG_TARGET, "Failed to handle sync state response from {:?}: {}.", peer, e @@ -401,10 +409,14 @@ where self.metrics .report_event_error(Event::HandleJustificationFromUser); match e { - HandlerError::Verifier(e) => debug!( + HandlerError::JustificationVerifier(e) => debug!( target: LOG_TARGET, "Could not verify justification from user: {}", e ), + HandlerError::HeaderVerifier(e) => debug!( + target: LOG_TARGET, + "Could not verify header from user: {}", e + ), e => warn!( target: LOG_TARGET, "Failed to handle justification from user: {}", e @@ -426,8 +438,17 @@ where .handler .handle_request_response(response_items, peer.clone()); match maybe_error { - Some(HandlerError::Verifier(e)) => { - debug!(target: LOG_TARGET, "Could not verify data from user: {}", e) + Some(HandlerError::JustificationVerifier(e)) => { + debug!( + target: LOG_TARGET, + "Could not verify justification from user: {}", e + ) + } + Some(HandlerError::HeaderVerifier(e)) => { + debug!( + target: LOG_TARGET, + "Could not verify header from user: {}", e + ) } Some(e) => warn!( target: LOG_TARGET, @@ -476,10 +497,14 @@ where Err(e) => { self.metrics.report_event_error(Event::HandleRequest); match e { - HandlerError::Verifier(e) => debug!( + HandlerError::JustificationVerifier(e) => debug!( target: LOG_TARGET, "Could not verify justification from user: {}", e ), + HandlerError::HeaderVerifier(e) => debug!( + target: LOG_TARGET, + "Could not verify header from user: {}", e + ), e => warn!( target: LOG_TARGET, "Error handling request from {:?}: {}.", peer, e @@ -543,10 +568,14 @@ where Err(e) => { self.metrics.report_event(Event::HandleInternalRequest); match e { - HandlerError::Verifier(e) => debug!( + HandlerError::JustificationVerifier(e) => debug!( target: LOG_TARGET, "Could not verify justification from user: {}", e ), + HandlerError::HeaderVerifier(e) => debug!( + target: LOG_TARGET, + "Could not verify header from user: {}", e + ), e => warn!( target: LOG_TARGET, "Error handling internal request for block {:?}: {}.", id, e @@ -575,10 +604,14 @@ where self.metrics .report_event_error(Event::HandleExtensionRequest); match e { - HandlerError::Verifier(e) => debug!( + HandlerError::JustificationVerifier(e) => debug!( target: LOG_TARGET, "Could not verify justification from {:?}: {}", peer, e ), + HandlerError::HeaderVerifier(e) => debug!( + target: LOG_TARGET, + "Could not verify header from {:?}: {}", peer, e + ), e => warn!( target: LOG_TARGET, "Error handling chain extension request from {:?}: {}.", peer, e diff --git a/finality-aleph/src/testing/data_store.rs b/finality-aleph/src/testing/data_store.rs index e170d13d4b..8da98fe008 100644 --- a/finality-aleph/src/testing/data_store.rs +++ b/finality-aleph/src/testing/data_store.rs @@ -24,7 +24,7 @@ use crate::{ client_chain_builder::ClientChainBuilder, mocks::{ aleph_data_from_blocks, aleph_data_from_headers, TBlock, THeader, TestClientBuilder, - TestClientBuilderExt, + TestClientBuilderExt, TestVerifier, }, }, BlockId, Recipient, @@ -49,10 +49,10 @@ impl RequestBlocks for TestBlockRequester { } } -type TestData = Vec; +type TestData = Vec>; -impl AlephNetworkMessage for TestData { - fn included_data(&self) -> Vec { +impl AlephNetworkMessage for TestData { + fn included_data(&self) -> Vec> { self.clone() } } @@ -170,6 +170,7 @@ fn prepare_data_store( let (mut data_store, network) = DataStore::new( session_boundaries, client.clone(), + TestVerifier, block_requester, data_store_config, test_network, diff --git a/finality-aleph/src/testing/mocks/mod.rs b/finality-aleph/src/testing/mocks/mod.rs index e8171bd755..42eec7ef03 100644 --- a/finality-aleph/src/testing/mocks/mod.rs +++ b/finality-aleph/src/testing/mocks/mod.rs @@ -1,3 +1,5 @@ +use std::fmt::{Display, Error as FmtError, Formatter}; + pub use acceptance_policy::AcceptancePolicy; pub use block_finalizer::MockedBlockFinalizer; pub use client::{TestClient, TestClientBuilder, TestClientBuilderExt}; @@ -7,13 +9,58 @@ pub use proposal::{ use sp_runtime::traits::BlakeTwo256; use substrate_test_runtime::Extrinsic; -use crate::aleph_primitives::BlockNumber; +use crate::{ + aleph_primitives::BlockNumber, + block::{EquivocationProof, HeaderVerifier, VerifiedHeader}, +}; type Hashing = BlakeTwo256; pub type TBlock = sp_runtime::generic::Block; pub type THeader = sp_runtime::generic::Header; pub type THash = substrate_test_runtime::Hash; +#[derive(Clone)] +pub struct TestVerifier; + +pub struct TestEquivocationProof; + +impl Display for TestEquivocationProof { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { + write!(f, "this should never get created") + } +} + +impl EquivocationProof for TestEquivocationProof { + fn are_we_equivocating(&self) -> bool { + false + } +} + +#[derive(Debug)] +pub struct TestVerificationError; + +impl Display for TestVerificationError { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { + write!(f, "this should never get created") + } +} + +impl HeaderVerifier for TestVerifier { + type EquivocationProof = TestEquivocationProof; + type Error = TestVerificationError; + + fn verify_header( + &mut self, + header: THeader, + _just_created: bool, + ) -> Result, Self::Error> { + Ok(VerifiedHeader { + header, + maybe_equivocation_proof: None, + }) + } +} + mod acceptance_policy; mod block_finalizer; mod client; diff --git a/finality-aleph/src/testing/mocks/proposal.rs b/finality-aleph/src/testing/mocks/proposal.rs index a68a80919e..4ebc1ea8b2 100644 --- a/finality-aleph/src/testing/mocks/proposal.rs +++ b/finality-aleph/src/testing/mocks/proposal.rs @@ -5,18 +5,20 @@ use crate::{ testing::mocks::{TBlock, THeader}, }; -pub fn unvalidated_proposal_from_headers(headers: Vec) -> UnvalidatedAlephProposal { - let num = headers.last().unwrap().number; - let hashes = headers.into_iter().map(|header| header.hash()).collect(); - UnvalidatedAlephProposal::new(hashes, num) +pub fn unvalidated_proposal_from_headers( + mut headers: Vec, +) -> UnvalidatedAlephProposal { + let head = headers.pop().unwrap(); + let tail = headers.into_iter().map(|header| header.hash()).collect(); + UnvalidatedAlephProposal::new(head, tail) } -pub fn aleph_data_from_blocks(blocks: Vec) -> AlephData { +pub fn aleph_data_from_blocks(blocks: Vec) -> AlephData { let headers = blocks.into_iter().map(|b| b.header().clone()).collect(); aleph_data_from_headers(headers) } -pub fn aleph_data_from_headers(headers: Vec) -> AlephData { +pub fn aleph_data_from_headers(headers: Vec) -> AlephData { AlephData { head_proposal: unvalidated_proposal_from_headers(headers), }