From 4ccf6672b1340920132ec6ee2cc2d24668520801 Mon Sep 17 00:00:00 2001
From: Bernhard Schuster <bernhard@ahoi.io>
Date: Mon, 30 Nov 2020 16:53:33 +0100
Subject: [PATCH] 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 | 2 +-
substrate/client/consensus/slots/src/lib.rs | 4 +-
substrate/client/executor/common/src/error.rs | 56 +++++++++----------
.../executor/src/integration_tests/mod.rs | 2 +-
.../transaction-pool/graph/src/error.rs | 20 +++----
substrate/primitives/allocator/Cargo.toml | 6 +-
substrate/primitives/allocator/src/error.rs | 16 ++----
7 files changed, 46 insertions(+), 60 deletions(-)
diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock
index 446b4442cd0..190dbaf7179 100644
--- a/substrate/Cargo.lock
+++ b/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]]
diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs
index ab8fc16007c..571766bc44b 100644
--- a/substrate/client/consensus/slots/src/lib.rs
+++ b/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,
diff --git a/substrate/client/executor/common/src/error.rs b/substrate/client/executor/common/src/error.rs
index caf6159da07..df0eaf8cc26 100644
--- a/substrate/client/executor/common/src/error.rs
+++ b/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`")]
+
+ #[error("The runtime doesn't provide a global named `__heap_base` of type `i32`")]
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),
diff --git a/substrate/client/executor/src/integration_tests/mod.rs b/substrate/client/executor/src/integration_tests/mod.rs
index 62368441f58..d41784f5aa0 100644
--- a/substrate/client/executor/src/integration_tests/mod.rs
+++ b/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();
diff --git a/substrate/client/transaction-pool/graph/src/error.rs b/substrate/client/transaction-pool/graph/src/error.rs
index b599715920b..01fcb9f8dc9 100644
--- a/substrate/client/transaction-pool/graph/src/error.rs
+++ b/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),
}
diff --git a/substrate/primitives/allocator/Cargo.toml b/substrate/primitives/allocator/Cargo.toml
index 93991a4aeb2..130723730c4 100644
--- a/substrate/primitives/allocator/Cargo.toml
+++ b/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 }
-derive_more = { version = "0.99.2", optional = true }
+log = { version = "0.4.11", 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",
]
diff --git a/substrate/primitives/allocator/src/error.rs b/substrate/primitives/allocator/src/error.rs
index 7b634af4d5b..77c911cef9d 100644
--- a/substrate/primitives/allocator/src/error.rs
+++ b/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,
- }
- }
-}
--
GitLab