diff --git a/substrate/frame/contracts/COMPLEXITY.md b/substrate/frame/contracts/COMPLEXITY.md index 1d8a122aa0..f11e835bc1 100644 --- a/substrate/frame/contracts/COMPLEXITY.md +++ b/substrate/frame/contracts/COMPLEXITY.md @@ -350,6 +350,20 @@ Loading `init_code` and `input_data` should be charged in any case. **complexity**: All complexity comes from loading buffers and executing `instantiate` executive function. The former component is proportional to the sizes of `init_code`, `value` and `input_data` buffers. The latter component completely depends on the complexity of `instantiate` executive function and also dominated by it. +## ext_terminate + +This function receives the following arguments: + +- `beneficiary`, buffer of a marshaled `AccountId` + +It consists of the following steps: + +1. Loading `beneficiary` buffer from the sandbox memory (see sandboxing memory get) and then decoding it. + +Loading of the `beneficiary` buffer should be charged. This is because the sizes of buffers are specified by the calling code, even though marshaled representations are, essentially, of constant size. This can be fixed by assigning an upper bound for sizes of `AccountId`. + +**complexity**: All complexity comes from loading buffers and executing `terminate` executive function. The former component is proportional to the size of the `beneficiary` buffer. The latter component completely depends on the complexity of `terminate` executive function and also dominated by it. + ## ext_return This function receives a `data` buffer as an argument. Execution of the function consists of the following steps: diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index 19d1d52497..d550756730 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -128,6 +128,13 @@ pub trait Ext { gas_meter: &mut GasMeter, ) -> Result<(), DispatchError>; + /// Transfer all funds to `beneficiary` and delete the contract. + fn terminate( + &mut self, + beneficiary: &AccountIdOf, + gas_meter: &mut GasMeter, + ) -> Result<(), DispatchError>; + /// Call (possibly transferring some amount of funds) into the specified account. fn call( &mut self, @@ -428,20 +435,6 @@ where gas_meter, )?; - // Destroy contract if insufficient remaining balance. - if nested.overlay.get_balance(&dest) < nested.config.existential_deposit { - let parent = nested.parent - .expect("a nested execution context must have a parent; qed"); - if parent.is_live(&dest) { - return Err(ExecError { - reason: "contract cannot be destroyed during recursive execution".into(), - buffer: output.data, - }); - } - - nested.overlay.destroy_contract(&dest); - } - Ok(output) } None => Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }), @@ -535,9 +528,37 @@ where Ok((dest, output)) } - fn new_call_context<'b>(&'b mut self, caller: T::AccountId, value: BalanceOf) - -> CallContext<'b, 'a, T, V, L> - { + pub fn terminate( + &mut self, + beneficiary: &T::AccountId, + gas_meter: &mut GasMeter, + ) -> Result<(), DispatchError> { + let self_id = self.self_account.clone(); + let value = self.overlay.get_balance(&self_id); + if let Some(parent) = self.parent { + if parent.is_live(&self_id) { + return Err(DispatchError::Other( + "Cannot terminate a contract that is present on the call stack", + )); + } + } + transfer( + gas_meter, + TransferCause::Terminate, + &self_id, + beneficiary, + value, + self, + )?; + self.overlay.destroy_contract(&self_id); + Ok(()) + } + + fn new_call_context<'b>( + &'b mut self, + caller: T::AccountId, + value: BalanceOf, + ) -> CallContext<'b, 'a, T, V, L> { let timestamp = self.timestamp.clone(); let block_number = self.block_number.clone(); CallContext { @@ -606,6 +627,7 @@ impl Token for TransferFeeToken> { enum TransferCause { Call, Instantiate, + Terminate, } /// Transfer some funds from `transactor` to `dest`. @@ -642,7 +664,7 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( Instantiate => ContractInstantiate, // Otherwise the fee is to transfer to an account. - Call => TransferFeeKind::Transfer, + Call | Terminate => TransferFeeKind::Transfer, }; TransferFeeToken { kind, @@ -664,11 +686,19 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( if to_balance.is_zero() && value < ctx.config.existential_deposit { Err("value too low to create account")? } + + // Only ext_terminate is allowed to bring the sender below the existential deposit + let required_balance = match cause { + Terminate => 0.into(), + _ => ctx.config.existential_deposit + }; + T::Currency::ensure_can_withdraw( transactor, value, WithdrawReason::Transfer.into(), - new_from_balance, + new_from_balance.checked_sub(&required_balance) + .ok_or("brings sender below existential deposit")?, )?; let new_to_balance = match to_balance.checked_add(&value) { @@ -740,6 +770,14 @@ where self.ctx.transfer(to.clone(), value, gas_meter) } + fn terminate( + &mut self, + beneficiary: &AccountIdOf, + gas_meter: &mut GasMeter, + ) -> Result<(), DispatchError> { + self.ctx.terminate(beneficiary, gas_meter) + } + fn call( &mut self, to: &T::AccountId, @@ -1321,7 +1359,7 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_balance(&ALICE, 1); + ctx.overlay.set_balance(&ALICE, 100); let result = ctx.instantiate( 1, @@ -1591,6 +1629,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_balance(&BOB, 100); ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); assert_matches!( @@ -1650,6 +1689,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_balance(&BOB, 100); ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); assert_matches!( @@ -1668,6 +1708,45 @@ mod tests { }); } + #[test] + fn termination_from_instantiate_fails() { + let vm = MockVm::new(); + + let mut loader = MockLoader::empty(); + + let terminate_ch = loader.insert(|mut ctx| { + ctx.ext.terminate(&ALICE, &mut ctx.gas_meter).unwrap(); + exec_success() + }); + + ExtBuilder::default() + .existential_deposit(15) + .build() + .execute_with(|| { + let cfg = Config::preload(); + let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); + ctx.overlay.set_balance(&ALICE, 1000); + + assert_matches!( + ctx.instantiate( + 100, + &mut GasMeter::::with_limit(10000, 1), + &terminate_ch, + vec![], + ), + Err(ExecError { + reason: DispatchError::Other("insufficient remaining balance"), + buffer + }) if buffer == Vec::::new() + ); + + assert_eq!( + &ctx.events(), + &[] + ); + }); + } + #[test] fn rent_allowance() { let vm = MockVm::new(); @@ -1683,7 +1762,7 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_balance(&ALICE, 1); + ctx.overlay.set_balance(&ALICE, 100); let result = ctx.instantiate( 1, diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs index f6b9e0e57e..d05d251b23 100644 --- a/substrate/frame/contracts/src/tests.rs +++ b/substrate/frame/contracts/src/tests.rs @@ -2088,13 +2088,107 @@ fn deploy_works_without_gas_price() { }); } +const CODE_DRAIN: &str = r#" +(module + (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) + (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_balance" (func $ext_balance)) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "memory" (memory 1 1)) + + (func $assert (param i32) + (block $ok + (br_if $ok + (get_local 0) + ) + (unreachable) + ) + ) + + (func (export "deploy")) + + (func (export "call") + ;; Send entire remaining balance to the 0 address. + (call $ext_balance) + + ;; Balance should be encoded as a u64. + (call $assert + (i32.eq + (call $ext_scratch_size) + (i32.const 8) + ) + ) + + ;; Read balance into memory. + (call $ext_scratch_read + (i32.const 8) ;; Pointer to write balance to + (i32.const 0) ;; Offset into scratch buffer + (i32.const 8) ;; Length of encoded balance + ) + + ;; Self-destruct by sending full balance to the 0 address. + (call $assert + (i32.eq + (call $ext_call + (i32.const 0) ;; Pointer to destination address + (i32.const 8) ;; Length of destination address + (i64.const 0) ;; How much gas to devote for the execution. 0 = all. + (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 8) ;; Length of the buffer with value to transfer + (i32.const 0) ;; Pointer to input data buffer address + (i32.const 0) ;; Length of input data buffer + ) + (i32.const 0) + ) + ) + ) +) +"#; + +#[test] +fn cannot_self_destruct_through_draning() { + let (wasm, code_hash) = compile_module::(CODE_DRAIN).unwrap(); + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + + // Instantiate the BOB contract. + assert_ok!(Contracts::instantiate( + Origin::signed(ALICE), + 100_000, + 100_000, + code_hash.into(), + vec![], + )); + + // Check that the BOB contract has been instantiated. + assert_matches!( + ContractInfoOf::::get(BOB), + Some(ContractInfo::Alive(_)) + ); + + // Call BOB with no input data, forcing it to run until out-of-balance + // and eventually trapping because below existential deposit. + assert_err!( + Contracts::call( + Origin::signed(ALICE), + BOB, + 0, + 100_000, + vec![], + ), + "contract trapped during execution" + ); + }); +} + const CODE_SELF_DESTRUCT: &str = r#" (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "ext_address" (func $ext_address)) - (import "env" "ext_balance" (func $ext_balance)) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) (import "env" "memory" (memory 1 1)) (func $assert (param i32) @@ -2148,81 +2242,21 @@ const CODE_SELF_DESTRUCT: &str = r#" ) ) ) - ) - - ;; Send entire remaining balance to the 0 address. - (call $ext_balance) - - ;; Balance should be encoded as a u64. - (call $assert - (i32.eq - (call $ext_scratch_size) - (i32.const 8) - ) - ) - - ;; Read balance into memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer to write balance to - (i32.const 0) ;; Offset into scratch buffer - (i32.const 8) ;; Length of encoded balance - ) - - ;; Self-destruct by sending full balance to the 0 address. - (call $assert - (i32.eq - (call $ext_call - (i32.const 0) ;; Pointer to destination address - (i32.const 8) ;; Length of destination address - (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 8) ;; Pointer to the buffer with value to transfer - (i32.const 8) ;; Length of the buffer with value to transfer - (i32.const 0) ;; Pointer to input data buffer address - (i32.const 0) ;; Length of input data buffer + (else + ;; Try to terminate and give balance to django. + (call $ext_terminate + (i32.const 32) ;; Pointer to beneficiary address + (i32.const 8) ;; Length of beneficiary address ) - (i32.const 0) + (unreachable) ;; ext_terminate never returns ) ) ) + ;; Address of django + (data (i32.const 32) "\04\00\00\00\00\00\00\00") ) "#; -#[test] -fn self_destruct_by_draining_balance() { - let (wasm, code_hash) = compile_module::(CODE_SELF_DESTRUCT).unwrap(); - ExtBuilder::default().existential_deposit(50).build().execute_with(|| { - Balances::deposit_creating(&ALICE, 1_000_000); - assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); - - // Instantiate the BOB contract. - assert_ok!(Contracts::instantiate( - Origin::signed(ALICE), - 100_000, - 100_000, - code_hash.into(), - vec![], - )); - - // Check that the BOB contract has been instantiated. - assert_matches!( - ContractInfoOf::::get(BOB), - Some(ContractInfo::Alive(_)) - ); - - // Call BOB with no input data, forcing it to self-destruct. - assert_ok!(Contracts::call( - Origin::signed(ALICE), - BOB, - 0, - 100_000, - vec![], - )); - - // Check that BOB is now dead. - assert!(ContractInfoOf::::get(BOB).is_none()); - }); -} - #[test] fn cannot_self_destruct_while_live() { let (wasm, code_hash) = compile_module::(CODE_SELF_DESTRUCT).unwrap(); @@ -2266,6 +2300,48 @@ fn cannot_self_destruct_while_live() { }); } +#[test] +fn self_destruct_works() { + let (wasm, code_hash) = compile_module::(CODE_SELF_DESTRUCT).unwrap(); + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + Balances::deposit_creating(&ALICE, 1_000_000); + assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); + + // Instantiate the BOB contract. + assert_ok!(Contracts::instantiate( + Origin::signed(ALICE), + 100_000, + 100_000, + code_hash.into(), + vec![], + )); + + // Check that the BOB contract has been instantiated. + assert_matches!( + ContractInfoOf::::get(BOB), + Some(ContractInfo::Alive(_)) + ); + + // Call BOB without input data which triggers termination. + assert_matches!( + Contracts::call( + Origin::signed(ALICE), + BOB, + 0, + 100_000, + vec![], + ), + Ok(()) + ); + + // Check that account is gone + assert!(ContractInfoOf::::get(BOB).is_none()); + + // check that the beneficiary (django) got remaining balance + assert_eq!(Balances::free_balance(DJANGO), 100_000); + }); +} + const CODE_DESTROY_AND_TRANSFER: &str = r#" (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) @@ -2524,8 +2600,8 @@ fn cannot_self_destruct_in_constructor() { Balances::deposit_creating(&ALICE, 1_000_000); assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm)); - // Fail to instantiate the BOB contract since its final balance is below existential - // deposit. + // Fail to instantiate the BOB because the call that is issued in the deploy + // function exhausts all balances which puts it below the existential deposit. assert_err!( Contracts::instantiate( Origin::signed(ALICE), @@ -2534,7 +2610,7 @@ fn cannot_self_destruct_in_constructor() { code_hash.into(), vec![], ), - "insufficient remaining balance" + "contract trapped during execution" ); }); } diff --git a/substrate/frame/contracts/src/wasm/mod.rs b/substrate/frame/contracts/src/wasm/mod.rs index 1b0401a562..8911fb72b6 100644 --- a/substrate/frame/contracts/src/wasm/mod.rs +++ b/substrate/frame/contracts/src/wasm/mod.rs @@ -182,6 +182,12 @@ mod tests { gas_left: u64, } + #[derive(Debug, PartialEq, Eq)] + struct TerminationEntry { + beneficiary: u64, + gas_left: u64, + } + #[derive(Debug, PartialEq, Eq)] struct TransferEntry { to: u64, @@ -195,6 +201,7 @@ mod tests { storage: HashMap>, rent_allowance: u64, instantiates: Vec, + terminations: Vec, transfers: Vec, dispatches: Vec, restores: Vec, @@ -242,7 +249,13 @@ mod tests { let address = self.next_account_id; self.next_account_id += 1; - Ok((address, ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() })) + Ok(( + address, + ExecReturnValue { + status: STATUS_SUCCESS, + data: Vec::new(), + }, + )) } fn transfer( &mut self, @@ -275,6 +288,17 @@ mod tests { // TODO: Add tests for different call outcomes. Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }) } + fn terminate( + &mut self, + beneficiary: &u64, + gas_meter: &mut GasMeter, + ) -> Result<(), DispatchError> { + self.terminations.push(TerminationEntry { + beneficiary: *beneficiary, + gas_left: gas_meter.gas_left(), + }); + Ok(()) + } fn note_dispatch_call(&mut self, call: Call) { self.dispatches.push(DispatchEntry(call)); } @@ -379,6 +403,13 @@ mod tests { ) -> Result<(), DispatchError> { (**self).transfer(to, value, gas_meter) } + fn terminate( + &mut self, + beneficiary: &u64, + gas_meter: &mut GasMeter, + ) -> Result<(), DispatchError> { + (**self).terminate(beneficiary, gas_meter) + } fn call( &mut self, to: &u64, @@ -649,6 +680,47 @@ mod tests { ); } + const CODE_TERMINATE: &str = r#" +(module + ;; ext_terminate( + ;; beneficiary_ptr: u32, + ;; beneficiary_len: u32, + ;; ) + (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) + (import "env" "memory" (memory 1 1)) + (func (export "call") + (call $ext_terminate + (i32.const 4) ;; Pointer to "beneficiary" address. + (i32.const 8) ;; Length of "beneficiary" address. + ) + ) + (func (export "deploy")) + + ;; Beneficiary AccountId to transfer the funds. + ;; Represented by u64 (8 bytes long) in little endian. + (data (i32.const 4) "\09\00\00\00\00\00\00\00") +) +"#; + + #[test] + fn contract_terminate() { + let mut mock_ext = MockExt::default(); + execute( + CODE_TERMINATE, + vec![], + &mut mock_ext, + &mut GasMeter::with_limit(50_000, 1), + ).unwrap(); + + assert_eq!( + &mock_ext.terminations, + &[TerminationEntry { + beneficiary: 0x09, + gas_left: 49989, + }] + ); + } + const CODE_TRANSFER_LIMITED_GAS: &str = r#" (module ;; ext_call( diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index 2460944687..c8c5e1e6a4 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -41,6 +41,9 @@ enum SpecialTrap { Return(Vec), /// Signals that trap was generated because the contract exhausted its gas limit. OutOfGas, + /// Signals that a trap was generated in response to a succesful call to the + /// `ext_terminate` host function. + Termination, } /// Can only be used for one call. @@ -83,14 +86,20 @@ pub(crate) fn to_execution_result( status: STATUS_SUCCESS, data, }) - } + }, + Some(SpecialTrap::Termination) => { + return Ok(ExecReturnValue { + status: STATUS_SUCCESS, + data: Vec::new(), + }) + }, Some(SpecialTrap::OutOfGas) => { return Err(ExecError { reason: "ran out of gas during contract execution".into(), buffer: runtime.scratch_buf, }) - } - _ => (), + }, + None => (), } // Check the exact type of the error. @@ -443,6 +452,9 @@ define_env!(Env, , // changes made by the called contract are reverted. The scratch buffer is filled with the // output data returned by the called contract, even in the case of a failure status. // + // This call fails if it would bring the calling contract below the existential deposit. + // In order to destroy a contract `ext_terminate` must be used. + // // If the contract traps during execution or otherwise fails to complete successfully, then // this function clears the scratch buffer and returns 0x0100. As with a failure status, any // state changes made by the called contract are reverted. @@ -523,6 +535,9 @@ define_env!(Env, , // of the newly instantiated contract. In the case of a failure status, the scratch buffer is // cleared. // + // This call fails if it would bring the calling contract below the existential deposit. + // In order to destroy a contract `ext_terminate` must be used. + // // If the contract traps during execution or otherwise fails to complete successfully, then // this function clears the scratch buffer and returns 0x0100. As with a failure status, any // state changes made by the called contract are reverted. @@ -601,6 +616,30 @@ define_env!(Env, , } }, + // Remove the calling account and transfer remaining balance. + // + // This function never returns. Either the termination was successful and the + // execution of the destroyed contract is halted. Or it failed during the termination + // which is considered fatal and results in a trap + rollback. + // + // - beneficiary_ptr: a pointer to the address of the beneficiary account where all + // where all remaining funds of the caller are transfered. + // Should be decodable as an `T::AccountId`. Traps otherwise. + // - beneficiary_len: length of the address buffer. + ext_terminate( + ctx, + beneficiary_ptr: u32, + beneficiary_len: u32 + ) => { + let beneficiary: <::T as frame_system::Trait>::AccountId = + read_sandbox_memory_as(ctx, beneficiary_ptr, beneficiary_len)?; + + if let Ok(_) = ctx.ext.terminate(&beneficiary, ctx.gas_meter) { + ctx.special_trap = Some(SpecialTrap::Termination); + } + Err(sp_sandbox::HostError) + }, + // Save a data buffer as a result of the execution, terminate the execution and return a // successful result to the caller. //