Resolve "Reverted report validator transactions get added in block #10580"

ethcore/call-contract/Cargo.toml

@@ -7,5 +7,6 @@ edition = "2018"
 
 [dependencies]
 types = { path = "../types", package = "common-types" }
+ethabi = "6.0"
 ethereum-types = "0.6.0"
 bytes = { version = "0.1", package = "parity-bytes" }

ethcore/call-contract/src/call_contract.rs

@@ -18,12 +18,36 @@
 
 use bytes::Bytes;
 use ethereum_types::Address;
+use ethabi::{decode, ParamType, Token};
 use types::ids::BlockId;
 
+const ERROR_SELECTOR: [u8; 4] = [0x08, 0xc3, 0x79, 0xa0];
+
 /// Provides `call_contract` method
 pub trait CallContract {
 	/// Like `call`, but with various defaults. Designed to be used for calling contracts.
+	/// Returns an error in case of vm exception. Tries to decode Solidity revert message if there is a revert exception.
 	fn call_contract(&self, id: BlockId, address: Address, data: Bytes) -> Result<Bytes, String>;
+
+	/// Like `call`, but with various defaults. Designed to be used for calling contracts with specified sender.
+	/// Returns an error in case of vm exception. Tries to decode Solidity revert message if there is a revert exception.
+	fn call_contract_from_sender(&self, block_id: BlockId, address: Address, sender: Address, data: Bytes) -> Result<Bytes, String>;
+
+	/// Try to decode Solidity revert string
+	fn try_decode_solidity_revert_msg(&self, data: &Bytes) -> Option<String> {
+		let mut result = None;
+		if data.len() > 4 {
+			let (fn_selector, enc_string) = data.split_at(4);
+			// Error(string) selector. Details: https://solidity.readthedocs.io/en/v0.5.8/control-structures.html#error-handling-assert-require-revert-and-exceptions
+			if fn_selector == ERROR_SELECTOR {
+				result = decode(&[ParamType::String], enc_string)
+					.as_ref()
+					.map(|d| if let Token::String(str) = &d[0] { Some(str.as_str().to_string()) } else { None })
+					.unwrap_or_else(|_| None);
+			}
+		}
+		result
+	}
 }
 
 /// Provides information on a blockchain service and it's registry

ethcore/call-contract/src/lib.rs

@@ -21,6 +21,8 @@
 //! This crate exposes traits required to call contracts at particular block.
 //! All utilities that depend on on-chain data should use those traits to access it.
 
+extern crate ethabi;
+
 pub mod call_contract;
 
 // Re-export

ethcore/private-tx/src/key_server_keys.rs

