From b7d4a2abc8955f883a56fe6349814c4149629bda Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 2 Jun 2020 15:45:34 +0200 Subject: [PATCH 01/33] draft --- frame/session/src/historical.rs | 2 + frame/session/src/historical/storage.rs | 116 ++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 frame/session/src/historical/storage.rs diff --git a/frame/session/src/historical.rs b/frame/session/src/historical.rs index a1c286eb39245..e6cfb6632989a 100644 --- a/frame/session/src/historical.rs +++ b/frame/session/src/historical.rs @@ -37,6 +37,8 @@ use sp_trie::{MemoryDB, Trie, TrieMut, Recorder, EMPTY_PREFIX}; use sp_trie::trie_types::{TrieDBMut, TrieDB}; use super::{SessionIndex, Module as SessionModule}; +pub use storage; + /// Trait necessary for the historical module. pub trait Trait: super::Trait { /// Full identification of the validator. diff --git a/frame/session/src/historical/storage.rs b/frame/session/src/historical/storage.rs new file mode 100644 index 0000000000000..7b637eb5ec8de --- /dev/null +++ b/frame/session/src/historical/storage.rs @@ -0,0 +1,116 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Validator Set Extracting an iterator from an offchain worker stored list containing historical validatorsets. +//! +//! This is used in conjunction with [`ProvingTrie`](super::ProvingTrie) and +//! the offchain indexing API. + +use sp_std::prelude::*; + + +use super::Trait; +use super::super::{SessionIndex, Module as SessionModule}; + +const PREFIX: &[u8] = b"historical"; +const LAST_PRUNE: &[u8] = b"historical_last_prune"; + +pub struct ValidatorSet { + validator_set: Vec<(T::ValidatorId, T::FullIdentification)> +} + +/// Derive the key used to store the list of validators +fn derive_key, T: Trait>(prefix: P, session_index: Vec) -> Vec { + assert!(session_index.len() > 0); + let prefix = prefix.as_ref(); + let mut concatenated = Vec::with_capacity(prefix.len() + 1 + session_index.len()); + concatenated.extend_from_slice(prefix); + concatenated.push('/'); + (&mut concatenated[(prefix.len()+1)..]).extend_from_slice(session_index.as_slice()); + concatenated +} + + +impl ValidatorSet { + /// Load the set of validators for a paritcular session index from the offchain storage. + /// + /// If none is found or decodable given `prefix` and `session`, it will return `None`. + /// Empty validator sets should only ever exist for genesis blocks. + fn load_from_offchain(session_index: SessionIndex) -> Option { + let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); + let validator_set = StorageValueRef::persistent(derived_key.as_ref()) + .get::>() + .flatten(); + validator_set + } + + fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { + self.validator_set.as_slice() + } + + fn to_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { + self.validator_set + } + + fn prune_item(everything_up_to: SessionIndex) { + StorageValueRef::persistent(derived_key.as_ref()).clear() + } + + fn prune() { + let move_pruning_marker = SessionModule::current_index(); + Self::prune_older_than(move_pruning_marker); + } + + /// Attempt to prune anything that is older than `first_to_keep`. + fn prune_older_than(first_to_keep: SessionIndex) { + let pruning_marker_key = derive_key(STATIC_LAST_PRUNE); + match StorageValueRef::persistent(derived_key.as_ref()) + .mutate(|x| { + Ok(x.encode()) + }) { + Ok(Ok(previous)) => { + for session_index in previous..first_to_keep { + let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); + let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); + } + }, + Ok(Err(e)) => {}, // failed to store the value calculated by the closure + Err(e) => {}, // failed to calculate the value to store with the given closure + } + } + + + /// Must be called from on chain. + fn store_current>(prefix: P, session: SessionIndex) { + let session_index = SessionModule::current_index(); + let derived_key = derive_key(prefix.as_ref(), session.encode()); + StorageValueRef::persistent(derived_key.as_ref()).set(); + } + + /// **Must** be called from on chain, i.e. `on_block_imported` + fn store() { + + } + + + +impl IntoIter for ValidatorSet { + type Item=(T::ValidatorId, T::FullIdentification); + fn into_iter(self) -> impl Iterator { + self.validator_set.into_iter() + } +} \ No newline at end of file From d2bb3b8d482eb1b207ef4e5a0bf06b459bede8b3 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 2 Jun 2020 17:16:47 +0200 Subject: [PATCH 02/33] steps --- frame/session/src/historical/storage.rs | 71 +++++++++++++++++-------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/frame/session/src/historical/storage.rs b/frame/session/src/historical/storage.rs index 7b637eb5ec8de..85bcd5dd28e6a 100644 --- a/frame/session/src/historical/storage.rs +++ b/frame/session/src/historical/storage.rs @@ -21,13 +21,14 @@ //! the offchain indexing API. use sp_std::prelude::*; - +use sp_io::offchain_index; use super::Trait; use super::super::{SessionIndex, Module as SessionModule}; const PREFIX: &[u8] = b"historical"; const LAST_PRUNE: &[u8] = b"historical_last_prune"; +const HEAD: &[u8] = b"historical_head"; pub struct ValidatorSet { validator_set: Vec<(T::ValidatorId, T::FullIdentification)> @@ -58,59 +59,83 @@ impl ValidatorSet { validator_set } + /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { self.validator_set.as_slice() } + /// Convert `self` to a vector and consume `self`. fn to_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { self.validator_set } - fn prune_item(everything_up_to: SessionIndex) { - StorageValueRef::persistent(derived_key.as_ref()).clear() - } - + /// Prune anything older than the current session index. + /// + /// For behaviour details refer to [`fn prune_older_than`](Self::prune_older_than). + /// + /// **Must** be called from the offchain worker. fn prune() { let move_pruning_marker = SessionModule::current_index(); Self::prune_older_than(move_pruning_marker); } - /// Attempt to prune anything that is older than `first_to_keep`. + /// Attempt to prune anything that is older than `first_to_keep` session index. + /// + /// Due to re-ogranisation it could be that the `first_to_keep` might be less + /// than the stored one, in which case the conservative choice is made to keep records + /// up to the one that is the lesser. + /// + /// **Must** be called from the offchain worker. fn prune_older_than(first_to_keep: SessionIndex) { let pruning_marker_key = derive_key(STATIC_LAST_PRUNE); - match StorageValueRef::persistent(derived_key.as_ref()) - .mutate(|x| { - Ok(x.encode()) + let mut entry = StorageValueRef::persistent(derived_key.as_ref()); + match entry.mutate(|current: Option>| { + match current { + Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), + // do not move the cursor, if the new one would be behind ours + Some(Some(current)) => Ok(current), + None => Ok(first_to_keep), + // if the storage contains undecodable data, overwrite with current anyways + // which might leak some entries being never purged + Some(None) => Ok(first_to_keep), + } }) { - Ok(Ok(previous)) => { - for session_index in previous..first_to_keep { - let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); - let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); + Ok(Ok(new_value)) => { + // on a re-org this is not necessarily true, with the above they might be equal + if previous < first_to_keep { + for session_index in previous..first_to_keep { + let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); + let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); + } } }, - Ok(Err(e)) => {}, // failed to store the value calculated by the closure + Ok(Err(e)) => {}, // failed to store the value calculated with the given closure Err(e) => {}, // failed to calculate the value to store with the given closure } } - /// Must be called from on chain. - fn store_current>(prefix: P, session: SessionIndex) { - let session_index = SessionModule::current_index(); + /// **Must** be called from on chain. + fn store_to_offchain(session: SessionIndex) { + let session_index = >::current_index(); let derived_key = derive_key(prefix.as_ref(), session.encode()); - StorageValueRef::persistent(derived_key.as_ref()).set(); + //let value = SessionModule::historical_root(session_index); + let value = >::validators().encode(); + offchain_index::set(value.as_slice()) } - /// **Must** be called from on chain, i.e. `on_block_imported` - fn store() { - + /// **Must** be called from on chain, i.e. `on_import` + fn store_current_to_offchain() { + Self::store_to_offchain(SessionModule::current_index()); } +} - +/// Implement conversion into iterator for usage +/// with [ProvingTrie](super::ProvingTrie::generate_for). impl IntoIter for ValidatorSet { type Item=(T::ValidatorId, T::FullIdentification); fn into_iter(self) -> impl Iterator { self.validator_set.into_iter() } -} \ No newline at end of file +} From 7a4426ce2e2a4c919a6ac7ad6ef9d8d59651afc6 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 2 Jun 2020 18:02:09 +0200 Subject: [PATCH 03/33] chore: fmt --- frame/session/src/historical.rs | 2 +- frame/session/src/historical/storage.rs | 193 ++++++++++++------------ 2 files changed, 96 insertions(+), 99 deletions(-) diff --git a/frame/session/src/historical.rs b/frame/session/src/historical.rs index e6cfb6632989a..e85efc92237e8 100644 --- a/frame/session/src/historical.rs +++ b/frame/session/src/historical.rs @@ -37,7 +37,7 @@ use sp_trie::{MemoryDB, Trie, TrieMut, Recorder, EMPTY_PREFIX}; use sp_trie::trie_types::{TrieDBMut, TrieDB}; use super::{SessionIndex, Module as SessionModule}; -pub use storage; +pub mod storage; /// Trait necessary for the historical module. pub trait Trait: super::Trait { diff --git a/frame/session/src/historical/storage.rs b/frame/session/src/historical/storage.rs index 85bcd5dd28e6a..fae632f19e76d 100644 --- a/frame/session/src/historical/storage.rs +++ b/frame/session/src/historical/storage.rs @@ -20,122 +20,119 @@ //! This is used in conjunction with [`ProvingTrie`](super::ProvingTrie) and //! the offchain indexing API. -use sp_std::prelude::*; use sp_io::offchain_index; +use sp_std::prelude::*; +use super::super::{Module as SessionModule, SessionIndex}; use super::Trait; -use super::super::{SessionIndex, Module as SessionModule}; const PREFIX: &[u8] = b"historical"; const LAST_PRUNE: &[u8] = b"historical_last_prune"; const HEAD: &[u8] = b"historical_head"; pub struct ValidatorSet { - validator_set: Vec<(T::ValidatorId, T::FullIdentification)> + validator_set: Vec<(T::ValidatorId, T::FullIdentification)>, } /// Derive the key used to store the list of validators -fn derive_key, T: Trait>(prefix: P, session_index: Vec) -> Vec { - assert!(session_index.len() > 0); - let prefix = prefix.as_ref(); - let mut concatenated = Vec::with_capacity(prefix.len() + 1 + session_index.len()); - concatenated.extend_from_slice(prefix); - concatenated.push('/'); - (&mut concatenated[(prefix.len()+1)..]).extend_from_slice(session_index.as_slice()); - concatenated +fn derive_key, T: Trait>(prefix: P, session_index: Vec) -> Vec { + assert!(session_index.len() > 0); + let prefix = prefix.as_ref(); + let mut concatenated = Vec::with_capacity(prefix.len() + 1 + session_index.len()); + concatenated.extend_from_slice(prefix); + concatenated.push('/'); + (&mut concatenated[(prefix.len() + 1)..]).extend_from_slice(session_index.as_slice()); + concatenated } - impl ValidatorSet { - /// Load the set of validators for a paritcular session index from the offchain storage. - /// - /// If none is found or decodable given `prefix` and `session`, it will return `None`. - /// Empty validator sets should only ever exist for genesis blocks. - fn load_from_offchain(session_index: SessionIndex) -> Option { - let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); - let validator_set = StorageValueRef::persistent(derived_key.as_ref()) - .get::>() - .flatten(); - validator_set - } - - /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. - fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { - self.validator_set.as_slice() - } - - /// Convert `self` to a vector and consume `self`. - fn to_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { - self.validator_set - } - - /// Prune anything older than the current session index. - /// - /// For behaviour details refer to [`fn prune_older_than`](Self::prune_older_than). - /// - /// **Must** be called from the offchain worker. - fn prune() { - let move_pruning_marker = SessionModule::current_index(); - Self::prune_older_than(move_pruning_marker); - } - - /// Attempt to prune anything that is older than `first_to_keep` session index. - /// - /// Due to re-ogranisation it could be that the `first_to_keep` might be less - /// than the stored one, in which case the conservative choice is made to keep records - /// up to the one that is the lesser. - /// - /// **Must** be called from the offchain worker. - fn prune_older_than(first_to_keep: SessionIndex) { - let pruning_marker_key = derive_key(STATIC_LAST_PRUNE); - let mut entry = StorageValueRef::persistent(derived_key.as_ref()); - match entry.mutate(|current: Option>| { - match current { - Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), - // do not move the cursor, if the new one would be behind ours - Some(Some(current)) => Ok(current), - None => Ok(first_to_keep), - // if the storage contains undecodable data, overwrite with current anyways - // which might leak some entries being never purged - Some(None) => Ok(first_to_keep), - } - }) { - Ok(Ok(new_value)) => { - // on a re-org this is not necessarily true, with the above they might be equal - if previous < first_to_keep { - for session_index in previous..first_to_keep { - let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); - let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); - } - } - }, - Ok(Err(e)) => {}, // failed to store the value calculated with the given closure - Err(e) => {}, // failed to calculate the value to store with the given closure - } - } - - - /// **Must** be called from on chain. - fn store_to_offchain(session: SessionIndex) { - let session_index = >::current_index(); - let derived_key = derive_key(prefix.as_ref(), session.encode()); - //let value = SessionModule::historical_root(session_index); - let value = >::validators().encode(); - offchain_index::set(value.as_slice()) - } - - /// **Must** be called from on chain, i.e. `on_import` - fn store_current_to_offchain() { - Self::store_to_offchain(SessionModule::current_index()); - } + /// Load the set of validators for a paritcular session index from the offchain storage. + /// + /// If none is found or decodable given `prefix` and `session`, it will return `None`. + /// Empty validator sets should only ever exist for genesis blocks. + fn load_from_offchain(session_index: SessionIndex) -> Option { + let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); + let validator_set = StorageValueRef::persistent(derived_key.as_ref()) + .get::>() + .flatten(); + validator_set + } + + /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. + fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { + self.validator_set.as_slice() + } + + /// Convert `self` to a vector and consume `self`. + fn to_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { + self.validator_set + } + + /// Prune anything older than the current session index. + /// + /// For behaviour details refer to [`fn prune_older_than`](Self::prune_older_than). + /// + /// **Must** be called from the offchain worker. + fn prune() { + let move_pruning_marker = SessionModule::current_index(); + Self::prune_older_than(move_pruning_marker); + } + + /// Attempt to prune anything that is older than `first_to_keep` session index. + /// + /// Due to re-ogranisation it could be that the `first_to_keep` might be less + /// than the stored one, in which case the conservative choice is made to keep records + /// up to the one that is the lesser. + /// + /// **Must** be called from the offchain worker. + fn prune_older_than(first_to_keep: SessionIndex) { + let pruning_marker_key = derive_key(STATIC_LAST_PRUNE); + let mut entry = StorageValueRef::persistent(derived_key.as_ref()); + match entry.mutate(|current: Option>| { + match current { + Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), + // do not move the cursor, if the new one would be behind ours + Some(Some(current)) => Ok(current), + None => Ok(first_to_keep), + // if the storage contains undecodable data, overwrite with current anyways + // which might leak some entries being never purged + Some(None) => Ok(first_to_keep), + } + }) { + Ok(Ok(new_value)) => { + // on a re-org this is not necessarily true, with the above they might be equal + if previous < first_to_keep { + for session_index in previous..first_to_keep { + let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); + let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); + } + } + } + Ok(Err(e)) => {} // failed to store the value calculated with the given closure + Err(e) => {} // failed to calculate the value to store with the given closure + } + } + + /// **Must** be called from on chain. + fn store_to_offchain(session: SessionIndex) { + let session_index = >::current_index(); + let derived_key = derive_key(prefix.as_ref(), session.encode()); + //let value = SessionModule::historical_root(session_index); + let value = >::validators().encode(); + offchain_index::set(value.as_slice()) + } + + /// **Must** be called from on chain, i.e. `on_import` + fn store_current_to_offchain() { + Self::store_to_offchain(SessionModule::current_index()); + } } - /// Implement conversion into iterator for usage /// with [ProvingTrie](super::ProvingTrie::generate_for). impl IntoIter for ValidatorSet { - type Item=(T::ValidatorId, T::FullIdentification); - fn into_iter(self) -> impl Iterator { - self.validator_set.into_iter() - } + type Item = (T::ValidatorId, T::FullIdentification); + fn into_iter(self) -> impl Iterator { + self.validator_set.into_iter() + } } From 8eb7a4caf220ba24b5564587f0d9aac0c8089f05 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 3 Jun 2020 15:38:26 +0200 Subject: [PATCH 04/33] step by step --- frame/session/src/historical/storage.rs | 55 +++++++++++-------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/frame/session/src/historical/storage.rs b/frame/session/src/historical/storage.rs index fae632f19e76d..fe8f0cdd2a315 100644 --- a/frame/session/src/historical/storage.rs +++ b/frame/session/src/historical/storage.rs @@ -20,7 +20,8 @@ //! This is used in conjunction with [`ProvingTrie`](super::ProvingTrie) and //! the offchain indexing API. -use sp_io::offchain_index; +use codec::{Codec, Decode, Encode}; +use sp_runtime::offchain::storage::StorageValueRef; use sp_std::prelude::*; use super::super::{Module as SessionModule, SessionIndex}; @@ -35,13 +36,13 @@ pub struct ValidatorSet { } /// Derive the key used to store the list of validators -fn derive_key, T: Trait>(prefix: P, session_index: Vec) -> Vec { +fn derive_key>(prefix: P, session_index: &[u8]) -> Vec { assert!(session_index.len() > 0); - let prefix = prefix.as_ref(); + let prefix: &[u8] = prefix.as_ref(); let mut concatenated = Vec::with_capacity(prefix.len() + 1 + session_index.len()); concatenated.extend_from_slice(prefix); - concatenated.push('/'); - (&mut concatenated[(prefix.len() + 1)..]).extend_from_slice(session_index.as_slice()); + concatenated.push('/' as u8); + concatenated.extend_from_slice(session_index); concatenated } @@ -51,11 +52,11 @@ impl ValidatorSet { /// If none is found or decodable given `prefix` and `session`, it will return `None`. /// Empty validator sets should only ever exist for genesis blocks. fn load_from_offchain(session_index: SessionIndex) -> Option { - let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); + let derived_key = derive_key(PREFIX, session_index.encode().as_slice()); let validator_set = StorageValueRef::persistent(derived_key.as_ref()) .get::>() .flatten(); - validator_set + validator_set.map(|validator_set| Self { validator_set }) } /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. @@ -68,16 +69,6 @@ impl ValidatorSet { self.validator_set } - /// Prune anything older than the current session index. - /// - /// For behaviour details refer to [`fn prune_older_than`](Self::prune_older_than). - /// - /// **Must** be called from the offchain worker. - fn prune() { - let move_pruning_marker = SessionModule::current_index(); - Self::prune_older_than(move_pruning_marker); - } - /// Attempt to prune anything that is older than `first_to_keep` session index. /// /// Due to re-ogranisation it could be that the `first_to_keep` might be less @@ -86,53 +77,55 @@ impl ValidatorSet { /// /// **Must** be called from the offchain worker. fn prune_older_than(first_to_keep: SessionIndex) { - let pruning_marker_key = derive_key(STATIC_LAST_PRUNE); + let derived_key = derive_key(LAST_PRUNE, b"---"); let mut entry = StorageValueRef::persistent(derived_key.as_ref()); - match entry.mutate(|current: Option>| { + match entry.mutate(|current: Option>| -> Result<_, ()> { match current { Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), // do not move the cursor, if the new one would be behind ours Some(Some(current)) => Ok(current), None => Ok(first_to_keep), // if the storage contains undecodable data, overwrite with current anyways - // which might leak some entries being never purged + // which might leak some entries being never purged, but that is acceptable + // in this context Some(None) => Ok(first_to_keep), } }) { Ok(Ok(new_value)) => { // on a re-org this is not necessarily true, with the above they might be equal - if previous < first_to_keep { - for session_index in previous..first_to_keep { - let derived_key = derive_key(STATIC_PREFIX, session_index.encode()); + if new_value < first_to_keep { + for session_index in new_value..first_to_keep { + let derived_key = derive_key(PREFIX, session_index.encode().as_slice()); let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); } } } - Ok(Err(e)) => {} // failed to store the value calculated with the given closure - Err(e) => {} // failed to calculate the value to store with the given closure + Ok(Err(_)) => {} // failed to store the value calculated with the given closure + Err(_) => {} // failed to calculate the value to store with the given closure } } /// **Must** be called from on chain. fn store_to_offchain(session: SessionIndex) { let session_index = >::current_index(); - let derived_key = derive_key(prefix.as_ref(), session.encode()); + let derived_key = derive_key(PREFIX, session.encode().as_slice()); //let value = SessionModule::historical_root(session_index); let value = >::validators().encode(); - offchain_index::set(value.as_slice()) + sp_io::offchain_index::set(derived_key.as_slice(), value.as_slice()) } - /// **Must** be called from on chain, i.e. `on_import` + /// **Must** be called from on chain, i.e. `on_initialize` or `on_finalization`. fn store_current_to_offchain() { - Self::store_to_offchain(SessionModule::current_index()); + Self::store_to_offchain(>::current_index()); } } /// Implement conversion into iterator for usage /// with [ProvingTrie](super::ProvingTrie::generate_for). -impl IntoIter for ValidatorSet { +impl core::iter::IntoIterator for ValidatorSet { type Item = (T::ValidatorId, T::FullIdentification); - fn into_iter(self) -> impl Iterator { + type IntoIter = std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { self.validator_set.into_iter() } } From fe0682b0d25b5e40253f14b8e69e2675965bd118 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 3 Jun 2020 16:29:58 +0200 Subject: [PATCH 05/33] more details --- frame/session/src/historical/storage.rs | 58 ++++++++++++++----------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/frame/session/src/historical/storage.rs b/frame/session/src/historical/storage.rs index fe8f0cdd2a315..af3f7ca879c9c 100644 --- a/frame/session/src/historical/storage.rs +++ b/frame/session/src/historical/storage.rs @@ -15,10 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Validator Set Extracting an iterator from an offchain worker stored list containing historical validatorsets. +//! Validator Set Extracting an iterator from an off-chain worker stored list containing historical validatorsets. //! //! This is used in conjunction with [`ProvingTrie`](super::ProvingTrie) and -//! the offchain indexing API. +//! the off-chain indexing API. use codec::{Codec, Decode, Encode}; use sp_runtime::offchain::storage::StorageValueRef; @@ -29,30 +29,30 @@ use super::Trait; const PREFIX: &[u8] = b"historical"; const LAST_PRUNE: &[u8] = b"historical_last_prune"; -const HEAD: &[u8] = b"historical_head"; pub struct ValidatorSet { validator_set: Vec<(T::ValidatorId, T::FullIdentification)>, } /// Derive the key used to store the list of validators -fn derive_key>(prefix: P, session_index: &[u8]) -> Vec { - assert!(session_index.len() > 0); +fn derive_key>(prefix: P, session_index: SessionIndex) -> Vec { let prefix: &[u8] = prefix.as_ref(); - let mut concatenated = Vec::with_capacity(prefix.len() + 1 + session_index.len()); + let encoded_session_index = session_index.encode(); + assert!(encoded_session_index.len() > 0); + let mut concatenated = Vec::with_capacity(prefix.len() + 1 + encoded_session_index.len()); concatenated.extend_from_slice(prefix); concatenated.push('/' as u8); - concatenated.extend_from_slice(session_index); + concatenated.extend_from_slice(encoded_session_index.as_slice()); concatenated } impl ValidatorSet { - /// Load the set of validators for a paritcular session index from the offchain storage. + /// Load the set of validators for a paritcular session index from the off-chain storage. /// /// If none is found or decodable given `prefix` and `session`, it will return `None`. /// Empty validator sets should only ever exist for genesis blocks. - fn load_from_offchain(session_index: SessionIndex) -> Option { - let derived_key = derive_key(PREFIX, session_index.encode().as_slice()); + pub fn load_from_offchain(session_index: SessionIndex) -> Option { + let derived_key = derive_key(PREFIX, session_index); let validator_set = StorageValueRef::persistent(derived_key.as_ref()) .get::>() .flatten(); @@ -60,12 +60,12 @@ impl ValidatorSet { } /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. - fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { + pub fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { self.validator_set.as_slice() } - /// Convert `self` to a vector and consume `self`. - fn to_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { + /// Convert `self` into a vector and consume `self`. + pub fn into_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { self.validator_set } @@ -75,10 +75,10 @@ impl ValidatorSet { /// than the stored one, in which case the conservative choice is made to keep records /// up to the one that is the lesser. /// - /// **Must** be called from the offchain worker. - fn prune_older_than(first_to_keep: SessionIndex) { - let derived_key = derive_key(LAST_PRUNE, b"---"); - let mut entry = StorageValueRef::persistent(derived_key.as_ref()); + /// **Must** be called from the off-chain worker. + pub fn prune_older_than(first_to_keep: SessionIndex) { + let derived_key = LAST_PRUNE.to_vec(); + let entry = StorageValueRef::persistent(derived_key.as_ref()); match entry.mutate(|current: Option>| -> Result<_, ()> { match current { Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), @@ -95,7 +95,7 @@ impl ValidatorSet { // on a re-org this is not necessarily true, with the above they might be equal if new_value < first_to_keep { for session_index in new_value..first_to_keep { - let derived_key = derive_key(PREFIX, session_index.encode().as_slice()); + let derived_key = derive_key(PREFIX, session_index); let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); } } @@ -105,17 +105,25 @@ impl ValidatorSet { } } - /// **Must** be called from on chain. - fn store_to_offchain(session: SessionIndex) { + pub fn keep_newest(number_of_sessions_to_keep: usize) { let session_index = >::current_index(); - let derived_key = derive_key(PREFIX, session.encode().as_slice()); + let number_of_sessions_to_keep = number_of_sessions_to_keep as SessionIndex; + if number_of_sessions_to_keep < session_index { + Self::prune_older_than(session_index - number_of_sessions_to_keep) + } + // otherwise we want to keep all of them + } + + /// **Must** be called from on-chain, i.e. `on_initialize` or `on_finalization`. + pub fn store_to_offchain(session_index: SessionIndex) { + let derived_key = derive_key(PREFIX, session_index); //let value = SessionModule::historical_root(session_index); - let value = >::validators().encode(); - sp_io::offchain_index::set(derived_key.as_slice(), value.as_slice()) + let encoded_validator_list = >::validators().encode(); + sp_io::offchain_index::set(derived_key.as_slice(), encoded_validator_list.as_slice()) } - /// **Must** be called from on chain, i.e. `on_initialize` or `on_finalization`. - fn store_current_to_offchain() { + /// **Must** be called from on-chain, i.e. `on_initialize` or `on_finalization`. + pub fn store_current_to_offchain() { Self::store_to_offchain(>::current_index()); } } From c24b0a3f147e00f95530124fd12bd61b1e9d0986 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 4 Jun 2020 17:36:51 +0200 Subject: [PATCH 06/33] make test public --- frame/session/src/historical.rs | 4 ++-- frame/session/src/historical/storage.rs | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/frame/session/src/historical.rs b/frame/session/src/historical.rs index e85efc92237e8..a73a3a9c52abd 100644 --- a/frame/session/src/historical.rs +++ b/frame/session/src/historical.rs @@ -313,7 +313,7 @@ impl> frame_support::traits::KeyOwnerProofSystem<(KeyTy } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use sp_core::crypto::key_types::DUMMY; use sp_runtime::testing::UintAuthorityId; @@ -325,7 +325,7 @@ mod tests { type Historical = Module; - fn new_test_ext() -> sp_io::TestExternalities { + pub(crate) fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); crate::GenesisConfig:: { keys: NEXT_VALIDATORS.with(|l| diff --git a/frame/session/src/historical/storage.rs b/frame/session/src/historical/storage.rs index af3f7ca879c9c..5407166a6d606 100644 --- a/frame/session/src/historical/storage.rs +++ b/frame/session/src/historical/storage.rs @@ -137,3 +137,17 @@ impl core::iter::IntoIterator for ValidatorSet { self.validator_set.into_iter() } } + + +#[cfg(test)] +mod tests { + use super::super::tests::new_test_ext; + use super::*; + + #[test] + fn justone() { + new_test_ext().execute_with(|| { + + }) + } +} \ No newline at end of file From 5aec81ec71e544002ffd1aa500b24ceb5ae809c8 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 4 Jun 2020 19:19:34 +0200 Subject: [PATCH 07/33] refactor: split into on and offchain --- frame/session/src/historical.rs | 7 +- .../historical/{storage.rs => offchain.rs} | 80 +++++++++---------- frame/session/src/historical/onchain.rs | 24 ++++++ frame/session/src/historical/shared.rs | 16 ++++ 4 files changed, 83 insertions(+), 44 deletions(-) rename frame/session/src/historical/{storage.rs => offchain.rs} (68%) create mode 100644 frame/session/src/historical/onchain.rs create mode 100644 frame/session/src/historical/shared.rs diff --git a/frame/session/src/historical.rs b/frame/session/src/historical.rs index a73a3a9c52abd..33306dcae2213 100644 --- a/frame/session/src/historical.rs +++ b/frame/session/src/historical.rs @@ -37,7 +37,9 @@ use sp_trie::{MemoryDB, Trie, TrieMut, Recorder, EMPTY_PREFIX}; use sp_trie::trie_types::{TrieDBMut, TrieDB}; use super::{SessionIndex, Module as SessionModule}; -pub mod storage; +mod shared; +pub mod offchain; +pub mod onchain; /// Trait necessary for the historical module. pub trait Trait: super::Trait { @@ -156,7 +158,7 @@ impl crate::SessionManager for NoteHistoricalRoot = (::ValidatorId, ::FullIdentification); -/// a trie instance for checking and generating proofs. +/// A trie instance for checking and generating proofs. pub struct ProvingTrie { db: MemoryDB, root: T::Hash, @@ -252,7 +254,6 @@ impl ProvingTrie { .ok()? .and_then(|raw| >::decode(&mut &*raw).ok()) } - } impl> frame_support::traits::KeyOwnerProofSystem<(KeyTypeId, D)> diff --git a/frame/session/src/historical/storage.rs b/frame/session/src/historical/offchain.rs similarity index 68% rename from frame/session/src/historical/storage.rs rename to frame/session/src/historical/offchain.rs index 5407166a6d606..0d7076fcbad20 100644 --- a/frame/session/src/historical/storage.rs +++ b/frame/session/src/historical/offchain.rs @@ -20,30 +20,16 @@ //! This is used in conjunction with [`ProvingTrie`](super::ProvingTrie) and //! the off-chain indexing API. -use codec::{Codec, Decode, Encode}; -use sp_runtime::offchain::storage::StorageValueRef; -use sp_std::prelude::*; +use sp_runtime::{offchain::storage::StorageValueRef, KeyTypeId}; +use sp_session::MembershipProof; use super::super::{Module as SessionModule, SessionIndex}; -use super::Trait; +use super::{IdentificationTuple, ProvingTrie, Trait}; -const PREFIX: &[u8] = b"historical"; -const LAST_PRUNE: &[u8] = b"historical_last_prune"; +use super::shared::*; pub struct ValidatorSet { - validator_set: Vec<(T::ValidatorId, T::FullIdentification)>, -} - -/// Derive the key used to store the list of validators -fn derive_key>(prefix: P, session_index: SessionIndex) -> Vec { - let prefix: &[u8] = prefix.as_ref(); - let encoded_session_index = session_index.encode(); - assert!(encoded_session_index.len() > 0); - let mut concatenated = Vec::with_capacity(prefix.len() + 1 + encoded_session_index.len()); - concatenated.extend_from_slice(prefix); - concatenated.push('/' as u8); - concatenated.extend_from_slice(encoded_session_index.as_slice()); - concatenated + validator_set: Vec>, } impl ValidatorSet { @@ -51,7 +37,7 @@ impl ValidatorSet { /// /// If none is found or decodable given `prefix` and `session`, it will return `None`. /// Empty validator sets should only ever exist for genesis blocks. - pub fn load_from_offchain(session_index: SessionIndex) -> Option { + pub fn load_from_offchain_db(session_index: SessionIndex) -> Option { let derived_key = derive_key(PREFIX, session_index); let validator_set = StorageValueRef::persistent(derived_key.as_ref()) .get::>() @@ -74,8 +60,6 @@ impl ValidatorSet { /// Due to re-ogranisation it could be that the `first_to_keep` might be less /// than the stored one, in which case the conservative choice is made to keep records /// up to the one that is the lesser. - /// - /// **Must** be called from the off-chain worker. pub fn prune_older_than(first_to_keep: SessionIndex) { let derived_key = LAST_PRUNE.to_vec(); let entry = StorageValueRef::persistent(derived_key.as_ref()); @@ -105,26 +89,18 @@ impl ValidatorSet { } } - pub fn keep_newest(number_of_sessions_to_keep: usize) { + /// Keep the newest `n` items, and prune all items odler than that. + pub fn keep_newest(n_to_keep: usize) { let session_index = >::current_index(); - let number_of_sessions_to_keep = number_of_sessions_to_keep as SessionIndex; - if number_of_sessions_to_keep < session_index { - Self::prune_older_than(session_index - number_of_sessions_to_keep) + let n_to_keep = n_to_keep as SessionIndex; + if n_to_keep < session_index { + Self::prune_older_than(session_index - n_to_keep) } - // otherwise we want to keep all of them - } - - /// **Must** be called from on-chain, i.e. `on_initialize` or `on_finalization`. - pub fn store_to_offchain(session_index: SessionIndex) { - let derived_key = derive_key(PREFIX, session_index); - //let value = SessionModule::historical_root(session_index); - let encoded_validator_list = >::validators().encode(); - sp_io::offchain_index::set(derived_key.as_slice(), encoded_validator_list.as_slice()) } - /// **Must** be called from on-chain, i.e. `on_initialize` or `on_finalization`. - pub fn store_current_to_offchain() { - Self::store_to_offchain(>::current_index()); + #[inline] + fn len(&self) -> usize { + self.validator_set.len() } } @@ -132,12 +108,33 @@ impl ValidatorSet { /// with [ProvingTrie](super::ProvingTrie::generate_for). impl core::iter::IntoIterator for ValidatorSet { type Item = (T::ValidatorId, T::FullIdentification); - type IntoIter = std::vec::IntoIter; + type IntoIter = sp_std::vec::IntoIter; fn into_iter(self) -> Self::IntoIter { self.validator_set.into_iter() } } +/// Create a proof based on the data available in the off-chain database. +/// +/// Based on the yielded `MembershipProof` the implementer may decide what +/// to do, i.e. in case of a failed proof, enqueue a transaction back on +/// chain reflecting that, with all its consequences such as i.e. slashing. +pub fn prove_session_membership>( + session_index: SessionIndex, + session_key: (KeyTypeId, D), +) -> Option { + let validators = ValidatorSet::::load_from_offchain_db(session_index)?; + let count = validators.len() as u32; + let trie = ProvingTrie::::generate_for(validators.into_iter()).ok()?; + + let (id, data) = session_key; + trie.prove(id, data.as_ref()) + .map(|trie_nodes| MembershipProof { + session: session_index, + trie_nodes, + validator_count: count, + }) +} #[cfg(test)] mod tests { @@ -147,7 +144,8 @@ mod tests { #[test] fn justone() { new_test_ext().execute_with(|| { - + // @todo think of a way to test this properly, not sure yet if we can just call + // on-chain and off-chain all at once from here and get meaningful results }) } -} \ No newline at end of file +} diff --git a/frame/session/src/historical/onchain.rs b/frame/session/src/historical/onchain.rs new file mode 100644 index 0000000000000..842e4692d5e95 --- /dev/null +++ b/frame/session/src/historical/onchain.rs @@ -0,0 +1,24 @@ + +use codec::Encode; + +use super::super::{Module as SessionModule, SessionIndex}; +use super::Trait; + +use super::shared; + +/// Store the validator set associated to the `session_index` to the off-chain database. +/// +/// **Must** be called from on-chain, i.e. `on_initialize(..)` or `on_finalization(..)`. +pub fn store_session_validator_set_to_offchain(session_index: SessionIndex) { + let derived_key = shared::derive_key(shared::PREFIX, session_index); + //let value = SessionModule::historical_root(session_index); + let encoded_validator_list = >::validators().encode(); + sp_io::offchain_index::set(derived_key.as_slice(), encoded_validator_list.as_slice()) +} + +/// Store the validator set associated to the _current_ session index to the off-chain database. +/// +/// **Must** be called from on-chain, i.e. `on_initialize(..)` or `on_finalization(..)`. +pub fn store_current_session_validator_set_to_offchain() { + store_session_validator_set_to_offchain::(>::current_index()); +} \ No newline at end of file diff --git a/frame/session/src/historical/shared.rs b/frame/session/src/historical/shared.rs new file mode 100644 index 0000000000000..5013de58a83a7 --- /dev/null +++ b/frame/session/src/historical/shared.rs @@ -0,0 +1,16 @@ +use super::*; + +pub(super) const PREFIX: &[u8] = b"historical"; +pub(super) const LAST_PRUNE: &[u8] = b"historical_last_prune"; + +/// Derive the key used to store the list of validators +pub(super) fn derive_key>(prefix: P, session_index: SessionIndex) -> Vec { + let prefix: &[u8] = prefix.as_ref(); + let encoded_session_index = session_index.encode(); + assert!(encoded_session_index.len() > 0); + let mut concatenated = Vec::with_capacity(prefix.len() + 1 + encoded_session_index.len()); + concatenated.extend_from_slice(prefix); + concatenated.push('/' as u8); + concatenated.extend_from_slice(encoded_session_index.as_slice()); + concatenated +} \ No newline at end of file From f1e936724b16e500a37c91a7d4292b1efc189e31 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Fri, 5 Jun 2020 10:53:10 +0200 Subject: [PATCH 08/33] test stab --- frame/session/src/historical/offchain.rs | 82 ++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 6 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 0d7076fcbad20..74420a402c590 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -27,6 +27,7 @@ use super::super::{Module as SessionModule, SessionIndex}; use super::{IdentificationTuple, ProvingTrie, Trait}; use super::shared::*; +use sp_std::prelude::*; pub struct ValidatorSet { validator_set: Vec>, @@ -138,14 +139,83 @@ pub fn prove_session_membership>( #[cfg(test)] mod tests { - use super::super::tests::new_test_ext; + use super::super::tests; + use super::super::{onchain,Module}; use super::*; + use codec::Encode; + use sp_core::crypto::key_types::DUMMY; + use sp_runtime::testing::UintAuthorityId; + use crate::mock::{ + NEXT_VALIDATORS, force_new_session, + set_next_validators, Test, System, Session, + }; + use frame_support::traits::{KeyOwnerProofSystem, OnInitialize}; + use sp_core::offchain::{ + OpaquePeerId, + OffchainExt, + TransactionPoolExt, + testing::{TestOffchainExt, TestTransactionPoolExt}, + }; + + type Historical = Module; + + pub fn new_test_ext() -> sp_io::TestExternalities { + let mut ext = frame_system::GenesisConfig::default() + .build_storage::() + .expect("Failed to create test externalities."); + + crate::GenesisConfig:: { + keys: NEXT_VALIDATORS.with(|l| + l.borrow().iter().cloned().map(|i| (i, i, UintAuthorityId(i).into())).collect() + ), + }.assimilate_storage(&mut ext).unwrap(); + + + let (offchain, offchain_state) = TestOffchainExt::new(); + + const ITERATIONS: u32 = 5u32; + let mut seed = [0u8; 32]; + seed[0..4].copy_from_slice(&ITERATIONS.to_le_bytes()); + offchain_state.write().seed = seed; + + let mut ext = sp_io::TestExternalities::new(ext); + ext.register_extension(OffchainExt::new(offchain)); + ext + } #[test] - fn justone() { - new_test_ext().execute_with(|| { - // @todo think of a way to test this properly, not sure yet if we can just call - // on-chain and off-chain all at once from here and get meaningful results - }) + fn historical_proof_offchain() { + let mut x = new_test_ext(); + let encoded_key_1 = UintAuthorityId(1).encode(); + + x.execute_with(|| { + set_next_validators(vec![1, 2]); + force_new_session(); + + System::set_block_number(1); + Session::on_initialize(1); + + // "on-chain" + onchain::store_current_session_validator_set_to_offchain::(); + }); + x.commit_all(); + + x.execute_with(|| { + + set_next_validators(vec![7, 8]); + + force_new_session(); + + System::set_block_number(2); + Session::on_initialize(2); + + // "off-chain" + let proof = prove_session_membership::(1, (DUMMY, &encoded_key_1)); + assert!(proof.is_some()); + let proof = proof.expect("Must be Some(Proof)"); + + + assert!(Historical::check_proof((DUMMY, &encoded_key_1[..]), proof.clone()).is_some()); + }); } } From f29babe0e27629194a7f606f57d82039b7bf2f48 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Fri, 5 Jun 2020 11:53:05 +0200 Subject: [PATCH 09/33] tabs my friend --- frame/session/src/historical/offchain.rs | 270 +++++++++++------------ 1 file changed, 133 insertions(+), 137 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 74420a402c590..2a4c3adc7fb14 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -30,89 +30,89 @@ use super::shared::*; use sp_std::prelude::*; pub struct ValidatorSet { - validator_set: Vec>, + validator_set: Vec>, } impl ValidatorSet { - /// Load the set of validators for a paritcular session index from the off-chain storage. - /// - /// If none is found or decodable given `prefix` and `session`, it will return `None`. - /// Empty validator sets should only ever exist for genesis blocks. - pub fn load_from_offchain_db(session_index: SessionIndex) -> Option { - let derived_key = derive_key(PREFIX, session_index); - let validator_set = StorageValueRef::persistent(derived_key.as_ref()) - .get::>() - .flatten(); - validator_set.map(|validator_set| Self { validator_set }) - } - - /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. - pub fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { - self.validator_set.as_slice() - } - - /// Convert `self` into a vector and consume `self`. - pub fn into_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { - self.validator_set - } - - /// Attempt to prune anything that is older than `first_to_keep` session index. - /// - /// Due to re-ogranisation it could be that the `first_to_keep` might be less - /// than the stored one, in which case the conservative choice is made to keep records - /// up to the one that is the lesser. - pub fn prune_older_than(first_to_keep: SessionIndex) { - let derived_key = LAST_PRUNE.to_vec(); - let entry = StorageValueRef::persistent(derived_key.as_ref()); - match entry.mutate(|current: Option>| -> Result<_, ()> { - match current { - Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), - // do not move the cursor, if the new one would be behind ours - Some(Some(current)) => Ok(current), - None => Ok(first_to_keep), - // if the storage contains undecodable data, overwrite with current anyways - // which might leak some entries being never purged, but that is acceptable - // in this context - Some(None) => Ok(first_to_keep), - } - }) { - Ok(Ok(new_value)) => { - // on a re-org this is not necessarily true, with the above they might be equal - if new_value < first_to_keep { - for session_index in new_value..first_to_keep { - let derived_key = derive_key(PREFIX, session_index); - let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); - } - } - } - Ok(Err(_)) => {} // failed to store the value calculated with the given closure - Err(_) => {} // failed to calculate the value to store with the given closure - } - } - - /// Keep the newest `n` items, and prune all items odler than that. - pub fn keep_newest(n_to_keep: usize) { - let session_index = >::current_index(); - let n_to_keep = n_to_keep as SessionIndex; - if n_to_keep < session_index { - Self::prune_older_than(session_index - n_to_keep) - } - } - - #[inline] - fn len(&self) -> usize { - self.validator_set.len() - } + /// Load the set of validators for a paritcular session index from the off-chain storage. + /// + /// If none is found or decodable given `prefix` and `session`, it will return `None`. + /// Empty validator sets should only ever exist for genesis blocks. + pub fn load_from_offchain_db(session_index: SessionIndex) -> Option { + let derived_key = derive_key(PREFIX, session_index); + let validator_set = StorageValueRef::persistent(derived_key.as_ref()) + .get::>() + .flatten(); + validator_set.map(|validator_set| Self { validator_set }) + } + + /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. + pub fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { + self.validator_set.as_slice() + } + + /// Convert `self` into a vector and consume `self`. + pub fn into_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { + self.validator_set + } + + /// Attempt to prune anything that is older than `first_to_keep` session index. + /// + /// Due to re-ogranisation it could be that the `first_to_keep` might be less + /// than the stored one, in which case the conservative choice is made to keep records + /// up to the one that is the lesser. + pub fn prune_older_than(first_to_keep: SessionIndex) { + let derived_key = LAST_PRUNE.to_vec(); + let entry = StorageValueRef::persistent(derived_key.as_ref()); + match entry.mutate(|current: Option>| -> Result<_, ()> { + match current { + Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), + // do not move the cursor, if the new one would be behind ours + Some(Some(current)) => Ok(current), + None => Ok(first_to_keep), + // if the storage contains undecodable data, overwrite with current anyways + // which might leak some entries being never purged, but that is acceptable + // in this context + Some(None) => Ok(first_to_keep), + } + }) { + Ok(Ok(new_value)) => { + // on a re-org this is not necessarily true, with the above they might be equal + if new_value < first_to_keep { + for session_index in new_value..first_to_keep { + let derived_key = derive_key(PREFIX, session_index); + let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); + } + } + } + Ok(Err(_)) => {} // failed to store the value calculated with the given closure + Err(_) => {} // failed to calculate the value to store with the given closure + } + } + + /// Keep the newest `n` items, and prune all items odler than that. + pub fn keep_newest(n_to_keep: usize) { + let session_index = >::current_index(); + let n_to_keep = n_to_keep as SessionIndex; + if n_to_keep < session_index { + Self::prune_older_than(session_index - n_to_keep) + } + } + + #[inline] + fn len(&self) -> usize { + self.validator_set.len() + } } /// Implement conversion into iterator for usage /// with [ProvingTrie](super::ProvingTrie::generate_for). impl core::iter::IntoIterator for ValidatorSet { - type Item = (T::ValidatorId, T::FullIdentification); - type IntoIter = sp_std::vec::IntoIter; - fn into_iter(self) -> Self::IntoIter { - self.validator_set.into_iter() - } + type Item = (T::ValidatorId, T::FullIdentification); + type IntoIter = sp_std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { + self.validator_set.into_iter() + } } /// Create a proof based on the data available in the off-chain database. @@ -121,28 +121,28 @@ impl core::iter::IntoIterator for ValidatorSet { /// to do, i.e. in case of a failed proof, enqueue a transaction back on /// chain reflecting that, with all its consequences such as i.e. slashing. pub fn prove_session_membership>( - session_index: SessionIndex, - session_key: (KeyTypeId, D), + session_index: SessionIndex, + session_key: (KeyTypeId, D), ) -> Option { - let validators = ValidatorSet::::load_from_offchain_db(session_index)?; - let count = validators.len() as u32; - let trie = ProvingTrie::::generate_for(validators.into_iter()).ok()?; - - let (id, data) = session_key; - trie.prove(id, data.as_ref()) - .map(|trie_nodes| MembershipProof { - session: session_index, - trie_nodes, - validator_count: count, - }) + let validators = ValidatorSet::::load_from_offchain_db(session_index)?; + let count = validators.len() as u32; + let trie = ProvingTrie::::generate_for(validators.into_iter()).ok()?; + + let (id, data) = session_key; + trie.prove(id, data.as_ref()) + .map(|trie_nodes| MembershipProof { + session: session_index, + trie_nodes, + validator_count: count, + }) } #[cfg(test)] mod tests { - use super::super::tests; - use super::super::{onchain,Module}; - use super::*; - use codec::Encode; + use super::super::tests; + use super::super::{onchain,Module}; + use super::*; + use codec::Encode; use sp_core::crypto::key_types::DUMMY; use sp_runtime::testing::UintAuthorityId; use crate::mock::{ @@ -150,72 +150,68 @@ mod tests { set_next_validators, Test, System, Session, }; use frame_support::traits::{KeyOwnerProofSystem, OnInitialize}; - use sp_core::offchain::{ - OpaquePeerId, - OffchainExt, - TransactionPoolExt, - testing::{TestOffchainExt, TestTransactionPoolExt}, - }; + use sp_core::offchain::{ + OpaquePeerId, + OffchainExt, + TransactionPoolExt, + testing::{TestOffchainExt, TestTransactionPoolExt}, + }; - type Historical = Module; + type Historical = Module; - pub fn new_test_ext() -> sp_io::TestExternalities { - let mut ext = frame_system::GenesisConfig::default() - .build_storage::() - .expect("Failed to create test externalities."); + pub fn new_test_ext() -> sp_io::TestExternalities { + let mut ext = frame_system::GenesisConfig::default() + .build_storage::() + .expect("Failed to create test externalities."); - crate::GenesisConfig:: { - keys: NEXT_VALIDATORS.with(|l| - l.borrow().iter().cloned().map(|i| (i, i, UintAuthorityId(i).into())).collect() - ), - }.assimilate_storage(&mut ext).unwrap(); + crate::GenesisConfig:: { + keys: NEXT_VALIDATORS.with(|l| + l.borrow().iter().cloned().map(|i| (i, i, UintAuthorityId(i).into())).collect() + ), + }.assimilate_storage(&mut ext).unwrap(); - let (offchain, offchain_state) = TestOffchainExt::new(); + let (offchain, offchain_state) = TestOffchainExt::new(); - const ITERATIONS: u32 = 5u32; - let mut seed = [0u8; 32]; - seed[0..4].copy_from_slice(&ITERATIONS.to_le_bytes()); - offchain_state.write().seed = seed; + const ITERATIONS: u32 = 5u32; + let mut seed = [0u8; 32]; + seed[0..4].copy_from_slice(&ITERATIONS.to_le_bytes()); + offchain_state.write().seed = seed; - let mut ext = sp_io::TestExternalities::new(ext); - ext.register_extension(OffchainExt::new(offchain)); - ext - } + let mut ext = sp_io::TestExternalities::new(ext); + ext.register_extension(OffchainExt::new(offchain)); + ext + } - #[test] - fn historical_proof_offchain() { - let mut x = new_test_ext(); - let encoded_key_1 = UintAuthorityId(1).encode(); + #[test] + fn historical_proof_offchain() { + let mut x = new_test_ext(); + let encoded_key_1 = UintAuthorityId(1).encode(); - x.execute_with(|| { + x.execute_with(|| { set_next_validators(vec![1, 2]); force_new_session(); System::set_block_number(1); Session::on_initialize(1); - // "on-chain" - onchain::store_current_session_validator_set_to_offchain::(); - }); - x.commit_all(); - - x.execute_with(|| { + // "on-chain" + onchain::store_current_session_validator_set_to_offchain::(); - set_next_validators(vec![7, 8]); + set_next_validators(vec![7, 8]); - force_new_session(); + force_new_session(); - System::set_block_number(2); + System::set_block_number(2); Session::on_initialize(2); - // "off-chain" - let proof = prove_session_membership::(1, (DUMMY, &encoded_key_1)); - assert!(proof.is_some()); - let proof = proof.expect("Must be Some(Proof)"); + // "off-chain" + let proof = prove_session_membership::(1, (DUMMY, &encoded_key_1)); + assert!(proof.is_some()); + let proof = proof.expect("Must be Some(Proof)"); assert!(Historical::check_proof((DUMMY, &encoded_key_1[..]), proof.clone()).is_some()); - }); - } + }); + } } From 388970b6ab2cb36878913bfb2cd319791444ff52 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 9 Jun 2020 18:00:37 +0200 Subject: [PATCH 10/33] offchain overlay: split key into prefix and true key Simplifies inspection and makes key actually unique. --- primitives/core/src/offchain/storage.rs | 26 +++++++++++-------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/primitives/core/src/offchain/storage.rs b/primitives/core/src/offchain/storage.rs index 52a7bbe857d9d..943fa7ffed4e9 100644 --- a/primitives/core/src/offchain/storage.rs +++ b/primitives/core/src/offchain/storage.rs @@ -102,7 +102,7 @@ pub enum OffchainOverlayedChanges { /// Writing overlay changes to the offchain worker database is disabled by configuration. Disabled, /// Overlay changes can be recorded using the inner collection of this variant. - Enabled(HashMap, OffchainOverlayedChange>), + Enabled(HashMap<(Vec, Vec), OffchainOverlayedChange>), } impl Default for OffchainOverlayedChanges { @@ -140,23 +140,21 @@ impl OffchainOverlayedChanges { /// Remove a key and its associated value from the offchain database. pub fn remove(&mut self, prefix: &[u8], key: &[u8]) { if let Self::Enabled(ref mut storage) = self { - let key: Vec = prefix.iter().chain(key).cloned().collect(); - let _ = storage.insert(key, OffchainOverlayedChange::Remove); + let _ = storage.insert((prefix.to_vec(), key.to_vec()), OffchainOverlayedChange::Remove); } } /// Set the value associated with a key under a prefix to the value provided. pub fn set(&mut self, prefix: &[u8], key: &[u8], value: &[u8]) { if let Self::Enabled(ref mut storage) = self { - let key = prefix.iter().chain(key).cloned().collect(); - let _ = storage.insert(key, OffchainOverlayedChange::SetValue(value.to_vec())); + let _ = storage.insert((prefix.to_vec(), key.to_vec()), OffchainOverlayedChange::SetValue(value.to_vec())); } } /// Obtain a associated value to the given key in storage with prefix. pub fn get(&self, prefix: &[u8], key: &[u8]) -> Option { if let Self::Enabled(ref storage) = self { - let key: Vec = prefix.iter().chain(key).cloned().collect(); + let key = (prefix.to_vec(), key.to_vec()); storage.get(&key).cloned() } else { None @@ -168,11 +166,11 @@ use std::collections::hash_map; /// Iterate by reference over the prepared offchain worker storage changes. pub struct OffchainOverlayedChangesIter<'i> { - inner: Option, OffchainOverlayedChange>>, + inner: Option, Vec), OffchainOverlayedChange>>, } impl<'i> Iterator for OffchainOverlayedChangesIter<'i> { - type Item = (&'i Vec, &'i OffchainOverlayedChange); + type Item = (&'i (Vec, Vec), &'i OffchainOverlayedChange); fn next(&mut self) -> Option { if let Some(ref mut iter) = self.inner { iter.next() @@ -197,11 +195,11 @@ impl<'i> OffchainOverlayedChangesIter<'i> { /// Iterate by value over the prepared offchain worker storage changes. pub struct OffchainOverlayedChangesIntoIter { - inner: Option,OffchainOverlayedChange>>, + inner: Option,Vec),OffchainOverlayedChange>>, } impl Iterator for OffchainOverlayedChangesIntoIter { - type Item = (Vec, OffchainOverlayedChange); + type Item = ((Vec, Vec), OffchainOverlayedChange); fn next(&mut self) -> Option { if let Some(ref mut iter) = self.inner { iter.next() @@ -225,11 +223,11 @@ impl OffchainOverlayedChangesIntoIter { /// Iterate over all items while draining them from the collection. pub struct OffchainOverlayedChangesDrain<'d> { - inner: Option,OffchainOverlayedChange>>, + inner: Option, Vec), OffchainOverlayedChange>>, } impl<'d> Iterator for OffchainOverlayedChangesDrain<'d> { - type Item = (Vec, OffchainOverlayedChange); + type Item = ((Vec, Vec), OffchainOverlayedChange); fn next(&mut self) -> Option { if let Some(ref mut iter) = self.inner { iter.next() @@ -286,9 +284,7 @@ mod test { ooc.set(STORAGE_PREFIX, b"ppp", b"rrr"); let mut iter = ooc.into_iter(); - let mut k = STORAGE_PREFIX.to_vec(); - k.extend_from_slice(&b"ppp"[..]); - assert_eq!(iter.next(), Some((k, OffchainOverlayedChange::SetValue(b"rrr".to_vec())))); + assert_eq!(iter.next(), Some((STORAGE_PREFIX.to_vec(), b"ppp".to_vec()), OffchainOverlayedChange::SetValue(b"rrr".to_vec()))); assert_eq!(iter.next(), None); } } From 1c41dfb29071129d926f9fb5a02958ce9cc6167c Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 9 Jun 2020 18:03:31 +0200 Subject: [PATCH 11/33] test: share state --- primitives/core/src/offchain/testing.rs | 88 +++++++++++++++++++++---- primitives/state-machine/src/testing.rs | 17 ++++- 2 files changed, 90 insertions(+), 15 deletions(-) diff --git a/primitives/core/src/offchain/testing.rs b/primitives/core/src/offchain/testing.rs index 76cf8915f2054..855e6d4158cfc 100644 --- a/primitives/core/src/offchain/testing.rs +++ b/primitives/core/src/offchain/testing.rs @@ -26,7 +26,7 @@ use std::{ }; use crate::offchain::{ self, - storage::InMemOffchainStorage, + storage::{InMemOffchainStorage, OffchainOverlayedChange, OffchainOverlayedChanges}, HttpError, HttpRequestId as RequestId, HttpRequestStatus as RequestStatus, @@ -36,6 +36,7 @@ use crate::offchain::{ TransactionPool, OffchainStorage, }; + use parking_lot::RwLock; /// Pending request. @@ -61,6 +62,58 @@ pub struct PendingRequest { pub response_headers: Vec<(String, String)>, } +/// Sharable "persistent" offchain storage for test. +#[derive(Debug, Clone, Default)] +pub struct TestPersistentOffchainDB { + persistent: Arc>, +} + +impl TestPersistentOffchainDB { + /// Create a new and empty offchain storage db for persistent items + pub fn new() -> Self { + Self { + persistent: Arc::new(RwLock::new(InMemOffchainStorage::default())) + } + } + + /// Apply a set of off-chain changes directly to the test backend + pub fn apply_offchain_changes(&mut self, changes: &mut OffchainOverlayedChanges) { + let mut me = self.persistent.write(); + for ((prefix, key), value_operation) in changes.drain() { + match value_operation { + OffchainOverlayedChange::SetValue(val) => me.set(b"", key.as_slice(), val.as_slice()), + OffchainOverlayedChange::Remove => me.remove(b"", key.as_slice()), + } + } + } +} + +impl OffchainStorage for TestPersistentOffchainDB { + fn set(&mut self, prefix: &[u8], key: &[u8], value: &[u8]) { + self.persistent.write().set(&b""[..], key, value); + } + + fn remove(&mut self, prefix: &[u8], key: &[u8]) { + self.persistent.write().remove(prefix, key); + } + + fn get(&self, prefix: &[u8], key: &[u8]) -> Option> { + let value = self.persistent.read().get(prefix, key); + value + } + + fn compare_and_set( + &mut self, + prefix: &[u8], + key: &[u8], + old_value: Option<&[u8]>, + new_value: &[u8], + ) -> bool { + self.persistent.write().compare_and_set(prefix, key, old_value, new_value) + } +} + + /// Internal state of the externalities. /// /// This can be used in tests to respond or assert stuff about interactions. @@ -70,7 +123,7 @@ pub struct OffchainState { pub requests: BTreeMap, expected_requests: BTreeMap, /// Persistent local storage - pub persistent_storage: InMemOffchainStorage, + pub persistent_storage: TestPersistentOffchainDB, /// Local storage pub local_storage: InMemOffchainStorage, /// A supposedly random seed. @@ -145,6 +198,13 @@ impl TestOffchainExt { let state = ext.0.clone(); (ext, state) } + + /// Create new `TestOffchainExt` and a reference to the internal state. + pub fn with_offchain_db(offchain_db: TestPersistentOffchainDB) -> (Self, Arc>) { + let (ext, state) = Self::new(); + ext.0.write().persistent_storage = offchain_db; + (ext, state) + } } impl offchain::Externalities for TestOffchainExt { @@ -174,17 +234,17 @@ impl offchain::Externalities for TestOffchainExt { fn local_storage_set(&mut self, kind: StorageKind, key: &[u8], value: &[u8]) { let mut state = self.0.write(); match kind { - StorageKind::LOCAL => &mut state.local_storage, - StorageKind::PERSISTENT => &mut state.persistent_storage, - }.set(b"", key, value); + StorageKind::LOCAL => state.local_storage.set(b"", key, value), + StorageKind::PERSISTENT => state.persistent_storage.set(b"", key, value), + }; } fn local_storage_clear(&mut self, kind: StorageKind, key: &[u8]) { let mut state = self.0.write(); match kind { - StorageKind::LOCAL => &mut state.local_storage, - StorageKind::PERSISTENT => &mut state.persistent_storage, - }.remove(b"", key); + StorageKind::LOCAL => state.local_storage.remove(b"", key), + StorageKind::PERSISTENT => state.persistent_storage.remove(b"", key), + }; } fn local_storage_compare_and_set( @@ -196,17 +256,17 @@ impl offchain::Externalities for TestOffchainExt { ) -> bool { let mut state = self.0.write(); match kind { - StorageKind::LOCAL => &mut state.local_storage, - StorageKind::PERSISTENT => &mut state.persistent_storage, - }.compare_and_set(b"", key, old_value, new_value) + StorageKind::LOCAL => state.local_storage.compare_and_set(b"", key, old_value, new_value), + StorageKind::PERSISTENT => state.persistent_storage.compare_and_set(b"", key, old_value, new_value), + } } fn local_storage_get(&mut self, kind: StorageKind, key: &[u8]) -> Option> { let state = self.0.read(); match kind { - StorageKind::LOCAL => &state.local_storage, - StorageKind::PERSISTENT => &state.persistent_storage, - }.get(b"", key) + StorageKind::LOCAL => state.local_storage.get(b"", key), + StorageKind::PERSISTENT => state.persistent_storage.get(b"", key), + } } fn http_request_start(&mut self, method: &str, uri: &str, meta: &[u8]) -> Result { diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index 71124a68bb5cf..a4d41078e3206 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -31,7 +31,8 @@ use crate::{ }, }; use sp_core::{ - offchain::storage::OffchainOverlayedChanges, + offchain::{OffchainStorage, testing::TestPersistentOffchainDB}, + offchain::storage::{InMemOffchainStorage, OffchainOverlayedChanges, OffchainOverlayedChange}, storage::{ well_known_keys::{CHANGES_TRIE_CONFIG, CODE, HEAP_PAGES, is_child_storage_key}, Storage, @@ -47,6 +48,7 @@ where { overlay: OverlayedChanges, offchain_overlay: OffchainOverlayedChanges, + offchain_db: TestPersistentOffchainDB, storage_transaction_cache: StorageTransactionCache< as Backend>::Transaction, H, N >, @@ -108,9 +110,12 @@ impl TestExternalities extensions.register(sp_core::traits::TaskExecutorExt(sp_core::tasks::executor())); + let offchain_db = TestPersistentOffchainDB::new(); + TestExternalities { overlay, offchain_overlay, + offchain_db, changes_trie_config, extensions, changes_trie_storage: ChangesTrieInMemoryStorage::new(), @@ -119,6 +124,16 @@ impl TestExternalities } } + /// Apply the changes made by the offchain indexing API. + pub fn sync_offchain_index_changes(&mut self) { + self.offchain_db.apply_offchain_changes(&mut self.offchain_overlay); + } + + /// Create a sync wrapper around the persistent part of the offchain worker storage. + pub fn offchain_db(&self) -> TestPersistentOffchainDB { + self.offchain_db.clone() + } + /// Insert key/value into backend pub fn insert(&mut self, k: StorageKey, v: StorageValue) { self.backend.insert(vec![(None, vec![(k, Some(v))])]); From 3249272ac576b910303ae7736974d65f9c25fb65 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 9 Jun 2020 17:59:04 +0200 Subject: [PATCH 12/33] fix & test --- Cargo.lock | 2 + frame/session/src/historical/offchain.rs | 107 ++++++++++++++++++----- frame/session/src/historical/onchain.rs | 29 ++++-- primitives/core/src/offchain/testing.rs | 4 +- primitives/state-machine/src/testing.rs | 6 +- 5 files changed, 114 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 525c2fe030f4d..8d81023dd9361 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4381,10 +4381,12 @@ dependencies = [ name = "pallet-session" version = "2.0.0-rc3" dependencies = [ + "env_logger 0.7.1", "frame-support", "frame-system", "impl-trait-for-tuples", "lazy_static", + "log", "pallet-timestamp", "parity-scale-codec", "serde", diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 2a4c3adc7fb14..72626bed2e97d 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -28,6 +28,9 @@ use super::{IdentificationTuple, ProvingTrie, Trait}; use super::shared::*; use sp_std::prelude::*; +use codec::Decode; + +use sp_core::offchain::StorageKind; pub struct ValidatorSet { validator_set: Vec>, @@ -40,9 +43,11 @@ impl ValidatorSet { /// Empty validator sets should only ever exist for genesis blocks. pub fn load_from_offchain_db(session_index: SessionIndex) -> Option { let derived_key = derive_key(PREFIX, session_index); - let validator_set = StorageValueRef::persistent(derived_key.as_ref()) - .get::>() - .flatten(); + let validator_set = sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, derived_key.as_ref()) + .map(|bytes| { + as Decode>::decode(&mut bytes.as_slice()).ok() + }); + let validator_set = dbg!(validator_set)?; validator_set.map(|validator_set| Self { validator_set }) } @@ -139,23 +144,19 @@ pub fn prove_session_membership>( #[cfg(test)] mod tests { - use super::super::tests; - use super::super::{onchain,Module}; + use super::super::{onchain, Module}; use super::*; - use codec::Encode; - use sp_core::crypto::key_types::DUMMY; - use sp_runtime::testing::UintAuthorityId; use crate::mock::{ - NEXT_VALIDATORS, force_new_session, - set_next_validators, Test, System, Session, + force_new_session, set_next_validators, Session, System, Test, NEXT_VALIDATORS, }; + use codec::Encode; use frame_support::traits::{KeyOwnerProofSystem, OnInitialize}; + use sp_core::crypto::key_types::DUMMY; use sp_core::offchain::{ - OpaquePeerId, + testing::TestOffchainExt, OffchainExt, - TransactionPoolExt, - testing::{TestOffchainExt, TestTransactionPoolExt}, }; + use sp_runtime::testing::UintAuthorityId; type Historical = Module; @@ -165,30 +166,84 @@ mod tests { .expect("Failed to create test externalities."); crate::GenesisConfig:: { - keys: NEXT_VALIDATORS.with(|l| - l.borrow().iter().cloned().map(|i| (i, i, UintAuthorityId(i).into())).collect() - ), - }.assimilate_storage(&mut ext).unwrap(); + keys: NEXT_VALIDATORS.with(|l| { + l.borrow() + .iter() + .cloned() + .map(|i| (i, i, UintAuthorityId(i).into())) + .collect() + }), + } + .assimilate_storage(&mut ext) + .unwrap(); - let (offchain, offchain_state) = TestOffchainExt::new(); + let mut ext = sp_io::TestExternalities::new(ext); + + let (offchain, offchain_state) = TestOffchainExt::with_offchain_db(ext.offchain_db()); const ITERATIONS: u32 = 5u32; let mut seed = [0u8; 32]; seed[0..4].copy_from_slice(&ITERATIONS.to_le_bytes()); offchain_state.write().seed = seed; - let mut ext = sp_io::TestExternalities::new(ext); ext.register_extension(OffchainExt::new(offchain)); ext } + + + #[test] + fn encode_decode_roundtrip() { + use super::super::super::Trait as SessionTrait; + use super::super::Trait as HistoricalTrait; + + let sample = ( + 22u32 as ::ValidatorId, + 7_777_777 as ::FullIdentification); + + let encoded = sample.encode(); + let decoded = Decode::decode(&mut encoded.as_slice()).expect("Must decode"); + assert_eq!(sample, decoded); + } + + #[test] + fn onchain_to_offchain() { + let _ = env_logger::Builder::new() + .filter_level(log::LevelFilter::Trace) + .filter(None, log::LevelFilter::Warn) + .is_test(true) + .try_init(); + + let mut ext = new_test_ext(); + + const DATA: &[u8] = &[7,8,9,10,11]; + ext.execute_with(|| { + b"alphaomega"[..].using_encoded(|key| sp_io::offchain_index::set(key, DATA)); + }); + + ext.sync_offchain_index_changes(); + + ext.execute_with(|| { + let data = + b"alphaomega"[..].using_encoded(|key| sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, key)); + assert_eq!(data, Some(DATA.to_vec())); + }); + } + + #[test] fn historical_proof_offchain() { - let mut x = new_test_ext(); + let _ = env_logger::Builder::new() + .filter_level(log::LevelFilter::Trace) + .filter(None, log::LevelFilter::Warn) + .is_test(true) + .try_init(); + + let mut ext = new_test_ext(); let encoded_key_1 = UintAuthorityId(1).encode(); - x.execute_with(|| { + ext.execute_with(|| { set_next_validators(vec![1, 2]); force_new_session(); @@ -197,20 +252,28 @@ mod tests { // "on-chain" onchain::store_current_session_validator_set_to_offchain::(); + assert_eq!(>::current_index(), 1); set_next_validators(vec![7, 8]); force_new_session(); + }); + + ext.sync_offchain_index_changes(); + + + ext.execute_with(|| { + System::set_block_number(2); Session::on_initialize(2); + assert_eq!(>::current_index(), 2); // "off-chain" let proof = prove_session_membership::(1, (DUMMY, &encoded_key_1)); assert!(proof.is_some()); let proof = proof.expect("Must be Some(Proof)"); - assert!(Historical::check_proof((DUMMY, &encoded_key_1[..]), proof.clone()).is_some()); }); } diff --git a/frame/session/src/historical/onchain.rs b/frame/session/src/historical/onchain.rs index 842e4692d5e95..3e25e27b2dbc5 100644 --- a/frame/session/src/historical/onchain.rs +++ b/frame/session/src/historical/onchain.rs @@ -1,24 +1,37 @@ - use codec::Encode; +use sp_runtime::traits::Convert; +use super::super::Trait as SessionTrait; use super::super::{Module as SessionModule, SessionIndex}; -use super::Trait; +use super::Trait as HistoricalTrait; use super::shared; /// Store the validator set associated to the `session_index` to the off-chain database. /// /// **Must** be called from on-chain, i.e. `on_initialize(..)` or `on_finalization(..)`. -pub fn store_session_validator_set_to_offchain(session_index: SessionIndex) { - let derived_key = shared::derive_key(shared::PREFIX, session_index); +pub fn store_session_validator_set_to_offchain( + session_index: SessionIndex, +) { //let value = SessionModule::historical_root(session_index); - let encoded_validator_list = >::validators().encode(); - sp_io::offchain_index::set(derived_key.as_slice(), encoded_validator_list.as_slice()) + let encoded_validator_list = >::validators() + .into_iter() + .filter_map(|validator_id: ::ValidatorId| { + let full_identification = + <::FullIdentificationOf>::convert(validator_id.clone()); + full_identification.map(|full_identification| (validator_id, full_identification)) + }) + .collect::>(); + + encoded_validator_list.using_encoded(|encoded_validator_list| { + let derived_key = shared::derive_key(shared::PREFIX, session_index); + sp_io::offchain_index::set(derived_key.as_slice(), encoded_validator_list); + }); } /// Store the validator set associated to the _current_ session index to the off-chain database. /// /// **Must** be called from on-chain, i.e. `on_initialize(..)` or `on_finalization(..)`. -pub fn store_current_session_validator_set_to_offchain() { +pub fn store_current_session_validator_set_to_offchain() { store_session_validator_set_to_offchain::(>::current_index()); -} \ No newline at end of file +} diff --git a/primitives/core/src/offchain/testing.rs b/primitives/core/src/offchain/testing.rs index 855e6d4158cfc..6060f1516979d 100644 --- a/primitives/core/src/offchain/testing.rs +++ b/primitives/core/src/offchain/testing.rs @@ -79,7 +79,7 @@ impl TestPersistentOffchainDB { /// Apply a set of off-chain changes directly to the test backend pub fn apply_offchain_changes(&mut self, changes: &mut OffchainOverlayedChanges) { let mut me = self.persistent.write(); - for ((prefix, key), value_operation) in changes.drain() { + for ((_prefix, key), value_operation) in changes.drain() { match value_operation { OffchainOverlayedChange::SetValue(val) => me.set(b"", key.as_slice(), val.as_slice()), OffchainOverlayedChange::Remove => me.remove(b"", key.as_slice()), @@ -90,7 +90,7 @@ impl TestPersistentOffchainDB { impl OffchainStorage for TestPersistentOffchainDB { fn set(&mut self, prefix: &[u8], key: &[u8], value: &[u8]) { - self.persistent.write().set(&b""[..], key, value); + self.persistent.write().set(prefix, key, value); } fn remove(&mut self, prefix: &[u8], key: &[u8]) { diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index a4d41078e3206..7161471d5a60f 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -31,8 +31,10 @@ use crate::{ }, }; use sp_core::{ - offchain::{OffchainStorage, testing::TestPersistentOffchainDB}, - offchain::storage::{InMemOffchainStorage, OffchainOverlayedChanges, OffchainOverlayedChange}, + offchain::{ + testing::TestPersistentOffchainDB, + storage::OffchainOverlayedChanges + }, storage::{ well_known_keys::{CHANGES_TRIE_CONFIG, CODE, HEAP_PAGES, is_child_storage_key}, Storage, From 2600769d46bf5c585bcca93bf687d42dc76bcdc5 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 9 Jun 2020 18:36:45 +0200 Subject: [PATCH 13/33] docs improv --- frame/session/src/historical/offchain.rs | 17 +++++++++++----- frame/session/src/historical/onchain.rs | 26 ++++++++++++++++++++++-- frame/session/src/historical/shared.rs | 21 +++++++++++++++++++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 72626bed2e97d..c3507ad0cf6be 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -15,8 +15,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Validator Set Extracting an iterator from an off-chain worker stored list containing historical validatorsets. +//! Off-chain logic for creating a proof based data provided by on-chain logic. //! +//! Validator-set extracting an iterator from an off-chain worker stored list containing +//! historical validator-sets. +//! Based on the logic of historical slashing, but the validation is done off-chain. +//! Use [`fn store_current_session_validator_set_to_offchain()`](super::onchain) to store the +//! required data to the offchain validator set. //! This is used in conjunction with [`ProvingTrie`](super::ProvingTrie) and //! the off-chain indexing API. @@ -37,7 +42,7 @@ pub struct ValidatorSet { } impl ValidatorSet { - /// Load the set of validators for a paritcular session index from the off-chain storage. + /// Load the set of validators for a particular session index from the off-chain storage. /// /// If none is found or decodable given `prefix` and `session`, it will return `None`. /// Empty validator sets should only ever exist for genesis blocks. @@ -63,7 +68,7 @@ impl ValidatorSet { /// Attempt to prune anything that is older than `first_to_keep` session index. /// - /// Due to re-ogranisation it could be that the `first_to_keep` might be less + /// Due to re-organisation it could be that the `first_to_keep` might be less /// than the stored one, in which case the conservative choice is made to keep records /// up to the one that is the lesser. pub fn prune_older_than(first_to_keep: SessionIndex) { @@ -95,7 +100,7 @@ impl ValidatorSet { } } - /// Keep the newest `n` items, and prune all items odler than that. + /// Keep the newest `n` items, and prune all items older than that. pub fn keep_newest(n_to_keep: usize) { let session_index = >::current_index(); let n_to_keep = n_to_keep as SessionIndex; @@ -226,7 +231,9 @@ mod tests { ext.execute_with(|| { let data = - b"alphaomega"[..].using_encoded(|key| sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, key)); + b"alphaomega"[..].using_encoded(|key| { + sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, key) + }); assert_eq!(data, Some(DATA.to_vec())); }); } diff --git a/frame/session/src/historical/onchain.rs b/frame/session/src/historical/onchain.rs index 3e25e27b2dbc5..72b209a7e5b25 100644 --- a/frame/session/src/historical/onchain.rs +++ b/frame/session/src/historical/onchain.rs @@ -1,3 +1,22 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! On-chain logic to store a validator-set for deferred validation using an off-chain worker. + use codec::Encode; use sp_runtime::traits::Convert; @@ -7,7 +26,9 @@ use super::Trait as HistoricalTrait; use super::shared; -/// Store the validator set associated to the `session_index` to the off-chain database. +/// Store the validator-set associated to the `session_index` to the off-chain database. +/// +/// Further processing is then done [`off-chain side`](super::offchain). /// /// **Must** be called from on-chain, i.e. `on_initialize(..)` or `on_finalization(..)`. pub fn store_session_validator_set_to_offchain( @@ -31,7 +52,8 @@ pub fn store_session_validator_set_to_offchain() { store_session_validator_set_to_offchain::(>::current_index()); } diff --git a/frame/session/src/historical/shared.rs b/frame/session/src/historical/shared.rs index 5013de58a83a7..f7a1edaad2a7e 100644 --- a/frame/session/src/historical/shared.rs +++ b/frame/session/src/historical/shared.rs @@ -1,3 +1,24 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Shared logic between on-chain and off-chain components used for slashing using an off-chain +//! worker. + + use super::*; pub(super) const PREFIX: &[u8] = b"historical"; From 5383907d4ab3139cc69b58954b98e4fab225547f Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 9 Jun 2020 18:50:10 +0200 Subject: [PATCH 14/33] address review comments --- frame/session/src/historical/shared.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/frame/session/src/historical/shared.rs b/frame/session/src/historical/shared.rs index f7a1edaad2a7e..ecfdfa46d7d43 100644 --- a/frame/session/src/historical/shared.rs +++ b/frame/session/src/historical/shared.rs @@ -21,17 +21,17 @@ use super::*; -pub(super) const PREFIX: &[u8] = b"historical"; -pub(super) const LAST_PRUNE: &[u8] = b"historical_last_prune"; +pub(super) const PREFIX: &[u8] = b"session_historical"; +pub(super) const LAST_PRUNE: &[u8] = b"session_historical_last_prune"; /// Derive the key used to store the list of validators pub(super) fn derive_key>(prefix: P, session_index: SessionIndex) -> Vec { - let prefix: &[u8] = prefix.as_ref(); - let encoded_session_index = session_index.encode(); - assert!(encoded_session_index.len() > 0); - let mut concatenated = Vec::with_capacity(prefix.len() + 1 + encoded_session_index.len()); - concatenated.extend_from_slice(prefix); - concatenated.push('/' as u8); - concatenated.extend_from_slice(encoded_session_index.as_slice()); - concatenated + let prefix: &[u8] = prefix.as_ref(); + session_index.using_encoded(|encoded_session_index| { + prefix.into_iter() + .chain(b"/".into_iter()) + .chain(encoded_session_index.into_iter()) + .copied() + .collect::>() + }) } \ No newline at end of file From d6a78ca46dff717b6fc5054807b8b6a7176a1ccb Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 9 Jun 2020 19:13:53 +0200 Subject: [PATCH 15/33] cleanup test chore --- Cargo.lock | 2 -- frame/session/Cargo.toml | 2 -- frame/session/src/historical/offchain.rs | 20 +++----------------- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d81023dd9361..525c2fe030f4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4381,12 +4381,10 @@ dependencies = [ name = "pallet-session" version = "2.0.0-rc3" dependencies = [ - "env_logger 0.7.1", "frame-support", "frame-system", "impl-trait-for-tuples", "lazy_static", - "log", "pallet-timestamp", "parity-scale-codec", "serde", diff --git a/frame/session/Cargo.toml b/frame/session/Cargo.toml index 6955940dc4d5a..5092a5f21838b 100644 --- a/frame/session/Cargo.toml +++ b/frame/session/Cargo.toml @@ -25,8 +25,6 @@ sp-trie = { version = "2.0.0-rc3", optional = true, default-features = false, pa impl-trait-for-tuples = "0.1.3" [dev-dependencies] -sp-core = { version = "2.0.0-rc3", path = "../../primitives/core" } -sp-io ={ version = "2.0.0-rc3", path = "../../primitives/io" } sp-application-crypto = { version = "2.0.0-rc3", path = "../../primitives/application-crypto" } lazy_static = "1.4.0" diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index c3507ad0cf6be..b89bba6a9c2fe 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -33,9 +33,6 @@ use super::{IdentificationTuple, ProvingTrie, Trait}; use super::shared::*; use sp_std::prelude::*; -use codec::Decode; - -use sp_core::offchain::StorageKind; pub struct ValidatorSet { validator_set: Vec>, @@ -160,7 +157,9 @@ mod tests { use sp_core::offchain::{ testing::TestOffchainExt, OffchainExt, + StorageKind, }; + use sp_runtime::testing::UintAuthorityId; type Historical = Module; @@ -196,10 +195,9 @@ mod tests { ext } - - #[test] fn encode_decode_roundtrip() { + use codec::{Decode, Encode}; use super::super::super::Trait as SessionTrait; use super::super::Trait as HistoricalTrait; @@ -214,12 +212,6 @@ mod tests { #[test] fn onchain_to_offchain() { - let _ = env_logger::Builder::new() - .filter_level(log::LevelFilter::Trace) - .filter(None, log::LevelFilter::Warn) - .is_test(true) - .try_init(); - let mut ext = new_test_ext(); const DATA: &[u8] = &[7,8,9,10,11]; @@ -241,12 +233,6 @@ mod tests { #[test] fn historical_proof_offchain() { - let _ = env_logger::Builder::new() - .filter_level(log::LevelFilter::Trace) - .filter(None, log::LevelFilter::Warn) - .is_test(true) - .try_init(); - let mut ext = new_test_ext(); let encoded_key_1 = UintAuthorityId(1).encode(); From 3e5d00c533e013f9b265863bf7886b3c38526516 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 9 Jun 2020 19:14:07 +0200 Subject: [PATCH 16/33] refactor, abbrev link text --- frame/session/src/historical/offchain.rs | 10 ++++------ frame/session/src/historical/onchain.rs | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index b89bba6a9c2fe..789959f42561d 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -45,12 +45,10 @@ impl ValidatorSet { /// Empty validator sets should only ever exist for genesis blocks. pub fn load_from_offchain_db(session_index: SessionIndex) -> Option { let derived_key = derive_key(PREFIX, session_index); - let validator_set = sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, derived_key.as_ref()) - .map(|bytes| { - as Decode>::decode(&mut bytes.as_slice()).ok() - }); - let validator_set = dbg!(validator_set)?; - validator_set.map(|validator_set| Self { validator_set }) + StorageValueRef::persistent(derived_key.as_ref()) + .get::>() + .flatten() + .map(|validator_set| Self { validator_set }) } /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. diff --git a/frame/session/src/historical/onchain.rs b/frame/session/src/historical/onchain.rs index 72b209a7e5b25..f2d0f125f0f23 100644 --- a/frame/session/src/historical/onchain.rs +++ b/frame/session/src/historical/onchain.rs @@ -52,7 +52,7 @@ pub fn store_session_validator_set_to_offchain() { store_session_validator_set_to_offchain::(>::current_index()); From dbbb282fc5d630ccad7b00b44f922a8d33cd9ed2 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 10 Jun 2020 10:13:02 +0200 Subject: [PATCH 17/33] chore: linewidth --- primitives/core/src/offchain/storage.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/primitives/core/src/offchain/storage.rs b/primitives/core/src/offchain/storage.rs index 943fa7ffed4e9..6abda0829576f 100644 --- a/primitives/core/src/offchain/storage.rs +++ b/primitives/core/src/offchain/storage.rs @@ -284,7 +284,10 @@ mod test { ooc.set(STORAGE_PREFIX, b"ppp", b"rrr"); let mut iter = ooc.into_iter(); - assert_eq!(iter.next(), Some((STORAGE_PREFIX.to_vec(), b"ppp".to_vec()), OffchainOverlayedChange::SetValue(b"rrr".to_vec()))); + assert_eq!( + iter.next(), Some((STORAGE_PREFIX.to_vec(), b"ppp".to_vec()), + OffchainOverlayedChange::SetValue(b"rrr".to_vec())) + ); assert_eq!(iter.next(), None); } } From 2032b6a6f13a39d7c93498a0f7ad286a0baa2ec3 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 10 Jun 2020 12:03:43 +0200 Subject: [PATCH 18/33] fix prefix key split fallout --- client/db/src/lib.rs | 3 ++- frame/session/src/historical/offchain.rs | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index f75693ec9f00e..c40a27784c3b8 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -544,7 +544,8 @@ pub struct BlockImportOperation { impl BlockImportOperation { fn apply_offchain(&mut self, transaction: &mut Transaction) { - for (key, value_operation) in self.offchain_storage_updates.drain() { + for (prefix, key, value_operation) in self.offchain_storage_updates.drain() { + let key: Vec = prefix.chain(b"/".into_iter()).chain(key).collect(); match value_operation { OffchainOverlayedChange::SetValue(val) => transaction.set_from_vec(columns::OFFCHAIN, &key, val), OffchainOverlayedChange::Remove => transaction.remove(columns::OFFCHAIN, &key), diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 789959f42561d..94ea7e712f734 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -252,7 +252,6 @@ mod tests { ext.sync_offchain_index_changes(); - ext.execute_with(|| { From 1bb092f5db9f3dfea0e7fd52c04a396d096437c4 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 10 Jun 2020 12:23:06 +0200 Subject: [PATCH 19/33] minor fallout --- client/db/src/lib.rs | 8 ++++++-- primitives/core/src/offchain/storage.rs | 7 +++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index c40a27784c3b8..ac73b9469d66e 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -544,8 +544,12 @@ pub struct BlockImportOperation { impl BlockImportOperation { fn apply_offchain(&mut self, transaction: &mut Transaction) { - for (prefix, key, value_operation) in self.offchain_storage_updates.drain() { - let key: Vec = prefix.chain(b"/".into_iter()).chain(key).collect(); + for ((prefix, key), value_operation) in self.offchain_storage_updates.drain() { + let key: Vec = prefix + .into_iter() + .chain(b"/".into_iter().copied()) + .chain(key.into_iter()) + .collect(); match value_operation { OffchainOverlayedChange::SetValue(val) => transaction.set_from_vec(columns::OFFCHAIN, &key, val), OffchainOverlayedChange::Remove => transaction.remove(columns::OFFCHAIN, &key), diff --git a/primitives/core/src/offchain/storage.rs b/primitives/core/src/offchain/storage.rs index 6abda0829576f..f544f6342de57 100644 --- a/primitives/core/src/offchain/storage.rs +++ b/primitives/core/src/offchain/storage.rs @@ -285,8 +285,11 @@ mod test { ooc.set(STORAGE_PREFIX, b"ppp", b"rrr"); let mut iter = ooc.into_iter(); assert_eq!( - iter.next(), Some((STORAGE_PREFIX.to_vec(), b"ppp".to_vec()), - OffchainOverlayedChange::SetValue(b"rrr".to_vec())) + iter.next(), + Some( + ((STORAGE_PREFIX.to_vec(), b"ppp".to_vec()), + OffchainOverlayedChange::SetValue(b"rrr".to_vec())) + ) ); assert_eq!(iter.next(), None); } From 4abd9699c16fc547daebeaf556ee4639b88332f4 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 10 Jun 2020 15:37:24 +0200 Subject: [PATCH 20/33] minor changes --- frame/session/Cargo.toml | 4 ++++ frame/session/src/historical.rs | 2 +- frame/session/src/historical/onchain.rs | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/frame/session/Cargo.toml b/frame/session/Cargo.toml index 5092a5f21838b..bee610a2b91a7 100644 --- a/frame/session/Cargo.toml +++ b/frame/session/Cargo.toml @@ -14,7 +14,9 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] serde = { version = "1.0.101", optional = true } codec = { package = "parity-scale-codec", version = "1.3.0", default-features = false, features = ["derive"] } +sp-core = { version = "2.0.0-rc3", default-features = false, path = "../../primitives/core" } sp-std = { version = "2.0.0-rc3", default-features = false, path = "../../primitives/std" } +sp-io = { version = "2.0.0-rc3", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "2.0.0-rc3", default-features = false, path = "../../primitives/runtime" } sp-session = { version = "2.0.0-rc3", default-features = false, path = "../../primitives/session" } sp-staking = { version = "2.0.0-rc3", default-features = false, path = "../../primitives/staking" } @@ -36,6 +38,8 @@ std = [ "codec/std", "sp-std/std", "frame-support/std", + "sp-core/std", + "sp-std/std", "sp-runtime/std", "sp-session/std", "sp-staking/std", diff --git a/frame/session/src/historical.rs b/frame/session/src/historical.rs index 33306dcae2213..c9ff9e0135b54 100644 --- a/frame/session/src/historical.rs +++ b/frame/session/src/historical.rs @@ -316,7 +316,7 @@ impl> frame_support::traits::KeyOwnerProofSystem<(KeyTy #[cfg(test)] pub(crate) mod tests { use super::*; - use sp_core::crypto::key_types::DUMMY; + use sp_runtime::key_types::DUMMY; use sp_runtime::testing::UintAuthorityId; use crate::mock::{ NEXT_VALIDATORS, force_new_session, diff --git a/frame/session/src/historical/onchain.rs b/frame/session/src/historical/onchain.rs index f2d0f125f0f23..0244f30c18f78 100644 --- a/frame/session/src/historical/onchain.rs +++ b/frame/session/src/historical/onchain.rs @@ -25,6 +25,7 @@ use super::super::{Module as SessionModule, SessionIndex}; use super::Trait as HistoricalTrait; use super::shared; +use sp_std::prelude::*; /// Store the validator-set associated to the `session_index` to the off-chain database. /// From 1fdc7c13ec7f578bc1059530538587b4668caf18 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 10 Jun 2020 18:08:04 +0200 Subject: [PATCH 21/33] addresses review comments --- client/db/src/lib.rs | 2 +- frame/session/src/historical/offchain.rs | 2 ++ frame/session/src/historical/onchain.rs | 4 +++- frame/session/src/historical/shared.rs | 5 ++++- primitives/core/src/offchain/storage.rs | 3 ++- 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index ac73b9469d66e..3bae23456753e 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -547,7 +547,7 @@ impl BlockImportOperation { for ((prefix, key), value_operation) in self.offchain_storage_updates.drain() { let key: Vec = prefix .into_iter() - .chain(b"/".into_iter().copied()) + .chain(sp_core::sp_std::iter::once(b'/')) .chain(key.into_iter()) .collect(); match value_operation { diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 94ea7e712f734..007084f50db5b 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -34,6 +34,8 @@ use super::{IdentificationTuple, ProvingTrie, Trait}; use super::shared::*; use sp_std::prelude::*; + +/// A set of validators, which was used for a fixed session index. pub struct ValidatorSet { validator_set: Vec>, } diff --git a/frame/session/src/historical/onchain.rs b/frame/session/src/historical/onchain.rs index 0244f30c18f78..9ab6e7f5d6172 100644 --- a/frame/session/src/historical/onchain.rs +++ b/frame/session/src/historical/onchain.rs @@ -32,10 +32,12 @@ use sp_std::prelude::*; /// Further processing is then done [`off-chain side`](super::offchain). /// /// **Must** be called from on-chain, i.e. `on_initialize(..)` or `on_finalization(..)`. +/// **Must** be called within the given session, otherwise the full identification +/// information is not available anymore. Storing validator sets can thus not be +/// done retrospectively. pub fn store_session_validator_set_to_offchain( session_index: SessionIndex, ) { - //let value = SessionModule::historical_root(session_index); let encoded_validator_list = >::validators() .into_iter() .filter_map(|validator_id: ::ValidatorId| { diff --git a/frame/session/src/historical/shared.rs b/frame/session/src/historical/shared.rs index ecfdfa46d7d43..b9ebeb5517560 100644 --- a/frame/session/src/historical/shared.rs +++ b/frame/session/src/historical/shared.rs @@ -19,7 +19,10 @@ //! worker. -use super::*; +use super::SessionIndex; +use sp_std::prelude::Vec; +use core::convert::AsRef; +use codec::Encode; pub(super) const PREFIX: &[u8] = b"session_historical"; pub(super) const LAST_PRUNE: &[u8] = b"session_historical_last_prune"; diff --git a/primitives/core/src/offchain/storage.rs b/primitives/core/src/offchain/storage.rs index f544f6342de57..7d7c711ed95f0 100644 --- a/primitives/core/src/offchain/storage.rs +++ b/primitives/core/src/offchain/storage.rs @@ -101,7 +101,8 @@ pub enum OffchainOverlayedChange { pub enum OffchainOverlayedChanges { /// Writing overlay changes to the offchain worker database is disabled by configuration. Disabled, - /// Overlay changes can be recorded using the inner collection of this variant. + /// Overlay changes can be recorded using the inner collection of this variant, + /// where the identifier is the tuple of `(prefix, key)`. Enabled(HashMap<(Vec, Vec), OffchainOverlayedChange>), } From 1d271dbe37d6fc788a764a43eca31ae4907f7ab1 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 10 Jun 2020 18:09:54 +0200 Subject: [PATCH 22/33] rename historical.rs -> historical/mod.rs --- frame/session/src/{historical.rs => historical/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename frame/session/src/{historical.rs => historical/mod.rs} (100%) diff --git a/frame/session/src/historical.rs b/frame/session/src/historical/mod.rs similarity index 100% rename from frame/session/src/historical.rs rename to frame/session/src/historical/mod.rs From acb67fdfedf2f96ede895342828dc4c93f670dd9 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Wed, 10 Jun 2020 18:23:09 +0200 Subject: [PATCH 23/33] avoid shared::* wildcard import --- frame/session/src/historical/offchain.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 007084f50db5b..7d71e4ae50381 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -31,7 +31,7 @@ use sp_session::MembershipProof; use super::super::{Module as SessionModule, SessionIndex}; use super::{IdentificationTuple, ProvingTrie, Trait}; -use super::shared::*; +use super::shared; use sp_std::prelude::*; @@ -46,7 +46,7 @@ impl ValidatorSet { /// If none is found or decodable given `prefix` and `session`, it will return `None`. /// Empty validator sets should only ever exist for genesis blocks. pub fn load_from_offchain_db(session_index: SessionIndex) -> Option { - let derived_key = derive_key(PREFIX, session_index); + let derived_key = shared::derive_key(PREFIX, session_index); StorageValueRef::persistent(derived_key.as_ref()) .get::>() .flatten() @@ -87,7 +87,7 @@ impl ValidatorSet { // on a re-org this is not necessarily true, with the above they might be equal if new_value < first_to_keep { for session_index in new_value..first_to_keep { - let derived_key = derive_key(PREFIX, session_index); + let derived_key = shared::derive_key(PREFIX, session_index); let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); } } From 359dec91b60a6990485e35d5c2be7f8f6c588fdf Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 11 Jun 2020 19:54:13 +0200 Subject: [PATCH 24/33] fix/compile: missing shared:: prefix --- frame/session/src/historical/offchain.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 7d71e4ae50381..ade28ff174ae9 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -46,7 +46,7 @@ impl ValidatorSet { /// If none is found or decodable given `prefix` and `session`, it will return `None`. /// Empty validator sets should only ever exist for genesis blocks. pub fn load_from_offchain_db(session_index: SessionIndex) -> Option { - let derived_key = shared::derive_key(PREFIX, session_index); + let derived_key = shared::derive_key(shared::PREFIX, session_index); StorageValueRef::persistent(derived_key.as_ref()) .get::>() .flatten() @@ -69,7 +69,7 @@ impl ValidatorSet { /// than the stored one, in which case the conservative choice is made to keep records /// up to the one that is the lesser. pub fn prune_older_than(first_to_keep: SessionIndex) { - let derived_key = LAST_PRUNE.to_vec(); + let derived_key = shared::LAST_PRUNE.to_vec(); let entry = StorageValueRef::persistent(derived_key.as_ref()); match entry.mutate(|current: Option>| -> Result<_, ()> { match current { @@ -87,7 +87,7 @@ impl ValidatorSet { // on a re-org this is not necessarily true, with the above they might be equal if new_value < first_to_keep { for session_index in new_value..first_to_keep { - let derived_key = shared::derive_key(PREFIX, session_index); + let derived_key = shared::derive_key(shared::PREFIX, session_index); let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); } } From 126f6ea971301e168e0e82bacba81702bac7d9fe Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 11 Jun 2020 19:53:51 +0200 Subject: [PATCH 25/33] fix: add missing call to store_session_validator_set_to_offchain --- frame/session/src/historical/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/session/src/historical/mod.rs b/frame/session/src/historical/mod.rs index c9ff9e0135b54..7da6b2ba017c3 100644 --- a/frame/session/src/historical/mod.rs +++ b/frame/session/src/historical/mod.rs @@ -120,6 +120,9 @@ impl crate::SessionManager for NoteHistoricalRoot { fn new_session(new_index: SessionIndex) -> Option> { + // the previous sessions validator set is the one we want to save for off-chain processing + onchain::store_session_validator_set_to_offchain::(new_index - 1); + StoredRange::mutate(|range| { range.get_or_insert_with(|| (new_index, new_index)).1 = new_index + 1; }); From 7a7c0b2490d6fa047bde77b8854166a3bbab29dd Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Fri, 12 Jun 2020 10:25:12 +0200 Subject: [PATCH 26/33] fix/test: flow --- frame/session/src/historical/mod.rs | 5 +++-- frame/session/src/historical/onchain.rs | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frame/session/src/historical/mod.rs b/frame/session/src/historical/mod.rs index 7da6b2ba017c3..20c3d57464c89 100644 --- a/frame/session/src/historical/mod.rs +++ b/frame/session/src/historical/mod.rs @@ -120,8 +120,6 @@ impl crate::SessionManager for NoteHistoricalRoot { fn new_session(new_index: SessionIndex) -> Option> { - // the previous sessions validator set is the one we want to save for off-chain processing - onchain::store_session_validator_set_to_offchain::(new_index - 1); StoredRange::mutate(|range| { range.get_or_insert_with(|| (new_index, new_index)).1 = new_index + 1; @@ -150,10 +148,13 @@ impl crate::SessionManager for NoteHistoricalRoot>::start_session(start_index) } + fn end_session(end_index: SessionIndex) { + onchain::store_session_validator_set_to_offchain::(end_index); >::end_session(end_index) } } diff --git a/frame/session/src/historical/onchain.rs b/frame/session/src/historical/onchain.rs index 9ab6e7f5d6172..745603a49829b 100644 --- a/frame/session/src/historical/onchain.rs +++ b/frame/session/src/historical/onchain.rs @@ -31,10 +31,10 @@ use sp_std::prelude::*; /// /// Further processing is then done [`off-chain side`](super::offchain). /// -/// **Must** be called from on-chain, i.e. `on_initialize(..)` or `on_finalization(..)`. -/// **Must** be called within the given session, otherwise the full identification -/// information is not available anymore. Storing validator sets can thus not be -/// done retrospectively. +/// **Must** be called from on-chain, i.e. a call that originates from +/// `on_initialize(..)` or `on_finalization(..)`. +/// **Must** be called during the session, which validator-set is to be stored for further +/// off-chain processing. Otherwise the `FullIdentification` might not be available. pub fn store_session_validator_set_to_offchain( session_index: SessionIndex, ) { From 23cc9cf49349cbfae22c4b7f383daf1699b5fa31 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Mon, 15 Jun 2020 15:34:59 +0200 Subject: [PATCH 27/33] fix/review: Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomasz Drwięga --- frame/session/src/historical/offchain.rs | 13 ++----------- primitives/core/src/offchain/testing.rs | 3 +-- primitives/state-machine/src/testing.rs | 4 ++-- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index ade28ff174ae9..f66fc98fe39c8 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -53,22 +53,13 @@ impl ValidatorSet { .map(|validator_set| Self { validator_set }) } - /// Access the underlying `ValidatorId` and `FullIdentification` tuples as slice. - pub fn as_slice(&self) -> &[(T::ValidatorId, T::FullIdentification)] { - self.validator_set.as_slice() - } - - /// Convert `self` into a vector and consume `self`. - pub fn into_vec(self) -> Vec<(T::ValidatorId, T::FullIdentification)> { - self.validator_set - } /// Attempt to prune anything that is older than `first_to_keep` session index. /// /// Due to re-organisation it could be that the `first_to_keep` might be less /// than the stored one, in which case the conservative choice is made to keep records /// up to the one that is the lesser. - pub fn prune_older_than(first_to_keep: SessionIndex) { + fn prune_older_than(first_to_keep: SessionIndex) { let derived_key = shared::LAST_PRUNE.to_vec(); let entry = StorageValueRef::persistent(derived_key.as_ref()); match entry.mutate(|current: Option>| -> Result<_, ()> { @@ -114,7 +105,7 @@ impl ValidatorSet { /// Implement conversion into iterator for usage /// with [ProvingTrie](super::ProvingTrie::generate_for). -impl core::iter::IntoIterator for ValidatorSet { +impl sp_std::iter::IntoIterator for ValidatorSet { type Item = (T::ValidatorId, T::FullIdentification); type IntoIter = sp_std::vec::IntoIter; fn into_iter(self) -> Self::IntoIter { diff --git a/primitives/core/src/offchain/testing.rs b/primitives/core/src/offchain/testing.rs index 6060f1516979d..a14e906f54308 100644 --- a/primitives/core/src/offchain/testing.rs +++ b/primitives/core/src/offchain/testing.rs @@ -98,8 +98,7 @@ impl OffchainStorage for TestPersistentOffchainDB { } fn get(&self, prefix: &[u8], key: &[u8]) -> Option> { - let value = self.persistent.read().get(prefix, key); - value + self.persistent.read().get(prefix, key) } fn compare_and_set( diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index 7161471d5a60f..2a55ed699a53d 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -126,8 +126,8 @@ impl TestExternalities } } - /// Apply the changes made by the offchain indexing API. - pub fn sync_offchain_index_changes(&mut self) { + /// Move offchain changes from overlay to the persistent store. + pub fn persist_offchain_overlay(&mut self) { self.offchain_db.apply_offchain_changes(&mut self.offchain_overlay); } From 68ad304eb1a5012f5c1f71ed7dccc28cbacf5c22 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Mon, 15 Jun 2020 16:02:26 +0200 Subject: [PATCH 28/33] fix/review: more review comment fixes --- frame/session/Cargo.toml | 2 +- primitives/state-machine/src/testing.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/session/Cargo.toml b/frame/session/Cargo.toml index bee610a2b91a7..6b74e3ef5f717 100644 --- a/frame/session/Cargo.toml +++ b/frame/session/Cargo.toml @@ -37,9 +37,9 @@ std = [ "serde", "codec/std", "sp-std/std", + "sp-io/std", "frame-support/std", "sp-core/std", - "sp-std/std", "sp-runtime/std", "sp-session/std", "sp-staking/std", diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index 2a55ed699a53d..2ea2961830fc9 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -131,7 +131,7 @@ impl TestExternalities self.offchain_db.apply_offchain_changes(&mut self.offchain_overlay); } - /// Create a sync wrapper around the persistent part of the offchain worker storage. + /// A shared reference type around the offchain worker storage. pub fn offchain_db(&self) -> TestPersistentOffchainDB { self.offchain_db.clone() } From bf34c2c3ff0e25d79d15b0422819016505c0310b Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Mon, 15 Jun 2020 16:15:33 +0200 Subject: [PATCH 29/33] fix/review: make ValidatorSet private --- frame/session/src/historical/offchain.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index f66fc98fe39c8..0972e637d66c4 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -36,7 +36,7 @@ use sp_std::prelude::*; /// A set of validators, which was used for a fixed session index. -pub struct ValidatorSet { +struct ValidatorSet { validator_set: Vec>, } From 55f427175aa2e1027d8fccf7824f249a9f8af740 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Mon, 15 Jun 2020 16:23:45 +0200 Subject: [PATCH 30/33] fix/include: core -> sp_core --- frame/session/src/historical/shared.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/session/src/historical/shared.rs b/frame/session/src/historical/shared.rs index b9ebeb5517560..fda0361b05959 100644 --- a/frame/session/src/historical/shared.rs +++ b/frame/session/src/historical/shared.rs @@ -20,8 +20,7 @@ use super::SessionIndex; -use sp_std::prelude::Vec; -use core::convert::AsRef; +use sp_std::prelude::*; use codec::Encode; pub(super) const PREFIX: &[u8] = b"session_historical"; From 6c6c4841a677cb987521eca77e10fb6910e7e02a Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Mon, 15 Jun 2020 17:27:43 +0200 Subject: [PATCH 31/33] fix/review: fallout --- frame/session/src/historical/offchain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 0972e637d66c4..3bcce032ddf08 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -210,7 +210,7 @@ mod tests { b"alphaomega"[..].using_encoded(|key| sp_io::offchain_index::set(key, DATA)); }); - ext.sync_offchain_index_changes(); + ext.persist_offchain_overlay(); ext.execute_with(|| { let data = @@ -243,7 +243,7 @@ mod tests { force_new_session(); }); - ext.sync_offchain_index_changes(); + ext.persist_offchain_overlay(); ext.execute_with(|| { From 70524840f0d21f5ec1648f74dca3691c76fab140 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Mon, 15 Jun 2020 17:40:05 +0200 Subject: [PATCH 32/33] fix/visbility: make them public API Ref #6358 --- frame/session/src/historical/offchain.rs | 88 ++++++++++++------------ 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index 3bcce032ddf08..a6a4b4d938010 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -53,50 +53,6 @@ impl ValidatorSet { .map(|validator_set| Self { validator_set }) } - - /// Attempt to prune anything that is older than `first_to_keep` session index. - /// - /// Due to re-organisation it could be that the `first_to_keep` might be less - /// than the stored one, in which case the conservative choice is made to keep records - /// up to the one that is the lesser. - fn prune_older_than(first_to_keep: SessionIndex) { - let derived_key = shared::LAST_PRUNE.to_vec(); - let entry = StorageValueRef::persistent(derived_key.as_ref()); - match entry.mutate(|current: Option>| -> Result<_, ()> { - match current { - Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), - // do not move the cursor, if the new one would be behind ours - Some(Some(current)) => Ok(current), - None => Ok(first_to_keep), - // if the storage contains undecodable data, overwrite with current anyways - // which might leak some entries being never purged, but that is acceptable - // in this context - Some(None) => Ok(first_to_keep), - } - }) { - Ok(Ok(new_value)) => { - // on a re-org this is not necessarily true, with the above they might be equal - if new_value < first_to_keep { - for session_index in new_value..first_to_keep { - let derived_key = shared::derive_key(shared::PREFIX, session_index); - let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); - } - } - } - Ok(Err(_)) => {} // failed to store the value calculated with the given closure - Err(_) => {} // failed to calculate the value to store with the given closure - } - } - - /// Keep the newest `n` items, and prune all items older than that. - pub fn keep_newest(n_to_keep: usize) { - let session_index = >::current_index(); - let n_to_keep = n_to_keep as SessionIndex; - if n_to_keep < session_index { - Self::prune_older_than(session_index - n_to_keep) - } - } - #[inline] fn len(&self) -> usize { self.validator_set.len() @@ -135,6 +91,50 @@ pub fn prove_session_membership>( }) } + +/// Attempt to prune anything that is older than `first_to_keep` session index. +/// +/// Due to re-organisation it could be that the `first_to_keep` might be less +/// than the stored one, in which case the conservative choice is made to keep records +/// up to the one that is the lesser. +pub fn prune_older_than(first_to_keep: SessionIndex) { + let derived_key = shared::LAST_PRUNE.to_vec(); + let entry = StorageValueRef::persistent(derived_key.as_ref()); + match entry.mutate(|current: Option>| -> Result<_, ()> { + match current { + Some(Some(current)) if current < first_to_keep => Ok(first_to_keep), + // do not move the cursor, if the new one would be behind ours + Some(Some(current)) => Ok(current), + None => Ok(first_to_keep), + // if the storage contains undecodable data, overwrite with current anyways + // which might leak some entries being never purged, but that is acceptable + // in this context + Some(None) => Ok(first_to_keep), + } + }) { + Ok(Ok(new_value)) => { + // on a re-org this is not necessarily true, with the above they might be equal + if new_value < first_to_keep { + for session_index in new_value..first_to_keep { + let derived_key = shared::derive_key(shared::PREFIX, session_index); + let _ = StorageValueRef::persistent(derived_key.as_ref()).clear(); + } + } + } + Ok(Err(_)) => {} // failed to store the value calculated with the given closure + Err(_) => {} // failed to calculate the value to store with the given closure + } +} + +/// Keep the newest `n` items, and prune all items older than that. +pub fn keep_newest(n_to_keep: usize) { + let session_index = >::current_index(); + let n_to_keep = n_to_keep as SessionIndex; + if n_to_keep < session_index { + Self::prune_older_than(session_index - n_to_keep) + } +} + #[cfg(test)] mod tests { use super::super::{onchain, Module}; From 5231d068cca94adcf62602f132adebdf250a2cf5 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Tue, 16 Jun 2020 10:05:25 +0200 Subject: [PATCH 33/33] fix/review: review changes fallout - again --- frame/session/src/historical/offchain.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index a6a4b4d938010..97655d1a18b32 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -97,7 +97,7 @@ pub fn prove_session_membership>( /// Due to re-organisation it could be that the `first_to_keep` might be less /// than the stored one, in which case the conservative choice is made to keep records /// up to the one that is the lesser. -pub fn prune_older_than(first_to_keep: SessionIndex) { +pub fn prune_older_than(first_to_keep: SessionIndex) { let derived_key = shared::LAST_PRUNE.to_vec(); let entry = StorageValueRef::persistent(derived_key.as_ref()); match entry.mutate(|current: Option>| -> Result<_, ()> { @@ -127,11 +127,11 @@ pub fn prune_older_than(first_to_keep: SessionIndex) { } /// Keep the newest `n` items, and prune all items older than that. -pub fn keep_newest(n_to_keep: usize) { +pub fn keep_newest(n_to_keep: usize) { let session_index = >::current_index(); let n_to_keep = n_to_keep as SessionIndex; if n_to_keep < session_index { - Self::prune_older_than(session_index - n_to_keep) + prune_older_than::(session_index - n_to_keep) } }