rpcdaemon: align eth_estimateGas implementation to erigon (#2787)

Author: lupin012

silkworm/core/execution/evm.cpp

@@ -98,7 +98,7 @@ CallResult EVM::execute(const Transaction& txn, uint64_t gas) noexcept {
 
     const auto gas_left = static_cast<uint64_t>(res.gas_left);
     const auto gas_refund = static_cast<uint64_t>(res.gas_refund);
-    return {ValidationResult::kOk, res.status_code, gas_left, gas_refund, {res.output_data, res.output_size}};
+    return {ValidationResult::kOk, res.status_code, gas_left, gas_refund, std::nullopt, {res.output_data, res.output_size}};
 }
 
 evmc::Result EVM::create(const evmc_message& message) noexcept {

silkworm/core/execution/evm.hpp

@@ -44,6 +44,7 @@ struct CallResult {
     evmc_status_code status{EVMC_SUCCESS};
     uint64_t gas_left{0};
     uint64_t gas_refund{0};
+    std::optional<uint64_t> gas_used{0};
     Bytes data;
     std::string error_message;
 };

silkworm/core/execution/evm_test.cpp

@@ -571,7 +571,7 @@ class TestTracer : public EvmTracer {
         execution_end_called_ = true;
         const auto gas_left = static_cast<uint64_t>(res.gas_left);
         const auto gas_refund = static_cast<uint64_t>(res.gas_refund);
-        result_ = {ValidationResult::kOk, res.status_code, gas_left, gas_refund, {res.output_data, res.output_size}};
+        result_ = {ValidationResult::kOk, res.status_code, gas_left, gas_refund, std::nullopt, {res.output_data, res.output_size}};
         if (contract_address_ && !pc_stack_.empty()) {
             const auto pc = pc_stack_.back();
             storage_stack_[pc] =

silkworm/core/execution/processor.cpp

@@ -325,8 +325,11 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector<st
     uint64_t gas_left{result.gas_left};
     uint64_t gas_used{txn.gas_limit - result.gas_left};
 
+    uint64_t gas_refund = 0;
     if (refund && !evm().bailout) {
-        gas_used = txn.gas_limit - calculate_refund_gas(txn, result.gas_left, result.gas_refund);
+        const uint64_t gas_left_plus_refund = calculate_refund_gas(txn, result.gas_left, result.gas_refund);
+        gas_refund = gas_left_plus_refund - result.gas_left;
+        gas_used = txn.gas_limit - gas_left_plus_refund;
         //  EIP-7623: Increase calldata cost
         if (evm().revision() >= EVMC_PRAGUE) {
             gas_used = std::max(gas_used, protocol::floor_cost(txn));
@@ -349,7 +352,7 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector<st
 
     evm_.remove_tracers();
 
-    return {ValidationResult::kOk, result.status, gas_left, gas_used, result.data, result.error_message};
+    return {ValidationResult::kOk, result.status, gas_left, gas_refund, gas_used, result.data, result.error_message};
 }
 
 void ExecutionProcessor::reset() {

silkworm/rpc/commands/eth_api.cpp

@@ -885,11 +885,12 @@ Task<void> EthereumRpcApi::handle_eth_estimate_gas(const nlohmann::json& request
     const auto chain_storage{tx->create_storage()};
     rpc::BlockReader block_reader{*chain_storage, *tx, state_cache_};
 
-    std::optional<BlockNum> block_num_for_gas_limit;
+    std::string block_id;
     if (params.size() >= 2) {
-        const auto block_id = params[1].get<std::string>();
+        block_id = params[1].get<std::string>();
         SILK_DEBUG << "block_id: " << block_id;
-        block_num_for_gas_limit = co_await block_reader.get_block_num(block_id);
+    } else {
+        block_id = kLatestBlockId;
     }
 
     AccountsOverrides accounts_overrides;
@@ -899,19 +900,18 @@ Task<void> EthereumRpcApi::handle_eth_estimate_gas(const nlohmann::json& request
     }
 
     try {
-        const BlockNumOrHash block_num_or_hash{kLatestBlockId};
-
         const auto chain_config = co_await chain_storage->read_chain_config();
-        const auto latest_block_num = co_await block_reader.get_block_num(kLatestBlockId);
-        SILK_DEBUG << "chain_id: " << chain_config.chain_id << ", latest_block_num: " << latest_block_num;
+        const auto req_block_num = co_await block_reader.get_block_num(block_id);
+        SILK_DEBUG << "chain_id: " << chain_config.chain_id << ", block_num: " << req_block_num;
 
-        const auto latest_block_with_hash = co_await core::read_block_by_number(*block_cache_, *chain_storage, latest_block_num);
-        if (!latest_block_with_hash) {
-            reply = make_json_error(request, 100, "block not found");
+        const auto block_with_hash = co_await core::read_block_by_number(*block_cache_, *chain_storage, req_block_num);
+        if (!block_with_hash) {
+            std::string error_msg = "could not find the block " + std::to_string(req_block_num) + " in cache or db";
+            reply = make_json_error(request, -32000, error_msg);
             co_await tx->close();  // RAII not (yet) available with coroutines
             co_return;
         }
-        const auto latest_block = latest_block_with_hash->block;
+        const auto block = block_with_hash->block;
 
         rpc::BlockHeaderProvider block_header_provider = [&chain_storage](BlockNum block_num) {
             return chain_storage->read_canonical_header(block_num);
@@ -923,7 +923,7 @@ Task<void> EthereumRpcApi::handle_eth_estimate_gas(const nlohmann::json& request
         };
 
         rpc::EstimateGasOracle estimate_gas_oracle{block_header_provider, account_reader, chain_config, workers_, *tx, *chain_storage, accounts_overrides};
-        const auto estimated_gas = co_await estimate_gas_oracle.estimate_gas(call, latest_block, /*txn_id=*/std::nullopt, block_num_for_gas_limit);
+        const auto estimated_gas = co_await estimate_gas_oracle.estimate_gas(call, block, /*txn_id=*/std::nullopt);
 
         reply = make_json_content(request, to_quantity(estimated_gas));
     } catch (const std::invalid_argument&) {

silkworm/rpc/core/estimate_gas_oracle.cpp

@@ -26,26 +26,31 @@
 
 namespace silkworm::rpc {
 
-Task<intx::uint256> EstimateGasOracle::estimate_gas(const Call& call, const silkworm::Block& block, std::optional<TxnId> txn_id, std::optional<BlockNum> block_num_for_gas_limit) {
+Task<intx::uint256> EstimateGasOracle::estimate_gas(const Call& call, const silkworm::Block& block, std::optional<TxnId> txn_id) {
     SILK_DEBUG << "EstimateGasOracle::estimate_gas called";
 
     const auto block_num = block.header.number;
 
     uint64_t hi = 0;
-    uint64_t lo = kTxGas - 1;
+    uint64_t lo;
 
     if (call.gas.value_or(0) >= kTxGas) {
         SILK_DEBUG << "Set gas limit using call args: " << call.gas.value_or(0);
         hi = call.gas.value();
     } else {
-        const auto header = co_await block_header_provider_(block_num_for_gas_limit.value_or(block_num));
+        const auto header = co_await block_header_provider_(block_num);
         if (!header) {
             co_return 0;
         }
         hi = header->gas_limit;
         SILK_DEBUG << "Set gas limit using block: " << header->gas_limit;
     }
 
+    if (hi > kGasCap) {
+        SILK_WARN << "caller gas above allowance, capping: requested " << hi << ", cap " << kGasCap;
+        hi = kGasCap;
+    }
+
     std::optional<intx::uint256> gas_price = call.gas_price;
     if (gas_price && gas_price != 0) {
         evmc::address from = call.from.value_or(evmc::address{0});
@@ -71,14 +76,6 @@ Task<intx::uint256> EstimateGasOracle::estimate_gas(const Call& call, const silk
         }
     }
 
-    if (hi > kGasCap) {
-        SILK_WARN << "caller gas above allowance, capping: requested " << hi << ", cap " << kGasCap;
-        hi = kGasCap;
-    }
-    auto cap = hi;
-
-    SILK_DEBUG << "hi: " << hi << ", lo: " << lo << ", cap: " << cap;
-
     auto this_executor = co_await boost::asio::this_coro::executor;
 
     execution::StateFactory state_factory{transaction_, /*state_cache=*/nullptr};
@@ -88,33 +85,40 @@ Task<intx::uint256> EstimateGasOracle::estimate_gas(const Call& call, const silk
 
         ExecutionResult result{evmc_status_code::EVMC_SUCCESS};
         silkworm::Transaction transaction{call.to_transaction()};
+
+        auto state_overrides = std::make_shared<state::OverrideState>(*state, accounts_overrides_);
+        EVMExecutor executor{block, config_, workers_, state_overrides};
+        transaction.gas_limit = hi;
+        result = try_execution(executor, transaction);
+        if (!result.success()) {
+            return result;
+        }
+
+        // Assuming a contract can freely run all the instructions, we have
+        // the true amount of gas it wants to consume to execute fully.
+        // We want to ensure that the gas used doesn't fall below this
+        auto true_gas = result.gas_used.value_or(0);
+        uint64_t refund = result.gas_refund.value_or(0);
+        lo = std::min(true_gas + refund - 1, kTxGas - 1);
+
+        SILK_DEBUG << "hi: " << hi << ", lo: " << lo;
+
         while (lo + 1 < hi) {
-            auto state_overrides = std::make_shared<state::OverrideState>(*state, accounts_overrides_);
-            EVMExecutor executor{block, config_, workers_, state_overrides};
+            state_overrides = std::make_shared<state::OverrideState>(*state, accounts_overrides_);
+            EVMExecutor curr_executor{block, config_, workers_, state_overrides};
             auto mid = (hi + lo) / 2;
             transaction.gas_limit = mid;
-            result = try_execution(executor, transaction);
-            if (result.success()) {
-                hi = mid;
-            } else {
-                if (result.pre_check_error_code && result.pre_check_error_code != PreCheckErrorCode::kIntrinsicGasTooLow) {
-                    result.status_code = evmc_status_code::EVMC_SUCCESS;
-                    return result;
-                }
+            result = try_execution(curr_executor, transaction);
+            if (result.pre_check_error_code) {
+                break;
+            }
+            if (!result.success() || result.gas_used.value_or(0) < true_gas) {
                 lo = mid;
+            } else {
+                hi = mid;
             }
         }
-
-        if (hi == cap) {
-            auto state_overrides = std::make_shared<state::OverrideState>(*state, accounts_overrides_);
-            EVMExecutor executor{block, config_, workers_, state_overrides};
-            transaction.gas_limit = hi;
-            result = try_execution(executor, transaction);
-            SILK_DEBUG << "HI == cap tested again with " << (result.status_code == evmc_status_code::EVMC_SUCCESS ? "succeed" : "failed");
-        } else if (!result.pre_check_error_code || result.pre_check_error_code == PreCheckErrorCode::kIntrinsicGasTooLow) {
-            result.pre_check_error = std::nullopt;
-            result.status_code = evmc_status_code::EVMC_SUCCESS;
-        }
+        result.status_code = evmc_status_code::EVMC_SUCCESS;
 
         SILK_DEBUG << "EstimateGasOracle::estimate_gas returns " << hi;
 

silkworm/rpc/core/estimate_gas_oracle.hpp

@@ -92,7 +92,7 @@ class EstimateGasOracle {
     EstimateGasOracle(const EstimateGasOracle&) = delete;
     EstimateGasOracle& operator=(const EstimateGasOracle&) = delete;
 
-    Task<intx::uint256> estimate_gas(const Call& call, const silkworm::Block& latest_block, std::optional<TxnId> txn_id, std::optional<BlockNum> block_num_for_gas_limit = {});
+    Task<intx::uint256> estimate_gas(const Call& call, const silkworm::Block& latest_block, std::optional<TxnId> txn_id);
 
   protected:
     virtual ExecutionResult try_execution(EVMExecutor& executor, const silkworm::Transaction& transaction);