Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use Storage for Tracking Transactional Layers #10744

Closed
wants to merge 15 commits into from
4 changes: 2 additions & 2 deletions frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
};
use frame_support::{
dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable},
storage::{with_transaction, TransactionOutcome},
storage::{execute_with_storage_layer, TransactionOutcome},
traits::{Contains, Currency, ExistenceRequirement, OriginTrait, Randomness, Time},
weights::Weight,
};
Expand Down Expand Up @@ -721,7 +721,7 @@ where
// All changes performed by the contract are executed under a storage transaction.
// This allows for roll back on error. Changes to the cached contract_info are
// comitted or rolled back when popping the frame.
let (success, output) = with_transaction(|| {
let (success, output) = execute_with_storage_layer(u8::MAX, || {
let output = do_transaction();
match &output {
Ok(result) if !result.did_revert() => TransactionOutcome::Commit((true, output)),
Expand Down
14 changes: 7 additions & 7 deletions frame/support/procedural/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mod match_and_insert;
mod pallet;
mod partial_eq_no_bound;
mod storage;
mod transactional;
mod storage_layer;

use proc_macro::TokenStream;
use std::{cell::RefCell, str::FromStr};
Expand Down Expand Up @@ -413,21 +413,21 @@ pub fn pallet(attr: TokenStream, item: TokenStream) -> TokenStream {
/// # Example
///
/// ```nocompile
/// #[transactional]
/// #[add_storage_layer]
/// fn value_commits(v: u32) -> result::Result<u32, &'static str> {
/// Value::set(v);
/// Ok(v)
/// }
///
/// #[transactional]
/// #[add_storage_layer]
/// fn value_rollbacks(v: u32) -> result::Result<u32, &'static str> {
/// Value::set(v);
/// Err("nah")
/// }
/// ```
#[proc_macro_attribute]
pub fn transactional(attr: TokenStream, input: TokenStream) -> TokenStream {
transactional::transactional(attr, input).unwrap_or_else(|e| e.to_compile_error().into())
pub fn add_storage_layer(attr: TokenStream, input: TokenStream) -> TokenStream {
storage_layer::add_storage_layer(attr, input).unwrap_or_else(|e| e.to_compile_error().into())
}

/// Derive [`Clone`] but do not bound any generic. Docs are at `frame_support::CloneNoBound`.
Expand Down Expand Up @@ -508,8 +508,8 @@ pub fn derive_default_no_bound(input: TokenStream) -> TokenStream {
}

#[proc_macro_attribute]
pub fn require_transactional(attr: TokenStream, input: TokenStream) -> TokenStream {
transactional::require_transactional(attr, input)
pub fn require_storage_layer(attr: TokenStream, input: TokenStream) -> TokenStream {
storage_layer::require_storage_layer(attr, input)
.unwrap_or_else(|e| e.to_compile_error().into())
}

Expand Down
103 changes: 103 additions & 0 deletions frame/support/procedural/src/storage_layer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// This file is part of Substrate.

// Copyright (C) 2020-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.

use frame_support_procedural_tools::generate_crate_access_2018;
use proc_macro::TokenStream;
use quote::quote;
use syn::{parse::Parse, ItemFn, LitInt, Result};

struct StorageLayerLimit {
limit: u8,
Copy link
Contributor

@kianenigma kianenigma Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this not a tuple struct?

}

impl Default for StorageLayerLimit {
fn default() -> Self {
Self { limit: 1 }
}
}

impl Parse for StorageLayerLimit {
fn parse(input: syn::parse::ParseStream) -> Result<Self> {
let limit: LitInt = input.parse()?;
Ok(Self { limit: limit.base10_parse()? })
}
}

pub fn add_storage_layer(attr: TokenStream, input: TokenStream) -> Result<TokenStream> {
let limit: StorageLayerLimit = syn::parse(attr).unwrap_or_default();
let limit = limit.limit;

let ItemFn { attrs, vis, sig, block } = syn::parse(input)?;

let crate_ = generate_crate_access_2018("frame-support")?;
let output = quote! {
#(#attrs)*
#vis #sig {
use #crate_::storage::execute_with_storage_layer;
// Otherwise, spawn a transaction layer.
execute_with_storage_layer(#limit, || {
(|| { #block })()
})
}
};

Ok(output.into())
}

// Similar to `add_storage_layer` but only spawns at most 1 layer.
pub fn with_storage_layer(attr: TokenStream, input: TokenStream) -> Result<TokenStream> {
let limit: StorageLayerLimit = syn::parse(attr).unwrap_or_default();
let limit = limit.limit;

let ItemFn { attrs, vis, sig, block } = syn::parse(input)?;

let crate_ = generate_crate_access_2018("frame-support")?;
let output = quote! {
#(#attrs)*
#vis #sig {
use #crate_::storage::{execute_with_storage_layer, has_storage_layer};
if has_storage_layer() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this prevents recursive storage layers?
Was that not the whole point of having a limit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be an entirely different approach to storage layers, where rather than spawning new transactional layers per thing, we would just spawn at most one.

This would be the thing we would probably use by default across all pallet extrinsics.

// We are already in a transaction layer, just execute the block.
(|| { #block })()
} else {
// Otherwise, spawn a transaction layer.
execute_with_storage_layer(#limit, || {
(|| { #block })()
})
}
}
};

Ok(output.into())
}

pub fn require_storage_layer(_attr: TokenStream, input: TokenStream) -> Result<TokenStream> {
let ItemFn { attrs, vis, sig, block } = syn::parse(input)?;

let crate_ = generate_crate_access_2018("frame-support")?;
let output = quote! {
#(#attrs)*
#vis #sig {
if !#crate_::storage::has_storage_layer(){
return Err(#crate_::pallet_prelude::DispatchError::StorageLayersLimit.into())
}
#block
}
};

Ok(output.into())
}
58 changes: 0 additions & 58 deletions frame/support/procedural/src/transactional.rs

This file was deleted.

4 changes: 2 additions & 2 deletions frame/support/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,12 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug +
/// ```
/// # #[macro_use]
/// # extern crate frame_support;
/// # use frame_support::transactional;
/// # use frame_support::add_storage_layer;
/// # use frame_system::Config;
/// decl_module! {
/// pub struct Module<T: Config> for enum Call where origin: T::Origin {
/// #[weight = 0]
/// #[transactional]
/// #[add_storage_layer]
/// fn my_short_function(origin) {
/// // Your implementation
/// }
Expand Down
10 changes: 5 additions & 5 deletions frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ pub fn debug(data: &impl sp_std::fmt::Debug) {

#[doc(inline)]
pub use frame_support_procedural::{
construct_runtime, decl_storage, match_and_insert, transactional, RuntimeDebugNoBound,
add_storage_layer, construct_runtime, decl_storage, match_and_insert, RuntimeDebugNoBound,
};

#[doc(hidden)]
Expand Down Expand Up @@ -704,17 +704,17 @@ pub use frame_support_procedural::DefaultNoBound;
///
/// ```
/// # use frame_support::{
/// # require_transactional, transactional, dispatch::DispatchResult
/// # require_storage_layer, add_storage_layer, dispatch::DispatchResult
/// # };
///
/// #[require_transactional]
/// #[require_storage_layer]
/// fn update_all(value: u32) -> DispatchResult {
/// // Update multiple storages.
/// // Return `Err` to indicate should revert.
/// Ok(())
/// }
///
/// #[transactional]
/// #[add_storage_layer]
/// fn safe_update(value: u32) -> DispatchResult {
/// // This is safe
/// update_all(value)
Expand All @@ -725,7 +725,7 @@ pub use frame_support_procedural::DefaultNoBound;
/// update_all(value)
/// }
/// ```
pub use frame_support_procedural::require_transactional;
pub use frame_support_procedural::require_storage_layer;

/// Convert the current crate version into a [`CrateVersion`](crate::traits::CrateVersion).
///
Expand Down
Loading