Merge pull request #13562 from ethereum/evmc-cleanup

EVMHost: Simplify some code and document functions

test/EVMHost.cpp

@@ -37,8 +37,6 @@ using namespace solidity::util;
 using namespace solidity::test;
 using namespace evmc::literals;
 
-using StorageMap = std::map<evmc::bytes32, evmc::storage_value>;
-
 evmc::VM& EVMHost::getVM(string const& _path)
 {
 	static evmc::VM NullVM{nullptr};
@@ -192,6 +190,7 @@ void EVMHost::transfer(evmc::MockedAccount& _sender, evmc::MockedAccount& _recip
 void EVMHost::selfdestruct(const evmc::address& _addr, const evmc::address& _beneficiary) noexcept
 {
 	// TODO actual selfdestruct is even more complicated.
+	// TODO reuse MockedHost::selfdestruct.
 
 	transfer(accounts[_addr], accounts[_beneficiary], convertFromEVMC(accounts[_addr].balance));
 	accounts.erase(_addr);
@@ -205,6 +204,8 @@ void EVMHost::recordCalls(evmc_message const& _message) noexcept
 		recorded_calls.emplace_back(_message);
 }
 
+// NOTE: this is used for both internal and external calls.
+// External calls are triggered from ExecutionFramework and contain only EVMC_CREATE or EVMC_CALL.
 evmc::result EVMHost::call(evmc_message const& _message) noexcept
 {
 	recordCalls(_message);
@@ -393,8 +394,14 @@ evmc::result EVMHost::precompileECRecover(evmc_message const& _message) noexcept
 		}
 	};
 	evmc::result result = precompileGeneric(_message, inputOutput);
-	result.status_code = EVMC_SUCCESS;
+	// ECRecover will return success with empty response in case of failure
-	result.gas_left = _message.gas;
+	if (result.status_code != EVMC_SUCCESS)
+	{
+		result.status_code = EVMC_SUCCESS;
+		result.gas_left = _message.gas;
+		result.output_data = {};
+		result.output_size = 0;
+	}
 	return result;
 }
 
@@ -407,11 +414,7 @@ evmc::result EVMHost::precompileSha256(evmc_message const& _message) noexcept
 		_message.input_data + _message.input_size
 	));
 
-	evmc::result result({});
+	return resultWithGas(_message, hash);
-	result.gas_left = _message.gas;
-	result.output_data = hash.data();
-	result.output_size = hash.size();
-	return result;
 }
 
 evmc::result EVMHost::precompileRipeMD160(evmc_message const& _message) noexcept
@@ -485,11 +488,8 @@ evmc::result EVMHost::precompileIdentity(evmc_message const& _message) noexcept
 	// static data so that we do not need a release routine...
 	bytes static data;
 	data = bytes(_message.input_data, _message.input_data + _message.input_size);
-	evmc::result result({});
+
-	result.gas_left = _message.gas;
+	return resultWithGas(_message, data);
-	result.output_data = data.data();
-	result.output_size = data.size();
-	return result;
 }
 
 evmc::result EVMHost::precompileModExp(evmc_message const&) noexcept

test/EVMHost.h

@@ -35,12 +35,30 @@
 namespace solidity::test
 {
 using Address = util::h160;
+using StorageMap = std::map<evmc::bytes32, evmc::storage_value>;
 
 class EVMHost: public evmc::MockedHost
 {
 public:
-	using MockedHost::get_code_size;
+	// Verbatim features of MockedHost.
+	using MockedHost::account_exists;
+	using MockedHost::get_storage;
+	using MockedHost::set_storage;
 	using MockedHost::get_balance;
+	using MockedHost::get_code_size;
+	using MockedHost::get_code_hash;
+	using MockedHost::copy_code;
+	using MockedHost::get_tx_context;
+	using MockedHost::emit_log;
+	using MockedHost::access_account;
+	using MockedHost::access_storage;
+
+	// Modified features of MockedHost.
+	void selfdestruct(evmc::address const& _addr, evmc::address const& _beneficiary) noexcept final;
+	evmc::result call(evmc_message const& _message) noexcept final;
+	evmc::bytes32 get_block_hash(int64_t number) const noexcept final;
+
+	// Solidity testing specific features.
 
 	/// Tries to dynamically load an evmc vm supporting evm1 or ewasm and caches the loaded VM.
 	/// @returns vmc::VM(nullptr) on failure.
@@ -55,9 +73,10 @@ public:
 
 	explicit EVMHost(langutil::EVMVersion _evmVersion, evmc::VM& _vm);
 
+	/// Reset entire state (including accounts).
 	void reset();
-	/// Clears EIP-2929 account and storage access indicator
+
-	void resetWarmAccess();
+	/// Start new block.
 	void newBlock()
 	{
 		tx_context.block_number++;
@@ -67,35 +86,21 @@ public:
 	}
 
 	/// @returns contents of storage at @param _addr.
-	std::map<evmc::bytes32, evmc::storage_value> const& get_address_storage(evmc::address const& _addr);
+	StorageMap const& get_address_storage(evmc::address const& _addr);
-
-	bool account_exists(evmc::address const& _addr) const noexcept final
-	{
-		return evmc::MockedHost::account_exists(_addr);
-	}
-
-	void selfdestruct(evmc::address const& _addr, evmc::address const& _beneficiary) noexcept final;
-
-	evmc::result call(evmc_message const& _message) noexcept final;
-
-	evmc::bytes32 get_block_hash(int64_t number) const noexcept final;
 
 	static Address convertFromEVMC(evmc::address const& _addr);
 	static evmc::address convertToEVMC(Address const& _addr);
 	static util::h256 convertFromEVMC(evmc::bytes32 const& _data);
 	static evmc::bytes32 convertToEVMC(util::h256 const& _data);
-
-	/// @returns true, if the evmc VM has the given capability.
-	bool hasCapability(evmc_capabilities capability) const noexcept
-	{
-		return m_vm.has_capability(capability);
-	}
-
 private:
 	evmc::address m_currentAddress = {};
 
+	/// Transfer value between accounts. Checks for sufficient balance.
 	void transfer(evmc::MockedAccount& _sender, evmc::MockedAccount& _recipient, u256 const& _value) noexcept;
 
+	/// Clears EIP-2929 account and storage access indicator
+	void resetWarmAccess();
+
 	/// Records calls made via @param _message.
 	void recordCalls(evmc_message const& _message) noexcept;
 
@@ -113,9 +118,9 @@ private:
 	static evmc::result resultWithGas(evmc_message const& _message, bytes const& _data) noexcept;
 
 	evmc::VM& m_vm;
-	// EVM version requested by the testing tool
+	/// EVM version requested by the testing tool
 	langutil::EVMVersion m_evmVersion;
-	// EVM version requested from EVMC (matches the above)
+	/// EVM version requested from EVMC (matches the above)
 	evmc_revision m_evmRevision;
 };