Fetching changes proof from remote nodes (#769)

* changes_trie

* changs_trie: continue

* changes_trie: adding tests

* fixed TODO

* removed obsolete ExtrinsicChanges

* encodable ChangesTrieConfiguration

* removed polkadot fle

* fixed grumbles

* ext_storage_changes_root returns u32

* moved changes trie root to digest

* removed commented code

* read storage values from native code

* fixed grumbles

* fixed grumbles

* missing comma

* key changes proof generation + query

* fix grumbles

* check that changes trie config is not changed by block.finalize()

* fixed changes trie config check

substrate/core/state-machine/src/changes_trie/changes_iterator.rs

@@ -23,7 +23,7 @@ use codec::{Decode, Encode};
 use hash_db::{HashDB, Hasher};
 use heapsize::HeapSizeOf;
 use substrate_trie::{Recorder, MemoryDB};
-use changes_trie::{Configuration, Storage};
+use changes_trie::{Configuration, RootsStorage, Storage};
 use changes_trie::input::{DigestIndex, ExtrinsicIndex, DigestIndexValue, ExtrinsicIndexValue};
 use changes_trie::storage::{TrieBackendAdapter, InMemoryStorage};
 use proving_backend::ProvingBackendEssence;
@@ -44,6 +44,8 @@ pub fn key_changes<S: Storage<H>, H: Hasher>(
 			key,
 			roots_storage: storage,
 			storage,
+			begin,
+			end,
 			surface: surface_iterator(config, max, begin, end)?,
 
 			extrinsics: Default::default(),
@@ -69,6 +71,8 @@ pub fn key_changes_proof<S: Storage<H>, H: Hasher>(
 			key,
 			roots_storage: storage.clone(),
 			storage,
+			begin,
+			end,
 			surface: surface_iterator(config, max, begin, end)?,
 
 			extrinsics: Default::default(),
@@ -89,9 +93,9 @@ pub fn key_changes_proof<S: Storage<H>, H: Hasher>(
 
 /// Check key changes proog and return changes of the key at given blocks range.
 /// `max` is the number of best known block.
-pub fn key_changes_proof_check<S: Storage<H>, H: Hasher>(
+pub fn key_changes_proof_check<S: RootsStorage<H>, H: Hasher>(
 	config: &Configuration,
-	roots_storage: &S, // TODO: use RootsStorage is only used to gather root
+	roots_storage: &S,
 	proof: Vec<Vec<u8>>,
 	begin: u64,
 	end: u64,
@@ -109,6 +113,8 @@ pub fn key_changes_proof_check<S: Storage<H>, H: Hasher>(
 			key,
 			roots_storage,
 			storage: &proof_db,
+			begin,
+			end,
 			surface: surface_iterator(config, max, begin, end)?,
 
 			extrinsics: Default::default(),
@@ -168,10 +174,12 @@ impl<'a> Iterator for SurfaceIterator<'a> {
 
 /// Drilldown iterator - receives 'digest points' from surface iterator and explores
 /// every point until extrinsic is found.
-pub struct DrilldownIteratorEssence<'a, RS: 'a + Storage<H>, S: 'a + Storage<H>, H: Hasher> {
+pub struct DrilldownIteratorEssence<'a, RS: 'a + RootsStorage<H>, S: 'a + Storage<H>, H: Hasher> {
 	key: &'a [u8],
 	roots_storage: &'a RS,
 	storage: &'a S,
+	begin: u64,
+	end: u64,
 	surface: SurfaceIterator<'a>,
 
 	extrinsics: VecDeque<(u64, u32)>,
@@ -180,7 +188,7 @@ pub struct DrilldownIteratorEssence<'a, RS: 'a + Storage<H>, S: 'a + Storage<H>,
 	_hasher: ::std::marker::PhantomData<H>,
 }
 
-impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> DrilldownIteratorEssence<'a, RS, S, H> {
+impl<'a, RS: 'a + RootsStorage<H>, S: Storage<H>, H: Hasher> DrilldownIteratorEssence<'a, RS, S, H> {
 	pub fn next<F>(&mut self, trie_reader: F) -> Option<Result<(u64, u32), String>>
 		where
 			F: FnMut(&S, H::Out, &[u8]) -> Result<Option<Vec<u8>>, String>,
@@ -202,7 +210,17 @@ impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> DrilldownIteratorEssence
 			}
 
 			if let Some((block, level)) = self.blocks.pop_front() {
-				if let Some(trie_root) = self.roots_storage.root(block)? {
+				// not having a changes trie root is an error because:
+				// we never query roots for future blocks
+				// AND trie roots for old blocks are known (both on full + light node)
+				let trie_root = self.roots_storage.root(block)?
+					.ok_or_else(|| format!("Changes trie root for block {} is not found", block))?;
+
+				// only return extrinsics for blocks before self.max
+				// most of blocks will be filtered out beore pushing to `self.blocks`
+				// here we just throwing away changes at digest blocks we're processing
+				debug_assert!(block >= self.begin, "We shall not touch digests earlier than a range' begin");
+				if block <= self.end {
 					let extrinsics_key = ExtrinsicIndex { block, key: self.key.to_vec() }.encode();
 					let extrinsics = trie_reader(&self.storage, trie_root, &extrinsics_key);
 					if let Some(extrinsics) = extrinsics? {
@@ -211,14 +229,22 @@ impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> DrilldownIteratorEssence
 							self.extrinsics.extend(extrinsics.into_iter().rev().map(|e| (block, e)));
 						}
 					}
+				}
 
-					let blocks_key = DigestIndex { block, key: self.key.to_vec() }.encode();
-					let blocks = trie_reader(&self.storage, trie_root, &blocks_key);
-					if let Some(blocks) = blocks? {
-						let blocks: Option<DigestIndexValue> = Decode::decode(&mut &blocks[..]);
-						if let Some(blocks) = blocks {
-							self.blocks.extend(blocks.into_iter().rev().map(|b| (b, level - 1)));
-						}
+				let blocks_key = DigestIndex { block, key: self.key.to_vec() }.encode();
+				let blocks = trie_reader(&self.storage, trie_root, &blocks_key);
+				if let Some(blocks) = blocks? {
+					let blocks: Option<DigestIndexValue> = Decode::decode(&mut &blocks[..]);
+					if let Some(blocks) = blocks {
+						// filter level0 blocks here because we tend to use digest blocks,
+						// AND digest block changes could also include changes for out-of-range blocks
+						let begin = self.begin;
+						let end = self.end;
+						self.blocks.extend(blocks.into_iter()
+							.rev()
+							.filter(|b| level > 1 || (*b >= begin && *b <= end))
+							.map(|b| (b, level - 1))
+						);
 					}
 				}
 
@@ -235,11 +261,11 @@ impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> DrilldownIteratorEssence
 }
 
 /// Exploring drilldown operator.
-struct DrilldownIterator<'a, RS: 'a + Storage<H>, S: 'a + Storage<H>, H: Hasher> {
+struct DrilldownIterator<'a, RS: 'a + RootsStorage<H>, S: 'a + Storage<H>, H: Hasher> {
 	essence: DrilldownIteratorEssence<'a, RS, S, H>,
 }
 
-impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> Iterator
+impl<'a, RS: 'a + RootsStorage<H>, S: Storage<H>, H: Hasher> Iterator
 	for DrilldownIterator<'a, RS, S, H>
 	where H::Out: HeapSizeOf
 {
@@ -252,12 +278,12 @@ impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> Iterator
 }
 
 /// Proving drilldown iterator.
-struct ProvingDrilldownIterator<'a, RS: 'a + Storage<H>, S: 'a + Storage<H>, H: Hasher> {
+struct ProvingDrilldownIterator<'a, RS: 'a + RootsStorage<H>, S: 'a + Storage<H>, H: Hasher> {
 	essence: DrilldownIteratorEssence<'a, RS, S, H>,
 	proof_recorder: RefCell<Recorder<H::Out>>,
 }
 
-impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> ProvingDrilldownIterator<'a, RS, S, H> {
+impl<'a, RS: 'a + RootsStorage<H>, S: Storage<H>, H: Hasher> ProvingDrilldownIterator<'a, RS, S, H> {
 	/// Consume the iterator, extracting the gathered proof in lexicographical order
 	/// by value.
 	pub fn extract_proof(self) -> Vec<Vec<u8>> {
@@ -268,7 +294,7 @@ impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> ProvingDrilldownIterator
 	}
 }
 
-impl<'a, RS: 'a + Storage<H>, S: Storage<H>, H: Hasher> Iterator for ProvingDrilldownIterator<'a, RS, S, H> where H::Out: HeapSizeOf {
+impl<'a, RS: 'a + RootsStorage<H>, S: Storage<H>, H: Hasher> Iterator for ProvingDrilldownIterator<'a, RS, S, H> where H::Out: HeapSizeOf {
 	type Item = Result<(u64, u32), String>;
 
 	fn next(&mut self) -> Option<Self::Item> {
@@ -401,9 +427,24 @@ mod tests {
 	fn drilldown_iterator_works() {
 		let (config, storage) = prepare_for_drilldown();
 		let drilldown_result = key_changes::<InMemoryStorage<Blake2Hasher>, Blake2Hasher>(
-			&config, &storage, 0, 100, 1000, &[42]);
-
+			&config, &storage, 0, 16, 16, &[42]);
 		assert_eq!(drilldown_result, Ok(vec![(8, 2), (8, 1), (6, 3), (3, 0)]));
+
+		let drilldown_result = key_changes::<InMemoryStorage<Blake2Hasher>, Blake2Hasher>(
+			&config, &storage, 0, 2, 4, &[42]);
+		assert_eq!(drilldown_result, Ok(vec![]));
+
+		let drilldown_result = key_changes::<InMemoryStorage<Blake2Hasher>, Blake2Hasher>(
+			&config, &storage, 0, 3, 4, &[42]);
+		assert_eq!(drilldown_result, Ok(vec![(3, 0)]));
+
+		let drilldown_result = key_changes::<InMemoryStorage<Blake2Hasher>, Blake2Hasher>(
+			&config, &storage, 7, 8, 8, &[42]);
+		assert_eq!(drilldown_result, Ok(vec![(8, 2), (8, 1)]));
+
+		let drilldown_result = key_changes::<InMemoryStorage<Blake2Hasher>, Blake2Hasher>(
+			&config, &storage, 5, 7, 8, &[42]);
+		assert_eq!(drilldown_result, Ok(vec![(6, 3)]));
 	}
 
 	#[test]
@@ -433,7 +474,7 @@ mod tests {
 		let (remote_config, remote_storage) = prepare_for_drilldown();
 		let remote_proof = key_changes_proof::<InMemoryStorage<Blake2Hasher>, Blake2Hasher>(
 			&remote_config, &remote_storage,
-			0, 100, 1000, &[42]).unwrap();
+			0, 16, 16, &[42]).unwrap();
 
 		// happens on local light node:
 
@@ -442,7 +483,7 @@ mod tests {
 		local_storage.clear_storage();
 		let local_result = key_changes_proof_check::<InMemoryStorage<Blake2Hasher>, Blake2Hasher>(
 			&local_config, &local_storage, remote_proof,
-			0, 100, 1000, &[42]);
+			0, 16, 16, &[42]);
 
 		// check that drilldown result is the same as if it was happening at the full node
 		assert_eq!(local_result, Ok(vec![(8, 2), (8, 1), (6, 3), (3, 0)]));

substrate/core/state-machine/src/changes_trie/mod.rs

@@ -54,10 +54,13 @@ use trie::{DBValue, trie_root};
 pub const NO_EXTRINSIC_INDEX: u32 = 0xffffffff;
 
 /// Changes trie storage. Provides access to trie roots and trie nodes.
-pub trait Storage<H: Hasher>: Send + Sync {
+pub trait RootsStorage<H: Hasher>: Send + Sync {
 	/// Get changes trie root for given block.
 	fn root(&self, block: u64) -> Result<Option<H::Out>, String>;
+}
 
+/// Changes trie storage. Provides access to trie roots and trie nodes.
+pub trait Storage<H: Hasher>: RootsStorage<H> {
 	/// Get a trie node.
 	fn get(&self, key: &H::Out) -> Result<Option<DBValue>, String>;
 }

substrate/core/state-machine/src/changes_trie/storage.rs

@@ -22,7 +22,7 @@ use trie::DBValue;
 use heapsize::HeapSizeOf;
 use trie::MemoryDB;
 use parking_lot::RwLock;
-use changes_trie::Storage;
+use changes_trie::{RootsStorage, Storage};
 use trie_backend_essence::TrieBackendStorage;
 
 #[cfg(test)]
@@ -94,11 +94,13 @@ impl<H: Hasher> InMemoryStorage<H> where H::Out: HeapSizeOf {
 	}
 }
 
-impl<H: Hasher> Storage<H> for InMemoryStorage<H> where H::Out: HeapSizeOf {
+impl<H: Hasher> RootsStorage<H> for InMemoryStorage<H> where H::Out: HeapSizeOf {
 	fn root(&self, block: u64) -> Result<Option<H::Out>, String> {
 		Ok(self.data.read().roots.get(&block).cloned())
 	}
+}
 
+impl<H: Hasher> Storage<H> for InMemoryStorage<H> where H::Out: HeapSizeOf {
 	fn get(&self, key: &H::Out) -> Result<Option<DBValue>, String> {
 		MemoryDB::<H>::get(&self.data.read().mdb, key)
 	}

substrate/core/state-machine/src/lib.rs

@@ -56,6 +56,7 @@ pub use testing::TestExternalities;
 pub use ext::Ext;
 pub use backend::Backend;
 pub use changes_trie::{Storage as ChangesTrieStorage,
+	RootsStorage as ChangesTrieRootsStorage,
 	InMemoryStorage as InMemoryChangesTrieStorage,
 	key_changes, key_changes_proof, key_changes_proof_check};
 pub use overlayed_changes::OverlayedChanges;
@@ -267,12 +268,15 @@ where
 	// proof-of-execution on light clients. And the proof is recorded by the backend which
 	// is created after OverlayedChanges
 
-	let changes_trie_config = try_read_overlay_value(
-		overlay,
-		backend,
-		well_known_keys::CHANGES_TRIE_CONFIG
-	)?;
-	set_changes_trie_config(overlay, changes_trie_config)?;
+	let init_overlay = |overlay: &mut OverlayedChanges, final_check: bool| {
+		let changes_trie_config = try_read_overlay_value(
+			overlay,
+			backend,
+			well_known_keys::CHANGES_TRIE_CONFIG
+		)?;
+		set_changes_trie_config(overlay, changes_trie_config, final_check)
+	};
+	init_overlay(overlay, false)?;
 
 	let result = {
 		let mut orig_prospective = overlay.prospective.clone();
@@ -334,6 +338,11 @@ where
 		result.map(move |out| (out, storage_delta, changes_delta))
 	};
 
+	// ensure that changes trie config has not been changed
+	if result.is_ok() {
+		init_overlay(overlay, true)?;
+	}
+
 	result.map_err(|e| Box::new(e) as _)
 }
 
@@ -429,18 +438,22 @@ where
 
 /// Sets overlayed changes' changes trie configuration. Returns error if configuration
 /// differs from previous OR config decode has failed.
-pub(crate) fn set_changes_trie_config(overlay: &mut OverlayedChanges, config: Option<Vec<u8>>) -> Result<(), Box<Error>> {
+pub(crate) fn set_changes_trie_config(overlay: &mut OverlayedChanges, config: Option<Vec<u8>>, final_check: bool) -> Result<(), Box<Error>> {
 	let config = match config {
 		Some(v) => Some(changes_trie::Configuration::decode(&mut &v[..])
 			.ok_or_else(|| Box::new("Failed to decode changes trie configuration".to_owned()) as Box<Error>)?),
 		None => None,
 	};
+
+	if final_check && overlay.changes_trie_config.is_some() != config.is_some() {
+		return Err(Box::new("Changes trie configuration change is not supported".to_owned()));
+	}
+
 	if let Some(config) = config {
 		if !overlay.set_changes_trie_config(config) {
 			return Err(Box::new("Changes trie configuration change is not supported".to_owned()));
 		}
 	}
-
 	Ok(())
 }
 
@@ -462,13 +475,18 @@ where
 #[cfg(test)]
 mod tests {
 	use std::collections::HashMap;
+	use codec::Encode;
 	use super::*;
 	use super::backend::InMemory;
 	use super::ext::Ext;
-	use super::changes_trie::InMemoryStorage as InMemoryChangesTrieStorage;
+	use super::changes_trie::{
+		InMemoryStorage as InMemoryChangesTrieStorage,
+		Configuration as ChangesTrieConfig,
+	};
 	use primitives::{Blake2Hasher};
 
 	struct DummyCodeExecutor {
+		change_changes_trie_config: bool,
 		native_available: bool,
 		native_succeeds: bool,
 		fallback_succeeds: bool,
@@ -486,6 +504,13 @@ mod tests {
 			_data: &[u8],
 			use_native: bool
 		) -> (Result<Vec<u8>, Self::Error>, bool) {
+			if self.change_changes_trie_config {
+				ext.place_storage(well_known_keys::CHANGES_TRIE_CONFIG.to_vec(), Some(ChangesTrieConfig {
+					digest_interval: 777,
+					digest_levels: 333,
+				}.encode()));
+			}
+
 			let using_native = use_native && self.native_available;
 			match (using_native, self.native_succeeds, self.fallback_succeeds) {
 				(true, true, _) | (false, _, true) =>
@@ -510,6 +535,7 @@ mod tests {
 			Some(&InMemoryChangesTrieStorage::new()),
 			&mut Default::default(),
 			&DummyCodeExecutor {
+				change_changes_trie_config: false,
 				native_available: true,
 				native_succeeds: true,
 				fallback_succeeds: true,
@@ -528,6 +554,7 @@ mod tests {
 			Some(&InMemoryChangesTrieStorage::new()),
 			&mut Default::default(),
 			&DummyCodeExecutor {
+				change_changes_trie_config: false,
 				native_available: true,
 				native_succeeds: true,
 				fallback_succeeds: false,
@@ -546,6 +573,7 @@ mod tests {
 	#[test]
 	fn prove_execution_and_proof_check_works() {
 		let executor = DummyCodeExecutor {
+			change_changes_trie_config: false,
 			native_available: true,
 			native_succeeds: true,
 			fallback_succeeds: true,
@@ -621,4 +649,22 @@ mod tests {
 		assert_eq!(local_result1, Some(vec![24]));
 		assert_eq!(local_result2, false);
 	}
+
+	#[test]
+	fn cannot_change_changes_trie_config() {
+		assert!(execute(
+			&trie_backend::tests::test_trie(),
+			Some(&InMemoryChangesTrieStorage::new()),
+			&mut Default::default(),
+			&DummyCodeExecutor {
+				change_changes_trie_config: true,
+				native_available: false,
+				native_succeeds: true,
+				fallback_succeeds: true,
+			},
+			"test",
+			&[],
+			ExecutionStrategy::NativeWhenPossible
+		).is_err());
+	}
 }

substrate/core/state-machine/src/overlayed_changes.rs

@@ -52,7 +52,6 @@ impl OverlayedChanges {
 	///
 	/// Returns false if configuration has been set already and we now trying
 	/// to install different configuration. This isn't supported now.
-	#[must_use = "Result must be checked"]
 	pub(crate) fn set_changes_trie_config(&mut self, config: ChangesTrieConfig) -> bool {
 		if let Some(ref old_config) = self.changes_trie_config {
 			// we do not support changes trie configuration' change now

substrate/core/state-machine/src/testing.rs

@@ -39,8 +39,9 @@ impl<H: Hasher> TestExternalities<H> where H::Out: HeapSizeOf {
 		let mut overlay = OverlayedChanges::default();
 		super::set_changes_trie_config(
 			&mut overlay,
-			inner.get(&CHANGES_TRIE_CONFIG.to_vec()).cloned())
-			.expect("changes trie configuration is correct in test env; qed");
+			inner.get(&CHANGES_TRIE_CONFIG.to_vec()).cloned(),
+			false,
+		).expect("changes trie configuration is correct in test env; qed");
 
 		TestExternalities {
 			inner,