Skip to content

Commit

Permalink
Remove dynamic storage allocator (#1148)
Browse files Browse the repository at this point in the history
* Remove `dynamic_storage_allocator` from Config structs

* Remove `dynamic_storage_allocator` from IR

* Make IR tests compile again

* Remove `dynamic_storage_alloc` from codegen

* Remove the dynamic storage allocator code

Goodnight Sweet Prince 🥲

* Remove UI tests related to `dynamic_storage_allocator`

* Remove mentions in code level docs
  • Loading branch information
HCastano authored Feb 28, 2022
1 parent 34bd952 commit c94d39c
Show file tree
Hide file tree
Showing 23 changed files with 7 additions and 1,749 deletions.
10 changes: 0 additions & 10 deletions crates/lang/codegen/src/generator/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,17 +566,12 @@ impl Dispatch<'_> {
}>>::IDS[#index]
}>>::PAYABLE
);
let is_dynamic_storage_allocation_enabled = self
.contract
.config()
.is_dynamic_storage_allocator_enabled();

quote_spanned!(constructor_span=>
Self::#constructor_ident(input) => {
::ink_lang::codegen::execute_constructor::<#storage_ident, _, _>(
::ink_lang::codegen::ExecuteConstructorConfig {
payable: #accepts_payment,
dynamic_storage_alloc: #is_dynamic_storage_allocation_enabled
},
move || { #constructor_callable(input) }
)
Expand Down Expand Up @@ -745,17 +740,12 @@ impl Dispatch<'_> {
}>>::IDS[#index]
}>>::MUTATES
);
let is_dynamic_storage_allocation_enabled = self
.contract
.config()
.is_dynamic_storage_allocator_enabled();

