Make restoration diverging.

frame/contracts/fixtures/restoration.wat

@@ -3,28 +3,25 @@
 	(import "env" "ext_restore_to"
 		(func $ext_restore_to
 			(param i32 i32 i32 i32 i32 i32 i32 i32)
-			(result i32)
 		)
 	)
 	(import "env" "memory" (memory 1 1))
 
 	(func (export "call")
-		(drop
-			(call $ext_restore_to
-				;; Pointer and length of the encoded dest buffer.
-				(i32.const 256)
-				(i32.const 8)
-				;; Pointer and length of the encoded code hash buffer
-				(i32.const 264)
-				(i32.const 32)
-				;; Pointer and length of the encoded rent_allowance buffer
-				(i32.const 296)
-				(i32.const 8)
-				;; Pointer and number of items in the delta buffer.
-				;; This buffer specifies multiple keys for removal before restoration.
-				(i32.const 100)
-				(i32.const 1)
-			)
+		(call $ext_restore_to
+			;; Pointer and length of the encoded dest buffer.
+			(i32.const 256)
+			(i32.const 8)
+			;; Pointer and length of the encoded code hash buffer
+			(i32.const 264)
+			(i32.const 32)
+			;; Pointer and length of the encoded rent_allowance buffer
+			(i32.const 296)
+			(i32.const 8)
+			;; Pointer and number of items in the delta buffer.
+			;; This buffer specifies multiple keys for removal before restoration.
+			(i32.const 100)
+			(i32.const 1)
 		)
 	)
 	(func (export "deploy")

frame/contracts/src/exec.rs

@@ -129,6 +129,9 @@ pub trait Ext {
 	) -> Result<(), DispatchError>;
 
 	/// Transfer all funds to `beneficiary` and delete the contract.
+	///
+	/// Since this function removes the self contract eagerly, no further actions should be performed
+	/// on this `Ext` instance.
 	fn terminate(
 		&mut self,
 		beneficiary: &AccountIdOf<Self::T>,
@@ -148,6 +151,9 @@ pub trait Ext {
 	fn note_dispatch_call(&mut self, call: CallOf<Self::T>);
 
 	/// Restores the given destination contract sacrificing the current one.
+	///
+	/// Since this function removes the self contract eagerly, no further actions should be performed
+	/// on this `Ext` instance.
 	fn restore_to(
 		&mut self,
 		dest: AccountIdOf<Self::T>,
@@ -681,6 +687,17 @@ fn transfer<'a, T: Trait, V: Vm<T>, L: Loader<T>>(
 	Ok(())
 }
 
+/// A context that is active within a call.
+///
+/// This context has some invariants that must be held at all times. Specifically:
+///`ctx` always points to a context of an alive contract. That implies that it has an existent
+/// `self_trie_id`.
+///
+/// Be advised that there are brief time spans where these invariants could be invalidated.
+/// For example, when a contract requests self-termination the contract is removed eagerly. That
+/// implies that the control won't be returned to the contract anymore, but there is still some code
+/// on the path of the return from that call context. Therefore, take must be taken in these
+/// situations.
 struct CallContext<'a, 'b: 'a, T: Trait + 'b, V: Vm<T> + 'b, L: Loader<T>> {
 	ctx: &'a mut ExecutionContext<'b, T, V, L>,
 	caller: T::AccountId,
@@ -778,17 +795,17 @@ where
 			rent_allowance,
 			delta,
 		);
-		// Deposit an event accordingly regardless if it succeeded.
-		deposit_event::<Self::T>(
-			vec![],
-			RawEvent::Restored(
-				self.ctx.self_account.clone(),
-				dest,
-				code_hash,
-				rent_allowance,
-				result.is_ok(),
-			),
-		);
+		if let Ok(_) = result {
+			deposit_event::<Self::T>(
+				vec![],
+				RawEvent::Restored(
+					self.ctx.self_account.clone(),
+					dest,
+					code_hash,
+					rent_allowance,
+				),
+			);
+		}
 		result
 	}
 
@@ -832,7 +849,13 @@ where
 	}
 
 	fn set_rent_allowance(&mut self, rent_allowance: BalanceOf<T>) {
-		storage::set_rent_allowance::<T>(&self.ctx.self_account, rent_allowance);
+		match storage::set_rent_allowance::<T>(&self.ctx.self_account, rent_allowance) {
+			Ok(()) => {},
+			Err(storage::AllowanceSetError) => {
+				panic!("`self_account` points to an alive contract within the `CallContext`;
+					set_rent_allowance cannot return `Err`; qed");
+			}
+		}
 	}
 
 	fn rent_allowance(&self) -> BalanceOf<T> {

frame/contracts/src/lib.rs

@@ -707,16 +707,15 @@ decl_event! {
 		/// - `tombstone`: `bool`: True if the evicted contract left behind a tombstone.
 		Evicted(AccountId, bool),
 
-		/// Restoration for a contract has been initiated.
+		/// Restoration for a contract has been successful.
 		///
 		/// # Params
 		///
 		/// - `donor`: `AccountId`: Account ID of the restoring contract
 		/// - `dest`: `AccountId`: Account ID of the restored contract
 		/// - `code_hash`: `Hash`: Code hash of the restored contract
 		/// - `rent_allowance: `Balance`: Rent allowance of the restored contract
-		/// - `success`: `bool`: True if the restoration was successful
-		Restored(AccountId, AccountId, Hash, Balance, bool),
+		Restored(AccountId, AccountId, Hash, Balance),
 
 		/// Code with the specified hash has been stored.
 		CodeStored(Hash),

frame/contracts/src/storage.rs

@@ -105,14 +105,22 @@ pub fn rent_allowance<T: Trait>(account: &AccountIdOf<T>) -> Option<BalanceOf<T>
 	<ContractInfoOf<T>>::get(account).and_then(|i| i.as_alive().map(|i| i.rent_allowance))
 }
 
-pub fn set_rent_allowance<T: Trait>(account: &AccountIdOf<T>, rent_allowance: BalanceOf<T>) {
-	<ContractInfoOf<T>>::mutate(account, |maybe_contract_info| {
-		match maybe_contract_info {
-			Some(ContractInfo::Alive(ref mut alive_info)) => {
-				alive_info.rent_allowance = rent_allowance
-			}
-			_ => panic!(), // TODO: Justify this.
+/// An error returned if `set_rent_allowance` was called on an account which doesn't have a contract.
+pub struct AllowanceSetError;
+
+/// Set the rent allowance for the contract given by the account id.
+///
+/// Returns `Err` if the contract is not alive.
+pub fn set_rent_allowance<T: Trait>(
+	account: &AccountIdOf<T>,
+	rent_allowance: BalanceOf<T>,
+) -> Result<(), AllowanceSetError> {
+	<ContractInfoOf<T>>::mutate(account, |maybe_contract_info| match maybe_contract_info {
+		Some(ContractInfo::Alive(ref mut alive_info)) => {
+			alive_info.rent_allowance = rent_allowance;
+			Ok(())
 		}
+		_ => Err(AllowanceSetError),
 	})
 }
 

frame/contracts/src/tests.rs

@@ -1352,9 +1352,6 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 			// Advance 4 blocks, to the 5th.
 			initialize_block(5);
 
-			// Preserve `BOB`'s code hash for later introspection.
-			let bob_code_hash = ContractInfoOf::<Test>::get(BOB).unwrap()
-				.get_alive().unwrap().code_hash;
 			// 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_err_ignore_postinfo!(
@@ -1396,17 +1393,25 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 
 			// Perform a call to `DJANGO`. This should either perform restoration successfully or
 			// fail depending on the test parameters.
-			assert_ok!(Contracts::call(
-				Origin::signed(ALICE),
-				DJANGO,
-				0,
-				GAS_LIMIT,
-				vec![],
-			));
+			let perform_the_restoration = || {
+				Contracts::call(
+					Origin::signed(ALICE),
+					DJANGO,
+					0,
+					GAS_LIMIT,
+					vec![],
+				)
+			};
 
 			if test_different_storage || test_restore_to_with_dirty_storage {
 				// Parametrization of the test imply restoration failure. Check that `DJANGO` aka
 				// restoration contract is still in place and also that `BOB` doesn't exist.
+
+				assert_err_ignore_postinfo!(
+					perform_the_restoration(),
+					"contract trapped during execution"
+				);
+
 				assert!(ContractInfoOf::<Test>::get(BOB).unwrap().get_tombstone().is_some());
 				let django_contract = ContractInfoOf::<Test>::get(DJANGO).unwrap()
 					.get_alive().unwrap();
@@ -1415,15 +1420,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 				assert_eq!(django_contract.deduct_block, System::block_number());
 				match (test_different_storage, test_restore_to_with_dirty_storage) {
 					(true, false) => {
-						assert_eq!(System::events(), vec![
-							EventRecord {
-								phase: Phase::Initialization,
-								event: MetaEvent::contracts(
-									RawEvent::Restored(DJANGO, BOB, bob_code_hash, 50, false)
-								),
-								topics: vec![],
-							},
-						]);
+						assert_eq!(System::events(), vec![]);
 					}
 					(_, true) => {
 						pretty_assertions::assert_eq!(System::events(), vec![
@@ -1464,22 +1461,13 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 								event: MetaEvent::contracts(RawEvent::Instantiated(CHARLIE, DJANGO)),
 								topics: vec![],
 							},
-							EventRecord {
-								phase: Phase::Initialization,
-								event: MetaEvent::contracts(RawEvent::Restored(
-									DJANGO,
-									BOB,
-									bob_code_hash,
-									50,
-									false,
-								)),
-								topics: vec![],
-							},
 						]);
 					}
 					_ => unreachable!(),
 				}
 			} else {
+				assert_ok!(perform_the_restoration());
+
 				// Here we expect that the restoration is succeeded. Check that the restoration
 				// contract `DJANGO` ceased to exist and that `BOB` returned back.
 				println!("{:?}", ContractInfoOf::<Test>::get(BOB));
@@ -1499,7 +1487,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 					EventRecord {
 						phase: Phase::Initialization,
 						event: MetaEvent::contracts(
-							RawEvent::Restored(DJANGO, BOB, bob_contract.code_hash, 50, true)
+							RawEvent::Restored(DJANGO, BOB, bob_contract.code_hash, 50)
 						),
 						topics: vec![],
 					},

frame/contracts/src/wasm/runtime.rs

@@ -51,6 +51,8 @@ enum SpecialTrap {
 	/// Signals that a trap was generated in response to a succesful call to the
 	/// `ext_terminate` host function.
 	Termination,
+	/// Signals that a trap was generated because of a successful restoration.
+	Restoration,
 }
 
 /// Can only be used for one call.
@@ -100,6 +102,12 @@ pub(crate) fn to_execution_result<E: Ext>(
 				data: Vec::new(),
 			})
 		},
+		Some(SpecialTrap::Restoration) => {
+			return Ok(ExecReturnValue {
+				status: STATUS_SUCCESS,
+				data: Vec::new(),
+			})
+		}
 		Some(SpecialTrap::OutOfGas) => {
 			return Err(ExecError {
 				reason: "ran out of gas during contract execution".into(),
@@ -830,7 +838,7 @@ define_env!(Env, <E: Ext>,
 		rent_allowance_len: u32,
 		delta_ptr: u32,
 		delta_count: u32
-	) -> u32 => {
+	) => {
 		let dest: <<E as Ext>::T as frame_system::Trait>::AccountId =
 			read_sandbox_memory_as(ctx, dest_ptr, dest_len)?;
 		let code_hash: CodeHash<<E as Ext>::T> =
@@ -858,15 +866,15 @@ define_env!(Env, <E: Ext>,
 			delta
 		};
 
-		match ctx.ext.restore_to(
+		if let Ok(()) = ctx.ext.restore_to(
 			dest,
 			code_hash,
 			rent_allowance,
 			delta,
 		) {
-			Ok(_) => Ok(0),
-			Err(_) => Ok(1),
+			ctx.special_trap = Some(SpecialTrap::Restoration);
 		}
+		Err(sp_sandbox::HostError)
 	},
 
 	// Returns the size of the scratch buffer.