Fix the undeterministic storage proof recorded for the same execution

client/executor/src/native_executor.rs

@@ -462,8 +462,9 @@ impl RuntimeSpawn for RuntimeInstanceSpawn {
 					// https://github.com/paritytech/substrate/issues/7354
 					let mut instance = match module.new_instance() {
 						Ok(instance) => instance,
-						Err(error) =>
-							panic!("failed to create new instance from module: {}", error),
+						Err(error) => {
+							panic!("failed to create new instance from module: {}", error)
+						},
 					};
 
 					match instance

client/offchain/src/api/http.rs

@@ -432,8 +432,9 @@ impl HttpApi {
 							);
 						},
 						None => {}, // can happen if we detected an IO error when sending the body
-						_ =>
-							tracing::error!(target: "offchain-worker::http", "State mismatch between the API and worker"),
+						_ => {
+							tracing::error!(target: "offchain-worker::http", "State mismatch between the API and worker")
+						},
 					}
 				},
 
@@ -443,8 +444,9 @@ impl HttpApi {
 						self.requests.insert(id, HttpApiRequest::Fail(error));
 					},
 					None => {}, // can happen if we detected an IO error when sending the body
-					_ =>
-						tracing::error!(target: "offchain-worker::http", "State mismatch between the API and worker"),
+					_ => {
+						tracing::error!(target: "offchain-worker::http", "State mismatch between the API and worker")
+					},
 				},
 
 				None => {

primitives/state-machine/src/proving_backend.rs

@@ -33,7 +33,7 @@ use sp_trie::{
 	LayoutV1, MemoryDB, Recorder, StorageProof,
 };
 use std::{
-	collections::{hash_map::Entry, HashMap},
+	collections::{btree_map::Entry, BTreeMap},
 	sync::Arc,
 };
 
@@ -115,7 +115,7 @@ where
 #[derive(Default)]
 struct ProofRecorderInner<Hash> {
 	/// All the records that we have stored so far.
-	records: HashMap<Hash, Option<DBValue>>,
+	records: BTreeMap<Hash, Option<DBValue>>,
 	/// The encoded size of all recorded values.
 	encoded_size: usize,
 }
@@ -126,7 +126,7 @@ pub struct ProofRecorder<Hash> {
 	inner: Arc<RwLock<ProofRecorderInner<Hash>>>,
 }
 
-impl<Hash: std::hash::Hash + Eq> ProofRecorder<Hash> {
+impl<Hash: std::hash::Hash + Eq + Ord> ProofRecorder<Hash> {
 	/// Record the given `key` => `val` combination.
 	pub fn record(&self, key: Hash, val: Option<DBValue>) {
 		let mut inner = self.inner.write();
@@ -158,15 +158,13 @@ impl<Hash: std::hash::Hash + Eq> ProofRecorder<Hash> {
 
 	/// Convert into a [`StorageProof`].
 	pub fn to_storage_proof(&self) -> StorageProof {
-		let trie_nodes = self
-			.inner
-			.read()
-			.records
-			.iter()
-			.filter_map(|(_k, v)| v.as_ref().map(|v| v.to_vec()))
-			.collect();
-
-		StorageProof::new(trie_nodes)
+		StorageProof::new(
+			self.inner
+				.read()
+				.records
+				.iter()
+				.filter_map(|(_k, v)| v.as_ref().map(|v| v.to_vec())),
+		)
 	}
 
 	/// Reset the internal state.
@@ -181,7 +179,9 @@ impl<Hash: std::hash::Hash + Eq> ProofRecorder<Hash> {
 /// These can be sent to remote node and used as a proof of execution.
 pub struct ProvingBackend<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hasher>(
 	TrieBackend<ProofRecorderBackend<'a, S, H>, H>,
-);
+)
+where
+	H::Out: Ord;
 
 /// Trie backend storage with its proof recorder.
 pub struct ProofRecorderBackend<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hasher> {
@@ -191,7 +191,7 @@ pub struct ProofRecorderBackend<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hashe
 
 impl<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hasher> ProvingBackend<'a, S, H>
 where
-	H::Out: Codec,
+	H::Out: Codec + Ord,
 {
 	/// Create new proving backend.
 	pub fn new(backend: &'a TrieBackend<S, H>) -> Self {
@@ -231,6 +231,8 @@ where
 
 impl<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hasher> TrieBackendStorage<H>
 	for ProofRecorderBackend<'a, S, H>
+where
+	H::Out: Ord,
 {
 	type Overlay = S::Overlay;
 
@@ -245,8 +247,9 @@ impl<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hasher> TrieBackendStorage<H>
 	}
 }
 
-impl<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hasher> std::fmt::Debug
-	for ProvingBackend<'a, S, H>
+impl<'a, S: 'a + TrieBackendStorage<H>, H: 'a + Hasher> std::fmt::Debug for ProvingBackend<'a, S, H>
+where
+	H::Out: Ord,
 {
 	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
 		write!(f, "ProvingBackend")
@@ -393,6 +396,7 @@ mod tests {
 		proving_backend::create_proof_check_backend, trie_backend::tests::test_trie,
 		InMemoryBackend,
 	};
+	use sp_core::H256;
 	use sp_runtime::traits::BlakeTwo256;
 	use sp_trie::PrefixedMemoryDB;
 
@@ -426,7 +430,6 @@ mod tests {
 
 	#[test]
 	fn proof_is_invalid_when_does_not_contains_root() {
-		use sp_core::H256;
 		let result = create_proof_check_backend::<BlakeTwo256>(
 			H256::from_low_u64_be(1),
 			StorageProof::empty(),
@@ -582,4 +585,35 @@ mod tests {
 		assert!(backend.storage(b"doesnotexist2").unwrap().is_none());
 		check_estimation(&backend);
 	}
+
+	#[test]
+	fn proof_recorded_for_same_execution_should_be_deterministic() {
+		let storage_changes = vec![
+			(H256::random(), Some(b"value1".to_vec())),
+			(H256::random(), Some(b"value2".to_vec())),
+			(H256::random(), Some(b"value3".to_vec())),
+			(H256::random(), Some(b"value4".to_vec())),
+			(H256::random(), Some(b"value5".to_vec())),
+			(H256::random(), Some(b"value6".to_vec())),
+			(H256::random(), Some(b"value7".to_vec())),
+			(H256::random(), Some(b"value8".to_vec())),
+		];
+
+		let proof_recorder =
+			ProofRecorder::<H256> { inner: Arc::new(RwLock::new(ProofRecorderInner::default())) };
+		storage_changes
+			.clone()
+			.into_iter()
+			.for_each(|(key, val)| proof_recorder.record(key, val));
+		let proof1 = proof_recorder.to_storage_proof();
+
+		let proof_recorder =
+			ProofRecorder::<H256> { inner: Arc::new(RwLock::new(ProofRecorderInner::default())) };
+		storage_changes
+			.into_iter()
+			.for_each(|(key, val)| proof_recorder.record(key, val));
+		let proof2 = proof_recorder.to_storage_proof();
+
+		assert_eq!(proof1, proof2);
+	}
 }

primitives/trie/src/storage_proof.rs

@@ -18,7 +18,7 @@
 use codec::{Decode, Encode};
 use hash_db::{HashDB, Hasher};
 use scale_info::TypeInfo;
-use sp_std::vec::Vec;
+use sp_std::{collections::btree_set::BTreeSet, iter::IntoIterator, vec::Vec};
 // Note that `LayoutV1` usage here (proof compaction) is compatible
 // with `LayoutV0`.
 use crate::LayoutV1 as Layout;
@@ -32,27 +32,27 @@ use crate::LayoutV1 as Layout;
 /// the serialized nodes and performing the key lookups.
 #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode, TypeInfo)]
 pub struct StorageProof {
-	trie_nodes: Vec<Vec<u8>>,
+	trie_nodes: BTreeSet<Vec<u8>>,
 }
 
 /// Storage proof in compact form.
 #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode, TypeInfo)]
 pub struct CompactProof {
-	pub encoded_nodes: Vec<Vec<u8>>,
+	pub encoded_nodes: BTreeSet<Vec<u8>>,
 }
 
 impl StorageProof {
 	/// Constructs a storage proof from a subset of encoded trie nodes in a storage backend.
-	pub fn new(trie_nodes: Vec<Vec<u8>>) -> Self {
-		StorageProof { trie_nodes }
+	pub fn new(trie_nodes: impl IntoIterator<Item = Vec<u8>>) -> Self {
+		StorageProof { trie_nodes: BTreeSet::from_iter(trie_nodes) }
 	}
 
 	/// Returns a new empty proof.
 	///
 	/// An empty proof is capable of only proving trivial statements (ie. that an empty set of
 	/// key-value pairs exist in storage).
 	pub fn empty() -> Self {
-		StorageProof { trie_nodes: Vec::new() }
+		StorageProof { trie_nodes: BTreeSet::new() }
 	}
 
 	/// Returns whether this is an empty proof.
@@ -67,7 +67,7 @@ impl StorageProof {
 	}
 
 	/// Convert into plain node vector.
-	pub fn into_nodes(self) -> Vec<Vec<u8>> {
+	pub fn into_nodes(self) -> BTreeSet<Vec<u8>> {
 		self.trie_nodes
 	}
 
@@ -86,7 +86,7 @@ impl StorageProof {
 		let trie_nodes = proofs
 			.into_iter()
 			.flat_map(|proof| proof.iter_nodes())
-			.collect::<sp_std::collections::btree_set::BTreeSet<_>>()
+			.collect::<BTreeSet<_>>()
 			.into_iter()
 			.collect();
 
@@ -135,12 +135,13 @@ impl CompactProof {
 			expected_root,
 		)?;
 		Ok((
-			StorageProof::new(
-				db.drain()
-					.into_iter()
-					.filter_map(|kv| if (kv.1).1 > 0 { Some((kv.1).0) } else { None })
-					.collect(),
-			),
+			StorageProof::new(db.drain().into_iter().filter_map(|kv| {
+				if (kv.1).1 > 0 {
+					Some((kv.1).0)
+				} else {
+					None
+				}
+			})),
 			root,
 		))
 	}
@@ -149,7 +150,7 @@ impl CompactProof {
 /// An iterator over trie nodes constructed from a storage proof. The nodes are not guaranteed to
 /// be traversed in any particular order.
 pub struct StorageProofNodeIterator {
-	inner: <Vec<Vec<u8>> as IntoIterator>::IntoIter,
+	inner: <BTreeSet<Vec<u8>> as IntoIterator>::IntoIter,
 }
 
 impl StorageProofNodeIterator {

primitives/trie/src/trie_codec.rs

@@ -23,7 +23,7 @@
 use crate::{
 	CompactProof, HashDBT, StorageProof, TrieConfiguration, TrieError, TrieHash, EMPTY_PREFIX,
 };
-use sp_std::{boxed::Box, vec::Vec};
+use sp_std::{boxed::Box, collections::btree_set::BTreeSet, vec::Vec};
 #[cfg(feature = "std")]
 use std::error::Error as StdError;
 #[cfg(feature = "std")]
@@ -246,5 +246,5 @@ where
 		compact_proof.extend(child_proof);
 	}
 
-	Ok(CompactProof { encoded_nodes: compact_proof })
+	Ok(CompactProof { encoded_nodes: BTreeSet::from_iter(compact_proof) })
 }