fix selfdestruct account removal

modules/evm/src/lib.rs

@@ -1290,11 +1290,11 @@ impl<T: Config> Pallet<T> {
 	}
 
 	/// Remove an account if its empty.
-	/// Keep the non-zero nonce exists.
+	/// NOTE: If the nonce is non-zero, it cannot be deleted to prevent the user from failing to
+	/// create a contract due to nonce reset
 	pub fn remove_account_if_empty(address: &H160) {
 		if Self::is_account_empty(address) {
-			let res = Self::remove_account(address);
-			debug_assert!(res.is_ok());
+			Self::remove_account(address);
 		}
 	}
 
@@ -1341,8 +1341,8 @@ impl<T: Config> Pallet<T> {
 	}
 
 	/// Removes an account from Accounts and AccountStorages.
-	/// Only used in `remove_account_if_empty`
-	fn remove_account(address: &EvmAddress) -> DispatchResult {
+	/// NOTE: It will reset account nonce.
+	fn remove_account(address: &EvmAddress) {
 		// Deref code, and remove it if ref count is zero.
 		Accounts::<T>::mutate_exists(address, |maybe_account| {
 			if let Some(account) = maybe_account {
@@ -1371,8 +1371,6 @@ impl<T: Config> Pallet<T> {
 				*maybe_account = None;
 			}
 		});
-
-		Ok(())
 	}
 
 	/// Create an account.
@@ -1795,7 +1793,14 @@ impl<T: Config> Pallet<T> {
 		)?;
 		debug_assert!(val.is_zero());
 
-		T::TransferAll::transfer_all(&contract_acc, &maintainer_acc)?;
+		// transfer to treasury if maintainer is contract itself
+		let dest = if contract_acc == maintainer_acc {
+			T::TreasuryAccount::get()
+		} else {
+			maintainer_acc
+		};
+
+		T::TransferAll::transfer_all(&contract_acc, &dest)?;
 
 		Ok(())
 	}
@@ -2102,7 +2107,7 @@ impl<T: Config> DispatchableTask for EvmTask<T> {
 					let result = Pallet::<T>::refund_storage(&caller, &contract, &maintainer);
 
 					// Remove account after all of the storages are cleared.
-					Pallet::<T>::remove_account_if_empty(&contract);
+					Pallet::<T>::remove_account(&contract);
 
 					TaskResult {
 						result,

modules/evm/src/tests.rs

@@ -1449,7 +1449,7 @@ fn should_selfdestruct() {
 
 		assert_eq!(System::providers(&contract_account_id), 0);
 		assert!(!System::account_exists(&contract_account_id));
-		assert!(Accounts::<Runtime>::contains_key(&contract_address));
+		assert!(!Accounts::<Runtime>::contains_key(&contract_address));
 		assert_eq!(AccountStorages::<Runtime>::iter_prefix(&contract_address).count(), 0);
 	});
 }
@@ -2036,14 +2036,6 @@ fn convert_decimals_should_not_work() {
 	});
 }
 
-#[test]
-fn remove_empty_account_works() {
-	new_test_ext().execute_with(|| {
-		let address = H160::from([1; 20]);
-		assert_ok!(Pallet::<Runtime>::remove_account(&address));
-	});
-}
-
 #[test]
 #[should_panic(expected = "removed account while is still linked to contract info")]
 fn remove_account_with_provides_should_panic() {
@@ -2085,7 +2077,7 @@ fn remove_account_works() {
 				contract_info: None,
 			},
 		);
-		assert_ok!(Pallet::<Runtime>::remove_account(&address));
+		Pallet::<Runtime>::remove_account(&address);
 		assert_eq!(Accounts::<Runtime>::contains_key(&address), false);
 	});
 }

runtime/integration-tests/src/evm.rs

@@ -659,20 +659,23 @@ fn should_not_kill_contract_on_transfer_all_tokens() {
 			assert_eq!(System::providers(&contract_account_id), 2);
 			assert!(EVM::accounts(contract).is_some());
 
+			// call kill
 			assert_ok!(EVM::call(RuntimeOrigin::signed(alice()), contract.clone(), hex_literal::hex!("41c0e1b5").to_vec(), 0, 1000000000, 100000, vec![]));
 
 			#[cfg(feature = "with-ethereum-compatibility")]
 			assert_eq!(System::providers(&contract_account_id), 0);
 			#[cfg(not(feature = "with-ethereum-compatibility"))]
 			assert_eq!(System::providers(&contract_account_id), 1);
 
+			// contract account will hang around until storage is cleared
 			assert_eq!(EVM::accounts(contract), Some(module_evm::AccountInfo{ nonce: 1, contract_info: None}));
 
 			// use IdleScheduler to remove contract
 			run_to_block(System::block_number() + 1);
 
 			assert_eq!(System::providers(&contract_account_id), 0);
-			assert_eq!(EVM::accounts(contract), Some(module_evm::AccountInfo{ nonce: 1, contract_info: None}));
+			// contract account should be gone
+			assert_eq!(EVM::accounts(contract), None);
 
 			// should be gone
 			assert!(!System::account_exists(&contract_account_id));