diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index 4287f2c1ce..31e3efb001 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -46,10 +46,6 @@ pub type DispatchResult = Result<(), sp_runtime::DispatchError>; pub type DispatchErrorWithPostInfo = sp_runtime::DispatchErrorWithPostInfo; - -/// A type that cannot be instantiated. -pub enum Never {} - /// Serializable version of Dispatchable. /// This value can be used as a "function" in an extrinsic. pub trait Callable { @@ -1316,7 +1312,7 @@ macro_rules! decl_module { { #[doc(hidden)] #[codec(skip)] - __PhantomItem($crate::sp_std::marker::PhantomData<($trait_instance, $($instance)?)>, $crate::dispatch::Never), + __PhantomItem($crate::sp_std::marker::PhantomData<($trait_instance, $($instance)?)>, $crate::Never), $( $generated_variants )* } }; diff --git a/substrate/frame/support/src/error.rs b/substrate/frame/support/src/error.rs index a06f468892..2fdba88156 100644 --- a/substrate/frame/support/src/error.rs +++ b/substrate/frame/support/src/error.rs @@ -89,7 +89,7 @@ macro_rules! decl_error { #[doc(hidden)] __Ignore( $crate::sp_std::marker::PhantomData<($generic, $( $inst_generic)?)>, - $crate::dispatch::Never, + $crate::Never, ), $( $( #[doc = $doc_attr] )* diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 8a88496eda..49ad802d07 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -78,6 +78,10 @@ pub use self::storage::{ pub use self::dispatch::{Parameter, Callable, IsSubType}; pub use sp_runtime::{self, ConsensusEngineId, print, traits::Printable}; +/// A type that cannot be instantiated. +#[derive(Debug)] +pub enum Never {} + /// Macro for easily creating a new implementation of the `Get` trait. Use similarly to /// how you would declare a `const`: /// diff --git a/substrate/frame/support/src/storage/generator/double_map.rs b/substrate/frame/support/src/storage/generator/double_map.rs index e23b332383..c7e4c10017 100644 --- a/substrate/frame/support/src/storage/generator/double_map.rs +++ b/substrate/frame/support/src/storage/generator/double_map.rs @@ -17,7 +17,7 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; use codec::{Ref, FullCodec, FullEncode, Decode, Encode, EncodeLike, EncodeAppend}; -use crate::{storage::{self, unhashed}, traits::Len}; +use crate::{storage::{self, unhashed}, traits::Len, Never}; use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; /// Generator for `StorageDoubleMap` used by `decl_storage`. @@ -223,14 +223,24 @@ impl storage::StorageDoubleMap for G where KArg1: EncodeLike, KArg2: EncodeLike, F: FnOnce(&mut Self::Query) -> R, + { + Self::try_mutate(k1, k2, |v| Ok::(f(v))).expect("`Never` can not be constructed; qed") + } + + fn try_mutate(k1: KArg1, k2: KArg2, f: F) -> Result where + KArg1: EncodeLike, + KArg2: EncodeLike, + F: FnOnce(&mut Self::Query) -> Result, { let final_key = Self::storage_double_map_final_key(k1, k2); let mut val = G::from_optional_value_to_query(unhashed::get(final_key.as_ref())); let ret = f(&mut val); - match G::from_query_to_optional_value(val) { - Some(ref val) => unhashed::put(final_key.as_ref(), val), - None => unhashed::kill(final_key.as_ref()), + if ret.is_ok() { + match G::from_query_to_optional_value(val) { + Some(ref val) => unhashed::put(final_key.as_ref(), val), + None => unhashed::kill(final_key.as_ref()), + } } ret } diff --git a/substrate/frame/support/src/storage/generator/map.rs b/substrate/frame/support/src/storage/generator/map.rs index c29a9a223a..cc871072f5 100644 --- a/substrate/frame/support/src/storage/generator/map.rs +++ b/substrate/frame/support/src/storage/generator/map.rs @@ -18,7 +18,7 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; use codec::{FullCodec, FullEncode, Decode, Encode, EncodeLike, Ref, EncodeAppend}; -use crate::{storage::{self, unhashed}, traits::Len}; +use crate::{storage::{self, unhashed}, traits::Len, Never}; use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; /// Generator for `StorageMap` used by `decl_storage`. @@ -229,27 +229,11 @@ impl> storage::StorageMap } fn mutate, R, F: FnOnce(&mut Self::Query) -> R>(key: KeyArg, f: F) -> R { - let final_key = Self::storage_map_final_key(key); - let mut val = G::from_optional_value_to_query(unhashed::get(final_key.as_ref())); - - let ret = f(&mut val); - match G::from_query_to_optional_value(val) { - Some(ref val) => unhashed::put(final_key.as_ref(), &val), - None => unhashed::kill(final_key.as_ref()), - } - ret + Self::try_mutate(key, |v| Ok::(f(v))).expect("`Never` can not be constructed; qed") } fn mutate_exists, R, F: FnOnce(&mut Option) -> R>(key: KeyArg, f: F) -> R { - let final_key = Self::storage_map_final_key(key); - let mut val = unhashed::get(final_key.as_ref()); - - let ret = f(&mut val); - match val { - Some(ref val) => unhashed::put(final_key.as_ref(), &val), - None => unhashed::kill(final_key.as_ref()), - } - ret + Self::try_mutate_exists(key, |v| Ok::(f(v))).expect("`Never` can not be constructed; qed") } fn try_mutate, R, E, F: FnOnce(&mut Self::Query) -> Result>( diff --git a/substrate/frame/support/src/storage/generator/mod.rs b/substrate/frame/support/src/storage/generator/mod.rs index 687d8a3c93..07e75055f3 100644 --- a/substrate/frame/support/src/storage/generator/mod.rs +++ b/substrate/frame/support/src/storage/generator/mod.rs @@ -37,6 +37,7 @@ mod tests { use sp_io::TestExternalities; use codec::Encode; use crate::storage::{unhashed, generator::StorageValue, IterableStorageMap}; + use crate::{assert_noop, assert_ok}; struct Runtime {} pub trait Trait { @@ -57,6 +58,7 @@ mod tests { trait Store for Module as Runtime { Value get(fn value) config(): (u64, u64); NumberMap: map hasher(identity) u32 => u64; + DoubleMap: double_map hasher(identity) u32, hasher(identity) u32 => u64; } } @@ -102,4 +104,54 @@ mod tests { ); }) } + + #[test] + fn try_mutate_works() { + let t = GenesisConfig::default().build_storage().unwrap(); + TestExternalities::new(t).execute_with(|| { + assert_eq!(Value::get(), (0, 0)); + assert_eq!(NumberMap::get(0), 0); + assert_eq!(DoubleMap::get(0, 0), 0); + + // `assert_noop` ensures that the state does not change + assert_noop!(Value::try_mutate(|value| -> Result<(), &'static str> { + *value = (2, 2); + Err("don't change value") + }), "don't change value"); + + assert_noop!(NumberMap::try_mutate(0, |value| -> Result<(), &'static str> { + *value = 4; + Err("don't change value") + }), "don't change value"); + + assert_noop!(DoubleMap::try_mutate(0, 0, |value| -> Result<(), &'static str> { + *value = 6; + Err("don't change value") + }), "don't change value"); + + // Showing this explicitly for clarity + assert_eq!(Value::get(), (0, 0)); + assert_eq!(NumberMap::get(0), 0); + assert_eq!(DoubleMap::get(0, 0), 0); + + assert_ok!(Value::try_mutate(|value| -> Result<(), &'static str> { + *value = (2, 2); + Ok(()) + })); + + assert_ok!(NumberMap::try_mutate(0, |value| -> Result<(), &'static str> { + *value = 4; + Ok(()) + })); + + assert_ok!(DoubleMap::try_mutate(0, 0, |value| -> Result<(), &'static str> { + *value = 6; + Ok(()) + })); + + assert_eq!(Value::get(), (2, 2)); + assert_eq!(NumberMap::get(0), 4); + assert_eq!(DoubleMap::get(0, 0), 6); + }); + } } diff --git a/substrate/frame/support/src/storage/generator/value.rs b/substrate/frame/support/src/storage/generator/value.rs index 9e26131f48..dd9bbded4b 100644 --- a/substrate/frame/support/src/storage/generator/value.rs +++ b/substrate/frame/support/src/storage/generator/value.rs @@ -17,7 +17,12 @@ #[cfg(not(feature = "std"))] use sp_std::prelude::*; use codec::{FullCodec, Encode, EncodeAppend, EncodeLike, Decode}; -use crate::{storage::{self, unhashed}, hash::{Twox128, StorageHasher}, traits::Len}; +use crate::{ + Never, + storage::{self, unhashed}, + hash::{Twox128, StorageHasher}, + traits::Len +}; /// Generator for `StorageValue` used by `decl_storage`. /// @@ -104,12 +109,18 @@ impl> storage::StorageValue for G { } fn mutate R>(f: F) -> R { + Self::try_mutate(|v| Ok::(f(v))).expect("`Never` can not be constructed; qed") + } + + fn try_mutate Result>(f: F) -> Result { let mut val = G::get(); let ret = f(&mut val); - match G::from_query_to_optional_value(val) { - Some(ref val) => G::put(val), - None => G::kill(), + if ret.is_ok() { + match G::from_query_to_optional_value(val) { + Some(ref val) => G::put(val), + None => G::kill(), + } } ret } diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 47201e22e6..3a811c20e3 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -80,6 +80,9 @@ pub trait StorageValue { /// Mutate the value fn mutate R>(f: F) -> R; + /// Mutate the value if closure returns `Ok` + fn try_mutate Result>(f: F) -> Result; + /// Clear the storage value. fn kill(); @@ -280,21 +283,25 @@ pub trait StorageDoubleMap { /// The type that get/take returns. type Query; + /// Get the storage key used to fetch a value corresponding to a specific key. fn hashed_key_for(k1: KArg1, k2: KArg2) -> Vec where KArg1: EncodeLike, KArg2: EncodeLike; + /// Does the value (explicitly) exist in storage? fn contains_key(k1: KArg1, k2: KArg2) -> bool where KArg1: EncodeLike, KArg2: EncodeLike; + /// Load the value associated with the given key from the double map. fn get(k1: KArg1, k2: KArg2) -> Self::Query where KArg1: EncodeLike, KArg2: EncodeLike; + /// Take a value from storage, removing it afterwards. fn take(k1: KArg1, k2: KArg2) -> Self::Query where KArg1: EncodeLike, @@ -308,28 +315,43 @@ pub trait StorageDoubleMap { YKArg1: EncodeLike, YKArg2: EncodeLike; + /// Store a value to be associated with the given keys from the double map. fn insert(k1: KArg1, k2: KArg2, val: VArg) where KArg1: EncodeLike, KArg2: EncodeLike, VArg: EncodeLike; + /// Remove the value under the given keys. fn remove(k1: KArg1, k2: KArg2) where KArg1: EncodeLike, KArg2: EncodeLike; + /// Remove all values under the first key. fn remove_prefix(k1: KArg1) where KArg1: ?Sized + EncodeLike; + /// Iterate over values that share the first key. fn iter_prefix_values(k1: KArg1) -> PrefixIterator where KArg1: ?Sized + EncodeLike; + /// Mutate the value under the given keys. fn mutate(k1: KArg1, k2: KArg2, f: F) -> R where KArg1: EncodeLike, KArg2: EncodeLike, F: FnOnce(&mut Self::Query) -> R; + /// Mutate the value under the given keys when the closure returns `Ok`. + fn try_mutate(k1: KArg1, k2: KArg2, f: F) -> Result + where + KArg1: EncodeLike, + KArg2: EncodeLike, + F: FnOnce(&mut Self::Query) -> Result; + + /// Append the given item to the value in the storage. + /// + /// `V` is required to implement `codec::EncodeAppend`. fn append( k1: KArg1, k2: KArg2, @@ -344,6 +366,10 @@ pub trait StorageDoubleMap { Items: IntoIterator, Items::IntoIter: ExactSizeIterator; + /// Safely append the given items to the value in the storage. If a codec error occurs, then the + /// old (presumably corrupt) value is replaced with the given `items`. + /// + /// `V` is required to implement `codec::EncodeAppend`. fn append_or_insert( k1: KArg1, k2: KArg2,