sp-trie: Switch to thiserror and some other small cleanups (#10954)

* sp-trie: Switch to thiserror and some other small cleanups

* Add some extra method for converting a compact proof to a memory db

substrate/Cargo.lock

@@ -10362,6 +10362,7 @@ dependencies = [
  "sp-core",
  "sp-runtime",
  "sp-std",
+ "thiserror",
  "trie-bench",
  "trie-db",
  "trie-root",

substrate/primitives/trie/Cargo.toml

@@ -26,6 +26,7 @@ trie-db = { version = "0.23.1", default-features = false }
 trie-root = { version = "0.17.0", default-features = false }
 memory-db = { version = "0.29.0", default-features = false }
 sp-core = { version = "6.0.0", default-features = false, path = "../core" }
+thiserror = { version = "1.0.30", optional = true }
 
 [dev-dependencies]
 trie-bench = "0.30.0"
@@ -45,5 +46,6 @@ std = [
 	"trie-db/std",
 	"trie-root/std",
 	"sp-core/std",
+	"thiserror",
 ]
 memory-tracker = []

substrate/primitives/trie/src/error.rs

@@ -15,18 +15,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#[cfg(feature = "std")]
-use std::error::Error as StdError;
-#[cfg(feature = "std")]
-use std::fmt;
-
-#[derive(Debug, PartialEq, Eq, Clone)]
 /// Error for trie node decoding.
+#[derive(Debug, PartialEq, Eq, Clone)]
+#[cfg_attr(feature = "std", derive(thiserror::Error))]
 pub enum Error {
-	/// Bad format.
+	#[cfg_attr(feature = "std", error("Bad format"))]
 	BadFormat,
-	/// Decoding error.
-	Decode(codec::Error),
+	#[cfg_attr(feature = "std", error("Decoding failed: {0}"))]
+	Decode(#[cfg_attr(feature = "std", source)] codec::Error),
 }
 
 impl From<codec::Error> for Error {
@@ -34,23 +30,3 @@ impl From<codec::Error> for Error {
 		Error::Decode(x)
 	}
 }
-
-#[cfg(feature = "std")]
-impl StdError for Error {
-	fn description(&self) -> &str {
-		match self {
-			Error::BadFormat => "Bad format error",
-			Error::Decode(_) => "Decoding error",
-		}
-	}
-}
-
-#[cfg(feature = "std")]
-impl fmt::Display for Error {
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		match self {
-			Error::Decode(e) => write!(f, "Decode error: {}", e),
-			Error::BadFormat => write!(f, "Bad format"),
-		}
-	}
-}

substrate/primitives/trie/src/node_codec.rs

@@ -23,7 +23,7 @@ use codec::{Compact, Decode, Encode, Input};
 use hash_db::Hasher;
 use sp_std::{borrow::Borrow, marker::PhantomData, ops::Range, vec::Vec};
 use trie_db::{
-	self, nibble_ops,
+	nibble_ops,
 	node::{NibbleSlicePlan, NodeHandlePlan, NodePlan, Value, ValuePlan},
 	ChildReference, NodeCodec as NodeCodecT, Partial,
 };
@@ -54,9 +54,7 @@ impl<'a> ByteSliceInput<'a> {
 
 impl<'a> Input for ByteSliceInput<'a> {
 	fn remaining_len(&mut self) -> Result<Option<usize>, codec::Error> {
-		let remaining =
-			if self.offset <= self.data.len() { Some(self.data.len() - self.offset) } else { None };
-		Ok(remaining)
+		Ok(Some(self.data.len().saturating_sub(self.offset)))
 	}
 
 	fn read(&mut self, into: &mut [u8]) -> Result<(), codec::Error> {
@@ -76,7 +74,9 @@ impl<'a> Input for ByteSliceInput<'a> {
 	}
 }
 
-/// Concrete implementation of a `NodeCodec` with Parity Codec encoding, generic over the `Hasher`
+/// Concrete implementation of a [`NodeCodecT`] with SCALE encoding.
+///
+/// It is generic over `H` the [`Hasher`].
 #[derive(Default, Clone)]
 pub struct NodeCodec<H>(PhantomData<H>);
 

substrate/primitives/trie/src/storage_proof.rs

@@ -35,12 +35,6 @@ pub struct StorageProof {
 	trie_nodes: Vec<Vec<u8>>,
 }
 
-/// Storage proof in compact form.
-#[derive(Debug, PartialEq, Eq, Clone, Encode, Decode, TypeInfo)]
-pub struct CompactProof {
-	pub encoded_nodes: Vec<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 {
@@ -71,7 +65,7 @@ impl StorageProof {
 		self.trie_nodes
 	}
 
-	/// Creates a `MemoryDB` from `Self`.
+	/// Creates a [`MemoryDB`](crate::MemoryDB) from `Self`.
 	pub fn into_memory_db<H: Hasher>(self) -> crate::MemoryDB<H> {
 		self.into()
 	}
@@ -79,10 +73,7 @@ impl StorageProof {
 	/// Merges multiple storage proofs covering potentially different sets of keys into one proof
 	/// covering all keys. The merged proof output may be smaller than the aggregate size of the
 	/// input proofs due to deduplication of trie nodes.
-	pub fn merge<I>(proofs: I) -> Self
-	where
-		I: IntoIterator<Item = Self>,
-	{
+	pub fn merge(proofs: impl IntoIterator<Item = Self>) -> Self {
 		let trie_nodes = proofs
 			.into_iter()
 			.flat_map(|proof| proof.iter_nodes())
@@ -93,12 +84,11 @@ impl StorageProof {
 		Self { trie_nodes }
 	}
 
-	/// Encode as a compact proof with default
-	/// trie layout.
+	/// Encode as a compact proof with default trie layout.
 	pub fn into_compact_proof<H: Hasher>(
 		self,
 		root: H::Out,
-	) -> Result<CompactProof, crate::CompactProofError<Layout<H>>> {
+	) -> Result<CompactProof, crate::CompactProofError<H::Out, crate::Error>> {
 		crate::encode_compact::<Layout<H>>(self, root)
 	}
 
@@ -114,6 +104,22 @@ impl StorageProof {
 	}
 }
 
+impl<H: Hasher> From<StorageProof> for crate::MemoryDB<H> {
+	fn from(proof: StorageProof) -> Self {
+		let mut db = crate::MemoryDB::default();
+		proof.iter_nodes().for_each(|n| {
+			db.insert(crate::EMPTY_PREFIX, &n);
+		});
+		db
+	}
+}
+
+/// Storage proof in compact form.
+#[derive(Debug, PartialEq, Eq, Clone, Encode, Decode, TypeInfo)]
+pub struct CompactProof {
+	pub encoded_nodes: Vec<Vec<u8>>,
+}
+
 impl CompactProof {
 	/// Return an iterator on the compact encoded nodes.
 	pub fn iter_compact_encoded_nodes(&self) -> impl Iterator<Item = &[u8]> {
@@ -121,13 +127,10 @@ impl CompactProof {
 	}
 
 	/// Decode to a full storage_proof.
-	///
-	/// Method use a temporary `HashDB`, and `sp_trie::decode_compact`
-	/// is often better.
 	pub fn to_storage_proof<H: Hasher>(
 		&self,
 		expected_root: Option<&H::Out>,
-	) -> Result<(StorageProof, H::Out), crate::CompactProofError<Layout<H>>> {
+	) -> Result<(StorageProof, H::Out), crate::CompactProofError<H::Out, crate::Error>> {
 		let mut db = crate::MemoryDB::<H>::new(&[]);
 		let root = crate::decode_compact::<Layout<H>, _, _>(
 			&mut db,
@@ -144,6 +147,25 @@ impl CompactProof {
 			root,
 		))
 	}
+
+	/// Convert self into a [`MemoryDB`](crate::MemoryDB).
+	///
+	/// `expected_root` is the expected root of this compact proof.
+	///
+	/// Returns the memory db and the root of the trie.
+	pub fn to_memory_db<H: Hasher>(
+		&self,
+		expected_root: Option<&H::Out>,
+	) -> Result<(crate::MemoryDB<H>, H::Out), crate::CompactProofError<H::Out, crate::Error>> {
+		let mut db = crate::MemoryDB::<H>::new(&[]);
+		let root = crate::decode_compact::<Layout<H>, _, _>(
+			&mut db,
+			self.iter_compact_encoded_nodes(),
+			expected_root,
+		)?;
+
+		Ok((db, root))
+	}
 }
 
 /// An iterator over trie nodes constructed from a storage proof. The nodes are not guaranteed to
@@ -165,13 +187,3 @@ impl Iterator for StorageProofNodeIterator {
 		self.inner.next()
 	}
 }
-
-impl<H: Hasher> From<StorageProof> for crate::MemoryDB<H> {
-	fn from(proof: StorageProof) -> Self {
-		let mut db = crate::MemoryDB::default();
-		for item in proof.iter_nodes() {
-			db.insert(crate::EMPTY_PREFIX, &item);
-		}
-		db
-	}
-}

substrate/primitives/trie/src/trie_codec.rs

@@ -20,80 +20,34 @@
 //! This uses compact proof from trie crate and extends
 //! it to substrate specific layout and child trie system.
 
-use crate::{
-	CompactProof, HashDBT, StorageProof, TrieConfiguration, TrieError, TrieHash, EMPTY_PREFIX,
-};
+use crate::{CompactProof, HashDBT, StorageProof, TrieConfiguration, TrieHash, EMPTY_PREFIX};
 use sp_std::{boxed::Box, vec::Vec};
-#[cfg(feature = "std")]
-use std::error::Error as StdError;
-#[cfg(feature = "std")]
-use std::fmt;
-use trie_db::Trie;
+use trie_db::{CError, Trie};
 
 /// Error for trie node decoding.
-pub enum Error<L: TrieConfiguration> {
-	/// Verification failed due to root mismatch.
-	RootMismatch(TrieHash<L>, TrieHash<L>),
-	/// Missing nodes in proof.
+#[derive(Debug)]
+#[cfg_attr(feature = "std", derive(thiserror::Error))]
+pub enum Error<H, CodecError> {
+	#[cfg_attr(feature = "std", error("Invalid root {0:x?}, expected {1:x?}"))]
+	RootMismatch(H, H),
+	#[cfg_attr(feature = "std", error("Missing nodes in the proof"))]
 	IncompleteProof,
-	/// Compact node is not needed.
+	#[cfg_attr(feature = "std", error("Child node content with no root in proof"))]
 	ExtraneousChildNode,
-	/// Child content with root not in proof.
-	ExtraneousChildProof(TrieHash<L>),
-	/// Bad child trie root.
+	#[cfg_attr(feature = "std", error("Proof of child trie {0:x?} not in parent proof"))]
+	ExtraneousChildProof(H),
+	#[cfg_attr(feature = "std", error("Invalid root {0:x?}, expected {1:x?}"))]
 	InvalidChildRoot(Vec<u8>, Vec<u8>),
-	/// Errors from trie crate.
-	TrieError(Box<TrieError<L>>),
+	#[cfg_attr(feature = "std", error("Trie error: {0:?}"))]
+	TrieError(Box<trie_db::TrieError<H, CodecError>>),
 }
 
-impl<L: TrieConfiguration> From<Box<TrieError<L>>> for Error<L> {
-	fn from(error: Box<TrieError<L>>) -> Self {
+impl<H, CodecError> From<Box<trie_db::TrieError<H, CodecError>>> for Error<H, CodecError> {
+	fn from(error: Box<trie_db::TrieError<H, CodecError>>) -> Self {
 		Error::TrieError(error)
 	}
 }
 
-#[cfg(feature = "std")]
-impl<L: TrieConfiguration> StdError for Error<L> {
-	fn description(&self) -> &str {
-		match self {
-			Error::InvalidChildRoot(..) => "Invalid child root error",
-			Error::TrieError(..) => "Trie db error",
-			Error::RootMismatch(..) => "Trie db error",
-			Error::IncompleteProof => "Incomplete proof",
-			Error::ExtraneousChildNode => "Extraneous child node",
-			Error::ExtraneousChildProof(..) => "Extraneous child proof",
-		}
-	}
-}
-
-#[cfg(feature = "std")]
-impl<L: TrieConfiguration> fmt::Debug for Error<L> {
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		<Self as fmt::Display>::fmt(&self, f)
-	}
-}
-
-#[cfg(feature = "std")]
-impl<L: TrieConfiguration> fmt::Display for Error<L> {
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		match self {
-			Error::InvalidChildRoot(k, v) => write!(f, "InvalidChildRoot at {:x?}: {:x?}", k, v),
-			Error::TrieError(e) => write!(f, "Trie error: {}", e),
-			Error::IncompleteProof => write!(f, "Incomplete proof"),
-			Error::ExtraneousChildNode => write!(f, "Child node content with no root in proof"),
-			Error::ExtraneousChildProof(root) => {
-				write!(f, "Proof of child trie {:x?} not in parent proof", root.as_ref())
-			},
-			Error::RootMismatch(root, expected) => write!(
-				f,
-				"Verification error, root is {:x?}, expected: {:x?}",
-				root.as_ref(),
-				expected.as_ref(),
-			),
-		}
-	}
-}
-
 /// Decode a compact proof.
 ///
 /// Takes as input a destination `db` for decoded node and `encoded`
@@ -105,7 +59,7 @@ pub fn decode_compact<'a, L, DB, I>(
 	db: &mut DB,
 	encoded: I,
 	expected_root: Option<&TrieHash<L>>,
-) -> Result<TrieHash<L>, Error<L>>
+) -> Result<TrieHash<L>, Error<TrieHash<L>, CError<L>>>
 where
 	L: TrieConfiguration,
 	DB: HashDBT<L::Hash, trie_db::DBValue> + hash_db::HashDBRef<L::Hash, trie_db::DBValue>,
@@ -195,7 +149,10 @@ where
 /// Then parse all child trie root and compress main trie content first
 /// then all child trie contents.
 /// Child trie are ordered by the order of their roots in the top trie.
-pub fn encode_compact<L>(proof: StorageProof, root: TrieHash<L>) -> Result<CompactProof, Error<L>>
+pub fn encode_compact<L>(
+	proof: StorageProof,
+	root: TrieHash<L>,
+) -> Result<CompactProof, Error<TrieHash<L>, CError<L>>>
 where
 	L: TrieConfiguration,
 {