@@ -160,6 +160,7 @@ mod tests {
 
 	impl CallContract for DummyRegistryClient {
 		fn call_contract(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result<Bytes, String> { Ok(vec![]) }
+		fn call_contract_from_sender(&self, _block_id: BlockId, _address: Address, _sender: Address, _data: Bytes) -> Result<Bytes, String> { Ok(vec![]) }
 	}
 
 	#[test]

ethcore/src/client/client.rs

@@ -24,6 +24,7 @@ use std::time::{Instant, Duration};
 use blockchain::{BlockReceipts, BlockChain, BlockChainDB, BlockProvider, TreeRoute, ImportRoute, TransactionAddress, ExtrasInsert, BlockNumberKey};
 use bytes::Bytes;
 use call_contract::{CallContract, RegistryInfo};
+use ethabi::{Token};
 use ethcore_miner::pool::VerifiedTransaction;
 use ethereum_types::{H256, H264, Address, U256};
 use evm::Schedule;
@@ -42,7 +43,7 @@ use types::filter::Filter;
 use types::log_entry::LocalizedLogEntry;
 use types::receipt::{Receipt, LocalizedReceipt};
 use types::{BlockNumber, header::{Header, ExtendedHeader}};
-use vm::{EnvInfo, LastHashes};
+use vm::{EnvInfo, LastHashes, Error as VmError};
 
 use block::{LockedBlock, Drain, ClosedBlock, OpenBlock, enact_verified, SealedBlock};
 use client::ancient_import::AncientVerifier;
@@ -1223,15 +1224,19 @@ impl Client {
 	// transaction for calling contracts from services like engine.
 	// from the null sender, with 50M gas.
 	fn contract_call_tx(&self, block_id: BlockId, address: Address, data: Bytes) -> SignedTransaction {
-		let from = Address::zero();
+		self.contract_call_tx_from_sender(block_id, address, Address::zero(), data)
+	}
+
+	// transaction for calling contracts with specified sender
+	fn contract_call_tx_from_sender(&self, block_id: BlockId, address: Address, sender: Address, data: Bytes) -> SignedTransaction {
 		transaction::Transaction {
-			nonce: self.nonce(&from, block_id).unwrap_or_else(|| self.engine.account_start_nonce(0)),
+			nonce: self.nonce(&sender, block_id).unwrap_or_else(|| self.engine.account_start_nonce(0)),
 			action: Action::Call(address),
 			gas: U256::from(50_000_000),
 			gas_price: U256::default(),
 			value: U256::default(),
-			data: data,
-		}.fake_sign(from)
+			data,
+		}.fake_sign(sender)
 	}
 
 	fn do_virtual_call(
@@ -1461,15 +1466,34 @@ impl RegistryInfo for Client {
 
 impl CallContract for Client {
 	fn call_contract(&self, block_id: BlockId, address: Address, data: Bytes) -> Result<Bytes, String> {
+		self.call_contract_from_sender(block_id, address, Address::default(), data)
+	}
+
+	fn call_contract_from_sender(
+		&self,
+		block_id: BlockId,
+		address: Address,
+		sender: Address,
+		data: Bytes
+	) -> Result<Bytes, String> {
 		let state_pruned = || CallError::StatePruned.to_string();
 		let state = &mut self.state_at(block_id).ok_or_else(&state_pruned)?;
 		let header = self.block_header_decoded(block_id).ok_or_else(&state_pruned)?;
 
-		let transaction = self.contract_call_tx(block_id, address, data);
+		let transaction = self.contract_call_tx_from_sender(block_id, address, sender, data);
 
-		self.call(&transaction, Default::default(), state, &header)
-			.map_err(|e| format!("{:?}", e))
-			.map(|executed| executed.output)
+		let executed = self.call(&transaction, Default::default(), state, &header).map_err(|e| format!("{:?}", e))?;
+		if let Some(exception) = executed.exception {
+			let mut error = format!("{:?}", exception);
+			if exception == VmError::Reverted {
+				let output = executed.output.clone();
+				let msg = self.try_decode_solidity_revert_msg(&output).unwrap_or_else(|| format!("{}", Token::Bytes(output)));
+				error = format!("{} {}", error, msg);
+			}
+			Err(error)
+		} else {
+			Ok(executed.output)
+		}
 	}
 }
 
@@ -2203,6 +2227,10 @@ impl BlockChainClient for Client {
 		self.importer.miner.import_own_transaction(self, signed.into())
 	}
 
+	fn call_contract_as_author(&self, id: BlockId, address: Address, data: Bytes) -> Result<Bytes, String> {
+		self.call_contract_from_sender(id, address, self.importer.miner.authoring_params().author, data)
+	}
+
 	fn registrar_address(&self) -> Option<Address> {
 		self.registrar_address.clone()
 	}

ethcore/src/client/test_client.rs

@@ -524,6 +524,7 @@ impl BlockInfo for TestBlockChainClient {
 
 impl CallContract for TestBlockChainClient {
 	fn call_contract(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result<Bytes, String> { Ok(vec![]) }
+	fn call_contract_from_sender(&self, _block_id: BlockId, _address: Address, _sender: Address, _data: Bytes) -> Result<Bytes, String> { Ok(vec![]) }
 }
 
 impl TransactionInfo for TestBlockChainClient {
@@ -900,6 +901,8 @@ impl BlockChainClient for TestBlockChainClient {
 		self.miner.import_own_transaction(self, signed.into())
 	}
 
+	fn call_contract_as_author(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result<Bytes, String> { Ok(vec![]) }
+
 	fn registrar_address(&self) -> Option<Address> { None }
 }
 

ethcore/src/client/traits.rs

@@ -378,6 +378,9 @@ pub trait BlockChainClient : Sync + Send + AccountData + BlockChain + CallContra
 	/// Schedule state-altering transaction to be executed on the next pending block.
 	fn transact_contract(&self, address: Address, data: Bytes) -> Result<(), transaction::Error>;
 
+	/// Call contract as block author
+	fn call_contract_as_author(&self, id: BlockId, address: Address, data: Bytes) -> Result<Bytes, String>;
+
 	/// Get the address of the registry itself.
 	fn registrar_address(&self) -> Option<Address>;
 }

ethcore/src/engines/validator_set/contract.rs

@@ -25,6 +25,7 @@ use machine::{AuxiliaryData, Call, EthereumMachine};
 use parking_lot::RwLock;
 use types::BlockNumber;
 use types::header::Header;
+use types::ids::BlockId;
 
 use client::EngineClient;
 
@@ -58,6 +59,7 @@ impl ValidatorContract {
 
 		match client.as_full_client() {
 			Some(c) => {
+				c.call_contract_as_author(BlockId::Latest, self.contract_address, data.clone())?;
 				c.transact_contract(self.contract_address, data)
 					.map_err(|e| format!("Transaction import error: {}", e))?;
 				Ok(())