diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index 27b843c5e1..67e2a4375e 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -16,17 +16,15 @@ use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, TrieId, BalanceOf, ContractInfo, TrieIdGenerator}; -use crate::gas::{Gas, GasMeter, Token}; -use crate::rent; -use crate::storage; +use crate::{gas::{Gas, GasMeter, Token}, rent, storage, Error, ContractInfoOf}; use bitflags::bitflags; - use sp_std::prelude::*; -use sp_runtime::traits::{Bounded, Zero, Convert}; +use sp_runtime::traits::{Bounded, Zero, Convert, Saturating}; use frame_support::{ dispatch::DispatchError, traits::{ExistenceRequirement, Currency, Time, Randomness}, weights::Weight, + ensure, StorageMap, }; pub type AccountIdOf = ::AccountId; @@ -46,6 +44,15 @@ bitflags! { } } +/// Describes whether we deal with a contract or a plain account. +pub enum TransactorKind { + /// Transaction was initiated from a plain account. That can be either be through a + /// signed transaction or through RPC. + PlainAccount, + /// The call was initiated by a contract account. + Contract, +} + /// Output of a contract call or instantiation which ran to completion. #[cfg_attr(test, derive(PartialEq, Eq, Debug))] pub struct ExecReturnValue { @@ -320,6 +327,7 @@ where Err("contract has been evicted")? }; + let transactor_kind = self.transactor_kind(); let caller = self.self_account.clone(); let dest_trie_id = contract_info.and_then(|i| i.as_alive().map(|i| i.trie_id.clone())); @@ -328,6 +336,7 @@ where transfer( gas_meter, TransferCause::Call, + transactor_kind, &caller, &dest, value, @@ -372,6 +381,7 @@ where Err("not enough gas to pay base instantiate fee")? } + let transactor_kind = self.transactor_kind(); let caller = self.self_account.clone(); let dest = T::DetermineContractAddress::contract_address_for( code_hash, @@ -399,6 +409,7 @@ where transfer( gas_meter, TransferCause::Instantiate, + transactor_kind, &caller, &dest, endowment, @@ -466,6 +477,17 @@ where &self.self_account == account || self.caller.map_or(false, |caller| caller.is_live(account)) } + + fn transactor_kind(&self) -> TransactorKind { + if self.depth == 0 { + debug_assert!(self.self_trie_id.is_none()); + debug_assert!(self.caller.is_none()); + debug_assert!(ContractInfoOf::::get(&self.self_account).is_none()); + TransactorKind::PlainAccount + } else { + TransactorKind::Contract + } + } } #[cfg_attr(test, derive(Debug, PartialEq, Eq))] @@ -519,6 +541,7 @@ enum TransferCause { fn transfer<'a, T: Trait, V: Vm, L: Loader>( gas_meter: &mut GasMeter, cause: TransferCause, + origin: TransactorKind, transactor: &T::AccountId, dest: &T::AccountId, value: BalanceOf, @@ -526,6 +549,7 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( ) -> Result<(), DispatchError> { use self::TransferCause::*; use self::TransferFeeKind::*; + use self::TransactorKind::*; let token = { let kind: TransferFeeKind = match cause { @@ -545,10 +569,19 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( Err("not enough gas to pay transfer fee")? } - // Only ext_terminate is allowed to bring the sender below the existential deposit - let existence_requirement = match cause { - Terminate => ExistenceRequirement::AllowDeath, - _ => ExistenceRequirement::KeepAlive, + // Only ext_terminate is allowed to bring the sender below the subsistence + // threshold or even existential deposit. + let existence_requirement = match (cause, origin) { + (Terminate, _) => ExistenceRequirement::AllowDeath, + (_, Contract) => { + ensure!( + T::Currency::total_balance(transactor).saturating_sub(value) >= + ctx.config.subsistence_threshold(), + Error::::InsufficientBalance, + ); + ExistenceRequirement::KeepAlive + }, + (_, PlainAccount) => ExistenceRequirement::KeepAlive, }; T::Currency::transfer(transactor, dest, value, existence_requirement)?; @@ -630,6 +663,7 @@ where transfer( gas_meter, TransferCause::Call, + TransactorKind::Contract, &self.ctx.self_account.clone(), &to, value, @@ -654,6 +688,7 @@ where transfer( gas_meter, TransferCause::Terminate, + TransactorKind::Contract, &self_id, beneficiary, value, diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index c00e07c062..18c88c02b4 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -102,7 +102,7 @@ use sp_std::{prelude::*, marker::PhantomData, fmt::Debug}; use codec::{Codec, Encode, Decode}; use sp_runtime::{ traits::{ - Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, Convert, + Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, Convert, Saturating, }, RuntimeDebug, }; @@ -415,6 +415,11 @@ decl_error! { OutOfGas, /// The output buffer supplied to a contract API call was too small. OutputBufferTooSmall, + /// Performing the requested transfer would have brought the contract below + /// the subsistence threshold. No transfer is allowed to do this in order to allow + /// for a tombstone to be created. Use `ext_terminate` to remove a contract without + /// leaving a tombstone behind. + InsufficientBalance, } } @@ -726,6 +731,25 @@ impl Config { max_value_size: T::MaxValueSize::get(), } } + + /// Subsistence threshold is the extension of the minimum balance (aka existential deposit) by the + /// tombstone deposit, required for leaving a tombstone. + /// + /// Rent or any contract initiated balance transfer mechanism cannot make the balance lower + /// than the subsistence threshold in order to guarantee that a tombstone is created. + /// + /// The only way to completely kill a contract without a tombstone is calling `ext_terminate`. + fn subsistence_threshold(&self) -> BalanceOf { + self.existential_deposit.saturating_add(self.tombstone_deposit) + } + + /// The same as `subsistence_threshold` but without the need for a preloaded instance. + /// + /// This is for cases where this value is needed in rent calculation rather than + /// during contract execution. + fn subsistence_threshold_uncached() -> BalanceOf { + T::Currency::minimum_balance().saturating_add(T::TombstoneDeposit::get()) + } } /// Definition of the cost schedule and other parameterizations for wasm vm. diff --git a/substrate/frame/contracts/src/rent.rs b/substrate/frame/contracts/src/rent.rs index 6afd85aa8e..a3f582810a 100644 --- a/substrate/frame/contracts/src/rent.rs +++ b/substrate/frame/contracts/src/rent.rs @@ -18,7 +18,7 @@ use crate::{ AliveContractInfo, BalanceOf, ContractInfo, ContractInfoOf, Module, RawEvent, - TombstoneContractInfo, Trait, CodeHash, + TombstoneContractInfo, Trait, CodeHash, Config }; use sp_std::prelude::*; use sp_io::hashing::blake2_256; @@ -87,10 +87,10 @@ enum Verdict { /// This function accounts for the storage rent deposit. I.e. if the contract possesses enough funds /// then the fee can drop to zero. fn compute_fee_per_block( - balance: &BalanceOf, + free_balance: &BalanceOf, contract: &AliveContractInfo, ) -> BalanceOf { - let free_storage = balance + let free_storage = free_balance .checked_div(&T::RentDepositOffset::get()) .unwrap_or_else(Zero::zero); @@ -107,30 +107,27 @@ fn compute_fee_per_block( .unwrap_or(>::max_value()) } -/// Subsistence threshold is the extension of the minimum balance (aka existential deposit) by the -/// tombstone deposit, required for leaving a tombstone. -/// -/// Rent mechanism cannot make the balance lower than subsistence threshold. -fn subsistence_threshold() -> BalanceOf { - T::Currency::minimum_balance() + T::TombstoneDeposit::get() -} - /// Returns amount of funds available to consume by rent mechanism. /// /// Rent mechanism cannot consume more than `rent_allowance` set by the contract and it cannot make /// the balance lower than [`subsistence_threshold`]. /// -/// In case the balance is below the subsistence threshold, this function returns `None`. +/// In case the toal_balance is below the subsistence threshold, this function returns `None`. fn rent_budget( - balance: &BalanceOf, + total_balance: &BalanceOf, + free_balance: &BalanceOf, contract: &AliveContractInfo, ) -> Option> { - let subsistence_threshold = subsistence_threshold::(); - if *balance < subsistence_threshold { + let subsistence_threshold = Config::::subsistence_threshold_uncached(); + // Reserved balance contributes towards the subsistence threshold to stay consistent + // with the existential deposit where the reserved balance is also counted. + if *total_balance < subsistence_threshold { return None; } - let rent_allowed_to_charge = *balance - subsistence_threshold; + // However, reserved balance cannot be charged so we need to use the free balance + // to calculate the actual budget (which can be 0). + let rent_allowed_to_charge = free_balance.saturating_sub(subsistence_threshold); Some(>::min( contract.rent_allowance, rent_allowed_to_charge, @@ -158,21 +155,22 @@ fn consider_case( return Verdict::Exempt; } - let balance = T::Currency::free_balance(account); + let total_balance = T::Currency::total_balance(account); + let free_balance = T::Currency::free_balance(account); // An amount of funds to charge per block for storage taken up by the contract. - let fee_per_block = compute_fee_per_block::(&balance, contract); + let fee_per_block = compute_fee_per_block::(&free_balance, contract); if fee_per_block.is_zero() { // The rent deposit offset reduced the fee to 0. This means that the contract // gets the rent for free. return Verdict::Exempt; } - let rent_budget = match rent_budget::(&balance, contract) { + let rent_budget = match rent_budget::(&total_balance, &free_balance, contract) { Some(rent_budget) => rent_budget, None => { - // The contract's balance is already below subsistence threshold. That indicates that - // the contract cannot afford to leave a tombstone. + // The contract's total balance is already below subsistence threshold. That + // indicates that the contract cannot afford to leave a tombstone. // // So cleanly wipe the contract. return Verdict::Kill; @@ -195,7 +193,7 @@ fn consider_case( account, dues_limited, WithdrawReason::Fee.into(), - balance.saturating_sub(dues_limited), + free_balance.saturating_sub(dues_limited), ) .is_ok(); @@ -369,14 +367,15 @@ pub fn compute_rent_projection( }; // Compute how much would the fee per block be with the *updated* balance. - let balance = T::Currency::free_balance(account); - let fee_per_block = compute_fee_per_block::(&balance, &alive_contract_info); + let total_balance = T::Currency::total_balance(account); + let free_balance = T::Currency::free_balance(account); + let fee_per_block = compute_fee_per_block::(&free_balance, &alive_contract_info); if fee_per_block.is_zero() { return Ok(RentProjection::NoEviction); } // Then compute how much the contract will sustain under these circumstances. - let rent_budget = rent_budget::(&balance, &alive_contract_info).expect( + let rent_budget = rent_budget::(&total_balance, &free_balance, &alive_contract_info).expect( "the contract exists and in the alive state; the updated balance must be greater than subsistence deposit; this function doesn't return `None`; diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs index 7af514f5dc..aac191e76e 100644 --- a/substrate/frame/contracts/src/tests.rs +++ b/substrate/frame/contracts/src/tests.rs @@ -797,6 +797,7 @@ fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) /// * if balance is reached and balance > subsistence threshold /// * if allowance is exceeded /// * if balance is reached and balance < subsistence threshold +/// * this case cannot be triggered by a contract: we check whether a tombstone is left fn removals(trigger_call: impl Fn() -> bool) { let (wasm, code_hash) = compile_module::("set_rent").unwrap(); @@ -898,10 +899,12 @@ fn removals(trigger_call: impl Fn() -> bool) { .execute_with(|| { // Create let _ = Balances::deposit_creating(&ALICE, 1_000_000); + let subsistence_threshold = + Balances::minimum_balance() + ::TombstoneDeposit::get(); assert_ok!(Contracts::put_code(Origin::signed(ALICE), wasm.clone())); assert_ok!(Contracts::instantiate( Origin::signed(ALICE), - 50 + Balances::minimum_balance(), + 50 + subsistence_threshold, GAS_LIMIT, code_hash.into(), ::Balance::from(1_000u32).encode() // rent allowance @@ -919,7 +922,7 @@ fn removals(trigger_call: impl Fn() -> bool) { ); assert_eq!( Balances::free_balance(BOB), - 50 + Balances::minimum_balance() + 50 + subsistence_threshold, ); // Transfer funds @@ -938,23 +941,23 @@ fn removals(trigger_call: impl Fn() -> bool) { .rent_allowance, 1_000 ); - assert_eq!(Balances::free_balance(BOB), Balances::minimum_balance()); + assert_eq!(Balances::free_balance(BOB), subsistence_threshold); // Advance blocks initialize_block(10); // Trigger rent through call assert!(trigger_call()); - assert!(ContractInfoOf::::get(BOB).is_none()); - assert_eq!(Balances::free_balance(BOB), Balances::minimum_balance()); + assert_matches!(ContractInfoOf::::get(BOB), Some(ContractInfo::Tombstone(_))); + assert_eq!(Balances::free_balance(BOB), subsistence_threshold); // Advance blocks initialize_block(20); // Trigger rent must have no effect assert!(trigger_call()); - assert!(ContractInfoOf::::get(BOB).is_none()); - assert_eq!(Balances::free_balance(BOB), Balances::minimum_balance()); + assert_matches!(ContractInfoOf::::get(BOB), Some(ContractInfo::Tombstone(_))); + assert_eq!(Balances::free_balance(BOB), subsistence_threshold); }); } diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index 6d272ce929..a221e3c7cf 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -379,10 +379,7 @@ fn write_sandbox_output( let len: u32 = read_sandbox_memory_as(ctx, out_len_ptr, 4)?; if len < buf_len { - ctx.trap_reason = Some(TrapReason::SupervisorError( - Error::::OutputBufferTooSmall.into() - )); - return Err(sp_sandbox::HostError); + Err(map_err(ctx, Error::::OutputBufferTooSmall))? } charge_gas( @@ -398,6 +395,17 @@ fn write_sandbox_output( Ok(()) } +/// Stores a DispatchError returned from an Ext function into the trap_reason. +/// +/// This allows through supervisor generated errors to the caller. +fn map_err(ctx: &mut Runtime, err: Error) -> sp_sandbox::HostError where + E: Ext, + Error: Into, +{ + ctx.trap_reason = Some(TrapReason::SupervisorError(err.into())); + sp_sandbox::HostError +} + // *********************************************************** // * AFTER MAKING A CHANGE MAKE SURE TO UPDATE COMPLEXITY.MD * // *********************************************************** @@ -517,7 +525,7 @@ define_env!(Env, , let value: BalanceOf<::T> = read_sandbox_memory_as(ctx, value_ptr, value_len)?; - ctx.ext.transfer(&callee, value, ctx.gas_meter).map_err(|_| sp_sandbox::HostError) + ctx.ext.transfer(&callee, value, ctx.gas_meter).map_err(|e| map_err(ctx, e)) }, // Make a call to another contract.