contracts: New contract events + unconfusions (#4685)

* contracts: during execution -> contract trapped during execution

This message confused many people so we are improving it to make clear what happened.

* contracts: rename Event::Contract -> Event::ContractExecution

* contracts: fix tests after ContractExecution renaming

* contracts: Add Evicted and Restored events

* fix doc comment

* wrap to not go over (soft) 100 column line limit

* add event deposit for eventual eviction upon pay_rent

* contracts: adjust tests for the new events

* emit Evicted event immediately and add tombstone flag bool

frame/contracts/src/exec.rs

@@ -782,7 +782,7 @@ where
 	fn deposit_event(&mut self, topics: Vec<T::Hash>, data: Vec<u8>) {
 		self.ctx.deferred.push(DeferredAction::DepositEvent {
 			topics,
-			event: RawEvent::Contract(self.ctx.self_account.clone(), data),
+			event: RawEvent::ContractExecution(self.ctx.self_account.clone(), data),
 		});
 	}
 

frame/contracts/src/lib.rs

@@ -786,7 +786,12 @@ impl<T: Trait> Module<T> {
 					rent_allowance,
 					delta,
 				} => {
-					let _result = Self::restore_to(donor, dest, code_hash, rent_allowance, delta);
+					let result = Self::restore_to(
+						donor.clone(), dest.clone(), code_hash.clone(), rent_allowance.clone(), delta
+					);
+					Self::deposit_event(
+						RawEvent::Restored(donor, dest, code_hash, rent_allowance, result.is_ok())
+					);
 				}
 			}
 		});
@@ -896,6 +901,25 @@ decl_event! {
 		/// Contract deployed by address at the specified address.
 		Instantiated(AccountId, AccountId),
 
+		/// Contract has been evicted and is now in tombstone state.
+		///
+		/// # Params
+		///
+		/// - `contract`: `AccountId`: The account ID of the evicted contract.
+		/// - `tombstone`: `bool`: True if the evicted contract left behind a tombstone.
+		Evicted(AccountId, bool),
+
+		/// Restoration for a contract has been initiated.
+		///
+		/// # 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),
+
 		/// Code with the specified hash has been stored.
 		CodeStored(Hash),
 
@@ -906,8 +930,8 @@ decl_event! {
 		/// successful execution or not.
 		Dispatched(AccountId, bool),
 
-		/// An event from contract of account.
-		Contract(AccountId, Vec<u8>),
+		/// An event deposited upon execution of a contract from the account.
+		ContractExecution(AccountId, Vec<u8>),
 	}
 }
 

frame/contracts/src/rent.rs

@@ -14,7 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate. If not, see <http://www.gnu.org/licenses/>.
 
-use crate::{BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo, Trait, AliveContractInfo};
+use crate::{Module, RawEvent, BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo,
+	Trait, AliveContractInfo};
 use sp_runtime::traits::{Bounded, CheckedDiv, CheckedMul, Saturating, Zero,
 	SaturatedConversion};
 use frame_support::traits::{Currency, ExistenceRequirement, Get, WithdrawReason, OnUnbalanced};
@@ -101,6 +102,7 @@ fn try_evict_or_and_pay_rent<T: Trait>(
 		// The contract cannot afford to leave a tombstone, so remove the contract info altogether.
 		<ContractInfoOf<T>>::remove(account);
 		child::kill_storage(&contract.trie_id, contract.child_trie_unique_id());
+		<Module<T>>::deposit_event(RawEvent::Evicted(account.clone(), false));
 		return (RentOutcome::Evicted, None);
 	}
 
@@ -160,6 +162,8 @@ fn try_evict_or_and_pay_rent<T: Trait>(
 
 		child::kill_storage(&contract.trie_id, contract.child_trie_unique_id());
 
+		<Module<T>>::deposit_event(RawEvent::Evicted(account.clone(), true));
+
 		return (RentOutcome::Evicted, Some(tombstone_info));
 	}
 

frame/contracts/src/tests.rs

@@ -453,7 +453,7 @@ fn instantiate_and_call_and_deposit_event() {
 			},
 			EventRecord {
 				phase: Phase::ApplyExtrinsic(0),
-				event: MetaEvent::contract(RawEvent::Contract(BOB, vec![1, 2, 3, 4])),
+				event: MetaEvent::contract(RawEvent::ContractExecution(BOB, vec![1, 2, 3, 4])),
 				topics: vec![],
 			},
 			EventRecord {
@@ -650,7 +650,7 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() {
 				100_000,
 				vec![],
 			),
-			"during execution"
+			"contract trapped during execution"
 		);
 		assert_eq!(System::events(), vec![
 			EventRecord {
@@ -1139,8 +1139,16 @@ fn call_removed_contract() {
 			Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()),
 			"contract has been evicted"
 		);
+		// Calling a contract that is about to evict shall emit an event.
+		assert_eq!(System::events(), vec![
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(0),
+				event: MetaEvent::contract(RawEvent::Evicted(BOB, true)),
+				topics: vec![],
+			},
+		]);
 
- 		// Subsequent contract calls should also fail.
+		// Subsequent contract calls should also fail.
 		assert_err!(
 			Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()),
 			"contract has been evicted"
@@ -1367,13 +1375,25 @@ 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!(
 			Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()),
 			"contract has been evicted"
 		);
 		assert!(ContractInfoOf::<Test>::get(BOB).unwrap().get_tombstone().is_some());
