feat(rpc) Implement `Filecoin.EthTraceBlock` #4991

elmattic · 2024-11-21T13:30:57Z

Summary of changes

Changes introduced in this pull request:

implement RPC method Filecoin.EthTraceBlock
API compare tests
fix bug in eth::decode_payload and add unit tests

$ forest-tool api compare --lotus /ip4/127.0.0.1/tcp/1234/http --forest /ip4/127.0.0.1/tcp/2345/http forest_snapshot_calibnet_2024-12-16_height_2233655.forest.car.zst --filter EthTraceBlock -n 200
| RPC Method                   | Forest | Lotus |
|------------------------------|--------|-------|
| Filecoin.EthTraceBlock (200) | Valid  | Valid |

Reference issue to close (if applicable)

Closes #4708

Change checklist

I have performed a self-review of my own code,
I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
I have added tests that prove my fix is effective or that my feature works (if possible),
I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

src/rpc/methods/eth/utils.rs

LesnyRumcajs · 2025-01-08T14:32:29Z

src/shim/error.rs

+                code if code > ExitCode_latest::SYS_MISSING_RETURN.value()
+                    && code < ExitCode_latest::FIRST_USER_EXIT_CODE =>
+                {
+                    write!(f, "SysErrReserved{}({})", code - 10, code)


Why subtract 10?

We want to align errors with Lotus display, so code - 10 does that.

Maybe using SYS_ASSERTION_FAILED code instead? (it equals to 10).

Do you know why Lotus subtracts 10? I'm okay with having it as well. Just please comment in the code the reason for this so the magic number is less magical.

Sure, I will explain in a comment the rational behind this.

I think there is a better, cleaner and more correct solution to match Lotus exit codes.

We just need to create a PR in ref-fvm to uncomment those dead lines:
https://github.com/filecoin-project/ref-fvm/blob/master/shared/src/error/mod.rs#L64
https://github.com/filecoin-project/ref-fvm/blob/master/shared/src/error/mod.rs#L74
https://github.com/filecoin-project/ref-fvm/blob/master/shared/src/error/mod.rs#L81-L84

And add those constants to our match expression in forest/src/shim/error.rs.

Let's address that in a subsequent PR.

LesnyRumcajs · 2025-01-08T14:35:23Z

src/rpc/methods/eth/trace.rs

+use fil_actor_eam_state::v12 as eam12;
+use fil_actor_evm_state::v15 as evm12;
+use fil_actor_init_state::v12::ExecReturn;
+use fil_actor_init_state::v15::Method as InitMethod;
+use fvm_ipld_blockstore::Blockstore;
+use fvm_shared4::{error::ExitCode as ExitCodeV4, METHOD_CONSTRUCTOR};


Do we have to lock the actor version and the FVM version here? It would be great to avoid it and work through actor interface and shims.

I can have a look.

LesnyRumcajs

In general, does Lotus have a more extensive suite for this tracing? If so, could we migrate some of those tests?

elmattic · 2025-01-09T10:35:51Z

In general, does Lotus have a more extensive suite for this tracing? If so, could we migrate some of those tests?

Unfortunately, no. They have very few tests. I've already migrated the one that tests decode_payload free function.

LesnyRumcajs · 2025-01-09T10:37:44Z

In general, does Lotus have a more extensive suite for this tracing? If so, could we migrate some of those tests?

Unfortunately, no. They have very few tests. I've already migrated the one that tests decode_payload free function.

Well, then, fortunately, I'm a man of great faith. 🙏

elmattic added 30 commits November 21, 2024 14:30

Make schema work for EthTraceBlock

5325bd5

Merge branch 'main' into elmattic/eth-trace-block

f43ce43

Add eth trace module

549247a

Merge branch 'main' into elmattic/eth-trace-block

b110384

Implement more functions

18cad8c

Add fil_actor_evm_state dependency and move to ExitCode v4

e95328a

Update to V4

ab51150

Implement more functions

d86fbb4

Complete trace_native_create impl

bc90c41

Scaffold the impl of decode_create_via_eam

fe79d4e

Implement trace_is_evm_or_eam function

ff3144f

Implement trace_eth_create function

3bd0b8b

Merge branch 'main' into elmattic/eth-trace-block

bfc11a7

Add trace_evm_private impl (wip)

3c53381

Add build_traces impl

e7ef3d7

Add build_trace impl

442144d

Remove from_repr usage

2cd28a2

Add header and minor cleanups

9990f05

Fix trace_eth_create function

549d41d

Move most functions to private

5325467

Fix build_trace

65938f6

Fix execution_trace function to return state root

f4ef00d

Merge branch 'main' into elmattic/eth-trace-block

29914cd

Fix trace_evm_private function

fd5783c

Fix trace_evm_call function

3872fd2

Appease clippy

efac542

Add comment and refactor

1dee860

Fix gas_used member init

2517812

Merge branch 'main' into elmattic/eth-trace-block

68ae2b2

Merge branch 'main' into elmattic/eth-trace-block

b0bc8ad

elmattic added 5 commits January 7, 2025 13:08

Fix header

55d82e7

Add decode payload tests

cfa0e96

Fix dev-dependencies

fee3058

Cleanup

a41f450

Merge branch 'main' into elmattic/eth-trace-block

2e31462

elmattic marked this pull request as ready for review January 7, 2025 17:19

elmattic requested a review from a team as a code owner January 7, 2025 17:19

elmattic requested review from hanabi1224 and LesnyRumcajs and removed request for a team January 7, 2025 17:19

LesnyRumcajs reviewed Jan 8, 2025

View reviewed changes

src/rpc/methods/eth/utils.rs Outdated Show resolved Hide resolved

LesnyRumcajs reviewed Jan 8, 2025

View reviewed changes

elmattic added 3 commits January 8, 2025 17:47

Merge branch 'main' into elmattic/eth-trace-block

4bb15ab

Use fvm_shared identity hash definition

d111d3d

Remove magic number

ff508b0

LesnyRumcajs reviewed Jan 9, 2025

View reviewed changes

LesnyRumcajs approved these changes Jan 9, 2025

View reviewed changes

Refactor to use exit code shim

b2c5430

hanabi1224 approved these changes Jan 9, 2025

View reviewed changes

Fix doc comment

284a66f

hanabi1224 approved these changes Jan 9, 2025

View reviewed changes

elmattic enabled auto-merge January 9, 2025 11:59

Merge branch 'main' into elmattic/eth-trace-block

d1e1ac6

elmattic added this pull request to the merge queue Jan 9, 2025

Merged via the queue into main with commit 1f1c818 Jan 9, 2025
42 checks passed

elmattic deleted the elmattic/eth-trace-block branch January 9, 2025 12:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rpc) Implement `Filecoin.EthTraceBlock` #4991

