Use binary chop to estimate gas accurately (#4100)

* Initial sketch.

* Building.

* Fix a few things.

* Fix issue, add tracing.

* Address grumbles

* Raise upper limit if needed

* Fix test.

ethcore/src/client/client.rs

@@ -848,7 +848,7 @@ impl BlockChainClient for Client {
 			difficulty: header.difficulty(),
 			last_hashes: last_hashes,
 			gas_used: U256::zero(),
-			gas_limit: U256::max_value(),
+			gas_limit: header.gas_limit(),
 		};
 		// that's just a copy of the state.
 		let mut state = self.state_at(block).ok_or(CallError::StatePruned)?;
@@ -873,6 +873,81 @@ impl BlockChainClient for Client {
 		Ok(ret)
 	}
 
+	fn estimate_gas(&self, t: &SignedTransaction, block: BlockId) -> Result<U256, CallError> {
+		let header = self.block_header(block).ok_or(CallError::StatePruned)?;
+		let last_hashes = self.build_last_hashes(header.parent_hash());
+		let env_info = EnvInfo {
+			number: header.number(),
+			author: header.author(),
+			timestamp: header.timestamp(),
+			difficulty: header.difficulty(),
+			last_hashes: last_hashes,
+			gas_used: U256::zero(),
+			gas_limit: header.gas_limit(),
+		};
+		// that's just a copy of the state.
+		let mut original_state = self.state_at(block).ok_or(CallError::StatePruned)?;
+		let sender = t.sender().map_err(|e| {
+			let message = format!("Transaction malformed: {:?}", e);
+			ExecutionError::TransactionMalformed(message)
+		})?;
+		let balance = original_state.balance(&sender);
+		let needed_balance = t.value + t.gas * t.gas_price;
+		if balance < needed_balance {
+			// give the sender a sufficient balance
+			original_state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty);
+		}
+		let options = TransactOptions { tracing: true, vm_tracing: false, check_nonce: false };
+		let mut tx = t.clone();
+
+		let mut cond = |gas| {
+			let mut state = original_state.clone();
+			tx.gas = gas;
+			Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm)
+				.transact(&tx, options.clone())
+				.map(|r| r.trace[0].result.succeeded())
+				.unwrap_or(false)
+		};
+
+		let mut upper = env_info.gas_limit;
+		if !cond(upper) {
+			// impossible at block gas limit - try `UPPER_CEILING` instead.
+			// TODO: consider raising limit by powers of two.
+			const UPPER_CEILING: u64 = 1_000_000_000_000u64;
+			upper = UPPER_CEILING.into();
+			if !cond(upper) {
+				trace!(target: "estimate_gas", "estimate_gas failed with {}", upper);
+				return Err(CallError::Execution(ExecutionError::Internal))
+			}
+		}
+		let lower = t.gas_required(&self.engine.schedule(&env_info)).into();
+		if cond(lower) {
+			trace!(target: "estimate_gas", "estimate_gas succeeded with {}", lower);
+			return Ok(lower)
+		}
+
+		/// Find transition point between `lower` and `upper` where `cond` changes from `false` to `true`.
+		/// Returns the lowest value between `lower` and `upper` for which `cond` returns true.
+		/// We assert: `cond(lower) = false`, `cond(upper) = true`
+		fn binary_chop<F>(mut lower: U256, mut upper: U256, mut cond: F) -> U256 where F: FnMut(U256) -> bool {
+			while upper - lower > 1.into() {
+				let mid = (lower + upper) / 2.into();
+				trace!(target: "estimate_gas", "{} .. {} .. {}", lower, mid, upper);
+				let c = cond(mid);
+				match c {
+					true => upper = mid,
+					false => lower = mid,
+				};
+				trace!(target: "estimate_gas", "{} => {} .. {}", c, lower, upper);
+			}
+			upper
+		}
+
+		// binary chop to non-excepting call with gas somewhere between 21000 and block gas limit
+		trace!(target: "estimate_gas", "estimate_gas chopping {} .. {}", lower, upper);
+		Ok(binary_chop(lower, upper, cond))
+	}
+
 	fn replay(&self, id: TransactionId, analytics: CallAnalytics) -> Result<Executed, CallError> {
 		let address = self.transaction_address(id).ok_or(CallError::TransactionNotFound)?;
 		let header = self.block_header(BlockId::Hash(address.block_hash)).ok_or(CallError::StatePruned)?;

ethcore/src/client/test_client.rs

@@ -379,6 +379,10 @@ impl BlockChainClient for TestBlockChainClient {
 		self.execution_result.read().clone().unwrap()
 	}
 
+	fn estimate_gas(&self, _t: &SignedTransaction, _block: BlockId) -> Result<U256, CallError> {
+		Ok(21000.into())
+	}
+
 	fn replay(&self, _id: TransactionId, _analytics: CallAnalytics) -> Result<Executed, CallError> {
 		self.execution_result.read().clone().unwrap()
 	}

ethcore/src/client/traits.rs

@@ -184,6 +184,9 @@ pub trait BlockChainClient : Sync + Send {
 	/// Makes a non-persistent transaction call.
 	fn call(&self, t: &SignedTransaction, block: BlockId, analytics: CallAnalytics) -> Result<Executed, CallError>;
 
+	/// Estimates how much gas will be necessary for a call.
+	fn estimate_gas(&self, t: &SignedTransaction, block: BlockId) -> Result<U256, CallError>;
+
 	/// Replays a given transaction for inspection.
 	fn replay(&self, t: TransactionId, analytics: CallAnalytics) -> Result<Executed, CallError>;
 

ethcore/src/executive.rs

@@ -45,7 +45,7 @@ pub fn contract_address(address: &Address, nonce: &U256) -> Address {
 }
 
 /// Transaction execution options.
-#[derive(Default)]
+#[derive(Default, Copy, Clone, PartialEq)]
 pub struct TransactOptions {
 	/// Enable call tracing.
 	pub tracing: bool,

ethcore/src/miner/miner.rs

@@ -673,7 +673,7 @@ impl MinerService for Miner {
 		}
 	}
 
-	fn call(&self, chain: &MiningBlockChainClient, t: &SignedTransaction, analytics: CallAnalytics) -> Result<Executed, CallError> {
+	fn call(&self, client: &MiningBlockChainClient, t: &SignedTransaction, analytics: CallAnalytics) -> Result<Executed, CallError> {
 		let sealing_work = self.sealing_work.lock();
 		match sealing_work.queue.peek_last_ref() {
 			Some(work) => {
@@ -681,7 +681,7 @@ impl MinerService for Miner {
 
 				// TODO: merge this code with client.rs's fn call somwhow.
 				let header = block.header();
-				let last_hashes = Arc::new(chain.last_hashes());
+				let last_hashes = Arc::new(client.last_hashes());
 				let env_info = EnvInfo {
 					number: header.number(),
 					author: *header.author(),
@@ -706,16 +706,14 @@ impl MinerService for Miner {
 					state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty);
 				}
 				let options = TransactOptions { tracing: analytics.transaction_tracing, vm_tracing: analytics.vm_tracing, check_nonce: false };
-				let mut ret = Executive::new(&mut state, &env_info, &*self.engine, chain.vm_factory()).transact(t, options)?;
+				let mut ret = Executive::new(&mut state, &env_info, &*self.engine, client.vm_factory()).transact(t, options)?;
 
 				// TODO gav move this into Executive.
 				ret.state_diff = original_state.map(|original| state.diff_from(original));
 
 				Ok(ret)
 			},
-			None => {
+			None => client.call(t, BlockId::Latest, analytics)
-				chain.call(t, BlockId::Latest, analytics)
-			}
 		}
 	}
 

ethcore/src/types/trace_types/trace.rs

@@ -391,6 +391,14 @@ impl Res {
 			Res::Call(_) | Res::FailedCall(_) | Res::FailedCreate(_) | Res::None => Default::default(),
 		}
 	}
+
+	/// Did this call fail?
+	pub fn succeeded(&self) -> bool {
+		match *self {
+			Res::Call(_) | Res::Create(_) => true,
+			_ => false,
+		}
+	}
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -561,4 +569,3 @@ impl Decodable for VMTrace {
 		Ok(res)
 	}
 }
-

ethcore/src/types/transaction.rs

@@ -16,7 +16,7 @@
 
 //! Transaction data structure.
 
-use std::ops::Deref;
+use std::ops::{Deref, DerefMut};
 use std::cell::*;
 use rlp::*;
 use util::sha3::Hashable;
@@ -239,6 +239,12 @@ impl Deref for SignedTransaction {
 	}
 }
 
+impl DerefMut for SignedTransaction {
+	fn deref_mut(&mut self) -> &mut Self::Target {
+		&mut self.unsigned
+	}
+}
+
 impl Decodable for SignedTransaction {
 	fn decode<D>(decoder: &D) -> Result<Self, DecoderError> where D: Decoder {
 		let d = decoder.as_rlp();

rpc/src/v1/impls/eth.rs

@@ -671,13 +671,8 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 
 		let request = CallRequest::into(request);
 		let signed = self.sign_call(request)?;
-		let result = match num.0 {
+		take_weak!(self.client).estimate_gas(&signed, num.0.into())
-			BlockNumber::Pending => take_weak!(self.miner).call(&*take_weak!(self.client), &signed, Default::default()),
+			.map(Into::into)
-			num => take_weak!(self.client).call(&signed, num.into(), Default::default()),
-		};
-
-		result
-			.map(|res| (res.gas_used + res.refunded).into())
 			.map_err(errors::from_call_error)
 	}
 

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

@@ -691,7 +691,7 @@ fn rpc_eth_estimate_gas() {
 		"latest"],
 		"id": 1
 	}"#;
-	let response = r#"{"jsonrpc":"2.0","result":"0xff35","id":1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":"0x5208","id":1}"#;
 
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 }
@@ -725,7 +725,7 @@ fn rpc_eth_estimate_gas_default_block() {
 		}],
 		"id": 1
 	}"#;
-	let response = r#"{"jsonrpc":"2.0","result":"0xff35","id":1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":"0x5208","id":1}"#;
 
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 }