Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127)

* Change how RPCs eth_call and eth_estimateGas handle "Pending"

Before this PR we would return a rather confusing error when calling `eth_call` and `eth_estimateGas` with `"Pending"`, e.g.:

```
{"jsonrpc":"2.0","error":{"code":-32000,"message":"This request is not supported because your node is running with state pruning. Run with --pruning=archive."},"id":"e237678f6648ed12ff05a74933d06d17"}
```

In reality what is going on is that users often use `"Pending"` when they really mean `"Latest"` (e.g. MyCrypto…) and when the block in question is not actually pending. This changes our behaviour for these two RPC calls to fall back to `"Latest"` when the query with `"Pending"` fails.
Note that we already behave this way for many other RPCs:

	- eth_call (after this PR)
	- eth_estimateGas (after this PR)
	- eth_getBalance
	- eth_getCode
	- eth_getStorageAt

Closes https://github.com/paritytech/parity-ethereum/issues/10096

* Fetch jsonrpc from git

* No real need to wait for new jsonrpc

* Add tests for calling eth_call/eth_estimateGas with "Pending"

* Fix a todo, add another

* Change client.latest_state to return the best header as well so we avoid potential data races and do less work

* Impl review suggestions

* Update rpc/src/v1/impls/eth.rs

Co-Authored-By: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* Review grumbles

* update docs

ethcore/client-traits/src/lib.rs

