rpcdaemon: new gas bailout handling (#2322)

erigontech · Sep 9, 2024 · 05a6ab8 · 05a6ab8
1 parent 48e7316
commit 05a6ab8
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 76 deletions.
diff --git a/.github/workflows/rpc-integration-tests.yml b/.github/workflows/rpc-integration-tests.yml
@@ -27,7 +27,7 @@ jobs:
       - name: Checkout RPC Tests Repository & Install Requirements
         run: |
           rm -rf ${{runner.workspace}}/rpc-tests
-          git -c advice.detachedHead=false clone --depth 1 --branch v0.46.0 https://github.com/erigontech/rpc-tests ${{runner.workspace}}/rpc-tests
+          git -c advice.detachedHead=false clone --depth 1 --branch v0.47.0 https://github.com/erigontech/rpc-tests ${{runner.workspace}}/rpc-tests
           cd ${{runner.workspace}}/rpc-tests
           pip3 install -r requirements.txt
 

diff --git a/silkworm/core/state/intra_block_state.cpp b/silkworm/core/state/intra_block_state.cpp
@@ -201,6 +201,7 @@ void IntraBlockState::set_nonce(const evmc::address& address, uint64_t nonce) no
     auto& obj{get_or_create_object(address)};
     journal_.emplace_back(std::make_unique<state::UpdateDelta>(address, obj));
     obj.current->nonce = nonce;
+    touch(address);
 }
 
 ByteView IntraBlockState::get_code(const evmc::address& address) const noexcept {
@@ -241,6 +242,7 @@ void IntraBlockState::set_code(const evmc::address& address, ByteView code) noex
     // Don't overwrite already existing code so that views of it
     // that were previously returned by get_code() are still valid.
     new_code_.try_emplace(obj.current->code_hash, code.begin(), code.end());
+    touch(address);
 }
 
 evmc_access_status IntraBlockState::access_account(const evmc::address& address) noexcept {

diff --git a/silkworm/rpc/commands/trace_api.cpp b/silkworm/rpc/commands/trace_api.cpp
@@ -422,7 +422,7 @@ Task<void> TraceRpcApi::handle_trace_get(const nlohmann::json& request, nlohmann
             reply = make_json_content(request);
         } else {
             trace::TraceCallExecutor executor{*block_cache_, *chain_storage, workers_, *tx};
-            const auto result = co_await executor.trace_transaction(*(tx_with_block->block_with_hash), tx_with_block->transaction);
+            const auto result = co_await executor.trace_transaction(*(tx_with_block->block_with_hash), tx_with_block->transaction, /* gas_bailout */ false);
 
             uint16_t index = indices[0];
             if (rpc::compatibility::is_erigon_json_api_compatibility_required()) {
@@ -467,7 +467,7 @@ Task<void> TraceRpcApi::handle_trace_transaction(const nlohmann::json& request,
             reply = make_json_content(request);
         } else {
             trace::TraceCallExecutor executor{*block_cache_, *chain_storage, workers_, *tx};
-            auto result = co_await executor.trace_transaction(*(tx_with_block->block_with_hash), tx_with_block->transaction);
+            auto result = co_await executor.trace_transaction(*(tx_with_block->block_with_hash), tx_with_block->transaction, /* gas_bailout */ false);
             reply = make_json_content(request, result);
         }
     } catch (const std::exception& e) {

diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp
@@ -266,38 +266,37 @@ ExecutionResult EVMExecutor::call(
         return {std::nullopt, txn.gas_limit, data, pre_check_result->pre_check_error, pre_check_result->pre_check_error_code};
     }
 
-    // EIP-1559 normal gas cost
-    intx::uint256 want;
-    if (txn.max_fee_per_gas > 0 || txn.max_priority_fee_per_gas > 0) {
-        // This method should be called after check (max_fee and base_fee) present in pre_check() method
-        const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas)
-                                                                                        : txn.max_priority_fee_per_gas};
-        want = txn.gas_limit * effective_gas_price;
-    } else {
-        want = 0;
-    }
+    if (!gas_bailout) {
+        // EIP-1559 normal gas cost
+        intx::uint256 want;
+        if (txn.max_fee_per_gas > 0 || txn.max_priority_fee_per_gas > 0) {
+            // This method should be called after check (max_fee and base_fee) present in pre_check() method
+            const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas)
+                                                                                            : txn.max_priority_fee_per_gas};
+            want = txn.gas_limit * effective_gas_price;
+        } else {
+            want = 0;
+        }
 
