From 57030ce7193eb3719f470e3993dcefb8492be4e5 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Mon, 8 Oct 2018 21:30:46 +0200 Subject: [PATCH] fix (light/provider) : Make `read_only executions` read-only (#9591) * `ExecutionsRequest` from light-clients as read-only This changes so all `ExecutionRequests` from light-clients are executed as read-only which the `virtual``flag == true ensures. This boost up the current transaction to always succeed Note, this only affects `eth_estimateGas` and `eth_call` AFAIK. * grumbles(revert renaming) : TransactionProof * grumbles(trace) : remove incorrect trace * grumbles(state/prove_tx) : explicit `virt` Remove the boolean flag to determine that a `state::prove_transaction` whether it should be executed in a virtual context or not. Because of that also rename the function to `state::prove_transction_virtual` to make more clear --- ethcore/src/client/client.rs | 3 +-- ethcore/src/spec/spec.rs | 3 +-- ethcore/src/state/mod.rs | 7 +++---- rpc/src/v1/helpers/light_fetch.rs | 20 ++++++++++---------- rpc/src/v1/impls/light/eth.rs | 4 ++-- 5 files changed, 17 insertions(+), 20 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 9e72eb36bb5..aa5a3019cbe 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -2315,14 +2315,13 @@ impl ProvingBlockChainClient for Client { env_info.gas_limit = transaction.gas.clone(); let mut jdb = self.state_db.read().journal_db().boxed_clone(); - state::prove_transaction( + state::prove_transaction_virtual( jdb.as_hashdb_mut(), header.state_root().clone(), &transaction, self.engine.machine(), &env_info, self.factories.clone(), - false, ) } diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index d88bdb83364..f0243be73c1 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -829,14 +829,13 @@ impl Spec { data: d, }.fake_sign(from); - let res = ::state::prove_transaction( + let res = ::state::prove_transaction_virtual( db.as_hashdb_mut(), *genesis.state_root(), &tx, self.engine.machine(), &env_info, factories.clone(), - true, ); res.map(|(out, proof)| { diff --git a/ethcore/src/state/mod.rs b/ethcore/src/state/mod.rs index 0da61425442..0118f4d850c 100644 --- a/ethcore/src/state/mod.rs +++ b/ethcore/src/state/mod.rs @@ -222,17 +222,16 @@ pub fn check_proof( } } -/// Prove a transaction on the given state. +/// Prove a `virtual` transaction on the given state. /// Returns `None` when the transacion could not be proved, /// and a proof otherwise. -pub fn prove_transaction + Send + Sync>( +pub fn prove_transaction_virtual + Send + Sync>( db: H, root: H256, transaction: &SignedTransaction, machine: &Machine, env_info: &EnvInfo, factories: Factories, - virt: bool, ) -> Option<(Bytes, Vec)> { use self::backend::Proving; @@ -250,7 +249,7 @@ pub fn prove_transaction + Send + Sync>( }; let options = TransactOptions::with_no_tracing().dont_check_nonce().save_output_from_contract(); - match state.execute(env_info, machine, transaction, options, virt) { + match state.execute(env_info, machine, transaction, options, true) { Err(ExecutionError::Internal(_)) => None, Err(e) => { trace!(target: "state", "Proved call failed: {}", e); diff --git a/rpc/src/v1/helpers/light_fetch.rs b/rpc/src/v1/helpers/light_fetch.rs index dd87a5011df..2ec3af631aa 100644 --- a/rpc/src/v1/helpers/light_fetch.rs +++ b/rpc/src/v1/helpers/light_fetch.rs @@ -188,7 +188,7 @@ impl LightFetch { } /// Helper for getting proved execution. - pub fn proved_execution(&self, req: CallRequest, num: Trailing) -> impl Future + Send { + pub fn proved_read_only_execution(&self, req: CallRequest, num: Trailing) -> impl Future + Send { const DEFAULT_GAS_PRICE: u64 = 21_000; // starting gas when gas not provided. const START_GAS: u64 = 50_000; @@ -253,14 +253,14 @@ impl LightFetch { _ => return Either::A(future::err(errors::unknown_block())), }; - Either::B(execute_tx(gas_known, ExecuteParams { - from: from, - tx: tx, - hdr: hdr, - env_info: env_info, + Either::B(execute_read_only_tx(gas_known, ExecuteParams { + from, + tx, + hdr, + env_info, engine: client.engine().clone(), - on_demand: on_demand, - sync: sync, + on_demand, + sync, })) })) } @@ -599,10 +599,10 @@ struct ExecuteParams { // has a peer execute the transaction with given params. If `gas_known` is false, // this will double the gas on each `OutOfGas` error. -fn execute_tx(gas_known: bool, params: ExecuteParams) -> impl Future + Send { +fn execute_read_only_tx(gas_known: bool, params: ExecuteParams) -> impl Future + Send { if !gas_known { Box::new(future::loop_fn(params, |mut params| { - execute_tx(true, params.clone()).and_then(move |res| { + execute_read_only_tx(true, params.clone()).and_then(move |res| { match res { Ok(executed) => { // TODO: how to distinguish between actual OOG and diff --git a/rpc/src/v1/impls/light/eth.rs b/rpc/src/v1/impls/light/eth.rs index 1f6a54da104..d90deb64815 100644 --- a/rpc/src/v1/impls/light/eth.rs +++ b/rpc/src/v1/impls/light/eth.rs @@ -399,7 +399,7 @@ impl Eth for EthClient { } fn call(&self, req: CallRequest, num: Trailing) -> BoxFuture { - Box::new(self.fetcher().proved_execution(req, num).and_then(|res| { + Box::new(self.fetcher().proved_read_only_execution(req, num).and_then(|res| { match res { Ok(exec) => Ok(exec.output.into()), Err(e) => Err(errors::execution(e)), @@ -409,7 +409,7 @@ impl Eth for EthClient { fn estimate_gas(&self, req: CallRequest, num: Trailing) -> BoxFuture { // TODO: binary chop for more accurate estimates. - Box::new(self.fetcher().proved_execution(req, num).and_then(|res| { + Box::new(self.fetcher().proved_read_only_execution(req, num).and_then(|res| { match res { Ok(exec) => Ok((exec.refunded + exec.gas_used).into()), Err(e) => Err(errors::execution(e)),