diff --git a/substrate/frame/contracts/CHANGELOG.md b/substrate/frame/contracts/CHANGELOG.md index 2a93c838bc..76fc09ad17 100644 --- a/substrate/frame/contracts/CHANGELOG.md +++ b/substrate/frame/contracts/CHANGELOG.md @@ -49,7 +49,7 @@ This version constitutes the first release that brings any stability guarantees [#8014](https://github.com/paritytech/substrate/pull/8014) - Charge rent for code stored on the chain in addition to the already existing -rent that is payed for data storage. +rent that is paid for data storage. [#7935](https://github.com/paritytech/substrate/pull/7935) - Allow the runtime to configure per storage item costs in addition diff --git a/substrate/frame/contracts/src/benchmarking/mod.rs b/substrate/frame/contracts/src/benchmarking/mod.rs index 91210f0888..2ba32069cb 100644 --- a/substrate/frame/contracts/src/benchmarking/mod.rs +++ b/substrate/frame/contracts/src/benchmarking/mod.rs @@ -409,7 +409,7 @@ benchmarks! { // We benchmark the costs for sucessfully evicting an empty contract. // The actual costs are depending on how many storage items the evicted contract - // does have. However, those costs are not to be payed by the sender but + // does have. However, those costs are not to be paid by the sender but // will be distributed over multiple blocks using a scheduler. Otherwise there is // no incentive to remove large contracts when the removal is more expensive than // the reward for removing them. @@ -435,7 +435,7 @@ benchmarks! { instance.ensure_tombstone()?; // the caller should get the reward for being a good snitch - // this is capped by the maximum amount of rent payed. So we only now that it should + // this is capped by the maximum amount of rent paid. So we only now that it should // have increased by at most the surcharge reward. assert!( T::Currency::free_balance(&instance.caller) > diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index d69964ff77..d5b489d891 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -686,8 +686,11 @@ where contract } else { >::get(&dest) - .and_then(|contract| contract.get_alive()) - .ok_or((Error::::NotCallable.into(), 0))? + .ok_or((>::ContractNotFound.into(), 0)) + .and_then(|contract| + contract.get_alive() + .ok_or((>::ContractIsTombstone.into(), 0)) + )? }; let executable = E::from_storage(contract.code_hash, schedule, gas_meter) @@ -701,7 +704,7 @@ where let contract = Rent:: ::charge(&dest, contract, executable.occupied_storage()) .map_err(|e| (e.into(), executable.code_len()))? - .ok_or((Error::::NotCallable.into(), executable.code_len()))?; + .ok_or((Error::::RentNotPaid.into(), executable.code_len()))?; (dest, contract, executable, ExportedFunction::Call) } FrameArgs::Instantiate{sender, trie_seed, executable, salt} => { @@ -791,7 +794,7 @@ where let code_len = executable.code_len(); // Every call or instantiate also optionally transferres balance. - self.initial_transfer().map_err(|e| (ExecError::from(e), 0))?; + self.initial_transfer().map_err(|e| (ExecError::from(e), code_len))?; // Call into the wasm blob. let output = executable.execute( @@ -954,12 +957,23 @@ where // The transfer as performed by a call or instantiate. fn initial_transfer(&self) -> DispatchResult { + let frame = self.top_frame(); + let value = frame.value_transferred; + let subsistence_threshold = >::subsistence_threshold(); + + // If the value transferred to a new contract is less than the subsistence threshold + // we can error out early. This avoids executing the constructor in cases where + // we already know that the contract has too little balance. + if frame.entry_point == ExportedFunction::Constructor && value < subsistence_threshold { + return Err(>::NewContractNotFunded.into()); + } + Self::transfer( self.caller_is_origin(), false, self.caller(), - &self.top_frame().account_id, - self.top_frame().value_transferred, + &frame.account_id, + value, ) } @@ -2005,7 +2019,7 @@ mod tests { ctx.ext.instantiate( 0, dummy_ch, - 15u64, + Contracts::::subsistence_threshold(), vec![], &[], ), @@ -2286,7 +2300,7 @@ mod tests { let code = MockLoader::insert(Constructor, |ctx, _| { assert_matches!( ctx.ext.call(0, ctx.ext.address().clone(), 0, vec![]), - Err((ExecError{error, ..}, _)) if error == >::NotCallable.into() + Err((ExecError{error, ..}, _)) if error == >::ContractNotFound.into() ); exec_success() }); diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index 67c5acee8f..c655a926d8 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -388,7 +388,7 @@ pub mod pallet { /// producer fails to do so, a regular users will be allowed to claim the reward. /// /// In case of a successful eviction no fees are charged from the sender. However, the - /// reward is capped by the total amount of rent that was payed by the contract while + /// reward is capped by the total amount of rent that was paid by the contract while /// it was alive. /// /// If contract is not evicted as a result of this call, [`Error::ContractNotEvictable`] @@ -421,10 +421,10 @@ pub mod pallet { // If poking the contract has lead to eviction of the contract, give out the rewards. match Rent::>::try_eviction(&dest, handicap)? { - (Some(rent_payed), code_len) => { + (Some(rent_paid), code_len) => { T::Currency::deposit_into_existing( &rewarded, - T::SurchargeReward::get().min(rent_payed), + T::SurchargeReward::get().min(rent_paid), ) .map(|_| PostDispatchInfo { actual_weight: Some(T::WeightInfo::claim_surcharge(code_len / 1024)), @@ -535,9 +535,20 @@ pub mod pallet { /// Performing a call was denied because the calling depth reached the limit /// of what is specified in the schedule. MaxCallDepthReached, - /// The contract that was called is either no contract at all (a plain account) - /// or is a tombstone. - NotCallable, + /// No contract was found at the specified address. + ContractNotFound, + /// A tombstone exist at the specified address. + /// + /// Tombstone cannot be called. Anyone can use `seal_restore_to` in order to revive + /// the contract, though. + ContractIsTombstone, + /// The called contract does not have enough balance to pay for its storage. + /// + /// The contract ran out of balance and is therefore eligible for eviction into a + /// tombstone. Anyone can evict the contract by submitting a `claim_surcharge` + /// extrinsic. Alternatively, a plain balance transfer can be used in order to + /// increase the contracts funds so that it can be called again. + RentNotPaid, /// The code supplied to `instantiate_with_code` exceeds the limit specified in the /// current schedule. CodeTooLarge, diff --git a/substrate/frame/contracts/src/rent.rs b/substrate/frame/contracts/src/rent.rs index 5999a152d0..68e8c57e9a 100644 --- a/substrate/frame/contracts/src/rent.rs +++ b/substrate/frame/contracts/src/rent.rs @@ -96,7 +96,7 @@ where /// Process a report that a contract under the given address should be evicted. /// /// Enact the eviction right away if the contract should be evicted and return the amount - /// of rent that the contract payed over its lifetime. + /// of rent that the contract paid over its lifetime. /// Otherwise, **do nothing** and return None. /// /// The `handicap` parameter gives a way to check the rent to a moment in the past instead @@ -130,15 +130,15 @@ where match verdict { Verdict::Evict { ref amount } => { // The outstanding `amount` is withdrawn inside `enact_verdict`. - let rent_payed = amount + let rent_paid = amount .as_ref() .map(|a| a.peek()) .unwrap_or_else(|| >::zero()) - .saturating_add(contract.rent_payed); + .saturating_add(contract.rent_paid); Self::enact_verdict( account, contract, current_block_number, verdict, Some(module), )?; - Ok((Some(rent_payed), code_len)) + Ok((Some(rent_paid), code_len)) } _ => Ok((None, code_len)), } @@ -297,7 +297,7 @@ where >::insert(&dest, ContractInfo::Alive(AliveContractInfo:: { code_hash, rent_allowance, - rent_payed: >::zero(), + rent_paid: >::zero(), deduct_block: current_block, last_write, .. origin_contract @@ -544,7 +544,7 @@ where let contract = ContractInfo::Alive(AliveContractInfo:: { rent_allowance: alive_contract_info.rent_allowance - amount.peek(), deduct_block: current_block_number, - rent_payed: alive_contract_info.rent_payed.saturating_add(amount.peek()), + rent_paid: alive_contract_info.rent_paid.saturating_add(amount.peek()), ..alive_contract_info }); >::insert(account, &contract); diff --git a/substrate/frame/contracts/src/storage.rs b/substrate/frame/contracts/src/storage.rs index bb3553529b..17486b274f 100644 --- a/substrate/frame/contracts/src/storage.rs +++ b/substrate/frame/contracts/src/storage.rs @@ -97,11 +97,11 @@ pub struct RawAliveContractInfo { pub code_hash: CodeHash, /// Pay rent at most up to this value. pub rent_allowance: Balance, - /// The amount of rent that was payed by the contract over its whole lifetime. + /// The amount of rent that was paid by the contract over its whole lifetime. /// /// A restored contract starts with a value of zero just like a new contract. - pub rent_payed: Balance, - /// Last block rent has been payed. + pub rent_paid: Balance, + /// Last block rent has been paid. pub deduct_block: BlockNumber, /// Last block child storage has been written. pub last_write: Option, @@ -243,7 +243,7 @@ where // charge rent for it during instantiation. >::block_number().saturating_sub(1u32.into()), rent_allowance: >::max_value(), - rent_payed: >::zero(), + rent_paid: >::zero(), pair_count: 0, last_write: None, _reserved: None, diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs index f3d6be6279..6fdaecebd8 100644 --- a/substrate/frame/contracts/src/tests.rs +++ b/substrate/frame/contracts/src/tests.rs @@ -366,7 +366,7 @@ fn calling_plain_account_fails() { Contracts::call(Origin::signed(ALICE), BOB, 0, GAS_LIMIT, Vec::new()), Err( DispatchErrorWithPostInfo { - error: Error::::NotCallable.into(), + error: Error::::ContractNotFound.into(), post_info: PostDispatchInfo { actual_weight: Some(base_cost), pays_fee: Default::default(), @@ -396,7 +396,7 @@ fn account_removal_does_not_remove_storage() { deduct_block: System::block_number(), code_hash: H256::repeat_byte(1), rent_allowance: 40, - rent_payed: 0, + rent_paid: 0, last_write: None, _reserved: None, }); @@ -412,7 +412,7 @@ fn account_removal_does_not_remove_storage() { deduct_block: System::block_number(), code_hash: H256::repeat_byte(2), rent_allowance: 40, - rent_payed: 0, + rent_paid: 0, last_write: None, _reserved: None, }); @@ -1088,7 +1088,7 @@ fn call_removed_contract() { // Calling contract should deny access because rent cannot be paid. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), addr.clone(), 0, GAS_LIMIT, call::null()), - Error::::NotCallable + Error::::RentNotPaid, ); // No event is generated because the contract is not actually removed. assert_eq!(System::events(), vec![]); @@ -1096,7 +1096,7 @@ fn call_removed_contract() { // Subsequent contract calls should also fail. assert_err_ignore_postinfo!( Contracts::call(Origin::signed(ALICE), addr.clone(), 0, GAS_LIMIT, call::null()), - Error::::NotCallable + Error::::RentNotPaid, ); // A snitch can now remove the contract @@ -1321,7 +1321,7 @@ fn restoration( Contracts::call( Origin::signed(ALICE), addr_bob.clone(), 0, GAS_LIMIT, call::null() ), - Error::::NotCallable + Error::::RentNotPaid, ); assert!(System::events().is_empty()); assert!(ContractInfoOf::::get(&addr_bob).unwrap().get_alive().is_some()); @@ -2669,11 +2669,11 @@ fn surcharge_reward_is_capped() { let balance = Balances::free_balance(&ALICE); let reward = ::SurchargeReward::get(); - // some rent should have payed due to instantiation - assert_ne!(contract.rent_payed, 0); + // some rent should have paid due to instantiation + assert_ne!(contract.rent_paid, 0); // the reward should be parameterized sufficiently high to make this test useful - assert!(reward > contract.rent_payed); + assert!(reward > contract.rent_paid); // make contract eligible for eviction initialize_block(40); @@ -2682,13 +2682,13 @@ fn surcharge_reward_is_capped() { assert_ok!(Contracts::claim_surcharge(Origin::none(), addr.clone(), Some(ALICE))); // this reward does not take into account the last rent payment collected during eviction - let capped_reward = reward.min(contract.rent_payed); + let capped_reward = reward.min(contract.rent_paid); // this is smaller than the actual reward because it does not take into account the // rent collected during eviction assert!(Balances::free_balance(&ALICE) > balance + capped_reward); - // the full reward is not payed out because of the cap introduced by rent_payed + // the full reward is not paid out because of the cap introduced by rent_paid assert!(Balances::free_balance(&ALICE) < balance + reward); }); } diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index f9e6e92832..3701c0d607 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -601,14 +601,16 @@ where let transfer_failed = Error::::TransferFailed.into(); let not_funded = Error::::NewContractNotFunded.into(); let no_code = Error::::CodeNotFound.into(); - let invalid_contract = Error::::NotCallable.into(); + let not_found = Error::::ContractNotFound.into(); + let is_tombstone = Error::::ContractIsTombstone.into(); + let rent_not_paid = Error::::RentNotPaid.into(); match from { x if x == below_sub => Ok(BelowSubsistenceThreshold), x if x == transfer_failed => Ok(TransferFailed), x if x == not_funded => Ok(NewContractNotFunded), x if x == no_code => Ok(CodeNotFound), - x if x == invalid_contract => Ok(NotCallable), + x if (x == not_found || x == is_tombstone || x == rent_not_paid) => Ok(NotCallable), err => Err(err) } }