Rewrite the storage clean precompile

frame/evm/precompile/storage-cleaner/Cargo.toml

@@ -8,6 +8,7 @@ edition = { workspace = true }
 repository = { workspace = true }
 
 [dependencies]
+log = { workspace = true }
 scale-codec = { package = "parity-scale-codec", workspace = true }
 # Substrate
 frame-support = { workspace = true }
@@ -37,6 +38,7 @@ precompile-utils = { workspace = true, features = ["std", "testing"] }
 [features]
 default = ["std"]
 std = [
+	"log/std",
 	"scale-codec/std",
 	# Substrate
 	"frame-support/std",

frame/evm/precompile/storage-cleaner/src/lib.rs

@@ -23,7 +23,6 @@ extern crate alloc;
 
 use alloc::vec::Vec;
 use core::marker::PhantomData;
-use fp_evm::{PrecompileFailure, ACCOUNT_BASIC_PROOF_SIZE, ACCOUNT_STORAGE_PROOF_SIZE};
 use pallet_evm::AddressMapping;
 use precompile_utils::{prelude::*, EvmResult};
 use sp_core::H160;
@@ -36,8 +35,6 @@ mod tests;
 
 pub const ARRAY_LIMIT: u32 = 1_000;
 type GetArrayLimit = ConstU32<ARRAY_LIMIT>;
-// Storage key for suicided contracts: Blake2_128(16) + Key (H160(20))
-pub const SUICIDED_STORAGE_KEY: u64 = 36;
 
 #[derive(Debug, Clone)]
 pub struct StorageCleanerPrecompile<Runtime>(PhantomData<Runtime>);
@@ -48,37 +45,20 @@ where
 	Runtime: pallet_evm::Config,
 {
 	/// Clear Storage entries of smart contracts that has been marked as suicided (self-destructed). It takes a list of
-	/// addresses and a limit as input. The limit is used to prevent the function from consuming too much gas. The
-	/// maximum number of storage entries that can be removed is limit - 1.
+	/// addresses and a limit as input. The limit is used to prevent the function from consuming too much gas.
 	#[precompile::public("clearSuicidedStorage(address[],uint64)")]
 	fn clear_suicided_storage(
-		handle: &mut impl PrecompileHandle,
+		_handle: &mut impl PrecompileHandle,
 		addresses: BoundedVec<Address, GetArrayLimit>,
 		limit: u64,
 	) -> EvmResult {
-		let addresses: Vec<_> = addresses.into();
-		let nb_addresses = addresses.len() as u64;
 		if limit == 0 {
 			return Err(revert("Limit should be greater than zero"));
 		}
-
-		Self::record_max_cost(handle, nb_addresses, limit)?;
-		let result = Self::clear_suicided_storage_inner(addresses, limit - 1)?;
-		Self::refund_cost(handle, result, nb_addresses, limit);
-
-		Ok(())
-	}
-
-	/// This function iterates over the addresses, checks if each address is marked as suicided, and then deletes the storage
-	/// entries associated with that address. If there are no remaining entries, we clear the suicided contract by calling the
-	/// `clear_suicided_contract` function.
-	fn clear_suicided_storage_inner(
-		addresses: Vec<Address>,
-		limit: u64,
-	) -> Result<RemovalResult, PrecompileFailure> {
 		let mut deleted_entries = 0u64;
 		let mut deleted_contracts = 0u64;
 
+		let addresses: Vec<_> = addresses.into();
 		for Address(address) in addresses {
 			if !pallet_evm::Pallet::<Runtime>::is_account_suicided(&address) {
 				return Err(revert(alloc::format!("NotSuicided: {}", address)));
@@ -89,7 +69,7 @@ where
 				.count();
 			deleted_entries = deleted_entries.saturating_add(deleted as u64);
 
-			// Check if the storage of this contract has been completly removed
+			// Check if the storage of this contract has been completely removed
 			if pallet_evm::AccountStorages::<Runtime>::iter_key_prefix(address)
 				.next()
 				.is_none()
@@ -102,96 +82,11 @@ where
 				break;
 			}
 		}
+		log::info!(target: "evm", "The storage cleaner removed {} entries and {} contracts", deleted_entries, deleted_contracts);
 
-		Ok(RemovalResult {
-			deleted_entries,
-			deleted_contracts,
-		})
-	}
-
-	/// Record the maximum cost (Worst case Scenario) of the clear_suicided_storage function.
-	fn record_max_cost(
-		handle: &mut impl PrecompileHandle,
-		nb_addresses: u64,
-		limit: u64,
-	) -> EvmResult {
-		let read_cost = RuntimeHelper::<Runtime>::db_read_gas_cost();
-		let write_cost = RuntimeHelper::<Runtime>::db_write_gas_cost();
-		let ref_time = 0u64
-			// EVM:: Suicided (reads = nb_addresses)
-			.saturating_add(read_cost.saturating_mul(nb_addresses))
-			// EVM:: Suicided (writes = nb_addresses)
-			.saturating_add(write_cost.saturating_mul(nb_addresses))
-			// System: AccountInfo (reads = nb_addresses) for decrementing sufficients
-			.saturating_add(read_cost.saturating_mul(nb_addresses))
-			// System: AccountInfo (writes = nb_addresses) for decrementing sufficients
-			.saturating_add(write_cost.saturating_mul(nb_addresses))
-			// EVM: AccountStorage (reads = limit)
-			.saturating_add(read_cost.saturating_mul(limit))
-			// EVM: AccountStorage (writes = limit)
-			.saturating_add(write_cost.saturating_mul(limit));
-
-		let proof_size = 0u64
-			// Proof: EVM::Suicided (SUICIDED_STORAGE_KEY) * nb_addresses
-			.saturating_add(SUICIDED_STORAGE_KEY.saturating_mul(nb_addresses))
-			// Proof: EVM::AccountStorage (ACCOUNT_BASIC_PROOF_SIZE) * limit
-			.saturating_add(ACCOUNT_STORAGE_PROOF_SIZE.saturating_mul(limit))
-			// Proof: System::AccountInfo (ACCOUNT_BASIC_PROOF_SIZE) * nb_addresses
-			.saturating_add(ACCOUNT_BASIC_PROOF_SIZE.saturating_mul(nb_addresses));
-
-		handle.record_external_cost(Some(ref_time), Some(proof_size), None)?;
 		Ok(())
 	}
 
-	/// Refund the additional cost recorded for the clear_suicided_storage function.
-	fn refund_cost(
-		handle: &mut impl PrecompileHandle,
-		result: RemovalResult,
-		nb_addresses: u64,
-		limit: u64,
-	) {
-		let read_cost = RuntimeHelper::<Runtime>::db_read_gas_cost();
-		let write_cost = RuntimeHelper::<Runtime>::db_write_gas_cost();
-
-		let extra_entries = limit.saturating_sub(result.deleted_entries);
-		let extra_contracts = nb_addresses.saturating_sub(result.deleted_contracts);
-
-		let mut ref_time = 0u64;
-		let mut proof_size = 0u64;
-
-		// Refund the cost of the remaining entries
-		if extra_entries > 0 {
-			ref_time = ref_time
-				// EVM:: AccountStorage (reads = extra_entries)
-				.saturating_add(read_cost.saturating_mul(extra_entries))
-				// EVM:: AccountStorage (writes = extra_entries)
-				.saturating_add(write_cost.saturating_mul(extra_entries));
-			proof_size = proof_size
-				// Proof: EVM::AccountStorage (ACCOUNT_BASIC_PROOF_SIZE) * extra_entries
-				.saturating_add(ACCOUNT_STORAGE_PROOF_SIZE.saturating_mul(extra_entries));
-		}
-
-		// Refund the cost of the remaining contracts
-		if extra_contracts > 0 {
-			ref_time = ref_time
-				// EVM:: Suicided (reads = extra_contracts)
-				.saturating_add(read_cost.saturating_mul(extra_contracts))
-				// EVM:: Suicided (writes = extra_contracts)
-				.saturating_add(write_cost.saturating_mul(extra_contracts))
-				// System: AccountInfo (reads = extra_contracts) for decrementing sufficients
-				.saturating_add(read_cost.saturating_mul(extra_contracts))
-				// System: AccountInfo (writes = extra_contracts) for decrementing sufficients
-				.saturating_add(write_cost.saturating_mul(extra_contracts));
-			proof_size = proof_size
-				// Proof: EVM::Suicided (SUICIDED_STORAGE_KEY) * extra_contracts
-				.saturating_add(SUICIDED_STORAGE_KEY.saturating_mul(extra_contracts))
-				// Proof: System::AccountInfo (ACCOUNT_BASIC_PROOF_SIZE) * extra_contracts
-				.saturating_add(ACCOUNT_BASIC_PROOF_SIZE.saturating_mul(extra_contracts));
-		}
-
-		handle.refund_external_cost(Some(ref_time), Some(proof_size));
-	}
-
 	/// Clears the storage of a suicided contract.
 	///
 	/// This function will remove the given address from the list of suicided contracts
@@ -203,8 +98,3 @@ where
 		let _ = frame_system::Pallet::<Runtime>::dec_sufficients(&account_id);
 	}
 }
