Skip to content
Snippets Groups Projects
Commit 9090c01e authored by Bastian Köcher's avatar Bastian Köcher Committed by GitHub
Browse files

transactional: Fix some nitpicks (#11163) 

* transactional: Fix some nitpicks

This fixes some nitpicks related to the transactional storage stuff from me. As everything was
merged too fast, here are some nitpicks from me. First, the entire functionality is moved into its
own file to have a clear separation. Secondly I changed the `set_transactional_level` to not take
`Layer` by reference. Besides that I have added some docs etc.

* Add some comment

* Move tests

* :face_palm:
parent d3d3df5b
Branches
No related merge requests found
Hide whitespace changes
Inline Side-by-side
substrate/frame/support/procedural/src/transactional.rs
+ 1
− 1
View file @ 9090c01e
......@@ -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
......
substrate/frame/support/src/storage/mod.rs
+ 9
− 203
View file @ 9090c01e
......@@ -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(|| {
......
substrate/frame/support/src/storage/transactional.rs 0 → 100644
+ 232
− 0
View file @ 9090c01e
// 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);
});
}
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment