historical slashing w ocw w adhoc tree creation (#6220)

* draft

* steps

* chore: fmt

* step by step

* more details

* make test public

* refactor: split into on and offchain

* test stab

* tabs my friend

* offchain overlay: split key into prefix and true key

Simplifies inspection and makes key actually unique.

* test: share state

* fix & test

* docs improv

* address review comments

* cleanup test chore

* refactor, abbrev link text

* chore: linewidth

* fix prefix key split fallout

* minor fallout

* minor changes

* addresses review comments

* rename historical.rs -> historical/mod.rs

* avoid shared::* wildcard import

* fix: add missing call to store_session_validator_set_to_offchain

* fix/compile: missing shared:: prefix

* fix/test: flow

* fix/review: Apply suggestions from code review

Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* fix/review: more review comment fixes

* fix/review: make ValidatorSet private

* fix/include: core -> sp_core

* fix/review: fallout

* fix/visbility: make them public API

Ref #6358

* fix/review: review changes fallout - again

Co-authored-by: Bernhard Schuster <bernhard@parity.io>
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

substrate/primitives/core/src/offchain/storage.rs

@@ -101,8 +101,9 @@ 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.
-	Enabled(HashMap<Vec<u8>, OffchainOverlayedChange>),
+	/// Overlay changes can be recorded using the inner collection of this variant,
+	/// where the identifier is the tuple of `(prefix, key)`.
+	Enabled(HashMap<(Vec<u8>, Vec<u8>), OffchainOverlayedChange>),
 }
 
 impl Default for OffchainOverlayedChanges {
@@ -140,23 +141,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<u8> = 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<OffchainOverlayedChange> {
 		if let Self::Enabled(ref storage) = self {
-			let key: Vec<u8> = prefix.iter().chain(key).cloned().collect();
+			let key = (prefix.to_vec(), key.to_vec());
 			storage.get(&key).cloned()
 		} else {
 			None
@@ -168,11 +167,11 @@ use std::collections::hash_map;
 
 /// Iterate by reference over the prepared offchain worker storage changes.
 pub struct OffchainOverlayedChangesIter<'i> {
-	inner: Option<hash_map::Iter<'i, Vec<u8>, OffchainOverlayedChange>>,
+	inner: Option<hash_map::Iter<'i, (Vec<u8>, Vec<u8>), OffchainOverlayedChange>>,
 }
 
 impl<'i> Iterator for OffchainOverlayedChangesIter<'i> {
-	type Item = (&'i Vec<u8>, &'i OffchainOverlayedChange);
+	type Item = (&'i (Vec<u8>, Vec<u8>), &'i OffchainOverlayedChange);
 	fn next(&mut self) -> Option<Self::Item> {
 		if let Some(ref mut iter) = self.inner {
 			iter.next()
@@ -197,11 +196,11 @@ impl<'i> OffchainOverlayedChangesIter<'i> {
 
 /// Iterate by value over the prepared offchain worker storage changes.
 pub struct OffchainOverlayedChangesIntoIter {
-	inner: Option<hash_map::IntoIter<Vec<u8>,OffchainOverlayedChange>>,
+	inner: Option<hash_map::IntoIter<(Vec<u8>,Vec<u8>),OffchainOverlayedChange>>,
 }
 
 impl Iterator for OffchainOverlayedChangesIntoIter {
-	type Item = (Vec<u8>, OffchainOverlayedChange);
+	type Item = ((Vec<u8>, Vec<u8>), OffchainOverlayedChange);
 	fn next(&mut self) -> Option<Self::Item> {
 		if let Some(ref mut iter) = self.inner {
 			iter.next()
@@ -225,11 +224,11 @@ impl OffchainOverlayedChangesIntoIter {
 
 /// Iterate over all items while draining them from the collection.
 pub struct OffchainOverlayedChangesDrain<'d> {
-	inner: Option<hash_map::Drain<'d, Vec<u8>,OffchainOverlayedChange>>,
+	inner: Option<hash_map::Drain<'d, (Vec<u8>, Vec<u8>), OffchainOverlayedChange>>,
 }
 
 impl<'d> Iterator for OffchainOverlayedChangesDrain<'d> {
-	type Item = (Vec<u8>, OffchainOverlayedChange);
+	type Item = ((Vec<u8>, Vec<u8>), OffchainOverlayedChange);
 	fn next(&mut self) -> Option<Self::Item> {
 		if let Some(ref mut iter) = self.inner {
 			iter.next()
@@ -286,9 +285,13 @@ 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);
 	}
 }

substrate/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,57 @@ pub struct PendingRequest {
 	pub response_headers: Vec<(String, String)>,
 }
 
+/// Sharable "persistent" offchain storage for test.
+#[derive(Debug, Clone, Default)]
+pub struct TestPersistentOffchainDB {
+	persistent: Arc<RwLock<InMemOffchainStorage>>,
+}
+
+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(prefix, key, value);
+	}
+
+	fn remove(&mut self, prefix: &[u8], key: &[u8]) {
+		self.persistent.write().remove(prefix, key);
+	}
+
+	fn get(&self, prefix: &[u8], key: &[u8]) -> Option<Vec<u8>> {
+		self.persistent.read().get(prefix, key)
+	}
+
+	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 +122,7 @@ pub struct OffchainState {
 	pub requests: BTreeMap<RequestId, PendingRequest>,
 	expected_requests: BTreeMap<RequestId, PendingRequest>,
 	/// Persistent local storage
-	pub persistent_storage: InMemOffchainStorage,
+	pub persistent_storage: TestPersistentOffchainDB,
 	/// Local storage
 	pub local_storage: InMemOffchainStorage,
 	/// A supposedly random seed.
@@ -145,6 +197,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<RwLock<OffchainState>>) {
+		let (ext, state) = Self::new();
+		ext.0.write().persistent_storage = offchain_db;
+		(ext, state)
+	}
 }
 
 impl offchain::Externalities for TestOffchainExt {
@@ -174,17 +233,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 +255,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<Vec<u8>> {
 		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<RequestId, ()> {