feat: Implement opcode logger functionality #8241

konstantinabl · 2024-05-07T15:47:15Z

Description:

The main goal of this PR is to add new tracer (OpcodeTracer) in the web3 module, in order to support opcodeLogger for debugging purposes.

Main files added:

OpcodeTracer - tracer class, which has methods for tracing contract calls and uses the message frame to get the executed opcodes and save them
ContractionActionService - service that queries the DB for relevant contract actions, used in the OpcodeTracer
EntityService - service that finds an entity in the DB, used to find the evmAddress of an account

Related issue(s):

Fixes #8173

Notes for reviewer:

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

…nto 8173-implement-opcode-logger-tracing-logic Need changes from other branch to debug

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

sonarcloud · 2024-05-14T15:45:39Z

Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarCloud

codecov · 2024-05-14T15:45:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.45%. Comparing base (9f37404) to head (91499c4).
Report is 27 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8241      +/-   ##
============================================
+ Coverage     92.28%   92.45%   +0.17%     
+ Complexity     7280     1270    -6010     
============================================
  Files           898      372     -526     
  Lines         29338    11455   -17883     
  Branches       3575     1549    -2026     
============================================
- Hits          27075    10591   -16484     
+ Misses         1446      699     -747     
+ Partials        817      165     -652

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

…nto 8173-implement-opcode-logger-tracing-logic # Conflicts: # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/contracts/execution/traceability/OpcodeTracer.java # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java # hedera-mirror-web3/src/main/java/com/hedera/node/app/service/evm/contracts/execution/HederaEvmTxProcessor.java # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/controller/OpcodesControllerTest.java

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/config/EvmConfiguration.java

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java

hedera-mirror-web3/src/main/resources/application.yml

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

…nto 8173-implement-opcode-logger-tracing-logic # Conflicts: # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/common/ContractCallContext.java # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/OpcodesController.java # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/controller/OpcodesControllerTest.java

…lement-opcode-logger-tracing-logic # Conflicts: # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/common/ContractCallContext.java # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/OpcodesController.java # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/controller/OpcodesControllerTest.java

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

victor-yanev

Nice job 🔥! This has been a tough task!

I've left a few suggestions below:

.../src/main/java/com/hedera/mirror/web3/evm/contracts/execution/traceability/OpcodeTracer.java

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractActionService.java

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractActionServiceImpl.java

.../src/main/java/com/hedera/node/app/service/evm/contracts/execution/HederaEvmTxProcessor.java

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/common/ContractCallContext.java

...ra-mirror-web3/src/main/java/com/hedera/mirror/web3/repository/ContractActionRepository.java

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

…cing-logic' into 8173-implement-opcode-logger-tracing-logic

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

…lement-opcode-logger-tracing-logic # Conflicts: # hedera-mirror-common/src/main/java/com/hedera/mirror/common/domain/transaction/Opcode.java # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/OpcodesController.java # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/controller/OpcodesControllerTest.java

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

…lement-opcode-logger-tracing-logic # Conflicts: # hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/OpcodesController.java # hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/controller/OpcodesControllerTest.java

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

…Exception when transaction cannot be found Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

IvanKavaldzhiev · 2024-05-30T08:20:00Z

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractActionServiceImpl.java

