diff --git a/substrate/srml/contracts/src/exec.rs b/substrate/srml/contracts/src/exec.rs index c9573152de..c726d93e13 100644 --- a/substrate/srml/contracts/src/exec.rs +++ b/substrate/srml/contracts/src/exec.rs @@ -15,13 +15,14 @@ // along with Substrate. If not, see . use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, - TrieId, BalanceOf, ContractInfoOf}; + TrieId, BalanceOf, ContractInfo}; use crate::account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}; use crate::gas::{Gas, GasMeter, Token, approx_gas_for_balance}; +use crate::rent; use rstd::prelude::*; use runtime_primitives::traits::{Bounded, CheckedAdd, CheckedSub, Zero}; -use srml_support::{StorageMap, traits::{WithdrawReason, Currency}}; +use srml_support::traits::{WithdrawReason, Currency}; use timestamp; pub type AccountIdOf = ::AccountId; @@ -267,11 +268,11 @@ where { /// Create the top level execution context. /// - /// The specified `origin` address will be used as `sender` for + /// The specified `origin` address will be used as `sender` for. The `origin` must be a regular + /// account (not a contract). pub fn top_level(origin: T::AccountId, cfg: &'a Config, vm: &'a V, loader: &'a L) -> Self { ExecutionContext { - self_trie_id: >::get(&origin) - .and_then(|i| i.as_alive().map(|i| i.trie_id.clone())), + self_trie_id: None, self_account: origin, overlay: OverlayAccountDb::::new(&DirectAccountDb), depth: 0, @@ -283,10 +284,11 @@ where } } - fn nested(&self, overlay: OverlayAccountDb<'a, T>, dest: T::AccountId) -> Self { + fn nested(&self, overlay: OverlayAccountDb<'a, T>, dest: T::AccountId, trie_id: Option) + -> Self + { ExecutionContext { - self_trie_id: >::get(&dest) - .and_then(|i| i.as_alive().map(|i| i.trie_id.clone())), + self_trie_id: trie_id, self_account: dest, overlay, depth: self.depth + 1, @@ -321,14 +323,20 @@ 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 changed before the first call - crate::rent::pay_rent::(&dest); + let contract_info = rent::pay_rent::(&dest); + + // Calls to dead contracts always fail. + if let Some(ContractInfo::Tombstone(_)) = contract_info { + return Err("contract has been evicted"); + }; let mut output_data = Vec::new(); let (change_set, events, calls) = { let mut nested = self.nested( OverlayAccountDb::new(&self.overlay), - dest.clone() + dest.clone(), + contract_info.and_then(|i| i.as_alive().map(|i| i.trie_id.clone())) ); if value > BalanceOf::::zero() { @@ -342,6 +350,8 @@ where )?; } + // If code_hash is not none, then the destination account is a live contract, otherwise + // it is a regular account since tombstone accounts have already been rejected. if let Some(dest_code_hash) = self.overlay.get_code_hash(&dest) { let executable = self.loader.load_main(&dest_code_hash)?; output_data = self @@ -400,7 +410,8 @@ where overlay.create_contract(&dest, code_hash.clone())?; - let mut nested = self.nested(overlay, dest.clone()); + // TrieId has not been generated yet and storage is empty since contract is new. + let mut nested = self.nested(overlay, dest.clone(), None); // Send funds unconditionally here. If the `endowment` is below existential_deposit // then error will be returned here. diff --git a/substrate/srml/contracts/src/rent.rs b/substrate/srml/contracts/src/rent.rs index 08175a12d9..96f8516a5f 100644 --- a/substrate/srml/contracts/src/rent.rs +++ b/substrate/srml/contracts/src/rent.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use crate::{BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo, Trait}; +use crate::{BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo, Trait, AliveContractInfo}; use runtime_primitives::traits::{Bounded, CheckedDiv, CheckedMul, Saturating, Zero, SaturatedConversion}; use srml_support::traits::{Currency, ExistenceRequirement, Get, WithdrawReason}; @@ -50,9 +50,10 @@ fn try_evict_or_and_pay_rent( account: &T::AccountId, handicap: T::BlockNumber, pay_rent: bool, -) -> RentOutcome { - let contract = match >::get(account) { - None | Some(ContractInfo::Tombstone(_)) => return RentOutcome::Exempted, +) -> (RentOutcome, Option>) { + let contract_info = >::get(account); + let contract = match contract_info { + None | Some(ContractInfo::Tombstone(_)) => return (RentOutcome::Exempted, contract_info), Some(ContractInfo::Alive(contract)) => contract, }; @@ -65,7 +66,7 @@ fn try_evict_or_and_pay_rent( let n = effective_block_number.saturating_sub(contract.deduct_block); if n.is_zero() { // Rent has already been paid - return RentOutcome::Exempted; + return (RentOutcome::Exempted, Some(ContractInfo::Alive(contract))); } n }; @@ -89,7 +90,7 @@ fn try_evict_or_and_pay_rent( 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 RentOutcome::Exempted; + return (RentOutcome::Exempted, Some(ContractInfo::Alive(contract))); } // The minimal amount of funds required for a contract not to be evicted. @@ -99,7 +100,7 @@ fn try_evict_or_and_pay_rent( // The contract cannot afford to leave a tombstone, so remove the contract info altogether. >::remove(account); runtime_io::kill_child_storage(&contract.trie_id); - return RentOutcome::Evicted; + return (RentOutcome::Evicted, None); } let dues = fee_per_block @@ -149,33 +150,34 @@ fn try_evict_or_and_pay_rent( &child_storage_root[..], contract.code_hash, ); - >::insert(account, ContractInfo::Tombstone(tombstone)); + let tombstone_info = ContractInfo::Tombstone(tombstone); + >::insert(account, &tombstone_info); runtime_io::kill_child_storage(&contract.trie_id); - return RentOutcome::Evicted; + return (RentOutcome::Evicted, Some(tombstone_info)); } if pay_rent { - >::mutate(account, |contract| { - let contract = contract - .as_mut() - .and_then(|c| c.as_alive_mut()) - .expect("Dead or nonexistent account has been exempt above; qed"); + let contract_info = ContractInfo::Alive(AliveContractInfo:: { + rent_allowance: contract.rent_allowance - dues, // rent_allowance is not exceeded + deduct_block: current_block_number, + ..contract + }); - contract.rent_allowance -= dues; // rent_allowance is not exceeded - contract.deduct_block = current_block_number; - }) + >::insert(account, &contract_info); + + return (RentOutcome::Ok, Some(contract_info)); } - RentOutcome::Ok + (RentOutcome::Ok, Some(ContractInfo::Alive(contract))) } /// Make account paying the rent for the current block number /// /// NOTE: This function acts eagerly. -pub fn pay_rent(account: &T::AccountId) { - let _ = try_evict_or_and_pay_rent::(account, Zero::zero(), true); +pub fn pay_rent(account: &T::AccountId) -> Option> { + try_evict_or_and_pay_rent::(account, Zero::zero(), true).1 } /// Evict the account if it should be evicted at the given block number. @@ -186,5 +188,5 @@ pub fn pay_rent(account: &T::AccountId) { /// /// NOTE: This function acts eagerly. pub fn try_evict(account: &T::AccountId, handicap: T::BlockNumber) -> RentOutcome { - try_evict_or_and_pay_rent::(account, handicap, false) + try_evict_or_and_pay_rent::(account, handicap, false).0 } diff --git a/substrate/srml/contracts/src/tests.rs b/substrate/srml/contracts/src/tests.rs index ab483298f6..3308a8cac6 100644 --- a/substrate/srml/contracts/src/tests.rs +++ b/substrate/srml/contracts/src/tests.rs @@ -927,7 +927,11 @@ fn deduct_blocks() { #[test] fn call_contract_removals() { - removals(|| Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()).is_ok()); + removals(|| { + // Call on already-removed account might fail, and this is fine. + Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()); + true + }); } #[test] @@ -1115,6 +1119,45 @@ fn removals(trigger_call: impl Fn() -> bool) { ); } +#[test] +fn call_removed_contract() { + 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::from(1_000u32).encode() // rent allowance + )); + + // Calling contract should succeed. + assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); + + // Advance blocks + System::initialize(&10, &[0u8; 32].into(), &[0u8; 32].into(), &Default::default()); + + // Calling contract should remove contract and fail. + assert_err!( + Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), + "contract has been evicted" + ); + + // Subsequent contract calls should also fail. + assert_err!( + Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), + "contract has been evicted" + ); + } + ) +} + const CODE_CHECK_DEFAULT_RENT_ALLOWANCE: &str = r#" (module (import "env" "ext_rent_allowance" (func $ext_rent_allowance)) @@ -1294,7 +1337,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: // same copy of this (modulo hex notation differences) in `CODE_RESTORATION`. // // When this assert is triggered make sure that you update the literals here and in - // `CODE_RESTORATION`. Hopefully, we switch to automatical injection of the code. + // `CODE_RESTORATION`. Hopefully, we switch to automatic injection of the code. const ENCODED_CALL_LITERAL: &str = "0105020000000000000069aedfb4f6c1c398e97f8a5204de0f95ad5e7dc3540960beab11a86c569fbfcf320000\ 0000000000080100000000000000000000000000000000000000000000000000000000000000010000000000000\ @@ -1368,7 +1411,10 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: // Call `BOB`, which makes it pay rent. Since the rent allowance is set to 0 // we expect that it will get removed leaving tombstone. - assert_ok!(Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null())); + assert_err!( + Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), + "contract has been evicted" + ); assert!(ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); /// Create another account with the address `DJANGO` with `CODE_RESTORATION`.