Revive, Estimate Gas with Binary Search

.gitignore

@@ -40,7 +40,8 @@ rls*.log
 runtime/wasm/target/
 substrate.code-workspace
 target/
+target.noindex/
 *.scale
 rustc-ice-*
 /.claude/settings.local.json
-*.local.md
+*.local.md

prdoc/pr_11000.prdoc

@@ -0,0 +1,16 @@
+title: Revive, Estimate Gas with Binary Search
+doc:
+- audience: Runtime Dev
+  description: |-
+    # Description
+
+    This PR implements binary search for the gas estimation logic in the eth-rpc which means that gas estimations are no longer just simple dry runs but that binary search is now used to find the smallest gas limit at which the transaction would run.
+
+    This PR closes https://github.com/paritytech/contract-issues/issues/217 and also _kind of_ fixes https://github.com/paritytech/contract-issues/issues/259 or at least makes it harder to trigger the case in which we observe it, but the underlying issue still exists.
+
+    The binary search algorithm implemented in this PR is as close as possible to that used in Geth
+crates:
+- name: pallet-revive
+  bump: minor
+- name: pallet-revive-eth-rpc
+  bump: minor

substrate/frame/revive/fixtures/contracts/ContractRequiringBinarySearchForGasEstimation.sol

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity >=0.8.4;
+
+contract ContractRequiringBinarySearchForGasEstimation {
+	function main() public view {
+		this.expensive_operation{gas: gasleft() / 2}();
+	}
+
+	function expensive_operation() external pure returns (uint256 sum) {
+		for (uint256 i = 0; i < 500; i++) {
+			sum += i * i;
+		}
+	}
+}

