A0-3134: Remove BlockIdentifier (#1424)

# Description

Trait `BlockIdentifier` is removed, struct `BlockId` is used.

## Type of change

- Refactor

Co-authored-by: Damian Leśniak <[email protected]>

finality-aleph/src/data_io/data_store.rs

@@ -149,7 +149,7 @@ where
     B: BlockT<Hash = BlockHash>,
     B::Header: HeaderT<Number = BlockNumber>,
     C: HeaderBackend<B> + BlockchainEvents<B> + Send + Sync + 'static,
-    RB: RequestBlocks<BlockId> + 'static,
+    RB: RequestBlocks + 'static,
     Message: AlephNetworkMessage
         + std::fmt::Debug
         + Send
@@ -182,7 +182,7 @@ where
     B: BlockT<Hash = BlockHash>,
     B::Header: HeaderT<Number = BlockNumber>,
     C: HeaderBackend<B> + BlockchainEvents<B> + Send + Sync + 'static,
-    RB: RequestBlocks<BlockId> + 'static,
+    RB: RequestBlocks + 'static,
     Message: AlephNetworkMessage
         + std::fmt::Debug
         + Send

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<BI: BlockIdentifier> {
-    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<B, BE, C>
@@ -45,7 +45,7 @@ where
     }
 }
 
-impl<B, BE, C> BlockFinalizer<BlockId> for AlephFinalizer<B, BE, C>
+impl<B, BE, C> BlockFinalizer for AlephFinalizer<B, BE, C>
 where
     B: Block<Hash = BlockHash>,
     B::Header: Header<Number = BlockNumber>,

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<BlakeTwo256>;
 
+/// 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.

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<BI: BlockIdentifier>: 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.

finality-aleph/src/network/substrate.rs

@@ -25,7 +25,7 @@ use crate::{
     BlockId,
 };
 
-impl<B> RequestBlocks<BlockId> for Arc<SyncingService<B>>
+impl<B> RequestBlocks for Arc<SyncingService<B>>
 where
     B: Block<Hash = BlockHash>,
     B::Header: Header<Number = BlockNumber>,

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<Hash = BlockHash>,
     B::Header: Header<Number = BlockNumber>,
     C: HeaderBackend<B> + BlockchainEvents<B> + Send + Sync + 'static,
-    RB: RequestBlocks<BlockId> + 'static,
+    RB: RequestBlocks + 'static,
     Message: AlephNetworkMessage + Debug + Send + Sync + Codec + 'static,
     R: Receiver<Message> + 'static,
 {

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<B, BE> + Send + Sync + 'static,
     BE: Backend<B> + 'static,
     SC: SelectChain<B> + 'static,
-    RB: RequestBlocks<BlockId>,
+    RB: RequestBlocks,
     SM: SessionManager<VersionedNetworkData> + 'static,
     JS: JustificationSubmissions<Justification> + Send + Sync + Clone,
 {
@@ -123,7 +123,7 @@ where
     C::Api: crate::aleph_primitives::AlephSessionApi<B>,
     BE: Backend<B> + 'static,
     SC: SelectChain<B> + 'static,
-    RB: RequestBlocks<BlockId>,
+    RB: RequestBlocks,
     SM: SessionManager<VersionedNetworkData>,
     JS: JustificationSubmissions<Justification> + Send + Sync + Clone + 'static,
 {
@@ -405,7 +405,7 @@ where
     C::Api: crate::aleph_primitives::AlephSessionApi<B>,
     BE: Backend<B> + 'static,
     SC: SelectChain<B> + 'static,
-    RB: RequestBlocks<BlockId>,
+    RB: RequestBlocks,
     SM: SessionManager<VersionedNetworkData>,
     JS: JustificationSubmissions<Justification> + Send + Sync + Clone + 'static,
 {

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<OBR, NBR, BI>
+pub struct OldSyncCompatibleRequestBlocks<OBR, NBR>
 where
-    OBR: OldRequestBlocks<BI>,
-    NBR: NewRequestBlocks<BI>,
-    BI: BlockIdentifier,
+    OBR: OldRequestBlocks,
+    NBR: NewRequestBlocks,
 {
     new: NBR,
     old: OBR,
-    _phantom: PhantomData<BI>,
 }
 
-impl<OBR, NBR, BI> OldSyncCompatibleRequestBlocks<OBR, NBR, BI>
+impl<OBR, NBR> OldSyncCompatibleRequestBlocks<OBR, NBR>
 where
-    OBR: OldRequestBlocks<BI>,
-    NBR: NewRequestBlocks<BI>,
-    BI: BlockIdentifier,
+    OBR: OldRequestBlocks,
+    NBR: NewRequestBlocks,
 {
     pub fn new(old: OBR, new: NBR) -> Self {
-        Self {
-            new,
-            old,
-            _phantom: PhantomData,
-        }
+        Self { new, old }
     }
 }
 
-impl<OBR, NBR, BI> NewRequestBlocks<BI> for OldSyncCompatibleRequestBlocks<OBR, NBR, BI>
+impl<OBR, NBR> NewRequestBlocks for OldSyncCompatibleRequestBlocks<OBR, NBR>
 where
-    OBR: OldRequestBlocks<BI>,
-    NBR: NewRequestBlocks<BI>,
-    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)

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<B, J> = Vec<ResponseItem<B, J>>;
 /// 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<J: Justification> {
+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<J>),
+    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<J>),
+    TopImported(BlockId),
 }
 
 /// Request content, first version.
 #[derive(Clone, Debug, Encode, Decode)]
 pub struct RequestV1<J: Justification> {
-    target_id: BlockIdFor<J>,
-    branch_knowledge: BranchKnowledge<J>,
+    target_id: BlockId,
+    branch_knowledge: BranchKnowledge,
     state: StateV1<J>,
 }
 