-
-struct RemovalResult {
-	pub deleted_entries: u64,
-	pub deleted_contracts: u64,
-}

frame/evm/precompile/storage-cleaner/src/tests.rs

@@ -295,7 +295,7 @@ fn test_clear_suicided_contract_limit_works() {
 
 			assert_eq!(
 				pallet_evm::AccountStorages::<Runtime>::iter_prefix(addresses[1].0).count(),
-				3
+				2
 			);
 
 			assert!(pallet_evm::Suicided::<Runtime>::contains_key(
@@ -310,7 +310,7 @@ fn test_clear_suicided_contract_limit_respected() {
 		.with_balances(vec![(Alice.into(), 10000000000000000000)])
 		.build()
 		.execute_with(|| {
-			let suicided_address = mock_contracts([5])[0].0;
+			let suicided_address = mock_contracts([6])[0].0;
 			// Add contract to the suicided contracts
 			pallet_evm::Suicided::<Runtime>::insert(suicided_address, ());
 

frame/evm/src/runner/stack.rs

@@ -754,6 +754,8 @@ where
 	}
 
 	fn exit_commit(&mut self) -> Result<(), ExitError> {
+		// For the parachains, it's essential to consider storage proof as the weight does.
+		// The consumed storage proof size utilizes the underlying host function. Return out of gas if the transaction requires too much storage proof.
 		match self.transaction_pov {
 			Some(pov) if pov.proof_size_used() > pov.weight_limit.proof_size() => {
 				self.substate.exit_discard()?;

primitives/evm/src/lib.rs

@@ -58,13 +58,6 @@ pub struct Vicinity {
 	/// Origin of the transaction.
 	pub origin: H160,
 }
-
-/// `System::Account` 16(hash) + 20 (key) + 60 (AccountInfo::max_encoded_len)
-pub const ACCOUNT_BASIC_PROOF_SIZE: u64 = 96;
-/// 16 (hash1) + 20 (key1) + 16 (hash2) + 32 (key2) + 32 (value)
-pub const ACCOUNT_STORAGE_PROOF_SIZE: u64 = 116;
-
-
 #[derive(Clone, Copy, Eq, PartialEq, Debug, Encode, Decode, TypeInfo)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct TransactionPov {