substrate/frame/revive/fixtures/contracts/ContractWithConsumeAllGas.sol

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity >=0.4.21;
+
+contract ContractWithConsumeAllGas {
+	function test() external {
+		assembly {
+			mstore(0, 0xcc572cf9) // main selector
+			mstore(32, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
+			mstore(64, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
+			let gas_value := div(mul(gas(), 1), 100)
+			let success := call(gas_value, address(), 0, 28, 68, 0, 0)
+
+			mstore(0, success)
+			return(0, 32)
+		}
+	}
+
+	function main(uint256 offset, uint256 len) external pure {
+		assembly {
+			// nullify memory ptr slot
+			mstore(0x40, 0)
+			revert(offset, len)
+		}
+	}
+}

substrate/frame/revive/rpc/src/client.rs

@@ -179,6 +179,10 @@ pub enum ClientError {
 	/// Transaction submission timeout.
 	#[error("Transaction submission timeout")]
 	Timeout,
+	/// All of the estimation methods `eth_estimate`, `eth_transact_with_config`, and
+	/// `eth_transact` were not found and therefore none of the estimation methods succeeded.
+	#[error("None of the estimation methods were found")]
+	NoEstimationMethodSucceeded,
 	/// Chain identity mismatch between stored genesis and connected node.
 	#[error("Genesis hash mismatch")]
 	ChainMismatch,

substrate/frame/revive/rpc/src/client/runtime_api.rs

@@ -20,7 +20,7 @@ use crate::{
 	client::Balance,
 	subxt_client::{self, SrcChainConfig},
 };
-use futures::TryFutureExt;
+use futures::{StreamExt, TryFutureExt, stream};
 use pallet_revive::{
 	DryRunConfig, EthTransactInfo,
 	evm::{
@@ -66,6 +66,68 @@ impl RuntimeApi {
 		Ok(result)
 	}
 
+	/// Estimates the minimum gas limit required for the transaction execution. Returns a [`U256`]
+	/// of the gas limit.
+	pub async fn estimate_gas(
+		&self,
+		tx: GenericTransaction,
+		block: BlockNumberOrTagOrHash,
+	) -> Result<U256, ClientError> {
+		let timestamp_override = match block {
+			BlockNumberOrTagOrHash::BlockTag(BlockTag::Pending) => {
+				Some(Timestamp::current().as_millis())
+			},
+			_ => None,
+		};
+
+		// Not all versions of pallet-revive have all of the runtime functions that we require. Thus
+		// we need to be able to perform the gas estimation through any of the runtime functions
+		// that the pallet may have available which is why we make use of this stream. The functions
+		// with higher priority are put at the start while the functions with lower priority are at
+		// the end.
+		let mut stream =
+			// Estimate through the `estimate_gas` function
+			stream::once(Box::pin(async {
+				let payload = subxt_client::apis().revive_api().eth_estimate_gas(
+					tx.clone().into(),
+					DryRunConfig::new(timestamp_override).into(),
+				);
+				self.0.call(payload).await.map(|value| value.map(|value| value.0))
+			}))
+			// Otherwise, estimate through `eth_transact_with_config`
+			.chain(stream::once(Box::pin(async {
+				let payload = subxt_client::apis().revive_api().eth_transact_with_config(
+					tx.clone().into(),
+					DryRunConfig::new(timestamp_override).into(),
+				);
+				self.0.call(payload).await.map(|value| value.map(|value| value.eth_gas))
+			})))
+			// Otherwise, estimate through `eth_transact`
+			.chain(stream::once(Box::pin(async {
+				let payload = subxt_client::apis().revive_api().eth_transact(tx.clone().into());
+				self.0.call(payload).await.map(|value| value.map(|value| value.eth_gas))
+			})));
+
+		while let Some(result) = stream.next().await {
+			match result {
+				Ok(estimation) => {
+					return estimation.map_err(|err| ClientError::TransactError(err.0));
+				},
+				Err(Metadata(MetadataError::RuntimeMethodNotFound(name))) => {
+					log::debug!(target: LOG_TARGET, "Method {name:?} not found falling back");
+				},
+				Err(subxt::Error::Rpc(subxt::error::RpcError::ClientError(
+					subxt::ext::subxt_rpcs::Error::User(UserError { message, .. }),
+				))) if message.contains("is not found") => {
+					log::debug!(target: LOG_TARGET, "{message:?} not found falling back")
+				},
+				Err(err) => return Err(err.into()),
+			}
+		}
+
+		Err(ClientError::NoEstimationMethodSucceeded.into())
+	}
+
 	/// Dry run a transaction and returns the [`EthTransactInfo`] for the transaction.
 	pub async fn dry_run(
 		&self,

substrate/frame/revive/rpc/src/lib.rs

@@ -164,24 +164,33 @@ impl EthRpcServer for EthRpcServerImpl {
 		Ok(receipt)
 	}
 
+	/// Performs gas estimations to find the lowest gas limit required to run the transaction.
+	///
+	/// This method implements the same gas estimation logic found in Geth which performs binary
+	/// search with some simple heuristics to find the smallest gas limit for the transaction.
 	async fn estimate_gas(
 		&self,
 		transaction: GenericTransaction,
 		block: Option<BlockNumberOrTag>,
 	) -> RpcResult<U256> {
 		log::trace!(target: LOG_TARGET, "estimate_gas transaction={transaction:?} block={block:?}");
+
 		let block = block.unwrap_or_else(|| {
 			if self.use_pending_for_estimate_gas {
 				BlockTag::Pending.into()
 			} else {
 				Default::default()
 			}
 		});
-		let hash = self.client.block_hash_for_tag(block.clone().into()).await?;
-		let runtime_api = self.client.runtime_api(hash);
-		let dry_run = runtime_api.dry_run(transaction, block.into()).await?;
-		log::trace!(target: LOG_TARGET, "estimate_gas result={dry_run:?}");
-		Ok(dry_run.eth_gas)
+		let hash = self.client.block_hash_for_tag(block.into()).await?;
+		let gas_estimate =
+			self.client.runtime_api(hash).estimate_gas(transaction, block.into()).await?;
+
+		log::trace!(
+			target: LOG_TARGET,
+			"estimate_gas result={gas_estimate:?}",
+		);
+		Ok(gas_estimate)
 	}
 
 	async fn call(