-    // EIP-4844 blob gas cost (calc_data_fee)
-    if (evm.block().header.blob_gas_used && rev >= EVMC_CANCUN) {
-        // compute blob fee for eip-4844 data blobs if any
-        const intx::uint256 blob_gas_price{evm.block().header.blob_gas_price().value_or(0)};
-        want += txn.total_blob_gas() * blob_gas_price;
-    }
+        // EIP-4844 blob gas cost (calc_data_fee)
+        if (evm.block().header.blob_gas_used && rev >= EVMC_CANCUN) {
+            // compute blob fee for eip-4844 data blobs if any
+            const intx::uint256 blob_gas_price{evm.block().header.blob_gas_price().value_or(0)};
+            want += txn.total_blob_gas() * blob_gas_price;
+        }
 
-    intx::uint512 max_want = want;
-    if (txn.type != silkworm::TransactionType::kLegacy && txn.type != silkworm::TransactionType::kAccessList) {
-        max_want = txn.maximum_gas_cost();
-    }
+        intx::uint512 max_want = want;
+        if (txn.type != silkworm::TransactionType::kLegacy && txn.type != silkworm::TransactionType::kAccessList) {
+            max_want = txn.maximum_gas_cost();
+        }
 
-    const auto have = ibs_state_.get_balance(*txn.sender());
-    if (have < max_want + txn.value) {
-        if (!gas_bailout) {
+        const auto have = ibs_state_.get_balance(*txn.sender());
+        if (have < max_want + txn.value) {
             Bytes data{};
             std::string from = address_to_hex(*txn.sender());
             std::string msg = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(have) + " want " + intx::to_string(max_want + txn.value);
             return {std::nullopt, txn.gas_limit, data, msg, PreCheckErrorCode::kInsufficientFunds};
         }
-    } else {
         ibs_state_.subtract_from_balance(*txn.sender(), want);
     }
 
@@ -329,9 +328,13 @@ ExecutionResult EVMExecutor::call(
     }
 
     uint64_t gas_left = result.gas_left;
-    const uint64_t gas_used{txn.gas_limit - refund_gas(evm, txn, result.gas_left, result.gas_refund)};
-    if (refund) {
-        gas_left = txn.gas_limit - gas_used;
+    uint64_t gas_used{txn.gas_limit - result.gas_left};
+
+    if (refund && !gas_bailout) {
+        gas_used = txn.gas_limit - refund_gas(evm, txn, result.gas_left, result.gas_refund);
+        if (refund) {
+            gas_left = txn.gas_limit - gas_used;
+        }
     }
 
     // Reward the fee recipient

diff --git a/silkworm/rpc/core/evm_executor_test.cpp b/silkworm/rpc/core/evm_executor_test.cpp
@@ -128,8 +128,8 @@ TEST_CASE_METHOD(EVMExecutorTest, "EVMExecutor") {
     }
 
     SECTION("doesn't fail if transaction cost greater user amount && gasBailout == true") {
-        EXPECT_CALL(transaction, get(_, _)).Times(8).WillRepeatedly(InvokeWithoutArgs([]() -> Task<KeyValue> { co_return KeyValue{}; }));
-        EXPECT_CALL(transaction, get_one(_, _)).Times(8).WillRepeatedly(InvokeWithoutArgs([]() -> Task<Bytes> { co_return Bytes{}; }));
+        EXPECT_CALL(transaction, get(_, _)).Times(7).WillRepeatedly(InvokeWithoutArgs([]() -> Task<KeyValue> { co_return KeyValue{}; }));
+        EXPECT_CALL(transaction, get_one(_, _)).Times(7).WillRepeatedly(InvokeWithoutArgs([]() -> Task<Bytes> { co_return Bytes{}; }));
 
         silkworm::Block block{};
         block.header.base_fee_per_gas = 0x1;
@@ -155,8 +155,8 @@ TEST_CASE_METHOD(EVMExecutorTest, "EVMExecutor") {
     };
 
     SECTION("call returns SUCCESS") {
-        EXPECT_CALL(transaction, get(_, _)).Times(6).WillRepeatedly(InvokeWithoutArgs([]() -> Task<KeyValue> { co_return KeyValue{}; }));
-        EXPECT_CALL(transaction, get_one(_, _)).Times(6).WillRepeatedly(InvokeWithoutArgs([]() -> Task<Bytes> { co_return Bytes{}; }));
+        EXPECT_CALL(transaction, get(_, _)).Times(7).WillRepeatedly(InvokeWithoutArgs([]() -> Task<KeyValue> { co_return KeyValue{}; }));
+        EXPECT_CALL(transaction, get_one(_, _)).Times(7).WillRepeatedly(InvokeWithoutArgs([]() -> Task<Bytes> { co_return Bytes{}; }));
 
         silkworm::Block block{};
         block.header.number = block_number;

diff --git a/silkworm/rpc/core/evm_trace.cpp b/silkworm/rpc/core/evm_trace.cpp
@@ -1493,7 +1493,7 @@ Task<std::vector<TraceCallResult>> TraceCallExecutor::trace_block_transactions(c
 
             tracers.push_back(ibs_tracer);
 
-            auto execution_result = executor.call(block, transaction, tracers, /*refund=*/true, /*gas_bailout=*/true);
+            auto execution_result = executor.call(block, transaction, tracers, /*refund=*/true, /*gas_bailout=*/false);
             if (execution_result.pre_check_error) {
                 result.pre_check_error = execution_result.pre_check_error.value();
             } else {
@@ -1509,7 +1509,7 @@ Task<std::vector<TraceCallResult>> TraceCallExecutor::trace_block_transactions(c
 
 Task<TraceCallResult> TraceCallExecutor::trace_call(const silkworm::Block& block, const Call& call, const TraceConfig& config) {
     rpc::Transaction transaction{call.to_transaction()};
-    auto result = co_await execute(block.header.number, block, transaction, -1, config);
+    auto result = co_await execute(block.header.number, block, transaction, -1, config, true);
     co_return result;
 }
 
@@ -1610,14 +1610,14 @@ Task<TraceDeployResult> TraceCallExecutor::trace_deploy_transaction(const silkwo
 }
 
 Task<TraceCallResult> TraceCallExecutor::trace_transaction(const silkworm::Block& block, const rpc::Transaction& transaction, const TraceConfig& config) {
-    return execute(block.header.number - 1, block, transaction, gsl::narrow<int32_t>(transaction.transaction_index), config);
+    return execute(block.header.number - 1, block, transaction, gsl::narrow<int32_t>(transaction.transaction_index), config, false);
 }
 
-Task<std::vector<Trace>> TraceCallExecutor::trace_transaction(const BlockWithHash& block_with_hash, const rpc::Transaction& transaction) {
+Task<std::vector<Trace>> TraceCallExecutor::trace_transaction(const BlockWithHash& block_with_hash, const rpc::Transaction& transaction, bool gas_bailout) {
     std::vector<Trace> traces;
 
     const auto result = co_await execute(block_with_hash.block.header.number - 1, block_with_hash.block, transaction,
-                                         gsl::narrow<int32_t>(transaction.transaction_index), {false, true, false});
+                                         gsl::narrow<int32_t>(transaction.transaction_index), {false, true, false}, gas_bailout);
     const auto& trace_result = result.traces.trace;
 
     const auto tnx_hash = transaction.hash();
@@ -1809,7 +1809,8 @@ Task<TraceCallResult> TraceCallExecutor::execute(
     const silkworm::Block& block,
     const rpc::Transaction& transaction,
     std::int32_t index,
-    const TraceConfig& config) {
+    const TraceConfig& config,
+    bool gas_bailout) {
     SILK_DEBUG << "execute: "
                << " block_number: " << std::dec << block_number
                << " transaction: {" << transaction << "}"
@@ -1831,7 +1832,7 @@ Task<TraceCallResult> TraceCallExecutor::execute(
         EVMExecutor executor{chain_config, workers_, curr_state};
         for (std::size_t idx{0}; idx < transaction.transaction_index; idx++) {
             silkworm::Transaction txn{block.transactions[idx]};
-            const auto execution_result = executor.call(block, txn, tracers, /*refund=*/true, /*gas_bailout=*/true);
+            const auto execution_result = executor.call(block, txn, tracers, /*refund=*/true, gas_bailout);
             if (execution_result.pre_check_error) {
                 SILK_ERROR << "execution failed for tx " << idx << " due to pre-check error: " << *execution_result.pre_check_error;
             }
@@ -1857,7 +1858,7 @@ Task<TraceCallResult> TraceCallExecutor::execute(
         if (index != -1) {
             traces.transaction_hash = transaction.hash();  // to have same behaviour as erigon, should be done PR on erigon
         }
-        const auto execution_result = executor.call(block, transaction, tracers, /*refund=*/true, /*gas_bailout=*/true);
+        const auto execution_result = executor.call(block, transaction, tracers, /*refund=*/true, gas_bailout);
 
         if (execution_result.pre_check_error) {
             result.pre_check_error = execution_result.pre_check_error.value();

diff --git a/silkworm/rpc/core/evm_trace.hpp b/silkworm/rpc/core/evm_trace.hpp
@@ -507,7 +507,7 @@ class TraceCallExecutor {
     Task<TraceManyCallResult> trace_calls(const silkworm::Block& block, const std::vector<TraceCall>& calls);
     Task<TraceDeployResult> trace_deploy_transaction(const silkworm::Block& block, const evmc::address& contract_address);
     Task<TraceCallResult> trace_transaction(const silkworm::Block& block, const rpc::Transaction& transaction, const TraceConfig& config);
-    Task<std::vector<Trace>> trace_transaction(const silkworm::BlockWithHash& block, const rpc::Transaction& transaction);
+    Task<std::vector<Trace>> trace_transaction(const silkworm::BlockWithHash& block, const rpc::Transaction& transaction, bool gas_bailout);
     Task<TraceEntriesResult> trace_transaction_entries(const TransactionWithBlock& transaction_with_block);
     Task<std::string> trace_transaction_error(const TransactionWithBlock& transaction_with_block);
     Task<TraceOperationsResult> trace_operations(const TransactionWithBlock& transaction_with_block);
@@ -522,7 +522,8 @@ class TraceCallExecutor {
         const silkworm::Block& block,
         const rpc::Transaction& transaction,
         std::int32_t index,
-        const TraceConfig& config);
+        const TraceConfig& config,
+        bool gas_bailout);
 
     silkworm::BlockCache& block_cache_;
     const ChainStorage& chain_storage_;

diff --git a/silkworm/rpc/core/evm_trace_test.cpp b/silkworm/rpc/core/evm_trace_test.cpp
@@ -382,12 +382,7 @@ TEST_CASE_METHOD(TraceCallExecutorTest, "TraceCallExecutor::trace_call 1") {
                 }
                 },
                 "0xe0a2bd4258d2768837baa26a28fe71dc079f84c7": {
-                "balance": {
-                    "*": {
-                    "from": "0x141e903194951083c424fd",
-                    "to": "0x141e903194951083bc1d57"
-                    }
-                },
+                "balance": "=",
                 "code": "=",
                 "nonce": {
                     "*": {
@@ -563,12 +558,7 @@ TEST_CASE_METHOD(TraceCallExecutorTest, "TraceCallExecutor::trace_call 1") {
                 }
                 },
                 "0xe0a2bd4258d2768837baa26a28fe71dc079f84c7": {
-                "balance": {
-                    "*": {
-                    "from": "0x141e903194951083c424fd",
-                    "to": "0x141e903194951083bc1d57"
-                    }
-                },
+                "balance": "=",
                 "code": "=",
                 "nonce": {
                     "*": {
@@ -681,12 +671,7 @@ TEST_CASE_METHOD(TraceCallExecutorTest, "TraceCallExecutor::trace_call 1") {
                 }
                 },
                 "0xe0a2bd4258d2768837baa26a28fe71dc079f84c7": {
-                "balance": {
-                    "*": {
-                    "from": "0x141e903194951083c424fd",
-                    "to": "0x141e903194951083bc1d57"
-                    }
-                },
+                "balance": "=",
                 "code": "=",
                 "nonce": {
                     "*": {
@@ -1176,7 +1161,7 @@ TEST_CASE_METHOD(TraceCallExecutorTest, "TraceCallExecutor::trace_call 2") {
                 "balance": {
                     "*": {
                     "from": "0x1a098914888dc0516d2",
-                    "to": "0x1a09891356e7914ae52"
+                    "to": "0x1a098914888d90a2652"
                     }
                 },
                 "code": "=",
@@ -1408,12 +1393,7 @@ TEST_CASE_METHOD(TraceCallExecutorTest, "TraceCallExecutor::trace_call with erro
             "storage": {}
             },
             "0x578f0a154b23be77fc2033197fbc775637648ad4": {
-            "balance": {
-                "*": {
-                "from": "0x207fbc719f215d705",
-                "to": "0x207fbc719f1e8b991"
-                }
-            },
+            "balance": "=",
             "code": "=",
             "nonce": {
                 "*": {
@@ -1700,12 +1680,7 @@ TEST_CASE_METHOD(TraceCallExecutorTest, "TraceCallExecutor::trace_calls") {
                     }
                 },
                 "0xe0a2bd4258d2768837baa26a28fe71dc079f84c7": {
-                    "balance": {
-                    "*": {
-                        "from": "0x141e903194951083c424fd",
-                        "to": "0x141e903194951083bc1d57"
-                    }
-                    },
+                    "balance": "=",
                     "code": "=",
                     "nonce": {
                     "*": {
@@ -3664,7 +3639,7 @@ TEST_CASE_METHOD(TraceCallExecutorTest, "TraceCallExecutor::trace_transaction")
     block_with_hash.block.transactions.push_back(txn);
 
     TraceCallExecutor executor{block_cache, chain_storage, workers, transaction};
-    const auto result = spawn_and_wait(executor.trace_transaction(block_with_hash, txn));
+    const auto result = spawn_and_wait(executor.trace_transaction(block_with_hash, txn, true));
 
     CHECK(nlohmann::json(result) == R"([
         {