rpcdaemon: fix result as error in some ots API (#2053)

Author: lupin012

.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.23.0 https://github.com/erigontech/rpc-tests ${{runner.workspace}}/rpc-tests
+          git -c advice.detachedHead=false clone --depth 1 --branch v0.24.0 https://github.com/erigontech/rpc-tests ${{runner.workspace}}/rpc-tests
           cd ${{runner.workspace}}/rpc-tests
           pip3 install -r requirements.txt
 

silkworm/rpc/commands/ots_api.cpp

@@ -547,8 +547,9 @@ Task<void> OtsRpcApi::handle_ots_trace_transaction(const nlohmann::json& request
         const auto transaction_with_block = co_await core::read_transaction_by_hash(*block_cache_, *chain_storage, transaction_hash);
 
         if (!transaction_with_block.has_value()) {
-            reply = make_json_content(request, nlohmann::detail::value_t::null);
-            co_await tx->close();
+            const auto error_msg = "transaction 0x" + silkworm::to_hex(transaction_hash) + " not found";
+            reply = make_json_error(request, -32000, error_msg);
+            co_await tx->close();  // RAII not (yet) available with coroutines
             co_return;
         }
 
@@ -594,7 +595,8 @@ Task<void> OtsRpcApi::handle_ots_get_transaction_error(const nlohmann::json& req
         const auto transaction_with_block = co_await core::read_transaction_by_hash(*block_cache_, *chain_storage, transaction_hash);
 
         if (!transaction_with_block.has_value()) {
-            reply = make_json_content(request, nlohmann::detail::value_t::null);
+            const auto error_msg = "transaction 0x" + silkworm::to_hex(transaction_hash) + " not found";
+            reply = make_json_error(request, -32000, error_msg);
             co_await tx->close();
             co_return;
         }
@@ -641,7 +643,8 @@ Task<void> OtsRpcApi::handle_ots_get_internal_operations(const nlohmann::json& r
         const auto transaction_with_block = co_await core::read_transaction_by_hash(*block_cache_, *chain_storage, transaction_hash);
 
         if (!transaction_with_block.has_value()) {
-            reply = make_json_content(request, nlohmann::detail::value_t::null);
+            const auto error_msg = "transaction 0x" + silkworm::to_hex(transaction_hash) + " not found";
+            reply = make_json_error(request, -32000, error_msg);
             co_await tx->close();
             co_return;
         }

silkworm/rpc/core/evm_executor.cpp

@@ -178,6 +178,13 @@ uint64_t EVMExecutor::refund_gas(const EVM& evm, const silkworm::Transaction& tx
     return gas_left;
 }
 
+void EVMExecutor::call_first_n(const silkworm::Block& block, const uint64_t n, const Tracers& tracers, bool refund, bool gas_bailout) {
+    for (size_t idx = 0; idx < block.transactions.size() && idx < n; idx++) {
+        const auto& txn = block.transactions.at(idx);
+        call(block, txn, tracers, refund, gas_bailout);
+    }
+}
+
 void EVMExecutor::reset() {
     ibs_state_.clear_journal_and_substate();
 }

silkworm/rpc/core/evm_executor.hpp

@@ -126,6 +126,8 @@ class EVMExecutor {
 
     void reset();
 
+    void call_first_n(const silkworm::Block& block, const uint64_t n, const Tracers& tracers = {}, bool refund = true, bool gas_bailout = false);
+
     const IntraBlockState& get_ibs_state() { return ibs_state_; }
 
   private:

silkworm/rpc/core/evm_trace.cpp

@@ -1460,21 +1460,6 @@ Task<std::vector<Trace>> TraceCallExecutor::trace_transaction(const BlockWithHas
     co_return traces;
 }
 
-bool TraceCallExecutor::run_previous_transactions(EVMExecutor& executor, const silkworm::Block& block, uint64_t tx_id) {
-    if (tx_id == 0) {
-        return true;
-    }
-    for (size_t idx = 0; idx < block.transactions.size(); idx++) {
-        const auto& txn = block.transactions.at(idx);
-
-        if (tx_id == idx) {
-            return true;
-        }
-        executor.call(block, txn, {}, /*refund=*/true, /*gas_bailout=*/false);
-    }
-    return false;
-}
-
 Task<TraceEntriesResult> TraceCallExecutor::trace_transaction_entries(const TransactionWithBlock& transaction_with_block) {
     const auto& block = transaction_with_block.block_with_hash->block;
     const auto block_number = block.header.number;
@@ -1489,10 +1474,7 @@ Task<TraceEntriesResult> TraceCallExecutor::trace_transaction_entries(const Tran
         auto curr_state = tx_.create_state(current_executor, database_reader_, chain_storage_, block_number - 1);
         EVMExecutor executor{*chain_config_ptr, workers_, curr_state};
 
-        if (!run_previous_transactions(executor, block, transaction_with_block.transaction.transaction_index)) {
-            SILK_ERROR << "trace_operations: transaction idx: " << transaction_with_block.transaction.transaction_index << " not found";
-            return {};
-        }
+        executor.call_first_n(block, transaction_with_block.transaction.transaction_index);
 
         const auto entry_tracer = std::make_shared<trace::EntryTracer>(initial_ibs);
         Tracers tracers{entry_tracer};
@@ -1520,10 +1502,7 @@ Task<std::string> TraceCallExecutor::trace_transaction_error(const TransactionWi
         auto curr_state = tx_.create_state(current_executor, database_reader_, chain_storage_, block_number - 1);
         EVMExecutor executor{*chain_config_ptr, workers_, curr_state};
 
-        if (!run_previous_transactions(executor, block, transaction_with_block.transaction.transaction_index)) {
-            SILK_ERROR << "trace_transaction_error: transaction idx: " << transaction_with_block.transaction.transaction_index << " not found";
-            return {};
-        }
+        executor.call_first_n(block, transaction_with_block.transaction.transaction_index);
 
         const auto& txn = block.transactions.at(transaction_with_block.transaction.transaction_index);
         auto execution_result = executor.call(block, txn, {}, /*refund=*/true, /*gas_bailout=*/false);
@@ -1553,10 +1532,7 @@ Task<TraceOperationsResult> TraceCallExecutor::trace_operations(const Transactio
         auto curr_state = tx_.create_state(current_executor, database_reader_, chain_storage_, block_number - 1);
         EVMExecutor executor{*chain_config_ptr, workers_, curr_state};
 
-        if (!run_previous_transactions(executor, block, transaction_with_block.transaction.transaction_index)) {
-            SILK_ERROR << "trace_operations: transaction idx: " << transaction_with_block.transaction.transaction_index << " not found";
-            return {};
-        }
+        executor.call_first_n(block, transaction_with_block.transaction.transaction_index);
 
         auto entry_tracer = std::make_shared<trace::OperationTracer>(initial_ibs);
         Tracers tracers{entry_tracer};
@@ -1785,6 +1761,8 @@ void EntryTracer::on_execution_start(evmc_revision rev, const evmc_message& msg,
     const auto str_value = "0x" + intx::hex(intx::be::load<intx::uint256>(msg.value));
     auto str_input = "0x" + silkworm::to_hex(input);
 
+    current_depth_ = msg.depth;
+
     // Ignore precompiles in the returned trace (maybe we shouldn't?)
     if (precompile::is_precompile(msg.code_address, rev)) {
         // writes special value for index
@@ -1817,7 +1795,6 @@ void EntryTracer::on_execution_start(evmc_revision rev, const evmc_message& msg,
         }
     }
     traces_stack_idx_.emplace(result_.size() - 1);
-    current_depth_ = msg.depth;
 
     SILK_DEBUG << "EntryTracer::on_execution_start: gas: " << std::dec << msg.gas
                << ", msg.depth: " << msg.depth

silkworm/rpc/core/evm_trace.hpp

@@ -494,8 +494,6 @@ class TraceCallExecutor {
         std::int32_t index,
         const TraceConfig& config);
 
-    bool run_previous_transactions(EVMExecutor& executor, const silkworm::Block& block, uint64_t tx_id);
-
     silkworm::BlockCache& block_cache_;
     const core::rawdb::DatabaseReader& database_reader_;
     const ChainStorage& chain_storage_;