Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
b7d4a2a
draft
Jun 2, 2020
d2bb3b8
steps
Jun 2, 2020
7a4426c
chore: fmt
Jun 2, 2020
8eb7a4c
step by step
Jun 3, 2020
fe0682b
more details
Jun 3, 2020
c24b0a3
make test public
Jun 4, 2020
5aec81e
refactor: split into on and offchain
Jun 4, 2020
f1e9367
test stab
Jun 5, 2020
f29babe
tabs my friend
Jun 5, 2020
388970b
offchain overlay: split key into prefix and true key
Jun 9, 2020
1c41dfb
test: share state
Jun 9, 2020
3249272
fix & test
Jun 9, 2020
2600769
docs improv
Jun 9, 2020
5383907
address review comments
Jun 9, 2020
d6a78ca
cleanup test chore
Jun 9, 2020
3e5d00c
refactor, abbrev link text
Jun 9, 2020
dbbb282
chore: linewidth
Jun 10, 2020
2032b6a
fix prefix key split fallout
Jun 10, 2020
1bb092f
minor fallout
Jun 10, 2020
4abd969
minor changes
Jun 10, 2020
1fdc7c1
addresses review comments
Jun 10, 2020
1d271db
rename historical.rs -> historical/mod.rs
Jun 10, 2020
acb67fd
avoid shared::* wildcard import
Jun 10, 2020
359dec9
fix/compile: missing shared:: prefix
Jun 11, 2020
126f6ea
fix: add missing call to store_session_validator_set_to_offchain
Jun 11, 2020
7a7c0b2
fix/test: flow
Jun 12, 2020
87c1696
Merge remote-tracking branch 'origin/master' into bernhard/historical…
Jun 15, 2020
23cc9cf
fix/review: Apply suggestions from code review
drahnr Jun 15, 2020
68ad304
fix/review: more review comment fixes
Jun 15, 2020
bf34c2c
fix/review: make ValidatorSet private
Jun 15, 2020
55f4271
fix/include: core -> sp_core
Jun 15, 2020
6c6c484
fix/review: fallout
Jun 15, 2020
7052484
fix/visbility: make them public API
Jun 15, 2020
6069f35
Merge remote-tracking branch 'origin/master' into bernhard/historical…
Jun 15, 2020
5231d06
fix/review: review changes fallout - again
Jun 16, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix & test
  • Loading branch information
Bernhard Schuster committed Jun 10, 2020
commit 3249272ac576b910303ae7736974d65f9c25fb65
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

107 changes: 85 additions & 22 deletions frame/session/src/historical/offchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: Trait> {
validator_set: Vec<IdentificationTuple<T>>,
Expand All @@ -40,9 +43,11 @@ impl<T: Trait> ValidatorSet<T> {
/// Empty validator sets should only ever exist for genesis blocks.
pub fn load_from_offchain_db(session_index: SessionIndex) -> Option<Self> {
let derived_key = derive_key(PREFIX, session_index);
let validator_set = StorageValueRef::persistent(derived_key.as_ref())
.get::<Vec<(T::ValidatorId, T::FullIdentification)>>()
.flatten();
let validator_set = sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, derived_key.as_ref())
.map(|bytes| {
<Vec<(T::ValidatorId, T::FullIdentification)> as Decode>::decode(&mut bytes.as_slice()).ok()
});
let validator_set = dbg!(validator_set)?;
validator_set.map(|validator_set| Self { validator_set })
}

Expand Down Expand Up @@ -139,23 +144,19 @@ pub fn prove_session_membership<T: Trait, D: AsRef<[u8]>>(

#[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<Test>;

Expand All @@ -165,30 +166,84 @@ mod tests {
.expect("Failed to create test externalities.");

crate::GenesisConfig::<Test> {
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 <Test as SessionTrait>::ValidatorId,
7_777_777 as <Test as HistoricalTrait>::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();

Expand All @@ -197,20 +252,28 @@ mod tests {

// "on-chain"
onchain::store_current_session_validator_set_to_offchain::<Test>();
assert_eq!(<SessionModule<Test>>::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!(<SessionModule<Test>>::current_index(), 2);

// "off-chain"
let proof = prove_session_membership::<Test, _>(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());
});
}
Expand Down
29 changes: 21 additions & 8 deletions frame/session/src/historical/onchain.rs
Original file line number Diff line number Diff line change
@@ -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<T: Trait>(session_index: SessionIndex) {
let derived_key = shared::derive_key(shared::PREFIX, session_index);
pub fn store_session_validator_set_to_offchain<T: HistoricalTrait + SessionTrait>(
session_index: SessionIndex,
) {
//let value = SessionModule::historical_root(session_index);
let encoded_validator_list = <SessionModule<T>>::validators().encode();
sp_io::offchain_index::set(derived_key.as_slice(), encoded_validator_list.as_slice())
let encoded_validator_list = <SessionModule<T>>::validators()
.into_iter()
.filter_map(|validator_id: <T as SessionTrait>::ValidatorId| {
let full_identification =
<<T as HistoricalTrait>::FullIdentificationOf>::convert(validator_id.clone());
full_identification.map(|full_identification| (validator_id, full_identification))
})
.collect::<Vec<_>>();

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<T: Trait>() {
pub fn store_current_session_validator_set_to_offchain<T: HistoricalTrait + SessionTrait>() {
store_session_validator_set_to_offchain::<T>(<SessionModule<T>>::current_index());
}
}
4 changes: 2 additions & 2 deletions primitives/core/src/offchain/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about the prefix:

  1. Why is it dropped here?
  2. Why isn't prefix merged with key in some of the upper layers? prefix seems like something very high-level, and on the KVDB level we should really be left with only keys and values.
  3. The prefix logic here introduces inconsistency with real-world backend and I think it would be best to avoid that.

Copy link
Contributor Author

@drahnr drahnr Jun 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue here is that Test* assumes a prefix of "" but the non-Test* structures use "storage" as prefix - to avoid this disconnect, the prefis is dropped here and a few lines down "" is used instead.

The whole point of this is to implement apply_offchain_changes which writes data to the correct key as used in TestOffchainExt*.

match value_operation {
OffchainOverlayedChange::SetValue(val) => me.set(b"", key.as_slice(), val.as_slice()),
OffchainOverlayedChange::Remove => me.remove(b"", key.as_slice()),
Expand All @@ -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]) {
Expand Down
6 changes: 4 additions & 2 deletions primitives/state-machine/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down