From 92d21528a21fee92a5c14549e6c83a18a444b8df Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 2 Apr 2019 18:23:50 +0200 Subject: [PATCH 01/54] first partial implementation --- srml/contract/src/account_db.rs | 104 +++++++++++------- srml/contract/src/exec.rs | 33 +++--- srml/contract/src/lib.rs | 173 ++++++++++++++++++++++++++++-- srml/contract/src/wasm/runtime.rs | 8 ++ 4 files changed, 254 insertions(+), 64 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index b560ab11975bb..db0fe8d754055 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -16,7 +16,8 @@ //! Auxilliaries to help with managing partial changes to accounts state. -use super::{CodeHash, CodeHashOf, Trait, TrieId, AccountInfoOf, BalanceOf, AccountInfo, TrieIdGenerator}; +use super::{CodeHash, Trait, TrieId, ContractInfoOf, BalanceOf, ContractInfo, + AliveContractInfo, TrieIdGenerator}; use system; use rstd::cell::RefCell; use rstd::collections::btree_map::{BTreeMap, Entry}; @@ -25,10 +26,12 @@ use runtime_primitives::traits::Zero; use srml_support::{StorageMap, traits::{UpdateBalanceOutcome, SignedImbalance, Currency, Imbalance}, storage::child}; +// TODO TODO: be careful update current doc !! + pub struct ChangeEntry { balance: Option>, - /// In the case the outer option is None, the code_hash remains untouched, while providing `Some(None)` signifies a removing of the code in question - code: Option>>, + /// If None, the code_hash remains untouched. + contract: Option>, storage: BTreeMap, Option>>, } @@ -37,22 +40,28 @@ impl Default for ChangeEntry { fn default() -> Self { ChangeEntry { balance: Default::default(), - code: Default::default(), + contract: Default::default(), storage: Default::default(), } } } +pub struct NewContract { + code_hash: CodeHash, + rent_allowance: BalanceOf, +} + pub type ChangeSet = BTreeMap<::AccountId, ChangeEntry>; pub trait AccountDb { /// Account is used when overlayed otherwise trie_id must be provided. /// This is for performance reason. /// - /// Trie id can be None iff account doesn't have an associated trie id in >. + /// Trie id is None iff account doesn't have an associated trie id in >. /// Because DirectAccountDb bypass the lookup for this association. fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &[u8]) -> Option>; - fn get_code(&self, account: &T::AccountId) -> Option>; + fn get_alive_code_hash(&self, account: &T::AccountId) -> Option>; + fn contract_exists(&self, account: &T::AccountId) -> bool; fn get_balance(&self, account: &T::AccountId) -> BalanceOf; fn commit(&mut self, change_set: ChangeSet); @@ -63,8 +72,11 @@ impl AccountDb for DirectAccountDb { fn get_storage(&self, _account: &T::AccountId, trie_id: Option<&TrieId>, location: &[u8]) -> Option> { trie_id.and_then(|id| child::get_raw(id, location)) } - fn get_code(&self, account: &T::AccountId) -> Option> { - >::get(account) + fn get_alive_code_hash(&self, account: &T::AccountId) -> Option> { + >::get(account).and_then(|i| i.as_alive().map(|i| i.code_hash)) + } + fn contract_exists(&self, account: &T::AccountId) -> bool { + >::exists(account) } fn get_balance(&self, account: &T::AccountId) -> BalanceOf { T::Currency::free_balance(account) @@ -82,42 +94,38 @@ impl AccountDb for DirectAccountDb { continue; } } - if changed.code.is_some() || !changed.storage.is_empty() { - let mut info = if !>::exists(&address) { - let info = AccountInfo { - trie_id: ::TrieIdGenerator::trie_id(&address), + + if changed.contract.is_some() || !changed.storage.is_empty() { + let old_info = >::get(&address).map(|c| { + c.get_alive().expect("Changes are only expected on alive contracts if existing") + }); + let mut new_info = if let Some(contract) = changed.contract { + assert!(!>::exists(&address), "Cannot overlap already existing contract with a new one"); + AliveContractInfo { + code_hash: contract.code_hash, storage_size: 0, - }; - >::insert(&address, &info); - info + trie_id: ::TrieIdGenerator::trie_id(&address), + deduct_block: >::block_number(), + rent_allowance: contract.rent_allowance, + } } else { - >::get(&address).unwrap() + old_info.clone().expect("Changes on storage requires a contract either instantiated or pre-existing") }; - if let Some(code) = changed.code { - if let Some(code) = code { - >::insert(&address, code); - } else { - >::remove(&address); - } - } - - let mut new_storage_size = info.storage_size; for (k, v) in changed.storage.into_iter() { - if let Some(value) = child::get::>(&info.trie_id[..], &k) { - new_storage_size -= value.len() as u64; + if let Some(value) = child::get::>(&new_info.trie_id[..], &k) { + new_info.storage_size -= value.len() as u64; } if let Some(value) = v { - new_storage_size += value.len() as u64; - child::put_raw(&info.trie_id[..], &k, &value[..]); + new_info.storage_size += value.len() as u64; + child::put_raw(&new_info.trie_id[..], &k, &value[..]); } else { - child::kill(&info.trie_id[..], &k); + child::kill(&new_info.trie_id[..], &k); } } - if new_storage_size != info.storage_size { - info.storage_size = new_storage_size; - >::insert(&address, info); + if old_info.map(|old_info| old_info == new_info).unwrap_or(false) { + >::insert(&address, ContractInfo::Alive(new_info)); } } } @@ -164,12 +172,21 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { .insert(location, value); } - pub fn set_code(&mut self, account: &T::AccountId, code: Option>) { + pub fn create_new_contract(&mut self, account: &T::AccountId, code_hash: CodeHash, rent_allowance: BalanceOf) -> Result<(), ()> { + if self.contract_exists(account) { + return Err(()); + } + self.local .borrow_mut() .entry(account.clone()) .or_insert(Default::default()) - .code = Some(code); + .contract = Some(NewContract { + code_hash, + rent_allowance, + }); + + Ok(()) } pub fn set_balance(&mut self, account: &T::AccountId, balance: BalanceOf) { self.local @@ -189,12 +206,19 @@ impl<'a, T: Trait> AccountDb for OverlayAccountDb<'a, T> { .cloned() .unwrap_or_else(|| self.underlying.get_storage(account, trie_id, location)) } - fn get_code(&self, account: &T::AccountId) -> Option> { + fn get_alive_code_hash(&self, account: &T::AccountId) -> Option> { + self.local + .borrow() + .get(account) + .and_then(|changes| changes.contract.as_ref().map(|c| c.code_hash)) + .or_else(|| self.underlying.get_alive_code_hash(account)) + } + fn contract_exists(&self, account: &T::AccountId) -> bool { self.local .borrow() .get(account) - .and_then(|a| a.code.clone()) - .unwrap_or_else(|| self.underlying.get_code(account)) + .map(|a| a.contract.is_some()) + .unwrap_or_else(|| self.underlying.contract_exists(account)) } fn get_balance(&self, account: &T::AccountId) -> BalanceOf { self.local @@ -213,8 +237,8 @@ impl<'a, T: Trait> AccountDb for OverlayAccountDb<'a, T> { if changed.balance.is_some() { value.balance = changed.balance; } - if changed.code.is_some() { - value.code = changed.code; + if changed.contract.is_some() { + value.contract = changed.contract; } value.storage.extend(changed.storage.into_iter()); } diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index 992dc02b0dec1..415c9f9748b16 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -14,7 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, TrieId, BalanceOf, AccountInfoOf}; +use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, + TrieId, BalanceOf, ContractInfoOf, pay_rent}; use crate::account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}; use crate::gas::{GasMeter, Token, approx_gas_for_balance}; @@ -65,6 +66,7 @@ pub trait Ext { fn instantiate( &mut self, code: &CodeHash, + rent_allowance: BalanceOf, value: BalanceOf, gas_meter: &mut GasMeter, input_data: &[u8], @@ -245,7 +247,8 @@ where /// The specified `origin` address will be used as `sender` for pub fn top_level(origin: T::AccountId, cfg: &'a Config, vm: &'a V, loader: &'a L) -> Self { ExecutionContext { - self_trie_id: >::get(&origin).map(|s| s.trie_id), + self_trie_id: >::get(&origin) + .and_then(|i| i.as_alive().map(|i| i.trie_id.clone())), self_account: origin, overlay: OverlayAccountDb::::new(&DirectAccountDb), depth: 0, @@ -259,7 +262,8 @@ where fn nested(&self, overlay: OverlayAccountDb<'a, T>, dest: T::AccountId) -> Self { ExecutionContext { - self_trie_id: >::get(&dest).map(|s| s.trie_id), + self_trie_id: >::get(&dest) + .and_then(|i| i.as_alive().map(|i| i.trie_id.clone())), self_account: dest, overlay, depth: self.depth + 1, @@ -291,7 +295,11 @@ where return Err("not enough gas to pay base call fee"); } - let dest_code_hash = self.overlay.get_code(&dest); + // Assumption: pay_rent doesn't collide with overlay because + // pay_rent will be done on first call and dest contract and balance + // cannot be change before first call + pay_rent::(&dest); + let mut output_data = Vec::new(); let (change_set, events, calls) = { @@ -311,7 +319,7 @@ where )?; } - if let Some(dest_code_hash) = dest_code_hash { + if let Some(dest_code_hash) = self.overlay.get_alive_code_hash(&dest) { let executable = self.loader.load_main(&dest_code_hash)?; output_data = self .vm @@ -346,6 +354,7 @@ where endowment: BalanceOf, gas_meter: &mut GasMeter, code_hash: &CodeHash, + rent_allowance: BalanceOf, input_data: &[u8], ) -> Result, &'static str> { if self.depth == self.config.max_depth as usize { @@ -365,15 +374,12 @@ where &self.self_account, ); - if self.overlay.get_code(&dest).is_some() { - // It should be enough to check only the code. - return Err("contract already exists"); - } - let (change_set, events, calls) = { let mut overlay = OverlayAccountDb::new(&self.overlay); - - overlay.set_code(&dest, Some(code_hash.clone())); + + overlay.create_new_contract(&dest, code_hash.clone(), rent_allowance) + .map_err(|_| "contract or tombstone already exsists")?; + let mut nested = self.nested(overlay, dest.clone()); // Send funds unconditionally here. If the `endowment` is below existential_deposit @@ -568,11 +574,12 @@ where fn instantiate( &mut self, code_hash: &CodeHash, + rent_allowance: BalanceOf, endowment: BalanceOf, gas_meter: &mut GasMeter, input_data: &[u8], ) -> Result>, &'static str> { - self.ctx.instantiate(endowment, gas_meter, code_hash, input_data) + self.ctx.instantiate(endowment, gas_meter, code_hash, rent_allowance, input_data) } fn call( diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 59347bbabc4b7..12cb1c2fa6547 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -101,10 +101,10 @@ use substrate_primitives::crypto::UncheckedFrom; use rstd::prelude::*; use rstd::marker::PhantomData; use parity_codec::{Codec, Encode, Decode}; -use runtime_primitives::traits::{Hash, As, SimpleArithmetic,Bounded, StaticLookup}; +use runtime_primitives::traits::{Hash, As, SimpleArithmetic,Bounded, StaticLookup, Saturating, CheckedDiv, Zero}; use srml_support::dispatch::{Result, Dispatchable}; use srml_support::{Parameter, StorageMap, StorageValue, decl_module, decl_event, decl_storage, storage::child}; -use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency}; +use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency, WithdrawReason, ExistenceRequirement}; use system::{ensure_signed, RawOrigin}; use timestamp; @@ -124,13 +124,72 @@ pub trait ComputeDispatchFee { #[derive(Encode,Decode,Clone,Debug)] /// Information for managing an acocunt and its sub trie abstraction. /// This is the required info to cache for an account -pub struct AccountInfo { - /// unique ID for the subtree encoded as a byte +pub enum ContractInfo { + Alive(AliveContractInfo), + Tombstone(TombstoneContractInfo), +} + +impl ContractInfo { + // fn is_alive(&self) -> bool { + // self.as_alive().is_some() + // } + fn as_alive(&self) -> Option<&AliveContractInfo> { + if let ContractInfo::Alive(ref alive) = self { + Some(alive) + } else { + None + } + } + fn get_alive(self) -> Option> { + if let ContractInfo::Alive(alive) = self { + Some(alive) + } else { + None + } + } + fn as_alive_mut(&mut self) -> Option<&mut AliveContractInfo> { + if let ContractInfo::Alive(ref mut alive) = self { + Some(alive) + } else { + None + } + } + // fn is_tombstone(&self) -> bool { + // self.as_tombstone().is_some() + // } + // fn as_tombstone(&self) -> Option<&TombstoneContractInfo> { + // if let ContractInfo::Tombstone(ref tombstone) = self { + // Some(tombstone) + // } else { + // None + // } + // } + // fn as_tombstone_mut(&mut self) -> Option<&mut TombstoneContractInfo> { + // if let ContractInfo::Tombstone(ref mut tombstone) = self { + // Some(tombstone) + // } else { + // None + // } + // } +} + +#[derive(Encode, Decode, Clone, Debug, PartialEq, Eq)] +pub struct AliveContractInfo { + /// Unique ID for the subtree encoded as a byte pub trie_id: TrieId, - /// the size of stored value in octet + /// The size of stored value in octet pub storage_size: u64, + // pub rent_allowance: u64, + // pub deduct_block: u64, + /// The code associated with a given account. + pub code_hash: CodeHash, + pub rent_allowance: BalanceOf, + pub deduct_block: T::BlockNumber, } +#[derive(Encode, Decode, Clone, Debug)] +pub struct TombstoneContractInfo(Vec); + /// Get a trie id (trie id must be unique and collision resistant depending upon its context) /// Note that it is different than encode because trie id should have collision resistance /// property (being a proper uniqueid). @@ -338,6 +397,7 @@ decl_module! { #[compact] endowment: BalanceOf, #[compact] gas_limit: T::Gas, code_hash: CodeHash, + #[compact] rent_allowance: BalanceOf, data: Vec ) -> Result { let origin = ensure_signed(origin)?; @@ -352,7 +412,7 @@ decl_module! { let vm = crate::wasm::WasmVm::new(&cfg.schedule); let loader = crate::wasm::WasmLoader::new(&cfg.schedule); let mut ctx = ExecutionContext::top_level(origin.clone(), &cfg, &vm, &loader); - let result = ctx.instantiate(endowment, &mut gas_meter, &code_hash, &data); + let result = ctx.instantiate(endowment, &mut gas_meter, &code_hash, rent_allowance, &data); if let Ok(_) = result { // Commit all changes that made it thus far into the persistant storage. @@ -377,6 +437,8 @@ decl_module! { result.map(|_| ()) } + // TODO TODO: claim surchage using check_rent + fn on_finalize() { >::kill(); } @@ -410,6 +472,14 @@ decl_event! { decl_storage! { trait Store for Module as Contract { + /// The minimum amount required to generate a tombstone. + TombstoneDeposit get(tombstone_deposit) config(): BalanceOf; + /// The fee to be paid for contract storage; the per-byte portion. + RentBaseFee get(rent_byte_fee) config(): BalanceOf; + /// The fee to be paid for contract storage; the base. + RentByteFee get(rent_base_fee) config(): BalanceOf; + /// TODO TODO + RentDepositOffset get(rent_deposit_offset) config(): BalanceOf; /// The fee required to make a transfer. TransferFee get(transfer_fee) config(): BalanceOf; /// The fee required to create an account. @@ -434,8 +504,6 @@ decl_storage! { GasSpent get(gas_spent): T::Gas; /// Current cost schedule for contracts. CurrentSchedule get(current_schedule) config(): Schedule = Schedule::default(); - /// The code associated with a given account. - pub CodeHashOf: map T::AccountId => Option>; /// A mapping from an original code hash to the original code, untouched by instrumentation. pub PristineCode: map CodeHash => Option>; /// A mapping between an original code hash and instrumented wasm code, ready for the execution. @@ -443,14 +511,16 @@ decl_storage! { /// The subtrie counter pub AccountCounter: u64 = 0; /// The code associated with a given account. - pub AccountInfoOf: map T::AccountId => Option; + pub ContractInfoOf: map T::AccountId => Option>; } } impl OnFreeBalanceZero for Module { fn on_free_balance_zero(who: &T::AccountId) { - >::remove(who); - >::get(who).map(|info| child::kill_storage(&info.trie_id)); + if let Some(ContractInfo::Alive(info)) = >::get(who) { + child::kill_storage(&info.trie_id); + } + >::remove(who); } } @@ -535,3 +605,84 @@ impl> Default for Schedule { } } } + +#[derive(Debug)] +enum RentDecision { + /// Happens when the rent is offset completely by the `rent_deposit_offset`. + /// Or Rent has already been payed + /// Or account doesn't has rent because no contract or tombstone contract + FreeFromRent, + /// The contract still have funds to keep itself from eviction. Collect dues. + /// It is ensure that account can withdraw dues without dying. + CollectDues(BalanceOf), + CantWithdrawRentWithoutDying(BalanceOf), + AllowanceExceeded(BalanceOf), +} + +fn check_rent(account: &T::AccountId, block_number: T::BlockNumber) -> RentDecision { + let contract = match >::get(account) { + None | Some(ContractInfo::Tombstone(_)) => return RentDecision::FreeFromRent, + Some(ContractInfo::Alive(contract)) => contract, + }; + + // Rent has already been payed + if contract.deduct_block == block_number { + return RentDecision::FreeFromRent + } + + let balance = T::Currency::free_balance(account); + + let effective_storage_size = >::sa(contract.storage_size) + >::rent_byte_fee() + .saturating_sub(balance.checked_div(&>::rent_deposit_offset()).unwrap_or(>::sa(0))); + + let fee_per_block: BalanceOf = effective_storage_size * >::rent_byte_fee(); + + 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 RentDecision::FreeFromRent + } + + let rent = fee_per_block * >::sa((block_number.saturating_sub(contract.deduct_block)).as_()); + + // Pay rent up to rent_allowance + if rent <= contract.rent_allowance { + // TODO TODO: this could be one call if ensure_can_withdraw took exitence requirement + if balance.saturating_sub(rent) > T::Currency::minimum_balance() + && T::Currency::ensure_can_withdraw(account, rent, WithdrawReason::Fee, balance.saturating_sub(rent)).is_ok() + { + RentDecision::CollectDues(rent) + } else { + RentDecision::CantWithdrawRentWithoutDying(rent) + } + } else { + RentDecision::AllowanceExceeded(contract.rent_allowance) + } +} + +fn pay_rent(account: &T::AccountId) { + match check_rent::(account, >::block_number()) { + RentDecision::FreeFromRent => (), + RentDecision::CollectDues(rent) => { + T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive).expect("Assumption of check_rent"); + }, + RentDecision::CantWithdrawRentWithoutDying(rent) + | RentDecision::AllowanceExceeded(rent) => { + match T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::AllowDeath) { + Ok(imbalance) => drop(imbalance), + Err(_) => { T::Currency::slash(account, rent); }, + } + + if T::Currency::free_balance(account) >= >::tombstone_deposit() + T::Currency::minimum_balance() { + // TODO TODO TODO TODO: generate tombstone + // let mut contract = match >::get(account) { + // None | Some(ContractInfo::Tombstone(_)) => return RentDecision::FreeFromRent, + // Some(ContractInfo::Alive(contract)) => contract, + // }; + } else { + // remove if less than substrantial deposit + T::Currency::make_free_balance_be(account, >::zero()); + } + } + } +} diff --git a/srml/contract/src/wasm/runtime.rs b/srml/contract/src/wasm/runtime.rs index b4a963f93192e..e08d73861f7b0 100644 --- a/srml/contract/src/wasm/runtime.rs +++ b/srml/contract/src/wasm/runtime.rs @@ -345,6 +345,8 @@ define_env!(Env, , ctx, init_code_ptr: u32, init_code_len: u32, + rent_allowance_ptr: u32, + rent_allowance_len: u32, gas: u64, value_ptr: u32, value_len: u32, @@ -355,6 +357,11 @@ define_env!(Env, , let code_hash_buf = read_sandbox_memory(ctx, init_code_ptr, init_code_len)?; ::T>>::decode(&mut &code_hash_buf[..]).ok_or_else(|| sandbox::HostError)? }; + let rent_allowance = { + let rent_allowance_buf = read_sandbox_memory(ctx, rent_allowance_ptr, rent_allowance_len)?; + BalanceOf::<::T>::decode(&mut &rent_allowance_buf[..]) + .ok_or_else(|| sandbox::HostError)? + }; let value = { let value_buf = read_sandbox_memory(ctx, value_ptr, value_len)?; BalanceOf::<::T>::decode(&mut &value_buf[..]) @@ -376,6 +383,7 @@ define_env!(Env, , Some(nested_meter) => { ext.instantiate( &code_hash, + rent_allowance, value, nested_meter, &input_data From ab1bcad7a3418922314073a3cb11131fad1c7075 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 8 Apr 2019 16:38:13 +0200 Subject: [PATCH 02/54] update rent allowance --- srml/contract/src/lib.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 88d91a0c826d8..a1b849165f978 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -104,7 +104,8 @@ use parity_codec::{Codec, Encode, Decode}; use runtime_primitives::traits::{Hash, As, SimpleArithmetic,Bounded, StaticLookup, Saturating, CheckedDiv, Zero}; use srml_support::dispatch::{Result, Dispatchable}; use srml_support::{Parameter, StorageMap, StorageValue, decl_module, decl_event, decl_storage, storage::child}; -use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency, WithdrawReason, ExistenceRequirement}; +use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency, + WithdrawReason, ExistenceRequirement, Imbalance}; use system::{ensure_signed, RawOrigin}; use timestamp; @@ -144,6 +145,13 @@ impl ContractInfo { None } } + fn as_alive_mut(&mut self) -> Option<&mut AliveContractInfo> { + if let ContractInfo::Alive(ref mut alive) = self { + Some(alive) + } else { + None + } + } } #[derive(Encode, Decode)] @@ -699,7 +707,15 @@ impl Module { match Self::check_rent(account, >::block_number()) { RentDecision::FreeFromRent => (), RentDecision::CollectDues(rent) => { - T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive).expect("Assumption of check_rent"); + let imbalance = T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive) + .expect("Assumption of check_rent"); + >::mutate(account, |contract| { + // rent_allowance isn't reached is guaranted by check_rent + contract.as_mut() + .and_then(|c| c.as_alive_mut()) + .expect("Only existing alive contracts pay rent") + .rent_allowance -= imbalance.peek(); + }) }, RentDecision::CantWithdrawRentWithoutDying(rent) | RentDecision::AllowanceExceeded(rent) => { From 94a8a00456e4cbbcf18116a4984482a3ed2661b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 8 Apr 2019 17:11:36 +0200 Subject: [PATCH 03/54] fmt Co-Authored-By: thiolliere --- srml/contract/src/account_db.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index fe1a67eee1b78..84043e728c09e 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -16,8 +16,10 @@ //! Auxilliaries to help with managing partial changes to accounts state. -use super::{CodeHash, Trait, TrieId, ContractInfoOf, BalanceOf, ContractInfo, - AliveContractInfo, TrieIdGenerator, Module}; +use super::{ + CodeHash, Trait, TrieId, ContractInfoOf, BalanceOf, ContractInfo, + AliveContractInfo, TrieIdGenerator, Module +}; use crate::exec::StorageKey; use system; use rstd::cell::RefCell; From 4a0937fbe9e84d45066be5741218647fb2e9d880 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 8 Apr 2019 17:24:35 +0200 Subject: [PATCH 04/54] remove comments --- srml/contract/src/account_db.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 84043e728c09e..b1086a66bb0a5 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -29,8 +29,6 @@ use runtime_primitives::traits::Zero; use srml_support::{StorageMap, traits::{UpdateBalanceOutcome, SignedImbalance, Currency, Imbalance}, storage::child}; -// TODO TODO: be careful update current doc !! - pub struct ChangeEntry { balance: Option>, /// If None, the code_hash remains untouched. From ef073f233e22b15827bfaf5da0edf7c5199754d5 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 8 Apr 2019 17:48:53 +0200 Subject: [PATCH 05/54] reward surcharge claims --- srml/contract/src/lib.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index a1b849165f978..5f62062becf4e 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -449,17 +449,30 @@ decl_module! { result.map(|_| ()) } - fn claim_surchage(origin, dest: T::AccountId) { + fn claim_surchage(origin, dest: T::AccountId, aux_sender: Option) { + let origin = origin.into(); + let (signed, rewarded) = match origin { + Some(system::RawOrigin::Signed(ref account)) if aux_sender.is_none() => { + (true, account) + }, + Some(system::RawOrigin::Inherent) if aux_sender.is_some() => { + (true, aux_sender.as_ref().expect("checked above")) + }, + _ => return Err("Invalid surcharge claim: origin must be signed or \ + inherent and auxiliary sender only provided on inherent") + }; + let mut check_block = >::block_number(); - if let Some(system::RawOrigin::Signed(_)) = origin.into() { + // Make validators advantaged + if signed { check_block -= T::BlockNumber::sa(2u64); } match Self::check_rent(&dest, check_block) { RentDecision::CantWithdrawRentWithoutDying(rent) | RentDecision::AllowanceExceeded(rent) => { - // TODO TODO: reward caller + T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; Self::evict(&dest, rent); } RentDecision::FreeFromRent | RentDecision::CollectDues(_) => (), @@ -514,7 +527,7 @@ decl_storage! { RentDepositOffset get(rent_deposit_offset) config(): BalanceOf; /// Reward that is received by the party whose touch has led /// to removal of a contract. - SurchargeAmount get(surcharge_amount) config(): BalanceOf; + SurchargeReward get(surcharge_reward) config(): BalanceOf; /// The fee required to make a transfer. TransferFee get(transfer_fee) config(): BalanceOf; /// The fee required to create an account. From cab4aa346dd85bdddebe20eb9650dccf8f676044 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 12:51:31 +0200 Subject: [PATCH 06/54] remove rent allowance in param + code_hash changed --- node/executor/src/lib.rs | 4 +- srml/contract/src/account_db.rs | 85 ++++++++++++++++++++----------- srml/contract/src/exec.rs | 31 +++++------ srml/contract/src/lib.rs | 3 +- srml/contract/src/tests.rs | 11 +++- srml/contract/src/wasm/mod.rs | 1 - srml/contract/src/wasm/runtime.rs | 8 --- 7 files changed, 82 insertions(+), 61 deletions(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index c855a4e6f00c5..f0bd9a0603882 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -727,7 +727,7 @@ mod tests { CheckedExtrinsic { signed: Some((charlie(), 1)), function: Call::Contract( - contract::Call::create::(10, 10_000, transfer_ch, Vec::new()) + contract::Call::create::(10, 10_000, transfer_ch, 40, Vec::new()) ), }, CheckedExtrinsic { @@ -745,7 +745,7 @@ mod tests { runtime_io::with_externalities(&mut t, || { // Verify that the contract constructor worked well and code of TRANSFER contract is actually deployed. - assert_eq!(&contract::CodeHashOf::::get(addr).unwrap(), &transfer_ch); + assert_eq!(&contract::ContractInfoOf::::get(addr).and_then(|c| c.get_alive()).unwrap().unwrap(), &transfer_ch); }); } diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index b1086a66bb0a5..b3e10017e307b 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -29,10 +29,14 @@ use runtime_primitives::traits::Zero; use srml_support::{StorageMap, traits::{UpdateBalanceOutcome, SignedImbalance, Currency, Imbalance}, storage::child}; +// Note: we don't provide Option because we can't create +// the trie_id in the overlay, thus we provide an overlay on the fields +// specifically. pub struct ChangeEntry { balance: Option>, /// If None, the code_hash remains untouched. - contract: Option>, + code_hash: Option>, + rent_allowance: Option>, storage: BTreeMap>>, } @@ -40,18 +44,14 @@ pub struct ChangeEntry { impl Default for ChangeEntry { fn default() -> Self { ChangeEntry { + rent_allowance: Default::default(), balance: Default::default(), - contract: Default::default(), + code_hash: Default::default(), storage: Default::default(), } } } -pub struct NewContract { - code_hash: CodeHash, - rent_allowance: BalanceOf, -} - pub type ChangeSet = BTreeMap<::AccountId, ChangeEntry>; pub trait AccountDb { @@ -62,6 +62,7 @@ pub trait AccountDb { /// Because DirectAccountDb bypass the lookup for this association. fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option>; fn get_alive_code_hash(&self, account: &T::AccountId) -> Option>; + fn get_rent_allowance(&self, account: &T::AccountId) -> Option>; fn contract_exists(&self, account: &T::AccountId) -> bool; fn get_balance(&self, account: &T::AccountId) -> BalanceOf; @@ -76,6 +77,9 @@ impl AccountDb for DirectAccountDb { fn get_alive_code_hash(&self, account: &T::AccountId) -> Option> { >::get(account).and_then(|i| i.as_alive().map(|i| i.code_hash)) } + fn get_rent_allowance(&self, account: &T::AccountId) -> Option> { + >::get(account).and_then(|i| i.as_alive().map(|i| i.rent_allowance)) + } fn contract_exists(&self, account: &T::AccountId) -> bool { >::exists(account) } @@ -96,23 +100,37 @@ impl AccountDb for DirectAccountDb { } } - if changed.contract.is_some() || !changed.storage.is_empty() { - let old_info = >::get(&address).map(|c| { - c.get_alive().expect("Changes are only expected on alive contracts if existing") - }); - let mut new_info = if let Some(contract) = changed.contract { - assert!(!>::exists(&address), "Cannot overlap already existing contract with a new one"); + if changed.code_hash.is_some() || changed.rent_allowance.is_some() || !changed.storage.is_empty() { + let old_info = match >::get(&address) { + Some(ContractInfo::Alive(alive)) => Some(alive), + None => None, + // Cannot commit changes to tombstone contract + Some(ContractInfo::Tombstone(_)) => continue, + }; + + let mut new_info = if let Some(info) = old_info.clone() { + info + } else if let Some(code_hash) = changed.code_hash { AliveContractInfo { - code_hash: contract.code_hash, + code_hash, storage_size: >::storage_size_offset(), trie_id: ::TrieIdGenerator::trie_id(&address), deduct_block: >::block_number(), - rent_allowance: contract.rent_allowance, + rent_allowance: >::zero(), } } else { - old_info.clone().expect("Changes on storage requires a contract either instantiated or pre-existing") + // No contract exist and no code_hash provided + continue; }; + if let Some(rent_allowance) = changed.rent_allowance { + new_info.rent_allowance = rent_allowance; + } + + if let Some(code_hash) = changed.code_hash { + new_info.code_hash = code_hash; + } + for (k, v) in changed.storage.into_iter() { if let Some(value) = child::get::>(&new_info.trie_id[..], &k) { new_info.storage_size -= value.len() as u64; @@ -173,7 +191,7 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { .insert(location, value); } - pub fn create_new_contract(&mut self, account: &T::AccountId, code_hash: CodeHash, rent_allowance: BalanceOf) -> Result<(), ()> { + pub fn create_new_contract(&mut self, account: &T::AccountId, code_hash: CodeHash) -> Result<(), ()> { if self.contract_exists(account) { return Err(()); } @@ -182,13 +200,18 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { .borrow_mut() .entry(account.clone()) .or_insert(Default::default()) - .contract = Some(NewContract { - code_hash, - rent_allowance, - }); + .code_hash = Some(code_hash); Ok(()) } + pub fn set_rent_allowance(&mut self, account: &T::AccountId, rent_allowance: BalanceOf) { + // TODO TODO: maybe assert contracts is alive + self.local + .borrow_mut() + .entry(account.clone()) + .or_insert(Default::default()) + .rent_allowance = Some(rent_allowance); + } pub fn set_balance(&mut self, account: &T::AccountId, balance: BalanceOf) { self.local .borrow_mut() @@ -211,14 +234,21 @@ impl<'a, T: Trait> AccountDb for OverlayAccountDb<'a, T> { self.local .borrow() .get(account) - .and_then(|changes| changes.contract.as_ref().map(|c| c.code_hash)) + .and_then(|changes| changes.code_hash) .or_else(|| self.underlying.get_alive_code_hash(account)) } + fn get_rent_allowance(&self, account: &T::AccountId) -> Option> { + self.local + .borrow() + .get(account) + .and_then(|changes| changes.rent_allowance) + .or_else(|| self.underlying.get_rent_allowance(account)) + } fn contract_exists(&self, account: &T::AccountId) -> bool { self.local .borrow() .get(account) - .map(|a| a.contract.is_some()) + .map(|a| a.code_hash.is_some()) .unwrap_or_else(|| self.underlying.contract_exists(account)) } fn get_balance(&self, account: &T::AccountId) -> BalanceOf { @@ -235,12 +265,9 @@ impl<'a, T: Trait> AccountDb for OverlayAccountDb<'a, T> { match local.entry(address) { Entry::Occupied(e) => { let mut value = e.into_mut(); - if changed.balance.is_some() { - value.balance = changed.balance; - } - if changed.contract.is_some() { - value.contract = changed.contract; - } + value.balance = changed.balance.or(value.balance); + value.code_hash = changed.code_hash.or(value.code_hash); + value.rent_allowance = changed.rent_allowance.or(value.rent_allowance); value.storage.extend(changed.storage.into_iter()); } Entry::Vacant(e) => { diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index 7f2d38c6d68d1..717ad2480ff25 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -68,7 +68,6 @@ pub trait Ext { fn instantiate( &mut self, code: &CodeHash, - rent_allowance: BalanceOf, value: BalanceOf, gas_meter: &mut GasMeter, input_data: &[u8], @@ -358,7 +357,6 @@ where endowment: BalanceOf, gas_meter: &mut GasMeter, code_hash: &CodeHash, - rent_allowance: BalanceOf, input_data: &[u8], ) -> Result, &'static str> { if self.depth == self.config.max_depth as usize { @@ -381,7 +379,7 @@ where let (change_set, events, calls) = { let mut overlay = OverlayAccountDb::new(&self.overlay); - overlay.create_new_contract(&dest, code_hash.clone(), rent_allowance) + overlay.create_new_contract(&dest, code_hash.clone()) .map_err(|_| "contract or tombstone already exsists")?; let mut nested = self.nested(overlay, dest.clone()); @@ -578,12 +576,11 @@ where fn instantiate( &mut self, code_hash: &CodeHash, - rent_allowance: BalanceOf, endowment: BalanceOf, gas_meter: &mut GasMeter, input_data: &[u8], ) -> Result>, &'static str> { - self.ctx.instantiate(endowment, gas_meter, code_hash, rent_allowance, input_data) + self.ctx.instantiate(endowment, gas_meter, code_hash, input_data) } fn call( @@ -770,7 +767,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_code(&BOB, Some(exec_ch)); + ctx.overlay.create_new_contract(&BOB, exec_ch).unwrap(); assert_matches!( ctx.call(BOB, value, &mut gas_meter, &data, EmptyOutputBuf::new()), @@ -1005,7 +1002,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_code(&BOB, Some(return_ch)); + ctx.overlay.create_new_contract(&BOB, return_ch).unwrap(); let result = ctx.call( dest, @@ -1033,7 +1030,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_code(&BOB, Some(input_data_ch)); + ctx.overlay.create_new_contract(&BOB, input_data_ch).unwrap(); let result = ctx.call( BOB, @@ -1092,7 +1089,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_code(&BOB, Some(recurse_ch)); + ctx.overlay.create_new_contract(&BOB, recurse_ch).unwrap(); let result = ctx.call( BOB, @@ -1139,8 +1136,8 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_code(&dest, Some(bob_ch)); - ctx.overlay.set_code(&CHARLIE, Some(charlie_ch)); + ctx.overlay.create_new_contract(&dest, bob_ch).unwrap(); + ctx.overlay.create_new_contract(&CHARLIE, charlie_ch).unwrap(); let result = ctx.call( dest, @@ -1182,8 +1179,8 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_code(&BOB, Some(bob_ch)); - ctx.overlay.set_code(&CHARLIE, Some(charlie_ch)); + ctx.overlay.create_new_contract(&BOB, bob_ch).unwrap(); + ctx.overlay.create_new_contract(&CHARLIE, charlie_ch).unwrap(); let result = ctx.call( BOB, @@ -1249,7 +1246,7 @@ mod tests { // Check that the newly created account has the expected code hash and // there are instantiation event. - assert_eq!(ctx.overlay.get_code(&created_contract_address).unwrap(), dummy_ch); + assert_eq!(ctx.overlay.get_alive_code_hash(&created_contract_address).unwrap(), dummy_ch); assert_eq!(&ctx.events, &[ RawEvent::Transfer(ALICE, created_contract_address, 100), RawEvent::Instantiated(ALICE, created_contract_address), @@ -1290,7 +1287,7 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); ctx.overlay.set_balance(&ALICE, 1000); - ctx.overlay.set_code(&BOB, Some(creator_ch)); + ctx.overlay.create_new_contract(&BOB, creator_ch).unwrap(); assert_matches!( ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), &[], EmptyOutputBuf::new()), @@ -1301,7 +1298,7 @@ mod tests { // Check that the newly created account has the expected code hash and // there are instantiation event. - assert_eq!(ctx.overlay.get_code(&created_contract_address).unwrap(), dummy_ch); + assert_eq!(ctx.overlay.get_alive_code_hash(&created_contract_address).unwrap(), dummy_ch); assert_eq!(&ctx.events, &[ RawEvent::Transfer(ALICE, BOB, 20), RawEvent::Transfer(BOB, created_contract_address, 15), @@ -1341,7 +1338,7 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); ctx.overlay.set_balance(&ALICE, 1000); - ctx.overlay.set_code(&BOB, Some(creator_ch)); + ctx.overlay.create_new_contract(&BOB, creator_ch).unwrap(); assert_matches!( ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), &[], EmptyOutputBuf::new()), diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 5f62062becf4e..8b5e4b6d73e84 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -409,7 +409,6 @@ decl_module! { #[compact] endowment: BalanceOf, #[compact] gas_limit: T::Gas, code_hash: CodeHash, - #[compact] rent_allowance: BalanceOf, data: Vec ) -> Result { let origin = ensure_signed(origin)?; @@ -424,7 +423,7 @@ decl_module! { let vm = crate::wasm::WasmVm::new(&cfg.schedule); let loader = crate::wasm::WasmLoader::new(&cfg.schedule); let mut ctx = ExecutionContext::top_level(origin.clone(), &cfg, &vm, &loader); - let result = ctx.instantiate(endowment, &mut gas_meter, &code_hash, rent_allowance, &data); + let result = ctx.instantiate(endowment, &mut gas_meter, &code_hash, &data); if let Ok(_) = result { // Commit all changes that made it thus far into the persistant storage. diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index e211691db5ed8..e923caf043ccb 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -199,6 +199,11 @@ impl ExtBuilder { ); t.extend( GenesisConfig:: { + rent_byte_price: 4, + rent_deposit_offset: 1000, + storage_size_offset: 8, + surcharge_reward: 150, + tombstone_deposit: 16, transaction_base_fee: 0, transaction_byte_fee: 0, transfer_fee: self.transfer_fee, @@ -249,8 +254,9 @@ fn account_removal_removes_storage() { ContractInfoOf::::insert(1, &ContractInfo::Alive(AliveContractInfo { trie_id: unique_id1.to_vec(), storage_size: Contract::storage_size_offset(), + deduct_block: System::block_number(), + code_hash: H256::repeat_byte(1), rent_allowance: 40, - deduct_block: System::block_number() })); child::put(&unique_id1[..], &b"foo".to_vec(), &b"1".to_vec()); assert_eq!(child::get(&unique_id1[..], &b"foo".to_vec()), Some(b"1".to_vec())); @@ -260,8 +266,9 @@ fn account_removal_removes_storage() { ContractInfoOf::::insert(2, &ContractInfo::Alive(AliveContractInfo { trie_id: unique_id2.to_vec(), storage_size: Contract::storage_size_offset(), + deduct_block: System::block_number(), + code_hash: H256::repeat_byte(2), rent_allowance: 40, - deduct_block: System::block_number() })); child::put(&unique_id2[..], &b"hello".to_vec(), &b"3".to_vec()); child::put(&unique_id2[..], &b"world".to_vec(), &b"4".to_vec()); diff --git a/srml/contract/src/wasm/mod.rs b/srml/contract/src/wasm/mod.rs index 252ab6af34826..51db3b2e5652f 100644 --- a/srml/contract/src/wasm/mod.rs +++ b/srml/contract/src/wasm/mod.rs @@ -219,7 +219,6 @@ mod tests { fn instantiate( &mut self, code_hash: &CodeHash, - rent_allowance: u64, endowment: u64, gas_meter: &mut GasMeter, data: &[u8], diff --git a/srml/contract/src/wasm/runtime.rs b/srml/contract/src/wasm/runtime.rs index 88fa6504441d1..37ab9fa88708f 100644 --- a/srml/contract/src/wasm/runtime.rs +++ b/srml/contract/src/wasm/runtime.rs @@ -372,8 +372,6 @@ define_env!(Env, , ctx, init_code_ptr: u32, init_code_len: u32, - rent_allowance_ptr: u32, - rent_allowance_len: u32, gas: u64, value_ptr: u32, value_len: u32, @@ -384,11 +382,6 @@ define_env!(Env, , let code_hash_buf = read_sandbox_memory(ctx, init_code_ptr, init_code_len)?; ::T>>::decode(&mut &code_hash_buf[..]).ok_or_else(|| sandbox::HostError)? }; - let rent_allowance = { - let rent_allowance_buf = read_sandbox_memory(ctx, rent_allowance_ptr, rent_allowance_len)?; - BalanceOf::<::T>::decode(&mut &rent_allowance_buf[..]) - .ok_or_else(|| sandbox::HostError)? - }; let value = { let value_buf = read_sandbox_memory(ctx, value_ptr, value_len)?; BalanceOf::<::T>::decode(&mut &value_buf[..]) @@ -410,7 +403,6 @@ define_env!(Env, , Some(nested_meter) => { ext.instantiate( &code_hash, - rent_allowance, value, nested_meter, &input_data From 0841bfe217c1c6510724a65ea58be138fca0b57a Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 14:13:43 +0200 Subject: [PATCH 07/54] Fix bug --- srml/contract/src/account_db.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index b3e10017e307b..fb9143cafd561 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -143,7 +143,7 @@ impl AccountDb for DirectAccountDb { } } - if old_info.map(|old_info| old_info == new_info).unwrap_or(false) { + if old_info.map(|old_info| old_info == new_info).unwrap_or(true) { >::insert(&address, ContractInfo::Alive(new_info)); } } From 1be670ef41958716cceca068163e686afca2b364 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 14:30:32 +0200 Subject: [PATCH 08/54] fix tests --- node/cli/src/chain_spec.rs | 10 ++++++++++ node/executor/src/lib.rs | 4 ++-- srml/contract/src/lib.rs | 13 ++++++++----- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/node/cli/src/chain_spec.rs b/node/cli/src/chain_spec.rs index a5ddd5ab81e54..a96e1712c918a 100644 --- a/node/cli/src/chain_spec.rs +++ b/node/cli/src/chain_spec.rs @@ -152,6 +152,11 @@ fn staging_testnet_config_genesis() -> GenesisConfig { burn: Permill::from_percent(50), }), contract: Some(ContractConfig { + rent_byte_price: 4, + rent_deposit_offset: 1000, + storage_size_offset: 8, + surcharge_reward: 150, + tombstone_deposit: 16, transaction_base_fee: 1 * CENTS, transaction_byte_fee: 10 * MILLICENTS, transfer_fee: 1 * CENTS, @@ -309,6 +314,11 @@ pub fn testnet_genesis( burn: Permill::from_percent(50), }), contract: Some(ContractConfig { + rent_byte_price: 4, + rent_deposit_offset: 1000, + storage_size_offset: 8, + surcharge_reward: 150, + tombstone_deposit: 16, transaction_base_fee: 1, transaction_byte_fee: 0, transfer_fee: 0, diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index f0bd9a0603882..48762800291fb 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -727,7 +727,7 @@ mod tests { CheckedExtrinsic { signed: Some((charlie(), 1)), function: Call::Contract( - contract::Call::create::(10, 10_000, transfer_ch, 40, Vec::new()) + contract::Call::create::(10, 10_000, transfer_ch, Vec::new()) ), }, CheckedExtrinsic { @@ -745,7 +745,7 @@ mod tests { runtime_io::with_externalities(&mut t, || { // Verify that the contract constructor worked well and code of TRANSFER contract is actually deployed. - assert_eq!(&contract::ContractInfoOf::::get(addr).and_then(|c| c.get_alive()).unwrap().unwrap(), &transfer_ch); + assert_eq!(&contract::ContractInfoOf::::get(addr).and_then(|c| c.get_alive()).unwrap().code_hash, &transfer_ch); }); } diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 8b5e4b6d73e84..8cb4de8fd7240 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -131,21 +131,24 @@ pub enum ContractInfo { } impl ContractInfo { - fn as_alive(&self) -> Option<&AliveContractInfo> { - if let ContractInfo::Alive(ref alive) = self { + /// If contract is alive then return some alive info + pub fn get_alive(self) -> Option> { + if let ContractInfo::Alive(alive) = self { Some(alive) } else { None } } - fn get_alive(self) -> Option> { - if let ContractInfo::Alive(alive) = self { + /// If contract is alive then return some reference to alive info + pub fn as_alive(&self) -> Option<&AliveContractInfo> { + if let ContractInfo::Alive(ref alive) = self { Some(alive) } else { None } } - fn as_alive_mut(&mut self) -> Option<&mut AliveContractInfo> { + /// If contract is alive then return some mutable reference to alive info + pub fn as_alive_mut(&mut self) -> Option<&mut AliveContractInfo> { if let ContractInfo::Alive(ref mut alive) = self { Some(alive) } else { From 723eb65067d32f6a76af9f10a32a99bf1afbdecb Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 14:40:19 +0200 Subject: [PATCH 09/54] fmt --- srml/contract/src/account_db.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index fb9143cafd561..2dbca300962fd 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -26,8 +26,11 @@ use rstd::cell::RefCell; use rstd::collections::btree_map::{BTreeMap, Entry}; use rstd::prelude::*; use runtime_primitives::traits::Zero; -use srml_support::{StorageMap, traits::{UpdateBalanceOutcome, - SignedImbalance, Currency, Imbalance}, storage::child}; +use srml_support::{StorageMap, storage::child}; +use srml_support::traits::{ + UpdateBalanceOutcome, SignedImbalance, Currency, Imbalance +}; + // Note: we don't provide Option because we can't create // the trie_id in the overlay, thus we provide an overlay on the fields From c7506a16c9484aaafabfa6b08b9a4de00afe2748 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 15:39:57 +0200 Subject: [PATCH 10/54] impl getter setter rent allowance --- srml/contract/COMPLEXITY.md | 19 +++++++++++++++++++ srml/contract/src/account_db.rs | 6 ++++++ srml/contract/src/exec.rs | 15 +++++++++++++++ srml/contract/src/wasm/mod.rs | 9 +++++++++ srml/contract/src/wasm/runtime.rs | 25 +++++++++++++++++++++++++ 5 files changed, 74 insertions(+) diff --git a/srml/contract/COMPLEXITY.md b/srml/contract/COMPLEXITY.md index 9b304055f766c..a2c771077e272 100644 --- a/srml/contract/COMPLEXITY.md +++ b/srml/contract/COMPLEXITY.md @@ -327,3 +327,22 @@ This function copies slice of data from the scratch buffer to the sandbox memory 1. Storing a specified slice of the scratch buffer into the sandbox memory (see sandboxing memory set) **complexity**: The computing complexity of this function is proportional to the length of the slice. No additional memory is required. + +## ext_set_rent_allowance + +This function receives the following argument: + +- `value` buffer of a marshaled `Balance`, + +It consists of the following steps: + +1. Loading `value` buffer from the sandbox memory and then decoding it. +2. Invoking `set_rent_allowance` executive function. + +**complexity**: The complexity of this function is proportional to the size of the `value` buffer. + +## ext_rent_allowance + +This function serializes the rent allowance of the current contract into the scratch buffer. + +**complexity**: Assuming that the rent allowance is of constant size, this function has constant complexity. diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 2dbca300962fd..19ef16d31e3ab 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -205,6 +205,12 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { .or_insert(Default::default()) .code_hash = Some(code_hash); + self.local + .borrow_mut() + .entry(account.clone()) + .or_insert(Default::default()) + .rent_allowance = Some(>::zero()); + Ok(()) } pub fn set_rent_allowance(&mut self, account: &T::AccountId, rent_allowance: BalanceOf) { diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index 717ad2480ff25..c65e5e9020fa4 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -108,6 +108,12 @@ pub trait Ext { /// Deposit an event. fn deposit_event(&mut self, data: Vec); + + /// Set rent allowance of the contract + fn set_rent_allowance(&mut self, rent_allowance: BalanceOf); + + /// Rent allowance of the contract + fn rent_allowance(&self) -> BalanceOf; } /// Loader is a companion of the `Vm` trait. It loads an appropriate abstract @@ -629,6 +635,15 @@ where fn deposit_event(&mut self, data: Vec) { self.ctx.events.push(RawEvent::Contract(self.ctx.self_account.clone(), data)); } + + fn set_rent_allowance(&mut self, rent_allowance: BalanceOf) { + self.ctx.overlay.set_rent_allowance(&self.ctx.self_account, rent_allowance) + } + + fn rent_allowance(&self) -> BalanceOf { + self.ctx.overlay.get_rent_allowance(&self.ctx.self_account) + .unwrap_or(>::zero()) // Must never be triggered actually + } } /// These tests exercise the executive layer. diff --git a/srml/contract/src/wasm/mod.rs b/srml/contract/src/wasm/mod.rs index 51db3b2e5652f..a64fbc48a324e 100644 --- a/srml/contract/src/wasm/mod.rs +++ b/srml/contract/src/wasm/mod.rs @@ -200,6 +200,7 @@ mod tests { #[derive(Default)] pub struct MockExt { storage: HashMap>, + rent_allowance: u64, creates: Vec, transfers: Vec, dispatches: Vec, @@ -281,6 +282,14 @@ mod tests { fn deposit_event(&mut self, data: Vec) { self.events.push(data) } + + fn set_rent_allowance(&mut self, rent_allowance: u64) { + self.rent_allowance = rent_allowance; + } + + fn rent_allowance(&self) -> u64 { + self.rent_allowance + } } fn execute( diff --git a/srml/contract/src/wasm/runtime.rs b/srml/contract/src/wasm/runtime.rs index 37ab9fa88708f..a23fe182870e9 100644 --- a/srml/contract/src/wasm/runtime.rs +++ b/srml/contract/src/wasm/runtime.rs @@ -628,4 +628,29 @@ define_env!(Env, , Ok(()) }, + + // Set rent allowance of the contract + // + // - value_ptr: a pointer to the buffer with value, how much value to send. + // Should be decodable as a `T::Balance`. Traps otherwise. + // - value_len: length of the value buffer. + ext_set_rent_allowance(ctx, value_ptr: u32, value_len: u32) => { + let value = { + let value_buf = read_sandbox_memory(ctx, value_ptr, value_len)?; + BalanceOf::<::T>::decode(&mut &value_buf[..]) + .ok_or_else(|| sandbox::HostError)? + }; + ctx.ext.set_rent_allowance(value); + + Ok(()) + }, + + // Stores the rent allowance into the scratch buffer. + // + // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. + ext_rent_allowance(ctx) => { + ctx.scratch_buf = ctx.ext.rent_allowance().encode(); + + Ok(()) + }, ); From d5c8477a18462f63e69994e171de78ddbc74947e Mon Sep 17 00:00:00 2001 From: Sergei Pepyakin Date: Tue, 9 Apr 2019 16:43:30 +0200 Subject: [PATCH 11/54] fmt Co-Authored-By: thiolliere --- node/executor/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 48762800291fb..a296042ed71ac 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -745,7 +745,13 @@ mod tests { runtime_io::with_externalities(&mut t, || { // Verify that the contract constructor worked well and code of TRANSFER contract is actually deployed. - assert_eq!(&contract::ContractInfoOf::::get(addr).and_then(|c| c.get_alive()).unwrap().code_hash, &transfer_ch); + assert_eq!( + &contract::ContractInfoOf::::get(addr) + .and_then(|c| c.get_alive()) + .unwrap() + .code_hash, + &transfer_ch + ); }); } From 3711ac220a444adfca344d8606e9d762f4e7050f Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 16:48:56 +0200 Subject: [PATCH 12/54] comments --- srml/contract/src/account_db.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 19ef16d31e3ab..38854acd83c58 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -64,8 +64,11 @@ pub trait AccountDb { /// Trie id is None iff account doesn't have an associated trie id in >. /// Because DirectAccountDb bypass the lookup for this association. fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option>; + /// If account has an alive contract then return the code_hash associated. fn get_alive_code_hash(&self, account: &T::AccountId) -> Option>; + /// If account has a contract then return the rent allowance associated. fn get_rent_allowance(&self, account: &T::AccountId) -> Option>; + /// Returns false iff account has no alive contract nor tombstone. fn contract_exists(&self, account: &T::AccountId) -> bool; fn get_balance(&self, account: &T::AccountId) -> BalanceOf; From 8a2bee2021224a1b10e9c783f39f806006510cb3 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 16:56:02 +0200 Subject: [PATCH 13/54] doc + be->le --- srml/contract/src/lib.rs | 6 +++--- srml/contract/src/wasm/runtime.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 8cb4de8fd7240..d763cbc50beb4 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -122,9 +122,9 @@ pub trait ComputeDispatchFee { fn compute_dispatch_fee(call: &Call) -> Balance; } -#[derive(Encode, Decode)] /// Information for managing an acocunt and its sub trie abstraction. /// This is the required info to cache for an account +#[derive(Encode, Decode)] pub enum ContractInfo { Alive(AliveContractInfo), Tombstone(TombstoneContractInfo), @@ -159,7 +159,7 @@ impl ContractInfo { #[derive(Encode, Decode)] pub struct AliveContractInfo { - /// Unique ID for the subtree encoded as a byte + /// Unique ID for the subtree encoded as a byte vector pub trie_id: TrieId, /// The size of stored value in octet pub storage_size: u64, @@ -199,7 +199,7 @@ pub struct TombstoneContractInfo(T::Hash); impl TombstoneContractInfo { fn new(storage_root: Vec, storage_size: u64, code_hash: CodeHash) -> Self { let mut buf = storage_root; - buf.extend_from_slice(&storage_size.to_be_bytes()); + buf.extend_from_slice(&storage_size.to_le_bytes()); buf.extend_from_slice(code_hash.as_ref()); TombstoneContractInfo(T::Hashing::hash(&buf[..])) } diff --git a/srml/contract/src/wasm/runtime.rs b/srml/contract/src/wasm/runtime.rs index a23fe182870e9..965b339075337 100644 --- a/srml/contract/src/wasm/runtime.rs +++ b/srml/contract/src/wasm/runtime.rs @@ -631,7 +631,7 @@ define_env!(Env, , // Set rent allowance of the contract // - // - value_ptr: a pointer to the buffer with value, how much value to send. + // - value_ptr: a pointer to the buffer with value, how much to allow for rent // Should be decodable as a `T::Balance`. Traps otherwise. // - value_len: length of the value buffer. ext_set_rent_allowance(ctx, value_ptr: u32, value_len: u32) => { From 65788953fa8488b4bca46f96af6f1fe15fdeba97 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 16:58:17 +0200 Subject: [PATCH 14/54] doc --- srml/contract/src/account_db.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 38854acd83c58..b2729fb9cf999 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -66,7 +66,7 @@ pub trait AccountDb { fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option>; /// If account has an alive contract then return the code_hash associated. fn get_alive_code_hash(&self, account: &T::AccountId) -> Option>; - /// If account has a contract then return the rent allowance associated. + /// If account has an alive contract then return the rent allowance associated. fn get_rent_allowance(&self, account: &T::AccountId) -> Option>; /// Returns false iff account has no alive contract nor tombstone. fn contract_exists(&self, account: &T::AccountId) -> bool; From 2598f006a185b8de1e6a0e9011488b0a006945c6 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Tue, 9 Apr 2019 17:06:36 +0200 Subject: [PATCH 15/54] doc --- srml/contract/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index d763cbc50beb4..74c58c6f95ef7 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -159,7 +159,7 @@ impl ContractInfo { #[derive(Encode, Decode)] pub struct AliveContractInfo { - /// Unique ID for the subtree encoded as a byte vector + /// Unique ID for the subtree encoded as a bytes vector pub trie_id: TrieId, /// The size of stored value in octet pub storage_size: u64, From 3fc2cf2cf63f484f6db00971630951ee959bca66 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 10 Apr 2019 10:10:04 +0200 Subject: [PATCH 16/54] fix improve fast return --- srml/contract/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 74c58c6f95ef7..864602fa65a83 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -677,6 +677,7 @@ enum RentDecision { } impl Module { + // Check rent at this block number fn check_rent(account: &T::AccountId, block_number: T::BlockNumber) -> RentDecision { let contract = match >::get(account) { None | Some(ContractInfo::Tombstone(_)) => return RentDecision::FreeFromRent, @@ -684,7 +685,7 @@ impl Module { }; // Rent has already been payed - if contract.deduct_block == block_number { + if contract.deduct_block >= block_number { return RentDecision::FreeFromRent } From aba8d7f5f3e7a9bf5c6e998abfe452408799ff13 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 10 Apr 2019 10:16:27 +0200 Subject: [PATCH 17/54] renamings --- srml/contract/src/account_db.rs | 14 +++++++------- srml/contract/src/exec.rs | 9 ++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index b2729fb9cf999..9e66a7ce2d08f 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -64,8 +64,8 @@ pub trait AccountDb { /// Trie id is None iff account doesn't have an associated trie id in >. /// Because DirectAccountDb bypass the lookup for this association. fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option>; - /// If account has an alive contract then return the code_hash associated. - fn get_alive_code_hash(&self, account: &T::AccountId) -> Option>; + /// If account has an alive contract then return the code hash associated. + fn get_code_hash(&self, account: &T::AccountId) -> Option>; /// If account has an alive contract then return the rent allowance associated. fn get_rent_allowance(&self, account: &T::AccountId) -> Option>; /// Returns false iff account has no alive contract nor tombstone. @@ -80,7 +80,7 @@ impl AccountDb for DirectAccountDb { fn get_storage(&self, _account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option> { trie_id.and_then(|id| child::get_raw(id, location)) } - fn get_alive_code_hash(&self, account: &T::AccountId) -> Option> { + fn get_code_hash(&self, account: &T::AccountId) -> Option> { >::get(account).and_then(|i| i.as_alive().map(|i| i.code_hash)) } fn get_rent_allowance(&self, account: &T::AccountId) -> Option> { @@ -197,9 +197,9 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { .insert(location, value); } - pub fn create_new_contract(&mut self, account: &T::AccountId, code_hash: CodeHash) -> Result<(), ()> { + pub fn create_new_contract(&mut self, account: &T::AccountId, code_hash: CodeHash) -> Result<(), &'static str> { if self.contract_exists(account) { - return Err(()); + return Err("Alive contract or tombstone already exists"); } self.local @@ -242,12 +242,12 @@ impl<'a, T: Trait> AccountDb for OverlayAccountDb<'a, T> { .cloned() .unwrap_or_else(|| self.underlying.get_storage(account, trie_id, location)) } - fn get_alive_code_hash(&self, account: &T::AccountId) -> Option> { + fn get_code_hash(&self, account: &T::AccountId) -> Option> { self.local .borrow() .get(account) .and_then(|changes| changes.code_hash) - .or_else(|| self.underlying.get_alive_code_hash(account)) + .or_else(|| self.underlying.get_code_hash(account)) } fn get_rent_allowance(&self, account: &T::AccountId) -> Option> { self.local diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index c65e5e9020fa4..b57e2eb59c02f 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -328,7 +328,7 @@ where )?; } - if let Some(dest_code_hash) = self.overlay.get_alive_code_hash(&dest) { + if let Some(dest_code_hash) = self.overlay.get_code_hash(&dest) { let executable = self.loader.load_main(&dest_code_hash)?; output_data = self .vm @@ -385,8 +385,7 @@ where let (change_set, events, calls) = { let mut overlay = OverlayAccountDb::new(&self.overlay); - overlay.create_new_contract(&dest, code_hash.clone()) - .map_err(|_| "contract or tombstone already exsists")?; + overlay.create_new_contract(&dest, code_hash.clone())?; let mut nested = self.nested(overlay, dest.clone()); @@ -1261,7 +1260,7 @@ mod tests { // Check that the newly created account has the expected code hash and // there are instantiation event. - assert_eq!(ctx.overlay.get_alive_code_hash(&created_contract_address).unwrap(), dummy_ch); + assert_eq!(ctx.overlay.get_code_hash(&created_contract_address).unwrap(), dummy_ch); assert_eq!(&ctx.events, &[ RawEvent::Transfer(ALICE, created_contract_address, 100), RawEvent::Instantiated(ALICE, created_contract_address), @@ -1313,7 +1312,7 @@ mod tests { // Check that the newly created account has the expected code hash and // there are instantiation event. - assert_eq!(ctx.overlay.get_alive_code_hash(&created_contract_address).unwrap(), dummy_ch); + assert_eq!(ctx.overlay.get_code_hash(&created_contract_address).unwrap(), dummy_ch); assert_eq!(&ctx.events, &[ RawEvent::Transfer(ALICE, BOB, 20), RawEvent::Transfer(BOB, created_contract_address, 15), From c30778c1e7848144ae11de47df8908439a52635a Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 10 Apr 2019 10:59:49 +0200 Subject: [PATCH 18/54] rename + COMPLEXITY --- srml/contract/COMPLEXITY.md | 17 +++++++++++++---- srml/contract/src/account_db.rs | 20 ++++++++------------ srml/contract/src/exec.rs | 22 +++++++++++----------- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/srml/contract/COMPLEXITY.md b/srml/contract/COMPLEXITY.md index a2c771077e272..10822eacafad5 100644 --- a/srml/contract/COMPLEXITY.md +++ b/srml/contract/COMPLEXITY.md @@ -85,7 +85,7 @@ execution contexts operate on the AccountDb. All changes are flushed into underl Today `AccountDb` is implemented as a cascade of overlays with the direct storage at the bottom. Each overlay is represented by a `Map`. On a commit from an overlay to an overlay, maps are merged. On commit from an overlay to the bottommost `AccountDb` all changes are flushed to the storage. On revert, the overlay is just discarded. -## get_storage, get_code, get_balance +## get_storage, get_code_hash, get_rent_allowance, get_balance, contract_exists These functions check the local cache for a requested value and, if it is there, the value is returned. Otherwise, these functions will ask an underlying `AccountDb` for the value. This means that the number of lookups is proportional to the depth of the overlay cascade. If the value can't be found before reaching the bottommost `AccountDb`, then a DB read will be performed (in case `get_balance` the function `free_balance` will be invoked). @@ -95,7 +95,7 @@ These functions return an owned value as its result, so memory usage depends on **complexity**: The memory complexity is proportional to the size of the value. The computational complexity is proportional to the depth of the overlay cascade and the size of the value; the cost is dominated by the DB read though. -## set_storage, set_code, set_balance +## set_storage, set_balance, set_rent_allowance These functions only modify the local `Map`. @@ -105,6 +105,12 @@ While these functions only modify the local `Map`, if changes made by them are c **complexity**: Each lookup has a logarithmical computing time to the number of already inserted entries. No additional memory is required. +## create_contract + +calls contract_exists and if not modify the local `Map` similarly to set_rent_allowance. + +**complexity**: The computational complexity is proportional to the depth of the overlay cascade and the size of the value; the cost is dominated by the DB read though. No additional memory is required. + ## commit In this function, all cached values will be inserted into the underlying `AccountDb` or into the storage. @@ -337,12 +343,15 @@ This function receives the following argument: It consists of the following steps: 1. Loading `value` buffer from the sandbox memory and then decoding it. -2. Invoking `set_rent_allowance` executive function. +2. Invoking `set_rent_allowance` AccountDB function. **complexity**: The complexity of this function is proportional to the size of the `value` buffer. ## ext_rent_allowance -This function serializes the rent allowance of the current contract into the scratch buffer. +It consists of the following steps: + +1. Invoking `get_rent_allowance` AccountDB function. +2. Serializing the rent allowance of the current contract into the scratch buffer. **complexity**: Assuming that the rent allowance is of constant size, this function has constant complexity. diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 9e66a7ce2d08f..6cbbf9876e10b 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -197,27 +197,23 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { .insert(location, value); } - pub fn create_new_contract(&mut self, account: &T::AccountId, code_hash: CodeHash) -> Result<(), &'static str> { + /// Return an error if contract already exists (either if it is alive or tombstone) + pub fn create_contract(&mut self, account: &T::AccountId, code_hash: CodeHash) -> Result<(), &'static str> { if self.contract_exists(account) { return Err("Alive contract or tombstone already exists"); } - self.local - .borrow_mut() - .entry(account.clone()) - .or_insert(Default::default()) - .code_hash = Some(code_hash); + let mut local = self.local.borrow_mut(); + let contract = local.entry(account.clone()) + .or_insert(Default::default()); - self.local - .borrow_mut() - .entry(account.clone()) - .or_insert(Default::default()) - .rent_allowance = Some(>::zero()); + contract.code_hash = Some(code_hash); + contract.rent_allowance = Some(>::zero()); Ok(()) } + /// Assume contract exists pub fn set_rent_allowance(&mut self, account: &T::AccountId, rent_allowance: BalanceOf) { - // TODO TODO: maybe assert contracts is alive self.local .borrow_mut() .entry(account.clone()) diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index b57e2eb59c02f..6ed12c9771b76 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -385,7 +385,7 @@ where let (change_set, events, calls) = { let mut overlay = OverlayAccountDb::new(&self.overlay); - overlay.create_new_contract(&dest, code_hash.clone())?; + overlay.create_contract(&dest, code_hash.clone())?; let mut nested = self.nested(overlay, dest.clone()); @@ -781,7 +781,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.create_new_contract(&BOB, exec_ch).unwrap(); + ctx.overlay.create_contract(&BOB, exec_ch).unwrap(); assert_matches!( ctx.call(BOB, value, &mut gas_meter, &data, EmptyOutputBuf::new()), @@ -1016,7 +1016,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.create_new_contract(&BOB, return_ch).unwrap(); + ctx.overlay.create_contract(&BOB, return_ch).unwrap(); let result = ctx.call( dest, @@ -1044,7 +1044,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.create_new_contract(&BOB, input_data_ch).unwrap(); + ctx.overlay.create_contract(&BOB, input_data_ch).unwrap(); let result = ctx.call( BOB, @@ -1103,7 +1103,7 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.create_new_contract(&BOB, recurse_ch).unwrap(); + ctx.overlay.create_contract(&BOB, recurse_ch).unwrap(); let result = ctx.call( BOB, @@ -1150,8 +1150,8 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.create_new_contract(&dest, bob_ch).unwrap(); - ctx.overlay.create_new_contract(&CHARLIE, charlie_ch).unwrap(); + ctx.overlay.create_contract(&dest, bob_ch).unwrap(); + ctx.overlay.create_contract(&CHARLIE, charlie_ch).unwrap(); let result = ctx.call( dest, @@ -1193,8 +1193,8 @@ mod tests { with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.create_new_contract(&BOB, bob_ch).unwrap(); - ctx.overlay.create_new_contract(&CHARLIE, charlie_ch).unwrap(); + ctx.overlay.create_contract(&BOB, bob_ch).unwrap(); + ctx.overlay.create_contract(&CHARLIE, charlie_ch).unwrap(); let result = ctx.call( BOB, @@ -1301,7 +1301,7 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); ctx.overlay.set_balance(&ALICE, 1000); - ctx.overlay.create_new_contract(&BOB, creator_ch).unwrap(); + ctx.overlay.create_contract(&BOB, creator_ch).unwrap(); assert_matches!( ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), &[], EmptyOutputBuf::new()), @@ -1352,7 +1352,7 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); ctx.overlay.set_balance(&ALICE, 1000); - ctx.overlay.create_new_contract(&BOB, creator_ch).unwrap(); + ctx.overlay.create_contract(&BOB, creator_ch).unwrap(); assert_matches!( ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), &[], EmptyOutputBuf::new()), From 36b70a9d79928aaa1f6b5acba47504e10404f9a7 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 10 Apr 2019 11:02:46 +0200 Subject: [PATCH 19/54] COMPLEXITY --- srml/contract/COMPLEXITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/contract/COMPLEXITY.md b/srml/contract/COMPLEXITY.md index 10822eacafad5..c03d407f399fc 100644 --- a/srml/contract/COMPLEXITY.md +++ b/srml/contract/COMPLEXITY.md @@ -345,7 +345,7 @@ It consists of the following steps: 1. Loading `value` buffer from the sandbox memory and then decoding it. 2. Invoking `set_rent_allowance` AccountDB function. -**complexity**: The complexity of this function is proportional to the size of the `value` buffer. +**complexity**: Complexity is proportional to the size of the `value`. This function induces a DB write of size proportional to the `value` size (if flushed to the storage), so should be priced accordingly. ## ext_rent_allowance From 712fe17c70153221b22284013142fd8e7abb41ba Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 10 Apr 2019 13:21:03 +0200 Subject: [PATCH 20/54] add test --- srml/contract/src/exec.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index 6ed12c9771b76..3b60d10f93521 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -1056,7 +1056,7 @@ mod tests { assert_matches!(result, Ok(_)); }); - // This one tests passing the input data into a contract via call. + // This one tests passing the input data into a contract via instantiate. with_externalities(&mut ExtBuilder::default().build(), || { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); @@ -1367,4 +1367,29 @@ mod tests { } ); } + + #[test] + fn rent_allowance() { + let vm = MockVm::new(); + let mut loader = MockLoader::empty(); + let rent_allowance_ch = loader.insert(|ctx| { + assert_eq!(ctx.ext.rent_allowance(), 0); + ctx.ext.set_rent_allowance(10); + assert_eq!(ctx.ext.rent_allowance(), 10); + VmExecResult::Ok + }); + + with_externalities(&mut ExtBuilder::default().build(), || { + let cfg = Config::preload(); + let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); + + let result = ctx.instantiate( + 0, + &mut GasMeter::::with_limit(10000, 1), + &rent_allowance_ch, + &[], + ); + assert_matches!(result, Ok(_)); + }); + } } From 005752b9862d5536b633e92b86fccb8dafb476c6 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Wed, 10 Apr 2019 13:33:33 +0200 Subject: [PATCH 21/54] etrinsic claim surcharge delay configurable --- node/cli/src/chain_spec.rs | 2 ++ srml/contract/src/lib.rs | 7 ++++++- srml/contract/src/tests.rs | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/node/cli/src/chain_spec.rs b/node/cli/src/chain_spec.rs index a96e1712c918a..c96fd75848af8 100644 --- a/node/cli/src/chain_spec.rs +++ b/node/cli/src/chain_spec.rs @@ -152,6 +152,7 @@ fn staging_testnet_config_genesis() -> GenesisConfig { burn: Permill::from_percent(50), }), contract: Some(ContractConfig { + extrinsic_claim_delay: 2, rent_byte_price: 4, rent_deposit_offset: 1000, storage_size_offset: 8, @@ -314,6 +315,7 @@ pub fn testnet_genesis( burn: Permill::from_percent(50), }), contract: Some(ContractConfig { + extrinsic_claim_delay: 2, rent_byte_price: 4, rent_deposit_offset: 1000, storage_size_offset: 8, diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 864602fa65a83..02ee1aa686ca1 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -468,7 +468,7 @@ decl_module! { // Make validators advantaged if signed { - check_block -= T::BlockNumber::sa(2u64); + check_block -= >::extrinsic_claim_delay(); } match Self::check_rent(&dest, check_block) { @@ -517,6 +517,11 @@ decl_event! { decl_storage! { trait Store for Module as Contract { + /// Number of block delay an extrinsic claim surcharge has. + /// + /// When claim surchage is called by an extrinsic the rent is checked + /// for current_block - delay + ExtrinsicClaimDelay get(extrinsic_claim_delay) config(): T::BlockNumber; /// The minimum amount required to generate a tombstone. TombstoneDeposit get(tombstone_deposit) config(): BalanceOf; /// Size of a contract at the time of creation. This is a simple way to ensure diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index e923caf043ccb..672efdc317a6e 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -199,6 +199,7 @@ impl ExtBuilder { ); t.extend( GenesisConfig:: { + extrinsic_claim_delay: 2, rent_byte_price: 4, rent_deposit_offset: 1000, storage_size_offset: 8, From dfc4e66f0731b952e5941da371540668d0c38a77 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 11 Apr 2019 16:15:44 +0200 Subject: [PATCH 22/54] comment addressed --- srml/contract/src/lib.rs | 103 ++++++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 35 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 02ee1aa686ca1..da5237286f4b0 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -451,7 +451,12 @@ decl_module! { result.map(|_| ()) } - fn claim_surchage(origin, dest: T::AccountId, aux_sender: Option) { + /// Allows validators to claim a small reward for evicting a contract. If a validator + /// fails to do so, a regular users will be allowed to do so. + /// + /// If contract is not evicted as a result of this call, no actions are taken and + /// the sender is not eligible for the reward. + fn claim_surcharge(origin, dest: T::AccountId, aux_sender: Option) { let origin = origin.into(); let (signed, rewarded) = match origin { Some(system::RawOrigin::Signed(ref account)) if aux_sender.is_none() => { @@ -468,16 +473,15 @@ decl_module! { // Make validators advantaged if signed { - check_block -= >::extrinsic_claim_delay(); + check_block = check_block.saturating_sub(>::signed_claim_handicap()); } match Self::check_rent(&dest, check_block) { - RentDecision::CantWithdrawRentWithoutDying(rent) - | RentDecision::AllowanceExceeded(rent) => { + RentDecision::Evict(rent) => { T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; Self::evict(&dest, rent); } - RentDecision::FreeFromRent | RentDecision::CollectDues(_) => (), + RentDecision::ExemptFromRent | RentDecision::CollectDues(_) => (), } } @@ -521,7 +525,7 @@ decl_storage! { /// /// When claim surchage is called by an extrinsic the rent is checked /// for current_block - delay - ExtrinsicClaimDelay get(extrinsic_claim_delay) config(): T::BlockNumber; + SignedClaimHandicap get(signed_claim_handicap) config(): T::BlockNumber; /// The minimum amount required to generate a tombstone. TombstoneDeposit get(tombstone_deposit) config(): BalanceOf; /// Size of a contract at the time of creation. This is a simple way to ensure @@ -531,6 +535,11 @@ decl_storage! { RentByteFee get(rent_byte_price) config(): BalanceOf; /// The amount of funds a contract should deposit in order to offset /// the cost of one byte. + /// + /// Let's suppose the deposit is 1,000 EDG/byte and the rent is 1 EDG/byte/day, then a contract + /// with 1,000,000 EDG that uses 1,000 bytes of storage would pay no rent. + /// But if the balance reduced to 500,000 EDG and the storage stayed the same at 1,000, + /// then it would pay 500 EDG/day. RentDepositOffset get(rent_deposit_offset) config(): BalanceOf; /// Reward that is received by the party whose touch has led /// to removal of a contract. @@ -670,76 +679,98 @@ impl> Default for Schedule { } enum RentDecision { - /// Happens when the rent is offset completely by the `rent_deposit_offset`. - /// Or Rent has already been payed - /// Or account doesn't has rent because no contract or tombstone contract - FreeFromRent, - /// The contract still have funds to keep itself from eviction. Collect dues. - /// It is ensure that account can withdraw dues without dying. + /// The contract is free from rent, either: + /// * rent is offset completely by the `rent_deposit_offset`, + /// * or rent has already been paid for this block number, + /// * or account doesn't have a contract, + /// * or account has a tombstone. + ExemptFromRent, + /// The contract still has funds to keep itself from eviction. Collect dues. + /// + /// It ensures that: + /// * account can withdraw without going below subsistance threshold + /// * account has an alive contract (otherwise he would be exempt from rent) + /// * rent is below or equal to rent allowance (otherwise must be evicted) CollectDues(BalanceOf), - CantWithdrawRentWithoutDying(BalanceOf), - AllowanceExceeded(BalanceOf), + /// The contract must be evicted, either: + /// * rent exceed rent allowance, then rent allowance is returned + /// * or rent allowance is not reached and: + /// * the account can't withdraw the rent, + /// * or he will go bellow subsistance threshold. + /// + /// It ensures that account had an alive contract + /// (otherwise he would be exempt from rent). + Evict(BalanceOf), } impl Module { - // Check rent at this block number + /// Check rent at this block number and return the RentDecition according to spec. fn check_rent(account: &T::AccountId, block_number: T::BlockNumber) -> RentDecision { let contract = match >::get(account) { - None | Some(ContractInfo::Tombstone(_)) => return RentDecision::FreeFromRent, + None | Some(ContractInfo::Tombstone(_)) => return RentDecision::ExemptFromRent, Some(ContractInfo::Alive(contract)) => contract, }; - // Rent has already been payed + // Rent has already been paid if contract.deduct_block >= block_number { - return RentDecision::FreeFromRent + return RentDecision::ExemptFromRent } let balance = T::Currency::free_balance(account); + let free_storage = balance.checked_div(&>::rent_deposit_offset()) + .unwrap_or(>::sa(0))); + let effective_storage_size = >::sa(contract.storage_size) - .saturating_sub(balance.checked_div(&>::rent_deposit_offset()).unwrap_or(>::sa(0))); + .saturating_sub(free_storage); let fee_per_block: BalanceOf = effective_storage_size * >::rent_byte_price(); 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 RentDecision::FreeFromRent + return RentDecision::ExemptFromRent } - let rent = fee_per_block * >::sa((block_number.saturating_sub(contract.deduct_block)).as_()); + let blocks_to_rent= block_number.saturating_sub(contract.deduct_block); + let rent = fee_per_block * >::sa(blocks_to_rent.as_()); + let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); // Pay rent up to rent_allowance if rent <= contract.rent_allowance { - // TODO TODO: this could be one call if ensure_can_withdraw took exitence requirement - if balance.saturating_sub(rent) > T::Currency::minimum_balance() - && T::Currency::ensure_can_withdraw(account, rent, WithdrawReason::Fee, balance.saturating_sub(rent)).is_ok() + let new_balance = balance.saturating_sub(rent); + if new_balance >= subsistence_threshold + && T::Currency::ensure_can_withdraw(account, rent, WithdrawReason::Fee, new_balance).is_ok() { RentDecision::CollectDues(rent) } else { - RentDecision::CantWithdrawRentWithoutDying(rent) + // TODO TODO: should we prefer returning balance - subsistence_deposit + // TODO TODO: when a contract is restored do we want it to pay for the unpaid rent ? + RentDecision::Evict(rent) } } else { - RentDecision::AllowanceExceeded(contract.rent_allowance) + RentDecision::Evict(contract.rent_allowance) } } fn pay_rent(account: &T::AccountId) { match Self::check_rent(account, >::block_number()) { - RentDecision::FreeFromRent => (), + RentDecision::ExemptFromRent => (), RentDecision::CollectDues(rent) => { let imbalance = T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive) - .expect("Assumption of check_rent"); + .expect("Rent decision collect dues ensure account can withdraw \ + rent while being above subsistance threshold; + subsistance threshold is superior to existencial deposit; + thus account can withdraw while being kept alive; qed"); >::mutate(account, |contract| { // rent_allowance isn't reached is guaranted by check_rent contract.as_mut() .and_then(|c| c.as_alive_mut()) - .expect("Only existing alive contracts pay rent") + .expect("Rent decision collect dues only on alive contract; qed") .rent_allowance -= imbalance.peek(); }) }, - RentDecision::CantWithdrawRentWithoutDying(rent) - | RentDecision::AllowanceExceeded(rent) => { + RentDecision::Evict(rent) => { Self::evict(account, rent); } } @@ -747,16 +778,18 @@ impl Module { fn evict(account: &T::AccountId, rent: BalanceOf) { match T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::AllowDeath) { - Ok(imbalance) => drop(imbalance), - Err(_) => { T::Currency::slash(account, rent); }, + Ok(_imbalance) => (), + Err(_) => { let (_imbalance, _remaining) = T::Currency::slash(account, rent); }, } - if T::Currency::free_balance(account) >= >::tombstone_deposit() + T::Currency::minimum_balance() { + let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); + if T::Currency::free_balance(account) >= subsistence_threshold { let contract = >::get(account) .and_then(|c| c.get_alive()) - .expect("Rent is only payed by alive contracts"); + .expect("Only alive contract can be evicted; "); let tombstone = TombstoneContractInfo::new( + // Note: this operation is heavy runtime_io::child_storage_root(&contract.trie_id).unwrap(), contract.storage_size, contract.code_hash, From ed016c24a87fd33d261562d36cbc5ffd4be7fe3d Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 11 Apr 2019 17:35:09 +0200 Subject: [PATCH 23/54] move and rewrite of pay_rent --- srml/contract/src/exec.rs | 4 +- srml/contract/src/lib.rs | 138 ++------------------------------------ srml/contract/src/rent.rs | 116 ++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 135 deletions(-) create mode 100644 srml/contract/src/rent.rs diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index 3b60d10f93521..51d3d320dd9f6 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -15,7 +15,7 @@ // along with Substrate. If not, see . use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, - TrieId, BalanceOf, ContractInfoOf, Module}; + TrieId, BalanceOf, ContractInfoOf}; use crate::account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}; use crate::gas::{GasMeter, Token, approx_gas_for_balance}; @@ -307,7 +307,7 @@ where // Assumption: pay_rent doesn't collide with overlay because // pay_rent will be done on first call and dest contract and balance // cannot be change before first call - >::pay_rent(&dest); + crate::rent::pay_rent::(&dest); let mut output_data = Vec::new(); diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index da5237286f4b0..e21a208af6b3b 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -88,6 +88,7 @@ mod gas; mod account_db; mod exec; mod wasm; +mod rent; #[cfg(test)] mod tests; @@ -101,11 +102,10 @@ use substrate_primitives::crypto::UncheckedFrom; use rstd::prelude::*; use rstd::marker::PhantomData; use parity_codec::{Codec, Encode, Decode}; -use runtime_primitives::traits::{Hash, As, SimpleArithmetic,Bounded, StaticLookup, Saturating, CheckedDiv, Zero}; +use runtime_primitives::traits::{Hash, As, SimpleArithmetic,Bounded, StaticLookup, Saturating}; use srml_support::dispatch::{Result, Dispatchable}; use srml_support::{Parameter, StorageMap, StorageValue, decl_module, decl_event, decl_storage, storage::child}; -use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency, - WithdrawReason, ExistenceRequirement, Imbalance}; +use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency}; use system::{ensure_signed, RawOrigin}; use timestamp; @@ -476,12 +476,8 @@ decl_module! { check_block = check_block.saturating_sub(>::signed_claim_handicap()); } - match Self::check_rent(&dest, check_block) { - RentDecision::Evict(rent) => { - T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; - Self::evict(&dest, rent); - } - RentDecision::ExemptFromRent | RentDecision::CollectDues(_) => (), + if rent::try_evict_at::(&dest, check_block) { + T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; } } @@ -677,127 +673,3 @@ impl> Default for Schedule { } } } - -enum RentDecision { - /// The contract is free from rent, either: - /// * rent is offset completely by the `rent_deposit_offset`, - /// * or rent has already been paid for this block number, - /// * or account doesn't have a contract, - /// * or account has a tombstone. - ExemptFromRent, - /// The contract still has funds to keep itself from eviction. Collect dues. - /// - /// It ensures that: - /// * account can withdraw without going below subsistance threshold - /// * account has an alive contract (otherwise he would be exempt from rent) - /// * rent is below or equal to rent allowance (otherwise must be evicted) - CollectDues(BalanceOf), - /// The contract must be evicted, either: - /// * rent exceed rent allowance, then rent allowance is returned - /// * or rent allowance is not reached and: - /// * the account can't withdraw the rent, - /// * or he will go bellow subsistance threshold. - /// - /// It ensures that account had an alive contract - /// (otherwise he would be exempt from rent). - Evict(BalanceOf), -} - -impl Module { - /// Check rent at this block number and return the RentDecition according to spec. - fn check_rent(account: &T::AccountId, block_number: T::BlockNumber) -> RentDecision { - let contract = match >::get(account) { - None | Some(ContractInfo::Tombstone(_)) => return RentDecision::ExemptFromRent, - Some(ContractInfo::Alive(contract)) => contract, - }; - - // Rent has already been paid - if contract.deduct_block >= block_number { - return RentDecision::ExemptFromRent - } - - let balance = T::Currency::free_balance(account); - - let free_storage = balance.checked_div(&>::rent_deposit_offset()) - .unwrap_or(>::sa(0))); - - let effective_storage_size = >::sa(contract.storage_size) - .saturating_sub(free_storage); - - let fee_per_block: BalanceOf = effective_storage_size * >::rent_byte_price(); - - 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 RentDecision::ExemptFromRent - } - - let blocks_to_rent= block_number.saturating_sub(contract.deduct_block); - let rent = fee_per_block * >::sa(blocks_to_rent.as_()); - let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); - - // Pay rent up to rent_allowance - if rent <= contract.rent_allowance { - let new_balance = balance.saturating_sub(rent); - if new_balance >= subsistence_threshold - && T::Currency::ensure_can_withdraw(account, rent, WithdrawReason::Fee, new_balance).is_ok() - { - RentDecision::CollectDues(rent) - } else { - // TODO TODO: should we prefer returning balance - subsistence_deposit - // TODO TODO: when a contract is restored do we want it to pay for the unpaid rent ? - RentDecision::Evict(rent) - } - } else { - RentDecision::Evict(contract.rent_allowance) - } - } - - fn pay_rent(account: &T::AccountId) { - match Self::check_rent(account, >::block_number()) { - RentDecision::ExemptFromRent => (), - RentDecision::CollectDues(rent) => { - let imbalance = T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive) - .expect("Rent decision collect dues ensure account can withdraw \ - rent while being above subsistance threshold; - subsistance threshold is superior to existencial deposit; - thus account can withdraw while being kept alive; qed"); - >::mutate(account, |contract| { - // rent_allowance isn't reached is guaranted by check_rent - contract.as_mut() - .and_then(|c| c.as_alive_mut()) - .expect("Rent decision collect dues only on alive contract; qed") - .rent_allowance -= imbalance.peek(); - }) - }, - RentDecision::Evict(rent) => { - Self::evict(account, rent); - } - } - } - - fn evict(account: &T::AccountId, rent: BalanceOf) { - match T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::AllowDeath) { - Ok(_imbalance) => (), - Err(_) => { let (_imbalance, _remaining) = T::Currency::slash(account, rent); }, - } - - let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); - if T::Currency::free_balance(account) >= subsistence_threshold { - let contract = >::get(account) - .and_then(|c| c.get_alive()) - .expect("Only alive contract can be evicted; "); - - let tombstone = TombstoneContractInfo::new( - // Note: this operation is heavy - runtime_io::child_storage_root(&contract.trie_id).unwrap(), - contract.storage_size, - contract.code_hash, - ); - >::insert(account, ContractInfo::Tombstone(tombstone)); - } else { - // remove if less than substantial deposit - T::Currency::make_free_balance_be(account, >::zero()); - } - } -} diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs new file mode 100644 index 0000000000000..f3fd5ae718e95 --- /dev/null +++ b/srml/contract/src/rent.rs @@ -0,0 +1,116 @@ +use crate::{Trait, Module, ContractInfoOf, ContractInfo, BalanceOf, TombstoneContractInfo}; +use srml_support::traits::{Currency, WithdrawReason, ExistenceRequirement, Imbalance}; +use srml_support::StorageMap; +use runtime_primitives::traits::{As, Saturating, CheckedDiv, Zero}; + +/// Evict and optionnaly pay rent, at block number. Return if evicted +/// +/// Exempted from rent iff: +/// * rent is offset completely by the `rent_deposit_offset`, +/// * or rent has already been paid for this block number, +/// * or account doesn't have a contract, +/// * or account has a tombstone. +/// +/// Evicted iff: +/// * rent exceed rent allowance, +/// * or can't withdraw the rent, +/// * or go below subsistence threshold. +fn try_evict_or_and_pay_rent(account: &T::AccountId, block_number: T::BlockNumber, pay_rent: bool) -> bool { + let contract = match >::get(account) { + None | Some(ContractInfo::Tombstone(_)) => return false, + Some(ContractInfo::Alive(contract)) => contract, + }; + + // Rent has already been paid + if contract.deduct_block >= block_number { + return false + } + + let balance = T::Currency::free_balance(account); + + let free_storage = balance.checked_div(&>::rent_deposit_offset()) + .unwrap_or(>::sa(0)); + + let effective_storage_size = >::sa(contract.storage_size) + .saturating_sub(free_storage); + + let fee_per_block: BalanceOf = effective_storage_size * >::rent_byte_price(); + + 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 false + } + + let blocks_to_rent= block_number.saturating_sub(contract.deduct_block); + let rent = fee_per_block * >::sa(blocks_to_rent.as_()); + let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); + + let rent_limited = rent.min(contract.rent_allowance); + + let rent_allowance_exceeded = rent > contract.rent_allowance; + let go_below_subsistence = balance.saturating_sub(rent_limited) >= subsistence_threshold; + let is_below_subsistence = balance >= subsistence_threshold; + let can_withdraw_rent = T::Currency::ensure_can_withdraw( + account, + rent_limited, + WithdrawReason::Fee, + balance.saturating_sub(rent_limited), + ).is_ok(); + + if !rent_allowance_exceeded + && can_withdraw_rent + && !go_below_subsistence + { + // Collect dues + + if pay_rent { + let imbalance = T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive) + .expect("Withdraw has been checked above; + go_below_subsistence is false and subsistence > existencial_deposit; + qed"); + + >::mutate(account, |contract| { + contract.as_mut() + .and_then(|c| c.as_alive_mut()) + .expect("Dead or inexistent account has been exempt above; qed") + .rent_allowance -= imbalance.peek(); // rent_allowance is not exceeded + }) + } + return false; + + } else { + // Evict + + if can_withdraw_rent && !go_below_subsistence { + T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive) + .expect("Can withdraw and don't go below subsistence"); + } else if !is_below_subsistence { + T::Currency::make_free_balance_be(account, subsistence_threshold); + } else { + T::Currency::make_free_balance_be(account, >::zero()); + } + + if !is_below_subsistence { + let tombstone = TombstoneContractInfo::new( + // Note: this operation is heavy + runtime_io::child_storage_root(&contract.trie_id).unwrap(), // TODO TODO: this unwrap ? + contract.storage_size, + contract.code_hash, + ); + >::insert(account, ContractInfo::Tombstone(tombstone)); + } + + return true; + } +} + +/// Make account paying the rent for the current block number +pub fn pay_rent(account: &T::AccountId) { + try_evict_or_and_pay_rent::(account, >::block_number(), true); +} + +/// Make account paying the rent for the current block number +pub fn try_evict_at(account: &T::AccountId, block: T::BlockNumber) -> bool { + try_evict_or_and_pay_rent::(account, block, false) +} From 37c6e9bcfc64fe75af2ab83ffa898a91539efb41 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 11 Apr 2019 17:40:44 +0200 Subject: [PATCH 24/54] remove child trie --- srml/contract/src/rent.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index f3fd5ae718e95..f9b3e078f4e2c 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -99,6 +99,7 @@ fn try_evict_or_and_pay_rent(account: &T::AccountId, block_number: T:: contract.code_hash, ); >::insert(account, ContractInfo::Tombstone(tombstone)); + runtime_io::kill_child_storage(&contract.trie_id); } return true; From 2b027f28e6ab8f3607aa0a9701a88e7b9e960b90 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 11 Apr 2019 17:48:52 +0200 Subject: [PATCH 25/54] fmt --- srml/contract/src/account_db.rs | 36 ++++++++++--------- srml/contract/src/rent.rs | 61 ++++++++++++++++++++------------- 2 files changed, 58 insertions(+), 39 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 6cbbf9876e10b..fe30430301cf8 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -17,20 +17,17 @@ //! Auxilliaries to help with managing partial changes to accounts state. use super::{ - CodeHash, Trait, TrieId, ContractInfoOf, BalanceOf, ContractInfo, - AliveContractInfo, TrieIdGenerator, Module + AliveContractInfo, BalanceOf, CodeHash, ContractInfo, ContractInfoOf, Module, Trait, TrieId, + TrieIdGenerator, }; use crate::exec::StorageKey; -use system; use rstd::cell::RefCell; use rstd::collections::btree_map::{BTreeMap, Entry}; use rstd::prelude::*; use runtime_primitives::traits::Zero; -use srml_support::{StorageMap, storage::child}; -use srml_support::traits::{ - UpdateBalanceOutcome, SignedImbalance, Currency, Imbalance -}; - +use srml_support::traits::{Currency, Imbalance, SignedImbalance, UpdateBalanceOutcome}; +use srml_support::{storage::child, StorageMap}; +use system; // Note: we don't provide Option because we can't create // the trie_id in the overlay, thus we provide an overlay on the fields @@ -106,7 +103,10 @@ impl AccountDb for DirectAccountDb { } } - if changed.code_hash.is_some() || changed.rent_allowance.is_some() || !changed.storage.is_empty() { + if changed.code_hash.is_some() + || changed.rent_allowance.is_some() + || !changed.storage.is_empty() + { let old_info = match >::get(&address) { Some(ContractInfo::Alive(alive)) => Some(alive), None => None, @@ -149,7 +149,10 @@ impl AccountDb for DirectAccountDb { } } - if old_info.map(|old_info| old_info == new_info).unwrap_or(true) { + if old_info + .map(|old_info| old_info == new_info) + .unwrap_or(true) + { >::insert(&address, ContractInfo::Alive(new_info)); } } @@ -171,9 +174,7 @@ pub struct OverlayAccountDb<'a, T: Trait + 'a> { underlying: &'a AccountDb, } impl<'a, T: Trait> OverlayAccountDb<'a, T> { - pub fn new( - underlying: &'a AccountDb, - ) -> OverlayAccountDb<'a, T> { + pub fn new(underlying: &'a AccountDb) -> OverlayAccountDb<'a, T> { OverlayAccountDb { local: RefCell::new(ChangeSet::new()), underlying, @@ -198,14 +199,17 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { } /// Return an error if contract already exists (either if it is alive or tombstone) - pub fn create_contract(&mut self, account: &T::AccountId, code_hash: CodeHash) -> Result<(), &'static str> { + pub fn create_contract( + &mut self, + account: &T::AccountId, + code_hash: CodeHash, + ) -> Result<(), &'static str> { if self.contract_exists(account) { return Err("Alive contract or tombstone already exists"); } let mut local = self.local.borrow_mut(); - let contract = local.entry(account.clone()) - .or_insert(Default::default()); + let contract = local.entry(account.clone()).or_insert(Default::default()); contract.code_hash = Some(code_hash); contract.rent_allowance = Some(>::zero()); diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index f9b3e078f4e2c..b13b581e02a7b 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -1,7 +1,7 @@ -use crate::{Trait, Module, ContractInfoOf, ContractInfo, BalanceOf, TombstoneContractInfo}; -use srml_support::traits::{Currency, WithdrawReason, ExistenceRequirement, Imbalance}; +use crate::{BalanceOf, ContractInfo, ContractInfoOf, Module, TombstoneContractInfo, Trait}; +use runtime_primitives::traits::{As, CheckedDiv, Saturating, Zero}; +use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; -use runtime_primitives::traits::{As, Saturating, CheckedDiv, Zero}; /// Evict and optionnaly pay rent, at block number. Return if evicted /// @@ -15,7 +15,11 @@ use runtime_primitives::traits::{As, Saturating, CheckedDiv, Zero}; /// * rent exceed rent allowance, /// * or can't withdraw the rent, /// * or go below subsistence threshold. -fn try_evict_or_and_pay_rent(account: &T::AccountId, block_number: T::BlockNumber, pay_rent: bool) -> bool { +fn try_evict_or_and_pay_rent( + account: &T::AccountId, + block_number: T::BlockNumber, + pay_rent: bool, +) -> bool { let contract = match >::get(account) { None | Some(ContractInfo::Tombstone(_)) => return false, Some(ContractInfo::Alive(contract)) => contract, @@ -23,26 +27,27 @@ fn try_evict_or_and_pay_rent(account: &T::AccountId, block_number: T:: // Rent has already been paid if contract.deduct_block >= block_number { - return false + return false; } let balance = T::Currency::free_balance(account); - let free_storage = balance.checked_div(&>::rent_deposit_offset()) + let free_storage = balance + .checked_div(&>::rent_deposit_offset()) .unwrap_or(>::sa(0)); - let effective_storage_size = >::sa(contract.storage_size) - .saturating_sub(free_storage); + let effective_storage_size = + >::sa(contract.storage_size).saturating_sub(free_storage); let fee_per_block: BalanceOf = effective_storage_size * >::rent_byte_price(); 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 false + return false; } - let blocks_to_rent= block_number.saturating_sub(contract.deduct_block); + let blocks_to_rent = block_number.saturating_sub(contract.deduct_block); let rent = fee_per_block * >::sa(blocks_to_rent.as_()); let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); @@ -56,41 +61,51 @@ fn try_evict_or_and_pay_rent(account: &T::AccountId, block_number: T:: rent_limited, WithdrawReason::Fee, balance.saturating_sub(rent_limited), - ).is_ok(); + ) + .is_ok(); - if !rent_allowance_exceeded - && can_withdraw_rent - && !go_below_subsistence - { + if !rent_allowance_exceeded && can_withdraw_rent && !go_below_subsistence { // Collect dues if pay_rent { - let imbalance = T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive) - .expect("Withdraw has been checked above; + let imbalance = T::Currency::withdraw( + account, + rent, + WithdrawReason::Fee, + ExistenceRequirement::KeepAlive, + ) + .expect( + "Withdraw has been checked above; go_below_subsistence is false and subsistence > existencial_deposit; - qed"); + qed", + ); >::mutate(account, |contract| { - contract.as_mut() + contract + .as_mut() .and_then(|c| c.as_alive_mut()) .expect("Dead or inexistent account has been exempt above; qed") .rent_allowance -= imbalance.peek(); // rent_allowance is not exceeded }) } return false; - } else { // Evict if can_withdraw_rent && !go_below_subsistence { - T::Currency::withdraw(account, rent, WithdrawReason::Fee, ExistenceRequirement::KeepAlive) - .expect("Can withdraw and don't go below subsistence"); + T::Currency::withdraw( + account, + rent, + WithdrawReason::Fee, + ExistenceRequirement::KeepAlive, + ) + .expect("Can withdraw and don't go below subsistence"); } else if !is_below_subsistence { T::Currency::make_free_balance_be(account, subsistence_threshold); } else { T::Currency::make_free_balance_be(account, >::zero()); } - + if !is_below_subsistence { let tombstone = TombstoneContractInfo::new( // Note: this operation is heavy From efa6afd4194e5bf86abc18ea71b7363256410ace Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 11 Apr 2019 18:05:14 +0200 Subject: [PATCH 26/54] use derive --- node/cli/src/chain_spec.rs | 4 ++-- srml/contract/src/account_db.rs | 2 +- srml/contract/src/lib.rs | 37 +++++++-------------------------- srml/contract/src/tests.rs | 2 +- 4 files changed, 12 insertions(+), 33 deletions(-) diff --git a/node/cli/src/chain_spec.rs b/node/cli/src/chain_spec.rs index c96fd75848af8..1c7236c935f0f 100644 --- a/node/cli/src/chain_spec.rs +++ b/node/cli/src/chain_spec.rs @@ -152,7 +152,7 @@ fn staging_testnet_config_genesis() -> GenesisConfig { burn: Permill::from_percent(50), }), contract: Some(ContractConfig { - extrinsic_claim_delay: 2, + signed_claim_handicap: 2, rent_byte_price: 4, rent_deposit_offset: 1000, storage_size_offset: 8, @@ -315,7 +315,7 @@ pub fn testnet_genesis( burn: Permill::from_percent(50), }), contract: Some(ContractConfig { - extrinsic_claim_delay: 2, + signed_claim_handicap: 2, rent_byte_price: 4, rent_deposit_offset: 1000, storage_size_offset: 8, diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index fe30430301cf8..126a289061b2c 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -117,7 +117,7 @@ impl AccountDb for DirectAccountDb { let mut new_info = if let Some(info) = old_info.clone() { info } else if let Some(code_hash) = changed.code_hash { - AliveContractInfo { + AliveContractInfo:: { code_hash, storage_size: >::storage_size_offset(), trie_id: ::TrieIdGenerator::trie_id(&address), diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index e21a208af6b3b..bdfacb828010c 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -157,40 +157,19 @@ impl ContractInfo { } } -#[derive(Encode, Decode)] -pub struct AliveContractInfo { +pub type AliveContractInfo = RawAliveContractInfo, BalanceOf, ::BlockNumber>; + +// Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. +#[derive(Encode, Decode, Clone, PartialEq, Eq)] +pub struct RawAliveContractInfo { /// Unique ID for the subtree encoded as a bytes vector pub trie_id: TrieId, /// The size of stored value in octet pub storage_size: u64, /// The code associated with a given account. - pub code_hash: CodeHash, - pub rent_allowance: BalanceOf, - pub deduct_block: T::BlockNumber, -} - -impl Clone for AliveContractInfo { - fn clone(&self) -> Self { - AliveContractInfo { - trie_id: self.trie_id.clone(), - storage_size: self.storage_size.clone(), - code_hash: self.code_hash.clone(), - rent_allowance: self.rent_allowance.clone(), - deduct_block: self.deduct_block.clone(), - } - } -} - -impl Eq for AliveContractInfo {} - -impl PartialEq for AliveContractInfo { - fn eq(&self, other: &Self) -> bool { - self.trie_id == other.trie_id - && self.storage_size == other.storage_size - && self.rent_allowance == other.rent_allowance - && self.code_hash == other.code_hash - && self.deduct_block == other.deduct_block - } + pub code_hash: CodeHash, + pub rent_allowance: Balance, + pub deduct_block: BlockNumber, } #[derive(Encode, Decode)] diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index 672efdc317a6e..bbea0e973f765 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -199,7 +199,7 @@ impl ExtBuilder { ); t.extend( GenesisConfig:: { - extrinsic_claim_delay: 2, + signed_claim_handicap: 2, rent_byte_price: 4, rent_deposit_offset: 1000, storage_size_offset: 8, From f640622779bb2df1825be8945f60c97ffc086973 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 11 Apr 2019 18:10:36 +0200 Subject: [PATCH 27/54] arithmetic operation --- srml/contract/src/rent.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index b13b581e02a7b..8ece8e080bc09 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -1,5 +1,5 @@ use crate::{BalanceOf, ContractInfo, ContractInfoOf, Module, TombstoneContractInfo, Trait}; -use runtime_primitives::traits::{As, CheckedDiv, Saturating, Zero}; +use runtime_primitives::traits::{As, CheckedDiv, Saturating, Zero, Bounded}; use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; @@ -39,7 +39,8 @@ fn try_evict_or_and_pay_rent( let effective_storage_size = >::sa(contract.storage_size).saturating_sub(free_storage); - let fee_per_block: BalanceOf = effective_storage_size * >::rent_byte_price(); + let fee_per_block: BalanceOf = effective_storage_size.checked_mul(>::rent_byte_price()) + .unwrap_or(BalanceOf::max_value()); if fee_per_block.is_zero() { // The rent deposit offset reduced the fee to 0. This means that the contract @@ -48,21 +49,21 @@ fn try_evict_or_and_pay_rent( } let blocks_to_rent = block_number.saturating_sub(contract.deduct_block); - let rent = fee_per_block * >::sa(blocks_to_rent.as_()); + let rent = fee_per_block.checked_mul(>::sa(blocks_to_rent.as_())) + .unwrap_or(BalanceOf::max_value()); let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); let rent_limited = rent.min(contract.rent_allowance); let rent_allowance_exceeded = rent > contract.rent_allowance; - let go_below_subsistence = balance.saturating_sub(rent_limited) >= subsistence_threshold; let is_below_subsistence = balance >= subsistence_threshold; + let go_below_subsistence = balance.saturating_sub(rent_limited) >= subsistence_threshold; let can_withdraw_rent = T::Currency::ensure_can_withdraw( account, rent_limited, WithdrawReason::Fee, balance.saturating_sub(rent_limited), - ) - .is_ok(); + ).is_ok(); if !rent_allowance_exceeded && can_withdraw_rent && !go_below_subsistence { // Collect dues From 1e6206b2f5ff12e8b48d061d54e0e4b2b6597988 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 11 Apr 2019 18:25:53 +0200 Subject: [PATCH 28/54] fix --- srml/contract/src/rent.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 8ece8e080bc09..28043edb046ef 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -56,7 +56,7 @@ fn try_evict_or_and_pay_rent( let rent_limited = rent.min(contract.rent_allowance); let rent_allowance_exceeded = rent > contract.rent_allowance; - let is_below_subsistence = balance >= subsistence_threshold; + let is_below_subsistence = balance < subsistence_threshold; let go_below_subsistence = balance.saturating_sub(rent_limited) >= subsistence_threshold; let can_withdraw_rent = T::Currency::ensure_can_withdraw( account, From ab4ca5b2d61cd470b6ecf91f2cc26a69097f8efc Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 12 Apr 2019 10:21:40 +0200 Subject: [PATCH 29/54] fix storage root + checked_mul + test --- srml/contract/src/lib.rs | 7 ++++--- srml/contract/src/rent.rs | 17 ++++++++++------- srml/contract/src/tests.rs | 6 +++--- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index bdfacb828010c..1defd3b3145af 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -176,9 +176,10 @@ pub struct RawAliveContractInfo { pub struct TombstoneContractInfo(T::Hash); impl TombstoneContractInfo { - fn new(storage_root: Vec, storage_size: u64, code_hash: CodeHash) -> Self { - let mut buf = storage_root; - buf.extend_from_slice(&storage_size.to_le_bytes()); + fn new(storage_root: Option>, storage_size: u64, code_hash: CodeHash) -> Self { + let mut buf = Vec::new(); + storage_root.using_encoded(|encoded| buf.extend_from_slice(encoded)); + storage_size.using_encoded(|encoded| buf.extend_from_slice(encoded)); buf.extend_from_slice(code_hash.as_ref()); TombstoneContractInfo(T::Hashing::hash(&buf[..])) } diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 28043edb046ef..9956a83c2377c 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -1,5 +1,5 @@ use crate::{BalanceOf, ContractInfo, ContractInfoOf, Module, TombstoneContractInfo, Trait}; -use runtime_primitives::traits::{As, CheckedDiv, Saturating, Zero, Bounded}; +use runtime_primitives::traits::{As, CheckedDiv, Saturating, Zero, Bounded, CheckedMul}; use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; @@ -39,8 +39,8 @@ fn try_evict_or_and_pay_rent( let effective_storage_size = >::sa(contract.storage_size).saturating_sub(free_storage); - let fee_per_block: BalanceOf = effective_storage_size.checked_mul(>::rent_byte_price()) - .unwrap_or(BalanceOf::max_value()); + let fee_per_block = effective_storage_size.checked_mul(&>::rent_byte_price()) + .unwrap_or(>::max_value()); if fee_per_block.is_zero() { // The rent deposit offset reduced the fee to 0. This means that the contract @@ -49,8 +49,8 @@ fn try_evict_or_and_pay_rent( } let blocks_to_rent = block_number.saturating_sub(contract.deduct_block); - let rent = fee_per_block.checked_mul(>::sa(blocks_to_rent.as_())) - .unwrap_or(BalanceOf::max_value()); + let rent = fee_per_block.checked_mul(&>::sa(blocks_to_rent.as_())) + .unwrap_or(>::max_value()); let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); let rent_limited = rent.min(contract.rent_allowance); @@ -108,9 +108,12 @@ fn try_evict_or_and_pay_rent( } if !is_below_subsistence { + // Note: this operation is heavy. + // Note: if the storage hasn't been touched then it returns None, which is OK. + let child_storage_root = runtime_io::child_storage_root(&contract.trie_id); + let tombstone = TombstoneContractInfo::new( - // Note: this operation is heavy - runtime_io::child_storage_root(&contract.trie_id).unwrap(), // TODO TODO: this unwrap ? + child_storage_root, contract.storage_size, contract.code_hash, ); diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index bbea0e973f765..d5d486b93ec1d 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -34,7 +34,7 @@ use assert_matches::assert_matches; use crate::{ ContractAddressFor, GenesisConfig, Module, RawEvent, Trait, ComputeDispatchFee, TrieIdGenerator, TrieId, - ContractInfoOf, ContractInfo, AliveContractInfo, + ContractInfoOf, ContractInfo, RawAliveContractInfo, }; use substrate_primitives::storage::well_known_keys; use parity_codec::{Encode, Decode, KeyedVec}; @@ -252,7 +252,7 @@ fn account_removal_removes_storage() { // Set up two accounts with free balance above the existential threshold. { Balances::deposit_creating(&1, 110); - ContractInfoOf::::insert(1, &ContractInfo::Alive(AliveContractInfo { + ContractInfoOf::::insert(1, &ContractInfo::Alive(RawAliveContractInfo { trie_id: unique_id1.to_vec(), storage_size: Contract::storage_size_offset(), deduct_block: System::block_number(), @@ -264,7 +264,7 @@ fn account_removal_removes_storage() { child::put(&unique_id1[..], &b"bar".to_vec(), &b"2".to_vec()); Balances::deposit_creating(&2, 110); - ContractInfoOf::::insert(2, &ContractInfo::Alive(AliveContractInfo { + ContractInfoOf::::insert(2, &ContractInfo::Alive(RawAliveContractInfo { trie_id: unique_id2.to_vec(), storage_size: Contract::storage_size_offset(), deduct_block: System::block_number(), From 14e3756ca52f3ba879e5f997ea5d6b9183651be9 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 12 Apr 2019 11:26:35 +0200 Subject: [PATCH 30/54] WIP: test --- srml/contract/src/tests.rs | 177 +++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index d5d486b93ec1d..addf9fc7d881b 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -502,3 +502,180 @@ fn dispatch_call() { }, ); } + +/// Call function is compiled with https://webassembly.studio/: +/// ```C +/// #define WASM_EXPORT __attribute__((visibility("default"))) +/// +/// extern void input_copy(int *ptr, int offset, int len); +/// extern void set_storage(int *key, int r, int *v, int len); +/// +/// extern int a; +/// WASM_EXPORT +/// int main() { +/// int do_set_storage, key, value_non_null, value_len; +/// input_copy(&do_set_storage, 0, 4); +/// input_copy(&key, 4, 4); +/// input_copy(&value_non_null, 8, 4); +/// input_copy(&value_len, 12, 4); +/// +/// if (do_set_storage) { +/// set_storage(&key, value_non_null, 0, value_len); +/// } +/// } +/// ``` +const CODE_SET_RENT: &str = r#" +(module + (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32))) + (import "env" "ext_set_rent_allowance" (func $ext_set_rent_allowance (param i32 i32))) + (import "env" "ext_input_size" (func $ext_input_size (result i32))) + (import "env" "ext_input_copy" (func $ext_input_copy (param i32 i32 i32))) + (import "env" "memory" (memory 1 1)) + + (func $assert (param i32) + (block $ok + (br_if $ok + (get_local 0) + ) + (unreachable) + ) + ) + + (start $start) + (func $start + (local $input_size i32) + (call $ext_set_storage + (i32.const 0) + (i32.const 1) + (i32.const 0) + (i32.const 4) + ) + (set_local $input_size + (call $ext_input_size) + ) + (call $ext_input_copy + (i32.const 0) + (i32.const 0) + (get_local $input_size) + ) + (call $ext_set_rent_allowance + (i32.const 0) + (get_local $input_size) + ) + ) + (func (export "call") + (call $ext_input_copy + (i32.const 0) + (i32.const 0) + (i32.const 16) + ) + (call $ext_set_storage + (i32.const 0) + (i32.load (4) + ($p2) + ($p3) + ) + ) + (func (export "deploy")) + (data (i32.const 0) "\00\01\02\03\04\05\06\07") +) +"#; +const HASH_SET_RENT: [u8; 32] = hex!("2bf4122e304b9bcc98dd691561149b9840de11b3557900829c9d1c9b6ae505d7"); + +#[test] +fn rent() { + let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); + + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + Balances::deposit_creating(&ALICE, 1_000_000); + + assert_ok!(Contract::put_code( + Origin::signed(ALICE), + 100_000, + wasm, + )); + + // Let's keep this assert even though it's redundant. If you ever need to update the + // wasm source this test will fail and will show you the actual hash. + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::CodeStored(HASH_SET_RENT.into())), + }, + ]); + + assert_ok!(Contract::create( + Origin::signed(ALICE), + 100, + 100_000, + HASH_SET_RENT.into(), + 10u64.to_le_bytes().to_vec(), + )); + + let bob_contract = super::ContractInfoOf::::get(BOB).unwrap() + .get_alive().unwrap(); + + assert_eq!(bob_contract.rent_allowance, 10); + assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); + + // assert_ok!(Contract::call( + // Origin::signed(ALICE), + // BOB, // newly created account + // 0, + // 100_000, + // vec![], + // )); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::CodeStored(HASH_SET_RENT.into())), + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances( + balances::RawEvent::NewAccount(BOB, 100) + ) + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)) + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)) + }, + + // // Dispatching the call. + // EventRecord { + // phase: Phase::ApplyExtrinsic(0), + // event: MetaEvent::balances( + // balances::RawEvent::NewAccount(CHARLIE, 50) + // ) + // }, + // EventRecord { + // phase: Phase::ApplyExtrinsic(0), + // event: MetaEvent::balances( + // balances::RawEvent::Transfer(BOB, CHARLIE, 50, 0) + // ) + // }, + + // // Event emited as a result of dispatch. + // EventRecord { + // phase: Phase::ApplyExtrinsic(0), + // event: MetaEvent::contract(RawEvent::Dispatched(BOB, true)) + // } + ]); + }, + ); +} From 867dc884e4ed6d6c0672012a8847d9f187627e3b Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 18 Apr 2019 16:20:06 +0200 Subject: [PATCH 31/54] WIP --- srml/contract/src/tests.rs | 177 +++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index 3105298fe2793..214007f33b55e 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -513,3 +513,180 @@ fn dispatch_call() { }, ); } + +/// Call function is compiled with https://webassembly.studio/ +/// ```C +/// #define WASM_EXPORT __attribute__((visibility("default"))) +/// +/// extern void input_copy(int *ptr, int offset, int len); +/// extern void set_storage(int *key, int r, int *v, int len); +/// +/// extern int a; +/// WASM_EXPORT +/// int main() { +/// int do_set_storage, key, value_non_null, value_len; +/// input_copy(&do_set_storage, 0, 4); +/// input_copy(&key, 4, 4); +/// input_copy(&value_non_null, 8, 4); +/// input_copy(&value_len, 12, 4); +/// +/// if (do_set_storage) { +/// set_storage(&key, value_non_null, 0, value_len); +/// } +/// } +/// ``` +const CODE_SET_RENT: &str = r#" +(module + (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32))) + (import "env" "ext_set_rent_allowance" (func $ext_set_rent_allowance (param i32 i32))) + (import "env" "ext_input_size" (func $ext_input_size (result i32))) + (import "env" "ext_input_copy" (func $ext_input_copy (param i32 i32 i32))) + (import "env" "memory" (memory 1 1)) + + (func $assert (param i32) + (block $ok + (br_if $ok + (get_local 0) + ) + (unreachable) + ) + ) + + (start $start) + (func $start + (local $input_size i32) + (call $ext_set_storage + (i32.const 0) + (i32.const 1) + (i32.const 0) + (i32.const 4) + ) + (set_local $input_size + (call $ext_input_size) + ) + (call $ext_input_copy + (i32.const 0) + (i32.const 0) + (get_local $input_size) + ) + (call $ext_set_rent_allowance + (i32.const 0) + (get_local $input_size) + ) + ) + (func (export "call") + (call $ext_input_copy + (i32.const 0) + (i32.const 0) + (i32.const 16) + ) + (call $ext_set_storage + (i32.const 0) + (i32.load (i32.const 4)) + ($p2) + ($p3) + ) + ) + (func (export "deploy")) + (data (i32.const 0) "\00\01\02\03\04\05\06\07") +) +"#; +const HASH_SET_RENT: [u8; 32] = hex!("2bf4122e304b9bcc98dd691561149b9840de11b3557900829c9d1c9b6ae505d7"); + +#[test] +fn rent() { + let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); + + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + Balances::deposit_creating(&ALICE, 1_000_000); + + assert_ok!(Contract::put_code( + Origin::signed(ALICE), + 100_000, + wasm, + )); + + // Let's keep this assert even though it's redundant. If you ever need to update the + // wasm source this test will fail and will show you the actual hash. + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::CodeStored(HASH_SET_RENT.into())), + }, + ]); + + assert_ok!(Contract::create( + Origin::signed(ALICE), + 100, + 100_000, + HASH_SET_RENT.into(), + 10u64.to_le_bytes().to_vec(), + )); + + let bob_contract = super::ContractInfoOf::::get(BOB).unwrap() + .get_alive().unwrap(); + + assert_eq!(bob_contract.rent_allowance, 10); + assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); + + // assert_ok!(Contract::call( + // Origin::signed(ALICE), + // BOB, // newly created account + // 0, + // 100_000, + // vec![], + // )); + + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::CodeStored(HASH_SET_RENT.into())), + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances( + balances::RawEvent::NewAccount(BOB, 100) + ) + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)) + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)) + }, + + // // Dispatching the call. + // EventRecord { + // phase: Phase::ApplyExtrinsic(0), + // event: MetaEvent::balances( + // balances::RawEvent::NewAccount(CHARLIE, 50) + // ) + // }, + // EventRecord { + // phase: Phase::ApplyExtrinsic(0), + // event: MetaEvent::balances( + // balances::RawEvent::Transfer(BOB, CHARLIE, 50, 0) + // ) + // }, + + // // Event emited as a result of dispatch. + // EventRecord { + // phase: Phase::ApplyExtrinsic(0), + // event: MetaEvent::contract(RawEvent::Dispatched(BOB, true)) + // } + ]); + }, + ); +} From 8b8159691c3e431a14a5e661909b617c2ddef5b2 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 25 Apr 2019 12:11:56 +0200 Subject: [PATCH 32/54] add tests and fix --- srml/contract/src/account_db.rs | 2 +- srml/contract/src/lib.rs | 27 ++- srml/contract/src/rent.rs | 2 +- srml/contract/src/tests.rs | 396 +++++++++++++++++++++++--------- 4 files changed, 313 insertions(+), 114 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 02a7522fc1833..0fc60b48eb7a6 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -150,7 +150,7 @@ impl AccountDb for DirectAccountDb { } if old_info - .map(|old_info| old_info == new_info) + .map(|old_info| old_info != new_info) .unwrap_or(true) { >::insert(&address, ContractInfo::Alive(new_info)); diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 1d87726a9c32a..146d8b6fd0ec0 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -151,6 +151,31 @@ impl ContractInfo { None } } + + /// If contract is tombstone then return some alive info + pub fn get_tombstone(self) -> Option> { + if let ContractInfo::Tombstone(tombstone) = self { + Some(tombstone) + } else { + None + } + } + /// If contract is tombstone then return some reference to tombstone info + pub fn as_tombstone(&self) -> Option<&TombstoneContractInfo> { + if let ContractInfo::Tombstone(ref tombstone) = self { + Some(tombstone) + } else { + None + } + } + /// If contract is tombstone then return some mutable reference to tombstone info + pub fn as_tombstone_mut(&mut self) -> Option<&mut TombstoneContractInfo> { + if let ContractInfo::Tombstone(ref mut tombstone) = self { + Some(tombstone) + } else { + None + } + } } pub type AliveContractInfo = RawAliveContractInfo, BalanceOf, ::BlockNumber>; @@ -445,7 +470,7 @@ decl_module! { (true, account) }, Some(system::RawOrigin::Inherent) if aux_sender.is_some() => { - (true, aux_sender.as_ref().expect("checked above")) + (false, aux_sender.as_ref().expect("checked above")) }, _ => return Err("Invalid surcharge claim: origin must be signed or \ inherent and auxiliary sender only provided on inherent") diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 9956a83c2377c..0501bdc92b754 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -57,7 +57,7 @@ fn try_evict_or_and_pay_rent( let rent_allowance_exceeded = rent > contract.rent_allowance; let is_below_subsistence = balance < subsistence_threshold; - let go_below_subsistence = balance.saturating_sub(rent_limited) >= subsistence_threshold; + let go_below_subsistence = balance.saturating_sub(rent_limited) < subsistence_threshold; let can_withdraw_rent = T::Currency::ensure_can_withdraw( account, rent_limited, diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index 214007f33b55e..356b46a68ba93 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -21,7 +21,7 @@ use runtime_io::with_externalities; use runtime_primitives::testing::{Digest, DigestItem, H256, Header, UintAuthorityId}; -use runtime_primitives::traits::{BlakeTwo256, IdentityLookup}; +use runtime_primitives::traits::{BlakeTwo256, IdentityLookup, As}; use runtime_primitives::BuildStorage; use runtime_io; use srml_support::{storage::child, StorageMap, assert_ok, impl_outer_event, impl_outer_dispatch, @@ -203,12 +203,12 @@ impl ExtBuilder { GenesisConfig:: { signed_claim_handicap: 2, rent_byte_price: 4, - rent_deposit_offset: 1000, + rent_deposit_offset: 10_000, storage_size_offset: 8, surcharge_reward: 150, tombstone_deposit: 16, - transaction_base_fee: 0, - transaction_byte_fee: 0, + transaction_base_fee: 2, + transaction_byte_fee: 6, transfer_fee: self.transfer_fee, creation_fee: self.creation_fee, contract_fee: 21, @@ -297,7 +297,6 @@ fn account_removal_removes_storage() { assert!(>::get_storage(&DirectAccountDb, &1, Some(&trie_id1), key1).is_none()); assert!(>::get_storage(&DirectAccountDb, &1, Some(&trie_id1), key2).is_none()); - // TODO TODO: check size assert_eq!( >::get_storage(&DirectAccountDb, &2, Some(&trie_id2), key1), Some(b"3".to_vec()) @@ -514,35 +513,49 @@ fn dispatch_call() { ); } -/// Call function is compiled with https://webassembly.studio/ -/// ```C -/// #define WASM_EXPORT __attribute__((visibility("default"))) -/// -/// extern void input_copy(int *ptr, int offset, int len); -/// extern void set_storage(int *key, int r, int *v, int len); -/// -/// extern int a; -/// WASM_EXPORT -/// int main() { -/// int do_set_storage, key, value_non_null, value_len; -/// input_copy(&do_set_storage, 0, 4); -/// input_copy(&key, 4, 4); -/// input_copy(&value_non_null, 8, 4); -/// input_copy(&value_len, 12, 4); -/// -/// if (do_set_storage) { -/// set_storage(&key, value_non_null, 0, value_len); -/// } -/// } -/// ``` const CODE_SET_RENT: &str = r#" (module + (import "env" "ext_dispatch_call" (func $ext_dispatch_call (param i32 i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32))) (import "env" "ext_set_rent_allowance" (func $ext_set_rent_allowance (param i32 i32))) (import "env" "ext_input_size" (func $ext_input_size (result i32))) (import "env" "ext_input_copy" (func $ext_input_copy (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) + (func $call_0 + (call $ext_set_rent_allowance + (i32.const 0) + (i32.const 1) + ) + ) + + (func $call_1 + (call $ext_set_storage + (i32.const 1) + (i32.const 1) + (i32.const 0) + (i32.const 4) + ) + ) + + (func $call_2 + (call $ext_set_storage + (i32.const 1) + (i32.const 0) + (i32.const 0) + (i32.const 0) + ) + ) + + (func $call_3 + (call $ext_dispatch_call + (i32.const 8) + (i32.const 11) + ) + ) + + (func $call_else) + (func $assert (param i32) (block $ok (br_if $ok @@ -552,8 +565,58 @@ const CODE_SET_RENT: &str = r#" ) ) - (start $start) - (func $start + (func (export "call") + (local $input_size i32) + (set_local $input_size + (call $ext_input_size) + ) + (block $IF_3 + (block $IF_2 + (block $IF_1 + (block $IF_0 + (br_if $IF_0 + (i32.eq + (get_local $input_size) + (i32.const 0) + ) + ) + (br_if $IF_1 + (i32.eq + (get_local $input_size) + (i32.const 1) + ) + ) + (br_if $IF_2 + (i32.eq + (get_local $input_size) + (i32.const 2) + ) + ) + (br_if $IF_3 + (i32.eq + (get_local $input_size) + (i32.const 3) + ) + ) + (call $call_else) + return + (unreachable) + ) + (call $call_0) + return + (unreachable) + ) + (call $call_1) + return + (unreachable) + ) + (call $call_2) + return + (unreachable) + ) + (call $call_3) + ) + (func (export "deploy") (local $input_size i32) (call $ext_set_storage (i32.const 0) @@ -574,42 +637,34 @@ const CODE_SET_RENT: &str = r#" (get_local $input_size) ) ) - (func (export "call") - (call $ext_input_copy - (i32.const 0) - (i32.const 0) - (i32.const 16) - ) - (call $ext_set_storage - (i32.const 0) - (i32.load (i32.const 4)) - ($p2) - ($p3) - ) - ) - (func (export "deploy")) - (data (i32.const 0) "\00\01\02\03\04\05\06\07") + (data (i32.const 0) "\28\01\02\03\04\05\06\07") + (data (i32.const 8) "\00\00\03\00\00\00\00\00\00\00\C8") ) "#; -const HASH_SET_RENT: [u8; 32] = hex!("2bf4122e304b9bcc98dd691561149b9840de11b3557900829c9d1c9b6ae505d7"); +const HASH_SET_RENT: [u8; 32] = hex!("88d2f5e0adae6f4e70e456e6509970325511c9933ae79165fd3faa50096db124"); + +fn call_set_rent_allowance_to_10() -> Vec { vec![] } +fn call_set_storage_4_byte() -> Vec { vec![0] } +fn call_remove_storage_4_byte() -> Vec { vec![0, 0] } +fn call_transfer() -> Vec { vec![0, 0, 0] } +fn call_null() -> Vec { vec![0, 0, 0, 0] } #[test] fn rent() { + // This test can fail due to the encoding changes. In case it becomes too annoying + // let's rewrite so as we use this module controlled call or we serialize it in runtime. + let encoded = parity_codec::Encode::encode(&Call::Balances(balances::Call::transfer(CHARLIE, 50))); + assert_eq!(&encoded[..], &hex!("00000300000000000000C8")[..]); + let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); + // Let's keep this assert even though it's redundant. If you ever need to update the + // wasm source this test will fail and will show you the actual hash. with_externalities( &mut ExtBuilder::default().existential_deposit(50).build(), || { Balances::deposit_creating(&ALICE, 1_000_000); - - assert_ok!(Contract::put_code( - Origin::signed(ALICE), - 100_000, - wasm, - )); - - // Let's keep this assert even though it's redundant. If you ever need to update the - // wasm source this test will fail and will show you the actual hash. + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), @@ -620,73 +675,192 @@ fn rent() { event: MetaEvent::contract(RawEvent::CodeStored(HASH_SET_RENT.into())), }, ]); + } + ); + // Storage size + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); assert_ok!(Contract::create( Origin::signed(ALICE), - 100, - 100_000, - HASH_SET_RENT.into(), - 10u64.to_le_bytes().to_vec(), + 30_000, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(1_000u64).encode() // rent allowance )); + let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); + assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); - let bob_contract = super::ContractInfoOf::::get(BOB).unwrap() - .get_alive().unwrap(); + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_set_storage_4_byte())); + let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); + assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4 + 4); - assert_eq!(bob_contract.rent_allowance, 10); + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_remove_storage_4_byte())); + let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); + } + ); - // assert_ok!(Contract::call( - // Origin::signed(ALICE), - // BOB, // newly created account - // 0, - // 100_000, - // vec![], - // )); + // Deducts between blocks + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::create( + Origin::signed(ALICE), + 30_000, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(1_000u64).encode() // rent allowance + )); - assert_eq!(System::events(), vec![ - EventRecord { - phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), - }, - EventRecord { - phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::CodeStored(HASH_SET_RENT.into())), - }, - EventRecord { - phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::balances( - balances::RawEvent::NewAccount(BOB, 100) - ) - }, - EventRecord { - phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)) - }, - EventRecord { - phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)) - }, + // Check creation + let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); + // assert_eq!(bob_contract.rent_allowance, 1000); + // assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); - // // Dispatching the call. - // EventRecord { - // phase: Phase::ApplyExtrinsic(0), - // event: MetaEvent::balances( - // balances::RawEvent::NewAccount(CHARLIE, 50) - // ) - // }, - // EventRecord { - // phase: Phase::ApplyExtrinsic(0), - // event: MetaEvent::balances( - // balances::RawEvent::Transfer(BOB, CHARLIE, 50, 0) - // ) - // }, - - // // Event emited as a result of dispatch. - // EventRecord { - // phase: Phase::ApplyExtrinsic(0), - // event: MetaEvent::contract(RawEvent::Dispatched(BOB, true)) - // } - ]); - }, + // Advance 4 blocks + System::initialize(&5, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent through call + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_null())); + + // Check result + let rent = (8 + 4 - 3) // storage size = size_offset + deploy_set_storage - deposit_offset + * 4 // rent byte price + * 4; // blocks to rent + let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); + assert_eq!(bob_contract.rent_allowance, 1_000 - rent); + assert_eq!(Balances::free_balance(BOB), 30_000 - rent); + } + ); + + // Test all kind of removals for each way. + removals(|| Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_null()).is_ok()); + removals(|| Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok()); + removals(|| Contract::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok()); + + // Test surcharge malus for inherent + claim_surcharge(4, || Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok(), true); + claim_surcharge(3, || Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok(), true); + claim_surcharge(2, || Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok(), true); + claim_surcharge(1, || Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok(), false); + + // Test surcharge malus for signed + claim_surcharge(4, || Contract::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok(), true); + claim_surcharge(3, || Contract::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok(), false); + claim_surcharge(2, || Contract::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok(), false); + claim_surcharge(1, || Contract::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok(), false); +} + +fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) { + let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); + + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::create( + Origin::signed(ALICE), + 100, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(1_000u64).encode() // rent allowance + )); + + // Advance blocks + System::initialize(&blocks, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent through call + assert!(trigger_call()); + + if removes { + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + } else { + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().is_some()); + } + } + ); +} + +fn removals(trigger_call: impl Fn() -> bool) { + let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); + + // Remove if balance reached + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::create( + Origin::signed(ALICE), + 100, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(1_000u64).encode() // rent allowance + )); + + // Advance blocks + System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent through call + assert!(trigger_call()); + + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + } + ); + + // Remove if allowance exceeded + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::create( + Origin::signed(ALICE), + 1_000, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(100u64).encode() // rent allowance + )); + + // Advance blocks + System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent through call + assert!(trigger_call()); + + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + } + ); + + // Remove without tombstone + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::create( + Origin::signed(ALICE), + 50, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(1_000u64).encode() // rent allowance + )); + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_transfer())); + + // Advance blocks + System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent through call + assert!(trigger_call()); + + assert!(super::ContractInfoOf::::get(BOB).is_none()); + } ); } From d0fe071915a624aa812b87fa7185b761a5f92fa3 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 25 Apr 2019 12:19:57 +0200 Subject: [PATCH 33/54] fmt --- srml/contract/src/rent.rs | 8 ++++-- srml/contract/src/tests.rs | 56 ++++++++++++++------------------------ 2 files changed, 26 insertions(+), 38 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 0501bdc92b754..5ecd181db868a 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -1,5 +1,5 @@ use crate::{BalanceOf, ContractInfo, ContractInfoOf, Module, TombstoneContractInfo, Trait}; -use runtime_primitives::traits::{As, CheckedDiv, Saturating, Zero, Bounded, CheckedMul}; +use runtime_primitives::traits::{As, Bounded, CheckedDiv, CheckedMul, Saturating, Zero}; use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; @@ -39,7 +39,8 @@ fn try_evict_or_and_pay_rent( let effective_storage_size = >::sa(contract.storage_size).saturating_sub(free_storage); - let fee_per_block = effective_storage_size.checked_mul(&>::rent_byte_price()) + let fee_per_block = effective_storage_size + .checked_mul(&>::rent_byte_price()) .unwrap_or(>::max_value()); if fee_per_block.is_zero() { @@ -49,7 +50,8 @@ fn try_evict_or_and_pay_rent( } let blocks_to_rent = block_number.saturating_sub(contract.deduct_block); - let rent = fee_per_block.checked_mul(&>::sa(blocks_to_rent.as_())) + let rent = fee_per_block + .checked_mul(&>::sa(blocks_to_rent.as_())) .unwrap_or(>::max_value()); let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index 356b46a68ba93..f5f6d20158c42 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -19,28 +19,28 @@ #![allow(unused)] +use crate::account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}; +use crate::{ + ComputeDispatchFee, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, Module, + RawAliveContractInfo, RawEvent, Trait, TrieId, TrieIdFromParentCounter, TrieIdGenerator, +}; +use assert_matches::assert_matches; +use hex_literal::*; +use parity_codec::{Decode, Encode, KeyedVec}; +use runtime_io; use runtime_io::with_externalities; -use runtime_primitives::testing::{Digest, DigestItem, H256, Header, UintAuthorityId}; -use runtime_primitives::traits::{BlakeTwo256, IdentityLookup, As}; +use runtime_primitives::testing::{Digest, DigestItem, Header, UintAuthorityId, H256}; +use runtime_primitives::traits::{As, BlakeTwo256, IdentityLookup}; use runtime_primitives::BuildStorage; -use runtime_io; -use srml_support::{storage::child, StorageMap, assert_ok, impl_outer_event, impl_outer_dispatch, - impl_outer_origin, traits::Currency}; -use substrate_primitives::Blake2Hasher; -use system::{self, Phase, EventRecord}; -use {wabt, balances, consensus}; -use hex_literal::*; -use assert_matches::assert_matches; -use crate::{ - ContractAddressFor, GenesisConfig, Module, RawEvent, - Trait, ComputeDispatchFee, TrieIdGenerator, TrieId, - ContractInfoOf, ContractInfo, RawAliveContractInfo, - TrieIdFromParentCounter, +use srml_support::{ + assert_ok, impl_outer_dispatch, impl_outer_event, impl_outer_origin, storage::child, + traits::Currency, StorageMap, }; -use substrate_primitives::storage::well_known_keys; -use parity_codec::{Encode, Decode, KeyedVec}; use std::sync::atomic::{AtomicUsize, Ordering}; -use crate::account_db::{DirectAccountDb, OverlayAccountDb, AccountDb}; +use substrate_primitives::storage::well_known_keys; +use substrate_primitives::Blake2Hasher; +use system::{self, EventRecord, Phase}; +use {balances, consensus, wabt}; mod contract { // Re-export contents of the root. This basically @@ -232,13 +232,7 @@ fn refunds_unused_gas() { with_externalities(&mut ExtBuilder::default().build(), || { Balances::deposit_creating(&0, 100_000_000); - assert_ok!(Contract::call( - Origin::signed(0), - 1, - 0, - 100_000, - Vec::new() - )); + assert_ok!(Contract::call(Origin::signed(0), 1, 0, 100_000, Vec::new())); assert_eq!(Balances::free_balance(&0), 100_000_000 - (2 * 135)); }); @@ -348,11 +342,7 @@ fn instantiate_and_call_and_deposit_event() { || { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code( - Origin::signed(ALICE), - 100_000, - wasm, - )); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm)); // Check at the end to get hash on error easily let creation = Contract::create( @@ -430,11 +420,7 @@ fn dispatch_call() { || { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code( - Origin::signed(ALICE), - 100_000, - wasm, - )); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm)); // Let's keep this assert even though it's redundant. If you ever need to update the // wasm source this test will fail and will show you the actual hash. From 1659fb012ccc86b0bfa52532a7a38b4deb9687d1 Mon Sep 17 00:00:00 2001 From: Sergei Pepyakin Date: Fri, 26 Apr 2019 18:37:08 +0200 Subject: [PATCH 34/54] typo and doc suggestions Co-Authored-By: thiolliere --- srml/contract/COMPLEXITY.md | 2 +- srml/contract/src/exec.rs | 2 +- srml/contract/src/rent.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/srml/contract/COMPLEXITY.md b/srml/contract/COMPLEXITY.md index c03d407f399fc..3cd7fee448204 100644 --- a/srml/contract/COMPLEXITY.md +++ b/srml/contract/COMPLEXITY.md @@ -107,7 +107,7 @@ While these functions only modify the local `Map`, if changes made by them are c ## create_contract -calls contract_exists and if not modify the local `Map` similarly to set_rent_allowance. +Calls `contract_exists` and if it doesn't exist, do not modify the local `Map` similarly to `set_rent_allowance`. **complexity**: The computational complexity is proportional to the depth of the overlay cascade and the size of the value; the cost is dominated by the DB read though. No additional memory is required. diff --git a/srml/contract/src/exec.rs b/srml/contract/src/exec.rs index 51d3d320dd9f6..980fd077e9a97 100644 --- a/srml/contract/src/exec.rs +++ b/srml/contract/src/exec.rs @@ -306,7 +306,7 @@ where // Assumption: pay_rent doesn't collide with overlay because // pay_rent will be done on first call and dest contract and balance - // cannot be change before first call + // cannot be changed before the first call crate::rent::pay_rent::(&dest); let mut output_data = Vec::new(); diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 5ecd181db868a..6a2a231a8529c 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -3,7 +3,7 @@ use runtime_primitives::traits::{As, Bounded, CheckedDiv, CheckedMul, Saturating use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; -/// Evict and optionnaly pay rent, at block number. Return if evicted +/// Evict and optionally pay rent, at block number. Return if evicted /// /// Exempted from rent iff: /// * rent is offset completely by the `rent_deposit_offset`, @@ -79,8 +79,8 @@ fn try_evict_or_and_pay_rent( ) .expect( "Withdraw has been checked above; - go_below_subsistence is false and subsistence > existencial_deposit; - qed", + go_below_subsistence is false and subsistence > existencial_deposit; + qed", ); >::mutate(account, |contract| { From d6f5d8c599efdcc323fe8aa8cdae1d7a28a6e21e Mon Sep 17 00:00:00 2001 From: thiolliere Date: Sun, 28 Apr 2019 12:44:48 +0200 Subject: [PATCH 35/54] WIP --- srml/contract/src/account_db.rs | 1 + srml/contract/src/lib.rs | 1 + srml/contract/src/rent.rs | 27 +++- srml/contract/src/tests.rs | 220 +++++++++++++++++++++----------- 4 files changed, 174 insertions(+), 75 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 0fc60b48eb7a6..7d2abb78a66c1 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -117,6 +117,7 @@ impl AccountDb for DirectAccountDb { let mut new_info = if let Some(info) = old_info.clone() { info } else if let Some(code_hash) = changed.code_hash { + println!("set : {}", >::block_number()); AliveContractInfo:: { code_hash, storage_size: >::storage_size_offset(), diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 92c0d1b6b362f..39a9896193a68 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -489,6 +489,7 @@ decl_module! { if signed { check_block = check_block.saturating_sub(>::signed_claim_handicap()); } + println!("claim for: {}", check_block); if rent::try_evict_at::(&dest, check_block) { T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 6a2a231a8529c..28daeed787966 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -1,9 +1,26 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + use crate::{BalanceOf, ContractInfo, ContractInfoOf, Module, TombstoneContractInfo, Trait}; use runtime_primitives::traits::{As, Bounded, CheckedDiv, CheckedMul, Saturating, Zero}; use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; -/// Evict and optionally pay rent, at block number. Return if evicted +/// Evict and optionally pay dues (or check he could pay them otherwise), at given block number. +/// Return true iff the account has been evicted. /// /// Exempted from rent iff: /// * rent is offset completely by the `rent_deposit_offset`, @@ -25,6 +42,7 @@ fn try_evict_or_and_pay_rent( Some(ContractInfo::Alive(contract)) => contract, }; + println!("rent : {} {}", contract.deduct_block, block_number); // Rent has already been paid if contract.deduct_block >= block_number { return false; @@ -128,11 +146,16 @@ fn try_evict_or_and_pay_rent( } /// Make account paying the rent for the current block number +/// +/// Call try_evict_or_and_pay_rent with setting pay rent to true pub fn pay_rent(account: &T::AccountId) { try_evict_or_and_pay_rent::(account, >::block_number(), true); } -/// Make account paying the rent for the current block number +/// Evict the account if he should be evicted at the given block number. +/// Return true iff the account has been evicted. +/// +/// Call try_evict_or_and_pay_rent with setting pay rent to false pub fn try_evict_at(account: &T::AccountId, block: T::BlockNumber) -> bool { try_evict_or_and_pay_rent::(account, block, false) } diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index a5f388100d517..d078d5717da3a 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -514,6 +514,7 @@ const CODE_SET_RENT: &str = r#" (import "env" "ext_input_copy" (func $ext_input_copy (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) + ;; set rent_allowance to 10 (func $call_0 (call $ext_set_rent_allowance (i32.const 0) @@ -521,6 +522,7 @@ const CODE_SET_RENT: &str = r#" ) ) + ;; insert a value of 4 bytes into storage (func $call_1 (call $ext_set_storage (i32.const 1) @@ -530,6 +532,7 @@ const CODE_SET_RENT: &str = r#" ) ) + ;; remove the value inserted by call_1 (func $call_2 (call $ext_set_storage (i32.const 1) @@ -539,6 +542,7 @@ const CODE_SET_RENT: &str = r#" ) ) + ;; transfer 50 to ALICE (func $call_3 (call $ext_dispatch_call (i32.const 8) @@ -546,6 +550,7 @@ const CODE_SET_RENT: &str = r#" ) ) + ;; do nothing (func $call_else) (func $assert (param i32) @@ -557,6 +562,7 @@ const CODE_SET_RENT: &str = r#" ) ) + ;; Dispatch the call according to input size (func (export "call") (local $input_size i32) (set_local $input_size @@ -592,22 +598,21 @@ const CODE_SET_RENT: &str = r#" ) (call $call_else) return - (unreachable) ) (call $call_0) return - (unreachable) ) (call $call_1) return - (unreachable) ) (call $call_2) return - (unreachable) ) (call $call_3) ) + + ;; Set into storage a 4 bytes value + ;; Set call set_rent_allowance with input (func (export "deploy") (local $input_size i32) (call $ext_set_storage @@ -629,20 +634,30 @@ const CODE_SET_RENT: &str = r#" (get_local $input_size) ) ) - (data (i32.const 0) "\28\01\02\03\04\05\06\07") + + ;; Encoding of 10 in balance + (data (i32.const 0) "\28") + + ;; Encoding of call transfer 50 to CHARLIE (data (i32.const 8) "\00\00\03\00\00\00\00\00\00\00\C8") ) "#; -const HASH_SET_RENT: [u8; 32] = hex!("88d2f5e0adae6f4e70e456e6509970325511c9933ae79165fd3faa50096db124"); +const HASH_SET_RENT: [u8; 32] = hex!("01f2ed6bf3136470b7c92bd29f2227c67cd788d705b09de2eb7ba926cdafb1e3"); -fn call_set_rent_allowance_to_10() -> Vec { vec![] } -fn call_set_storage_4_byte() -> Vec { vec![0] } -fn call_remove_storage_4_byte() -> Vec { vec![0, 0] } -fn call_transfer() -> Vec { vec![0, 0, 0] } -fn call_null() -> Vec { vec![0, 0, 0, 0] } +/// Input data for each call in set_rent code +mod call { + pub fn set_rent_allowance_to_10() -> Vec { vec![] } + pub fn set_storage_4_byte() -> Vec { vec![0] } + pub fn remove_storage_4_byte() -> Vec { vec![0, 0] } + pub fn transfer() -> Vec { vec![0, 0, 0] } + pub fn null() -> Vec { vec![0, 0, 0, 0] } +} + +/// Test correspondance of set_rent code and its hash. +/// Also test that encoded extrinsic in code correspond to the correct transfer #[test] -fn rent() { +fn set_rent_hash_and_code() { // This test can fail due to the encoding changes. In case it becomes too annoying // let's rewrite so as we use this module controlled call or we serialize it in runtime. let encoded = parity_codec::Encode::encode(&Call::Balances(balances::Call::transfer(CHARLIE, 50))); @@ -650,13 +665,13 @@ fn rent() { let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); - // Let's keep this assert even though it's redundant. If you ever need to update the - // wasm source this test will fail and will show you the actual hash. with_externalities( &mut ExtBuilder::default().existential_deposit(50).build(), || { Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm)); + + // If you ever need to update the wasm source this test will fail and will show you the actual hash. assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), @@ -669,6 +684,11 @@ fn rent() { ]); } ); +} + +#[test] +fn storage_size() { + let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); // Storage size with_externalities( @@ -676,7 +696,7 @@ fn rent() { || { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm)); assert_ok!(Contract::create( Origin::signed(ALICE), 30_000, @@ -686,23 +706,27 @@ fn rent() { let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); - assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_set_storage_4_byte())); + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::set_storage_4_byte())); let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4 + 4); - assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_remove_storage_4_byte())); + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::remove_storage_4_byte())); let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); } ); +} + +#[test] +fn deduct_blocks() { + let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); - // Deducts between blocks with_externalities( &mut ExtBuilder::default().existential_deposit(50).build(), || { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm)); assert_ok!(Contract::create( Origin::signed(ALICE), 30_000, @@ -712,14 +736,13 @@ fn rent() { // Check creation let bob_contract = super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap(); - // assert_eq!(bob_contract.rent_allowance, 1000); - // assert_eq!(bob_contract.storage_size, Contract::storage_size_offset() + 4); + assert_eq!(bob_contract.rent_allowance, 1_000); // Advance 4 blocks System::initialize(&5, &[0u8; 32].into(), &[0u8; 32].into()); // Trigger rent through call - assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_null())); + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); // Check result let rent = (8 + 4 - 3) // storage size = size_offset + deploy_set_storage - deposit_offset @@ -730,12 +753,25 @@ fn rent() { assert_eq!(Balances::free_balance(BOB), 30_000 - rent); } ); +} - // Test all kind of removals for each way. - removals(|| Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_null()).is_ok()); +#[test] +fn call_contract_removals() { + removals(|| Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()).is_ok()); +} + +#[test] +fn inherent_claim_surcharge_contract_removals() { removals(|| Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok()); +} + +#[test] +fn signed_claim_surcharge_contract_removals() { removals(|| Contract::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok()); +} +#[test] +fn claim_surcharge_malus() { // Test surcharge malus for inherent claim_surcharge(4, || Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok(), true); claim_surcharge(3, || Contract::claim_surcharge(Origin::INHERENT, BOB, Some(ALICE)).is_ok(), true); @@ -749,6 +785,8 @@ fn rent() { claim_surcharge(1, || Contract::claim_surcharge(Origin::signed(ALICE), BOB, None).is_ok(), false); } +/// Claim surcharge with the given trigger_call at the given blocks. +/// if removes is true then assert that the contract is a tombstonedead fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) { let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); @@ -757,7 +795,7 @@ fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) || { // Create Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm)); assert_ok!(Contract::create( Origin::signed(ALICE), 100, @@ -780,10 +818,82 @@ fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) ); } +/// Test for all kind of removals for the given trigger: +/// * if balance is reached and balance > subsistence threshold +/// * if allowance is exceeded +/// * if balance is reached and balance < subsistence threshold fn removals(trigger_call: impl Fn() -> bool) { let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); - // Remove if balance reached + // // Balance reached and superior to subsistence threshold + // with_externalities( + // &mut ExtBuilder::default().existential_deposit(50).build(), + // || { + // // Create + // Balances::deposit_creating(&ALICE, 1_000_000); + // assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + // assert_ok!(Contract::create( + // Origin::signed(ALICE), + // 100, + // 100_000, HASH_SET_RENT.into(), + // ::Balance::sa(1_000u64).encode() // rent allowance + // )); + + // // Trigger rent must have no effect + // assert!(trigger_call()); + // assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 1_000); + + // // Advance blocks + // System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + + // // Trigger rent through call + // assert!(trigger_call()); + // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + + // // Advance blocks + // System::initialize(&20, &[0u8; 32].into(), &[0u8; 32].into()); + + // // Trigger rent must have no effect + // assert!(trigger_call()); + // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + // } + // ); + + // // Allowance exceeded + // with_externalities( + // &mut ExtBuilder::default().existential_deposit(50).build(), + // || { + // // Create + // Balances::deposit_creating(&ALICE, 1_000_000); + // assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + // assert_ok!(Contract::create( + // Origin::signed(ALICE), + // 1_000, + // 100_000, HASH_SET_RENT.into(), + // ::Balance::sa(100u64).encode() // rent allowance + // )); + + // // Trigger rent must have no effect + // assert!(trigger_call()); + // assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 100); + + // // Advance blocks + // System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + + // // Trigger rent through call + // assert!(trigger_call()); + // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + + // // Advance blocks + // System::initialize(&20, &[0u8; 32].into(), &[0u8; 32].into()); + + // // Trigger rent must have no effect + // assert!(trigger_call()); + // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + // } + // ); + + // Balance reached and inferior to subsistence threshold with_externalities( &mut ExtBuilder::default().existential_deposit(50).build(), || { @@ -792,66 +902,30 @@ fn removals(trigger_call: impl Fn() -> bool) { assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); assert_ok!(Contract::create( Origin::signed(ALICE), - 100, + 50, 100_000, HASH_SET_RENT.into(), ::Balance::sa(1_000u64).encode() // rent allowance )); - // Advance blocks - System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); - - // Trigger rent through call + // Trigger rent must have no effect assert!(trigger_call()); + assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 1_000); - assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); - } - ); - - // Remove if allowance exceeded - with_externalities( - &mut ExtBuilder::default().existential_deposit(50).build(), - || { - // Create - Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); - assert_ok!(Contract::create( - Origin::signed(ALICE), - 1_000, - 100_000, HASH_SET_RENT.into(), - ::Balance::sa(100u64).encode() // rent allowance - )); + // Transfer funds + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::transfer())); - // Advance blocks - System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + // TODO TODO: why this doesn't fails ? + // TODO TODO: weird ????? // Trigger rent through call assert!(trigger_call()); - - assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); - } - ); - - // Remove without tombstone - with_externalities( - &mut ExtBuilder::default().existential_deposit(50).build(), - || { - // Create - Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); - assert_ok!(Contract::create( - Origin::signed(ALICE), - 50, - 100_000, HASH_SET_RENT.into(), - ::Balance::sa(1_000u64).encode() // rent allowance - )); - assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call_transfer())); + assert!(super::ContractInfoOf::::get(BOB).is_none()); // Advance blocks - System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + System::initialize(&20, &[0u8; 32].into(), &[0u8; 32].into()); - // Trigger rent through call + // Trigger rent must have no effect assert!(trigger_call()); - assert!(super::ContractInfoOf::::get(BOB).is_none()); } ); From 25fc7f023b174664079d2fdec999f3c1fcd61600 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Sun, 28 Apr 2019 13:09:56 +0200 Subject: [PATCH 36/54] address some comments divide tests + some docs --- srml/contract/src/account_db.rs | 1 - srml/contract/src/lib.rs | 1 - srml/contract/src/rent.rs | 5 +- srml/contract/src/tests.rs | 141 ++++++++++++++++---------------- 4 files changed, 74 insertions(+), 74 deletions(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 7d2abb78a66c1..0fc60b48eb7a6 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -117,7 +117,6 @@ impl AccountDb for DirectAccountDb { let mut new_info = if let Some(info) = old_info.clone() { info } else if let Some(code_hash) = changed.code_hash { - println!("set : {}", >::block_number()); AliveContractInfo:: { code_hash, storage_size: >::storage_size_offset(), diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 39a9896193a68..92c0d1b6b362f 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -489,7 +489,6 @@ decl_module! { if signed { check_block = check_block.saturating_sub(>::signed_claim_handicap()); } - println!("claim for: {}", check_block); if rent::try_evict_at::(&dest, check_block) { T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 28daeed787966..12747e129b43a 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -19,7 +19,7 @@ use runtime_primitives::traits::{As, Bounded, CheckedDiv, CheckedMul, Saturating use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; -/// Evict and optionally pay dues (or check he could pay them otherwise), at given block number. +/// Evict and optionally pay dues (or check account can pay them otherwise), for given block number. /// Return true iff the account has been evicted. /// /// Exempted from rent iff: @@ -32,6 +32,8 @@ use srml_support::StorageMap; /// * rent exceed rent allowance, /// * or can't withdraw the rent, /// * or go below subsistence threshold. +/// +/// This function acts eagerly, all modification are committed into storages. fn try_evict_or_and_pay_rent( account: &T::AccountId, block_number: T::BlockNumber, @@ -42,7 +44,6 @@ fn try_evict_or_and_pay_rent( Some(ContractInfo::Alive(contract)) => contract, }; - println!("rent : {} {}", contract.deduct_block, block_number); // Rent has already been paid if contract.deduct_block >= block_number { return false; diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index d078d5717da3a..33c4fb4b76d14 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -825,73 +825,73 @@ fn claim_surcharge(blocks: u64, trigger_call: impl Fn() -> bool, removes: bool) fn removals(trigger_call: impl Fn() -> bool) { let wasm = wabt::wat2wasm(CODE_SET_RENT).unwrap(); - // // Balance reached and superior to subsistence threshold - // with_externalities( - // &mut ExtBuilder::default().existential_deposit(50).build(), - // || { - // // Create - // Balances::deposit_creating(&ALICE, 1_000_000); - // assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); - // assert_ok!(Contract::create( - // Origin::signed(ALICE), - // 100, - // 100_000, HASH_SET_RENT.into(), - // ::Balance::sa(1_000u64).encode() // rent allowance - // )); - - // // Trigger rent must have no effect - // assert!(trigger_call()); - // assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 1_000); - - // // Advance blocks - // System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); - - // // Trigger rent through call - // assert!(trigger_call()); - // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); - - // // Advance blocks - // System::initialize(&20, &[0u8; 32].into(), &[0u8; 32].into()); - - // // Trigger rent must have no effect - // assert!(trigger_call()); - // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); - // } - // ); - - // // Allowance exceeded - // with_externalities( - // &mut ExtBuilder::default().existential_deposit(50).build(), - // || { - // // Create - // Balances::deposit_creating(&ALICE, 1_000_000); - // assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); - // assert_ok!(Contract::create( - // Origin::signed(ALICE), - // 1_000, - // 100_000, HASH_SET_RENT.into(), - // ::Balance::sa(100u64).encode() // rent allowance - // )); - - // // Trigger rent must have no effect - // assert!(trigger_call()); - // assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 100); - - // // Advance blocks - // System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); - - // // Trigger rent through call - // assert!(trigger_call()); - // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); - - // // Advance blocks - // System::initialize(&20, &[0u8; 32].into(), &[0u8; 32].into()); - - // // Trigger rent must have no effect - // assert!(trigger_call()); - // assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); - // } - // ); + // Balance reached and superior to subsistence threshold + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::create( + Origin::signed(ALICE), + 100, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(1_000u64).encode() // rent allowance + )); + + // Trigger rent must have no effect + assert!(trigger_call()); + assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 1_000); + + // Advance blocks + System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent through call + assert!(trigger_call()); + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + + // Advance blocks + System::initialize(&20, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent must have no effect + assert!(trigger_call()); + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + } + ); + + // Allowance exceeded + with_externalities( + &mut ExtBuilder::default().existential_deposit(50).build(), + || { + // Create + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); + assert_ok!(Contract::create( + Origin::signed(ALICE), + 1_000, + 100_000, HASH_SET_RENT.into(), + ::Balance::sa(100u64).encode() // rent allowance + )); + + // Trigger rent must have no effect + assert!(trigger_call()); + assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 100); + + // Advance blocks + System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent through call + assert!(trigger_call()); + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + + // Advance blocks + System::initialize(&20, &[0u8; 32].into(), &[0u8; 32].into()); + + // Trigger rent must have no effect + assert!(trigger_call()); + assert!(super::ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + } + ); // Balance reached and inferior to subsistence threshold with_externalities( @@ -902,7 +902,7 @@ fn removals(trigger_call: impl Fn() -> bool) { assert_ok!(Contract::put_code(Origin::signed(ALICE), 100_000, wasm.clone())); assert_ok!(Contract::create( Origin::signed(ALICE), - 50, + 50+Balances::minimum_balance(), 100_000, HASH_SET_RENT.into(), ::Balance::sa(1_000u64).encode() // rent allowance )); @@ -913,9 +913,10 @@ fn removals(trigger_call: impl Fn() -> bool) { // Transfer funds assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::transfer())); + assert_eq!(super::ContractInfoOf::::get(BOB).unwrap().get_alive().unwrap().rent_allowance, 1_000); - // TODO TODO: why this doesn't fails ? - // TODO TODO: weird ????? + // Advance blocks + System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into()); // Trigger rent through call assert!(trigger_call()); From 1651e0b9970ac8c192687265064416d466bf572f Mon Sep 17 00:00:00 2001 From: thiolliere Date: Sun, 28 Apr 2019 16:54:11 +0200 Subject: [PATCH 37/54] use br_table --- srml/contract/src/tests.rs | 44 +++++++++++--------------------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index 33c4fb4b76d14..b3885c3a761a6 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -568,47 +568,29 @@ const CODE_SET_RENT: &str = r#" (set_local $input_size (call $ext_input_size) ) - (block $IF_3 - (block $IF_2 - (block $IF_1 - (block $IF_0 - (br_if $IF_0 - (i32.eq + (block $IF_ELSE + (block $IF_3 + (block $IF_2 + (block $IF_1 + (block $IF_0 + (br_table $IF_0 $IF_1 $IF_2 $IF_3 $IF_ELSE (get_local $input_size) - (i32.const 0) ) + (unreachable) ) - (br_if $IF_1 - (i32.eq - (get_local $input_size) - (i32.const 1) - ) - ) - (br_if $IF_2 - (i32.eq - (get_local $input_size) - (i32.const 2) - ) - ) - (br_if $IF_3 - (i32.eq - (get_local $input_size) - (i32.const 3) - ) - ) - (call $call_else) + (call $call_0) return ) - (call $call_0) + (call $call_1) return ) - (call $call_1) + (call $call_2) return ) - (call $call_2) + (call $call_3) return ) - (call $call_3) + (call $call_else) ) ;; Set into storage a 4 bytes value @@ -642,7 +624,7 @@ const CODE_SET_RENT: &str = r#" (data (i32.const 8) "\00\00\03\00\00\00\00\00\00\00\C8") ) "#; -const HASH_SET_RENT: [u8; 32] = hex!("01f2ed6bf3136470b7c92bd29f2227c67cd788d705b09de2eb7ba926cdafb1e3"); +const HASH_SET_RENT: [u8; 32] = hex!("12b5abdb10d268e47ba06e5ce4def55c5b6361360b4611cc9a1f3357a0395419"); /// Input data for each call in set_rent code From 323fb6a53039e8b11c00e32458682c19eaed3520 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Sun, 28 Apr 2019 19:23:23 +0200 Subject: [PATCH 38/54] remove unused function --- srml/contract/src/tests.rs | 47 ++++++++++++++------------------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index b3885c3a761a6..e18a99b8327bb 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -514,16 +514,8 @@ const CODE_SET_RENT: &str = r#" (import "env" "ext_input_copy" (func $ext_input_copy (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) - ;; set rent_allowance to 10 - (func $call_0 - (call $ext_set_rent_allowance - (i32.const 0) - (i32.const 1) - ) - ) - ;; insert a value of 4 bytes into storage - (func $call_1 + (func $call_0 (call $ext_set_storage (i32.const 1) (i32.const 1) @@ -533,7 +525,7 @@ const CODE_SET_RENT: &str = r#" ) ;; remove the value inserted by call_1 - (func $call_2 + (func $call_1 (call $ext_set_storage (i32.const 1) (i32.const 0) @@ -543,7 +535,7 @@ const CODE_SET_RENT: &str = r#" ) ;; transfer 50 to ALICE - (func $call_3 + (func $call_2 (call $ext_dispatch_call (i32.const 8) (i32.const 11) @@ -569,25 +561,21 @@ const CODE_SET_RENT: &str = r#" (call $ext_input_size) ) (block $IF_ELSE - (block $IF_3 - (block $IF_2 - (block $IF_1 - (block $IF_0 - (br_table $IF_0 $IF_1 $IF_2 $IF_3 $IF_ELSE - (get_local $input_size) - ) - (unreachable) + (block $IF_2 + (block $IF_1 + (block $IF_0 + (br_table $IF_0 $IF_1 $IF_2 $IF_ELSE + (get_local $input_size) ) - (call $call_0) - return + (unreachable) ) - (call $call_1) + (call $call_0) return ) - (call $call_2) + (call $call_1) return ) - (call $call_3) + (call $call_2) return ) (call $call_else) @@ -624,16 +612,15 @@ const CODE_SET_RENT: &str = r#" (data (i32.const 8) "\00\00\03\00\00\00\00\00\00\00\C8") ) "#; -const HASH_SET_RENT: [u8; 32] = hex!("12b5abdb10d268e47ba06e5ce4def55c5b6361360b4611cc9a1f3357a0395419"); +const HASH_SET_RENT: [u8; 32] = hex!("a51c2a6f3f68936d4ae9abdb93b28eedcbd0f6f39770e168f9025f0c1e7094ef"); /// Input data for each call in set_rent code mod call { - pub fn set_rent_allowance_to_10() -> Vec { vec![] } - pub fn set_storage_4_byte() -> Vec { vec![0] } - pub fn remove_storage_4_byte() -> Vec { vec![0, 0] } - pub fn transfer() -> Vec { vec![0, 0, 0] } - pub fn null() -> Vec { vec![0, 0, 0, 0] } + pub fn set_storage_4_byte() -> Vec { vec![] } + pub fn remove_storage_4_byte() -> Vec { vec![0] } + pub fn transfer() -> Vec { vec![0, 0] } + pub fn null() -> Vec { vec![0, 0, 0] } } /// Test correspondance of set_rent code and its hash. From 3618ace43b075203298555ac0af793cce989a1ef Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 12:25:20 +0200 Subject: [PATCH 39/54] Bump the runtime version --- node/runtime/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 52bffacebd6d8..3ec297b17b7c8 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -58,8 +58,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("node"), impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, - spec_version: 66, - impl_version: 67, + spec_version: 67, + impl_version: 68, apis: RUNTIME_API_VERSIONS, }; From 59a4a17212d5c043acfe532475086bb8e81042c9 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 12:28:13 +0200 Subject: [PATCH 40/54] insert_with --- srml/contract/src/account_db.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/contract/src/account_db.rs b/srml/contract/src/account_db.rs index 0fc60b48eb7a6..912560c562c6c 100644 --- a/srml/contract/src/account_db.rs +++ b/srml/contract/src/account_db.rs @@ -209,7 +209,7 @@ impl<'a, T: Trait> OverlayAccountDb<'a, T> { } let mut local = self.local.borrow_mut(); - let contract = local.entry(account.clone()).or_insert(Default::default()); + let contract = local.entry(account.clone()).or_insert_with(|| Default::default()); contract.code_hash = Some(code_hash); contract.rent_allowance = Some(>::zero()); From 86e330bcaac8ff421b885e8c0d1261ae7395ef25 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 12:41:57 +0200 Subject: [PATCH 41/54] Add some comments. --- srml/contract/src/lib.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 92c0d1b6b362f..22872b87fa24f 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -465,8 +465,8 @@ decl_module! { result.map(|_| ()) } - /// Allows validators to claim a small reward for evicting a contract. If a validator - /// fails to do so, a regular users will be allowed to do so. + /// Allows block producers to claim a small reward for evicting a contract. If a block producer + /// fails to do so, a regular users will be allowed to claim the reward. /// /// If contract is not evicted as a result of this call, no actions are taken and /// the sender is not eligible for the reward. @@ -485,11 +485,14 @@ decl_module! { let mut check_block = >::block_number(); - // Make validators advantaged + // Add some advantage for block producers (who send unsigned extrinsics) by + // adding a handicap: for signed extrinsics we use a slightly older block number + // for the eviction check. This can be viewed as if we pushed regular users back in past. if signed { check_block = check_block.saturating_sub(>::signed_claim_handicap()); } + // If poking the contract has lead to eviction of the contract, give out the rewards. if rent::try_evict_at::(&dest, check_block) { T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; } From 7952ff323f93f9f7f6c60ed28eda1ea5304e3165 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 14:15:00 +0200 Subject: [PATCH 42/54] Refactor --- srml/contract/src/lib.rs | 2 +- srml/contract/src/rent.rs | 74 ++++++++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 22872b87fa24f..53150f26d22d5 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -493,7 +493,7 @@ decl_module! { } // If poking the contract has lead to eviction of the contract, give out the rewards. - if rent::try_evict_at::(&dest, check_block) { + if rent::try_evict_at::(&dest, check_block) == rent::RentOutcome::Evicted { T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; } } diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 12747e129b43a..73792ef42e772 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -19,6 +19,14 @@ use runtime_primitives::traits::{As, Bounded, CheckedDiv, CheckedMul, Saturating use srml_support::traits::{Currency, ExistenceRequirement, Imbalance, WithdrawReason}; use srml_support::StorageMap; +#[derive(PartialEq, Eq, Copy, Clone)] +#[must_use] +pub enum RentOutcome { + Exempted, + Evicted, + Paid, +} + /// Evict and optionally pay dues (or check account can pay them otherwise), for given block number. /// Return true iff the account has been evicted. /// @@ -38,45 +46,53 @@ fn try_evict_or_and_pay_rent( account: &T::AccountId, block_number: T::BlockNumber, pay_rent: bool, -) -> bool { +) -> RentOutcome { + use self::RentOutcome; + let contract = match >::get(account) { - None | Some(ContractInfo::Tombstone(_)) => return false, + None | Some(ContractInfo::Tombstone(_)) => return RentOutcome::Exempted, Some(ContractInfo::Alive(contract)) => contract, }; - // Rent has already been paid - if contract.deduct_block >= block_number { - return false; - } + // How much block has passed since the last deduction for the contract. + let blocks_passed = match block_number.saturating_sub(contract.deduct_block) { + // Rent has already been paid + n if n.is_zero() => return RentOutcome::Exempted, + n => n, + }; let balance = T::Currency::free_balance(account); - let free_storage = balance - .checked_div(&>::rent_deposit_offset()) - .unwrap_or(>::sa(0)); + // An amount of funds to charge per block for storage taken up by the contract. + let fee_per_block = { + let free_storage = balance + .checked_div(&>::rent_deposit_offset()) + .unwrap_or(>::sa(0)); - let effective_storage_size = - >::sa(contract.storage_size).saturating_sub(free_storage); + let effective_storage_size = + >::sa(contract.storage_size).saturating_sub(free_storage); - let fee_per_block = effective_storage_size - .checked_mul(&>::rent_byte_price()) - .unwrap_or(>::max_value()); + effective_storage_size + .checked_mul(&>::rent_byte_price()) + .unwrap_or(>::max_value()) + }; 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 false; + return RentOutcome::Exempted; } - let blocks_to_rent = block_number.saturating_sub(contract.deduct_block); - let rent = fee_per_block - .checked_mul(&>::sa(blocks_to_rent.as_())) - .unwrap_or(>::max_value()); + // The minimal amount of funds required for a contract not to be evicted. let subsistence_threshold = T::Currency::minimum_balance() + >::tombstone_deposit(); - let rent_limited = rent.min(contract.rent_allowance); + let dues = fee_per_block + .checked_mul(&>::sa(blocks_passed.as_())) + .unwrap_or(>::max_value()); - let rent_allowance_exceeded = rent > contract.rent_allowance; + // TODO: Rename as well + let rent_limited = dues.min(contract.rent_allowance); + let rent_allowance_exceeded = dues > contract.rent_allowance; let is_below_subsistence = balance < subsistence_threshold; let go_below_subsistence = balance.saturating_sub(rent_limited) < subsistence_threshold; let can_withdraw_rent = T::Currency::ensure_can_withdraw( @@ -84,7 +100,8 @@ fn try_evict_or_and_pay_rent( rent_limited, WithdrawReason::Fee, balance.saturating_sub(rent_limited), - ).is_ok(); + ) + .is_ok(); if !rent_allowance_exceeded && can_withdraw_rent && !go_below_subsistence { // Collect dues @@ -92,7 +109,7 @@ fn try_evict_or_and_pay_rent( if pay_rent { let imbalance = T::Currency::withdraw( account, - rent, + dues, WithdrawReason::Fee, ExistenceRequirement::KeepAlive, ) @@ -110,14 +127,15 @@ fn try_evict_or_and_pay_rent( .rent_allowance -= imbalance.peek(); // rent_allowance is not exceeded }) } - return false; + + RentOutcome::Paid } else { // Evict if can_withdraw_rent && !go_below_subsistence { T::Currency::withdraw( account, - rent, + dues, WithdrawReason::Fee, ExistenceRequirement::KeepAlive, ) @@ -142,7 +160,7 @@ fn try_evict_or_and_pay_rent( runtime_io::kill_child_storage(&contract.trie_id); } - return true; + RentOutcome::Evicted } } @@ -150,13 +168,13 @@ fn try_evict_or_and_pay_rent( /// /// Call try_evict_or_and_pay_rent with setting pay rent to true pub fn pay_rent(account: &T::AccountId) { - try_evict_or_and_pay_rent::(account, >::block_number(), true); + let _ = try_evict_or_and_pay_rent::(account, >::block_number(), true); } /// Evict the account if he should be evicted at the given block number. /// Return true iff the account has been evicted. /// /// Call try_evict_or_and_pay_rent with setting pay rent to false -pub fn try_evict_at(account: &T::AccountId, block: T::BlockNumber) -> bool { +pub fn try_evict_at(account: &T::AccountId, block: T::BlockNumber) -> RentOutcome { try_evict_or_and_pay_rent::(account, block, false) } From 806a479ee2f0c6c60eccc2a6c6e2d0306368ed0f Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 14:15:23 +0200 Subject: [PATCH 43/54] Shuffle and fix comments --- srml/contract/src/rent.rs | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 73792ef42e772..8cf644a5599ea 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -22,7 +22,16 @@ use srml_support::StorageMap; #[derive(PartialEq, Eq, Copy, Clone)] #[must_use] pub enum RentOutcome { + /// Exempted from rent iff: + /// * rent is offset completely by the `rent_deposit_offset`, + /// * or rent has already been paid for this block number, + /// * or account doesn't have a contract, + /// * or account has a tombstone. Exempted, + /// Evicted iff: + /// * rent exceed rent allowance, + /// * or can't withdraw the rent, + /// * or go below subsistence threshold. Evicted, Paid, } @@ -30,18 +39,7 @@ pub enum RentOutcome { /// Evict and optionally pay dues (or check account can pay them otherwise), for given block number. /// Return true iff the account has been evicted. /// -/// Exempted from rent iff: -/// * rent is offset completely by the `rent_deposit_offset`, -/// * or rent has already been paid for this block number, -/// * or account doesn't have a contract, -/// * or account has a tombstone. -/// -/// Evicted iff: -/// * rent exceed rent allowance, -/// * or can't withdraw the rent, -/// * or go below subsistence threshold. -/// -/// This function acts eagerly, all modification are committed into storages. +/// NOTE: This function acts eagerly, all modification are committed into storages. fn try_evict_or_and_pay_rent( account: &T::AccountId, block_number: T::BlockNumber, @@ -166,15 +164,15 @@ fn try_evict_or_and_pay_rent( /// Make account paying the rent for the current block number /// -/// Call try_evict_or_and_pay_rent with setting pay rent to true +/// NOTE: This function acts eagerly. pub fn pay_rent(account: &T::AccountId) { let _ = try_evict_or_and_pay_rent::(account, >::block_number(), true); } /// Evict the account if he should be evicted at the given block number. -/// Return true iff the account has been evicted. +/// Return `true` iff the account has been evicted. /// -/// Call try_evict_or_and_pay_rent with setting pay rent to false +/// NOTE: This function acts eagerly. pub fn try_evict_at(account: &T::AccountId, block: T::BlockNumber) -> RentOutcome { try_evict_or_and_pay_rent::(account, block, false) } From f6c696d43d77c61cdfec050be670a39c94af3b00 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 14:20:50 +0200 Subject: [PATCH 44/54] More comment fixes. --- srml/contract/src/rent.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 8cf644a5599ea..48dd360b289f5 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -33,13 +33,17 @@ pub enum RentOutcome { /// * or can't withdraw the rent, /// * or go below subsistence threshold. Evicted, + /// The outstanding dues were paid. Paid, } /// Evict and optionally pay dues (or check account can pay them otherwise), for given block number. /// Return true iff the account has been evicted. /// -/// NOTE: This function acts eagerly, all modification are committed into storages. +/// `pay_rent` gives an ability to pay or skip paying rent. This is useful for the +/// `try_evict_at` use-case. +/// +/// NOTE: This function acts eagerly, all modification are committed into the storage. fn try_evict_or_and_pay_rent( account: &T::AccountId, block_number: T::BlockNumber, @@ -170,7 +174,6 @@ pub fn pay_rent(account: &T::AccountId) { } /// Evict the account if he should be evicted at the given block number. -/// Return `true` iff the account has been evicted. /// /// NOTE: This function acts eagerly. pub fn try_evict_at(account: &T::AccountId, block: T::BlockNumber) -> RentOutcome { From 405ae981c3a93b582c8808695463809ff79e0490 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 14:40:51 +0200 Subject: [PATCH 45/54] dues limited --- srml/contract/src/rent.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 48dd360b289f5..8b3ee230e2206 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -92,16 +92,15 @@ fn try_evict_or_and_pay_rent( .checked_mul(&>::sa(blocks_passed.as_())) .unwrap_or(>::max_value()); - // TODO: Rename as well - let rent_limited = dues.min(contract.rent_allowance); + let dues_limited = dues.min(contract.rent_allowance); let rent_allowance_exceeded = dues > contract.rent_allowance; let is_below_subsistence = balance < subsistence_threshold; - let go_below_subsistence = balance.saturating_sub(rent_limited) < subsistence_threshold; + let go_below_subsistence = balance.saturating_sub(dues_limited) < subsistence_threshold; let can_withdraw_rent = T::Currency::ensure_can_withdraw( account, - rent_limited, + dues_limited, WithdrawReason::Fee, - balance.saturating_sub(rent_limited), + balance.saturating_sub(dues_limited), ) .is_ok(); From b3c7795c8409b3cb4a7f6ef3d23b81b60903fbae Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 14:49:24 +0200 Subject: [PATCH 46/54] Add comment --- srml/contract/src/rent.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 8b3ee230e2206..8c48bbe0b2e77 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -148,6 +148,9 @@ fn try_evict_or_and_pay_rent( } if !is_below_subsistence { + // The contract has funds above subsistence deposit and that means it can afford to + // leave tombstone. + // Note: this operation is heavy. // Note: if the storage hasn't been touched then it returns None, which is OK. let child_storage_root = runtime_io::child_storage_root(&contract.trie_id); From c6338ffecbe3ec79053c9fffbc628bf8f15e8a9e Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 14:59:18 +0200 Subject: [PATCH 47/54] Handicap --- srml/contract/src/lib.rs | 14 +++++++------- srml/contract/src/rent.rs | 17 +++++++++-------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 53150f26d22d5..c8563d231c8b6 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -97,7 +97,7 @@ use substrate_primitives::crypto::UncheckedFrom; use rstd::prelude::*; use rstd::marker::PhantomData; use parity_codec::{Codec, Encode, Decode}; -use runtime_primitives::traits::{Hash, As, SimpleArithmetic,Bounded, StaticLookup, Saturating}; +use runtime_primitives::traits::{Hash, As, SimpleArithmetic, Bounded, StaticLookup, Zero}; use srml_support::dispatch::{Result, Dispatchable}; use srml_support::{Parameter, StorageMap, StorageValue, decl_module, decl_event, decl_storage, storage::child}; use srml_support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency}; @@ -483,17 +483,17 @@ decl_module! { inherent and auxiliary sender only provided on inherent") }; - let mut check_block = >::block_number(); - // Add some advantage for block producers (who send unsigned extrinsics) by // adding a handicap: for signed extrinsics we use a slightly older block number // for the eviction check. This can be viewed as if we pushed regular users back in past. - if signed { - check_block = check_block.saturating_sub(>::signed_claim_handicap()); - } + let handicap = if signed { + >::signed_claim_handicap() + } else { + Zero::zero() + }; // If poking the contract has lead to eviction of the contract, give out the rewards. - if rent::try_evict_at::(&dest, check_block) == rent::RentOutcome::Evicted { + if rent::try_evict_at::(&dest, handicap) == rent::RentOutcome::Evicted { T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; } } diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 8c48bbe0b2e77..d02d195ead7d6 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -46,18 +46,19 @@ pub enum RentOutcome { /// NOTE: This function acts eagerly, all modification are committed into the storage. fn try_evict_or_and_pay_rent( account: &T::AccountId, - block_number: T::BlockNumber, + handicap: T::BlockNumber, pay_rent: bool, ) -> RentOutcome { - use self::RentOutcome; - let contract = match >::get(account) { None | Some(ContractInfo::Tombstone(_)) => return RentOutcome::Exempted, Some(ContractInfo::Alive(contract)) => contract, }; + // Calculate an effective block number, i.e. after adjusting for handicap. + let effective_block_number = >::block_number().saturating_sub(handicap); + // How much block has passed since the last deduction for the contract. - let blocks_passed = match block_number.saturating_sub(contract.deduct_block) { + let blocks_passed = match effective_block_number.saturating_sub(contract.deduct_block) { // Rent has already been paid n if n.is_zero() => return RentOutcome::Exempted, n => n, @@ -172,12 +173,12 @@ fn try_evict_or_and_pay_rent( /// /// NOTE: This function acts eagerly. pub fn pay_rent(account: &T::AccountId) { - let _ = try_evict_or_and_pay_rent::(account, >::block_number(), true); + let _ = try_evict_or_and_pay_rent::(account, Zero::zero(), true); } -/// Evict the account if he should be evicted at the given block number. +/// Evict the account if it should be evicted at the given block number. /// /// NOTE: This function acts eagerly. -pub fn try_evict_at(account: &T::AccountId, block: T::BlockNumber) -> RentOutcome { - try_evict_or_and_pay_rent::(account, block, false) +pub fn try_evict_at(account: &T::AccountId, handicap: T::BlockNumber) -> RentOutcome { + try_evict_or_and_pay_rent::(account, handicap, false) } From d040fb90894d349332fdc95b609f2380c3f90926 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 15:11:26 +0200 Subject: [PATCH 48/54] Docs. --- srml/contract/src/rent.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index d02d195ead7d6..81f0fa2776579 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -37,11 +37,12 @@ pub enum RentOutcome { Paid, } -/// Evict and optionally pay dues (or check account can pay them otherwise), for given block number. -/// Return true iff the account has been evicted. +/// Evict and optionally pay dues (or check account can pay them otherwise) at the current +/// block number (modulo `handicap`, read on). /// -/// `pay_rent` gives an ability to pay or skip paying rent. This is useful for the -/// `try_evict_at` use-case. +/// `pay_rent` gives an ability to pay or skip paying rent. +/// `handicap` gives a way to shift the state of the contract to some moment in the past. +/// These parameters are useful for the `try_evict_at` use-case. /// /// NOTE: This function acts eagerly, all modification are committed into the storage. fn try_evict_or_and_pay_rent( @@ -178,6 +179,10 @@ pub fn pay_rent(account: &T::AccountId) { /// Evict the account if it should be evicted at the given block number. /// +/// `handicap` gives a way to shift the state of the contract to some moment in the past. E.g. +/// if the contract is going to be evicted at the current block, `handicap=1` can defer +/// the eviction for 1 block. +/// /// NOTE: This function acts eagerly. pub fn try_evict_at(account: &T::AccountId, handicap: T::BlockNumber) -> RentOutcome { try_evict_or_and_pay_rent::(account, handicap, false) From a2520955f652a9f99eedc97467a7a862614386ce Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 29 Apr 2019 15:15:52 +0200 Subject: [PATCH 49/54] Apply suggestions from code review Co-Authored-By: pepyakin --- srml/contract/src/rent.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index d02d195ead7d6..71009c1d225f0 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -153,7 +153,6 @@ fn try_evict_or_and_pay_rent( // leave tombstone. // Note: this operation is heavy. - // Note: if the storage hasn't been touched then it returns None, which is OK. let child_storage_root = runtime_io::child_storage_root(&contract.trie_id); let tombstone = TombstoneContractInfo::new( @@ -179,6 +178,6 @@ pub fn pay_rent(account: &T::AccountId) { /// Evict the account if it should be evicted at the given block number. /// /// NOTE: This function acts eagerly. -pub fn try_evict_at(account: &T::AccountId, handicap: T::BlockNumber) -> RentOutcome { +pub fn try_evict(account: &T::AccountId, handicap: T::BlockNumber) -> RentOutcome { try_evict_or_and_pay_rent::(account, handicap, false) } From bfb8a13eeadd7146bd486fe6568750ce5023c473 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 15:19:43 +0200 Subject: [PATCH 50/54] Coalesce block_passed in a block --- srml/contract/src/rent.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 15b0129e10d6c..85ef7f508d09f 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -55,14 +55,16 @@ fn try_evict_or_and_pay_rent( Some(ContractInfo::Alive(contract)) => contract, }; - // Calculate an effective block number, i.e. after adjusting for handicap. - let effective_block_number = >::block_number().saturating_sub(handicap); - // How much block has passed since the last deduction for the contract. - let blocks_passed = match effective_block_number.saturating_sub(contract.deduct_block) { - // Rent has already been paid - n if n.is_zero() => return RentOutcome::Exempted, - n => n, + let blocks_passed = { + // Calculate an effective block number, i.e. after adjusting for handicap. + let effective_block_number = >::block_number().saturating_sub(handicap); + + match effective_block_number.saturating_sub(contract.deduct_block) { + // Rent has already been paid + n if n.is_zero() => return RentOutcome::Exempted, + n => n, + } }; let balance = T::Currency::free_balance(account); From 9a491c7fb77036e7f817d623fa143035b2e77030 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 15:30:49 +0200 Subject: [PATCH 51/54] Fix build --- srml/contract/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index c8563d231c8b6..d7b19009ab306 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -493,7 +493,7 @@ decl_module! { }; // If poking the contract has lead to eviction of the contract, give out the rewards. - if rent::try_evict_at::(&dest, handicap) == rent::RentOutcome::Evicted { + if rent::try_evict::(&dest, handicap) == rent::RentOutcome::Evicted { T::Currency::deposit_into_existing(rewarded, Self::surcharge_reward())?; } } From be7e1d85c6feabb89314c15bb07b0bf6750323d5 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 15:31:04 +0200 Subject: [PATCH 52/54] =?UTF-8?q?Paid=20=E2=86=92=20Ok?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- srml/contract/src/rent.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 85ef7f508d09f..95062a5cc6009 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -33,8 +33,8 @@ pub enum RentOutcome { /// * or can't withdraw the rent, /// * or go below subsistence threshold. Evicted, - /// The outstanding dues were paid. - Paid, + /// The outstanding dues were paid or were able to be paid. + Ok, } /// Evict and optionally pay dues (or check account can pay them otherwise) at the current @@ -133,7 +133,7 @@ fn try_evict_or_and_pay_rent( }) } - RentOutcome::Paid + RentOutcome::Ok } else { // Evict From 63620c590126dd213efa8e689ea25ce4ec63402e Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 15:34:32 +0200 Subject: [PATCH 53/54] =?UTF-8?q?match=20=E2=86=92=20if?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- srml/contract/src/rent.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index 95062a5cc6009..f0bd3ad92863e 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -59,12 +59,12 @@ fn try_evict_or_and_pay_rent( let blocks_passed = { // Calculate an effective block number, i.e. after adjusting for handicap. let effective_block_number = >::block_number().saturating_sub(handicap); - - match effective_block_number.saturating_sub(contract.deduct_block) { + let n = effective_block_number.saturating_sub(contract.deduct_block); + if n.is_zero() { // Rent has already been paid - n if n.is_zero() => return RentOutcome::Exempted, - n => n, + return RentOutcome::Exempted; } + n }; let balance = T::Currency::free_balance(account); From 381cf6237c866a261913738409f372772bbb6b3d Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 29 Apr 2019 15:47:13 +0200 Subject: [PATCH 54/54] Imrpove handicap description --- srml/contract/src/rent.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/srml/contract/src/rent.rs b/srml/contract/src/rent.rs index f0bd3ad92863e..b914e41af3198 100644 --- a/srml/contract/src/rent.rs +++ b/srml/contract/src/rent.rs @@ -41,8 +41,8 @@ pub enum RentOutcome { /// block number (modulo `handicap`, read on). /// /// `pay_rent` gives an ability to pay or skip paying rent. -/// `handicap` gives a way to shift the state of the contract to some moment in the past. -/// These parameters are useful for the `try_evict_at` use-case. +/// `handicap` gives a way to check or pay the rent up to a moment in the past instead +/// of current block. /// /// NOTE: This function acts eagerly, all modification are committed into the storage. fn try_evict_or_and_pay_rent( @@ -180,9 +180,9 @@ pub fn pay_rent(account: &T::AccountId) { /// Evict the account if it should be evicted at the given block number. /// -/// `handicap` gives a way to shift the state of the contract to some moment in the past. E.g. -/// if the contract is going to be evicted at the current block, `handicap=1` can defer -/// the eviction for 1 block. +/// `handicap` gives a way to check or pay the rent up to a moment in the past instead +/// of current block. E.g. if the contract is going to be evicted at the current block, +/// `handicap=1` can defer the eviction for 1 block. /// /// NOTE: This function acts eagerly. pub fn try_evict(account: &T::AccountId, handicap: T::BlockNumber) -> RentOutcome {