feat(rpc) Implement `Filecoin.EthTraceBlock` #4991

elmattic commented Nov 21, 2024 •

edited

Loading

LesnyRumcajs Jan 8, 2025

elmattic Jan 8, 2025 •

edited

Loading

LesnyRumcajs Jan 8, 2025

elmattic Jan 8, 2025

elmattic Jan 9, 2025 •

edited

Loading

elmattic Jan 9, 2025

LesnyRumcajs Jan 8, 2025 •

edited

Loading

elmattic Jan 8, 2025

LesnyRumcajs left a comment

elmattic commented Jan 9, 2025

LesnyRumcajs commented Jan 9, 2025

feat(rpc) Implement Filecoin.EthTraceBlock #4991

feat(rpc) Implement Filecoin.EthTraceBlock #4991

Conversation

elmattic commented Nov 21, 2024 • edited Loading

Summary of changes

Reference issue to close (if applicable)

Other information and links

Change checklist

LesnyRumcajs Jan 8, 2025

Choose a reason for hiding this comment

elmattic Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

LesnyRumcajs Jan 8, 2025

Choose a reason for hiding this comment

elmattic Jan 8, 2025

Choose a reason for hiding this comment

elmattic Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

elmattic Jan 9, 2025

Choose a reason for hiding this comment

LesnyRumcajs Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

elmattic Jan 8, 2025

Choose a reason for hiding this comment

LesnyRumcajs left a comment

Choose a reason for hiding this comment

elmattic commented Jan 9, 2025

LesnyRumcajs commented Jan 9, 2025

feat(rpc) Implement `Filecoin.EthTraceBlock` #4991

feat(rpc) Implement `Filecoin.EthTraceBlock` #4991

elmattic commented Nov 21, 2024 •

edited

Loading

elmattic Jan 8, 2025 •

edited

Loading

elmattic Jan 9, 2025 •

edited

Loading

LesnyRumcajs Jan 8, 2025 •

edited

Loading