Evm tests (#1792)

* setup evm-tests

* Revert "Use ExitReason::Revert instead of ExitReason::Error (#1772)"

This reverts commit a43acda.

* update evm-tests commit hash

* clippy

* fix warnings

* cleanup

* fix selfdestruct remaining balance

* check if address exists only on evm-tests

* add block_coinbase to vicinity

* update vicinity

* fix test

* update evm-tests

* make vicinity coinbase, gas_limit and difficulty optional

.gitmodules

@@ -10,3 +10,7 @@
 [submodule "evm-bench"]
 	path = evm-bench
 	url = https://github.com/AcalaNetwork/evm-bench.git
+[submodule "evm-tests"]
+	path = evm-tests
+	url = https://github.com/AcalaNetwork/evm-tests.git
+	branch = acala-evm

Cargo.toml

@@ -44,6 +44,11 @@ members = [
 	"ecosystem-modules/starport",
 	"ecosystem-modules/compound-cash",
 	"ecosystem-modules/stable-asset/lib/stable-asset",
+
+]
+
+exclude = [
+	"evm-tests",
 ]
 
 resolver = "2"

Makefile

@@ -76,11 +76,15 @@ test: githooks
 	SKIP_WASM_BUILD= cargo test --features with-mandala-runtime --all
 
 .PHONY: test-eth
-test-eth: githooks
+test-eth: githooks test-evm
 	SKIP_WASM_BUILD= cargo test -p runtime-common --features with-ethereum-compatibility schedule_call_precompile_should_work
 	SKIP_WASM_BUILD= cargo test -p runtime-integration-tests --features with-mandala-runtime --features with-ethereum-compatibility should_not_kill_contract_on_transfer_all
 	SKIP_WASM_BUILD= cargo test -p runtime-integration-tests --features with-mandala-runtime --features with-ethereum-compatibility schedule_call_precompile_should_handle_invalid_input
 
+.PHONY: test-evm
+test-evm: githooks
+	cargo test --manifest-path evm-tests/jsontests/Cargo.toml
+
 .PHONY: test-runtimes
 test-runtimes:
 	SKIP_WASM_BUILD= cargo test --all --features with-all-runtime

modules/evm/Cargo.toml

@@ -87,3 +87,6 @@ bench = [
 	"hex",
 	"module-idle-scheduler"
 ]
+evm-tests = [
+	"primitives/evm-tests"
+]

modules/evm/src/bench/mod.rs

@@ -70,7 +70,7 @@ fn whitelist_keys(b: &mut Bencher, from: H160, code: Vec<u8>) -> H160 {
 	let address = H160::from_str("2000000000000000000000000000000000000001").unwrap();
 	let vicinity = Vicinity {
 		gas_price: U256::one(),
-		origin: Default::default(),
+		..Default::default()
 	};
 	let context = Context {
 		caller: from,

modules/evm/src/lib.rs

@@ -440,7 +440,7 @@ pub mod module {
 					// Use the EVM Runtime
 					let vicinity = Vicinity {
 						gas_price: U256::one(),
-						origin: Default::default(),
+						..Default::default()
 					};
 					let context = Context {
 						caller: source,

modules/evm/src/mock.rs

@@ -162,7 +162,9 @@ impl FindAuthor<AccountId32> for AuthorGiven {
 	where
 		I: 'a + IntoIterator<Item = (ConsensusEngineId, &'a [u8])>,
 	{
-		Some(AccountId32::from_str("1234500000000000000000000000000000000000").unwrap())
+		Some(<Runtime as Config>::AddressMapping::get_account_id(
+			&H160::from_str("1234500000000000000000000000000000000000").unwrap(),
+		))
 	}
 }
 

modules/evm/src/runner/stack.rs

@@ -69,7 +69,11 @@ impl<T: Config> Runner<T> {
 		F: FnOnce(&mut StackExecutor<'config, SubstrateStackState<'_, 'config, T>>) -> (ExitReason, R),
 	{
 		let gas_price = U256::one();
-		let vicinity = Vicinity { gas_price, origin };
+		let vicinity = Vicinity {
+			gas_price,
+			origin,
+			..Default::default()
+		};
 
 		let metadata = StackSubstateMetadata::new(gas_limit, storage_limit, config);
 		let state = SubstrateStackState::new(&vicinity, metadata);
@@ -457,7 +461,7 @@ impl<'vicinity, 'config, T: Config> BackendT for SubstrateStackState<'vicinity,
 	}
 
 	fn block_coinbase(&self) -> H160 {
-		Pallet::<T>::find_author()
+		self.vicinity.block_coinbase.unwrap_or(Pallet::<T>::find_author())
 	}
 
 	fn block_timestamp(&self) -> U256 {
@@ -466,17 +470,23 @@ impl<'vicinity, 'config, T: Config> BackendT for SubstrateStackState<'vicinity,
 	}
 
 	fn block_difficulty(&self) -> U256 {
-		U256::zero()
+		self.vicinity.block_difficulty.unwrap_or_default()
 	}
 
 	fn block_gas_limit(&self) -> U256 {
-		U256::zero()
+		self.vicinity.block_gas_limit.unwrap_or_default()
 	}
 
 	fn chain_id(&self) -> U256 {
 		U256::from(T::ChainId::get())
 	}
 
+	#[cfg(feature = "evm-tests")]
+	fn exists(&self, address: H160) -> bool {
+		Accounts::<T>::contains_key(&address)
+	}
+
+	#[cfg(not(feature = "evm-tests"))]
 	fn exists(&self, _address: H160) -> bool {
 		true
 	}
@@ -491,23 +501,15 @@ impl<'vicinity, 'config, T: Config> BackendT for SubstrateStackState<'vicinity,
 	}
 
 	fn code(&self, address: H160) -> Vec<u8> {
-		let code = Pallet::<T>::code_at_address(&address).into_inner();
-		if code.len().is_zero() {
-			log::debug!(
-				target: "evm",
-				"contract does not exist, address: {:?}",
-				address
-			);
-		}
-		code
+		Pallet::<T>::code_at_address(&address).into_inner()
 	}
 
 	fn storage(&self, address: H160, index: H256) -> H256 {
-		<AccountStorages<T>>::get(address, index)
+		AccountStorages::<T>::get(&address, index)
 	}
 
-	fn original_storage(&self, _address: H160, _index: H256) -> Option<H256> {
-		None
+	fn original_storage(&self, address: H160, index: H256) -> Option<H256> {
+		Some(self.storage(address, index))
 	}
 }
 
@@ -672,16 +674,33 @@ impl<'vicinity, 'config, T: Config> StackStateT<'config> for SubstrateStackState
 			amount
 		);
 
+		if T::Currency::free_balance(&source) < amount {
+			return Err(ExitError::OutOfFund);
+		}
+
 		T::Currency::transfer(&source, &target, amount, ExistenceRequirement::AllowDeath)
 			.map_err(|e| ExitError::Other(Into::<&str>::into(e).into()))
 	}
 
-	fn reset_balance(&mut self, _address: H160) {
-		// Do nothing on reset balance in Substrate.
-		//
-		// This function exists in EVM because a design issue
-		// (arguably a bug) in SELFDESTRUCT that can cause total
-		// issurance to be reduced. We do not need to replicate this.
+	fn reset_balance(&mut self, address: H160) {
+		// Address and target can be the same during SELFDESTRUCT. In that case we transfer the
+		// remaining balance to treasury
+		let source = T::AddressMapping::get_account_id(&address);
+		let balance = T::Currency::free_balance(&source);
+		if !balance.is_zero() {
+			if let Err(e) = T::Currency::transfer(
+				&source,
+				&T::TreasuryAccount::get(),
+				balance,
+				ExistenceRequirement::AllowDeath,
+			) {
+				debug_assert!(
+					false,
+					"Failed to transfer remaining balance to treasury with error: {:?}",
+					e
+				);
+			}
+		}
 	}
 
 	fn touch(&mut self, _address: H160) {

modules/evm/src/runner/state.rs

@@ -244,10 +244,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 	/// Execute the runtime until it returns.
 	pub fn execute(&mut self, runtime: &mut Runtime) -> ExitReason {
 		match runtime.run(self) {
-			Capture::Exit(s) => {
-				log::debug!(target: "evm", "runtime.run exit: {:?}", s);
-				s
-			}
+			Capture::Exit(s) => s,
 			Capture::Trap(_) => unreachable!("Trap is Infallible"),
 		}
 	}
@@ -431,6 +428,12 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 		self.state.basic(address).nonce
 	}
 
+	#[cfg(feature = "evm-tests")]
+	pub fn handle_mirrored_token(&self, address: H160) -> H160 {
+		address
+	}
+
+	#[cfg(not(feature = "evm-tests"))]
 	pub fn handle_mirrored_token(&self, address: H160) -> H160 {
 		log::debug!(
 			target: "evm",
@@ -532,11 +535,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 
 		let address = match self.create_address(scheme) {
 			Err(e) => {
-				return Capture::Exit((
-					ExitReason::Revert(ExitRevert::Reverted),
-					None,
-					encode_revert_message(&e),
-				));
+				return Capture::Exit((ExitReason::Error(e), None, Vec::new()));
 			}
 			Ok(address) => address,
 		};
@@ -555,20 +554,12 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 
 		if let Some(depth) = self.state.metadata().depth() {
 			if depth > self.config.call_stack_limit {
-				return Capture::Exit((
-					ExitReason::Revert(ExitRevert::Reverted),
-					None,
-					encode_revert_message(&ExitError::CallTooDeep),
-				));
+				return Capture::Exit((ExitError::CallTooDeep.into(), None, Vec::new()));
 			}
 		}
 
 		if self.balance(caller) < value {
-			return Capture::Exit((
-				ExitReason::Revert(ExitRevert::Reverted),
-				None,
-				encode_revert_message(&ExitError::OutOfFund),
-			));
+			return Capture::Exit((ExitError::OutOfFund.into(), None, Vec::new()));
 		}
 
 		let after_gas = if take_l64 && self.config.call_l64_after_gas {
@@ -596,21 +587,13 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 		{
 			if self.code_size(address) != U256::zero() {
 				let _ = self.exit_substate(StackExitKind::Failed);
-				return Capture::Exit((
-					ExitReason::Revert(ExitRevert::Reverted),
-					None,
-					encode_revert_message(&ExitError::CreateCollision),
-				));
+				return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new()));
 			}
 
 			// We will keep the nonce until the storages are cleared.
 			if self.nonce(address) > U256::zero() {
 				let _ = self.exit_substate(StackExitKind::Failed);
-				return Capture::Exit((
-					ExitReason::Revert(ExitRevert::Reverted),
-					None,
-					encode_revert_message(&ExitError::CreateCollision),
-				));
+				return Capture::Exit((ExitError::CreateCollision.into(), None, Vec::new()));
 			}
 
 			// Still do this, although it is superfluous.
@@ -631,11 +614,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 			Ok(()) => (),
 			Err(e) => {
 				let _ = self.exit_substate(StackExitKind::Reverted);
-				return Capture::Exit((
-					ExitReason::Revert(ExitRevert::Reverted),
-					None,
-					encode_revert_message(&e),
-				));
+				return Capture::Exit((ExitReason::Error(e), None, Vec::new()));
 			}
 		}
 
@@ -656,11 +635,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 					if out.len() > limit {
 						self.state.metadata_mut().gasometer_mut().fail();
 						let _ = self.exit_substate(StackExitKind::Failed);
-						return Capture::Exit((
-							ExitReason::Revert(ExitRevert::Reverted),
-							None,
-							encode_revert_message(&ExitError::CreateContractLimit),
-						));
+						return Capture::Exit((ExitError::CreateContractLimit.into(), None, Vec::new()));
 					}
 				}
 
@@ -673,22 +648,14 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 					}
 					Err(e) => {
 						let _ = self.exit_substate(StackExitKind::Failed);
-						Capture::Exit((
-							ExitReason::Revert(ExitRevert::Reverted),
-							None,
-							encode_revert_message(&e),
-						))
+						Capture::Exit((ExitReason::Error(e), None, Vec::new()))
 					}
 				}
 			}
 			ExitReason::Error(e) => {
 				self.state.metadata_mut().gasometer_mut().fail();
 				let _ = self.exit_substate(StackExitKind::Failed);
-				Capture::Exit((
-					ExitReason::Revert(ExitRevert::Reverted),
-					None,
-					encode_revert_message(&e),
-				))
+				Capture::Exit((ExitReason::Error(e), None, Vec::new()))
 			}
 			ExitReason::Revert(e) => {
 				let _ = self.exit_substate(StackExitKind::Reverted);
@@ -770,10 +737,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 		if let Some(depth) = self.state.metadata().depth() {
 			if depth > self.config.call_stack_limit {
 				let _ = self.exit_substate(StackExitKind::Reverted);
-				return Capture::Exit((
-					ExitReason::Revert(ExitRevert::Reverted),
-					encode_revert_message(&ExitError::CallTooDeep),
-				));
+				return Capture::Exit((ExitError::CallTooDeep.into(), Vec::new()));
 			}
 		}
 
@@ -782,7 +746,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 				Ok(()) => (),
 				Err(e) => {
 					let _ = self.exit_substate(StackExitKind::Reverted);
-					return Capture::Exit((ExitReason::Revert(ExitRevert::Reverted), encode_revert_message(&e)));
+					return Capture::Exit((ExitReason::Error(e), Vec::new()));
 				}
 			}
 		}
