From 503eb5fe3a00b377dc19e4f44b9727561dea9a54 Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Fri, 2 Feb 2024 11:27:41 +0100 Subject: [PATCH 1/3] Respect block gas limit in gasEstimate buffer --- .../domain/competition/solution/settlement.rs | 9 ++++---- crates/driver/src/domain/eth/gas.rs | 2 +- crates/driver/src/infra/blockchain/mod.rs | 15 ++++++++++++- crates/driver/src/tests/cases/settle.rs | 17 ++++++++++++++ crates/driver/src/tests/setup/blockchain.rs | 2 ++ crates/driver/src/tests/setup/mod.rs | 22 ++++++++++++++++--- 6 files changed, 57 insertions(+), 10 deletions(-) diff --git a/crates/driver/src/domain/competition/solution/settlement.rs b/crates/driver/src/domain/competition/solution/settlement.rs index dc44160cff..a1e04cd598 100644 --- a/crates/driver/src/domain/competition/solution/settlement.rs +++ b/crates/driver/src/domain/competition/solution/settlement.rs @@ -42,7 +42,6 @@ pub struct Settlement { pub access_list: eth::AccessList, /// The gas parameters used by the settlement. pub gas: Gas, - /// See the [`Settlement::solutions`] method. solutions: HashMap, } @@ -135,7 +134,7 @@ impl Settlement { ) .await?; let price = eth.gas_price().await?; - let gas = Gas::new(gas, price); + let gas = Gas::new(gas, eth.gas_limit().await?, price); // Ensure that the solver has sufficient balance for the settlement to be mined. if eth.balance(settlement.solver).await? < gas.required_balance() { @@ -374,7 +373,7 @@ pub struct Gas { impl Gas { /// Computes settlement gas parameters given estimates for gas and gas /// price. - pub fn new(estimate: eth::Gas, price: eth::GasPrice) -> Self { + pub fn new(estimate: eth::Gas, limit: eth::Gas, price: eth::GasPrice) -> Self { // Specify a different gas limit than the estimated gas when executing a // settlement transaction. This allows the transaction to be resilient // to small variations in actual gas usage. @@ -382,13 +381,13 @@ impl Gas { // the end of execution, so we want to increase gas limit enough so // those solutions don't revert with out of gas error. const GAS_LIMIT_FACTOR: f64 = 2.0; - let limit = + let estimate_with_buffer = eth::U256::from_f64_lossy(eth::U256::to_f64_lossy(estimate.into()) * GAS_LIMIT_FACTOR) .into(); Self { estimate, - limit, + limit: std::cmp::min(limit, estimate_with_buffer), price, } } diff --git a/crates/driver/src/domain/eth/gas.rs b/crates/driver/src/domain/eth/gas.rs index b7b4fe01ae..c91bd60301 100644 --- a/crates/driver/src/domain/eth/gas.rs +++ b/crates/driver/src/domain/eth/gas.rs @@ -9,7 +9,7 @@ use { /// /// The amount of Ether that is paid in transaction fees is proportional to this /// amount as well as the transaction's [`EffectiveGasPrice`]. -#[derive(Debug, Default, Clone, Copy)] +#[derive(Debug, Default, Clone, Copy, Ord, Eq, PartialOrd, PartialEq)] pub struct Gas(pub U256); impl From for Gas { diff --git a/crates/driver/src/infra/blockchain/mod.rs b/crates/driver/src/infra/blockchain/mod.rs index 5ec076fedf..24829bda3d 100644 --- a/crates/driver/src/infra/blockchain/mod.rs +++ b/crates/driver/src/infra/blockchain/mod.rs @@ -1,7 +1,7 @@ use { self::contracts::ContractAt, crate::{boundary, domain::eth}, - ethcontract::dyns::DynWeb3, + ethcontract::{dyns::DynWeb3, BlockId, BlockNumber}, ethrpc::current_block::CurrentBlockStream, std::{fmt, sync::Arc}, thiserror::Error, @@ -177,6 +177,19 @@ impl Ethereum { self.gas.estimate().await } + pub async fn gas_limit(&self) -> Result { + self.web3 + .eth() + .block(BlockId::Number(BlockNumber::Latest)) + .await? + .map(|block| block.gas_limit.into()) + .ok_or_else(|| { + Error::Web3(web3::error::Error::InvalidResponse( + "No latest block".into(), + )) + }) + } + /// Returns the current [`eth::Ether`] balance of the specified account. pub async fn balance(&self, address: eth::Address) -> Result { self.web3 diff --git a/crates/driver/src/tests/cases/settle.rs b/crates/driver/src/tests/cases/settle.rs index 6391810820..f52aca73d4 100644 --- a/crates/driver/src/tests/cases/settle.rs +++ b/crates/driver/src/tests/cases/settle.rs @@ -72,3 +72,20 @@ async fn private_rpc_with_high_risk_solution() { let err = test.settle().await.err(); err.kind("FailedToSubmit"); } + +/// Checks that we can settle transactions that have a gas limit higher than +/// half the block size +#[tokio::test] +#[ignore] +async fn high_gas_limit() { + let test = tests::setup() + .name("high gas limit") + .pool(ab_pool()) + .order(ab_order()) + .solution(ab_solution().increase_gas(15_000_000)) + .done() + .await; + + test.solve().await.ok(); + test.settle().await.ok().await; +} diff --git a/crates/driver/src/tests/setup/blockchain.rs b/crates/driver/src/tests/setup/blockchain.rs index 5dce82f90b..3de1f79f1d 100644 --- a/crates/driver/src/tests/setup/blockchain.rs +++ b/crates/driver/src/tests/setup/blockchain.rs @@ -724,6 +724,8 @@ impl Node { .arg("0") // use 0 to let `anvil` use any open port .arg("--balance") .arg("1000000") + .arg("--gas-limit") + .arg("30000000") .stdout(std::process::Stdio::piped()) .spawn() .unwrap(); diff --git a/crates/driver/src/tests/setup/mod.rs b/crates/driver/src/tests/setup/mod.rs index 800b26a024..e92cdb37d4 100644 --- a/crates/driver/src/tests/setup/mod.rs +++ b/crates/driver/src/tests/setup/mod.rs @@ -350,9 +350,9 @@ pub struct Setup { pub enum Calldata { /// Set up the solver to return a solution with valid calldata. Valid { - /// Include additional meaningless bytes appended to the calldata. This - /// is useful for lowering the solution score in a controlled - /// way. + /// Include additional meaningless non-zero bytes appended to the + /// calldata. This is useful for lowering the solution score in + /// a controlled way. additional_bytes: usize, }, /// Set up the solver to return a solution with bogus calldata. @@ -380,6 +380,22 @@ impl Solution { } } + pub fn increase_gas(self, units: usize) -> Self { + // non-zero bytes costs 16 gas + let additional_bytes = units / 16; + Self { + calldata: match self.calldata { + Calldata::Valid { + additional_bytes: existing, + } => Calldata::Valid { + additional_bytes: existing + additional_bytes, + }, + Calldata::Invalid => Calldata::Invalid, + }, + ..self + } + } + /// Make the solution return invalid calldata. pub fn invalid(self) -> Self { Self { From 80e65777a3aeb0e496a781fe690d00201a4313d0 Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Mon, 5 Feb 2024 09:07:49 +0100 Subject: [PATCH 2/3] Moving gas limit into current block stream (Fetch.sol) --- crates/contracts/artifacts/FetchBlock.json | 2 +- crates/contracts/solidity/FetchBlock.sol | 3 ++- .../src/domain/competition/solution/settlement.rs | 2 +- crates/driver/src/infra/blockchain/mod.rs | 15 +++------------ crates/ethrpc/src/current_block/mod.rs | 4 +++- crates/ethrpc/src/current_block/retriever.rs | 5 ++++- 6 files changed, 14 insertions(+), 17 deletions(-) diff --git a/crates/contracts/artifacts/FetchBlock.json b/crates/contracts/artifacts/FetchBlock.json index a952327598..c4b017e60b 100644 --- a/crates/contracts/artifacts/FetchBlock.json +++ b/crates/contracts/artifacts/FetchBlock.json @@ -1 +1 @@ -{"abi":[{"inputs":[],"stateMutability":"nonpayable","type":"constructor"}],"bytecode":"0x6080604052348015600f57600080fd5b506000804311601e5760006027565b60276001436084565b9050804060008260375760006042565b60406001846084565b405b604080516020810186905290810184905260608101829052426080820181905291925060009060a0016040516020818303038152906040529050805181602001f35b8181038181111560a457634e487b7160e01b600052601160045260246000fd5b9291505056fe","deployedBytecode":"0x6080604052600080fdfea164736f6c6343000811000a","devdoc":{"methods":{}},"userdoc":{"methods":{}}} +{"abi":[{"inputs":[],"stateMutability":"nonpayable","type":"constructor"}],"bytecode":"0x6080604052348015600f57600080fd5b506000804311601e5760006027565b6027600143608e565b9050804060008260375760006042565b6040600184608e565b405b60408051602081018690529081018490526060810182905242608082018190524560a08301819052929350919060009060c0016040516020818303038152906040529050805181602001f35b8181038181111560ae57634e487b7160e01b600052601160045260246000fd5b9291505056fe","deployedBytecode":"0x6080604052600080fdfea164736f6c6343000811000a","devdoc":{"methods":{}},"userdoc":{"methods":{}}} diff --git a/crates/contracts/solidity/FetchBlock.sol b/crates/contracts/solidity/FetchBlock.sol index a38458028a..9384920990 100644 --- a/crates/contracts/solidity/FetchBlock.sol +++ b/crates/contracts/solidity/FetchBlock.sol @@ -16,8 +16,9 @@ contract FetchBlock { ? blockhash(blockNumber - 1) : bytes32(0); uint timestamp = block.timestamp; + uint gasLimit = block.gaslimit; - bytes memory result = abi.encode(blockNumber, blockHash, parentHash, timestamp); + bytes memory result = abi.encode(blockNumber, blockHash, parentHash, timestamp, gasLimit); assembly { return(add(32, result), mload(result)) } diff --git a/crates/driver/src/domain/competition/solution/settlement.rs b/crates/driver/src/domain/competition/solution/settlement.rs index a1e04cd598..5e58d990ad 100644 --- a/crates/driver/src/domain/competition/solution/settlement.rs +++ b/crates/driver/src/domain/competition/solution/settlement.rs @@ -134,7 +134,7 @@ impl Settlement { ) .await?; let price = eth.gas_price().await?; - let gas = Gas::new(gas, eth.gas_limit().await?, price); + let gas = Gas::new(gas, eth.gas_limit(), price); // Ensure that the solver has sufficient balance for the settlement to be mined. if eth.balance(settlement.solver).await? < gas.required_balance() { diff --git a/crates/driver/src/infra/blockchain/mod.rs b/crates/driver/src/infra/blockchain/mod.rs index 24829bda3d..6227ce1562 100644 --- a/crates/driver/src/infra/blockchain/mod.rs +++ b/crates/driver/src/infra/blockchain/mod.rs @@ -1,7 +1,7 @@ use { self::contracts::ContractAt, crate::{boundary, domain::eth}, - ethcontract::{dyns::DynWeb3, BlockId, BlockNumber}, + ethcontract::dyns::DynWeb3, ethrpc::current_block::CurrentBlockStream, std::{fmt, sync::Arc}, thiserror::Error, @@ -177,17 +177,8 @@ impl Ethereum { self.gas.estimate().await } - pub async fn gas_limit(&self) -> Result { - self.web3 - .eth() - .block(BlockId::Number(BlockNumber::Latest)) - .await? - .map(|block| block.gas_limit.into()) - .ok_or_else(|| { - Error::Web3(web3::error::Error::InvalidResponse( - "No latest block".into(), - )) - }) + pub fn gas_limit(&self) -> eth::Gas { + self.current_block.borrow().gas_limit.into() } /// Returns the current [`eth::Ether`] balance of the specified account. diff --git a/crates/ethrpc/src/current_block/mod.rs b/crates/ethrpc/src/current_block/mod.rs index 622f3e5c08..dd4a42ade3 100644 --- a/crates/ethrpc/src/current_block/mod.rs +++ b/crates/ethrpc/src/current_block/mod.rs @@ -5,7 +5,7 @@ pub mod retriever; use { crate::Web3, anyhow::{anyhow, ensure, Context as _, Result}, - primitive_types::H256, + primitive_types::{H256, U256}, std::{sync::Arc, time::Duration}, tokio::sync::watch, tokio_stream::wrappers::WatchStream, @@ -52,6 +52,7 @@ pub struct BlockInfo { pub hash: H256, pub parent_hash: H256, pub timestamp: u64, + pub gas_limit: U256, } impl TryFrom> for BlockInfo { @@ -63,6 +64,7 @@ impl TryFrom> for BlockInfo { hash: value.hash.context("block missing hash")?, parent_hash: value.parent_hash, timestamp: value.timestamp.as_u64(), + gas_limit: value.gas_limit, }) } } diff --git a/crates/ethrpc/src/current_block/retriever.rs b/crates/ethrpc/src/current_block/retriever.rs index b17851913a..41b26a4c28 100644 --- a/crates/ethrpc/src/current_block/retriever.rs +++ b/crates/ethrpc/src/current_block/retriever.rs @@ -78,6 +78,7 @@ impl BlockRetrieving for BlockRetriever { hash: fetch.parent_hash, timestamp: fetch.timestamp, parent_hash: call.hash, + gas_limit: fetch.gas_limit, }) } else { bail!("large differential between eth_getBlock {fetch:?} and eth_call {call:?}"); @@ -94,19 +95,21 @@ impl BlockRetrieving for BlockRetriever { } /// Decodes the return data from the `FetchBlock` contract. -fn decode(return_data: [u8; 128]) -> Result { +fn decode(return_data: [u8; 160]) -> Result { let number = u64::try_from(U256::from_big_endian(&return_data[0..32])) .ok() .context("block number overflows u64")?; let hash = H256::from_slice(&return_data[32..64]); let parent_hash = H256::from_slice(&return_data[64..96]); let timestamp = U256::from_big_endian(&return_data[96..128]).as_u64(); + let gas_limit = U256::from_big_endian(&return_data[128..160]); Ok(BlockInfo { number, hash, parent_hash, timestamp, + gas_limit, }) } From 084aaa3a35c06f99b67f13f4803916620bdf81f7 Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Mon, 5 Feb 2024 14:43:40 +0100 Subject: [PATCH 3/3] @MartinquaXD --- crates/driver/src/tests/setup/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/driver/src/tests/setup/mod.rs b/crates/driver/src/tests/setup/mod.rs index e92cdb37d4..ac97b81a7d 100644 --- a/crates/driver/src/tests/setup/mod.rs +++ b/crates/driver/src/tests/setup/mod.rs @@ -380,9 +380,10 @@ impl Solution { } } + /// Increase the solution gas consumption by at least `units`. pub fn increase_gas(self, units: usize) -> Self { // non-zero bytes costs 16 gas - let additional_bytes = units / 16; + let additional_bytes = (units / 16) + 1; Self { calldata: match self.calldata { Calldata::Valid {