diff --git a/substrate/frame/support/procedural/src/transactional.rs b/substrate/frame/support/procedural/src/transactional.rs index ba75fbc9737aa353a9fc3722d5c76443ab4bc04d..eb77a320a509fa56c0f025f1623620715d291f8d 100644 --- a/substrate/frame/support/procedural/src/transactional.rs +++ b/substrate/frame/support/procedural/src/transactional.rs @@ -49,7 +49,7 @@ pub fn require_transactional(_attr: TokenStream, input: TokenStream) -> Result<T let output = quote! { #(#attrs)* #vis #sig { - if !#crate_::storage::is_transactional() { + if !#crate_::storage::transactional::is_transactional() { return Err(#crate_::sp_runtime::TransactionalError::NoLayer.into()); } #block diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 106d3e3f459aabe1d5dd479292d6506a4c52cade..f3eb94498ed923750b528d775803b0c3a197ca7b 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -17,7 +17,6 @@ //! Stuff to do with the runtime's storage. -pub use self::types::StorageEntryMetadataBuilder; use crate::{ hash::{ReversibleStorageHasher, StorageHasher}, storage::types::{ @@ -27,12 +26,14 @@ use crate::{ }; use codec::{Decode, Encode, EncodeLike, FullCodec, FullEncode}; use sp_core::storage::ChildInfo; -pub use sp_runtime::TransactionOutcome; -use sp_runtime::{ - generic::{Digest, DigestItem}, - DispatchError, TransactionalError, -}; +use sp_runtime::generic::{Digest, DigestItem}; use sp_std::prelude::*; + +pub use self::{ + transactional::{with_transaction, with_transaction_unchecked}, + types::StorageEntryMetadataBuilder, +}; +pub use sp_runtime::TransactionOutcome; pub use types::Key; pub mod bounded_btree_map; @@ -43,140 +44,11 @@ pub mod child; pub mod generator; pub mod hashed; pub mod migration; +pub mod transactional; pub mod types; pub mod unhashed; pub mod weak_bounded_vec; -mod transaction_level_tracker { - type Layer = u32; - const TRANSACTION_LEVEL_KEY: &'static [u8] = b":transaction_level:"; - const TRANSACTIONAL_LIMIT: Layer = 255; - - pub fn get_transaction_level() -> Layer { - crate::storage::unhashed::get_or_default::<Layer>(TRANSACTION_LEVEL_KEY) - } - - fn set_transaction_level(level: &Layer) { - crate::storage::unhashed::put::<Layer>(TRANSACTION_LEVEL_KEY, level); - } - - fn kill_transaction_level() { - crate::storage::unhashed::kill(TRANSACTION_LEVEL_KEY); - } - - /// Increments the transaction level. Returns an error if levels go past the limit. - /// - /// Returns a guard that when dropped decrements the transaction level automatically. - pub fn inc_transaction_level() -> Result<StorageLayerGuard, ()> { - let existing_levels = get_transaction_level(); - if existing_levels >= TRANSACTIONAL_LIMIT { - return Err(()) - } - // Cannot overflow because of check above. - set_transaction_level(&(existing_levels + 1)); - Ok(StorageLayerGuard) - } - - fn dec_transaction_level() { - let existing_levels = get_transaction_level(); - if existing_levels == 0 { - log::warn!( - "We are underflowing with calculating transactional levels. Not great, but let's not panic...", - ); - } else if existing_levels == 1 { - // Don't leave any trace of this storage item. - kill_transaction_level(); - } else { - // Cannot underflow because of checks above. - set_transaction_level(&(existing_levels - 1)); - } - } - - pub fn is_transactional() -> bool { - get_transaction_level() > 0 - } - - pub struct StorageLayerGuard; - - impl Drop for StorageLayerGuard { - fn drop(&mut self) { - dec_transaction_level() - } - } -} - -/// Check if the current call is within a transactional layer. -pub fn is_transactional() -> bool { - transaction_level_tracker::is_transactional() -} - -/// Execute the supplied function in a new storage transaction. -/// -/// All changes to storage performed by the supplied function are discarded if the returned -/// outcome is `TransactionOutcome::Rollback`. -/// -/// Transactions can be nested up to `TRANSACTIONAL_LIMIT` times; more than that will result in an -/// error. -/// -/// Commits happen to the parent transaction. -pub fn with_transaction<T, E>(f: impl FnOnce() -> TransactionOutcome<Result<T, E>>) -> Result<T, E> -where - E: From<DispatchError>, -{ - use sp_io::storage::{commit_transaction, rollback_transaction, start_transaction}; - use TransactionOutcome::*; - - let _guard = transaction_level_tracker::inc_transaction_level() - .map_err(|()| TransactionalError::LimitReached.into())?; - - start_transaction(); - - match f() { - Commit(res) => { - commit_transaction(); - res - }, - Rollback(res) => { - rollback_transaction(); - res - }, - } -} - -/// Same as [`with_transaction`] but without a limit check on nested transactional layers. -/// -/// This is mostly for backwards compatibility before there was a transactional layer limit. -/// It is recommended to only use [`with_transaction`] to avoid users from generating too many -/// transactional layers. -pub fn with_transaction_unchecked<R>(f: impl FnOnce() -> TransactionOutcome<R>) -> R { - use sp_io::storage::{commit_transaction, rollback_transaction, start_transaction}; - use TransactionOutcome::*; - - let maybe_guard = transaction_level_tracker::inc_transaction_level(); - - if maybe_guard.is_err() { - log::warn!( - "The transactional layer limit has been reached, and new transactional layers are being - spawned with `with_transaction_unchecked`. This could be caused by someone trying to - attack your chain, and you should investigate usage of `with_transaction_unchecked` and - potentially migrate to `with_transaction`, which enforces a transactional limit.", - ); - } - - start_transaction(); - - match f() { - Commit(res) => { - commit_transaction(); - res - }, - Rollback(res) => { - rollback_transaction(); - res - }, - } -} - /// A trait for working with macro-generated storage values under the substrate storage API. /// /// Details on implementation can be found at [`generator::StorageValue`]. @@ -1473,13 +1345,12 @@ pub fn storage_prefix(pallet_name: &[u8], storage_name: &[u8]) -> [u8; 32] { #[cfg(test)] mod test { use super::*; - use crate::{assert_noop, assert_ok, hash::Identity, Twox128}; + use crate::{assert_ok, hash::Identity, Twox128}; use bounded_vec::BoundedVec; use frame_support::traits::ConstU32; use generator::StorageValue as _; use sp_core::hashing::twox_128; use sp_io::TestExternalities; - use sp_runtime::DispatchResult; use weak_bounded_vec::WeakBoundedVec; #[test] @@ -1590,71 +1461,6 @@ mod test { }); } - #[test] - fn is_transactional_should_return_false() { - TestExternalities::default().execute_with(|| { - assert!(!is_transactional()); - }); - } - - #[test] - fn is_transactional_should_not_error_in_with_transaction() { - TestExternalities::default().execute_with(|| { - assert_ok!(with_transaction(|| -> TransactionOutcome<DispatchResult> { - assert!(is_transactional()); - TransactionOutcome::Commit(Ok(())) - })); - - assert_noop!( - with_transaction(|| -> TransactionOutcome<DispatchResult> { - assert!(is_transactional()); - TransactionOutcome::Rollback(Err("revert".into())) - }), - "revert" - ); - }); - } - - fn recursive_transactional(num: u32) -> DispatchResult { - if num == 0 { - return Ok(()) - } - - with_transaction(|| -> TransactionOutcome<DispatchResult> { - let res = recursive_transactional(num - 1); - TransactionOutcome::Commit(res) - }) - } - - #[test] - fn transaction_limit_should_work() { - TestExternalities::default().execute_with(|| { - assert_eq!(transaction_level_tracker::get_transaction_level(), 0); - - assert_ok!(with_transaction(|| -> TransactionOutcome<DispatchResult> { - assert_eq!(transaction_level_tracker::get_transaction_level(), 1); - TransactionOutcome::Commit(Ok(())) - })); - - assert_ok!(with_transaction(|| -> TransactionOutcome<DispatchResult> { - assert_eq!(transaction_level_tracker::get_transaction_level(), 1); - let res = with_transaction(|| -> TransactionOutcome<DispatchResult> { - assert_eq!(transaction_level_tracker::get_transaction_level(), 2); - TransactionOutcome::Commit(Ok(())) - }); - TransactionOutcome::Commit(res) - })); - - assert_ok!(recursive_transactional(255)); - assert_noop!( - recursive_transactional(256), - sp_runtime::TransactionalError::LimitReached - ); - - assert_eq!(transaction_level_tracker::get_transaction_level(), 0); - }); - } - #[test] fn key_prefix_iterator_works() { TestExternalities::default().execute_with(|| { diff --git a/substrate/frame/support/src/storage/transactional.rs b/substrate/frame/support/src/storage/transactional.rs new file mode 100644 index 0000000000000000000000000000000000000000..d1c59d44e258196e8d0059149fbd2fc38874591d --- /dev/null +++ b/substrate/frame/support/src/storage/transactional.rs @@ -0,0 +1,232 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Provides functionality around the transaction storage. +//! +//! Transactional storage provides functionality to run an entire code block +//! in a storage transaction. This means that either the entire changes to the +//! storage are committed or everything is thrown away. This simplifies the +//! writing of functionality that may bail at any point of operation. Otherwise +//! you would need to first verify all storage accesses and then do the storage +//! modifications. +//! +//! [`with_transaction`] provides a way to run a given closure in a transactional context. + +use sp_io::storage::{commit_transaction, rollback_transaction, start_transaction}; +use sp_runtime::{DispatchError, TransactionOutcome, TransactionalError}; + +/// The type that is being used to store the current number of active layers. +type Layer = u32; +/// The key that is holds the current number of active layers. +const TRANSACTION_LEVEL_KEY: &[u8] = b":transaction_level:"; +/// The maximum number of nested layers. +const TRANSACTIONAL_LIMIT: Layer = 255; + +/// Returns the current number of nested transactional layers. +fn get_transaction_level() -> Layer { + crate::storage::unhashed::get_or_default::<Layer>(TRANSACTION_LEVEL_KEY) +} + +/// Set the current number of nested transactional layers. +fn set_transaction_level(level: Layer) { + crate::storage::unhashed::put::<Layer>(TRANSACTION_LEVEL_KEY, &level); +} + +/// Kill the transactional layers storage. +fn kill_transaction_level() { + crate::storage::unhashed::kill(TRANSACTION_LEVEL_KEY); +} + +/// Increments the transaction level. Returns an error if levels go past the limit. +/// +/// Returns a guard that when dropped decrements the transaction level automatically. +fn inc_transaction_level() -> Result<StorageLayerGuard, ()> { + let existing_levels = get_transaction_level(); + if existing_levels >= TRANSACTIONAL_LIMIT { + return Err(()) + } + // Cannot overflow because of check above. + set_transaction_level(existing_levels + 1); + Ok(StorageLayerGuard) +} + +fn dec_transaction_level() { + let existing_levels = get_transaction_level(); + if existing_levels == 0 { + log::warn!( + "We are underflowing with calculating transactional levels. Not great, but let's not panic...", + ); + } else if existing_levels == 1 { + // Don't leave any trace of this storage item. + kill_transaction_level(); + } else { + // Cannot underflow because of checks above. + set_transaction_level(existing_levels - 1); + } +} + +struct StorageLayerGuard; + +impl Drop for StorageLayerGuard { + fn drop(&mut self) { + dec_transaction_level() + } +} + +/// Check if the current call is within a transactional layer. +pub fn is_transactional() -> bool { + get_transaction_level() > 0 +} + +/// Execute the supplied function in a new storage transaction. +/// +/// All changes to storage performed by the supplied function are discarded if the returned +/// outcome is `TransactionOutcome::Rollback`. +/// +/// Transactions can be nested up to `TRANSACTIONAL_LIMIT` times; more than that will result in an +/// error. +/// +/// Commits happen to the parent transaction. +pub fn with_transaction<T, E>(f: impl FnOnce() -> TransactionOutcome<Result<T, E>>) -> Result<T, E> +where + E: From<DispatchError>, +{ + // This needs to happen before `start_transaction` below. + // Otherwise we may rollback the increase, then decrease as the guard goes out of scope + // and then end in some bad state. + let _guard = inc_transaction_level().map_err(|()| TransactionalError::LimitReached.into())?; + + start_transaction(); + + match f() { + TransactionOutcome::Commit(res) => { + commit_transaction(); + res + }, + TransactionOutcome::Rollback(res) => { + rollback_transaction(); + res + }, + } +} + +/// Same as [`with_transaction`] but without a limit check on nested transactional layers. +/// +/// This is mostly for backwards compatibility before there was a transactional layer limit. +/// It is recommended to only use [`with_transaction`] to avoid users from generating too many +/// transactional layers. +pub fn with_transaction_unchecked<R>(f: impl FnOnce() -> TransactionOutcome<R>) -> R { + // This needs to happen before `start_transaction` below. + // Otherwise we may rollback the increase, then decrease as the guard goes out of scope + // and then end in some bad state. + let maybe_guard = inc_transaction_level(); + + if maybe_guard.is_err() { + log::warn!( + "The transactional layer limit has been reached, and new transactional layers are being + spawned with `with_transaction_unchecked`. This could be caused by someone trying to + attack your chain, and you should investigate usage of `with_transaction_unchecked` and + potentially migrate to `with_transaction`, which enforces a transactional limit.", + ); + } + + start_transaction(); + + match f() { + TransactionOutcome::Commit(res) => { + commit_transaction(); + res + }, + TransactionOutcome::Rollback(res) => { + rollback_transaction(); + res + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{assert_noop, assert_ok}; + use sp_io::TestExternalities; + use sp_runtime::DispatchResult; + + #[test] + fn is_transactional_should_return_false() { + TestExternalities::default().execute_with(|| { + assert!(!is_transactional()); + }); + } + + #[test] + fn is_transactional_should_not_error_in_with_transaction() { + TestExternalities::default().execute_with(|| { + assert_ok!(with_transaction(|| -> TransactionOutcome<DispatchResult> { + assert!(is_transactional()); + TransactionOutcome::Commit(Ok(())) + })); + + assert_noop!( + with_transaction(|| -> TransactionOutcome<DispatchResult> { + assert!(is_transactional()); + TransactionOutcome::Rollback(Err("revert".into())) + }), + "revert" + ); + }); + } + + fn recursive_transactional(num: u32) -> DispatchResult { + if num == 0 { + return Ok(()) + } + + with_transaction(|| -> TransactionOutcome<DispatchResult> { + let res = recursive_transactional(num - 1); + TransactionOutcome::Commit(res) + }) + } + + #[test] + fn transaction_limit_should_work() { + TestExternalities::default().execute_with(|| { + assert_eq!(get_transaction_level(), 0); + + assert_ok!(with_transaction(|| -> TransactionOutcome<DispatchResult> { + assert_eq!(get_transaction_level(), 1); + TransactionOutcome::Commit(Ok(())) + })); + + assert_ok!(with_transaction(|| -> TransactionOutcome<DispatchResult> { + assert_eq!(get_transaction_level(), 1); + let res = with_transaction(|| -> TransactionOutcome<DispatchResult> { + assert_eq!(get_transaction_level(), 2); + TransactionOutcome::Commit(Ok(())) + }); + TransactionOutcome::Commit(res) + })); + + assert_ok!(recursive_transactional(255)); + assert_noop!( + recursive_transactional(256), + sp_runtime::TransactionalError::LimitReached + ); + + assert_eq!(get_transaction_level(), 0); + }); + } +}