Some error improvements (#1956)

* Use `HeaderChainError` in parachains module

* Use MessageProofError instead of 'static str in some places

* Avoid implementing Into<'static str> for some errors

We avoid deriving `Debug` for the structs that we use in the runtime and
we derive `RuntimeDebug` instead in order to avoid bloating th eruntime
with static strs. But implementing `Into<'static str>` does the same. So
in some places it makes sense to replace `Into<'static str>` with `Debug`.

* Move the messages error definition

Move the messages error definition outside of `mod target`

bridges/primitives/runtime/src/lib.rs

@@ -531,6 +531,37 @@ macro_rules! generate_static_str_provider {
 	};
 }
 
+#[derive(Encode, Decode, Clone, Eq, PartialEq, PalletError, TypeInfo)]
+#[scale_info(skip_type_params(T))]
+pub struct StrippableError<T> {
+	_phantom_data: sp_std::marker::PhantomData<T>,
+	#[codec(skip)]
+	#[cfg(feature = "std")]
+	message: String,
+}
+
+impl<T: Debug> From<T> for StrippableError<T> {
+	fn from(err: T) -> Self {
+		Self {
+			_phantom_data: Default::default(),
+			#[cfg(feature = "std")]
+			message: format!("{:?}", err),
+		}
+	}
+}
+
+impl<T> Debug for StrippableError<T> {
+	#[cfg(feature = "std")]
+	fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result {
+		f.write_str(&self.message)
+	}
+
+	#[cfg(not(feature = "std"))]
+	fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result {
+		f.write_str("Stripped error")
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;

bridges/primitives/runtime/src/storage_proof.rs

@@ -16,9 +16,11 @@
 
 //! Logic for checking Substrate storage proofs.
 
-use codec::Decode;
+use crate::StrippableError;
+use codec::{Decode, Encode};
+use frame_support::PalletError;
 use hash_db::{HashDB, Hasher, EMPTY_PREFIX};
-use sp_runtime::RuntimeDebug;
+use scale_info::TypeInfo;
 use sp_std::{boxed::Box, collections::btree_set::BTreeSet, vec::Vec};
 use sp_trie::{
 	read_trie_value, LayoutV1, MemoryDB, Recorder, StorageProof, Trie, TrieConfiguration,
@@ -116,14 +118,32 @@ where
 	/// read, but decoding fails, this function returns an error.
 	pub fn read_and_decode_value<T: Decode>(&mut self, key: &[u8]) -> Result<Option<T>, Error> {
 		self.read_value(key).and_then(|v| {
-			v.map(|v| T::decode(&mut &v[..]).map_err(Error::StorageValueDecodeFailed))
+			v.map(|v| T::decode(&mut &v[..]).map_err(|e| Error::StorageValueDecodeFailed(e.into())))
 				.transpose()
 		})
 	}
+
+	/// Reads and decodes a value from the available subset of storage. If the value cannot be read
+	/// due to an incomplete or otherwise invalid proof, or if the value is `None`, this function
+	/// returns an error. If value is read, but decoding fails, this function returns an error.
+	pub fn read_and_decode_mandatory_value<T: Decode>(&mut self, key: &[u8]) -> Result<T, Error> {
+		self.read_and_decode_value(key)?.ok_or(Error::StorageValueEmpty)
+	}
+
+	/// Reads and decodes a value from the available subset of storage. If the value cannot be read
+	/// due to an incomplete or otherwise invalid proof, this function returns `Ok(None)`.
+	/// If value is read, but decoding fails, this function returns an error.
+	pub fn read_and_decode_opt_value<T: Decode>(&mut self, key: &[u8]) -> Result<Option<T>, Error> {
+		match self.read_and_decode_value(key) {
+			Ok(outbound_lane_data) => Ok(outbound_lane_data),
+			Err(Error::StorageValueUnavailable) => Ok(None),
+			Err(e) => Err(e),
+		}
+	}
 }
 
 /// Storage proof related errors.
-#[derive(Clone, Eq, PartialEq, RuntimeDebug)]
+#[derive(Encode, Decode, Clone, Eq, PartialEq, PalletError, Debug, TypeInfo)]
 pub enum Error {
 	/// Duplicate trie nodes are found in the proof.
 	DuplicateNodesInProof,
@@ -133,21 +153,10 @@ pub enum Error {
 	StorageRootMismatch,
 	/// Unable to reach expected storage value using provided trie nodes.
 	StorageValueUnavailable,
+	/// The storage value is `None`.
+	StorageValueEmpty,
 	/// Failed to decode storage value.
-	StorageValueDecodeFailed(codec::Error),
-}
-
-impl From<Error> for &'static str {
-	fn from(err: Error) -> &'static str {
-		match err {
-			Error::DuplicateNodesInProof => "Storage proof contains duplicate nodes",
-			Error::UnusedNodesInTheProof => "Storage proof contains unused nodes",
-			Error::StorageRootMismatch => "Storage root is missing from the storage proof",
-			Error::StorageValueUnavailable => "Storage value is missing from the storage proof",
-			Error::StorageValueDecodeFailed(_) =>
-				"Failed to decode storage value from the storage proof",
-		}
-	}
+	StorageValueDecodeFailed(StrippableError<codec::Error>),
 }
 
 /// Return valid storage proof and state root.
@@ -155,7 +164,6 @@ impl From<Error> for &'static str {
 /// NOTE: This should only be used for **testing**.
 #[cfg(feature = "std")]
 pub fn craft_valid_storage_proof() -> (sp_core::H256, RawStorageProof) {
-	use codec::Encode;
 	use sp_state_machine::{backend::Backend, prove_read, InMemoryBackend};
 
 	let state_version = sp_runtime::StateVersion::default();