@@ -412,8 +412,8 @@ pub trait StateClient {
 	/// Type representing chain state
 	type State: StateInfo;
 
-	/// Get a copy of the best block's state.
+	/// Get a copy of the best block's state and header.
-	fn latest_state(&self) -> Self::State;
+	fn latest_state_and_header(&self) -> (Self::State, Header);
 
 	/// Attempt to get a copy of a specific block's final state.
 	///

ethcore/src/client/client.rs

@@ -1033,15 +1033,16 @@ impl Client {
 	}
 
 	/// Get a copy of the best block's state.
-	pub fn latest_state(&self) -> State<StateDB> {
+	pub fn latest_state_and_header(&self) -> (State<StateDB>, Header) {
 		let header = self.best_block_header();
-		State::from_existing(
+		let state = State::from_existing(
 			self.state_db.read().boxed_clone_canon(&header.hash()),
 			*header.state_root(),
 			self.engine.account_start_nonce(header.number()),
 			self.factories.clone()
 		)
-		.expect("State root of best block header always valid.")
+		.expect("State root of best block header always valid.");
+		(state, header)
 	}
 
 	/// Attempt to get a copy of a specific block's final state.
@@ -1051,9 +1052,9 @@ impl Client {
 	/// is unknown.
 	pub fn state_at(&self, id: BlockId) -> Option<State<StateDB>> {
 		// fast path for latest state.
-		match id.clone() {
+		if let BlockId::Latest = id {
-			BlockId::Latest => return Some(self.latest_state()),
+			let (state, _) = self.latest_state_and_header();
-			_ => {},
+			return Some(state)
 		}
 
 		let block_number = match self.block_number(id) {
@@ -1087,8 +1088,9 @@ impl Client {
 	}
 
 	/// Get a copy of the best block's state.
-	pub fn state(&self) -> Box<dyn StateInfo> {
+	pub fn state(&self) -> impl StateInfo {
-		Box::new(self.latest_state()) as Box<_>
+		let (state, _) = self.latest_state_and_header();
+		state
 	}
 
 	/// Get info on the cache.
@@ -1476,8 +1478,8 @@ impl ImportBlock for Client {
 impl StateClient for Client {
 	type State = State<::state_db::StateDB>;
 
-	fn latest_state(&self) -> Self::State {
+	fn latest_state_and_header(&self) -> (Self::State, Header) {
-		Client::latest_state(self)
+		Client::latest_state_and_header(self)
 	}
 
 	fn state_at(&self, id: BlockId) -> Option<Self::State> {

ethcore/src/miner/mod.rs

@@ -45,13 +45,16 @@ use types::{
 	receipt::RichReceipt,
 };
 
-use block::SealedBlock;
 use call_contract::CallContract;
 use registrar::RegistrarClient;
-use client::{BlockProducer, SealedBlockImporter};
 use client_traits::{BlockChain, ChainInfo, AccountData, Nonce, ScheduleInfo};
 use account_state::state::StateInfo;
 
+use crate::{
+	block::SealedBlock,
+	client::{BlockProducer, SealedBlockImporter},
+};
+
 /// Provides methods to verify incoming external transactions
 pub trait TransactionVerifierClient: Send + Sync
 	// Required for ServiceTransactionChecker

ethcore/src/test_helpers/test_client.rs

@@ -644,8 +644,8 @@ impl StateInfo for TestState {
 impl StateClient for TestBlockChainClient {
 	type State = TestState;
 
-	fn latest_state(&self) -> Self::State {
+	fn latest_state_and_header(&self) -> (Self::State, Header) {
-		TestState
+		(TestState, self.best_block_header())
 	}
 
 	fn state_at(&self, _id: BlockId) -> Option<Self::State> {

ethcore/src/tests/client.rs

@@ -17,6 +17,7 @@
 use std::str::{FromStr, from_utf8};
 use std::sync::Arc;
 
+use account_state::state::StateInfo;
 use ethereum_types::{U256, Address};
 use ethkey::KeyPair;
 use hash::keccak;

rpc/src/v1/impls/eth.rs

@@ -37,6 +37,7 @@ use types::{
 	BlockNumber as EthBlockNumber,
 	client_types::StateResult,
 	encoded,
+	header::Header,
 	ids::{BlockId, TransactionId, UncleId},
 	filter::Filter as EthcoreFilter,
 	transaction::{SignedTransaction, LocalizedTransaction},
@@ -182,10 +183,11 @@ pub fn base_logs<C, M, T: StateInfo + 'static> (client: &C, miner: &M, filter: F
 	Box::new(future::ok(logs))
 }
 
-impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> EthClient<C, SN, S, M, EM> where
+impl<C, SN: ?Sized, S: ?Sized, M, EM, T> EthClient<C, SN, S, M, EM> where
 	C: miner::BlockChainClient + BlockChainClient + StateClient<State=T> + Call<State=T> + EngineInfo,
 	SN: SnapshotService,
 	S: SyncProvider,
+	T: StateInfo + 'static,
 	M: MinerService<State=T>,
 	EM: ExternalMinerService {
 
@@ -439,6 +441,10 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> EthClient<C, SN, S
 		Ok(Some(block))
 	}
 
+	/// Get state for the given block number. Returns either the State or a block from which state
+	/// can be retrieved.
+	/// Note: When passing `BlockNumber::Pending` we fall back to the state of the current best block
+	/// if no state found for the best pending block.
 	fn get_state(&self, number: BlockNumber) -> StateOrBlock {
 		match number {
 			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash).into(),
@@ -451,11 +457,33 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> EthClient<C, SN, S
 				self.miner
 					.pending_state(info.best_block_number)
 					.map(|s| Box::new(s) as Box<dyn StateInfo>)
-					.unwrap_or(Box::new(self.client.latest_state()) as Box<dyn StateInfo>)
+					.unwrap_or_else(|| {
+						warn!("Asked for best pending state, but none found. Falling back to latest state");
+						let (state, _) = self.client.latest_state_and_header();
+						Box::new(state) as Box<dyn StateInfo>
+					})
 					.into()
 			}
 		}
 	}
+
+	/// Get the state and header of best pending block. On failure, fall back to the best imported
+	/// blocks state&header.
+	fn pending_state_and_header_with_fallback(&self) -> (T, Header) {
+		let best_block_number = self.client.chain_info().best_block_number;
+		let (maybe_state, maybe_header) =
+			self.miner.pending_state(best_block_number).map_or_else(|| (None, None),|s| {
+				(Some(s), self.miner.pending_block_header(best_block_number))
+			});
+
+		match (maybe_state, maybe_header) {
+			(Some(state), Some(header)) => (state, header),
+			_ => {
+				warn!("Falling back to \"Latest\"");
+				self.client.latest_state_and_header()
+			}
+		}
+	}
 }
 
 pub fn pending_logs<M>(miner: &M, best_block: EthBlockNumber, filter: &EthcoreFilter) -> Vec<Log> where M: MinerService {
@@ -647,12 +675,13 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 		let num = num.unwrap_or_default();
 
 		try_bf!(check_known(&*self.client, num.clone()));
-		let res = match self.client.storage_at(&address, &BigEndianHash::from_uint(&position), self.get_state(num)) {
+		let storage = self.client.storage_at(
-			Some(s) => Ok(s),
+			&address,
-			None => Err(errors::state_pruned()),
+			&BigEndianHash::from_uint(&position),
-		};
+			self.get_state(num)
+		).ok_or_else(errors::state_pruned);
 
-		Box::new(future::done(res))
+		Box::new(future::done(storage))
 	}
 
 	fn transaction_count(&self, address: H160, num: Option<BlockNumber>) -> BoxFuture<U256> {
@@ -937,12 +966,9 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 		let num = num.unwrap_or_default();
 		try_bf!(check_known(&*self.client, num.clone()));
 
-		let (mut state, header) = if num == BlockNumber::Pending {
+		let (mut state, header) =
-			let info = self.client.chain_info();
+			if num == BlockNumber::Pending {
-			let state = try_bf!(self.miner.pending_state(info.best_block_number).ok_or_else(errors::state_pruned));
+				self.pending_state_and_header_with_fallback()
-			let header = try_bf!(self.miner.pending_block_header(info.best_block_number).ok_or_else(errors::state_pruned));
-
-			(state, header)
 			} else {
 				let id = match num {
 					BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
@@ -953,7 +979,10 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 				};
 
 				let state = try_bf!(self.client.state_at(id).ok_or_else(errors::state_pruned));
-			let header = try_bf!(self.client.block_header(id).ok_or_else(errors::state_pruned).and_then(|h| h.decode().map_err(errors::decode)));
+				let header = try_bf!(
+					self.client.block_header(id).ok_or_else(errors::state_pruned)
+						.and_then(|h| h.decode().map_err(errors::decode))
+				);
 
 				(state, header)
 			};
@@ -978,13 +1007,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 		let num = num.unwrap_or_default();
 
 		let (state, header) = if num == BlockNumber::Pending {
-			let info = self.client.chain_info();
+			self.pending_state_and_header_with_fallback()
-			let state = try_bf!(self.miner.pending_state(info.best_block_number)
-								.ok_or_else(errors::state_pruned));
-			let header = try_bf!(self.miner.pending_block_header(info.best_block_number)
-								 .ok_or_else(errors::state_pruned));
-
-			(state, header)
 		} else {
 			let id = match num {
 				BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),

rpc/src/v1/tests/helpers/miner_service.rs

@@ -93,8 +93,8 @@ impl StateClient for TestMinerService {
 	// State will not be used by test client anyway, since all methods that accept state are mocked
 	type State = TestState;
 
-	fn latest_state(&self) -> Self::State {
+	fn latest_state_and_header(&self) -> (Self::State, Header) {
-		TestState
+		(TestState, Header::default())
 	}
 
 	fn state_at(&self, _id: BlockId) -> Option<Self::State> {

rpc/src/v1/tests/mocked/eth.rs

@@ -663,6 +663,43 @@ fn rpc_eth_call_latest() {
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 }
 
+#[test]
+fn rpc_eth_call_pending() {
+	let tester = EthTester::default();
+	tester.client.set_execution_result(Ok(Executed {
+		exception: None,
+		gas: U256::zero(),
+		gas_used: U256::from(0xff30),
+		refunded: U256::from(0x5),
+		cumulative_gas_used: U256::zero(),
+		logs: vec![],
+		contracts_created: vec![],
+		output: vec![0x12, 0x34, 0xff],
+		trace: vec![],
+		vm_trace: None,
+		state_diff: None,
+	}));
+
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_call",
+		"params": [{
+			"from": "0xb60e8dd61c5d32be8058bb8eb970870f07233155",
+			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
+			"gas": "0x76c0",
+			"gasPrice": "0x9184e72a000",
+			"value": "0x9184e72a",
+			"data": "0xd46e8dd67c5d32be8d46e8dd67c5d32be8058bb8eb970870f072445675058bb8eb970870f072445675"
+		},
+		"pending"],
+		"id": 1
+	}"#;
+	// Falls back to "Latest" and gives the same result.
+	let response = r#"{"jsonrpc":"2.0","result":"0x1234ff","id":1}"#;
+
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+}
+
 #[test]
 fn rpc_eth_call() {
 	let tester = EthTester::default();
@@ -770,6 +807,43 @@ fn rpc_eth_estimate_gas() {
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 }
 
+#[test]
+fn rpc_eth_estimate_gas_pending() {
+	let tester = EthTester::default();
+	tester.client.set_execution_result(Ok(Executed {
+		exception: None,
+		gas: U256::zero(),
+		gas_used: U256::from(0xff30),
+		refunded: U256::from(0x5),
+		cumulative_gas_used: U256::zero(),
+		logs: vec![],
+		contracts_created: vec![],
+		output: vec![0x12, 0x34, 0xff],
+		trace: vec![],
+		vm_trace: None,
+		state_diff: None,
+	}));
+
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_estimateGas",
+		"params": [{
+			"from": "0xb60e8dd61c5d32be8058bb8eb970870f07233155",
+			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
+			"gas": "0x76c0",
+			"gasPrice": "0x9184e72a000",
+			"value": "0x9184e72a",
+			"data": "0xd46e8dd67c5d32be8d46e8dd67c5d32be8058bb8eb970870f072445675058bb8eb970870f072445675"
+		},
+		"pending"],
+		"id": 1
+	}"#;
+	// Falls back to "Latest" so the result is the same
+	let response = r#"{"jsonrpc":"2.0","result":"0x5208","id":1}"#;
+
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+}
+
 #[test]
 fn rpc_eth_estimate_gas_default_block() {
 	let tester = EthTester::default();