+
+    @Override
+    public List<ContractAction> findFromTransaction(@NonNull @Valid TransactionIdOrHashParameter transactionIdOrHash) {
+        Assert.isTrue(transactionIdOrHash.isValid(), "Invalid transactionIdOrHash");


Isn't the @Valid annotation doing this check automatically?

IvanKavaldzhiev · 2024-05-30T09:52:35Z

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java

@@ -88,20 +97,9 @@ public String processCall(final CallServiceParameters params) {
                if (params.isEstimate()) {
                    // eth_estimateGas initialization - historical timestamp is Optional.empty()
                    ctx.initializeStackFrames(store.getStackedStateFrames());
-                    result = estimateGas(params);
+                    result = estimateGas(params, ctx);


I think it would be cleaner if we have a dedicated ContractCallDebugService, which would be dedicated only for all existing and future debug related RPC calls. The workflow there is different - we replay an already executed transaction, while in ContractCallService we simulate a hypothetical transaction. So both services would have different responsibilities and needs.

If some components seem to be common for both services, they can be extracted in a parent Service class.

I agree
Will do.

IvanKavaldzhiev · 2024-05-30T09:56:16Z

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java

                } else {
-                    BlockType block = params.getBlock();
-                    // if we have historical call then set corresponding file record
-                    if (block != BlockType.LATEST) {


In the case of debug_traceTransaction we won't have a defined block explicitly. We should find the record_file corresponding to the timestamp for the transactionHash that is sent as an input, so that the ContractCallContext contains proper data.

This is already done when constructing the call service parameters, and the block number is passed to the ContractCallService.

IvanKavaldzhiev · 2024-05-30T10:00:49Z

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java

-                    // if the call is historical
-                    ctx.initializeStackFrames(store.getStackedStateFrames());
-                    final var ethCallTxnResult = doProcessCall(params, params.getGas(), true);
+                    final var ethCallTxnResult = callContract(params, TracerType.OPERATION, ctx);


That's a symptom of why we should split the logic in 2 different services. We shouldn't use any tracing in the case of eth_call, eth_estimateGas, so that we don't make this operations more CPU and memory intensive.

I agree, but the other operation tracer was used previously as well, we haven’t introduced any changes to the other methods.

Previously, the MirrorOperationTracer was injected directly into the processor and now we inject a Map<TracerType, HederaEvmTracer> instead, and thats why we need to pass the tracer type here, in order to get the right tracer that we want to use.

If we don’t want to do any tracing in eth_call and eth_estimateGas, we can create a new TracerType.NONE that’s used by default and doesn’t invoke any tracers when processing the call.

What’s your opinion here?

IvanKavaldzhiev · 2024-05-30T10:06:50Z

...b3/src/main/java/com/hedera/mirror/web3/evm/contracts/execution/traceability/TracerType.java

+
+public enum TracerType {
+    OPCODE,
+    OPERATION


For OPERATION we use directly the json-rpc-relay implementation, which utilises the /actions endpoint. The REST API in the web3 module should be used only for OPCODE tracing. I think we should delete the OPERATION type.

#8241 (comment)

IvanKavaldzhiev · 2024-05-30T10:52:00Z

hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/EntityServiceImpl.java

+@Service
+@RequiredArgsConstructor
+public class EntityServiceImpl implements EntityService {
+    private final EntityRepository entityRepository;


I think we can directly use the EntityDatabaseAccessor for the moment and delete this Service definition.

It's used in the StackedStateFrames component, but could be used as a service layer abstraction over the EntityRepository.

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

konstantinabl added 3 commits May 13, 2024 19:41

Some draft changes

1961f22

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Merge remote-tracking branch 'origin/8174-add-new-opcodes-endpoint' i…

68e41ad

…nto 8173-implement-opcode-logger-tracing-logic Need changes from other branch to debug

debugs precompile transaction

91499c4

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Merge 8174

ddae789

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

konstantinabl changed the base branch from main to 8174-add-new-opcodes-endpoint May 15, 2024 12:47

konstantinabl added 6 commits May 16, 2024 14:02

Adds main implementation for tracing non-precompile calls

a04dbaa

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Changes test according to new changes in processOpcodeCall

94d1736

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Removes unused import from class

4b6781e

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Removes unused repository

b193628

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

merges 8174

a2ee522

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

victor-yanev requested changes May 17, 2024

View reviewed changes

konstantinabl and others added 7 commits May 17, 2024 16:08

Adds new logic for calculating storage, memory and stack

8fde98c

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

chore: add tests for OpcodeTracer

88260ef

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

Fixes some types

36c8277

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Fixes merge conflicts

f87966a

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Improves opcodes response format; Puts tracer options iin context

f5f0bde

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

victor-yanev requested changes May 20, 2024

View reviewed changes

victor-yanev reviewed May 20, 2024

View reviewed changes

...ra-mirror-web3/src/main/java/com/hedera/mirror/web3/repository/ContractActionRepository.java Outdated Show resolved Hide resolved

victor-yanev added 7 commits May 20, 2024 15:33

test: improve OpcodeTracerTest

9a335fd

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

Merge remote-tracking branch 'origin/8173-implement-opcode-logger-tra…

93fcaab

…cing-logic' into 8173-implement-opcode-logger-tracing-logic

test: improve OpcodeTracerTest

94fed9d

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

test: fix OpcodesControllerTest

c9b78cc

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

chore: resolve conflicts

1dd4a6a

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

chore: resolve conflicts

30a4cc6

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

chore: address comments

73706b5

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

victor-yanev added enhancement Type: New feature web3 Area: Web3 API limechain Work planned for the LimeChain team labels May 22, 2024

victor-yanev changed the title ~~Drafts opcode logger functionality~~ Implement opcode logger functionality May 22, 2024

victor-yanev changed the title ~~Implement opcode logger functionality~~ feat: Implement opcode logger functionality May 22, 2024

victor-yanev marked this pull request as ready for review May 22, 2024 10:29

victor-yanev requested review from IvanKavaldzhiev, steven-sheehy and kselveliev May 22, 2024 10:30

victor-yanev added 4 commits May 23, 2024 13:42

chore: use revert reason from contract actions in tracePrecompileCall

3e4ed15

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

chore: add warning and trace logs in OpcodeTracer

81f1edc

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

chore: throw EntityNotFoundException error instead of IllegalArgument…

d06ca0b

…Exception when transaction cannot be found Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

victor-yanev assigned victor-yanev and konstantinabl May 23, 2024

victor-yanev and others added 6 commits May 23, 2024 17:58

test: extend OpcodeTracerTest with additional scenarios

aa4a9e0

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

chore: add docs + simplify logic in ContractCallService

a9359b0

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

test: Improve coverage in OpcodeTracerTest

c7c2264

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

test: fix sonar issue in OpcodeTracerTest

2052196

Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>

Adds entity service

bda5e6d

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Fixes wrong sender lon zero address

4552e1d

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

konstantinabl requested a review from victor-yanev May 29, 2024 15:49

IvanKavaldzhiev reviewed May 30, 2024

View reviewed changes

Merge 8174

4b47076

Signed-off-by: Konstantina Blazhukova <konstantina.blajukova@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement opcode logger functionality #8241

feat: Implement opcode logger functionality #8241

konstantinabl commented May 7, 2024 •

edited

sonarcloud bot commented May 14, 2024

codecov bot commented May 14, 2024 •

edited

victor-yanev left a comment •

edited

IvanKavaldzhiev May 30, 2024

IvanKavaldzhiev May 30, 2024

konstantinabl May 30, 2024

IvanKavaldzhiev May 30, 2024

victor-yanev May 30, 2024

IvanKavaldzhiev May 30, 2024

victor-yanev May 30, 2024

IvanKavaldzhiev May 30, 2024

victor-yanev May 30, 2024

IvanKavaldzhiev May 30, 2024

IvanKavaldzhiev May 30, 2024

feat: Implement opcode logger functionality #8241

Are you sure you want to change the base?

feat: Implement opcode logger functionality #8241

Conversation

konstantinabl commented May 7, 2024 • edited

sonarcloud bot commented May 14, 2024

Quality Gate failed

codecov bot commented May 14, 2024 • edited

Codecov Report

victor-yanev left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

konstantinabl commented May 7, 2024 •

edited

codecov bot commented May 14, 2024 •

edited

victor-yanev left a comment •

edited