@@ -798,12 +762,8 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 					for Log { address, topics, data } in logs {
 						match self.log(address, topics, data) {
 							Ok(_) => continue,
-							Err(e) => {
-								// this should not happen
-								return Capture::Exit((
-									ExitReason::Revert(ExitRevert::Reverted),
-									encode_revert_message(&e),
-								));
+							Err(error) => {
+								return Capture::Exit((ExitReason::Error(error), output));
 							}
 						}
 					}
@@ -838,7 +798,7 @@ impl<'config, S: StackState<'config>> StackExecutor<'config, S> {
 			}
 			ExitReason::Error(e) => {
 				let _ = self.exit_substate(StackExitKind::Failed);
-				Capture::Exit((ExitReason::Revert(ExitRevert::Reverted), encode_revert_message(&e)))
+				Capture::Exit((ExitReason::Error(e), Vec::new()))
 			}
 			ExitReason::Revert(e) => {
 				let _ = self.exit_substate(StackExitKind::Reverted);

modules/evm/src/tests.rs

@@ -55,7 +55,7 @@ fn should_calculate_contract_address() {
 
 		let vicinity = Vicinity {
 			gas_price: U256::one(),
-			origin: Default::default(),
+			..Default::default()
 		};
 		let metadata = StackSubstateMetadata::new(1000, 1000, &ACALA_CONFIG);
 		let state = SubstrateStackState::<Runtime>::new(&vicinity, metadata);

primitives/Cargo.toml

@@ -48,3 +48,4 @@ std = [
 	"module-evm-utiltity/std",
 	"nutsfinance-stable-asset/std",
 ]
+evm-tests = []

primitives/src/evm.rs

@@ -44,6 +44,12 @@ pub struct Vicinity {
 	pub gas_price: U256,
 	/// Origin of the transaction.
 	pub origin: EvmAddress,
+	/// Environmental coinbase.
+	pub block_coinbase: Option<EvmAddress>,
+	/// Environmental block gas limit. Used only for testing
+	pub block_gas_limit: Option<U256>,
+	/// Environmental block difficulty. Used only for testing
+	pub block_difficulty: Option<U256>,
 }
 
 #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)]