diff --git a/Cargo.lock b/Cargo.lock index e883ba8c606e7..a613b7ba03686 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8095,7 +8095,6 @@ dependencies = [ name = "sc-rpc-api" version = "0.10.0-dev" dependencies = [ - "derive_more", "futures 0.3.16", "jsonrpc-core", "jsonrpc-core-client", @@ -8113,6 +8112,7 @@ dependencies = [ "sp-runtime", "sp-tracing", "sp-version", + "thiserror", ] [[package]] diff --git a/client/rpc-api/Cargo.toml b/client/rpc-api/Cargo.toml index b0d7c28b788ea..86fd24c24e7fa 100644 --- a/client/rpc-api/Cargo.toml +++ b/client/rpc-api/Cargo.toml @@ -14,7 +14,6 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "2.0.0" } -derive_more = "0.99.2" futures = "0.3.16" jsonrpc-core = "18.0.0" jsonrpc-core-client = "18.0.0" @@ -22,6 +21,8 @@ jsonrpc-derive = "18.0.0" jsonrpc-pubsub = "18.0.0" log = "0.4.8" parking_lot = "0.11.1" +thiserror = "1.0" + sp-core = { version = "4.0.0-dev", path = "../../primitives/core" } sp-version = { version = "4.0.0-dev", path = "../../primitives/version" } sp-runtime = { path = "../../primitives/runtime", version = "4.0.0-dev" } diff --git a/client/rpc-api/src/author/error.rs b/client/rpc-api/src/author/error.rs index 249c8df39518d..c7e3ccffabbb7 100644 --- a/client/rpc-api/src/author/error.rs +++ b/client/rpc-api/src/author/error.rs @@ -29,51 +29,38 @@ pub type Result = std::result::Result; pub type FutureResult = jsonrpc_core::BoxFuture>; /// Author RPC errors. -#[derive(Debug, derive_more::Display, derive_more::From)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Client error. - #[display(fmt = "Client error: {}", _0)] - #[from(ignore)] + #[error("Client error: {}", .0)] Client(Box), /// Transaction pool error, - #[display(fmt = "Transaction pool error: {}", _0)] - Pool(sc_transaction_pool_api::error::Error), + #[error("Transaction pool error: {}", .0)] + Pool(#[from] sc_transaction_pool_api::error::Error), /// Verification error - #[display(fmt = "Extrinsic verification error: {}", _0)] - #[from(ignore)] + #[error("Extrinsic verification error: {}", .0)] Verification(Box), /// Incorrect extrinsic format. - #[display(fmt = "Invalid extrinsic format: {}", _0)] - BadFormat(codec::Error), + #[error("Invalid extrinsic format: {}", .0)] + BadFormat(#[from] codec::Error), /// Incorrect seed phrase. - #[display(fmt = "Invalid seed phrase/SURI")] + #[error("Invalid seed phrase/SURI")] BadSeedPhrase, /// Key type ID has an unknown format. - #[display(fmt = "Invalid key type ID format (should be of length four)")] + #[error("Invalid key type ID format (should be of length four)")] BadKeyType, /// Key type ID has some unsupported crypto. - #[display(fmt = "The crypto of key type ID is unknown")] + #[error("The crypto of key type ID is unknown")] UnsupportedKeyType, /// Some random issue with the key store. Shouldn't happen. - #[display(fmt = "The key store is unavailable")] + #[error("The key store is unavailable")] KeyStoreUnavailable, /// Invalid session keys encoding. - #[display(fmt = "Session keys are not encoded correctly")] + #[error("Session keys are not encoded correctly")] InvalidSessionKeys, /// Call to an unsafe RPC was denied. - UnsafeRpcCalled(crate::policy::UnsafeRpcError), -} - -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Error::Client(ref err) => Some(&**err), - Error::Pool(ref err) => Some(err), - Error::Verification(ref err) => Some(&**err), - Error::UnsafeRpcCalled(ref err) => Some(err), - _ => None, - } - } + #[error(transparent)] + UnsafeRpcCalled(#[from] crate::policy::UnsafeRpcError), } /// Base code for all authorship errors. diff --git a/client/rpc-api/src/chain/error.rs b/client/rpc-api/src/chain/error.rs index b1ce800d27312..c7f14b2dfc168 100644 --- a/client/rpc-api/src/chain/error.rs +++ b/client/rpc-api/src/chain/error.rs @@ -28,24 +28,16 @@ pub type Result = std::result::Result; pub type FutureResult = jsonrpc_core::BoxFuture>; /// Chain RPC errors. -#[derive(Debug, derive_more::Display, derive_more::From)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Client error. - #[display(fmt = "Client error: {}", _0)] - Client(Box), + #[error("Client error: {}", .0)] + Client(#[from] Box), /// Other error type. + #[error("{0}")] Other(String), } -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Error::Client(ref err) => Some(&**err), - _ => None, - } - } -} - /// Base error code for all chain errors. const BASE_ERROR: i64 = 3000; diff --git a/client/rpc-api/src/offchain/error.rs b/client/rpc-api/src/offchain/error.rs index f2567707bc5f2..6b8e2bfe189b1 100644 --- a/client/rpc-api/src/offchain/error.rs +++ b/client/rpc-api/src/offchain/error.rs @@ -24,22 +24,14 @@ use jsonrpc_core as rpc; pub type Result = std::result::Result; /// Offchain RPC errors. -#[derive(Debug, derive_more::Display, derive_more::From)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Unavailable storage kind error. - #[display(fmt = "This storage kind is not available yet.")] + #[error("This storage kind is not available yet.")] UnavailableStorageKind, /// Call to an unsafe RPC was denied. - UnsafeRpcCalled(crate::policy::UnsafeRpcError), -} - -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::UnsafeRpcCalled(err) => Some(err), - _ => None, - } - } + #[error(transparent)] + UnsafeRpcCalled(#[from] crate::policy::UnsafeRpcError), } /// Base error code for all offchain errors. diff --git a/client/rpc-api/src/state/error.rs b/client/rpc-api/src/state/error.rs index e30757f0dd39a..d700863476329 100644 --- a/client/rpc-api/src/state/error.rs +++ b/client/rpc-api/src/state/error.rs @@ -28,13 +28,13 @@ pub type Result = std::result::Result; pub type FutureResult = jsonrpc_core::BoxFuture>; /// State RPC errors. -#[derive(Debug, derive_more::Display, derive_more::From)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Client error. - #[display(fmt = "Client error: {}", _0)] - Client(Box), + #[error("Client error: {}", .0)] + Client(#[from] Box), /// Provided block range couldn't be resolved to a list of blocks. - #[display(fmt = "Cannot resolve a block range ['{:?}' ... '{:?}]. {}", from, to, details)] + #[error("Cannot resolve a block range ['{:?}' ... '{:?}]. {}", .from, .to, .details)] InvalidBlockRange { /// Beginning of the block range. from: String, @@ -44,7 +44,7 @@ pub enum Error { details: String, }, /// Provided count exceeds maximum value. - #[display(fmt = "count exceeds maximum value. value: {}, max: {}", value, max)] + #[error("count exceeds maximum value. value: {}, max: {}", .value, .max)] InvalidCount { /// Provided value value: u32, @@ -52,16 +52,8 @@ pub enum Error { max: u32, }, /// Call to an unsafe RPC was denied. - UnsafeRpcCalled(crate::policy::UnsafeRpcError), -} - -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Error::Client(ref err) => Some(&**err), - _ => None, - } - } + #[error(transparent)] + UnsafeRpcCalled(#[from] crate::policy::UnsafeRpcError), } /// Base code for all state errors. diff --git a/client/rpc-api/src/system/error.rs b/client/rpc-api/src/system/error.rs index b16a7abb6ea52..4ba5125d82bc1 100644 --- a/client/rpc-api/src/system/error.rs +++ b/client/rpc-api/src/system/error.rs @@ -25,17 +25,16 @@ use jsonrpc_core as rpc; pub type Result = std::result::Result; /// System RPC errors. -#[derive(Debug, derive_more::Display, derive_more::From)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Provided block range couldn't be resolved to a list of blocks. - #[display(fmt = "Node is not fully functional: {}", _0)] + #[error("Node is not fully functional: {}", .0)] NotHealthy(Health), /// Peer argument is malformatted. + #[error("{0}")] MalformattedPeerArg(String), } -impl std::error::Error for Error {} - /// Base code for all system errors. const BASE_ERROR: i64 = 2000; diff --git a/client/rpc/src/chain/mod.rs b/client/rpc/src/chain/mod.rs index 8685b3f93c4ef..a06c3a094b40f 100644 --- a/client/rpc/src/chain/mod.rs +++ b/client/rpc/src/chain/mod.rs @@ -87,7 +87,7 @@ where // FIXME <2329>: Database seems to limit the block number to u32 for no reason let block_num: u32 = num_or_hex.try_into().map_err(|_| { - Error::from(format!( + Error::Other(format!( "`{:?}` > u32::MAX, the max block number is u32.", num_or_hex )) @@ -332,7 +332,9 @@ fn subscribe_headers( let header = client .header(BlockId::Hash(best_block_hash())) .map_err(client_err) - .and_then(|header| header.ok_or_else(|| "Best header missing.".to_string().into())) + .and_then(|header| { + header.ok_or_else(|| Error::Other("Best header missing.".to_string())) + }) .map_err(Into::into); // send further subscriptions diff --git a/frame/democracy/src/types.rs b/frame/democracy/src/types.rs index 4e643006e5167..5c4002a46dd38 100644 --- a/frame/democracy/src/types.rs +++ b/frame/democracy/src/types.rs @@ -28,20 +28,20 @@ use sp_runtime::{ #[derive(Encode, Decode, Default, Clone, PartialEq, Eq, RuntimeDebug)] pub struct Tally { /// The number of aye votes, expressed in terms of post-conviction lock-vote. - pub(crate) ayes: Balance, + pub ayes: Balance, /// The number of nay votes, expressed in terms of post-conviction lock-vote. - pub(crate) nays: Balance, + pub nays: Balance, /// The amount of funds currently expressing its opinion. Pre-conviction. - pub(crate) turnout: Balance, + pub turnout: Balance, } /// Amount of votes and capital placed in delegation for an account. #[derive(Encode, Decode, Default, Copy, Clone, PartialEq, Eq, RuntimeDebug)] pub struct Delegations { /// The number of votes (this is post-conviction). - pub(crate) votes: Balance, + pub votes: Balance, /// The amount of raw capital, used for the turnout. - pub(crate) capital: Balance, + pub capital: Balance, } impl Saturating for Delegations { @@ -162,15 +162,15 @@ impl< #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] pub struct ReferendumStatus { /// When voting on this referendum will end. - pub(crate) end: BlockNumber, + pub end: BlockNumber, /// The hash of the proposal being voted on. - pub(crate) proposal_hash: Hash, + pub proposal_hash: Hash, /// The thresholding mechanism to determine whether it passed. - pub(crate) threshold: VoteThreshold, + pub threshold: VoteThreshold, /// The delay (in blocks) to wait after a successful referendum before deploying. - pub(crate) delay: BlockNumber, + pub delay: BlockNumber, /// The current tally of votes in this referendum. - pub(crate) tally: Tally, + pub tally: Tally, } /// Info regarding a referendum, present or past. diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 4aef03a34389b..96f54c4c03db5 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1236,7 +1236,9 @@ impl Pallet { } // After election finalization, clear OCW solution storage. - if >::events() + // + // We can read the events here because offchain worker doesn't affect PoV. + if >::read_events_no_consensus() .into_iter() .filter_map(|event_record| { let local_event = ::Event::from(event_record.event); diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index bf314161c7f87..105b2328f2325 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1693,7 +1693,19 @@ pub mod pallet_prelude { /// E.g. if runtime names the pallet "MyExample" then the storage `type Foo = ...` use the /// prefix: `Twox128(b"MyExample") ++ Twox128(b"Foo")`. /// -/// The optional attribute `#[pallet::getter(fn $my_getter_fn_name)]` allow to define a +/// The optional attribute `#[pallet::storage_prefix = "$custom_name"]` allows to define a +/// specific name to use for the prefix. +/// +/// E.g: +/// ```ignore +/// #[pallet::storage] +/// #[pallet::storage_prefix = "OtherName"] +/// pub(super) type MyStorage = StorageMap; +/// ``` +/// In this case the final prefix used by the map is +/// `Twox128(b"MyExample") ++ Twox128(b"OtherName")`. +/// +/// The optional attribute `#[pallet::getter(fn $my_getter_fn_name)]` allows to define a /// getter function on `Pallet`. /// /// E.g: @@ -2023,6 +2035,7 @@ pub mod pallet_prelude { /// // Another storage declaration /// #[pallet::storage] /// #[pallet::getter(fn my_storage)] +/// #[pallet::storage_prefix = "SomeOtherName"] /// pub(super) type MyStorage = /// StorageMap; /// @@ -2165,6 +2178,7 @@ pub mod pallet_prelude { /// /// #[pallet::storage] /// #[pallet::getter(fn my_storage)] +/// #[pallet::storage_prefix = "SomeOtherName"] /// pub(super) type MyStorage = /// StorageMap; /// diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index a8bf253c392c4..7b6ec9856d9f4 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -612,9 +612,12 @@ pub mod pallet { pub(super) type Digest = StorageValue<_, DigestOf, ValueQuery>; /// Events deposited for the current block. + /// + /// NOTE: This storage item is explicitly unbounded since it is never intended to be read + /// from within the runtime. #[pallet::storage] - #[pallet::getter(fn events)] - pub type Events = StorageValue<_, Vec>, ValueQuery>; + pub(super) type Events = + StorageValue<_, Vec>, ValueQuery>; /// The number of events in the `Events` list. #[pallet::storage] @@ -1448,6 +1451,24 @@ impl Pallet { }) } + /// Get the current events deposited by the runtime. + /// + /// NOTE: This should only be used in tests. Reading events from the runtime can have a large + /// impact on the PoV size of a block. Users should use alternative and well bounded storage + /// items for any behavior like this. + #[cfg(any(feature = "std", feature = "runtime-benchmarks", test))] + pub fn events() -> Vec> { + Self::read_events_no_consensus() + } + + /// Get the current events deposited by the runtime. + /// + /// Should only be called if you know what you are doing and outside of the runtime block + /// execution else it can have a large impact on the PoV size of a block. + pub fn read_events_no_consensus() -> Vec> { + Events::::get() + } + /// Set the block number to something in particular. Can be used as an alternative to /// `initialize` for tests that don't need to bother with the other environment entries. #[cfg(any(feature = "std", feature = "runtime-benchmarks", test))] diff --git a/primitives/arithmetic/src/per_things.rs b/primitives/arithmetic/src/per_things.rs index 7fbf6bed3f5a2..59c7f36d0fa34 100644 --- a/primitives/arithmetic/src/per_things.rs +++ b/primitives/arithmetic/src/per_things.rs @@ -19,15 +19,16 @@ use serde::{Deserialize, Serialize}; use crate::traits::{ - BaseArithmetic, Bounded, One, SaturatedConversion, Saturating, UniqueSaturatedInto, Unsigned, - Zero, + BaseArithmetic, Bounded, CheckedAdd, CheckedMul, CheckedSub, One, SaturatedConversion, + Saturating, UniqueSaturatedInto, Unsigned, Zero, }; use codec::{CompactAs, Encode}; -use num_traits::Pow; +use num_traits::{Pow, SaturatingAdd, SaturatingSub}; use sp_debug_derive::RuntimeDebug; use sp_std::{ convert::{TryFrom, TryInto}, fmt, ops, + ops::{Add, Sub}, prelude::*, }; @@ -768,6 +769,69 @@ macro_rules! implement_per_thing { } } + impl Add for $name { + type Output = $name; + + #[inline] + fn add(self, rhs: Self) -> Self::Output { + let inner = self.deconstruct().add(rhs.deconstruct()); + debug_assert!(inner < $max); + $name::from_parts(inner) + } + } + + impl CheckedAdd for $name { + // For PerU16, $max == u16::MAX, so we need this `allow`. + #[allow(unused_comparisons)] + #[inline] + fn checked_add(&self, rhs: &Self) -> Option { + self.deconstruct() + .checked_add(rhs.deconstruct()) + .map(|inner| if inner > $max { None } else { Some($name::from_parts(inner)) }) + .flatten() + } + } + + impl Sub for $name { + type Output = $name; + + #[inline] + fn sub(self, rhs: Self) -> Self::Output { + $name::from_parts(self.deconstruct().sub(rhs.deconstruct())) + } + } + + impl CheckedSub for $name { + #[inline] + fn checked_sub(&self, v: &Self) -> Option { + self.deconstruct().checked_sub(v.deconstruct()).map($name::from_parts) + } + } + + impl SaturatingAdd for $name { + #[inline] + fn saturating_add(&self, v: &Self) -> Self { + $name::from_parts(self.deconstruct().saturating_add(v.deconstruct())) + } + } + + impl SaturatingSub for $name { + #[inline] + fn saturating_sub(&self, v: &Self) -> Self { + $name::from_parts(self.deconstruct().saturating_sub(v.deconstruct())) + } + } + + /// # Note + /// CheckedMul will never fail for PerThings. + impl CheckedMul for $name { + #[inline] + fn checked_mul(&self, rhs: &Self) -> Option { + Some(*self * *rhs) + } + } + + #[cfg(test)] mod $test_mod { use codec::{Encode, Decode}; @@ -1354,6 +1418,106 @@ macro_rules! implement_per_thing { assert_eq!((p.0).0, $max); assert_eq!($name::from(p), $name::max_value()); } + + #[allow(unused_imports)] + use super::*; + + #[test] + fn test_add_basic() { + assert_eq!($name::from_parts(1) + $name::from_parts(1), $name::from_parts(2)); + assert_eq!($name::from_parts(10) + $name::from_parts(10), $name::from_parts(20)); + } + + #[test] + fn test_basic_checked_add() { + assert_eq!( + $name::from_parts(1).checked_add(&$name::from_parts(1)), + Some($name::from_parts(2)) + ); + assert_eq!( + $name::from_parts(10).checked_add(&$name::from_parts(10)), + Some($name::from_parts(20)) + ); + assert_eq!( + $name::from_parts(<$type>::MAX).checked_add(&$name::from_parts(<$type>::MAX)), + None + ); + assert_eq!( + $name::from_parts($max).checked_add(&$name::from_parts(1)), + None + ); + } + + #[test] + fn test_basic_saturating_add() { + assert_eq!( + $name::from_parts(1).saturating_add($name::from_parts(1)), + $name::from_parts(2) + ); + assert_eq!( + $name::from_parts(10).saturating_add($name::from_parts(10)), + $name::from_parts(20) + ); + assert_eq!( + $name::from_parts(<$type>::MAX).saturating_add($name::from_parts(<$type>::MAX)), + $name::from_parts(<$type>::MAX) + ); + } + + #[test] + fn test_basic_sub() { + assert_eq!($name::from_parts(2) - $name::from_parts(1), $name::from_parts(1)); + assert_eq!($name::from_parts(20) - $name::from_parts(10), $name::from_parts(10)); + } + + #[test] + fn test_basic_checked_sub() { + assert_eq!( + $name::from_parts(2).checked_sub(&$name::from_parts(1)), + Some($name::from_parts(1)) + ); + assert_eq!( + $name::from_parts(20).checked_sub(&$name::from_parts(10)), + Some($name::from_parts(10)) + ); + assert_eq!($name::from_parts(0).checked_sub(&$name::from_parts(1)), None); + } + + #[test] + fn test_basic_saturating_sub() { + assert_eq!( + $name::from_parts(2).saturating_sub($name::from_parts(1)), + $name::from_parts(1) + ); + assert_eq!( + $name::from_parts(20).saturating_sub($name::from_parts(10)), + $name::from_parts(10) + ); + assert_eq!( + $name::from_parts(0).saturating_sub($name::from_parts(1)), + $name::from_parts(0) + ); + } + + #[test] + fn test_basic_checked_mul() { + assert_eq!( + $name::from_parts($max).checked_mul(&$name::from_parts($max)), + Some($name::from_percent(100)) + ); + assert_eq!( + $name::from_percent(100).checked_mul(&$name::from_percent(100)), + Some($name::from_percent(100)) + ); + assert_eq!( + $name::from_percent(50).checked_mul(&$name::from_percent(26)), + Some($name::from_percent(13)) + ); + assert_eq!( + $name::from_percent(0).checked_mul(&$name::from_percent(0)), + Some($name::from_percent(0)) + ); + } } }; } diff --git a/primitives/npos-elections/solution-type/src/lib.rs b/primitives/npos-elections/solution-type/src/lib.rs index 16b4e8e047437..9b0ec56fc74de 100644 --- a/primitives/npos-elections/solution-type/src/lib.rs +++ b/primitives/npos-elections/solution-type/src/lib.rs @@ -134,20 +134,23 @@ struct SolutionDef { } fn check_attributes(input: ParseStream) -> syn::Result { - let attrs = input.call(syn::Attribute::parse_outer).unwrap_or_default(); + let mut attrs = input.call(syn::Attribute::parse_outer).unwrap_or_default(); if attrs.len() > 1 { - return Err(syn_err("compact solution can accept only #[compact]")) + let extra_attr = attrs.pop().expect("attributes vec with len > 1 can be popped"); + return Err(syn::Error::new_spanned( + extra_attr.clone(), + "compact solution can accept only #[compact]", + )) + } + if attrs.is_empty() { + return Ok(false) + } + let attr = attrs.pop().expect("attributes vec with len 1 can be popped."); + if attr.path.is_ident("compact") { + Ok(true) + } else { + Err(syn::Error::new_spanned(attr.clone(), "compact solution can accept only #[compact]")) } - - Ok(attrs.iter().any(|attr| { - if attr.path.segments.len() == 1 { - let segment = attr.path.segments.first().expect("Vec with len 1 can be popped."); - if segment.ident == Ident::new("compact", Span::call_site()) { - return true - } - } - false - })) } impl Parse for SolutionDef { diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/wrong_page.rs b/primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.rs similarity index 100% rename from primitives/npos-elections/solution-type/tests/ui/fail/wrong_page.rs rename to primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.rs diff --git a/primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.stderr b/primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.stderr new file mode 100644 index 0000000000000..ab700a3f2afcb --- /dev/null +++ b/primitives/npos-elections/solution-type/tests/ui/fail/wrong_attribute.stderr @@ -0,0 +1,5 @@ +error: compact solution can accept only #[compact] + --> $DIR/wrong_attribute.rs:4:2 + | +4 | #[pages(1)] pub struct TestSolution::< + | ^^^^^^^^^^^