From 380ed5539a282ef0f6f20918fd2ff938f753df96 Mon Sep 17 00:00:00 2001 From: david Date: Fri, 14 Jan 2022 21:00:58 +0100 Subject: [PATCH 1/7] create session boundary api --- frame/session/src/lib.rs | 22 ++++++++++++++++++++-- primitives/session/src/lib.rs | 4 ++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 7a0783718705a..6487631dc4c7c 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -145,6 +145,11 @@ pub trait ShouldEndSession { /// Return `true` if the session should be ended. fn should_end_session(now: BlockNumber) -> bool; } +/// Returns the session boundary +pub trait SessionBoundary { + /// Returns the block at which the last session ended + fn get_session_boundary() -> BlockNumber; +} /// Ends the session after a fixed period of blocks. /// @@ -493,6 +498,7 @@ pub mod pallet { Validators::::put(initial_validators_0); >::put(queued_keys); + SessionStart::::put(T::BlockNumber::default()); T::SessionManager::start_session(0); } @@ -508,6 +514,11 @@ pub mod pallet { #[pallet::getter(fn current_index)] pub type CurrentIndex = StorageValue<_, SessionIndex, ValueQuery>; + /// Block number at which the current session began + #[pallet::storage] + #[pallet::getter(fn session_start)] + pub type SessionStart = StorageValue<_, T::BlockNumber, ValueQuery>; + /// True if the underlying economic identities or weighting behind the validators /// has changed in the queued validator set. #[pallet::storage] @@ -571,7 +582,7 @@ pub mod pallet { /// block of the current session. fn on_initialize(n: T::BlockNumber) -> Weight { if T::ShouldEndSession::should_end_session(n) { - Self::rotate_session(); + Self::rotate_session(n); T::BlockWeights::get().max_block } else { // NOTE: the non-database part of the weight for `should_end_session(n)` is @@ -636,7 +647,7 @@ impl Pallet { /// Move on to next session. Register new validator set and session keys. Changes to the /// validator set have a session of delay to take effect. This allows for equivocation /// punishment after a fork. - pub fn rotate_session() { + pub fn rotate_session(n: T::BlockNumber) { let session_index = >::get(); log::trace!(target: "runtime::session", "rotating session {:?}", session_index); @@ -710,6 +721,7 @@ impl Pallet { >::put(queued_amalgamated.clone()); >::put(next_changed); + SessionStart::::put(n); // Record that this happened. Self::deposit_event(Event::NewSession { session_index }); @@ -955,3 +967,9 @@ impl> FindAuthor validators.get(i as usize).map(|k| k.clone()) } } + +impl SessionBoundary for Pallet { + fn get_session_boundary() -> T::BlockNumber { + Pallet::::session_start() + } +} diff --git a/primitives/session/src/lib.rs b/primitives/session/src/lib.rs index 1b25d285e3bca..2af8d3d65f33b 100644 --- a/primitives/session/src/lib.rs +++ b/primitives/session/src/lib.rs @@ -47,6 +47,10 @@ sp_api::decl_runtime_apis! { /// Returns the list of public raw public keys + key type. fn decode_session_keys(encoded: Vec) -> Option, KeyTypeId)>>; } + + pub trait SessionBoundaryApi { + fn get_session_boundary() -> BlockNumber; + } } /// Number of validators in a given session. From 8908f11b88374c169eeff545223313a088d1351e Mon Sep 17 00:00:00 2001 From: david Date: Fri, 14 Jan 2022 21:25:10 +0100 Subject: [PATCH 2/7] update docs --- frame/session/src/lib.rs | 8 ++++---- primitives/session/src/lib.rs | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 6487631dc4c7c..e609533c6d4a4 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -147,7 +147,7 @@ pub trait ShouldEndSession { } /// Returns the session boundary pub trait SessionBoundary { - /// Returns the block at which the last session ended + /// Returns the block number at which the session began fn get_session_boundary() -> BlockNumber; } @@ -582,7 +582,8 @@ pub mod pallet { /// block of the current session. fn on_initialize(n: T::BlockNumber) -> Weight { if T::ShouldEndSession::should_end_session(n) { - Self::rotate_session(n); + Self::rotate_session(); + SessionStart::::put(n); T::BlockWeights::get().max_block } else { // NOTE: the non-database part of the weight for `should_end_session(n)` is @@ -647,7 +648,7 @@ impl Pallet { /// Move on to next session. Register new validator set and session keys. Changes to the /// validator set have a session of delay to take effect. This allows for equivocation /// punishment after a fork. - pub fn rotate_session(n: T::BlockNumber) { + pub fn rotate_session() { let session_index = >::get(); log::trace!(target: "runtime::session", "rotating session {:?}", session_index); @@ -721,7 +722,6 @@ impl Pallet { >::put(queued_amalgamated.clone()); >::put(next_changed); - SessionStart::::put(n); // Record that this happened. Self::deposit_event(Event::NewSession { session_index }); diff --git a/primitives/session/src/lib.rs b/primitives/session/src/lib.rs index 2af8d3d65f33b..c28b63f566ef1 100644 --- a/primitives/session/src/lib.rs +++ b/primitives/session/src/lib.rs @@ -48,7 +48,9 @@ sp_api::decl_runtime_apis! { fn decode_session_keys(encoded: Vec) -> Option, KeyTypeId)>>; } + /// Sesson boundary runtime api pub trait SessionBoundaryApi { + /// Returns the block number at which the current session began fn get_session_boundary() -> BlockNumber; } } From e53fa1df602ceae4310ac82a5ea608950695513b Mon Sep 17 00:00:00 2001 From: david Date: Fri, 14 Jan 2022 23:10:48 +0100 Subject: [PATCH 3/7] update best beefy block to session boundary at start of node --- Cargo.lock | 1 + client/beefy/Cargo.toml | 1 + client/beefy/src/lib.rs | 6 +++--- client/beefy/src/worker.rs | 15 +++++++++------ 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b65795a373b35..337a461c0f757 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -516,6 +516,7 @@ dependencies = [ "sp-core", "sp-keystore", "sp-runtime", + "sp-session", "sp-tracing", "strum", "substrate-prometheus-endpoint", diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index d57d053c16f42..aba7390e444c0 100644 --- a/client/beefy/Cargo.toml +++ b/client/beefy/Cargo.toml @@ -25,6 +25,7 @@ sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" } sp-core = { version = "4.1.0-dev", path = "../../primitives/core" } sp-keystore = { version = "0.10.0", path = "../../primitives/keystore" } sp-runtime = { version = "4.0.0", path = "../../primitives/runtime" } +sp-session = { version = "4.0.0-dev", path = "../../primitives/session" } sc-chain-spec = { version = "4.0.0-dev", path = "../../client/chain-spec" } sc-utils = { version = "4.0.0-dev", path = "../utils" } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 9b2bf383df8ef..f5c07285ca9fe 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -27,7 +27,7 @@ use sc_network_gossip::{GossipEngine, Network as GossipNetwork}; use sp_api::ProvideRuntimeApi; use sp_blockchain::HeaderBackend; use sp_keystore::SyncCryptoStorePtr; -use sp_runtime::traits::Block; +use sp_runtime::traits::{Block, NumberFor}; use beefy_primitives::BeefyApi; @@ -111,7 +111,7 @@ where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi, + C::Api: BeefyApi + sp_session::SessionBoundaryApi>, N: GossipNetwork + Clone + Send + 'static, { /// BEEFY client @@ -142,7 +142,7 @@ where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi, + C::Api: BeefyApi + sp_session::SessionBoundaryApi>, N: GossipNetwork + Clone + Send + 'static, { let BeefyParams { diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 0c7d8d4ffdc9c..87fd37cabc9a5 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -100,7 +100,7 @@ where B: Block + Codec, BE: Backend, C: Client, - C::Api: BeefyApi, + C::Api: BeefyApi + sp_session::SessionBoundaryApi>, { /// Return a new BEEFY worker instance. /// @@ -146,7 +146,7 @@ where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi, + C::Api: BeefyApi + sp_session::SessionBoundaryApi>, { /// Return `true`, if we should vote on block `number` fn should_vote_on(&self, number: NumberFor) -> bool { @@ -247,14 +247,17 @@ where debug!(target: "beefy", "🥩 New Rounds for id: {:?}", id); - self.best_beefy_block = Some(*notification.header.number()); + let at = BlockId::hash(notification.header.hash()); + let session_boundary = self.client.runtime_api().get_session_boundary(&at).ok(); + self.best_beefy_block = Some(session_boundary); self.beefy_best_block_sender .notify(|| Ok::<_, ()>(notification.hash.clone())) .expect("forwards closure result; the closure always returns Ok; qed."); - // this metric is kind of 'fake'. Best BEEFY block should only be updated once we - // have a signed commitment for the block. Remove once the above TODO is done. - metric_set!(self, beefy_best_block, *notification.header.number()); + // this metric is kind of 'fake'. Best BEEFY block should only be updated once + // we have a signed commitment for the block. Remove once the above TODO is + // done. + metric_set!(self, beefy_best_block, session_boundary); } } From e103fe120cdae9698f879dc12293f58b28c7af37 Mon Sep 17 00:00:00 2001 From: david Date: Mon, 17 Jan 2022 09:16:48 +0100 Subject: [PATCH 4/7] minor fix --- client/beefy/src/worker.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 87fd37cabc9a5..cdab5d1499de4 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -249,7 +249,7 @@ where let at = BlockId::hash(notification.header.hash()); let session_boundary = self.client.runtime_api().get_session_boundary(&at).ok(); - self.best_beefy_block = Some(session_boundary); + self.best_beefy_block = session_boundary.clone(); self.beefy_best_block_sender .notify(|| Ok::<_, ()>(notification.hash.clone())) .expect("forwards closure result; the closure always returns Ok; qed."); @@ -257,7 +257,9 @@ where // this metric is kind of 'fake'. Best BEEFY block should only be updated once // we have a signed commitment for the block. Remove once the above TODO is // done. - metric_set!(self, beefy_best_block, session_boundary); + if session_boundary.is_some() { + metric_set!(self, beefy_best_block, session_boundary.unwrap()); + } } } From 87de5e8c30339f93d593a83cf07722eb9551af02 Mon Sep 17 00:00:00 2001 From: david Date: Mon, 17 Jan 2022 15:01:38 +0100 Subject: [PATCH 5/7] add session boundary to beefy api --- client/beefy/src/lib.rs | 4 ++-- client/beefy/src/worker.rs | 8 ++++---- frame/session/src/lib.rs | 16 +++++----------- primitives/beefy/src/lib.rs | 4 +++- primitives/session/src/lib.rs | 6 ------ 5 files changed, 14 insertions(+), 24 deletions(-) diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index f5c07285ca9fe..4dceccafa290a 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -111,7 +111,7 @@ where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi + sp_session::SessionBoundaryApi>, + C::Api: BeefyApi>, N: GossipNetwork + Clone + Send + 'static, { /// BEEFY client @@ -142,7 +142,7 @@ where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi + sp_session::SessionBoundaryApi>, + C::Api: BeefyApi>, N: GossipNetwork + Clone + Send + 'static, { let BeefyParams { diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index cdab5d1499de4..1e38ae0fcff50 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -100,7 +100,7 @@ where B: Block + Codec, BE: Backend, C: Client, - C::Api: BeefyApi + sp_session::SessionBoundaryApi>, + C::Api: BeefyApi>, { /// Return a new BEEFY worker instance. /// @@ -146,7 +146,7 @@ where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi + sp_session::SessionBoundaryApi>, + C::Api: BeefyApi>, { /// Return `true`, if we should vote on block `number` fn should_vote_on(&self, number: NumberFor) -> bool { @@ -257,8 +257,8 @@ where // this metric is kind of 'fake'. Best BEEFY block should only be updated once // we have a signed commitment for the block. Remove once the above TODO is // done. - if session_boundary.is_some() { - metric_set!(self, beefy_best_block, session_boundary.unwrap()); + if let Some(session_boundary) = session_boundary { + metric_set!(self, beefy_best_block, session_boundary); } } } diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index e609533c6d4a4..6dce457255266 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -145,11 +145,6 @@ pub trait ShouldEndSession { /// Return `true` if the session should be ended. fn should_end_session(now: BlockNumber) -> bool; } -/// Returns the session boundary -pub trait SessionBoundary { - /// Returns the block number at which the session began - fn get_session_boundary() -> BlockNumber; -} /// Ends the session after a fixed period of blocks. /// @@ -910,6 +905,11 @@ impl Pallet { fn clear_key_owner(id: KeyTypeId, key_data: &[u8]) { >::remove((id, key_data)); } + + /// Returns the block number at which the session began + fn get_session_boundary() -> T::BlockNumber { + Pallet::::session_start() + } } impl ValidatorRegistration for Pallet { @@ -967,9 +967,3 @@ impl> FindAuthor validators.get(i as usize).map(|k| k.clone()) } } - -impl SessionBoundary for Pallet { - fn get_session_boundary() -> T::BlockNumber { - Pallet::::session_start() - } -} diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 8dbdd66f3559b..7ce07b75997e8 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -156,10 +156,12 @@ pub struct VoteMessage { sp_api::decl_runtime_apis! { /// API necessary for BEEFY voters. - pub trait BeefyApi + pub trait BeefyApi { /// Return the current active BEEFY validator set fn validator_set() -> Option>; + /// Returns the block number at which the current session began + fn get_session_boundary() -> BlockNumber; } } diff --git a/primitives/session/src/lib.rs b/primitives/session/src/lib.rs index c28b63f566ef1..1b25d285e3bca 100644 --- a/primitives/session/src/lib.rs +++ b/primitives/session/src/lib.rs @@ -47,12 +47,6 @@ sp_api::decl_runtime_apis! { /// Returns the list of public raw public keys + key type. fn decode_session_keys(encoded: Vec) -> Option, KeyTypeId)>>; } - - /// Sesson boundary runtime api - pub trait SessionBoundaryApi { - /// Returns the block number at which the current session began - fn get_session_boundary() -> BlockNumber; - } } /// Number of validators in a given session. From 7d4b5ff3dd27bf812c8654a4599bdcca008cd926 Mon Sep 17 00:00:00 2001 From: david Date: Mon, 17 Jan 2022 15:04:27 +0100 Subject: [PATCH 6/7] minor fix --- Cargo.lock | 1 - client/beefy/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 337a461c0f757..b65795a373b35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -516,7 +516,6 @@ dependencies = [ "sp-core", "sp-keystore", "sp-runtime", - "sp-session", "sp-tracing", "strum", "substrate-prometheus-endpoint", diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index aba7390e444c0..d57d053c16f42 100644 --- a/client/beefy/Cargo.toml +++ b/client/beefy/Cargo.toml @@ -25,7 +25,6 @@ sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" } sp-core = { version = "4.1.0-dev", path = "../../primitives/core" } sp-keystore = { version = "0.10.0", path = "../../primitives/keystore" } sp-runtime = { version = "4.0.0", path = "../../primitives/runtime" } -sp-session = { version = "4.0.0-dev", path = "../../primitives/session" } sc-chain-spec = { version = "4.0.0-dev", path = "../../client/chain-spec" } sc-utils = { version = "4.0.0-dev", path = "../utils" } From bfcac9088028d1660030d955b77a6b4c9ca5ada2 Mon Sep 17 00:00:00 2001 From: david Date: Mon, 17 Jan 2022 16:22:49 +0100 Subject: [PATCH 7/7] minor fix --- frame/session/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 6dce457255266..a3d144ab012eb 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -907,7 +907,7 @@ impl Pallet { } /// Returns the block number at which the session began - fn get_session_boundary() -> T::BlockNumber { + pub fn get_session_boundary() -> T::BlockNumber { Pallet::::session_start() } }