A0-1876: refactor session info (#1000)

maciejnems · web-flow · commit 24216c2e2af5 · 2023-03-14T11:43:15.000+01:00
* refactor SessionInfo to use BlockNumber type

* rename SessionInfo to SessionBoundaryInfo

* rename AuthorityProvider argument
diff --git a/finality-aleph/src/lib.rs b/finality-aleph/src/lib.rs
@@ -23,10 +23,7 @@ use crate::{
     abft::{CurrentNetworkData, LegacyNetworkData, CURRENT_VERSION, LEGACY_VERSION},
     aggregation::{CurrentRmcNetworkData, LegacyRmcNetworkData},
     network::data::split::Split,
-    session::{
-        first_block_of_session, last_block_of_session, session_id_from_block_num,
-        SessionBoundaries, SessionId,
-    },
+    session::{SessionBoundaries, SessionId},
     VersionedTryFromError::{ExpectedNewGotOld, ExpectedOldGotNew},
 };
 
diff --git a/finality-aleph/src/nodes/mod.rs b/finality-aleph/src/nodes/mod.rs
@@ -3,6 +3,7 @@ mod validator_node;
 
 use std::{future::Future, sync::Arc};
 
+use aleph_primitives::BlockNumber;
 pub use nonvalidator_node::run_nonvalidator_node;
 use sc_client_api::Backend;
 use sc_network::NetworkService;
@@ -15,9 +16,9 @@ use crate::{
     justification::{
         JustificationHandler, JustificationRequestSchedulerImpl, SessionInfo, SessionInfoProvider,
     },
-    last_block_of_session, mpsc,
+    mpsc,
     mpsc::UnboundedSender,
-    session_id_from_block_num,
+    session::SessionBoundaryInfo,
     session_map::ReadOnlySessionMap,
     sync::SessionVerifier,
     BlockchainBackend, JustificationNotification, Metrics, MillisecsPerBlock, SessionPeriod,
@@ -44,23 +45,26 @@ struct JustificationParams<B: Block, H: ExHashT, C, BB> {
 
 struct SessionInfoProviderImpl {
     session_authorities: ReadOnlySessionMap,
-    session_period: SessionPeriod,
+    session_info: SessionBoundaryInfo,
 }
 
 impl SessionInfoProviderImpl {
     fn new(session_authorities: ReadOnlySessionMap, session_period: SessionPeriod) -> Self {
         Self {
             session_authorities,
-            session_period,
+            session_info: SessionBoundaryInfo::new(session_period),
         }
     }
 }
 
 #[async_trait::async_trait]
-impl<B: Block> SessionInfoProvider<B, SessionVerifier> for SessionInfoProviderImpl {
+impl<B: Block> SessionInfoProvider<B, SessionVerifier> for SessionInfoProviderImpl
+where
+    B::Header: Header<Number = BlockNumber>,
+{
     async fn for_block_num(&self, number: NumberFor<B>) -> SessionInfo<B, SessionVerifier> {
-        let current_session = session_id_from_block_num(number, self.session_period);
-        let last_block_height = last_block_of_session(current_session, self.session_period);
+        let current_session = self.session_info.session_id_from_block_num(number);
+        let last_block_height = self.session_info.last_block_of_session(current_session);
         let verifier = self
             .session_authorities
             .get(current_session)
@@ -83,6 +87,7 @@ fn setup_justification_handler<B, H, C, BB, BE>(
 )
 where
     B: Block,
+    B::Header: Header<Number = BlockNumber>,
     H: ExHashT,
     C: crate::ClientForAleph<B, BE> + Send + Sync + 'static,
     C::Api: aleph_primitives::AlephSessionApi<B>,
diff --git a/finality-aleph/src/nodes/nonvalidator_node.rs b/finality-aleph/src/nodes/nonvalidator_node.rs
@@ -1,8 +1,9 @@
+use aleph_primitives::BlockNumber;
 use log::{debug, error};
 use sc_client_api::Backend;
 use sc_network_common::ExHashT;
 use sp_consensus::SelectChain;
-use sp_runtime::traits::Block;
+use sp_runtime::traits::{Block, Header};
 
 use crate::{
     nodes::{setup_justification_handler, JustificationParams},
@@ -13,6 +14,7 @@ use crate::{
 pub async fn run_nonvalidator_node<B, H, C, BB, BE, SC>(aleph_config: AlephConfig<B, H, C, SC, BB>)
 where
     B: Block,
+    B::Header: Header<Number = BlockNumber>,
     H: ExHashT,
     C: crate::ClientForAleph<B, BE> + Send + Sync + 'static,
     C::Api: aleph_primitives::AlephSessionApi<B>,
diff --git a/finality-aleph/src/nodes/validator_node.rs b/finality-aleph/src/nodes/validator_node.rs
@@ -20,10 +20,10 @@ use crate::{
     },
     nodes::{setup_justification_handler, JustificationParams},
     party::{
-        impls::{ChainStateImpl, SessionInfoImpl},
-        manager::NodeSessionManagerImpl,
-        ConsensusParty, ConsensusPartyParams,
+        impls::ChainStateImpl, manager::NodeSessionManagerImpl, ConsensusParty,
+        ConsensusPartyParams,
     },
+    session::SessionBoundaryInfo,
     session_map::{AuthorityProviderImpl, FinalityNotificatorImpl, SessionMapUpdater},
     AlephConfig, BlockchainBackend,
 };
@@ -165,7 +165,7 @@ where
             connection_manager,
             keystore,
         ),
-        session_info: SessionInfoImpl::new(session_period),
+        session_info: SessionBoundaryInfo::new(session_period),
     });
 
     debug!(target: "aleph-party", "Consensus party has started.");
diff --git a/finality-aleph/src/party/impls.rs b/finality-aleph/src/party/impls.rs
@@ -8,9 +8,8 @@ use sp_consensus::SyncOracle;
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 
 use crate::{
-    party::traits::{ChainState, SessionInfo, SyncState},
-    session::{first_block_of_session, last_block_of_session, session_id_from_block_num},
-    ClientForAleph, SessionId, SessionPeriod,
+    party::traits::{ChainState, SyncState},
+    ClientForAleph,
 };
 
 pub struct ChainStateImpl<B, BE, CFA>
@@ -38,30 +37,6 @@ where
     }
 }
 
-pub struct SessionInfoImpl {
-    session_period: SessionPeriod,
-}
-
-impl SessionInfoImpl {
-    pub fn new(session_period: SessionPeriod) -> Self {
-        Self { session_period }
-    }
-}
-
-impl SessionInfo for SessionInfoImpl {
-    fn session_id_from_block_num(&self, n: BlockNumber) -> SessionId {
-        session_id_from_block_num(n, self.session_period)
-    }
-
-    fn last_block_of_session(&self, session_id: SessionId) -> BlockNumber {
-        last_block_of_session(session_id, self.session_period)
-    }
-
-    fn first_block_of_session(&self, session_id: SessionId) -> BlockNumber {
-        first_block_of_session(session_id, self.session_period)
-    }
-}
-
 impl<B: BlockT, H: ExHashT> SyncState for Arc<NetworkService<B, H>> {
     fn is_major_syncing(&self) -> bool {
         NetworkService::is_major_syncing(self)
diff --git a/finality-aleph/src/party/mocks.rs b/finality-aleph/src/party/mocks.rs
@@ -13,10 +13,9 @@ use crate::{
     party::{
         backup::ABFTBackup,
         manager::AuthorityTask,
-        traits::{ChainState, NodeSessionManager, SessionInfo, SyncState},
+        traits::{ChainState, NodeSessionManager, SyncState},
     },
-    session::{first_block_of_session, last_block_of_session, session_id_from_block_num},
-    AuthorityId, NodeIndex, SessionId, SessionPeriod,
+    AuthorityId, NodeIndex, SessionId,
 };
 
 type AMutex<T> = Arc<Mutex<T>>;
@@ -176,29 +175,3 @@ impl NodeSessionManager for Arc<MockNodeSessionManager> {
         None
     }
 }
-
-pub struct MockSessionInfo {
-    pub session_period: SessionPeriod,
-}
-
-impl MockSessionInfo {
-    pub fn new(session_period: u32) -> Self {
-        Self {
-            session_period: SessionPeriod(session_period),
-        }
-    }
-}
-
-impl SessionInfo for MockSessionInfo {
-    fn session_id_from_block_num(&self, n: BlockNumber) -> SessionId {
-        session_id_from_block_num(n, self.session_period)
-    }
-
-    fn last_block_of_session(&self, session_id: SessionId) -> BlockNumber {
-        last_block_of_session(session_id, self.session_period)
-    }
-
-    fn first_block_of_session(&self, session_id: SessionId) -> BlockNumber {
-        first_block_of_session(session_id, self.session_period)
-    }
-}
diff --git a/finality-aleph/src/party/mod.rs b/finality-aleph/src/party/mod.rs
@@ -7,8 +7,9 @@ use tokio::{task::spawn_blocking, time::sleep};
 use crate::{
     party::{
         manager::{Handle, SubtaskCommon as AuthoritySubtaskCommon, Task},
-        traits::{ChainState, NodeSessionManager, SessionInfo, SyncState},
+        traits::{ChainState, NodeSessionManager, SyncState},
     },
+    session::SessionBoundaryInfo,
     session_map::ReadOnlySessionMap,
     SessionId,
 };
@@ -21,40 +22,38 @@ pub mod traits;
 #[cfg(test)]
 mod mocks;
 
-pub(crate) struct ConsensusPartyParams<ST, CS, NSM, SI> {
+pub(crate) struct ConsensusPartyParams<ST, CS, NSM> {
     pub session_authorities: ReadOnlySessionMap,
     pub chain_state: CS,
     pub sync_state: ST,
     pub backup_saving_path: Option<PathBuf>,
     pub session_manager: NSM,
-    pub session_info: SI,
+    pub session_info: SessionBoundaryInfo,
 }
 
-pub(crate) struct ConsensusParty<ST, CS, NSM, SI>
+pub(crate) struct ConsensusParty<ST, CS, NSM>
 where
     ST: SyncState,
     CS: ChainState,
     NSM: NodeSessionManager,
-    SI: SessionInfo,
 {
     session_authorities: ReadOnlySessionMap,
     chain_state: CS,
     sync_state: ST,
     backup_saving_path: Option<PathBuf>,
     session_manager: NSM,
-    session_info: SI,
+    session_info: SessionBoundaryInfo,
 }
 
 const SESSION_STATUS_CHECK_PERIOD: Duration = Duration::from_millis(1000);
 
-impl<ST, CS, NSM, SI> ConsensusParty<ST, CS, NSM, SI>
+impl<ST, CS, NSM> ConsensusParty<ST, CS, NSM>
 where
     ST: SyncState,
     CS: ChainState,
     NSM: NodeSessionManager,
-    SI: SessionInfo,
 {
-    pub(crate) fn new(params: ConsensusPartyParams<ST, CS, NSM, SI>) -> Self {
+    pub(crate) fn new(params: ConsensusPartyParams<ST, CS, NSM>) -> Self {
         let ConsensusPartyParams {
             session_authorities,
             sync_state,
@@ -269,19 +268,16 @@ mod tests {
 
     use crate::{
         party::{
-            mocks::{MockChainState, MockNodeSessionManager, MockSessionInfo, MockSyncState},
+            mocks::{MockChainState, MockNodeSessionManager, MockSyncState},
             ConsensusParty, ConsensusPartyParams, SESSION_STATUS_CHECK_PERIOD,
         },
+        session::SessionBoundaryInfo,
         session_map::SharedSessionMap,
         SessionId, SessionPeriod,
     };
 
-    type Party = ConsensusParty<
-        Arc<MockSyncState>,
-        Arc<MockChainState>,
-        Arc<MockNodeSessionManager>,
-        MockSessionInfo,
-    >;
+    type Party =
+        ConsensusParty<Arc<MockSyncState>, Arc<MockChainState>, Arc<MockNodeSessionManager>>;
 
     struct PartyState {
         validator_started: Vec<SessionId>,
@@ -504,12 +500,7 @@ mod tests {
     fn create_mocked_consensus_party(
         session_period: SessionPeriod,
     ) -> (
-        ConsensusParty<
-            Arc<MockSyncState>,
-            Arc<MockChainState>,
-            Arc<MockNodeSessionManager>,
-            MockSessionInfo,
-        >,
+        ConsensusParty<Arc<MockSyncState>, Arc<MockChainState>, Arc<MockNodeSessionManager>>,
         MockController,
     ) {
         let shared_map = SharedSessionMap::new();
@@ -518,7 +509,7 @@ mod tests {
         let chain_state = Arc::new(MockChainState::new());
         let sync_state = Arc::new(MockSyncState::new());
         let session_manager = Arc::new(MockNodeSessionManager::new());
-        let session_info = MockSessionInfo::new(session_period.0);
+        let session_info = SessionBoundaryInfo::new(session_period);
 
         let controller = MockController {
             shared_session_map: shared_map,
diff --git a/finality-aleph/src/party/traits.rs b/finality-aleph/src/party/traits.rs
@@ -60,13 +60,3 @@ pub trait SyncState {
     /// [1]: finality_aleph::network::RequestBlocks::is_major_syncing
     fn is_major_syncing(&self) -> bool;
 }
-
-/// Abstraction of the session boundaries.
-pub trait SessionInfo {
-    /// Returns session id of the session that block belongs to.
-    fn session_id_from_block_num(&self, n: BlockNumber) -> SessionId;
-    /// Returns block number which is the last block of the session.
-    fn last_block_of_session(&self, session_id: SessionId) -> BlockNumber;
-    /// Returns block number which is the first block of the session.
-    fn first_block_of_session(&self, session_id: SessionId) -> BlockNumber;
-}
diff --git a/finality-aleph/src/session.rs b/finality-aleph/src/session.rs
@@ -1,8 +1,6 @@
+use aleph_primitives::BlockNumber;
 use codec::{Decode, Encode};
-use sp_runtime::{
-    traits::{AtLeast32BitUnsigned, Block},
-    SaturatedConversion,
-};
+use sp_runtime::traits::Block;
 
 use crate::NumberFor;
 
@@ -15,8 +13,8 @@ pub struct SessionBoundaries<B: Block> {
 impl<B: Block> SessionBoundaries<B> {
     pub fn new(session_id: SessionId, period: SessionPeriod) -> Self {
         SessionBoundaries {
-            first_block: first_block_of_session(session_id, period),
-            last_block: last_block_of_session(session_id, period),
+            first_block: first_block_of_session(session_id, period).into(),
+            last_block: last_block_of_session(session_id, period).into(),
         }
     }
 
@@ -29,25 +27,42 @@ impl<B: Block> SessionBoundaries<B> {
     }
 }
 
-pub fn first_block_of_session<N: AtLeast32BitUnsigned>(
-    session_id: SessionId,
-    period: SessionPeriod,
-) -> N {
-    (session_id.0 * period.0).into()
+fn first_block_of_session(session_id: SessionId, period: SessionPeriod) -> BlockNumber {
+    session_id.0 * period.0
 }
 
-pub fn last_block_of_session<N: AtLeast32BitUnsigned>(
-    session_id: SessionId,
-    period: SessionPeriod,
-) -> N {
-    ((session_id.0 + 1) * period.0 - 1).into()
+fn last_block_of_session(session_id: SessionId, period: SessionPeriod) -> BlockNumber {
+    (session_id.0 + 1) * period.0 - 1
 }
 
-pub fn session_id_from_block_num<N: AtLeast32BitUnsigned>(
-    num: N,
-    period: SessionPeriod,
-) -> SessionId {
-    SessionId((num / period.0.into()).saturated_into())
+fn session_id_from_block_num(num: BlockNumber, period: SessionPeriod) -> SessionId {
+    SessionId(num / period.0)
+}
+
+pub struct SessionBoundaryInfo {
+    session_period: SessionPeriod,
+}
+
+/// Struct for getting the session boundaries.
+impl SessionBoundaryInfo {
+    pub fn new(session_period: SessionPeriod) -> Self {
+        Self { session_period }
+    }
+
+    /// Returns session id of the session that block belongs to.
+    pub fn session_id_from_block_num(&self, n: BlockNumber) -> SessionId {
+        session_id_from_block_num(n, self.session_period)
+    }
+
+    /// Returns block number which is the last block of the session.
+    pub fn last_block_of_session(&self, session_id: SessionId) -> BlockNumber {
+        last_block_of_session(session_id, self.session_period)
+    }
+
+    /// Returns block number which is the first block of the session.
+    pub fn first_block_of_session(&self, session_id: SessionId) -> BlockNumber {
+        first_block_of_session(session_id, self.session_period)
+    }
 }
 
 #[cfg(test)]
diff --git a/finality-aleph/src/session_map.rs b/finality-aleph/src/session_map.rs
diff --git a/finality-aleph/src/sync/handler.rs b/finality-aleph/src/sync/handler.rs
diff --git a/finality-aleph/src/sync/substrate/verification/cache.rs b/finality-aleph/src/sync/substrate/verification/cache.rs
diff --git a/finality-aleph/src/testing/mocks/session_info.rs b/finality-aleph/src/testing/mocks/session_info.rs