+		assert_eq!(System::events(), vec![
+			EventRecord {
+				phase: Phase::ApplyExtrinsic(0),
+				event: MetaEvent::contract(
+					RawEvent::Evicted(BOB.clone(), true)
+				),
+				topics: vec![],
+			},
+		]);
 
 		/// Create another account with the address `DJANGO` with `CODE_RESTORATION`.
 		///
@@ -1416,6 +1436,60 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 			assert_eq!(django_contract.storage_size, 16);
 			assert_eq!(django_contract.trie_id, django_trie_id);
 			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::ApplyExtrinsic(0),
+							event: MetaEvent::contract(
+								RawEvent::Restored(DJANGO, BOB, bob_code_hash, 50, false)
+							),
+							topics: vec![],
+						},
+					]);
+				}
+				(_, true) => {
+					assert_eq!(System::events(), vec![
+						EventRecord {
+							phase: Phase::ApplyExtrinsic(0),
+							event: MetaEvent::contract(RawEvent::Evicted(BOB, true)),
+							topics: vec![],
+						},
+						EventRecord {
+							phase: Phase::ApplyExtrinsic(0),
+							event: MetaEvent::balances(pallet_balances::RawEvent::NewAccount(CHARLIE, 1_000_000)),
+							topics: vec![],
+						},
+						EventRecord {
+							phase: Phase::ApplyExtrinsic(0),
+							event: MetaEvent::balances(pallet_balances::RawEvent::NewAccount(DJANGO, 30_000)),
+							topics: vec![],
+						},
+						EventRecord {
+							phase: Phase::ApplyExtrinsic(0),
+							event: MetaEvent::contract(RawEvent::Transfer(CHARLIE, DJANGO, 30_000)),
+							topics: vec![],
+						},
+						EventRecord {
+							phase: Phase::ApplyExtrinsic(0),
+							event: MetaEvent::contract(RawEvent::Instantiated(CHARLIE, DJANGO)),
+							topics: vec![],
+						},
+						EventRecord {
+							phase: Phase::ApplyExtrinsic(0),
+							event: MetaEvent::contract(RawEvent::Restored(
+								DJANGO,
+								BOB,
+								bob_code_hash,
+								50,
+								false,
+							)),
+							topics: vec![],
+						},
+					]);
+				}
+				_ => unreachable!(),
+			}
 		} else {
 			// Here we expect that the restoration is succeeded. Check that the restoration
 			// contract `DJANGO` ceased to exist and that `BOB` returned back.
@@ -1427,6 +1501,20 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 			assert_eq!(bob_contract.trie_id, django_trie_id);
 			assert_eq!(bob_contract.deduct_block, System::block_number());
 			assert!(ContractInfoOf::<Test>::get(DJANGO).is_none());
+			assert_eq!(System::events(), vec![
+				EventRecord {
+					phase: Phase::ApplyExtrinsic(0),
+					event: MetaEvent::balances(balances::RawEvent::ReapedAccount(DJANGO, 0)),
+					topics: vec![],
+				},
+				EventRecord {
+					phase: Phase::ApplyExtrinsic(0),
+					event: MetaEvent::contract(
+						RawEvent::Restored(DJANGO, BOB, bob_contract.code_hash, 50, true)
+					),
+					topics: vec![],
+				},
+			]);
 		}
 	});
 }
@@ -1533,7 +1621,7 @@ fn storage_max_value_limit() {
 				100_000,
 				Encode::encode(&(self::MaxValueSize::get() + 1)),
 			),
-			"during execution"
+			"contract trapped during execution"
 		);
 	});
 }
@@ -2056,7 +2144,7 @@ fn cannot_self_destruct_while_live() {
 				100_000,
 				vec![0],
 			),
-			"during execution"
+			"contract trapped during execution"
 		);
 
 		// Check that BOB is still alive.

frame/contracts/src/wasm/mod.rs

@@ -1430,7 +1430,9 @@ mod tests {
 				MockExt::default(),
 				&mut gas_meter
 			),
-			Err(ExecError { reason: DispatchError::Other("during execution"), buffer: _ })
+			Err(ExecError {
+				reason: DispatchError::Other("contract trapped during execution"), buffer: _
+			})
 		);
 	}
 
@@ -1472,7 +1474,7 @@ mod tests {
 				MockExt::default(),
 				&mut gas_meter
 			),
-			Err(ExecError { reason: DispatchError::Other("during execution"), buffer: _ })
+			Err(ExecError { reason: DispatchError::Other("contract trapped during execution"), buffer: _ })
 		);
 	}
 

frame/contracts/src/wasm/runtime.rs

@@ -109,7 +109,7 @@ pub(crate) fn to_execution_result<E: Ext>(
 			Err(ExecError { reason: "validation error".into(), buffer: runtime.scratch_buf }),
 		// Any other kind of a trap should result in a failure.
 		Err(sp_sandbox::Error::Execution) | Err(sp_sandbox::Error::OutOfBounds) =>
-			Err(ExecError { reason: "during execution".into(), buffer: runtime.scratch_buf }),
+			Err(ExecError { reason: "contract trapped during execution".into(), buffer: runtime.scratch_buf }),
 	}
 }