quote_spanned!(message_span=>
Self::#message_ident(input) => {
let config = ::ink_lang::codegen::ExecuteMessageConfig {
payable: #accepts_payment,
mutates: #mutates_storage,
dynamic_storage_alloc: #is_dynamic_storage_allocation_enabled,
};
let mut contract: ::core::mem::ManuallyDrop<#storage_ident> =
::core::mem::ManuallyDrop::new(
Expand Down
59 changes: 1 addition & 58 deletions crates/lang/ir/src/ir/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ use syn::spanned::Spanned;
/// The ink! configuration.
#[derive(Debug, Default, PartialEq, Eq)]
pub struct Config {
/// If `true` enables the dynamic storage allocator
/// facilities and code generation of the ink! smart
/// contract. Does incur some overhead. The default is
/// `true`.
dynamic_storage_allocator: Option<bool>,
/// If `true` compiles this ink! smart contract always as
/// if it was a dependency of another smart contract.
/// This configuration is mainly needed for testing and
Expand Down Expand Up @@ -125,29 +120,12 @@ impl TryFrom<ast::AttributeArgs> for Config {
type Error = syn::Error;

fn try_from(args: ast::AttributeArgs) -> Result<Self, Self::Error> {
let mut dynamic_storage_allocator: Option<(bool, ast::MetaNameValue)> = None;
let mut as_dependency: Option<(bool, ast::MetaNameValue)> = None;
let mut env: Option<(Environment, ast::MetaNameValue)> = None;
let mut whitelisted_attributes = WhitelistedAttributes::default();

for arg in args.into_iter() {
if arg.name.is_ident("dynamic_storage_allocator") {
if let Some((_, ast)) = dynamic_storage_allocator {
return Err(duplicate_config_err(
ast,
arg,
"dynamic_storage_allocator",
))
}
if let ast::PathOrLit::Lit(syn::Lit::Bool(lit_bool)) = &arg.value {
dynamic_storage_allocator = Some((lit_bool.value, arg))
} else {
return Err(format_err_spanned!(
arg,
"expected a bool literal for `dynamic_storage_allocator` ink! configuration argument",
))
}
} else if arg.name.is_ident("compile_as_dependency") {
if arg.name.is_ident("compile_as_dependency") {
if let Some((_, ast)) = as_dependency {
return Err(duplicate_config_err(ast, arg, "compile_as_dependency"))
}
Expand Down Expand Up @@ -183,7 +161,6 @@ impl TryFrom<ast::AttributeArgs> for Config {
}
}
Ok(Config {
dynamic_storage_allocator: dynamic_storage_allocator.map(|(value, _)| value),
as_dependency: as_dependency.map(|(value, _)| value),
env: env.map(|(value, _)| value),
whitelisted_attributes,
Expand All @@ -203,14 +180,6 @@ impl Config {
.unwrap_or(Environment::default().path)
}

/// Returns `true` if the dynamic storage allocator facilities are enabled
/// for the ink! smart contract, `false` otherwise.
///
/// If nothing has been specified returns the default which is `false`.
pub fn is_dynamic_storage_allocator_enabled(&self) -> bool {
self.dynamic_storage_allocator.unwrap_or(false)
}

/// Return `true` if this ink! smart contract shall always be compiled as
/// if it was a dependency of another smart contract, returns `false`
/// otherwise.
Expand Down Expand Up @@ -263,37 +232,13 @@ mod tests {
assert_try_from(syn::parse_quote! {}, Ok(Config::default()))
}

#[test]
fn storage_alloc_works() {
assert_try_from(
syn::parse_quote! {
dynamic_storage_allocator = true
},
Ok(Config {
dynamic_storage_allocator: Some(true),
as_dependency: None,
env: None,
whitelisted_attributes: Default::default(),
}),
)
}

#[test]
fn storage_alloc_invalid_value_fails() {
assert_try_from(
syn::parse_quote! { dynamic_storage_allocator = "invalid" },
Err("expected a bool literal for `dynamic_storage_allocator` ink! configuration argument"),
)
}

#[test]
fn as_dependency_works() {
assert_try_from(
syn::parse_quote! {
compile_as_dependency = false
},
Ok(Config {
dynamic_storage_allocator: None,
as_dependency: Some(false),
env: None,
whitelisted_attributes: Default::default(),
Expand All @@ -318,7 +263,6 @@ mod tests {
env = ::my::env::Types
},
Ok(Config {
dynamic_storage_allocator: None,
as_dependency: None,
env: Some(Environment {
path: syn::parse_quote! { ::my::env::Types },
Expand Down Expand Up @@ -365,7 +309,6 @@ mod tests {
keep_attr = "foo, bar"
},
Ok(Config {
dynamic_storage_allocator: None,
as_dependency: None,
env: None,
whitelisted_attributes: attrs,
Expand Down
4 changes: 0 additions & 4 deletions crates/lang/ir/src/ir/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ impl Contract {
///
/// - `types`: To specify `Environment` different from the default environment
/// types.
/// - `storage-alloc`: If `true` enables the dynamic storage allocator
/// facilities and code generation of the ink! smart
/// contract. Does incur some overhead. The default is
/// `true`.
/// - `as-dependency`: If `true` compiles this ink! smart contract always as
/// if it was a dependency of another smart contract.
/// This configuration is mainly needed for testing and
Expand Down
49 changes: 1 addition & 48 deletions crates/lang/macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,53 +119,6 @@ pub fn selector_bytes(input: TokenStream) -> TokenStream {
/// The `#[ink::contract]` macro can be provided with some additional comma-separated
/// header arguments:
///
/// - `dynamic_storage_allocator: bool`
///
/// Tells the ink! code generator to allow usage of ink!'s built-in dynamic
/// storage allocator.
/// - `true`: Use the dynamic storage allocator provided by ink!.
/// - `false`: Do NOT use the dynamic storage allocator provided by ink!.
///
/// This feature is generally only needed for smart contracts that try to model
/// their data in a way that contains storage entities within other storage
/// entities.
///
/// Contract writers should try to write smart contracts that do not depend on the
/// dynamic storage allocator since enabling it comes at a cost of increased Wasm
/// file size. Although it will enable interesting use cases. Use it with care!
///
/// An example for this is the following type that could potentially be used
/// within a contract's storage struct definition:
///
///
/// ```ignore
/// # // Tracking issue [#1119]: Right now we've hidden the `StorageVec` from public access so
/// # // this doesn't compile.
/// # use ink_storage as storage;
/// # type _unused =
/// storage::Vec<storage::Vec<i32>>
/// # ;
/// ```
///
/// **Usage Example:**
/// ```
/// # use ink_lang as ink;
/// #[ink::contract(dynamic_storage_allocator = true)]
/// mod my_contract {
/// # #[ink(storage)]
/// # pub struct MyStorage;
/// # impl MyStorage {
/// # #[ink(constructor)]
/// # pub fn construct() -> Self { MyStorage {} }
/// # #[ink(message)]
/// # pub fn message(&self) {}
/// # }
/// // ...
/// }
/// ```
///
/// **Default value:** `false`
///
/// - `compile_as_dependency: bool`
///
/// Tells the ink! code generator to **always** or **never**
Expand Down Expand Up @@ -710,7 +663,7 @@ pub fn contract(attr: TokenStream, item: TokenStream) -> TokenStream {
/// pub trait TraitDefinition {
/// #[ink(message)]
/// fn message1(&self);
///
///
/// #[ink(message, selector = 42)]
/// fn message2(&self);
/// }
Expand Down
38 changes: 5 additions & 33 deletions crates/lang/src/codegen/dispatch/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ use ink_primitives::{
Key,
KeyPtr,
};
use ink_storage::{
alloc,
alloc::ContractPhase,
traits::{
pull_spread_root,
push_spread_root,
SpreadAllocate,
SpreadLayout,
},
use ink_storage::traits::{
pull_spread_root,
push_spread_root,
SpreadAllocate,
SpreadLayout,
};

/// The root key of the ink! smart contract.
Expand Down Expand Up @@ -75,12 +71,6 @@ where
pub struct ExecuteConstructorConfig {
/// Yields `true` if the ink! constructor accepts payment.
pub payable: bool,
/// Yields `true` if the dynamic storage allocator has been enabled.
///
/// # Note
///
/// Authors can enable it via `#[ink::contract(dynamic_storage_allocator = true)]`.
pub dynamic_storage_alloc: bool,
}

/// Executes the given ink! constructor.
Expand All @@ -103,9 +93,6 @@ where
if !config.payable {
deny_payment::<<Contract as ContractEnv>::Env>()?;
}
if config.dynamic_storage_alloc {
alloc::initialize(ContractPhase::Deploy);
}
let result = ManuallyDrop::new(private::Seal(f()));
match result.as_result() {
Ok(contract) => {
Expand All @@ -114,9 +101,6 @@ where
// This requires us to sync back the changes of the contract storage.
let root_key = <Contract as ContractRootKey>::ROOT_KEY;
push_spread_root::<Contract>(contract, &root_key);
if config.dynamic_storage_alloc {
alloc::finalize();
}
Ok(())
}
Err(_) => {
Expand Down Expand Up @@ -292,12 +276,6 @@ pub struct ExecuteMessageConfig {
///
/// This is usually true for `&mut self` ink! messages.
pub mutates: bool,
/// Yields `true` if the dynamic storage allocator has been enabled.
///
/// # Note
///
/// Authors can enable it via `#[ink::contract(dynamic_storage_allocator = true)]`.
pub dynamic_storage_alloc: bool,
}

/// Initiates an ink! message call with the given configuration.
Expand All @@ -319,9 +297,6 @@ where
if !config.payable {
deny_payment::<<Contract as ContractEnv>::Env>()?;
}
if config.dynamic_storage_alloc {
alloc::initialize(ContractPhase::Call);
}
let root_key = Key::from([0x00; 32]);
let contract = pull_spread_root::<Contract>(&root_key);
Ok(contract)
Expand Down Expand Up @@ -374,9 +349,6 @@ where
let root_key = Key::from([0x00; 32]);
push_spread_root::<Contract>(contract, &root_key);
}
if config.dynamic_storage_alloc {
alloc::finalize();
}
if TypeId::of::<R>() != TypeId::of::<()>() {
// In case the return type is `()` we do not return a value.
ink_env::return_value::<R>(ReturnFlags::default(), result)
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading

0 comments on commit c94d39c

Please sign in to comment.