From f637e2df9ecce892c6da5b2d8072b7e5cc8099d4 Mon Sep 17 00:00:00 2001 From: lesniak43 Date: Tue, 3 Oct 2023 13:10:20 +0200 Subject: [PATCH] A0-3134: Remove BlockIdentifier (#1424) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Trait `BlockIdentifier` is removed, struct `BlockId` is used. ## Type of change - Refactor Co-authored-by: Damian Leśniak --- finality-aleph/src/data_io/data_store.rs | 4 +- finality-aleph/src/finalization.rs | 8 +- finality-aleph/src/lib.rs | 19 ++--- finality-aleph/src/network/mod.rs | 6 +- finality-aleph/src/network/substrate.rs | 2 +- .../src/party/manager/data_store.rs | 3 +- finality-aleph/src/party/manager/mod.rs | 8 +- finality-aleph/src/sync/compatibility.rs | 35 +++----- finality-aleph/src/sync/data.rs | 44 ++++------ finality-aleph/src/sync/forest/mod.rs | 82 +++++++++---------- finality-aleph/src/sync/forest/vertex.rs | 55 +++++++------ finality-aleph/src/sync/handler/mod.rs | 48 +++++------ .../src/sync/handler/request_handler.rs | 58 ++++++------- finality-aleph/src/sync/mock/backend.rs | 26 +++--- finality-aleph/src/sync/mock/mod.rs | 57 ++++--------- finality-aleph/src/sync/mod.rs | 22 ++--- finality-aleph/src/sync/service.rs | 26 +++--- .../src/sync/substrate/chain_status.rs | 17 ++-- finality-aleph/src/sync/substrate/mod.rs | 6 +- finality-aleph/src/sync/tasks.rs | 26 +++--- finality-aleph/src/testing/data_store.rs | 2 +- .../src/testing/mocks/block_finalizer.rs | 11 +-- finality-aleph/src/testing/mocks/mod.rs | 3 +- 23 files changed, 247 insertions(+), 321 deletions(-) diff --git a/finality-aleph/src/data_io/data_store.rs b/finality-aleph/src/data_io/data_store.rs index 6e11449850..98c5cfe9aa 100644 --- a/finality-aleph/src/data_io/data_store.rs +++ b/finality-aleph/src/data_io/data_store.rs @@ -149,7 +149,7 @@ where B: BlockT, B::Header: HeaderT, C: HeaderBackend + BlockchainEvents + Send + Sync + 'static, - RB: RequestBlocks + 'static, + RB: RequestBlocks + 'static, Message: AlephNetworkMessage + std::fmt::Debug + Send @@ -182,7 +182,7 @@ where B: BlockT, B::Header: HeaderT, C: HeaderBackend + BlockchainEvents + Send + Sync + 'static, - RB: RequestBlocks + 'static, + RB: RequestBlocks + 'static, Message: AlephNetworkMessage + std::fmt::Debug + Send diff --git a/finality-aleph/src/finalization.rs b/finality-aleph/src/finalization.rs index 36565ccdb4..db5d7800fd 100644 --- a/finality-aleph/src/finalization.rs +++ b/finality-aleph/src/finalization.rs @@ -12,11 +12,11 @@ use sp_runtime::{ use crate::{ aleph_primitives::{BlockHash, BlockNumber}, metrics::Checkpoint, - BlockId, BlockIdentifier, TimingBlockMetrics, + BlockId, TimingBlockMetrics, }; -pub trait BlockFinalizer { - fn finalize_block(&self, block: BI, justification: Justification) -> Result<(), Error>; +pub trait BlockFinalizer { + fn finalize_block(&self, block: BlockId, justification: Justification) -> Result<(), Error>; } pub struct AlephFinalizer @@ -45,7 +45,7 @@ where } } -impl BlockFinalizer for AlephFinalizer +impl BlockFinalizer for AlephFinalizer where B: Block, B::Header: Header, diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs index 8e0f1e4c4a..48be643b43 100644 --- a/finality-aleph/src/lib.rs +++ b/finality-aleph/src/lib.rs @@ -10,7 +10,7 @@ use futures::{ channel::{mpsc, oneshot}, Future, }; -use parity_scale_codec::{Codec, Decode, Encode, Output}; +use parity_scale_codec::{Decode, Encode, Output}; use primitives as aleph_primitives; use primitives::{AuthorityId, Block as AlephBlock, BlockHash, BlockNumber, Hash as AlephHash}; use sc_client_api::{ @@ -229,14 +229,9 @@ where { } -/// The identifier of a block, the least amount of knowledge we can have about a block. -pub trait BlockIdentifier: Clone + Hash + Debug + Eq + Codec + Send + Sync + 'static { - /// The block number, useful when reasoning about hopeless forks. - fn number(&self) -> BlockNumber; -} - type Hasher = abft::HashWrapper; +/// The identifier of a block, the least amount of knowledge we can have about a block. #[derive(PartialEq, Eq, Clone, Debug, Encode, Decode, Hash)] pub struct BlockId { hash: BlockHash, @@ -247,6 +242,10 @@ impl BlockId { pub fn new(hash: BlockHash, number: BlockNumber) -> Self { BlockId { hash, number } } + + pub fn number(&self) -> BlockNumber { + self.number + } } impl From<(BlockHash, BlockNumber)> for BlockId { @@ -261,12 +260,6 @@ impl Display for BlockId { } } -impl BlockIdentifier for BlockId { - fn number(&self) -> BlockNumber { - self.number - } -} - #[derive(Clone)] pub struct RateLimiterConfig { /// Maximum bit-rate per node in bytes per second of the alephbft validator network. diff --git a/finality-aleph/src/network/mod.rs b/finality-aleph/src/network/mod.rs index dc2b914477..a9513c111c 100644 --- a/finality-aleph/src/network/mod.rs +++ b/finality-aleph/src/network/mod.rs @@ -16,12 +16,12 @@ pub use gossip::{ use network_clique::{AddressingInformation, NetworkIdentity, PeerId}; pub use substrate::{ProtocolNaming, SubstrateNetwork}; -use crate::BlockIdentifier; +use crate::BlockId; /// Abstraction for requesting stale blocks. -pub trait RequestBlocks: Clone + Send + Sync + 'static { +pub trait RequestBlocks: Clone + Send + Sync + 'static { /// Request the given block -- this is supposed to be used only for "old forks". - fn request_stale_block(&self, block: BI); + fn request_stale_block(&self, block: BlockId); } /// A basic alias for properties we expect basic data to satisfy. diff --git a/finality-aleph/src/network/substrate.rs b/finality-aleph/src/network/substrate.rs index 68e9b808fb..3464ab35a0 100644 --- a/finality-aleph/src/network/substrate.rs +++ b/finality-aleph/src/network/substrate.rs @@ -25,7 +25,7 @@ use crate::{ BlockId, }; -impl RequestBlocks for Arc> +impl RequestBlocks for Arc> where B: Block, B::Header: Header, diff --git a/finality-aleph/src/party/manager/data_store.rs b/finality-aleph/src/party/manager/data_store.rs index 16f4f81ca7..63c379d03a 100644 --- a/finality-aleph/src/party/manager/data_store.rs +++ b/finality-aleph/src/party/manager/data_store.rs @@ -13,7 +13,6 @@ use crate::{ network::data::component::Receiver, party::{AuthoritySubtaskCommon, Task}, sync::RequestBlocks, - BlockId, }; /// Runs the data store within a single session. @@ -25,7 +24,7 @@ where B: Block, B::Header: Header, C: HeaderBackend + BlockchainEvents + Send + Sync + 'static, - RB: RequestBlocks + 'static, + RB: RequestBlocks + 'static, Message: AlephNetworkMessage + Debug + Send + Sync + Codec + 'static, R: Receiver + 'static, { diff --git a/finality-aleph/src/party/manager/mod.rs b/finality-aleph/src/party/manager/mod.rs index 198f8f77cc..0ae8fcbc01 100644 --- a/finality-aleph/src/party/manager/mod.rs +++ b/finality-aleph/src/party/manager/mod.rs @@ -30,7 +30,7 @@ use crate::{ backup::ABFTBackup, manager::aggregator::AggregatorVersion, traits::NodeSessionManager, }, sync::{substrate::Justification, JustificationSubmissions, JustificationTranslator}, - AuthorityId, BlockId, CurrentRmcNetworkData, Keychain, LegacyRmcNetworkData, NodeIndex, + AuthorityId, CurrentRmcNetworkData, Keychain, LegacyRmcNetworkData, NodeIndex, SessionBoundaries, SessionBoundaryInfo, SessionId, SessionPeriod, TimingBlockMetrics, UnitCreationDelay, VersionedNetworkData, }; @@ -97,7 +97,7 @@ where C: crate::ClientForAleph + Send + Sync + 'static, BE: Backend + 'static, SC: SelectChain + 'static, - RB: RequestBlocks, + RB: RequestBlocks, SM: SessionManager + 'static, JS: JustificationSubmissions + Send + Sync + Clone, { @@ -123,7 +123,7 @@ where C::Api: crate::aleph_primitives::AlephSessionApi, BE: Backend + 'static, SC: SelectChain + 'static, - RB: RequestBlocks, + RB: RequestBlocks, SM: SessionManager, JS: JustificationSubmissions + Send + Sync + Clone + 'static, { @@ -405,7 +405,7 @@ where C::Api: crate::aleph_primitives::AlephSessionApi, BE: Backend + 'static, SC: SelectChain + 'static, - RB: RequestBlocks, + RB: RequestBlocks, SM: SessionManager, JS: JustificationSubmissions + Send + Sync + Clone + 'static, { diff --git a/finality-aleph/src/sync/compatibility.rs b/finality-aleph/src/sync/compatibility.rs index 4891e4c8aa..99c462f194 100644 --- a/finality-aleph/src/sync/compatibility.rs +++ b/finality-aleph/src/sync/compatibility.rs @@ -1,48 +1,37 @@ -use std::marker::PhantomData; - use crate::{ - network::RequestBlocks as OldRequestBlocks, sync::RequestBlocks as NewRequestBlocks, - BlockIdentifier, + network::RequestBlocks as OldRequestBlocks, sync::RequestBlocks as NewRequestBlocks, BlockId, }; /// BlockRequester that uses both new and old RequestBlocks so that /// every request goes into old and new engine. #[derive(Clone)] -pub struct OldSyncCompatibleRequestBlocks +pub struct OldSyncCompatibleRequestBlocks where - OBR: OldRequestBlocks, - NBR: NewRequestBlocks, - BI: BlockIdentifier, + OBR: OldRequestBlocks, + NBR: NewRequestBlocks, { new: NBR, old: OBR, - _phantom: PhantomData, } -impl OldSyncCompatibleRequestBlocks +impl OldSyncCompatibleRequestBlocks where - OBR: OldRequestBlocks, - NBR: NewRequestBlocks, - BI: BlockIdentifier, + OBR: OldRequestBlocks, + NBR: NewRequestBlocks, { pub fn new(old: OBR, new: NBR) -> Self { - Self { - new, - old, - _phantom: PhantomData, - } + Self { new, old } } } -impl NewRequestBlocks for OldSyncCompatibleRequestBlocks +impl NewRequestBlocks for OldSyncCompatibleRequestBlocks where - OBR: OldRequestBlocks, - NBR: NewRequestBlocks, - BI: BlockIdentifier, + OBR: OldRequestBlocks, + NBR: NewRequestBlocks, { type Error = NBR::Error; - fn request_block(&self, block_id: BI) -> Result<(), Self::Error> { + fn request_block(&self, block_id: BlockId) -> Result<(), Self::Error> { self.old.request_stale_block(block_id.clone()); self.new.request_block(block_id) diff --git a/finality-aleph/src/sync/data.rs b/finality-aleph/src/sync/data.rs index b681abe281..fd4d31cbbf 100644 --- a/finality-aleph/src/sync/data.rs +++ b/finality-aleph/src/sync/data.rs @@ -7,8 +7,8 @@ use static_assertions::const_assert; use crate::{ aleph_primitives::MAX_BLOCK_SIZE, network::GossipNetwork, - sync::{Block, BlockIdFor, Header, Justification, PeerId, UnverifiedJustification, LOG_TARGET}, - Version, + sync::{Block, Header, Justification, PeerId, UnverifiedJustification, LOG_TARGET}, + BlockId, Version, }; /// The representation of the database state to be sent to other nodes. @@ -81,21 +81,21 @@ pub type ResponseItems = Vec>; /// with a given one. All the variants are exhaustive and exclusive due to the /// properties of the `Forest` structure. #[derive(Clone, Debug, Encode, Decode, PartialEq, Eq)] -pub enum BranchKnowledge { +pub enum BranchKnowledge { /// ID of the oldest known ancestor if none of them are imported. /// It must be different from the, imported by definition, root. - LowestId(BlockIdFor), + LowestId(BlockId), /// ID of the top imported ancestor if any of them is imported. /// Since imported vertices are connected to the root, the oldest known /// ancestor is, implicitly, the root. - TopImported(BlockIdFor), + TopImported(BlockId), } /// Request content, first version. #[derive(Clone, Debug, Encode, Decode)] pub struct RequestV1 { - target_id: BlockIdFor, - branch_knowledge: BranchKnowledge, + target_id: BlockId, + branch_knowledge: BranchKnowledge, state: StateV1, } @@ -116,8 +116,8 @@ impl RequestV1 { /// Request content, current version. #[derive(Clone, Debug, Encode, Decode)] pub struct Request { - target_id: BlockIdFor, - branch_knowledge: BranchKnowledge, + target_id: BlockId, + branch_knowledge: BranchKnowledge, state: State, } @@ -152,11 +152,7 @@ impl From> for RequestV1 { } impl Request { - pub fn new( - target_id: BlockIdFor, - branch_knowledge: BranchKnowledge, - state: State, - ) -> Self { + pub fn new(target_id: BlockId, branch_knowledge: BranchKnowledge, state: State) -> Self { Self { target_id, branch_knowledge, @@ -169,27 +165,23 @@ impl Request { pub fn state(&self) -> &State { &self.state } - pub fn target_id(&self) -> &BlockIdFor { + pub fn target_id(&self) -> &BlockId { &self.target_id } - pub fn branch_knowledge(&self) -> &BranchKnowledge { + pub fn branch_knowledge(&self) -> &BranchKnowledge { &self.branch_knowledge } } /// Data that can be used to generate a request given our state. -pub struct PreRequest { - id: BlockIdFor, - branch_knowledge: BranchKnowledge, +pub struct PreRequest { + id: BlockId, + branch_knowledge: BranchKnowledge, know_most: HashSet, } -impl PreRequest { - pub fn new( - id: BlockIdFor, - branch_knowledge: BranchKnowledge, - know_most: HashSet, - ) -> Self { +impl PreRequest { + pub fn new(id: BlockId, branch_knowledge: BranchKnowledge, know_most: HashSet) -> Self { PreRequest { id, branch_knowledge, @@ -198,7 +190,7 @@ impl PreRequest { } /// Convert to a request and recipients given a state. - pub fn with_state(self, state: State) -> (Request, HashSet) { + pub fn with_state(self, state: State) -> (Request, HashSet) { let PreRequest { id, branch_knowledge, diff --git a/finality-aleph/src/sync/forest/mod.rs b/finality-aleph/src/sync/forest/mod.rs index 0bb635aa6f..f7a9bdc478 100644 --- a/finality-aleph/src/sync/forest/mod.rs +++ b/finality-aleph/src/sync/forest/mod.rs @@ -10,8 +10,8 @@ use static_assertions::const_assert; use crate::{ aleph_primitives::DEFAULT_SESSION_PERIOD, - sync::{data::BranchKnowledge, Block, BlockIdFor, ChainStatus, Header, Justification, PeerId}, - BlockIdentifier, BlockNumber, + sync::{data::BranchKnowledge, Block, BlockId, ChainStatus, Header, Justification, PeerId}, + BlockNumber, }; mod vertex; @@ -28,8 +28,8 @@ pub enum SpecialState { enum VertexHandleMut<'a, I: PeerId, J: Justification> { Special(SpecialState), - Unknown(VacantEntry<'a, BlockIdFor, VertexWithChildren>), - Candidate(OccupiedEntry<'a, BlockIdFor, VertexWithChildren>), + Unknown(VacantEntry<'a, BlockId, VertexWithChildren>), + Candidate(OccupiedEntry<'a, BlockId, VertexWithChildren>), } enum VertexHandle<'a, I: PeerId, J: Justification> { @@ -41,28 +41,28 @@ enum VertexHandle<'a, I: PeerId, J: Justification> { /// Our interest in a branch referred to by a vertex, /// including all the information required to prepare a request. #[derive(Clone, PartialEq, Eq, Debug)] -pub enum Interest { +pub enum Interest { /// We are not interested in requesting this branch. Uninterested, /// We would like to have this branch. Required { know_most: HashSet, - branch_knowledge: BranchKnowledge, + branch_knowledge: BranchKnowledge, }, } /// What kind of extension we should request and from whom. #[derive(Clone, PartialEq, Eq, Debug)] -pub enum ExtensionRequest { +pub enum ExtensionRequest { /// We are not interested in requesting anything at this point. Noop, /// We would like to have children of our favourite block. FavouriteBlock { know_most: HashSet }, /// We would like to have the justified block. HighestJustified { - id: BlockIdFor, + id: BlockId, know_most: HashSet, - branch_knowledge: BranchKnowledge, + branch_knowledge: BranchKnowledge, }, } @@ -125,7 +125,7 @@ where pub struct VertexWithChildren { vertex: Vertex, - children: HashSet>, + children: HashSet, } impl VertexWithChildren { @@ -136,7 +136,7 @@ impl VertexWithChildren { } } - fn add_child(&mut self, child: BlockIdFor) { + fn add_child(&mut self, child: BlockId) { self.children.insert(child); } } @@ -153,17 +153,17 @@ where I: PeerId, J: Justification, { - vertices: HashMap, VertexWithChildren>, - highest_justified: BlockIdFor, - justified_blocks: HashMap>, - imported_leaves: HashSet>, - favourite: BlockIdFor, - root_id: BlockIdFor, - root_children: HashSet>, - compost_bin: HashSet>, + vertices: HashMap>, + highest_justified: BlockId, + justified_blocks: HashMap, + imported_leaves: HashSet, + favourite: BlockId, + root_id: BlockId, + root_children: HashSet, + compost_bin: HashSet, } -type Edge = (BlockIdFor, BlockIdFor); +type Edge = (BlockId, BlockId); impl Forest where @@ -208,7 +208,7 @@ where Ok(forest) } - fn special_state(&self, id: &BlockIdFor) -> Option { + fn special_state(&self, id: &BlockId) -> Option { use SpecialState::*; if id == &self.root_id { Some(HighestFinalized) @@ -223,7 +223,7 @@ where } } - fn get_mut(&mut self, id: &BlockIdFor) -> VertexHandleMut { + fn get_mut(&mut self, id: &BlockId) -> VertexHandleMut { use VertexHandleMut::*; if let Some(state) = self.special_state(id) { Special(state) @@ -235,7 +235,7 @@ where } } - fn get(&self, id: &BlockIdFor) -> VertexHandle { + fn get(&self, id: &BlockId) -> VertexHandle { use VertexHandle::*; if let Some(state) = self.special_state(id) { Special(state) @@ -247,7 +247,7 @@ where } } - fn connect_parent(&mut self, id: &BlockIdFor) { + fn connect_parent(&mut self, id: &BlockId) { use SpecialState::*; use VertexHandleMut::*; if let Candidate(mut entry) = self.get_mut(id) { @@ -280,7 +280,7 @@ where }; } - fn set_required(&mut self, id: &BlockIdFor) { + fn set_required(&mut self, id: &BlockId) { if let VertexHandleMut::Candidate(mut entry) = self.get_mut(id) { let vertex = entry.get_mut(); if vertex.vertex.set_required() { @@ -291,7 +291,7 @@ where } } - fn set_explicitly_required(&mut self, id: &BlockIdFor) -> bool { + fn set_explicitly_required(&mut self, id: &BlockId) -> bool { match self.get_mut(id) { VertexHandleMut::Candidate(mut entry) => { match entry.get_mut().vertex.set_explicitly_required() { @@ -308,7 +308,7 @@ where } } - fn insert_id(&mut self, id: BlockIdFor, holder: Option) -> Result<(), Error> { + fn insert_id(&mut self, id: BlockId, holder: Option) -> Result<(), Error> { match self.special_state(&id) { Some(SpecialState::TooNew) => Err(Error::TooNew), Some(_) => Ok(()), @@ -323,7 +323,7 @@ where } } - fn process_header(&mut self, header: &J::Header) -> Result, Error> { + fn process_header(&mut self, header: &J::Header) -> Result { Ok(( header.id(), header.parent_id().ok_or(Error::HeaderMissingParentId)?, @@ -333,7 +333,7 @@ where /// Updates the provider block identifier, returns whether it became a new explicitly required. pub fn update_block_identifier( &mut self, - id: &BlockIdFor, + id: &BlockId, holder: Option, required: bool, ) -> Result { @@ -407,7 +407,7 @@ where } /// Updates the `highest_justified` if the given id is higher. - fn try_update_highest_justified(&mut self, id: BlockIdFor) -> bool { + fn try_update_highest_justified(&mut self, id: BlockId) -> bool { match id.number() > self.highest_justified.number() { true => { self.highest_justified = id; @@ -462,7 +462,7 @@ where .clone(); } - fn prune(&mut self, id: &BlockIdFor) { + fn prune(&mut self, id: &BlockId) { if let Some(VertexWithChildren { children, .. }) = self.vertices.remove(id) { self.imported_leaves.remove(id); self.compost_bin.insert(id.clone()); @@ -508,7 +508,7 @@ where /// Returns the BranchKnowledge regarding the given block id, /// or None if there is no branch at all. - fn branch_knowledge(&self, mut id: BlockIdFor) -> Option> { + fn branch_knowledge(&self, mut id: BlockId) -> Option { use SpecialState::*; use VertexHandle::*; // traverse ancestors till we reach something imported or a parentless vertex @@ -547,9 +547,9 @@ where /// Can be forced to fake interest, but only for blocks we know about. fn prepare_request_info( &self, - id: &BlockIdFor, + id: &BlockId, force: bool, - ) -> Option<(HashSet, BranchKnowledge)> { + ) -> Option<(HashSet, BranchKnowledge)> { use VertexHandle::Candidate; match self.get(id) { Candidate(vertex) => { @@ -568,7 +568,7 @@ where } /// How much interest we have for requesting the block. - pub fn request_interest(&self, id: &BlockIdFor) -> Interest { + pub fn request_interest(&self, id: &BlockId) -> Interest { match self.prepare_request_info(id, false) { Some((know_most, branch_knowledge)) => Interest::Required { know_most, @@ -579,7 +579,7 @@ where } /// Whether we would like to eventually import this block. - pub fn importable(&self, id: &BlockIdFor) -> bool { + pub fn importable(&self, id: &BlockId) -> bool { use VertexHandle::Candidate; match self.get(id) { Candidate(vertex) => { @@ -589,7 +589,7 @@ where } } - fn know_most(&self, id: &BlockIdFor) -> HashSet { + fn know_most(&self, id: &BlockId) -> HashSet { match self.get(id) { VertexHandle::Candidate(vertex) => vertex.vertex.know_most().clone(), _ => HashSet::new(), @@ -606,7 +606,7 @@ where /// Returns an extension request with the appropriate data if either: /// 1. We know of a justified header for which we do not have a block, or /// 2. We know of nodes which have children of our favourite block. - pub fn extension_request(&self) -> ExtensionRequest { + pub fn extension_request(&self) -> ExtensionRequest { use ExtensionRequest::*; use VertexHandle::*; if self.behind_finalization() > 0 { @@ -646,7 +646,7 @@ where /// Whether this block should be skipped during importing. /// It either needs to be already imported, or too old to be checked. - pub fn skippable(&self, id: &BlockIdFor) -> bool { + pub fn skippable(&self, id: &BlockId) -> bool { use SpecialState::{BelowMinimal, HighestFinalized}; use VertexHandle::{Candidate, Special}; match self.get(id) { @@ -657,7 +657,7 @@ where } /// The ID of the favourite block, i.e. the one for which we will accept imports of children. - pub fn favourite_block(&self) -> BlockIdFor { + pub fn favourite_block(&self) -> BlockId { self.favourite.clone() } } @@ -674,7 +674,7 @@ mod tests { mock::{Backend, MockHeader, MockJustification, MockPeerId}, ChainStatus, Header, Justification, }, - BlockIdentifier, BlockNumber, SessionPeriod, + BlockNumber, SessionPeriod, }; type MockForest = Forest; diff --git a/finality-aleph/src/sync/forest/vertex.rs b/finality-aleph/src/sync/forest/vertex.rs index 2622b43f8d..96b17d250d 100644 --- a/finality-aleph/src/sync/forest/vertex.rs +++ b/finality-aleph/src/sync/forest/vertex.rs @@ -1,6 +1,9 @@ use std::collections::HashSet; -use crate::sync::{BlockIdFor, Justification, PeerId}; +use crate::{ + sync::{Justification, PeerId}, + BlockId, +}; #[derive(Clone, Debug, Copy, PartialEq, Eq)] enum Importance { @@ -22,13 +25,13 @@ enum InnerVertex { /// Vertex with added Header. Header { importance: HeaderImportance, - parent: BlockIdFor, + parent: BlockId, }, /// Vertex with added Header and Justification. Justification { imported: bool, justification: J, - parent: BlockIdFor, + parent: BlockId, }, } @@ -118,7 +121,7 @@ impl Vertex { } /// The parent of the vertex, if known. - pub fn parent(&self) -> Option<&BlockIdFor> { + pub fn parent(&self) -> Option<&BlockId> { match &self.inner { InnerVertex::Empty { .. } => None, InnerVertex::Header { parent, .. } => Some(parent), @@ -199,7 +202,7 @@ impl Vertex { /// Adds the information the header provides to the vertex. /// Returns whether this is a new header. - pub fn insert_header(&mut self, parent: BlockIdFor, holder: Option) -> bool { + pub fn insert_header(&mut self, parent: BlockId, holder: Option) -> bool { self.add_block_holder(holder); match self.inner { InnerVertex::Empty { required } => { @@ -215,7 +218,7 @@ impl Vertex { /// Adds the information the header provides to the vertex and marks it as imported. Returns /// whether it was not imported before. - pub fn insert_body(&mut self, parent: BlockIdFor) -> bool { + pub fn insert_body(&mut self, parent: BlockId) -> bool { use InnerVertex::*; match &self.inner { Empty { .. } @@ -246,12 +249,7 @@ impl Vertex { } /// Adds a justification to the vertex. - pub fn insert_justification( - &mut self, - parent: BlockIdFor, - justification: J, - holder: Option, - ) { + pub fn insert_justification(&mut self, parent: BlockId, justification: J, holder: Option) { use InnerVertex::*; match self.inner { Empty { .. } @@ -289,9 +287,12 @@ impl Vertex { #[cfg(test)] mod tests { use super::Vertex; - use crate::sync::{ - mock::{MockHeader, MockIdentifier, MockJustification, MockPeerId}, - Header, + use crate::{ + sync::{ + mock::{MockHeader, MockJustification, MockPeerId}, + Header, + }, + BlockId, }; type MockVertex = Vertex; @@ -363,7 +364,7 @@ mod tests { fn empty_to_header() { let mut vertex = MockVertex::new(); let peer_id = rand::random(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); vertex.insert_header(parent.clone(), Some(peer_id)); assert!(!vertex.importable()); assert!(!vertex.requestable()); @@ -377,7 +378,7 @@ mod tests { fn header_remembers_block_holders() { let mut vertex = MockVertex::new(); let peer_id = rand::random(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); vertex.insert_header(parent, Some(peer_id)); let other_peer_id = rand::random(); vertex.add_block_holder(Some(other_peer_id)); @@ -389,7 +390,7 @@ mod tests { fn header_set_required() { let mut vertex = MockVertex::new(); let peer_id = rand::random(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); vertex.insert_header(parent, Some(peer_id)); assert!(vertex.set_required()); assert!(vertex.importable()); @@ -401,7 +402,7 @@ mod tests { fn header_set_explicitly_required() { let mut vertex = MockVertex::new(); let peer_id = rand::random(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); vertex.insert_header(parent, Some(peer_id)); assert!(vertex.set_explicitly_required()); assert!(vertex.importable()); @@ -416,7 +417,7 @@ mod tests { let mut vertex = MockVertex::new(); assert!(vertex.set_required()); let peer_id = rand::random(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); vertex.insert_header(parent, Some(peer_id)); assert!(vertex.importable()); assert!(!vertex.set_required()); @@ -428,7 +429,7 @@ mod tests { let mut vertex = MockVertex::new(); assert!(vertex.set_explicitly_required()); let peer_id = rand::random(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); vertex.insert_header(parent, Some(peer_id)); assert!(vertex.importable()); assert!(vertex.requestable()); @@ -440,7 +441,7 @@ mod tests { #[test] fn empty_to_body() { let mut vertex = MockVertex::new(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); assert!(vertex.insert_body(parent.clone())); assert!(!vertex.importable()); assert!(!vertex.requestable()); @@ -452,7 +453,7 @@ mod tests { #[test] fn body_twice() { let mut vertex = MockVertex::new(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); assert!(vertex.insert_body(parent.clone())); assert!(!vertex.insert_body(parent.clone())); assert!(!vertex.importable()); @@ -466,7 +467,7 @@ mod tests { fn header_to_body() { let mut vertex = MockVertex::new(); let peer_id = rand::random(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); vertex.insert_header(parent.clone(), Some(peer_id)); assert!(vertex.insert_body(parent.clone())); assert!(!vertex.importable()); @@ -479,7 +480,7 @@ mod tests { #[test] fn body_set_required() { let mut vertex = MockVertex::new(); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); assert!(vertex.insert_body(parent)); assert!(!vertex.set_required()); assert!(!vertex.importable()); @@ -492,7 +493,7 @@ mod tests { fn body_no_longer_required() { let mut vertex = MockVertex::new(); assert!(vertex.set_required()); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); assert!(vertex.insert_body(parent)); assert!(!vertex.importable()); assert!(!vertex.requestable()); @@ -502,7 +503,7 @@ mod tests { fn body_no_longer_explicitly_required() { let mut vertex = MockVertex::new(); assert!(vertex.set_explicitly_required()); - let parent = MockIdentifier::new_random(43); + let parent = BlockId::new_random(43); assert!(vertex.insert_body(parent)); assert!(!vertex.importable()); assert!(!vertex.requestable()); diff --git a/finality-aleph/src/sync/handler/mod.rs b/finality-aleph/src/sync/handler/mod.rs index b779e699e8..822b5d45f6 100644 --- a/finality-aleph/src/sync/handler/mod.rs +++ b/finality-aleph/src/sync/handler/mod.rs @@ -15,10 +15,10 @@ use crate::{ InitializationError as ForestInitializationError, Interest, }, handler::request_handler::RequestHandler, - Block, BlockIdFor, BlockImport, BlockStatus, ChainStatus, Finalizer, Header, Justification, - PeerId, UnverifiedJustification, Verifier, + Block, BlockImport, BlockStatus, ChainStatus, Finalizer, Header, Justification, PeerId, + UnverifiedJustification, Verifier, }, - BlockIdentifier, BlockNumber, SyncOracle, + BlockId, BlockNumber, SyncOracle, }; mod request_handler; @@ -73,7 +73,7 @@ where I: PeerId, J: Justification, { - pub fn get(&self, id: &BlockIdFor) -> Interest { + pub fn get(&self, id: &BlockId) -> Interest { self.forest.request_interest(id) } } @@ -263,7 +263,7 @@ where Finalizer(F::Error), Forest(ForestError), ForestInitialization(ForestInitializationError), - RequestHandlerError(RequestHandlerError), + RequestHandlerError(RequestHandlerError), MissingJustification, BlockNotImportable, HeaderNotRequired, @@ -332,7 +332,7 @@ where } } -impl From> for Error +impl From> for Error where J: Justification, B: Block
, @@ -340,7 +340,7 @@ where V: Verifier, F: Finalizer, { - fn from(e: RequestHandlerError) -> Self { + fn from(e: RequestHandlerError) -> Self { Error::RequestHandlerError(e) } } @@ -570,7 +570,7 @@ where ) -> (bool, Option<::Error>) { let mut new_highest = false; // Lets us import descendands of importable blocks, useful for favourite blocks. - let mut last_imported_block: Option> = None; + let mut last_imported_block: Option = None; for item in response_items { match item { ResponseItem::Justification(j) => { @@ -711,7 +711,7 @@ where /// Returns `true` if this was the first time something indicated interest in this block. pub fn handle_internal_request( &mut self, - id: &BlockIdFor, + id: &BlockId, ) -> Result::Error> { let should_request = self.forest.update_block_identifier(id, None, true)?; @@ -719,7 +719,7 @@ where } /// Returns the extension request we could be making right now. - pub fn extension_request(&self) -> ExtensionRequest { + pub fn extension_request(&self) -> ExtensionRequest { self.forest.extension_request() } } @@ -735,12 +735,12 @@ mod tests { data::{BranchKnowledge::*, NetworkData, Request, ResponseItem, ResponseItems, State}, forest::{ExtensionRequest, Interest}, handler::Action, - mock::{Backend, MockBlock, MockHeader, MockIdentifier, MockJustification, MockPeerId}, + mock::{Backend, MockBlock, MockHeader, MockJustification, MockPeerId}, Block, BlockImport, ChainStatus, ChainStatusNotification::*, ChainStatusNotifier, Header, Justification, }, - BlockIdentifier, BlockNumber, SessionPeriod, SyncOracle, + BlockId, BlockNumber, SessionPeriod, SyncOracle, }; type TestHandler = @@ -753,7 +753,7 @@ mod tests { TestHandler, Backend, impl ChainStatusNotifier, - MockIdentifier, + BlockId, ) { let (backend, notifier) = Backend::setup(SESSION_BOUNDARY_INFO); let verifier = backend.clone(); @@ -785,8 +785,8 @@ mod tests { fn assert_dangling_branch_required( handler: &TestHandler, - top: &MockIdentifier, - bottom: &MockIdentifier, + top: &BlockId, + bottom: &BlockId, know_most: HashSet, ) { assert_eq!( @@ -806,7 +806,7 @@ mod tests { fn grow_light_branch_till( handler: &mut TestHandler, - bottom: &MockIdentifier, + bottom: &BlockId, top: &BlockNumber, peer_id: MockPeerId, ) -> Vec { @@ -816,7 +816,7 @@ mod tests { fn grow_light_branch( handler: &mut TestHandler, - bottom: &MockIdentifier, + bottom: &BlockId, length: usize, peer_id: MockPeerId, ) -> Vec { @@ -857,8 +857,8 @@ mod tests { height: BlockNumber, length: usize, peer_id: MockPeerId, - ) -> (MockIdentifier, MockIdentifier) { - let bottom = MockIdentifier::new_random(height); + ) -> (BlockId, BlockId) { + let bottom = BlockId::new_random(height); let top = grow_light_branch(handler, &bottom, length, peer_id) .last() .expect("branch should not be empty") @@ -903,9 +903,9 @@ mod tests { handler: &mut TestHandler, backend: &mut Backend, notifier: &mut impl ChainStatusNotifier, - bottom: &MockIdentifier, + bottom: &BlockId, length: usize, - ) -> MockIdentifier { + ) -> BlockId { let branch: Vec<_> = bottom.random_branch().take(length).collect(); let top = branch.last().expect("should not be empty").id(); for header in branch.iter() { @@ -1280,7 +1280,7 @@ mod tests { // grow the dangling branch that will be pruned let fork_peer = 6; - let fork_bottom = MockIdentifier::new_random(15); + let fork_bottom = BlockId::new_random(15); let fork_child = fork_bottom.random_child(); let fork = grow_light_branch(&mut handler, &fork_child.id(), 10, fork_peer); let fork_top = fork.last().expect("fork not empty").id(); @@ -1891,8 +1891,8 @@ mod tests { let header = MockHeader::random_parentless(105); let state = State::new(MockJustification::for_header(header.clone()), header); - let requested_id = MockIdentifier::new_random(120); - let lowest_id = MockIdentifier::new_random(110); + let requested_id = BlockId::new_random(120); + let lowest_id = BlockId::new_random(110); let request = Request::new(requested_id.clone(), LowestId(lowest_id), state); diff --git a/finality-aleph/src/sync/handler/request_handler.rs b/finality-aleph/src/sync/handler/request_handler.rs index 4eb3bcda57..1932e1f1f1 100644 --- a/finality-aleph/src/sync/handler/request_handler.rs +++ b/finality-aleph/src/sync/handler/request_handler.rs @@ -8,28 +8,28 @@ use crate::{ sync::{ data::{BranchKnowledge, ResponseItem}, handler::Request, - Block, BlockIdFor, BlockStatus, ChainStatus, FinalizationStatus, Header, Justification, + Block, BlockStatus, ChainStatus, FinalizationStatus, Header, Justification, UnverifiedJustification, }, - BlockIdentifier, + BlockId, }; #[derive(Debug, Clone)] -pub enum RequestHandlerError { - MissingBlock(BlockIdFor), - MissingParent(BlockIdFor), +pub enum RequestHandlerError { + MissingBlock(BlockId), + MissingParent(BlockId), RootMismatch, LastBlockOfSessionNotJustified, ChainStatusError(T), } -impl From for RequestHandlerError { +impl From for RequestHandlerError { fn from(value: T) -> Self { RequestHandlerError::ChainStatusError(value) } } -impl Display for RequestHandlerError { +impl Display for RequestHandlerError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { RequestHandlerError::RootMismatch => write!( @@ -53,13 +53,7 @@ pub trait HandlerTypes { type ChainStatusError: Display; } -type HandlerResult = Result< - T, - RequestHandlerError< - ::Justification, - ::ChainStatusError, - >, ->; +type HandlerResult = Result::ChainStatusError>>; #[derive(Debug)] enum HeadOfChunk { @@ -68,14 +62,14 @@ enum HeadOfChunk { } impl HeadOfChunk { - pub fn id(&self) -> BlockIdFor { + pub fn id(&self) -> BlockId { match self { HeadOfChunk::Justification(j) => j.header().id(), HeadOfChunk::Header(h) => h.id(), } } - pub fn parent_id(&self) -> Option> { + pub fn parent_id(&self) -> Option { match self { HeadOfChunk::Justification(j) => j.header().parent_id(), HeadOfChunk::Header(h) => h.parent_id(), @@ -117,14 +111,14 @@ where } } - pub fn current_id(&self) -> BlockIdFor { + pub fn current_id(&self) -> BlockId { self.head.id() } pub fn update>( &mut self, chain_status: &CS, - ) -> Result> { + ) -> Result> { match self.state { State::EverythingButHeader => self.add_block(self.head.id(), chain_status)?, State::Everything if self.head.is_justification() => { @@ -141,9 +135,9 @@ where fn add_block>( &mut self, - id: BlockIdFor, + id: BlockId, chain_status: &CS, - ) -> Result<(), RequestHandlerError> { + ) -> Result<(), RequestHandlerError> { let block = chain_status .block(id.clone())? .ok_or(RequestHandlerError::MissingBlock(id))?; @@ -154,9 +148,9 @@ where fn add_block_and_header>( &mut self, - id: BlockIdFor, + id: BlockId, chain_status: &CS, - ) -> Result<(), RequestHandlerError> { + ) -> Result<(), RequestHandlerError> { let block = chain_status .block(id.clone())? .ok_or(RequestHandlerError::MissingBlock(id))?; @@ -167,7 +161,7 @@ where fn next_head>( &self, chain_status: &CS, - ) -> Result, RequestHandlerError> { + ) -> Result, RequestHandlerError> { let parent_id = self .head .parent_id() @@ -203,7 +197,7 @@ where B: Block, J: Justification
, { - RequestBlock(BlockIdFor), + RequestBlock(BlockId), Response(Vec>), Noop, } @@ -213,7 +207,7 @@ where B: Block, J: Justification
, { - fn request_block(id: BlockIdFor) -> Self { + fn request_block(id: BlockId) -> Self { Action::RequestBlock(id) } @@ -324,7 +318,7 @@ where } } - fn upper_limit(&self, id: BlockIdFor) -> BlockNumber { + fn upper_limit(&self, id: BlockId) -> BlockNumber { let session = self.session_info.session_id_from_block_num(id.number()); self.session_info .last_block_of_session(SessionId(session.0 + 1)) @@ -333,8 +327,8 @@ where fn is_result_complete( &self, result: &mut StepResult, - branch_knowledge: &BranchKnowledge, - to: &BlockIdFor, + branch_knowledge: &BranchKnowledge, + to: &BlockId, ) -> HandlerResult { Ok(match branch_knowledge { _ if result.current_id() == *to => true, @@ -357,8 +351,8 @@ where &self, state: State, from: HeadOfChunk, - to: &BlockIdFor, - branch_knowledge: &BranchKnowledge, + to: &BlockId, + branch_knowledge: &BranchKnowledge, ) -> HandlerResult>, Self> { if from.id() == *to { return Ok(None); @@ -373,8 +367,8 @@ where fn response_items( self, mut head: HeadOfChunk, - branch_knowledge: BranchKnowledge, - to: BlockIdFor, + branch_knowledge: BranchKnowledge, + to: BlockId, ) -> HandlerResult>, Self> { let mut response_items = vec![]; let mut state = State::EverythingButHeader; diff --git a/finality-aleph/src/sync/mock/backend.rs b/finality-aleph/src/sync/mock/backend.rs index ea3779e8f9..dd43ed4e1d 100644 --- a/finality-aleph/src/sync/mock/backend.rs +++ b/finality-aleph/src/sync/mock/backend.rs @@ -11,19 +11,19 @@ use crate::{ nodes::VERIFIER_CACHE_SIZE, session::{SessionBoundaryInfo, SessionId}, sync::{ - mock::{MockBlock, MockHeader, MockIdentifier, MockJustification, MockNotification}, + mock::{MockBlock, MockHeader, MockJustification, MockNotification}, Block, BlockImport, BlockStatus, ChainStatus, ChainStatusNotifier, FinalizationStatus, Finalizer, Header, Justification as JustificationT, Verifier, }, - BlockIdentifier, + BlockId, }; #[derive(Clone, Debug)] struct BackendStorage { session_boundary_info: SessionBoundaryInfo, - blockchain: HashMap, - finalized: Vec, - prune_candidates: HashSet, + blockchain: HashMap, + finalized: Vec, + prune_candidates: HashSet, } #[derive(Clone, Debug)] @@ -33,11 +33,11 @@ pub struct Backend { } fn is_predecessor( - blockchain: &HashMap, - id: &MockIdentifier, - maybe_predecessor: &MockIdentifier, - definitely_not: &HashSet, - definitely: &HashSet, + blockchain: &HashMap, + id: &BlockId, + maybe_predecessor: &BlockId, + definitely_not: &HashSet, + definitely: &HashSet, ) -> bool { let mut header = blockchain.get(id).expect("should exist").header(); while let Some(parent) = header.parent_id() { @@ -308,7 +308,7 @@ impl Display for StatusError { impl ChainStatus for Backend { type Error = StatusError; - fn status_of(&self, id: MockIdentifier) -> Result, Self::Error> { + fn status_of(&self, id: BlockId) -> Result, Self::Error> { let storage = self.inner.lock(); let block = match storage.blockchain.get(&id) { Some(block) => block, @@ -322,7 +322,7 @@ impl ChainStatus for Backend { } } - fn block(&self, id: MockIdentifier) -> Result, Self::Error> { + fn block(&self, id: BlockId) -> Result, Self::Error> { Ok(self.inner.lock().blockchain.get(&id).cloned()) } @@ -369,7 +369,7 @@ impl ChainStatus for Backend { .ok_or(StatusError) } - fn children(&self, id: MockIdentifier) -> Result, Self::Error> { + fn children(&self, id: BlockId) -> Result, Self::Error> { match self.status_of(id.clone())? { BlockStatus::Unknown => Err(StatusError), _ => { diff --git a/finality-aleph/src/sync/mock/mod.rs b/finality-aleph/src/sync/mock/mod.rs index e9d576f002..20335d069a 100644 --- a/finality-aleph/src/sync/mock/mod.rs +++ b/finality-aleph/src/sync/mock/mod.rs @@ -1,40 +1,27 @@ use std::hash::Hash; use parity_scale_codec::{Decode, Encode}; -use sp_core::H256; +use primitives::{BlockHash, BlockNumber}; use crate::{ sync::{Block, ChainStatusNotification, Header, Justification, UnverifiedJustification}, - BlockIdentifier, + BlockId, }; mod backend; mod status_notifier; -type MockNumber = u32; -type MockHash = H256; - pub use backend::Backend; pub type MockPeerId = u32; -#[derive(Clone, Hash, Debug, PartialEq, Eq, Encode, Decode)] -pub struct MockIdentifier { - number: MockNumber, - hash: MockHash, -} - -impl MockIdentifier { - fn new(number: MockNumber, hash: MockHash) -> Self { - MockIdentifier { number, hash } - } - - pub fn new_random(number: MockNumber) -> Self { - MockIdentifier::new(number, MockHash::random()) +impl BlockId { + pub fn new_random(number: BlockNumber) -> Self { + Self::new(BlockHash::random(), number) } pub fn random_child(&self) -> MockHeader { - let id = MockIdentifier::new_random(self.number + 1); + let id = Self::new_random(self.number + 1); let parent = Some(self.clone()); MockHeader { id, parent } } @@ -46,31 +33,25 @@ impl MockIdentifier { } } -impl BlockIdentifier for MockIdentifier { - fn number(&self) -> u32 { - self.number - } -} - #[derive(Clone, Hash, Debug, PartialEq, Eq, Encode, Decode)] pub struct MockHeader { - id: MockIdentifier, - parent: Option, + id: BlockId, + parent: Option, } impl MockHeader { pub fn genesis() -> Self { MockHeader { - id: MockIdentifier { + id: BlockId { number: 0, - hash: MockHash::zero(), + hash: BlockHash::zero(), }, parent: None, } } - pub fn random_parentless(number: MockNumber) -> Self { - let id = MockIdentifier::new_random(number); + pub fn random_parentless(number: BlockNumber) -> Self { + let id = BlockId::new_random(number); MockHeader { id, parent: None } } @@ -84,7 +65,7 @@ impl MockHeader { } struct RandomBranch { - parent: MockIdentifier, + parent: BlockId, } impl Iterator for RandomBranch { @@ -98,13 +79,11 @@ impl Iterator for RandomBranch { } impl Header for MockHeader { - type Identifier = MockIdentifier; - - fn id(&self) -> Self::Identifier { + fn id(&self) -> BlockId { self.id.clone() } - fn parent_id(&self) -> Option { + fn parent_id(&self) -> Option { self.parent.clone() } } @@ -135,13 +114,11 @@ impl MockBlock { } impl Header for MockBlock { - type Identifier = MockIdentifier; - - fn id(&self) -> Self::Identifier { + fn id(&self) -> BlockId { self.header().id() } - fn parent_id(&self) -> Option { + fn parent_id(&self) -> Option { self.header().parent_id() } } diff --git a/finality-aleph/src/sync/mod.rs b/finality-aleph/src/sync/mod.rs index 83e1e30f6a..b6b26f336c 100644 --- a/finality-aleph/src/sync/mod.rs +++ b/finality-aleph/src/sync/mod.rs @@ -6,6 +6,8 @@ use std::{ use parity_scale_codec::Codec; +use crate::BlockId; + mod compatibility; mod data; mod forest; @@ -27,8 +29,6 @@ pub use substrate::{ SubstrateChainStatus, SubstrateChainStatusNotifier, SubstrateFinalizationInfo, VerifierCache, }; -use crate::BlockIdentifier; - const LOG_TARGET: &str = "aleph-block-sync"; /// The identifier of a connected peer. @@ -38,13 +38,11 @@ impl PeerId for T {} /// The header of a block, containing information about the parent relation. pub trait Header: Clone + Codec + Debug + Send + Sync + 'static { - type Identifier: BlockIdentifier; - /// The identifier of this block. - fn id(&self) -> Self::Identifier; + fn id(&self) -> BlockId; /// The identifier of this block's parent. - fn parent_id(&self) -> Option; + fn parent_id(&self) -> Option; } /// The block, including a header. @@ -61,8 +59,6 @@ pub trait BlockImport: Send + 'static { fn import_block(&mut self, block: B); } -type BlockIdFor = <::Header as Header>::Identifier; - pub trait UnverifiedJustification: Clone + Codec + Send + Sync + Debug + 'static { type Header: Header; @@ -159,10 +155,10 @@ where type Error: Display; /// The status of the block. - fn status_of(&self, id: BlockIdFor) -> Result, Self::Error>; + fn status_of(&self, id: BlockId) -> Result, Self::Error>; /// Export a copy of the block. - fn block(&self, id: BlockIdFor) -> Result, Self::Error>; + fn block(&self, id: BlockId) -> Result, Self::Error>; /// The justification at this block number, if we have it otherwise just block id if /// the block is finalized without justification. Should return NotFinalized variant if @@ -176,7 +172,7 @@ where fn top_finalized(&self) -> Result; /// Children of the specified block. - fn children(&self, id: BlockIdFor) -> Result, Self::Error>; + fn children(&self, id: BlockId) -> Result, Self::Error>; } /// An interface for submitting additional justifications to the justification sync. @@ -192,9 +188,9 @@ pub trait JustificationSubmissions: Clone + Send + 'static { /// An interface for requesting specific blocks from the block sync. /// Required by the data availability mechanism in ABFT. -pub trait RequestBlocks: Clone + Send + Sync + 'static { +pub trait RequestBlocks: Clone + Send + Sync + 'static { type Error: Display; /// Request the given block. - fn request_block(&self, block_id: BI) -> Result<(), Self::Error>; + fn request_block(&self, block_id: BlockId) -> Result<(), Self::Error>; } diff --git a/finality-aleph/src/sync/service.rs b/finality-aleph/src/sync/service.rs index 221cd1932f..22b0f300cf 100644 --- a/finality-aleph/src/sync/service.rs +++ b/finality-aleph/src/sync/service.rs @@ -20,9 +20,9 @@ use crate::{ task_queue::TaskQueue, tasks::{Action as TaskAction, RequestTask}, ticker::Ticker, - Block, BlockIdFor, BlockIdentifier, BlockImport, ChainStatus, ChainStatusNotification, - ChainStatusNotifier, Finalizer, Header, Justification, JustificationSubmissions, - RequestBlocks, UnverifiedJustification, Verifier, LOG_TARGET, + Block, BlockId, BlockImport, ChainStatus, ChainStatusNotification, ChainStatusNotifier, + Finalizer, Header, Justification, JustificationSubmissions, RequestBlocks, + UnverifiedJustification, Verifier, LOG_TARGET, }, SyncOracle, }; @@ -92,13 +92,13 @@ where { network: VersionWrapper, handler: Handler, - tasks: TaskQueue>>, + tasks: TaskQueue, broadcast_ticker: Ticker, chain_extension_ticker: Ticker, chain_events: CE, justifications_from_user: mpsc::UnboundedReceiver, additional_justifications_from_user: mpsc::UnboundedReceiver, - block_requests_from_user: mpsc::UnboundedReceiver>, + block_requests_from_user: mpsc::UnboundedReceiver, _phantom: PhantomData, metrics: Metrics, } @@ -111,10 +111,10 @@ impl JustificationSubmissions for mpsc::UnboundedSender RequestBlocks for mpsc::UnboundedSender { - type Error = mpsc::TrySendError; +impl RequestBlocks for mpsc::UnboundedSender { + type Error = mpsc::TrySendError; - fn request_block(&self, block_id: BI) -> Result<(), Self::Error> { + fn request_block(&self, block_id: BlockId) -> Result<(), Self::Error> { self.unbounded_send(block_id) } } @@ -142,7 +142,7 @@ where ( Self, impl JustificationSubmissions + Clone, - impl RequestBlocks>, + impl RequestBlocks, ), HandlerError, > { @@ -187,7 +187,7 @@ where )) } - fn request_block(&mut self, block_id: BlockIdFor) { + fn request_block(&mut self, block_id: BlockId) { debug!( target: LOG_TARGET, "Initiating a request for block {:?}.", block_id @@ -276,7 +276,7 @@ where } } - fn send_request(&mut self, pre_request: PreRequest) { + fn send_request(&mut self, pre_request: PreRequest) { self.metrics.report_event(Event::SendRequest); let state = match self.handler.state() { Ok(state) => state, @@ -481,7 +481,7 @@ where } } - fn handle_task(&mut self, task: RequestTask>) { + fn handle_task(&mut self, task: RequestTask) { trace!(target: LOG_TARGET, "Handling task {}.", task); if let TaskAction::Request(pre_request, (task, delay)) = task.process(self.handler.interest_provider()) @@ -519,7 +519,7 @@ where } } - fn handle_internal_request(&mut self, id: BlockIdFor) { + fn handle_internal_request(&mut self, id: BlockId) { trace!( target: LOG_TARGET, "Handling an internal request for block {:?}.", diff --git a/finality-aleph/src/sync/substrate/chain_status.rs b/finality-aleph/src/sync/substrate/chain_status.rs index c9d24adf20..35d237a4ea 100644 --- a/finality-aleph/src/sync/substrate/chain_status.rs +++ b/finality-aleph/src/sync/substrate/chain_status.rs @@ -15,10 +15,10 @@ use crate::{ }, justification::backwards_compatible_decode, sync::{ - substrate::{BlockId, Justification}, - BlockIdFor, BlockStatus, ChainStatus, FinalizationStatus, Header, Justification as _, - LOG_TARGET, + substrate::Justification, BlockStatus, ChainStatus, FinalizationStatus, Header, + Justification as _, LOG_TARGET, }, + BlockId, }; /// What can go wrong when checking chain status @@ -108,7 +108,7 @@ impl SubstrateChainStatus { self.backend.blockchain().body(hash) } - fn header(&self, id: &BlockIdFor) -> Result, Error> { + fn header(&self, id: &BlockId) -> Result, Error> { let maybe_header = self.header_for_hash(id.hash)?; match maybe_header .as_ref() @@ -186,7 +186,7 @@ impl ChainStatus for SubstrateChainStatus { } } - fn block(&self, id: BlockIdFor) -> Result, Self::Error> { + fn block(&self, id: BlockId) -> Result, Self::Error> { let header = match self.header(&id)? { Some(header) => header, None => return Ok(None), @@ -198,10 +198,7 @@ impl ChainStatus for SubstrateChainStatus { Ok(Some(Block::new(header, body))) } - fn status_of( - &self, - id: BlockIdFor, - ) -> Result, Self::Error> { + fn status_of(&self, id: BlockId) -> Result, Self::Error> { let header = match self.header(&id)? { Some(header) => header, None => return Ok(BlockStatus::Unknown), @@ -230,7 +227,7 @@ impl ChainStatus for SubstrateChainStatus { .ok_or(Error::MissingJustification(finalized_hash)) } - fn children(&self, id: BlockIdFor) -> Result, Self::Error> { + fn children(&self, id: BlockId) -> Result, Self::Error> { // This checks whether we have the block at all and the provided id is consistent. self.header(&id)?; Ok(self diff --git a/finality-aleph/src/sync/substrate/mod.rs b/finality-aleph/src/sync/substrate/mod.rs index a63bab165b..2151da3af9 100644 --- a/finality-aleph/src/sync/substrate/mod.rs +++ b/finality-aleph/src/sync/substrate/mod.rs @@ -66,16 +66,14 @@ impl BlockImport for BlockImporter { } impl HeaderT for Header { - type Identifier = BlockId; - - fn id(&self) -> Self::Identifier { + fn id(&self) -> BlockId { BlockId { hash: self.hash(), number: *self.number(), } } - fn parent_id(&self) -> Option { + fn parent_id(&self) -> Option { let number = self.number().checked_sub(&One::one())?; Some(BlockId { hash: *self.parent_hash(), diff --git a/finality-aleph/src/sync/tasks.rs b/finality-aleph/src/sync/tasks.rs index 4eaa0cac4c..9b6978b1ee 100644 --- a/finality-aleph/src/sync/tasks.rs +++ b/finality-aleph/src/sync/tasks.rs @@ -7,11 +7,8 @@ use std::{ use rand::{thread_rng, Rng}; use crate::{ - sync::{ - data::PreRequest, forest::Interest, handler::InterestProvider, BlockIdFor, Header, - Justification, PeerId, - }, - BlockIdentifier, + sync::{data::PreRequest, forest::Interest, handler::InterestProvider, Justification, PeerId}, + BlockId, }; const MIN_DELAY: Duration = Duration::from_millis(300); @@ -27,37 +24,36 @@ fn delay_for_attempt(attempt: u32) -> Duration { } /// A task for requesting blocks. Keeps track of how many times it was executed. -pub struct RequestTask { - id: BI, +pub struct RequestTask { + id: BlockId, tries: u32, } -impl Display for RequestTask { +impl Display for RequestTask { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { write!(f, "block request for {:?}, attempt {}", self.id, self.tries) } } -type DelayedTask = (RequestTask, Duration); +type DelayedTask = (RequestTask, Duration); /// What do to with the task, either ignore or perform a request and add a delayed task. -pub enum Action { +pub enum Action { Ignore, - Request(PreRequest, DelayedTask>), + Request(PreRequest, DelayedTask), } -impl RequestTask { +impl RequestTask { /// A new task for requesting block with the provided ID. - pub fn new(id: BI) -> Self { + pub fn new(id: BlockId) -> Self { RequestTask { id, tries: 0 } } /// Process the task. - pub fn process(self, interest_provider: InterestProvider) -> Action + pub fn process(self, interest_provider: InterestProvider) -> Action where I: PeerId, J: Justification, - J::Header: Header, { let RequestTask { id, tries } = self; match interest_provider.get(&id) { diff --git a/finality-aleph/src/testing/data_store.rs b/finality-aleph/src/testing/data_store.rs index 4c2928265a..89d5428174 100644 --- a/finality-aleph/src/testing/data_store.rs +++ b/finality-aleph/src/testing/data_store.rs @@ -42,7 +42,7 @@ impl TestBlockRequester { } } -impl RequestBlocks for TestBlockRequester { +impl RequestBlocks for TestBlockRequester { type Error = TrySendError; fn request_block(&self, block_id: BlockId) -> Result<(), Self::Error> { self.blocks.unbounded_send(block_id) diff --git a/finality-aleph/src/testing/mocks/block_finalizer.rs b/finality-aleph/src/testing/mocks/block_finalizer.rs index 8b317f4440..f867fe5926 100644 --- a/finality-aleph/src/testing/mocks/block_finalizer.rs +++ b/finality-aleph/src/testing/mocks/block_finalizer.rs @@ -1,7 +1,6 @@ use sp_blockchain::Error; use sp_runtime::{traits::Block, Justification}; -use super::TBlockIdentifier; use crate::{ finalization::BlockFinalizer, testing::mocks::{single_action_mock::SingleActionMock, TBlock}, @@ -27,19 +26,15 @@ impl MockedBlockFinalizer { pub async fn has_been_invoked_with(&self, block: TBlock) -> bool { self.mock - .has_been_invoked_with(|(TBlockIdentifier { hash, number }, _)| { + .has_been_invoked_with(|(BlockId { hash, number }, _)| { block.hash() == hash && block.header.number == number }) .await } } -impl BlockFinalizer for MockedBlockFinalizer { - fn finalize_block( - &self, - block_id: TBlockIdentifier, - justification: Justification, - ) -> Result<(), Error> { +impl BlockFinalizer for MockedBlockFinalizer { + fn finalize_block(&self, block_id: BlockId, justification: Justification) -> Result<(), Error> { self.mock.invoke_with((block_id, justification)); Ok(()) } diff --git a/finality-aleph/src/testing/mocks/mod.rs b/finality-aleph/src/testing/mocks/mod.rs index 17ed01e7b5..e8171bd755 100644 --- a/finality-aleph/src/testing/mocks/mod.rs +++ b/finality-aleph/src/testing/mocks/mod.rs @@ -7,13 +7,12 @@ pub use proposal::{ use sp_runtime::traits::BlakeTwo256; use substrate_test_runtime::Extrinsic; -use crate::{aleph_primitives::BlockNumber, BlockId}; +use crate::aleph_primitives::BlockNumber; type Hashing = BlakeTwo256; pub type TBlock = sp_runtime::generic::Block; pub type THeader = sp_runtime::generic::Header; pub type THash = substrate_test_runtime::Hash; -pub type TBlockIdentifier = BlockId; mod acceptance_policy; mod block_finalizer;