From ef059f55e21aed1f28cbe7abb18ef9cf2b14e878 Mon Sep 17 00:00:00 2001 From: Sixtysixter <20945591+Sixtysixter@users.noreply.github.com> Date: Mon, 16 Sep 2024 01:08:38 +0200 Subject: [PATCH 1/7] Fix for gas cost in trace_replay* API --- silkworm/rpc/core/evm_trace.cpp | 109 +++++++------------------------- silkworm/rpc/core/evm_trace.hpp | 8 +-- 2 files changed, 25 insertions(+), 92 deletions(-) diff --git a/silkworm/rpc/core/evm_trace.cpp b/silkworm/rpc/core/evm_trace.cpp index 3a48f529f2..8e488e7715 100644 --- a/silkworm/rpc/core/evm_trace.cpp +++ b/silkworm/rpc/core/evm_trace.cpp @@ -599,13 +599,13 @@ void VmTraceTracer::on_execution_start(evmc_revision rev, const evmc_message& ms index_prefix_.push(index_prefix); auto& op = vm_trace.ops[vm_trace.ops.size() - 1]; - if (op.op_code == evmc_opcode::OP_STATICCALL || op.op_code == evmc_opcode::OP_DELEGATECALL || op.op_code == evmc_opcode::OP_CALL) { - auto& op_1 = vm_trace.ops[vm_trace.ops.size() - 2]; - auto cap = op_1.trace_ex->used - msg.gas; - op.depth = msg.depth; - op.gas_cost = op.gas_cost - msg.gas; - op.call_gas_cap = cap; + + if (op.op_code == OP_CREATE || op.op_code == OP_CREATE2) { + op.gas_cost = msg.gas; + } else { + op.gas_cost = msg.gas_cost; } + op.sub = std::make_shared(); traces_stack_.emplace(*op.sub); op.sub->code = "0x" + silkworm::to_hex(code); @@ -623,34 +623,25 @@ void VmTraceTracer::on_execution_start(evmc_revision rev, const evmc_message& ms << ", index_prefix: " << index_prefix; } -void VmTraceTracer::on_instruction_start(uint32_t pc, const intx::uint256* stack_top, const int stack_height, const int64_t gas, - const evmone::ExecutionState& execution_state, const silkworm::IntraBlockState& intra_block_state) noexcept { +void VmTraceTracer::on_instruction_start(uint32_t pc, const intx::uint256* stack_top, const int /*stack_height*/, const int64_t gas, + const evmone::ExecutionState& execution_state, const silkworm::IntraBlockState& /*intra_block_state*/) noexcept { const auto op_code = execution_state.original_code[pc]; auto op_name = get_opcode_name(opcode_names_, op_code); last_opcode_ = op_code; - if (fix_call_gas_info_) { // previous opcode was a CALL - auto& trace_op = fix_call_gas_info_->trace_op_; - if (execution_state.msg->depth == fix_call_gas_info_->depth) { - if (fix_call_gas_info_->gas_cost) { - trace_op.gas_cost = fix_call_gas_info_->gas_cost + fix_call_gas_info_->code_cost; - } - } else { - trace_op.gas_cost = gas + fix_call_gas_info_->stipend + fix_call_gas_info_->code_cost; - } - - fix_call_gas_info_.reset(); - } - + int64_t used = 0; auto& vm_trace = traces_stack_.top().get(); if (!vm_trace.ops.empty()) { auto& op = vm_trace.ops[vm_trace.ops.size() - 1]; - if (op.precompiled_call_gas) { - op.gas_cost = op.gas_cost - op.precompiled_call_gas.value(); + if (op.op_code == OP_RETURN || op.op_code == OP_STOP || op.op_code == OP_REVERT) { + op.gas_cost = 0; + } else if (op.op_code == OP_CREATE || op.op_code == OP_CREATE2) { + op.gas_cost += execution_state.last_opcode_gas_cost; } else if (op.depth == execution_state.msg->depth) { - op.gas_cost = op.gas_cost - gas; + op.gas_cost = execution_state.last_opcode_gas_cost; } op.trace_ex->used = gas; + used = op.trace_ex->used; copy_memory(execution_state.memory, op.trace_ex->memory); copy_stack(op.op_code, stack_top, op.trace_ex->stack); } @@ -658,13 +649,13 @@ void VmTraceTracer::on_instruction_start(uint32_t pc, const intx::uint256* stack auto index_prefix = index_prefix_.top() + std::to_string(vm_trace.ops.size()); TraceOp trace_op; - trace_op.gas_cost = gas; + trace_op.gas_cost = metrics_[op_code].gas_cost; trace_op.idx = index_prefix; trace_op.depth = execution_state.msg->depth; trace_op.op_code = op_code; trace_op.op_name = op_name; trace_op.pc = pc; - trace_op.trace_ex = std::make_optional(); + trace_op.trace_ex = std::make_optional({used}); if (op_code == OP_SELFDESTRUCT) { trace_op.sub = std::make_shared(); @@ -676,8 +667,6 @@ void VmTraceTracer::on_instruction_start(uint32_t pc, const intx::uint256* stack vm_trace.ops.push_back(trace_op); - fill_call_gas_info(vm_trace.ops.back(), execution_state, stack_top, stack_height, intra_block_state); - SILK_DEBUG << "VmTraceTracer::on_instruction_start:" << " pc: " << std::dec << pc << ", opcode: 0x" << std::hex << evmc::hex(op_code) @@ -703,11 +692,6 @@ void VmTraceTracer::on_precompiled_run(const evmc_result& result, int64_t gas, c op.sub->code = "0x"; } } - if (fix_call_gas_info_) { - fix_call_gas_info_->gas_cost += gas + fix_call_gas_info_->code_cost; - fix_call_gas_info_->code_cost = 0; - fix_call_gas_info_->precompiled = true; - } } void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm::IntraBlockState& /*intra_block_state*/) noexcept { @@ -737,7 +721,9 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: case evmc_status_code::EVMC_OUT_OF_GAS: // If we run out of gas, we reset trace_ex to null (no matter what the content is) as Erigon does op.trace_ex = std::nullopt; - op.gas_cost -= result.gas_left; + if (op.op_code != OP_CALLCODE) { + op.gas_cost = result.gas_cost; + } break; // We need to adjust gas used and gas cost from evmone to match evm.go values @@ -749,39 +735,21 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: op.gas_cost = 0; } else { /* EVM WA: EVMONE in case of this error returns always zero on gas-left */ - op.trace_ex->used = op.gas_cost - metrics_[op.op_code].gas_cost; + op.trace_ex->used -= op.gas_cost; op.gas_cost = metrics_[op.op_code].gas_cost; } break; case evmc_status_code::EVMC_UNDEFINED_INSTRUCTION: case evmc_status_code::EVMC_INVALID_INSTRUCTION: - op.trace_ex->used = op.gas_cost; op.gas_cost = 0; break; - case evmc_status_code::EVMC_REVERT: - op.gas_cost = op.gas_cost - result.gas_left; - op.trace_ex->used = result.gas_left; - break; - default: - op.gas_cost = op.gas_cost - result.gas_left; - op.trace_ex->used = result.gas_left; - if (fix_call_gas_info_) { - auto& trace_op = fix_call_gas_info_->trace_op_; - if (result.gas_left == 0 && !fix_call_gas_info_->precompiled) { - trace_op.gas_cost = fix_call_gas_info_->stipend + fix_call_gas_info_->gas_cost; - } else if (!fix_call_gas_info_->precompiled) { - trace_op.gas_cost = result.gas_left + fix_call_gas_info_->gas_cost + fix_call_gas_info_->code_cost; - fix_call_gas_info_->gas_cost = 0; - } else if (fix_call_gas_info_->precompiled) { - trace_op.gas_cost = fix_call_gas_info_->gas_cost; - fix_call_gas_info_->gas_cost = 0; - } else { - fix_call_gas_info_->gas_cost = 0; - } + if (op.op_code == OP_CALL || op.op_code == OP_CALLCODE || op.op_code == OP_STATICCALL || op.op_code == OP_DELEGATECALL || op.op_code == OP_CREATE || op.op_code == OP_CREATE2) { + op.gas_cost += result.gas_cost; } + op.trace_ex->used = result.gas_left; break; } @@ -807,35 +775,6 @@ void VmTraceTracer::on_pre_check_failed(const evmc_result& /*result*/, const evm vm_trace_.code = "0x" + silkworm::to_hex(ByteView{msg.input_data, msg.input_size}); } -void VmTraceTracer::fill_call_gas_info(TraceOp& trace_op, const evmone::ExecutionState& execution_state, const intx::uint256* stack_top, const int stack_height, const silkworm::IntraBlockState& intra_block_state) { - auto op_code = trace_op.op_code; - if (op_code == evmc_opcode::OP_CALL || op_code == evmc_opcode::OP_CALLCODE || op_code == evmc_opcode::OP_STATICCALL || op_code == evmc_opcode::OP_DELEGATECALL || op_code == evmc_opcode::OP_CREATE || op_code == evmc_opcode::OP_CREATE2) { - fix_call_gas_info_.emplace(FixCallGasInfo{execution_state.msg->depth, 0, metrics_[op_code].gas_cost, trace_op}); - - const auto value = stack_top[-2]; // value - if (value != 0) { - fix_call_gas_info_->gas_cost += 9000; - } - if (op_code == OP_CALL) { - if (op_code == OP_CALL && stack_height >= 7 && value != 0) { - fix_call_gas_info_->stipend = 2300; // for CALLs with value, include stipend - } - const auto call_gas = stack_top[0]; // gas - const auto dst = intx::be::trunc(stack_top[-1]); // dst - - if ((value != 0 || execution_state.rev < EVMC_SPURIOUS_DRAGON) && !intra_block_state.exists(dst)) { - fix_call_gas_info_->gas_cost += 25000; // add ACCOUNT_CREATION_COST as in instructions_calls.cpp:105 - } - SILK_DEBUG << "DebugTracer::evaluate_call_fixes:" - << " call_gas: " << call_gas - << " dst: " << dst - << " value: " << value - << " gas_cost: " << fix_call_gas_info_->gas_cost - << " stipend: " << fix_call_gas_info_->stipend; - } - } -} - void TraceTracer::on_execution_start(evmc_revision rev, const evmc_message& msg, evmone::bytes_view code) noexcept { if (opcode_names_ == nullptr) { opcode_names_ = evmc_get_instruction_names_table(rev); diff --git a/silkworm/rpc/core/evm_trace.hpp b/silkworm/rpc/core/evm_trace.hpp index 3639622edc..4860f984a8 100644 --- a/silkworm/rpc/core/evm_trace.hpp +++ b/silkworm/rpc/core/evm_trace.hpp @@ -89,10 +89,10 @@ struct TraceMemory { }; struct TraceEx { + int64_t used{0}; std::optional memory; std::vector stack; std::optional storage; - int64_t used{0}; }; struct VmTrace; @@ -153,11 +153,6 @@ class VmTraceTracer : public silkworm::EvmTracer { void on_precompiled_run(const evmc_result& result, int64_t gas, const silkworm::IntraBlockState& intra_block_state) noexcept override; private: - void fill_call_gas_info(TraceOp& trace_op, - const evmone::ExecutionState& execution_state, - const intx::uint256* stack_top, - int stack_height, - const silkworm::IntraBlockState& intra_block_state); VmTrace& vm_trace_; std::int32_t transaction_index_; @@ -167,7 +162,6 @@ class VmTraceTracer : public silkworm::EvmTracer { const evmc_instruction_metrics* metrics_ = nullptr; std::stack start_gas_; std::stack trace_memory_stack_; - std::optional fix_call_gas_info_; std::optional last_opcode_; }; From eff60b6d17f2e0e57676d29ea8fd5884b392e86d Mon Sep 17 00:00:00 2001 From: GitHub Date: Sun, 15 Sep 2024 23:09:56 +0000 Subject: [PATCH 2/7] make fmt --- silkworm/rpc/core/evm_trace.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/silkworm/rpc/core/evm_trace.hpp b/silkworm/rpc/core/evm_trace.hpp index 4860f984a8..e8c1346b88 100644 --- a/silkworm/rpc/core/evm_trace.hpp +++ b/silkworm/rpc/core/evm_trace.hpp @@ -153,7 +153,6 @@ class VmTraceTracer : public silkworm::EvmTracer { void on_precompiled_run(const evmc_result& result, int64_t gas, const silkworm::IntraBlockState& intra_block_state) noexcept override; private: - VmTrace& vm_trace_; std::int32_t transaction_index_; std::stack index_prefix_; From a1c11ce457f0a32a4584e7b7e331e5bc62027620 Mon Sep 17 00:00:00 2001 From: canepat <16927169+canepat@users.noreply.github.com> Date: Mon, 16 Sep 2024 10:32:36 +0200 Subject: [PATCH 3/7] Update evm_trace.cpp --- silkworm/rpc/core/evm_trace.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silkworm/rpc/core/evm_trace.cpp b/silkworm/rpc/core/evm_trace.cpp index 8e488e7715..c9100a4ec1 100644 --- a/silkworm/rpc/core/evm_trace.cpp +++ b/silkworm/rpc/core/evm_trace.cpp @@ -655,7 +655,7 @@ void VmTraceTracer::on_instruction_start(uint32_t pc, const intx::uint256* stack trace_op.op_code = op_code; trace_op.op_name = op_name; trace_op.pc = pc; - trace_op.trace_ex = std::make_optional({used}); + trace_op.trace_ex = TraceEx{used}; if (op_code == OP_SELFDESTRUCT) { trace_op.sub = std::make_shared(); From 36af6f6362534f9c2e5b628ab89de6d9cc039584 Mon Sep 17 00:00:00 2001 From: canepat <16927169+canepat@users.noreply.github.com> Date: Mon, 16 Sep 2024 10:32:56 +0200 Subject: [PATCH 4/7] Update rpc-integration-tests.yml --- .github/workflows/rpc-integration-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rpc-integration-tests.yml b/.github/workflows/rpc-integration-tests.yml index 258cf2ac44..8c859fc9b7 100644 --- 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.50.0 https://github.com/erigontech/rpc-tests ${{runner.workspace}}/rpc-tests + git -c advice.detachedHead=false clone --depth 1 --branch v0.51.0 https://github.com/erigontech/rpc-tests ${{runner.workspace}}/rpc-tests cd ${{runner.workspace}}/rpc-tests pip3 install -r requirements.txt From e2f49f4b2b84fbe12e2bc179d54cd09fb52fe126 Mon Sep 17 00:00:00 2001 From: Sixtysixter <20945591+Sixtysixter@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:54:26 +0200 Subject: [PATCH 5/7] Fix for used field --- silkworm/rpc/core/evm_trace.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/silkworm/rpc/core/evm_trace.cpp b/silkworm/rpc/core/evm_trace.cpp index c9100a4ec1..965292db10 100644 --- a/silkworm/rpc/core/evm_trace.cpp +++ b/silkworm/rpc/core/evm_trace.cpp @@ -731,11 +731,16 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: case evmc_status_code::EVMC_STACK_OVERFLOW: case evmc_status_code::EVMC_BAD_JUMP_DESTINATION: if (op.op_code == evmc_opcode::OP_EXP) { // In Erigon the static part is 0 - op.trace_ex->used = op.gas_cost; +// op.trace_ex->used = op.gas_cost; + op.trace_ex->used = start_gas; // modificata per trace_call 6 op.gas_cost = 0; } else { /* EVM WA: EVMONE in case of this error returns always zero on gas-left */ - op.trace_ex->used -= op.gas_cost; + if (op.trace_ex->used > 0) { // modificata per trace_call 11 + op.trace_ex->used -= op.gas_cost; + } else { + op.trace_ex->used = start_gas - op.gas_cost; + } op.gas_cost = metrics_[op.op_code].gas_cost; } break; @@ -743,6 +748,9 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: case evmc_status_code::EVMC_UNDEFINED_INSTRUCTION: case evmc_status_code::EVMC_INVALID_INSTRUCTION: op.gas_cost = 0; + if (op.trace_ex->used == 0) { + op.trace_ex->used = start_gas; // aggiunta per trace_call 5 + } break; default: From 46dde29287e5ea353d6cf63e7a78982f03af0e9d Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 18 Sep 2024 15:54:55 +0000 Subject: [PATCH 6/7] make fmt --- silkworm/rpc/core/evm_trace.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/silkworm/rpc/core/evm_trace.cpp b/silkworm/rpc/core/evm_trace.cpp index 965292db10..e581224f8d 100644 --- a/silkworm/rpc/core/evm_trace.cpp +++ b/silkworm/rpc/core/evm_trace.cpp @@ -731,12 +731,12 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: case evmc_status_code::EVMC_STACK_OVERFLOW: case evmc_status_code::EVMC_BAD_JUMP_DESTINATION: if (op.op_code == evmc_opcode::OP_EXP) { // In Erigon the static part is 0 -// op.trace_ex->used = op.gas_cost; - op.trace_ex->used = start_gas; // modificata per trace_call 6 + // op.trace_ex->used = op.gas_cost; + op.trace_ex->used = start_gas; // modificata per trace_call 6 op.gas_cost = 0; } else { /* EVM WA: EVMONE in case of this error returns always zero on gas-left */ - if (op.trace_ex->used > 0) { // modificata per trace_call 11 + if (op.trace_ex->used > 0) { // modificata per trace_call 11 op.trace_ex->used -= op.gas_cost; } else { op.trace_ex->used = start_gas - op.gas_cost; @@ -749,7 +749,7 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: case evmc_status_code::EVMC_INVALID_INSTRUCTION: op.gas_cost = 0; if (op.trace_ex->used == 0) { - op.trace_ex->used = start_gas; // aggiunta per trace_call 5 + op.trace_ex->used = start_gas; // aggiunta per trace_call 5 } break; From 1508d09733c1ead599b2fe933049ef470fc73c5e Mon Sep 17 00:00:00 2001 From: Sixtysixter <20945591+Sixtysixter@users.noreply.github.com> Date: Wed, 18 Sep 2024 19:49:41 +0200 Subject: [PATCH 7/7] cleanup --- silkworm/rpc/core/evm_trace.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/silkworm/rpc/core/evm_trace.cpp b/silkworm/rpc/core/evm_trace.cpp index e581224f8d..1e5c9f4e13 100644 --- a/silkworm/rpc/core/evm_trace.cpp +++ b/silkworm/rpc/core/evm_trace.cpp @@ -731,12 +731,11 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: case evmc_status_code::EVMC_STACK_OVERFLOW: case evmc_status_code::EVMC_BAD_JUMP_DESTINATION: if (op.op_code == evmc_opcode::OP_EXP) { // In Erigon the static part is 0 - // op.trace_ex->used = op.gas_cost; - op.trace_ex->used = start_gas; // modificata per trace_call 6 + op.trace_ex->used = start_gas; op.gas_cost = 0; } else { /* EVM WA: EVMONE in case of this error returns always zero on gas-left */ - if (op.trace_ex->used > 0) { // modificata per trace_call 11 + if (op.trace_ex->used > 0) { op.trace_ex->used -= op.gas_cost; } else { op.trace_ex->used = start_gas - op.gas_cost; @@ -749,7 +748,7 @@ void VmTraceTracer::on_execution_end(const evmc_result& result, const silkworm:: case evmc_status_code::EVMC_INVALID_INSTRUCTION: op.gas_cost = 0; if (op.trace_ex->used == 0) { - op.trace_ex->used = start_gas; // aggiunta per trace_call 5 + op.trace_ex->used = start_gas; } break;