resolve unresolved error nits of #7617 (#7631)

* handle executor should_panic test better

* Revert "reduce should panic, due to extended error messages"

This reverts commit c0805940184a62cd9302603ad911c3591e70a60c.

* remove excessive constraints

* remove duplicate documentation messages for error variants

* reduce T: constraints to the abs minimum

* whoops

* fewer bounds again

Co-authored-by: Bernhard Schuster <bernhard@parity.io>

substrate/Cargo.lock

@@ -7977,11 +7977,11 @@ dependencies = [
 name = "sp-allocator"
 version = "2.0.0"
 dependencies = [
- "derive_more",
  "log",
  "sp-core",
  "sp-std",
  "sp-wasm-interface",
+ "thiserror",
 ]
 
 [[package]]

substrate/client/consensus/slots/src/lib.rs

@@ -475,7 +475,7 @@ pub enum CheckedHeader<H, S> {
 
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
-pub enum Error<T> where T: SlotData + Clone + Debug + Send + Sync + 'static {
+pub enum Error<T> where T: Debug {
 	#[error("Slot duration is invalid: {0:?}")]
 	SlotDurationInvalid(SlotDuration<T>),
 }
@@ -493,7 +493,7 @@ impl<T> Deref for SlotDuration<T> {
 	}
 }
 
-impl<T: SlotData + Clone> SlotData for SlotDuration<T> {
+impl<T: SlotData> SlotData for SlotDuration<T> {
 	/// Get the slot duration in milliseconds.
 	fn slot_duration(&self) -> u64
 		where T: SlotData,

substrate/client/executor/common/src/error.rs

@@ -28,75 +28,69 @@ pub type Result<T> = std::result::Result<T, Error>;
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
 pub enum Error {
-	/// Unserializable Data
 	#[error("Unserializable data encountered")]
 	InvalidData(#[from] sp_serializer::Error),
-	/// Trap occurred during execution
+
 	#[error(transparent)]
 	Trap(#[from] wasmi::Trap),
-	/// Wasmi loading/instantiating error
+
 	#[error(transparent)]
 	Wasmi(#[from] wasmi::Error),
-	/// Error in the API. Parameter is an error message.
+
 	#[error("API Error: {0}")]
 	ApiError(String),
-	/// Method is not found
+
 	#[error("Method not found: '{0}'")]
 	MethodNotFound(String),
-	/// Code is invalid (expected single byte)
+
-	#[error("Invalid Code: '{0}'")]
+	#[error("Invalid Code (expected single byte): '{0}'")]
 	InvalidCode(String),
-	/// Could not get runtime version.
+
 	#[error("On-chain runtime does not specify version")]
 	VersionInvalid,
-	/// Externalities have failed.
+
 	#[error("Externalities error")]
 	Externalities,
-	/// Invalid index.
+
 	#[error("Invalid index provided")]
 	InvalidIndex,
-	/// Invalid return type.
+
 	#[error("Invalid type returned (should be u64)")]
 	InvalidReturn,
-	/// Runtime failed.
+
 	#[error("Runtime error")]
 	Runtime,
-	/// Runtime panicked.
+
 	#[error("Runtime panicked: {0}")]
 	RuntimePanicked(String),
-	/// Invalid memory reference.
+
 	#[error("Invalid memory reference")]
 	InvalidMemoryReference,
-	/// The runtime must provide a global named `__heap_base` of type i32 for specifying where the
+
-	/// allocator is allowed to place its data.
+	#[error("The runtime doesn't provide a global named `__heap_base` of type `i32`")]
-	#[error("The runtime doesn't provide a global named `__heap_base`")]
 	HeapBaseNotFoundOrInvalid,
-	/// The runtime WebAssembly module is not allowed to have the `start` function.
+
-	#[error("The runtime has the `start` function")]
+	#[error("The runtime must not have the `start` function defined")]
 	RuntimeHasStartFn,
-	/// Some other error occurred
+
 	#[error("Other: {0}")]
 	Other(String),
-	/// Some error occurred in the allocator
+
-	#[error("Allocation Error")]
+	#[error(transparent)]
 	Allocator(#[from] sp_allocator::Error),
-	/// Execution of a host function failed.
+
 	#[error("Host function {0} execution failed with: {1}")]
 	FunctionExecution(String, String),
-	/// No table is present.
+
-	///
-	/// Call was requested that requires table but none was present in the instance.
 	#[error("No table exported by wasm blob")]
 	NoTable,
-	/// No table entry is present.
+
-	///
-	/// Call was requested that requires specific entry in the table to be present.
 	#[error("No table entry with index {0} in wasm blob exported table")]
 	NoTableEntryWithIndex(u32),
-	/// Table entry is not a function.
+
 	#[error("Table element with index {0} is not a function in wasm blob exported table")]
 	TableElementIsNotAFunction(u32),
-	/// Function in table is null and thus cannot be called.
+
 	#[error("Table entry with index {0} in wasm blob is null")]
 	FunctionRefIsNull(u32),
 

substrate/client/executor/src/integration_tests/mod.rs

@@ -523,7 +523,7 @@ fn offchain_http_should_work(wasm_method: WasmExecutionMethod) {
 
 #[test_case(WasmExecutionMethod::Interpreted)]
 #[cfg_attr(feature = "wasmtime", test_case(WasmExecutionMethod::Compiled))]
-#[should_panic]
+#[should_panic(expected = "Allocator ran out of space")]
 fn should_trap_when_heap_exhausted(wasm_method: WasmExecutionMethod) {
 	let mut ext = TestExternalities::default();
 

substrate/client/transaction-pool/graph/src/error.rs

@@ -32,36 +32,36 @@ pub enum Error {
 	/// Transaction is not verifiable yet, but might be in the future.
 	#[error("Unknown transaction validity: {0:?}")]
 	UnknownTransaction(UnknownTransaction),
-	/// Transaction is invalid.
+
 	#[error("Invalid transaction validity: {0:?}")]
 	InvalidTransaction(InvalidTransaction),
 	/// The transaction validity returned no "provides" tag.
 	///
 	/// Such transactions are not accepted to the pool, since we use those tags
 	/// to define identity of transactions (occupance of the same "slot").
-	#[error("The transaction does not provide any tags, so the pool can't identify it.")]
+	#[error("The transaction validity returned no `provides` tags, so the pool can't identify it.")]
 	NoTagsProvided,
 
 	#[error("Temporarily Banned")]
 	TemporarilyBanned,
-	/// The transaction is already in the pool.
+
-	#[error("[{0:?}] Already imported")]
+	#[error("[{0:?}] Transaction is already in the pool")]
 	AlreadyImported(Box<dyn std::any::Any + Send>),
-	/// The transaction cannot be imported cause it's a replacement and has too low priority.
+
-	#[error("Too low priority ({0} > {1})", old, new)]
+	#[error("Transaction cannot be imported due to too low priority ({0} > {1})", old, new)]
 	TooLowPriority {
 		/// Transaction already in the pool.
 		old: Priority,
 		/// Transaction entering the pool.
 		new: Priority
 	},
-	/// Deps cycle detected and we couldn't import transaction.
+
-	#[error("Cycle Detected")]
+	#[error("Dependency cycle detected")]
 	CycleDetected,
-	/// Transaction was dropped immediately after it got inserted.
+
 	#[error("Transaction couldn't enter the pool because of the limit.")]
 	ImmediatelyDropped,
-	/// Invalid block id.
+
 	#[error("Invlaid block id: {0}")]
 	InvalidBlockId(String),
 }

substrate/primitives/allocator/Cargo.toml

@@ -17,8 +17,8 @@ targets = ["x86_64-unknown-linux-gnu"]
 sp-std = { version = "2.0.0", path = "../std", default-features = false }
 sp-core = { version = "2.0.0", path = "../core", default-features = false }
 sp-wasm-interface = { version = "2.0.0", path = "../wasm-interface", default-features = false }
-log = { version = "0.4.8", optional = true }
+log = { version = "0.4.11", optional = true }
-derive_more = { version = "0.99.2", optional = true }
+thiserror = { version = "1.0.21", optional = true }
 
 [features]
 default = [ "std" ]
@@ -27,5 +27,5 @@ std = [
 	"sp-core/std",
 	"sp-wasm-interface/std",
 	"log",
-	"derive_more",
+	"thiserror",
 ]

substrate/primitives/allocator/src/error.rs

@@ -17,23 +17,15 @@
 
 /// The error type used by the allocators.
 #[derive(sp_core::RuntimeDebug)]
-#[cfg_attr(feature = "std", derive(derive_more::Display))]
+#[cfg_attr(feature = "std", derive(thiserror::Error))]
 pub enum Error {
 	/// Someone tried to allocate more memory than the allowed maximum per allocation.
-	#[cfg_attr(feature = "std", display(fmt="Requested allocation size is too large"))]
+	#[cfg_attr(feature = "std", error("Requested allocation size is too large"))]
 	RequestedAllocationTooLarge,
 	/// Allocator run out of space.
-	#[cfg_attr(feature = "std", display(fmt="Allocator ran out of space"))]
+	#[cfg_attr(feature = "std", error("Allocator ran out of space"))]
 	AllocatorOutOfSpace,
 	/// Some other error occurred.
+	#[cfg_attr(feature = "std", error("Other: {0}"))]
 	Other(&'static str)
 }
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {
-	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-		match self {
-			_ => None,
-		}
-	}
-}