@@ -116,8 +116,8 @@ impl<J: Justification> RequestV1<J> {
 /// Request content, current version.
 #[derive(Clone, Debug, Encode, Decode)]
 pub struct Request<J: Justification> {
-    target_id: BlockIdFor<J>,
-    branch_knowledge: BranchKnowledge<J>,
+    target_id: BlockId,
+    branch_knowledge: BranchKnowledge,
     state: State<J>,
 }
 
@@ -152,11 +152,7 @@ impl<J: Justification> From<Request<J>> for RequestV1<J> {
 }
 
 impl<J: Justification> Request<J> {
-    pub fn new(
-        target_id: BlockIdFor<J>,
-        branch_knowledge: BranchKnowledge<J>,
-        state: State<J>,
-    ) -> Self {
+    pub fn new(target_id: BlockId, branch_knowledge: BranchKnowledge, state: State<J>) -> Self {
         Self {
             target_id,
             branch_knowledge,
@@ -169,27 +165,23 @@ impl<J: Justification> Request<J> {
     pub fn state(&self) -> &State<J> {
         &self.state
     }
-    pub fn target_id(&self) -> &BlockIdFor<J> {
+    pub fn target_id(&self) -> &BlockId {
         &self.target_id
     }
-    pub fn branch_knowledge(&self) -> &BranchKnowledge<J> {
+    pub fn branch_knowledge(&self) -> &BranchKnowledge {
         &self.branch_knowledge
     }
 }
 
 /// Data that can be used to generate a request given our state.
-pub struct PreRequest<I: PeerId, J: Justification> {
-    id: BlockIdFor<J>,
-    branch_knowledge: BranchKnowledge<J>,
+pub struct PreRequest<I: PeerId> {
+    id: BlockId,
+    branch_knowledge: BranchKnowledge,
     know_most: HashSet<I>,
 }
 
-impl<I: PeerId, J: Justification> PreRequest<I, J> {
-    pub fn new(
-        id: BlockIdFor<J>,
-        branch_knowledge: BranchKnowledge<J>,
-        know_most: HashSet<I>,
-    ) -> Self {
+impl<I: PeerId> PreRequest<I> {
+    pub fn new(id: BlockId, branch_knowledge: BranchKnowledge, know_most: HashSet<I>) -> Self {
         PreRequest {
             id,
             branch_knowledge,
@@ -198,7 +190,7 @@ impl<I: PeerId, J: Justification> PreRequest<I, J> {
     }
 
     /// Convert to a request and recipients given a state.
-    pub fn with_state(self, state: State<J>) -> (Request<J>, HashSet<I>) {
+    pub fn with_state<J: Justification>(self, state: State<J>) -> (Request<J>, HashSet<I>) {
         let PreRequest {
             id,
             branch_knowledge,