From 493b58bd4a475080d428ce47193ee9ea9757a808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 21 Sep 2022 19:31:22 +0200 Subject: [PATCH] Make automatic storage deposits resistant against changing deposit prices (#12083) * Require `FixedPointOperand` for Balances * Delay deposit calculation * Make refunds pro rata of consumed storage * Add storage migration * Fix clippy * Add liquidity checks * Fixe delayed deposit limit enforcement * Defer charges * Import Vec * Add try-runtime hooks for migration * Fix warning * Adapt to new OnRuntimeUpgrade trait * Apply suggestions from code review Co-authored-by: Sasha Gryaznov * fmt * Apply suggestions from code review Co-authored-by: Sasha Gryaznov * More suggestions from code review Co-authored-by: Sasha Gryaznov --- bin/node/runtime/src/lib.rs | 1 + frame/balances/src/lib.rs | 5 +- .../fixtures/create_storage_and_call.wat | 55 ++ frame/contracts/src/benchmarking/mod.rs | 18 +- frame/contracts/src/exec.rs | 23 +- frame/contracts/src/lib.rs | 33 +- frame/contracts/src/migration.rs | 198 ++++++- frame/contracts/src/storage.rs | 51 +- frame/contracts/src/storage/meter.rs | 495 ++++++++++-------- frame/contracts/src/tests.rs | 386 ++++++++++++-- frame/contracts/src/wasm/prepare.rs | 2 +- frame/support/src/traits/tokens/currency.rs | 4 +- 12 files changed, 958 insertions(+), 313 deletions(-) create mode 100644 frame/contracts/fixtures/create_storage_and_call.wat diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 0d8f00550ed1a..d6a9798b4d9ca 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1706,6 +1706,7 @@ pub type Executive = frame_executive::Executive< type Migrations = ( pallet_nomination_pools::migration::v2::MigrateToV2, pallet_alliance::migration::Migration, + pallet_contracts::Migration, ); /// MMR helper types. diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 82d6f87dbde1a..4014efdd26e4e 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -185,7 +185,7 @@ use sp_runtime::{ AtLeast32BitUnsigned, Bounded, CheckedAdd, CheckedSub, MaybeSerializeDeserialize, Saturating, StaticLookup, Zero, }, - ArithmeticError, DispatchError, RuntimeDebug, + ArithmeticError, DispatchError, FixedPointOperand, RuntimeDebug, }; use sp_std::{cmp, fmt::Debug, mem, ops::BitOr, prelude::*, result}; pub use weights::WeightInfo; @@ -212,7 +212,8 @@ pub mod pallet { + MaybeSerializeDeserialize + Debug + MaxEncodedLen - + TypeInfo; + + TypeInfo + + FixedPointOperand; /// Handler for the unbalanced reduction when removing a dust account. type DustRemoval: OnUnbalanced>; diff --git a/frame/contracts/fixtures/create_storage_and_call.wat b/frame/contracts/fixtures/create_storage_and_call.wat new file mode 100644 index 0000000000000..2a1e53f7ce08a --- /dev/null +++ b/frame/contracts/fixtures/create_storage_and_call.wat @@ -0,0 +1,55 @@ +;; This calls another contract as passed as its account id. It also creates some storage. +(module + (import "seal0" "seal_input" (func $seal_input (param i32 i32))) + (import "seal0" "seal_set_storage" (func $seal_set_storage (param i32 i32 i32))) + (import "seal1" "seal_call" (func $seal_call (param i32 i32 i64 i32 i32 i32 i32 i32) (result i32))) + (import "env" "memory" (memory 1 1)) + + (func $assert (param i32) + (block $ok + (br_if $ok + (get_local 0) + ) + (unreachable) + ) + ) + + (func (export "deploy")) + + (func (export "call") + ;; store length of input buffer + (i32.store (i32.const 0) (i32.const 512)) + + ;; copy input at address 4 + (call $seal_input (i32.const 4) (i32.const 0)) + + ;; create 4 byte of storage before calling + (call $seal_set_storage + (i32.const 0) ;; Pointer to storage key + (i32.const 0) ;; Pointer to value + (i32.const 4) ;; Size of value + ) + + ;; call passed contract + (call $assert (i32.eqz + (call $seal_call + (i32.const 0) ;; No flags + (i32.const 8) ;; Pointer to "callee" address. + (i64.const 0) ;; How much gas to devote for the execution. 0 = all. + (i32.const 512) ;; Pointer to the buffer with value to transfer + (i32.const 4) ;; Pointer to input data buffer address + (i32.const 4) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case + ) + )) + + ;; create 8 byte of storage after calling + ;; item of 12 bytes because we override 4 bytes + (call $seal_set_storage + (i32.const 0) ;; Pointer to storage key + (i32.const 0) ;; Pointer to value + (i32.const 12) ;; Size of value + ) + ) +) diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 4a987fd8dde78..186ac6f63503e 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -87,7 +87,7 @@ where module: WasmModule, data: Vec, ) -> Result, &'static str> { - let value = T::Currency::minimum_balance(); + let value = Pallet::::min_balance(); T::Currency::make_free_balance_be(&caller, caller_funding::()); let salt = vec![0xff]; let addr = Contracts::::contract_address(&caller, &module.hash, &salt); @@ -257,7 +257,7 @@ benchmarks! { let instance = Contract::::with_caller( whitelisted_caller(), WasmModule::sized(c, Location::Deploy), vec![], )?; - let value = T::Currency::minimum_balance(); + let value = Pallet::::min_balance(); let origin = RawOrigin::Signed(instance.caller.clone()); let callee = instance.addr; }: call(origin, callee, value, Weight::MAX, None, vec![]) @@ -280,7 +280,7 @@ benchmarks! { let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get()); let s in 0 .. code::max_pages::() * 64 * 1024; let salt = vec![42u8; s as usize]; - let value = T::Currency::minimum_balance(); + let value = Pallet::::min_balance(); let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, caller_funding::()); let WasmModule { code, hash, .. } = WasmModule::::sized(c, Location::Call); @@ -307,7 +307,7 @@ benchmarks! { instantiate { let s in 0 .. code::max_pages::() * 64 * 1024; let salt = vec![42u8; s as usize]; - let value = T::Currency::minimum_balance(); + let value = Pallet::::min_balance(); let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, caller_funding::()); let WasmModule { code, hash, .. } = WasmModule::::dummy(); @@ -338,7 +338,7 @@ benchmarks! { let instance = Contract::::with_caller( whitelisted_caller(), WasmModule::dummy(), vec![], )?; - let value = T::Currency::minimum_balance(); + let value = Pallet::::min_balance(); let origin = RawOrigin::Signed(instance.caller.clone()); let callee = instance.addr.clone(); let before = T::Currency::free_balance(&instance.account_id); @@ -767,13 +767,13 @@ benchmarks! { let instance = Contract::::new(code, vec![])?; let origin = RawOrigin::Signed(instance.caller.clone()); assert_eq!(T::Currency::total_balance(&beneficiary), 0u32.into()); - assert_eq!(T::Currency::free_balance(&instance.account_id), T::Currency::minimum_balance()); + assert_eq!(T::Currency::free_balance(&instance.account_id), Pallet::::min_balance()); assert_ne!(T::Currency::reserved_balance(&instance.account_id), 0u32.into()); }: call(origin, instance.addr.clone(), 0u32.into(), Weight::MAX, None, vec![]) verify { if r > 0 { assert_eq!(T::Currency::total_balance(&instance.account_id), 0u32.into()); - assert_eq!(T::Currency::total_balance(&beneficiary), T::Currency::minimum_balance()); + assert_eq!(T::Currency::total_balance(&beneficiary), Pallet::::min_balance()); } } @@ -1469,7 +1469,7 @@ benchmarks! { .collect::>(); let account_len = accounts.get(0).map(|i| i.encode().len()).unwrap_or(0); let account_bytes = accounts.iter().flat_map(|x| x.encode()).collect(); - let value = T::Currency::minimum_balance(); + let value = Pallet::::min_balance(); assert!(value > 0u32.into()); let value_bytes = value.encode(); let value_len = value_bytes.len(); @@ -1705,7 +1705,7 @@ benchmarks! { let hash_len = hashes.get(0).map(|x| x.encode().len()).unwrap_or(0); let hashes_bytes = hashes.iter().flat_map(|x| x.encode()).collect::>(); let hashes_len = hashes_bytes.len(); - let value = T::Currency::minimum_balance(); + let value = Pallet::::min_balance(); assert!(value > 0u32.into()); let value_bytes = value.encode(); let value_len = value_bytes.len(); diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 38b47a915cddc..6260dd41de707 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -813,6 +813,15 @@ where return Ok(output) } + // Storage limit is enforced as late as possible (when the last frame returns) so that + // the ordering of storage accesses does not matter. + if self.frames.is_empty() { + let frame = &mut self.first_frame; + frame.contract_info.load(&frame.account_id); + let contract = frame.contract_info.as_contract(); + frame.nested_storage.enforce_limit(contract)?; + } + let frame = self.top_frame(); let account_id = &frame.account_id; match (entry_point, delegated_code_hash) { @@ -911,12 +920,7 @@ where // it was invalidated. frame.contract_info.load(account_id); let mut contract = frame.contract_info.into_contract(); - prev.nested_storage.absorb( - frame.nested_storage, - &self.origin, - account_id, - contract.as_mut(), - ); + prev.nested_storage.absorb(frame.nested_storage, account_id, contract.as_mut()); // In case the contract wasn't terminated we need to persist changes made to it. if let Some(contract) = contract { @@ -954,7 +958,6 @@ where let mut contract = self.first_frame.contract_info.as_contract(); self.storage_meter.absorb( mem::take(&mut self.first_frame.nested_storage), - &self.origin, &self.first_frame.account_id, contract.as_deref_mut(), ); @@ -2354,10 +2357,10 @@ mod tests { let code_bob = MockLoader::insert(Call, |ctx, _| { if ctx.input_data[0] == 0 { let info = ctx.ext.contract_info(); - assert_eq!(info.storage_deposit, 0); - info.storage_deposit = 42; + assert_eq!(info.storage_byte_deposit, 0); + info.storage_byte_deposit = 42; assert_eq!(ctx.ext.call(Weight::zero(), CHARLIE, 0, vec![], true), exec_trapped()); - assert_eq!(ctx.ext.contract_info().storage_deposit, 42); + assert_eq!(ctx.ext.contract_info().storage_byte_deposit, 42); } exec_success() }); diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index abe91a9046ce2..fc44e4507ca00 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -87,12 +87,12 @@ mod gas; mod benchmarking; mod exec; +mod migration; mod schedule; mod storage; mod wasm; pub mod chain_extension; -pub mod migration; pub mod weights; #[cfg(test)] @@ -109,7 +109,10 @@ use codec::{Encode, HasCompact}; use frame_support::{ dispatch::{DispatchClass, Dispatchable, GetDispatchInfo, Pays, PostDispatchInfo}, ensure, - traits::{ConstU32, Contains, Currency, Get, Randomness, ReservableCurrency, Time}, + traits::{ + tokens::fungible::Inspect, ConstU32, Contains, Currency, Get, Randomness, + ReservableCurrency, Time, + }, weights::Weight, BoundedVec, WeakBoundedVec, }; @@ -126,6 +129,7 @@ use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; pub use crate::{ exec::{Frame, VarSizedKey as StorageKey}, + migration::Migration, pallet::*, schedule::{HostFnWeights, InstructionWeights, Limits, Schedule}, }; @@ -225,7 +229,7 @@ pub mod pallet { use frame_system::pallet_prelude::*; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(7); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(8); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] @@ -236,11 +240,12 @@ pub mod pallet { /// The time implementation used to supply timestamps to contracts through `seal_now`. type Time: Time; - /// The generator used to supply randomness to contracts through `seal_random`. + /// The generator used to supply randomness to contracts through `seal_random` type Randomness: Randomness; /// The currency in which fees are paid and contract balances are held. - type Currency: ReservableCurrency; + type Currency: ReservableCurrency + + Inspect>; /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -1036,7 +1041,7 @@ where }; let schedule = T::Schedule::get(); let result = ExecStack::>::run_call( - origin, + origin.clone(), dest, &mut gas_meter, &mut storage_meter, @@ -1045,7 +1050,11 @@ where data, debug_message, ); - InternalCallOutput { result, gas_meter, storage_deposit: storage_meter.into_deposit() } + InternalCallOutput { + result, + gas_meter, + storage_deposit: storage_meter.into_deposit(&origin), + } } /// Internal function that does the actual instantiation. @@ -1089,7 +1098,7 @@ where value.saturating_add(extra_deposit), )?; let result = ExecStack::>::run_instantiate( - origin, + origin.clone(), executable, &mut gas_meter, &mut storage_meter, @@ -1100,17 +1109,23 @@ where debug_message, ); storage_deposit = storage_meter - .into_deposit() + .into_deposit(&origin) .saturating_add(&StorageDeposit::Charge(extra_deposit)); result }; InternalInstantiateOutput { result: try_exec(), gas_meter, storage_deposit } } + /// Deposit a pallet contracts event. Handles the conversion to the overarching event type. fn deposit_event(topics: Vec, event: Event) { >::deposit_event_indexed( &topics, ::RuntimeEvent::from(event).into(), ) } + + /// Return the existential deposit of [`Config::Currency`]. + fn min_balance() -> BalanceOf { + >>::minimum_balance() + } } diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 0db285e81a91f..5ea821aac7682 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -18,37 +18,80 @@ use crate::{BalanceOf, CodeHash, Config, Pallet, TrieId, Weight}; use codec::{Decode, Encode}; use frame_support::{ - codec, pallet_prelude::*, storage::migration, storage_alias, traits::Get, Identity, - Twox64Concat, + codec, + pallet_prelude::*, + storage::migration, + storage_alias, + traits::{Get, OnRuntimeUpgrade}, + Identity, Twox64Concat, }; +use sp_runtime::traits::Saturating; use sp_std::{marker::PhantomData, prelude::*}; -/// Wrapper for all migrations of this pallet, based on `StorageVersion`. -pub fn migrate() -> Weight { - let version = StorageVersion::get::>(); - let mut weight = Weight::zero(); +/// Performs all necessary migrations based on `StorageVersion`. +pub struct Migration(PhantomData); +impl OnRuntimeUpgrade for Migration { + fn on_runtime_upgrade() -> Weight { + let version = StorageVersion::get::>(); + let mut weight = Weight::zero(); - if version < 4 { - weight = weight.saturating_add(v4::migrate::()); - StorageVersion::new(4).put::>(); - } + if version < 4 { + weight = weight.saturating_add(v4::migrate::()); + StorageVersion::new(4).put::>(); + } - if version < 5 { - weight = weight.saturating_add(v5::migrate::()); - StorageVersion::new(5).put::>(); - } + if version < 5 { + weight = weight.saturating_add(v5::migrate::()); + StorageVersion::new(5).put::>(); + } + + if version < 6 { + weight = weight.saturating_add(v6::migrate::()); + StorageVersion::new(6).put::>(); + } + + if version < 7 { + weight = weight.saturating_add(v7::migrate::()); + StorageVersion::new(7).put::>(); + } + + if version < 8 { + weight = weight.saturating_add(v8::migrate::()); + StorageVersion::new(8).put::>(); + } - if version < 6 { - weight = weight.saturating_add(v6::migrate::()); - StorageVersion::new(6).put::>(); + weight } - if version < 7 { - weight = weight.saturating_add(v7::migrate::()); - StorageVersion::new(7).put::>(); + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + let version = StorageVersion::get::>(); + + if version < 7 { + return Ok(vec![]) + } + + if version < 8 { + v8::pre_upgrade::()?; + } + + Ok(vec![]) } - weight + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + let version = StorageVersion::get::>(); + + if version < 7 { + return Ok(()) + } + + if version < 8 { + v8::post_upgrade::()?; + } + + Ok(()) + } } /// V4: `Schedule` is changed to be a config item rather than an in-storage value. @@ -185,9 +228,9 @@ mod v6 { #[derive(Encode, Decode)] pub struct RawContractInfo { - trie_id: TrieId, - code_hash: CodeHash, - storage_deposit: Balance, + pub trie_id: TrieId, + pub code_hash: CodeHash, + pub storage_deposit: Balance, } #[derive(Encode, Decode)] @@ -199,7 +242,7 @@ mod v6 { refcount: u64, } - type ContractInfo = RawContractInfo, BalanceOf>; + pub type ContractInfo = RawContractInfo, BalanceOf>; #[storage_alias] type ContractInfoOf = StorageMap< @@ -266,3 +309,108 @@ mod v7 { T::DbWeight::get().reads_writes(1, 2) } } + +/// Update `ContractInfo` with new fields that track storage deposits. +mod v8 { + use super::*; + use sp_io::default_child_storage as child; + use v6::ContractInfo as OldContractInfo; + + #[derive(Encode, Decode)] + struct ContractInfo { + trie_id: TrieId, + code_hash: CodeHash, + storage_bytes: u32, + storage_items: u32, + storage_byte_deposit: BalanceOf, + storage_item_deposit: BalanceOf, + storage_base_deposit: BalanceOf, + } + + #[storage_alias] + type ContractInfoOf = + StorageMap, Twox64Concat, ::AccountId, V>; + + pub fn migrate() -> Weight { + let mut weight = Weight::zero(); + + >>::translate_values(|old: OldContractInfo| { + // Count storage items of this contract + let mut storage_bytes = 0u32; + let mut storage_items = 0u32; + let mut key = Vec::new(); + while let Some(next) = child::next_key(&old.trie_id, &key) { + key = next; + let mut val_out = []; + let len = child::read(&old.trie_id, &key, &mut val_out, 0) + .expect("The loop conditions checks for existence of the key; qed"); + storage_bytes.saturating_accrue(len); + storage_items.saturating_accrue(1); + } + + let storage_byte_deposit = + T::DepositPerByte::get().saturating_mul(storage_bytes.into()); + let storage_item_deposit = + T::DepositPerItem::get().saturating_mul(storage_items.into()); + let storage_base_deposit = old + .storage_deposit + .saturating_sub(storage_byte_deposit) + .saturating_sub(storage_item_deposit); + + // Reads: One read for each storage item plus the contract info itself. + // Writes: Only the new contract info. + weight = weight + .saturating_add(T::DbWeight::get().reads_writes(u64::from(storage_items) + 1, 1)); + + Some(ContractInfo { + trie_id: old.trie_id, + code_hash: old.code_hash, + storage_bytes, + storage_items, + storage_byte_deposit, + storage_item_deposit, + storage_base_deposit, + }) + }); + + weight + } + + #[cfg(feature = "try-runtime")] + pub fn pre_upgrade() -> Result<(), &'static str> { + use frame_support::traits::ReservableCurrency; + for (key, value) in ContractInfoOf::>::iter() { + let reserved = T::Currency::reserved_balance(&key); + ensure!(reserved >= value.storage_deposit, "Reserved balance out of sync."); + } + Ok(()) + } + + #[cfg(feature = "try-runtime")] + pub fn post_upgrade() -> Result<(), &'static str> { + use frame_support::traits::ReservableCurrency; + for (key, value) in ContractInfoOf::>::iter() { + let reserved = T::Currency::reserved_balance(&key); + let stored = value + .storage_base_deposit + .saturating_add(value.storage_byte_deposit) + .saturating_add(value.storage_item_deposit); + ensure!(reserved >= stored, "Reserved balance out of sync."); + + let mut storage_bytes = 0u32; + let mut storage_items = 0u32; + let mut key = Vec::new(); + while let Some(next) = child::next_key(&value.trie_id, &key) { + key = next; + let mut val_out = []; + let len = child::read(&value.trie_id, &key, &mut val_out, 0) + .expect("The loop conditions checks for existence of the key; qed"); + storage_bytes.saturating_accrue(len); + storage_items.saturating_accrue(1); + } + ensure!(storage_bytes == value.storage_bytes, "Storage bytes do not match.",); + ensure!(storage_items == value.storage_items, "Storage items do not match.",); + } + Ok(()) + } +} diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs index ec49a6325c125..c3fc0840d8649 100644 --- a/frame/contracts/src/storage.rs +++ b/frame/contracts/src/storage.rs @@ -34,31 +34,51 @@ use scale_info::TypeInfo; use sp_core::crypto::UncheckedFrom; use sp_io::KillStorageResult; use sp_runtime::{ - traits::{Hash, Zero}, + traits::{Hash, Saturating, Zero}, RuntimeDebug, }; use sp_std::{marker::PhantomData, prelude::*}; -pub type ContractInfo = RawContractInfo, BalanceOf>; - /// Information for managing an account and its sub trie abstraction. /// This is the required info to cache for an account. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)] -pub struct RawContractInfo { +#[scale_info(skip_type_params(T))] +pub struct ContractInfo { /// Unique ID for the subtree encoded as a bytes vector. pub trie_id: TrieId, /// The code associated with a given account. - pub code_hash: CodeHash, - /// The amount of balance that is currently deposited to pay for consumed storage. - pub storage_deposit: Balance, + pub code_hash: CodeHash, + /// How many bytes of storage are accumulated in this contract's child trie. + pub storage_bytes: u32, + /// How many items of storage are accumulated in this contract's child trie. + pub storage_items: u32, + /// This records to how much deposit the accumulated `storage_bytes` amount to. + pub storage_byte_deposit: BalanceOf, + /// This records to how much deposit the accumulated `storage_items` amount to. + pub storage_item_deposit: BalanceOf, + /// This records how much deposit is put down in order to pay for the contract itself. + /// + /// We need to store this information separately so it is not used when calculating any refunds + /// since the base deposit can only ever be refunded on contract termination. + pub storage_base_deposit: BalanceOf, } -impl RawContractInfo { +impl ContractInfo { /// Associated child trie unique id is built from the hash part of the trie id. #[cfg(test)] pub fn child_trie_info(&self) -> ChildInfo { child_trie_info(&self.trie_id[..]) } + + /// The deposit paying for the accumulated storage generated within the contract's child trie. + pub fn extra_deposit(&self) -> BalanceOf { + self.storage_byte_deposit.saturating_add(self.storage_item_deposit) + } + + /// Same as [`Self::extra_deposit`] but including the base deposit. + pub fn total_deposit(&self) -> BalanceOf { + self.extra_deposit().saturating_add(self.storage_base_deposit) + } } /// Associated child trie unique id is built from the hash part of the trie id. @@ -178,7 +198,7 @@ where }, (None, None) => (), } - storage_meter.charge(&diff)?; + storage_meter.charge(&diff); } match &new_value { @@ -200,14 +220,21 @@ where pub fn new_contract( account: &AccountIdOf, trie_id: TrieId, - ch: CodeHash, + code_hash: CodeHash, ) -> Result, DispatchError> { if >::contains_key(account) { return Err(Error::::DuplicateContract.into()) } - let contract = - ContractInfo:: { code_hash: ch, trie_id, storage_deposit: >::zero() }; + let contract = ContractInfo:: { + code_hash, + trie_id, + storage_bytes: 0, + storage_items: 0, + storage_byte_deposit: Zero::zero(), + storage_item_deposit: Zero::zero(), + storage_base_deposit: Zero::zero(), + }; Ok(contract) } diff --git a/frame/contracts/src/storage/meter.rs b/frame/contracts/src/storage/meter.rs index d89208812bcac..0a63eb42b86cb 100644 --- a/frame/contracts/src/storage/meter.rs +++ b/frame/contracts/src/storage/meter.rs @@ -17,17 +17,24 @@ //! This module contains functions to meter the storage deposit. -use crate::{storage::ContractInfo, BalanceOf, Config, Error}; +use crate::{storage::ContractInfo, BalanceOf, Config, Error, Inspect, Pallet}; use codec::Encode; use frame_support::{ dispatch::DispatchError, - traits::{tokens::BalanceStatus, Currency, ExistenceRequirement, Get, ReservableCurrency}, - DefaultNoBound, + ensure, + traits::{ + tokens::{BalanceStatus, WithdrawConsequence}, + Currency, ExistenceRequirement, Get, ReservableCurrency, + }, + DefaultNoBound, RuntimeDebugNoBound, }; use pallet_contracts_primitives::StorageDeposit as Deposit; use sp_core::crypto::UncheckedFrom; -use sp_runtime::traits::{Saturating, Zero}; -use sp_std::marker::PhantomData; +use sp_runtime::{ + traits::{Saturating, Zero}, + FixedPointNumber, FixedU128, +}; +use sp_std::{marker::PhantomData, vec::Vec}; /// Deposit that uses the native currency's balance type. pub type DepositOf = Deposit>; @@ -99,22 +106,25 @@ impl State for Root {} impl State for Nested {} /// A type that allows the metering of consumed or freed storage of a single contract call stack. -#[derive(DefaultNoBound)] -pub struct RawMeter, S: State> { +#[derive(DefaultNoBound, RuntimeDebugNoBound)] +pub struct RawMeter { /// The limit of how much balance this meter is allowed to consume. limit: BalanceOf, /// The amount of balance that was used in this meter and all of its already absorbed children. total_deposit: DepositOf, - /// The amount of balance that was used in this meter alone. - own_deposit: DepositOf, - /// Only when a contract was terminated we allow it to drop below the minimum balance. - terminated: bool, + /// The amount of storage changes that were recorded in this meter alone. + own_contribution: Contribution, + /// List of charges that should be applied at the end of a contract stack execution. + /// + /// We only have one charge per contract hence the size of this vector is + /// limited by the maximum call depth. + charges: Vec>, /// Type parameters are only used in impls. _phantom: PhantomData<(E, S)>, } /// This type is used to describe a storage change when charging from the meter. -#[derive(Default)] +#[derive(Default, RuntimeDebugNoBound)] pub struct Diff { /// How many bytes were added to storage. pub bytes_added: u32, @@ -124,43 +134,120 @@ pub struct Diff { pub items_added: u32, /// How many storage items were removed from storage. pub items_removed: u32, - /// If set to true the derived deposit will always a `Charge` larger than the - /// the existential deposit. - pub require_ed: bool, } impl Diff { - /// Calculate how much of a charge or refund results from applying the diff. - pub fn to_deposit(&self) -> DepositOf { - let mut deposit = Deposit::default(); + /// Calculate how much of a charge or refund results from applying the diff and store it + /// in the passed `info` if any. + /// + /// # Note + /// + /// In case `None` is passed for `info` only charges are calculated. This is because refunds + /// are calculated pro rata of the existing storage within a contract and hence need extract + /// this information from the passed `info`. + pub fn update_contract(&self, info: Option<&mut ContractInfo>) -> DepositOf { let per_byte = T::DepositPerByte::get(); let per_item = T::DepositPerItem::get(); + let bytes_added = self.bytes_added.saturating_sub(self.bytes_removed); + let items_added = self.items_added.saturating_sub(self.items_removed); + let mut bytes_deposit = Deposit::Charge(per_byte.saturating_mul((bytes_added).into())); + let mut items_deposit = Deposit::Charge(per_item.saturating_mul((items_added).into())); + + // Without any contract info we can only calculate diffs which add storage + let info = if let Some(info) = info { + info + } else { + debug_assert_eq!(self.bytes_removed, 0); + debug_assert_eq!(self.items_removed, 0); + return bytes_deposit.saturating_add(&items_deposit) + }; - if self.bytes_added > self.bytes_removed { - deposit = deposit.saturating_add(&Deposit::Charge( - per_byte.saturating_mul((self.bytes_added - self.bytes_removed).into()), - )); - } else if self.bytes_removed > self.bytes_added { - deposit = deposit.saturating_add(&Deposit::Refund( - per_byte.saturating_mul((self.bytes_removed - self.bytes_added).into()), - )); + // Refunds are calculated pro rata based on the accumulated storage within the contract + let bytes_removed = self.bytes_removed.saturating_sub(self.bytes_added); + let items_removed = self.items_removed.saturating_sub(self.items_added); + let ratio = FixedU128::checked_from_rational(bytes_removed, info.storage_bytes) + .unwrap_or_default() + .min(FixedU128::from_u32(1)); + bytes_deposit = bytes_deposit + .saturating_add(&Deposit::Refund(ratio.saturating_mul_int(info.storage_byte_deposit))); + let ratio = FixedU128::checked_from_rational(items_removed, info.storage_items) + .unwrap_or_default() + .min(FixedU128::from_u32(1)); + items_deposit = items_deposit + .saturating_add(&Deposit::Refund(ratio.saturating_mul_int(info.storage_item_deposit))); + + // We need to update the contract info structure with the new deposits + info.storage_bytes = + info.storage_bytes.saturating_add(bytes_added).saturating_sub(bytes_removed); + info.storage_items = + info.storage_items.saturating_add(items_added).saturating_sub(items_removed); + match &bytes_deposit { + Deposit::Charge(amount) => + info.storage_byte_deposit = info.storage_byte_deposit.saturating_add(*amount), + Deposit::Refund(amount) => + info.storage_byte_deposit = info.storage_byte_deposit.saturating_sub(*amount), } + match &items_deposit { + Deposit::Charge(amount) => + info.storage_item_deposit = info.storage_item_deposit.saturating_add(*amount), + Deposit::Refund(amount) => + info.storage_item_deposit = info.storage_item_deposit.saturating_sub(*amount), + } + + bytes_deposit.saturating_add(&items_deposit) + } +} - if self.items_added > self.items_removed { - deposit = deposit.saturating_add(&Deposit::Charge( - per_item.saturating_mul((self.items_added - self.items_removed).into()), - )); - } else if self.items_removed > self.items_added { - deposit = deposit.saturating_add(&Deposit::Refund( - per_item.saturating_mul((self.items_removed - self.items_added).into()), - )); +impl Diff { + fn saturating_add(&self, rhs: &Self) -> Self { + Self { + bytes_added: self.bytes_added.saturating_add(rhs.bytes_added), + bytes_removed: self.bytes_removed.saturating_add(rhs.bytes_removed), + items_added: self.items_added.saturating_add(rhs.items_added), + items_removed: self.items_removed.saturating_add(rhs.items_removed), } + } +} + +/// Records information to charge or refund a plain account. +/// +/// All the charges are deferred to the end of a whole call stack. Reason is that by doing +/// this we can do all the refunds before doing any charge. This way a plain account can use +/// more deposit than it has balance as along as it is covered by a refund. This +/// essentially makes the order of storage changes irrelevant with regard to the deposit system. +#[derive(RuntimeDebugNoBound, Clone)] +struct Charge { + contract: T::AccountId, + amount: DepositOf, + terminated: bool, +} + +/// Records the storage changes of a storage meter. +#[derive(RuntimeDebugNoBound)] +enum Contribution { + /// The contract the meter belongs to is alive and accumulates changes using a [`Diff`]. + Alive(Diff), + /// The meter was checked against its limit using [`RawMeter::enforce_limit`] at the end of + /// its execution. In this process the [`Diff`] was converted into a [`Deposit`]. + Checked(DepositOf), + /// The contract was terminated. In this process the [`Diff`] was converted into a [`Deposit`] + /// in order to calculate the refund. + Terminated(DepositOf), +} - if self.require_ed { - deposit = deposit.max(Deposit::Charge(T::Currency::minimum_balance())) +impl Contribution { + /// See [`Diff::update_contract`]. + fn update_contract(&self, info: Option<&mut ContractInfo>) -> DepositOf { + match self { + Self::Alive(diff) => diff.update_contract::(info), + Self::Terminated(deposit) | Self::Checked(deposit) => deposit.clone(), } + } +} - deposit +impl Default for Contribution { + fn default() -> Self { + Self::Alive(Default::default()) } } @@ -178,6 +265,7 @@ where /// usage for this sub call separately. This is necessary because we want to exchange balance /// with the current contract we are interacting with. pub fn nested(&self) -> RawMeter { + debug_assert!(self.is_alive()); RawMeter { limit: self.available(), ..Default::default() } } @@ -192,45 +280,28 @@ where /// /// # Parameters /// - /// `absorbed`: The child storage meter that should be absorbed. - /// `origin`: The origin that spawned the original root meter. - /// `contract`: The contract that this sub call belongs to. - /// `info`: The info of the contract in question. `None` if the contract was terminated. + /// - `absorbed`: The child storage meter that should be absorbed. + /// - `origin`: The origin that spawned the original root meter. + /// - `contract`: The contract that this sub call belongs to. + /// - `info`: The info of the contract in question. `None` if the contract was terminated. pub fn absorb( &mut self, - mut absorbed: RawMeter, - origin: &T::AccountId, + absorbed: RawMeter, contract: &T::AccountId, info: Option<&mut ContractInfo>, ) { - // Absorbing from an existing (non terminated) contract. - if let Some(info) = info { - match &mut absorbed.own_deposit { - Deposit::Charge(amount) => - info.storage_deposit = info.storage_deposit.saturating_add(*amount), - Deposit::Refund(amount) => { - // We need to make sure to never refund more than what was deposited and - // still leave the existential deposit inside the contract's account. - // This case can happen when costs change due to a runtime upgrade where - // increased costs could remove an account due to refunds. - let amount = { - let corrected_amount = (*amount).min( - info.storage_deposit.saturating_sub(T::Currency::minimum_balance()), - ); - let correction = (*amount).saturating_sub(corrected_amount); - absorbed.total_deposit = - absorbed.total_deposit.saturating_sub(&Deposit::Refund(correction)); - *amount = corrected_amount; - corrected_amount - }; - info.storage_deposit = info.storage_deposit.saturating_sub(amount); - }, - } - } - - self.total_deposit = self.total_deposit.saturating_add(&absorbed.total_deposit); - if !absorbed.own_deposit.is_zero() { - E::charge(origin, contract, &absorbed.own_deposit, absorbed.terminated); + let own_deposit = absorbed.own_contribution.update_contract(info); + self.total_deposit = self + .total_deposit + .saturating_add(&absorbed.total_deposit) + .saturating_add(&own_deposit); + if !own_deposit.is_zero() { + self.charges.extend_from_slice(&absorbed.charges); + self.charges.push(Charge { + contract: contract.clone(), + amount: own_deposit, + terminated: absorbed.is_terminated(), + }); } } @@ -238,6 +309,16 @@ where fn available(&self) -> BalanceOf { self.total_deposit.available(&self.limit) } + + /// True if the contract is alive. + fn is_alive(&self) -> bool { + matches!(self.own_contribution, Contribution::Alive(_)) + } + + /// True if the contract is terminated. + fn is_terminated(&self) -> bool { + matches!(self.own_contribution, Contribution::Terminated(_)) + } } /// Functions that only apply to the root state. @@ -260,11 +341,18 @@ where } /// The total amount of deposit that should change hands as result of the execution - /// that this meter was passed into. + /// that this meter was passed into. This will also perform all the charges accumulated + /// in the whole contract stack. /// /// This drops the root meter in order to make sure it is only called when the whole /// execution did finish. - pub fn into_deposit(self) -> DepositOf { + pub fn into_deposit(self, origin: &T::AccountId) -> DepositOf { + for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Refund(_))) { + E::charge(origin, &charge.contract, &charge.amount, charge.terminated); + } + for charge in self.charges.iter().filter(|c| matches!(c.amount, Deposit::Charge(_))) { + E::charge(origin, &charge.contract, &charge.amount, charge.terminated); + } self.total_deposit } } @@ -277,18 +365,12 @@ where E: Ext, { /// Try to charge the `diff` from the meter. Fails if this would exceed the original limit. - pub fn charge(&mut self, diff: &Diff) -> Result, DispatchError> { - debug_assert!(!self.terminated); - let deposit = diff.to_deposit::(); - let total_deposit = self.total_deposit.saturating_add(&deposit); - if let Deposit::Charge(amount) = total_deposit { - if amount > self.limit { - return Err(>::StorageDepositLimitExhausted.into()) - } - } - self.total_deposit = total_deposit; - self.own_deposit = self.own_deposit.saturating_add(&deposit); - Ok(deposit) + pub fn charge(&mut self, diff: &Diff) { + debug_assert!(self.is_alive()); + match &mut self.own_contribution { + Contribution::Alive(own) => *own = own.saturating_add(diff), + _ => panic!("Charge is never called after termination; qed"), + }; } /// Charge from `origin` a storage deposit for contract instantiation. @@ -300,25 +382,22 @@ where contract: &T::AccountId, info: &mut ContractInfo, ) -> Result, DispatchError> { - debug_assert!(!self.terminated); - let deposit = Diff { - bytes_added: info.encoded_size() as u32, - items_added: 1, - require_ed: true, - ..Default::default() + debug_assert!(self.is_alive()); + let mut deposit = + Diff { bytes_added: info.encoded_size() as u32, items_added: 1, ..Default::default() } + .update_contract::(None); + + // Instantiate needs to transfer the minimum balance at least in order to pull the + // contract's account into existence. + deposit = deposit.max(Deposit::Charge(Pallet::::min_balance())); + if deposit.charge_or_zero() > self.limit { + return Err(>::StorageDepositLimitExhausted.into()) } - .to_deposit::(); - debug_assert!(matches!(deposit, Deposit::Charge(_))); - // We do not increase `own_deposit` because this will be charged later when the contract - // execution does conclude. - let total_deposit = self.total_deposit.saturating_add(&deposit); - if let Deposit::Charge(amount) = &total_deposit { - if amount > &self.limit { - return Err(>::StorageDepositLimitExhausted.into()) - } - } - info.storage_deposit = info.storage_deposit.saturating_add(deposit.charge_or_zero()); - self.total_deposit = total_deposit; + + // We do not increase `own_contribution` because this will be charged later when the + // contract execution does conclude and hence would lead to a double charge. + self.total_deposit = deposit.clone(); + info.storage_base_deposit = deposit.charge_or_zero(); if !deposit.is_zero() { // We need to charge immediately so that the account is created before the `value` // is transferred from the caller to the contract. @@ -331,34 +410,55 @@ where /// /// This will manipulate the meter so that all storage deposit accumulated in /// `contract_info` will be refunded to the `origin` of the meter. - pub fn terminate(&mut self, contract_info: &ContractInfo) { - debug_assert!(!self.terminated); - let refund = Deposit::Refund(contract_info.storage_deposit); - - // The deposit for `own_deposit` isn't persisted into the contract info until the current - // frame is dropped. This means that whatever changes were introduced during the - // current frame are dicarded when terminating. - self.total_deposit = - self.total_deposit.saturating_add(&refund).saturating_sub(&self.own_deposit); - self.own_deposit = refund; - self.terminated = true; + pub fn terminate(&mut self, info: &ContractInfo) { + debug_assert!(self.is_alive()); + self.own_contribution = Contribution::Terminated(Deposit::Refund(info.total_deposit())); + } + + /// [`Self::charge`] does not enforce the storage limit since we want to do this check as late + /// as possible to allow later refunds to offset earlier charges. + /// + /// # Note + /// + /// We only need to call this **once** for every call stack and not for every cross contract + /// call. Hence this is only called when the last call frame returns. + pub fn enforce_limit( + &mut self, + info: Option<&mut ContractInfo>, + ) -> Result<(), DispatchError> { + let deposit = self.own_contribution.update_contract(info); + let total_deposit = self.total_deposit.saturating_add(&deposit); + // We don't want to override a `Terminated` with a `Checked`. + if self.is_alive() { + self.own_contribution = Contribution::Checked(deposit); + } + if let Deposit::Charge(amount) = total_deposit { + if amount > self.limit { + return Err(>::StorageDepositLimitExhausted.into()) + } + } + Ok(()) } } -impl Ext for ReservingExt { +impl Ext for ReservingExt +where + T: Config, + T::AccountId: UncheckedFrom + AsRef<[u8]>, +{ fn check_limit( origin: &T::AccountId, limit: Option>, min_leftover: BalanceOf, ) -> Result, DispatchError> { - let max = T::Currency::free_balance(origin) - .saturating_sub(T::Currency::minimum_balance()) - .saturating_sub(min_leftover); - match limit { - Some(limit) if limit <= max => Ok(limit), - None => Ok(max), - _ => Err(>::StorageDepositNotEnoughFunds.into()), - } + let max = T::Currency::reducible_balance(origin, true).saturating_sub(min_leftover); + let limit = limit.unwrap_or(max); + ensure!( + limit <= max && + matches!(T::Currency::can_withdraw(origin, limit), WithdrawConsequence::Success), + >::StorageDepositNotEnoughFunds, + ); + Ok(limit) } fn charge( @@ -393,6 +493,9 @@ impl Ext for ReservingExt { "Failed to transfer storage deposit {:?} from origin {:?} to contract {:?}: {:?}", amount, origin, contract, err, ); + if cfg!(debug_assertions) { + panic!("Unable to collect storage deposit. This is a bug."); + } } }, // For `Refund(_)` no error happen because the initial value transfer from the @@ -411,7 +514,7 @@ impl Ext for ReservingExt { // refund because we consider this unexpected behaviour. *amount.min( &T::Currency::reserved_balance(contract) - .saturating_sub(T::Currency::minimum_balance()), + .saturating_sub(Pallet::::min_balance()), ) }; let result = @@ -422,6 +525,9 @@ impl Ext for ReservingExt { "Failed to repatriate storage deposit {:?} from contract {:?} to origin {:?}: {:?}", amount, contract, origin, result, ); + if cfg!(debug_assertions) { + panic!("Unable to refund storage deposit. This is a bug."); + } } }, }; @@ -513,14 +619,26 @@ mod tests { TestExtTestValue::mutate(|ext| ext.clear()) } - fn new_info(deposit: BalanceOf) -> ContractInfo { + #[derive(Default)] + struct StorageInfo { + bytes: u32, + items: u32, + bytes_deposit: BalanceOf, + items_deposit: BalanceOf, + } + + fn new_info(info: StorageInfo) -> ContractInfo { use crate::storage::Storage; use sp_runtime::traits::Hash; ContractInfo:: { trie_id: >::generate_trie_id(&ALICE, 42), code_hash: ::Hashing::hash(b"42"), - storage_deposit: deposit, + storage_bytes: info.bytes, + storage_items: info.items, + storage_byte_deposit: info.bytes_deposit, + storage_item_deposit: info.items_deposit, + storage_base_deposit: Default::default(), } } @@ -548,8 +666,8 @@ mod tests { // an empty charge does not create a `Charge` entry let mut nested0 = meter.nested(); - nested0.charge(&Default::default()).unwrap(); - meter.absorb(nested0, &ALICE, &BOB, None); + nested0.charge(&Default::default()); + meter.absorb(nested0, &BOB, None); assert_eq!( TestExtTestValue::get(), @@ -560,79 +678,49 @@ mod tests { ) } - #[test] - fn existential_deposit_works() { - clear_ext(); - - let mut meter = TestMeter::new(&ALICE, Some(1_000), 0).unwrap(); - assert_eq!(meter.available(), 1_000); - - // a `Refund` will be turned into a `Charge(ed)` which is intended behaviour - let mut nested0 = meter.nested(); - nested0.charge(&Diff { require_ed: true, ..Default::default() }).unwrap(); - nested0 - .charge(&Diff { bytes_removed: 1, require_ed: true, ..Default::default() }) - .unwrap(); - meter.absorb(nested0, &ALICE, &BOB, None); - - assert_eq!( - TestExtTestValue::get(), - TestExt { - limit_checks: vec![LimitCheck { origin: ALICE, limit: 1_000, min_leftover: 0 }], - charges: vec![Charge { - origin: ALICE, - contract: BOB, - amount: Deposit::Charge(::Currency::minimum_balance() * 2), - terminated: false, - }] - } - ) - } - #[test] fn charging_works() { clear_ext(); - let min_balance = ::Currency::minimum_balance(); - - let mut meter = TestMeter::new(&ALICE, Some(1_000), 0).unwrap(); - assert_eq!(meter.available(), 1_000); + let mut meter = TestMeter::new(&ALICE, Some(100), 0).unwrap(); + assert_eq!(meter.available(), 100); - let mut nested0_info = new_info(100); + let mut nested0_info = + new_info(StorageInfo { bytes: 100, items: 5, bytes_deposit: 100, items_deposit: 10 }); let mut nested0 = meter.nested(); - nested0 - .charge(&Diff { - bytes_added: 10, - bytes_removed: 5, - items_added: 1, - items_removed: 2, - ..Default::default() - }) - .unwrap(); - nested0.charge(&Diff { bytes_removed: 1, ..Default::default() }).unwrap(); + nested0.charge(&Diff { + bytes_added: 108, + bytes_removed: 5, + items_added: 1, + items_removed: 2, + }); + nested0.charge(&Diff { bytes_removed: 99, ..Default::default() }); - let mut nested1_info = new_info(50); + let mut nested1_info = + new_info(StorageInfo { bytes: 100, items: 10, bytes_deposit: 100, items_deposit: 20 }); let mut nested1 = nested0.nested(); - nested1.charge(&Diff { items_removed: 5, ..Default::default() }).unwrap(); - nested0.absorb(nested1, &ALICE, &CHARLIE, Some(&mut nested1_info)); + nested1.charge(&Diff { items_removed: 5, ..Default::default() }); + nested0.absorb(nested1, &CHARLIE, Some(&mut nested1_info)); - // Trying to refund more than is available in the contract will cap the charge - // to (deposit_in_contract - ed). - let mut nested2_info = new_info(5); + let mut nested2_info = + new_info(StorageInfo { bytes: 100, items: 7, bytes_deposit: 100, items_deposit: 20 }); let mut nested2 = nested0.nested(); - nested2.charge(&Diff { bytes_removed: 7, ..Default::default() }).unwrap(); - nested0.absorb(nested2, &ALICE, &CHARLIE, Some(&mut nested2_info)); + nested2.charge(&Diff { items_removed: 7, ..Default::default() }); + nested0.absorb(nested2, &CHARLIE, Some(&mut nested2_info)); + + nested0.enforce_limit(Some(&mut nested0_info)).unwrap(); + meter.absorb(nested0, &BOB, Some(&mut nested0_info)); - meter.absorb(nested0, &ALICE, &BOB, Some(&mut nested0_info)); + meter.into_deposit(&ALICE); - assert_eq!(nested0_info.storage_deposit, 102); - assert_eq!(nested1_info.storage_deposit, 40); - assert_eq!(nested2_info.storage_deposit, min_balance); + assert_eq!(nested0_info.extra_deposit(), 112); + assert_eq!(nested1_info.extra_deposit(), 110); + assert_eq!(nested2_info.extra_deposit(), 100); assert_eq!( TestExtTestValue::get(), TestExt { - limit_checks: vec![LimitCheck { origin: ALICE, limit: 1_000, min_leftover: 0 }], + limit_checks: vec![LimitCheck { origin: ALICE, limit: 100, min_leftover: 0 }], charges: vec![ Charge { origin: ALICE, @@ -643,7 +731,7 @@ mod tests { Charge { origin: ALICE, contract: CHARLIE, - amount: Deposit::Refund(4), + amount: Deposit::Refund(20), terminated: false }, Charge { @@ -665,26 +753,25 @@ mod tests { assert_eq!(meter.available(), 1_000); let mut nested0 = meter.nested(); - nested0 - .charge(&Diff { - bytes_added: 5, - bytes_removed: 1, - items_added: 3, - items_removed: 1, - ..Default::default() - }) - .unwrap(); - nested0.charge(&Diff { items_added: 2, ..Default::default() }).unwrap(); - - let nested1_info = new_info(400); + nested0.charge(&Diff { + bytes_added: 5, + bytes_removed: 1, + items_added: 3, + items_removed: 1, + }); + nested0.charge(&Diff { items_added: 2, ..Default::default() }); + + let mut nested1_info = + new_info(StorageInfo { bytes: 100, items: 10, bytes_deposit: 100, items_deposit: 20 }); let mut nested1 = nested0.nested(); - nested1.charge(&Diff { items_removed: 5, ..Default::default() }).unwrap(); - nested1.charge(&Diff { bytes_added: 20, ..Default::default() }).unwrap(); + nested1.charge(&Diff { items_removed: 5, ..Default::default() }); + nested1.charge(&Diff { bytes_added: 20, ..Default::default() }); nested1.terminate(&nested1_info); - nested0.absorb(nested1, &ALICE, &CHARLIE, None); + nested0.enforce_limit(Some(&mut nested1_info)).unwrap(); + nested0.absorb(nested1, &CHARLIE, None); - meter.absorb(nested0, &ALICE, &BOB, None); - drop(meter); + meter.absorb(nested0, &BOB, None); + meter.into_deposit(&ALICE); assert_eq!( TestExtTestValue::get(), @@ -694,7 +781,7 @@ mod tests { Charge { origin: ALICE, contract: CHARLIE, - amount: Deposit::Refund(400), + amount: Deposit::Refund(120), terminated: true }, Charge { diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 12029abd59e46..a56e4f5564845 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -23,6 +23,7 @@ use crate::{ }, exec::{FixSizedKey, Frame}, storage::Storage, + tests::test_utils::{get_contract, get_contract_checked}, wasm::{PrefabWasmModule, ReturnCode as RuntimeReturnCode}, weights::WeightInfo, BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator, @@ -36,8 +37,8 @@ use frame_support::{ parameter_types, storage::child, traits::{ - BalanceStatus, ConstU32, ConstU64, Contains, Currency, Get, OnIdle, OnInitialize, - ReservableCurrency, + BalanceStatus, ConstU32, ConstU64, Contains, Currency, Get, LockableCurrency, OnIdle, + OnInitialize, ReservableCurrency, WithdrawReasons, }, weights::{constants::WEIGHT_PER_SECOND, Weight}, }; @@ -76,7 +77,9 @@ frame_support::construct_runtime!( #[macro_use] pub mod test_utils { use super::{Balances, Hash, SysConfig, Test}; - use crate::{exec::AccountIdOf, storage::Storage, CodeHash, Config, ContractInfoOf, Nonce}; + use crate::{ + exec::AccountIdOf, storage::Storage, CodeHash, Config, ContractInfo, ContractInfoOf, Nonce, + }; use codec::Encode; use frame_support::traits::Currency; @@ -97,6 +100,12 @@ pub mod test_utils { pub fn get_balance(who: &AccountIdOf) -> u64 { Balances::free_balance(who) } + pub fn get_contract(addr: &AccountIdOf) -> ContractInfo { + get_contract_checked(addr).unwrap() + } + pub fn get_contract_checked(addr: &AccountIdOf) -> Option> { + ContractInfoOf::::get(addr) + } pub fn hash(s: &S) -> <::Hashing as Hash>::Output { <::Hashing as Hash>::hash_of(s) } @@ -105,6 +114,7 @@ pub mod test_utils { assert_eq!(u32::from_le_bytes($x.data[..].try_into().unwrap()), $y as u32); }}; } + macro_rules! assert_refcount { ( $code_hash:expr , $should:expr $(,)? ) => {{ let is = crate::OwnerInfoOf::::get($code_hash).map(|m| m.refcount()).unwrap(); @@ -693,7 +703,7 @@ fn instantiate_unique_trie_id() { vec![], vec![], )); - let trie_id = ContractInfoOf::::get(&addr).unwrap().trie_id; + let trie_id = get_contract(&addr).trie_id; // Try to instantiate it again without termination should yield an error. assert_err_ignore_postinfo!( @@ -731,7 +741,7 @@ fn instantiate_unique_trie_id() { )); // Trie ids shouldn't match or we might have a collision - assert_ne!(trie_id, ContractInfoOf::::get(&addr).unwrap().trie_id); + assert_ne!(trie_id, get_contract(&addr).trie_id); }); } @@ -752,7 +762,7 @@ fn storage_max_value_limit() { vec![], )); let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); - ContractInfoOf::::get(&addr).unwrap(); + get_contract(&addr); // Call contract with allowed storage value. assert_ok!(Contracts::call( @@ -962,7 +972,7 @@ fn cannot_self_destruct_through_draning() { let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); // Check that the BOB contract has been instantiated. - assert_matches!(ContractInfoOf::::get(&addr), Some(_)); + get_contract(&addr); // Call BOB which makes it send all funds to the zero address // The contract code asserts that the transfer was successful @@ -1003,11 +1013,11 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() { let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); // Check that the BOB contract has been instantiated and has the minimum balance - let info = ContractInfoOf::::get(&addr).unwrap(); - assert_eq!(info.storage_deposit, min_balance); + assert_eq!(get_contract(&addr).total_deposit(), min_balance); + assert_eq!(get_contract(&addr).extra_deposit(), 0); assert_eq!(::Currency::total_balance(&addr), min_balance); - // Create 100 bytes of storage with a price of per byte + // Create 100 bytes of storage with a price of per byte and a single storage item of price 2 assert_ok!(Contracts::call( RuntimeOrigin::signed(ALICE), addr.clone(), @@ -1016,9 +1026,10 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() { None, 100u32.to_le_bytes().to_vec() )); + assert_eq!(get_contract(&addr).total_deposit(), min_balance + 102); - // Increase the byte price and trigger a refund. This could potentially destroy the account - // because the refund removes the reserved existential deposit. This should not happen. + // Increase the byte price and trigger a refund. This should not have any influence because + // the removal is pro rata and exactly those 100 bytes should have been removed. DEPOSIT_PER_BYTE.with(|c| *c.borrow_mut() = 500); assert_ok!(Contracts::call( RuntimeOrigin::signed(ALICE), @@ -1032,8 +1043,9 @@ fn cannot_self_destruct_through_storage_refund_after_price_change() { // Make sure the account wasn't removed by the refund assert_eq!( ::Currency::total_balance(&addr), - ::Currency::minimum_balance(), + get_contract(&addr).total_deposit(), ); + assert_eq!(get_contract(&addr).extra_deposit(), 2,); }); } @@ -1086,9 +1098,6 @@ fn cannot_self_destruct_by_refund_after_slash() { // Make sure the account kept the minimum balance and was not destroyed assert_eq!(::Currency::total_balance(&addr), min_balance); - // even though it was not charged it is still substracted from the storage deposit tracker - assert_eq!(ContractInfoOf::::get(&addr).unwrap().storage_deposit, 550); - assert_eq!( System::events(), vec![ @@ -1142,7 +1151,7 @@ fn cannot_self_destruct_while_live() { let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); // Check that the BOB contract has been instantiated. - assert_matches!(ContractInfoOf::::get(&addr), Some(_)); + get_contract(&addr); // Call BOB with input data, forcing it make a recursive call to itself to // self-destruct, resulting in a trap. @@ -1159,7 +1168,7 @@ fn cannot_self_destruct_while_live() { ); // Check that BOB is still there. - assert_matches!(ContractInfoOf::::get(&addr), Some(_)); + get_contract(&addr); }); } @@ -1183,7 +1192,7 @@ fn self_destruct_works() { let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); // Check that the BOB contract has been instantiated. - assert_matches!(ContractInfoOf::::get(&addr), Some(_)); + get_contract(&addr); // Drop all previous events initialize_block(2); @@ -1198,7 +1207,7 @@ fn self_destruct_works() { assert_refcount!(&code_hash, 0); // Check that account is gone - assert!(ContractInfoOf::::get(&addr).is_none()); + assert!(get_contract_checked(&addr).is_none()); assert_eq!(Balances::total_balance(&addr), 0); // check that the beneficiary (django) got remaining balance @@ -1289,7 +1298,7 @@ fn destroy_contract_and_transfer_funds() { let addr_charlie = Contracts::contract_address(&addr_bob, &callee_code_hash, &[0x47, 0x11]); // Check that the CHARLIE contract has been instantiated. - assert_matches!(ContractInfoOf::::get(&addr_charlie), Some(_)); + get_contract(&addr_charlie); // Call BOB, which calls CHARLIE, forcing CHARLIE to self-destruct. assert_ok!(Contracts::call( @@ -1302,7 +1311,7 @@ fn destroy_contract_and_transfer_funds() { )); // Check that CHARLIE has moved on to the great beyond (ie. died). - assert!(ContractInfoOf::::get(&addr_charlie).is_none()); + assert!(get_contract_checked(&addr_charlie).is_none()); }); } @@ -1862,7 +1871,7 @@ fn lazy_removal_works() { ),); let addr = Contracts::contract_address(&ALICE, &hash, &[]); - let info = >::get(&addr).unwrap(); + let info = get_contract(&addr); let trie = &info.child_trie_info(); // Put value into the contracts child trie @@ -1931,7 +1940,7 @@ fn lazy_batch_removal_works() { ),); let addr = Contracts::contract_address(&ALICE, &hash, &[i]); - let info = >::get(&addr).unwrap(); + let info = get_contract(&addr); let trie = &info.child_trie_info(); // Put value into the contracts child trie @@ -1993,7 +2002,7 @@ fn lazy_removal_partial_remove_works() { ),); let addr = Contracts::contract_address(&ALICE, &hash, &[]); - let info = >::get(&addr).unwrap(); + let info = get_contract(&addr); // Put value into the contracts child trie for val in &vals { @@ -2105,7 +2114,7 @@ fn lazy_removal_does_no_run_on_low_remaining_weight() { ),); let addr = Contracts::contract_address(&ALICE, &hash, &[]); - let info = >::get(&addr).unwrap(); + let info = get_contract(&addr); let trie = &info.child_trie_info(); // Put value into the contracts child trie @@ -2173,7 +2182,7 @@ fn lazy_removal_does_not_use_all_weight() { ),); let addr = Contracts::contract_address(&ALICE, &hash, &[]); - let info = >::get(&addr).unwrap(); + let info = get_contract(&addr); let (weight_per_key, max_keys) = Storage::::deletion_budget(1, weight_limit); // We create a contract with one less storage item than we can remove within the limit @@ -2264,7 +2273,7 @@ fn deletion_queue_full() { ); // Contract should exist because removal failed - >::get(&addr).unwrap(); + get_contract(&addr); }); } @@ -2909,7 +2918,7 @@ fn instantiate_with_zero_balance_works() { let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); // Check that the BOB contract has been instantiated. - assert_matches!(ContractInfoOf::::get(&addr), Some(_)); + get_contract(&addr); // Make sure the account exists even though no free balance was send assert_eq!(::Currency::free_balance(&addr), 0,); @@ -3002,7 +3011,7 @@ fn instantiate_with_below_existential_deposit_works() { let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); // Check that the BOB contract has been instantiated. - assert_matches!(ContractInfoOf::::get(&addr), Some(_)); + get_contract(&addr); // Make sure the account exists even though no free balance was send assert_eq!(::Currency::free_balance(&addr), 50,); @@ -3114,7 +3123,7 @@ fn storage_deposit_works() { // 4 is for creating 2 storage items let charged0 = 4 + 1_000 + 5_000; deposit += charged0; - assert_eq!(>::get(&addr).unwrap().storage_deposit, deposit); + assert_eq!(get_contract(&addr).total_deposit(), deposit); // Add more storage (but also remove some) assert_ok!(Contracts::call( @@ -3127,7 +3136,7 @@ fn storage_deposit_works() { )); let charged1 = 1_000 - 100; deposit += charged1; - assert_eq!(>::get(&addr).unwrap().storage_deposit, deposit); + assert_eq!(get_contract(&addr).total_deposit(), deposit); // Remove more storage (but also add some) assert_ok!(Contracts::call( @@ -3138,9 +3147,10 @@ fn storage_deposit_works() { None, (2_100u32, 900u32).encode(), )); - let refunded0 = 4_000 - 100; + // -1 for numeric instability + let refunded0 = 4_000 - 100 - 1; deposit -= refunded0; - assert_eq!(>::get(&addr).unwrap().storage_deposit, deposit); + assert_eq!(get_contract(&addr).total_deposit(), deposit); assert_eq!( System::events(), @@ -3253,7 +3263,7 @@ fn set_code_extrinsic() { // Drop previous events initialize_block(2); - assert_eq!(>::get(&addr).unwrap().code_hash, code_hash); + assert_eq!(get_contract(&addr).code_hash, code_hash); assert_refcount!(&code_hash, 1); assert_refcount!(&new_code_hash, 0); @@ -3262,7 +3272,7 @@ fn set_code_extrinsic() { Contracts::set_code(RuntimeOrigin::signed(ALICE), addr.clone(), new_code_hash), sp_runtime::traits::BadOrigin, ); - assert_eq!(>::get(&addr).unwrap().code_hash, code_hash); + assert_eq!(get_contract(&addr).code_hash, code_hash); assert_refcount!(&code_hash, 1); assert_refcount!(&new_code_hash, 0); assert_eq!(System::events(), vec![],); @@ -3272,7 +3282,7 @@ fn set_code_extrinsic() { Contracts::set_code(RuntimeOrigin::root(), BOB, new_code_hash), >::ContractNotFound, ); - assert_eq!(>::get(&addr).unwrap().code_hash, code_hash); + assert_eq!(get_contract(&addr).code_hash, code_hash); assert_refcount!(&code_hash, 1); assert_refcount!(&new_code_hash, 0); assert_eq!(System::events(), vec![],); @@ -3282,14 +3292,14 @@ fn set_code_extrinsic() { Contracts::set_code(RuntimeOrigin::root(), addr.clone(), Default::default()), >::CodeNotFound, ); - assert_eq!(>::get(&addr).unwrap().code_hash, code_hash); + assert_eq!(get_contract(&addr).code_hash, code_hash); assert_refcount!(&code_hash, 1); assert_refcount!(&new_code_hash, 0); assert_eq!(System::events(), vec![],); // successful call assert_ok!(Contracts::set_code(RuntimeOrigin::root(), addr.clone(), new_code_hash)); - assert_eq!(>::get(&addr).unwrap().code_hash, new_code_hash); + assert_eq!(get_contract(&addr).code_hash, new_code_hash); assert_refcount!(&code_hash, 0); assert_refcount!(&new_code_hash, 1); assert_eq!( @@ -3639,3 +3649,301 @@ fn set_code_hash() { ); }); } + +#[test] +fn storage_deposit_limit_is_enforced() { + let (wasm, code_hash) = compile_module::("store").unwrap(); + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + let min_balance = ::Currency::minimum_balance(); + + // Instantiate the BOB contract. + assert_ok!(Contracts::instantiate_with_code( + RuntimeOrigin::signed(ALICE), + 0, + GAS_LIMIT, + None, + wasm, + vec![], + vec![], + )); + let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); + + // Check that the BOB contract has been instantiated and has the minimum balance + assert_eq!(get_contract(&addr).total_deposit(), min_balance); + assert_eq!(::Currency::total_balance(&addr), min_balance); + + // Create 100 bytes of storage with a price of per byte + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(ALICE), + addr.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(1)), + 100u32.to_le_bytes().to_vec() + ), + >::StorageDepositLimitExhausted, + ); + }); +} + +#[test] +fn storage_deposit_limit_is_enforced_late() { + let (wasm_caller, code_hash_caller) = + compile_module::("create_storage_and_call").unwrap(); + let (wasm_callee, code_hash_callee) = compile_module::("store").unwrap(); + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + + // Create both contracts: Constructors do nothing. + assert_ok!(Contracts::instantiate_with_code( + RuntimeOrigin::signed(ALICE), + 0, + GAS_LIMIT, + None, + wasm_caller, + vec![], + vec![], + )); + let addr_caller = Contracts::contract_address(&ALICE, &code_hash_caller, &[]); + assert_ok!(Contracts::instantiate_with_code( + RuntimeOrigin::signed(ALICE), + 0, + GAS_LIMIT, + None, + wasm_callee, + vec![], + vec![], + )); + let addr_callee = Contracts::contract_address(&ALICE, &code_hash_callee, &[]); + + // Create 100 bytes of storage with a price of per byte + // This is 100 Balance + 2 Balance for the item + assert_ok!(Contracts::call( + RuntimeOrigin::signed(ALICE), + addr_callee.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(102)), + 100u32.to_le_bytes().to_vec() + )); + + // We do not remove any storage but require 14 bytes of storage for the new + // storage created in the immediate contract. + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(ALICE), + addr_caller.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(5)), + 100u32 + .to_le_bytes() + .as_ref() + .iter() + .chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee)) + .cloned() + .collect(), + ), + >::StorageDepositLimitExhausted, + ); + + // Allow for the additional 14 bytes but demand an additional byte in the callee contract. + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(ALICE), + addr_caller.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(14)), + 101u32 + .to_le_bytes() + .as_ref() + .iter() + .chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee)) + .cloned() + .collect(), + ), + >::StorageDepositLimitExhausted, + ); + + // Refund in the callee contract but not enough to cover the 14 balance required by the + // caller. + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(ALICE), + addr_caller.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(0)), + 87u32 + .to_le_bytes() + .as_ref() + .iter() + .chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee)) + .cloned() + .collect(), + ), + >::StorageDepositLimitExhausted, + ); + + let _ = Balances::make_free_balance_be(&ALICE, 1_000); + + // Send more than the sender has balance. + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(ALICE), + addr_caller.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(50)), + 1_200u32 + .to_le_bytes() + .as_ref() + .iter() + .chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee)) + .cloned() + .collect(), + ), + >::StorageDepositLimitExhausted, + ); + + // Same as above but allow for the additional balance. + assert_ok!(Contracts::call( + RuntimeOrigin::signed(ALICE), + addr_caller.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(1)), + 87u32 + .to_le_bytes() + .as_ref() + .iter() + .chain(<_ as AsRef<[u8]>>::as_ref(&addr_callee)) + .cloned() + .collect(), + )); + }); +} + +#[test] +fn deposit_limit_honors_liquidity_restrictions() { + let (wasm, code_hash) = compile_module::("store").unwrap(); + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + let _ = Balances::deposit_creating(&BOB, 1_000); + let min_balance = ::Currency::minimum_balance(); + + // Instantiate the BOB contract. + assert_ok!(Contracts::instantiate_with_code( + RuntimeOrigin::signed(ALICE), + 0, + GAS_LIMIT, + None, + wasm, + vec![], + vec![], + )); + let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); + + // Check that the contract has been instantiated and has the minimum balance + assert_eq!(get_contract(&addr).total_deposit(), min_balance); + assert_eq!(::Currency::total_balance(&addr), min_balance); + + // check that the lock ins honored + Balances::set_lock([0; 8], &BOB, 1_000, WithdrawReasons::TRANSFER); + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(BOB), + addr.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(200)), + 100u32.to_le_bytes().to_vec() + ), + >::StorageDepositNotEnoughFunds, + ); + assert_eq!(Balances::free_balance(&BOB), 1_000); + }); +} + +#[test] +fn deposit_limit_honors_existential_deposit() { + let (wasm, code_hash) = compile_module::("store").unwrap(); + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + let _ = Balances::deposit_creating(&BOB, 1_000); + let min_balance = ::Currency::minimum_balance(); + + // Instantiate the BOB contract. + assert_ok!(Contracts::instantiate_with_code( + RuntimeOrigin::signed(ALICE), + 0, + GAS_LIMIT, + None, + wasm, + vec![], + vec![], + )); + let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); + + // Check that the contract has been instantiated and has the minimum balance + assert_eq!(get_contract(&addr).total_deposit(), min_balance); + assert_eq!(::Currency::total_balance(&addr), min_balance); + + // check that the deposit can't bring the account below the existential deposit + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(BOB), + addr.clone(), + 0, + GAS_LIMIT, + Some(codec::Compact(900)), + 100u32.to_le_bytes().to_vec() + ), + >::StorageDepositNotEnoughFunds, + ); + assert_eq!(Balances::free_balance(&BOB), 1_000); + }); +} + +#[test] +fn deposit_limit_honors_min_leftover() { + let (wasm, code_hash) = compile_module::("store").unwrap(); + ExtBuilder::default().existential_deposit(200).build().execute_with(|| { + let _ = Balances::deposit_creating(&ALICE, 1_000_000); + let _ = Balances::deposit_creating(&BOB, 1_000); + let min_balance = ::Currency::minimum_balance(); + + // Instantiate the BOB contract. + assert_ok!(Contracts::instantiate_with_code( + RuntimeOrigin::signed(ALICE), + 0, + GAS_LIMIT, + None, + wasm, + vec![], + vec![], + )); + let addr = Contracts::contract_address(&ALICE, &code_hash, &[]); + + // Check that the contract has been instantiated and has the minimum balance + assert_eq!(get_contract(&addr).total_deposit(), min_balance); + assert_eq!(::Currency::total_balance(&addr), min_balance); + + // check that the minumum leftover (value send) is considered + assert_err_ignore_postinfo!( + Contracts::call( + RuntimeOrigin::signed(BOB), + addr.clone(), + 400, + GAS_LIMIT, + Some(codec::Compact(500)), + 100u32.to_le_bytes().to_vec() + ), + >::StorageDepositNotEnoughFunds, + ); + assert_eq!(Balances::free_balance(&BOB), 1_000); + }); +} diff --git a/frame/contracts/src/wasm/prepare.rs b/frame/contracts/src/wasm/prepare.rs index 9c130b562c275..e8873f604c9c7 100644 --- a/frame/contracts/src/wasm/prepare.rs +++ b/frame/contracts/src/wasm/prepare.rs @@ -426,7 +426,7 @@ fn do_preparation( .saturating_add(original_code_len) .saturating_add(>::max_encoded_len()) as u32; let deposit = Diff { bytes_added, items_added: 3, ..Default::default() } - .to_deposit::() + .update_contract::(None) .charge_or_zero(); module.owner_info = Some(OwnerInfo { owner, deposit, refcount: 0 }); diff --git a/frame/support/src/traits/tokens/currency.rs b/frame/support/src/traits/tokens/currency.rs index 9a1634fd89313..d0beb66d34923 100644 --- a/frame/support/src/traits/tokens/currency.rs +++ b/frame/support/src/traits/tokens/currency.rs @@ -26,7 +26,7 @@ use crate::{ traits::Get, }; use codec::MaxEncodedLen; -use sp_runtime::traits::MaybeSerializeDeserialize; +use sp_runtime::{traits::MaybeSerializeDeserialize, FixedPointOperand}; use sp_std::fmt::Debug; mod reservable; @@ -37,7 +37,7 @@ pub use lockable::{LockIdentifier, LockableCurrency, VestingSchedule}; /// Abstraction over a fungible assets system. pub trait Currency { /// The balance of an account. - type Balance: Balance + MaybeSerializeDeserialize + Debug + MaxEncodedLen; + type Balance: Balance + MaybeSerializeDeserialize + Debug + MaxEncodedLen + FixedPointOperand; /// The opaque token type for an imbalance. This is returned by unbalanced operations /// and must be dealt with. It may be dropped but cannot be cloned.