Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127)
Browse files Browse the repository at this point in the history
* 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 #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
  • Loading branch information
dvdplm committed Nov 6, 2019
1 parent c3e1c5c commit 541e064
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 20 deletions.
22 changes: 12 additions & 10 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1017,15 +1017,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.
Expand All @@ -1035,9 +1036,9 @@ impl Client {
/// is unknown.
pub fn state_at(&self, id: BlockId) -> Option<State<StateDB>> {
// fast path for latest state.
match id.clone() {
BlockId::Latest => return Some(self.latest_state()),
_ => {},
if let BlockId::Latest = id {
let (state, _) = self.latest_state_and_header();
return Some(state)
}

let block_number = match self.block_number(id) {
Expand Down Expand Up @@ -1071,8 +1072,9 @@ impl Client {
}

/// Get a copy of the best block's state.
pub fn state(&self) -> Box<StateInfo> {
Box::new(self.latest_state()) as Box<_>
pub fn state(&self) -> impl StateInfo {
let (state, _) = self.latest_state_and_header();
state
}

/// Get info on the cache.
Expand Down Expand Up @@ -1525,8 +1527,8 @@ impl ImportBlock for Client {
impl StateClient for Client {
type State = State<::state_db::StateDB>;

fn latest_state(&self) -> Self::State {
Client::latest_state(self)
fn latest_state_and_header(&self) -> (Self::State, Header) {
Client::latest_state_and_header(self)
}

fn state_at(&self, id: BlockId) -> Option<Self::State> {
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,8 @@ impl StateClient for TestBlockChainClient {
// State will not be used by test client anyway, since all methods that accept state are mocked
type State = ();

fn latest_state(&self) -> Self::State {
()
fn latest_state_and_header(&self) -> (Self::State, Header) {
(TestState, self.best_block_header())
}

fn state_at(&self, _id: BlockId) -> Option<Self::State> {
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/client/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ pub trait StateClient {
/// Type representing chain state
type State: StateInfo;

/// Get a copy of the best block's state.
fn latest_state(&self) -> Self::State;
/// Get a copy of the best block's state and header.
fn latest_state_and_header(&self) -> (Self::State, Header);

/// Attempt to get a copy of a specific block's final state.
///
Expand Down
1 change: 1 addition & 0 deletions ethcore/src/tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use std::str::FromStr;
use std::sync::Arc;

use account_state::state::StateInfo;
use ethereum_types::{U256, Address};
use ethkey::KeyPair;
use hash::keccak;
Expand Down
33 changes: 29 additions & 4 deletions rpc/src/v1/impls/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use types::transaction::{SignedTransaction, LocalizedTransaction};
use types::BlockNumber as EthBlockNumber;
use types::encoded;
use types::filter::Filter as EthcoreFilter;
use types::header::Header;

use jsonrpc_core::{BoxFuture, Result};
use jsonrpc_core::futures::future;
Expand Down Expand Up @@ -432,23 +433,48 @@ 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::Num(num) => BlockId::Number(num).into(),
BlockNumber::Earliest => BlockId::Earliest.into(),
BlockNumber::Latest => BlockId::Latest.into(),

BlockNumber::Pending => {
let info = self.client.chain_info();

self.miner
.pending_state(info.best_block_number)
.map(|s| Box::new(s) as Box<StateInfo>)
.unwrap_or(Box::new(self.client.latest_state()) as Box<StateInfo>)
.map(|s| Box::new(s) 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 {
Expand All @@ -473,7 +499,6 @@ fn check_known<C>(client: &C, number: BlockNumber) -> Result<()> where C: BlockC

let id = match number {
BlockNumber::Pending => return Ok(()),

BlockNumber::Num(n) => BlockId::Number(n),
BlockNumber::Latest => BlockId::Latest,
BlockNumber::Earliest => BlockId::Earliest,
Expand Down
4 changes: 2 additions & 2 deletions rpc/src/v1/tests/helpers/miner_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ impl StateClient for TestMinerService {
// State will not be used by test client anyway, since all methods that accept state are mocked
type State = ();

fn latest_state(&self) -> Self::State {
()
fn latest_state_and_header(&self) -> (Self::State, Header) {
(TestState, Header::default())
}

fn state_at(&self, _id: BlockId) -> Option<Self::State> {
Expand Down
74 changes: 74 additions & 0 deletions rpc/src/v1/tests/mocked/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,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();
Expand Down Expand Up @@ -760,6 +797,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();
Expand Down

0 comments on commit 541e064

Please sign in to comment.