Go native tracer #1978

jsvisa · 2022-10-31T13:14:02Z

implements #1970

DON'T MERGE, test in process.

jsvisa · 2022-11-01T01:57:48Z

@palango Seems we don't have a way to decode all bytes into ethcompatible transactions, those test cases https://github.com/celo-org/celo-blockchain/blob/master/eth/tracers/tracers_test.go#L205-L297 can't pass 😭

palango · 2022-11-07T17:42:39Z

@jsvisa Thanks a lot for looking into this. I don't have much experience with this code, but someone from team is going to have a look. It might take a few days though.

hbandura · 2022-11-07T21:53:24Z

@jsvisa @palango if I'm not mistaken that's the address for the directory of core contracts. Without taking a real look at this I'd say it could have something to do with either paying vaildator rewards at the end of the tx or paying the gas with a different currency than the native one (celo stable tokens)

jsvisa · 2022-11-08T02:03:58Z

@hbandura thanks for your explanation.

eg:

  "gas": "0x186a0",
   "gasUsed": "0xffee3628",

Does this means gas in the CELO native token, but gasUsed is in celo stable token? If that's the case, how do we distinguish between native and stable, and how to exchange the stable back to native?

Thank you in advance.

palango · 2023-02-20T13:51:14Z

Does this means gas in the CELO native token, but gasUsed is in celo stable token? If that's the case, how do we distinguish between native and stable, and how to exchange the stable back to native?

You can set the currency used to pay fees with using the feeCurrency field in a transaction. See also https://docs.celo.org/protocol/transaction/erc20-transaction-fees

palango · 2023-02-20T16:54:09Z

@jsvisa I have time to look deeper into this now. How did you pick the commits to include in this?

eth/tracers/api_test.go

jsvisa · 2023-02-21T06:34:51Z

@jsvisa I have time to look deeper into this now. How did you pick the commits to include in this?

Do you means the git commits? I pick up in manually

palango · 2023-03-09T11:48:51Z

Do you means the git commits? I pick up in manually

How did you select them? I see more tracing related commit in the upstream repository.

Also, I found out that the commit eth/tracers: package restructuring (#23857) in this PR differs from the upstream commit ethereum/go-ethereum@6b9c77f

This makes the transactions undecodable, so it would be interesting to know where you found that particular commit.

palango · 2023-03-16T11:13:17Z

I'm putting this aside for now, there's higher priority things and one test still fails, for unclear reasons.

The test is TestBlockTracingConcurrentMapAccess which tests parallel tracing calls via the json rpc api. Below are some things I found out during debugging, as possible hints for the next person looking into this.

=== RUN   TestBlockTracingConcurrentMapAccess
Checking getExchangeSpenders. spenders = [0x000000000000000000000000000000000000d028]
Checking medianRate. numerator = 1000000000000000000000000  denominator = 1000000000000000000000000
Checking gas price minimum. cusdValue = 100000000
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x90 pc=0x105291b30]

goroutine 75 [running]:
github.com/celo-org/celo-blockchain/core/vm.(*StructLogger).CaptureState(0x14000242850, 0x0, 0x60, 0x186a0, 0x3, 0x20?, {0x0, 0x0, 0xa19?}, 0x1, ...)
	/Users/paul/Projects/celo-blockchain/core/vm/logger.go:211 +0x970
github.com/celo-org/celo-blockchain/core/vm.(*EVMInterpreter).Run(0x1400805c900, 0x140080152c0, {0x14008028800, 0x24, 0x40}, 0x1)
	/Users/paul/Projects/celo-blockchain/core/vm/interpreter.go:257 +0x7ac
github.com/celo-org/celo-blockchain/core/vm.(*EVM).StaticCall(0x1400805aa80, {0x105d20560, 0x140080549c0}, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...}, ...)
	/Users/paul/Projects/celo-blockchain/core/vm/evm.go:430 +0x4ec
github.com/celo-org/celo-blockchain/core/vm/vmcontext.(*SharedEVMRunner).Query(0x140077fc8a0, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...}, ...)
	/Users/paul/Projects/celo-blockchain/core/vm/vmcontext/runner.go:102 +0x94
github.com/celo-org/celo-blockchain/contracts.(*BoundMethod).run(0x14000328c30, {0x105d274a0, 0x140077fc8a0}, {0x105ccf220, 0x14008054990}, 0x1, 0x14006fb01a8?, {0x14008060400, 0x1, 0x1})
	/Users/paul/Projects/celo-blockchain/contracts/contract_call.go:103 +0x258
github.com/celo-org/celo-blockchain/contracts.(*BoundMethod).Query(...)
	/Users/paul/Projects/celo-blockchain/contracts/contract_call.go:76
github.com/celo-org/celo-blockchain/contracts.GetRegisteredAddress({0x105d274a0, 0x140077fc8a0}, {0xaa, 0xb7, 0x4a, 0x77, 0xd8, 0xce, 0x26, 0xd2, ...})
	/Users/paul/Projects/celo-blockchain/contracts/registry.go:23 +0x13c
github.com/celo-org/celo-blockchain/contracts/reserve.TobinTax({0x105d274a0, 0x140077fc8a0}, {0xf1, 0x39, 0xb6, 0x59, 0xfb, 0x1b, 0xea, 0xe2, ...})
	/Users/paul/Projects/celo-blockchain/contracts/reserve/tobin_tax.go:29 +0x54
github.com/celo-org/celo-blockchain/contracts/reserve.ComputeTobinTax({0x105d274a0?, 0x140077fc8a0?}, {0xf1, 0x39, 0xb6, 0x59, 0xfb, 0x1b, 0xea, 0xe2, ...}, ...)
	/Users/paul/Projects/celo-blockchain/contracts/reserve/tobin_tax.go:56 +0x4c
github.com/celo-org/celo-blockchain/core/vm/vmcontext.TobinTransfer(0x1400805aa80, {0xf1, 0x39, 0xb6, 0x59, 0xfb, 0x1b, 0xea, 0xe2, 0x7d, ...}, ...)
	/Users/paul/Projects/celo-blockchain/core/vm/vmcontext/context.go:153 +0xec
github.com/celo-org/celo-blockchain/core/vm.(*EVM).Call(0x1400805aa80, {0x105d20560, 0x14008054948}, {0xbd, 0x3a, 0x2c, 0x79, 0x1d, 0xf9, 0x32, ...}, ...)
	/Users/paul/Projects/celo-blockchain/core/vm/evm.go:235 +0x4f4
github.com/celo-org/celo-blockchain/core.(*StateTransition).TransitionDb(0x14007e8fea0)
	/Users/paul/Projects/celo-blockchain/core/state_transition.go:568 +0x8ec
github.com/celo-org/celo-blockchain/core.ApplyMessage(0x105d17c18?, {0x105d2bba0, 0x14007e8fe00}, 0x1400807a5a0?, {0x105d274e0, 0x14008037ca0}, 0xec0eae7139df5d94?)
	/Users/paul/Projects/celo-blockchain/core/state_transition.go:242 +0x314
github.com/celo-org/celo-blockchain/eth/tracers.(*API).traceTx(0x140004d2510, {0x105d25c38, 0x140081ba140}, {0x105d2bba0?, 0x14007e8fe00?}, 0x1400802bf90, {0x105d17c18, 0x105d17c38, 0x140080653e0, 0x1400807a5a0, ...}, ...)
	/Users/paul/Projects/celo-blockchain/eth/tracers/api.go:975 +0x4fc
github.com/celo-org/celo-blockchain/eth/tracers.(*API).traceBlock.func1()
	/Users/paul/Projects/celo-blockchain/eth/tracers/api.go:645 +0x494
created by github.com/celo-org/celo-blockchain/eth/tracers.(*API).traceBlock
	/Users/paul/Projects/celo-blockchain/eth/tracers/api.go:632 +0x5b4
FAIL	github.com/celo-org/celo-blockchain/e2e_test	1.166s

The nil pointer dereference happens in the StructLogger.CaptureState call, where l.env is nil for some reason (https://github.com/jsvisa/celo-blockchain/blob/f56b115f10ce0e3417ce75da9c6beafa3b9cef91/core/vm/logger.go#L211). This means that StructLogger.CaptureStart has never been called on that instance. This should be done, when debug mode is enabled, see https://github.com/jsvisa/celo-blockchain/blob/f56b115f10ce0e3417ce75da9c6beafa3b9cef91/core/vm/evm.go#L238-L240 . So it seems that debug is set to false for some reason. However, at least the EVM instance used for tracing definitely has the debug flag set (https://github.com/jsvisa/celo-blockchain/blob/f56b115f10ce0e3417ce75da9c6beafa3b9cef91/eth/tracers/api.go#L970).

My best guess currently is that some object is shared between goroutines and this flag is overwritten incorrectly somewhere.

jsvisa · 2023-03-16T11:28:47Z

@palango I will take some time in the next days to have a look.

eelanagaraj · 2023-03-24T17:27:34Z

Looked into this a bit and I may have found some more information on what's going on here, though still don't have a thorough understanding/solution:

this not only fails for concurrent calls but also for tracing a block that has any transactions in it (used a test with a single transfer tx, making sequential calls)
- @jsvisa were you able to trace any txs with the new functionality? Wondering if there's something particular about native transfers vs. any other transactions.
this fails even if the logic of tracing txs (in eth/tracers/api.go in traceBlock) happens sequentially (modified this temporarily to debug/test)
failure is caused in core/vm/evm.go in Call, specifically in the transfer call in L235. (This is before Capture.Start has been called)
- transfer sets the env.Config.Debug to False to intentionally not trace within the scope of that call -- it's not clear to me exactly where/why Capture.State is being called within this scope
  - my current hypothesis/thing that I want to investigate further is that it may be due to the change in the PR of not passing in the env to CaptureState and instead using the calling' scope's env.StructLogger.evm, in conjunction with how the same EVM is reused (in SharedEVMRunner in TobinTransfer). It's unfortunately pretty hard to follow/debug this logic even in synchronous mode.

…828) Signed-off-by: Delweng <delweng@gmail.com>

This PR changes long-running chain tracing, so that it at some points releases the memory trie db, and switch over to a fresh disk-backed trie. Signed-off-by: Delweng <delweng@gmail.com>

Signed-off-by: Delweng <delweng@gmail.com>

* eth/tracers: add basic native loader * eth/tracers: add GetResult to tracer interface * eth/tracers: add native call tracer * eth/tracers: fix call tracer json result * eth/tracers: minor fix * eth/tracers: fix * eth/tracers: fix benchTracer * eth/tracers: test native call tracer * eth/tracers: fix * eth/tracers: rm extra make Co-authored-by: Martin Holst Swende <martin@swende.se> * eth/tracers: rm extra make * eth/tracers: make callFrame private * eth/tracers: clean-up and comments * eth/tracers: add license * eth/tracers: rework the model a bit * eth/tracers: move tracecall tests to subpackage * cmd/geth: load native tracers * eth/tracers: minor fix * eth/tracers: impl stop * eth/tracers: add native noop tracer * renamings Co-authored-by: Martin Holst Swende <martin@swende.se> * eth/tracers: more renamings * eth/tracers: make jstracer non-exported, avoid cast * eth/tracers, core/vm: rename vm.Tracer to vm.EVMLogger for clarity * eth/tracers: minor comment fix * eth/tracers/testing: lint nitpicks * core,eth: cancel evm on nativecalltracer stop * Revert "core,eth: cancel evm on nativecalltracer stop" This reverts commit 01bb908790a369c1bb9d3937df9325c6857bf855. * eth/tracers: linter nits * eth/tracers: fix output on err Co-authored-by: Martin Holst Swende <martin@swende.se> Signed-off-by: Delweng <delweng@gmail.com>

Signed-off-by: Delweng <delweng@gmail.com>

Make linter happy

jsvisa · 2023-04-21T14:15:00Z

@eelanagaraj after you cherry pick efbc25a the datarace issue resolved or not?

jsvisa · 2023-04-21T14:16:19Z

BTW, seems some trace testcases in https://github.com/celo-org/celo-blockchain/blob/master/eth/tracers/tracers_test.go were removed, I think it is necessary to keep them

eelanagaraj · 2023-04-21T14:24:22Z

Hey @jsvisa so the actual issue causing that test failure looks like it was caused by the bug fixed here: #2061. In that PR, we also re-included the commented-out tests, to make sure that the frozen test output was then the same in this PR (to have a basis for comparison for this PR).

Let me know if something looks like it was missing/went wrong in the process of rebasing this PR though, but the test case outputs should now already be updated (from master)!

We are currently doing some more thorough testing on the go native tracer vs. the old one for a variety of txs and investigating some strange discrepancies with gasUsed values, but will keep you updated!

jsvisa · 2023-04-21T14:57:46Z

@eelanagaraj thanks for your quick reply, seems the testcases(callTracer tester) more moved inside the internal dir, so it's ok.

jsvisa · 2023-04-21T14:59:05Z

@hbandura thanks for your explanation.

eg:
  "gas": "0x186a0",
   "gasUsed": "0xffee3628",
Does this means gas in the CELO native token, but gasUsed is in celo stable token? If that's the case, how do we distinguish between native and stable, and how to exchange the stable back to native?

Thank you in advance.

And does this issue also resolved?

eelanagaraj · 2023-04-24T11:14:58Z

Hey @jsvisa ! That's what I'm currently looking into -- the current working theory is that this actually represents a negative value (2s complement; this may have not been an unsigned int in the past and apparently we have had negative gasUsed values in these traces before). These occurrences seem to correlate strongly with calls to the transfer precompile (0x00000000000000000000000000000000000000fd), but I'm not totally sure why that is.

I'll let you know if/when I understand if negative gasUsed values are "correct"/expected (possibly the case in the past, but that's a bit divergent from ethereum) & if I can come up with a more useful mini-repro of this behavior in a unit test. 🙏

eelanagaraj · 2023-04-28T15:35:25Z

Still not 100% what's going on with the underflows/negative gas values, but it looks like that is not a new thing with this PR. The mismatches I found between the callTraces' gasUsed values between the old tracer & this PR are accounted for by a change from uint -> uint64 in the gasUsed vals, which are apparent during the underflows.

Once this PR is merged (adds a few more Celo-specific tracing tests), I'll update this PR and it should be ready to merge from my view. Thanks for your patience 🙏

…-trace

codecov · 2023-05-03T14:55:39Z

Codecov Report

Patch coverage: 66.64% and project coverage change: +0.94 🎉

Comparison is base (71bdbcf) 54.30% compared to head (493743e) 55.24%.

❗ Current head 493743e differs from pull request most recent head 7f2410b. Consider uploading reports for the commit 7f2410b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1978      +/-   ##
==========================================
+ Coverage   54.30%   55.24%   +0.94%     
==========================================
  Files         692      675      -17     
  Lines      115642   113693    -1949     
==========================================
+ Hits        62795    62810      +15     
+ Misses      49014    47028    -1986     
- Partials     3833     3855      +22

Impacted Files	Coverage Δ
cmd/devp2p/internal/ethtest/transaction.go	`0.00% <0.00%> (ø)`
cmd/evm/runner.go	`0.00% <0.00%> (ø)`
cmd/evm/staterunner.go	`0.00% <0.00%> (ø)`
cmd/geth/chaincmd.go	`0.00% <0.00%> (ø)`
cmd/geth/config.go	`0.00% <0.00%> (ø)`
cmd/geth/main.go	`21.39% <ø> (+0.91%)`	⬆️
cmd/geth/usage.go	`10.90% <ø> (ø)`
cmd/utils/flags.go	`2.56% <0.00%> (ø)`
consensus/istanbul/utils.go	`47.10% <ø> (+1.13%)`	⬆️
contracts/currency/currency.go	`55.00% <ø> (ø)`
... and 61 more

... and 37 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

eelanagaraj · 2023-05-04T09:13:20Z

unit tests that don't work with the new tracer package structure have been moved to e2e
- unit tests look like they pass but are failing CI due to this issue https://github.com/celo-org/celo-blockchain-planning/issues/65
we understand what's going on with the underflows (has to do with reused state on specific unmetered calls; this doesn't have to do with this PR and is tracked in these two issues: Traces should not include EVM Call's made while executing precompile logic #2078, https://github.com/celo-org/celo-blockchain-planning/issues/66)
tested a bunch of txs against the old deployment in blockscout

I think we just need second reviews from me & @palango, and any additional blockscout testing that @carterqw2 wants to do!

Fix typo

eelanagaraj

LGTM, will merge pending testing/sign-off from @carterqw2

palango · 2023-06-22T12:54:31Z

Continued in #2137

jsvisa requested review from hbandura, nategraf and a team as code owners October 31, 2022 13:14

jsvisa force-pushed the feature/go-native-trace branch from f28e518 to cdd905e Compare November 7, 2022 12:47

This comment was marked as outdated.

Sign in to view

palango reviewed Feb 20, 2023

View reviewed changes

eth/tracers/api_test.go Outdated Show resolved Hide resolved

palango removed request for hbandura and nategraf March 9, 2023 12:02

palango force-pushed the feature/go-native-trace branch from e7e3fa5 to 30c5647 Compare March 13, 2023 14:34

palango marked this pull request as draft March 14, 2023 15:59

carterqw2 mentioned this pull request Mar 15, 2023

Implement fetching internal transactions from callTracer celo-org/blockscout#885

Merged

palango force-pushed the feature/go-native-trace branch from d6528ef to f56b115 Compare March 16, 2023 11:02

carterqw2 changed the title ~~WIP: Feature/go native trace~~ Go native tracer Mar 22, 2023

eelanagaraj mentioned this pull request Apr 6, 2023

Set interpreter config to debug as well in evm.SetDebug and update tracing tests #2061

Merged

2 tasks

holiman and others added 5 commits April 19, 2023 18:14

eth/tracers: invoke enter/exit on 0-value calls to inex accounts (#23…

5aa170e

…828) Signed-off-by: Delweng <delweng@gmail.com>

eth: make traceChain avoid OOM on long-running tracing (#23736)

f890a7d

This PR changes long-running chain tracing, so that it at some points releases the memory trie db, and switch over to a fresh disk-backed trie. Signed-off-by: Delweng <delweng@gmail.com>

eth/tracers: missing argument in IsEspresso's trace

35a7b1d

Signed-off-by: Delweng <delweng@gmail.com>

eth/tracers: make native calltracer default (#23867)

8b55933

Signed-off-by: Delweng <delweng@gmail.com>

holiman and others added 3 commits April 19, 2023 18:14

eth/tracers: avoid using blockCtx concurrently (#24286)

efbc25a

Fix tests

53c7c7b

Make linter happy

Fix rebase with moved files

a7d11fe

eelanagaraj force-pushed the feature/go-native-trace branch from f56b115 to a7d11fe Compare April 19, 2023 16:15

jsvisa marked this pull request as ready for review April 21, 2023 15:01

Merge remote-tracking branch 'upstream/master' into feature/go-native…

9ffa6f7

…-trace

eelanagaraj force-pushed the feature/go-native-trace branch from d049c36 to 9ffa6f7 Compare May 3, 2023 14:27

Eela Nagaraj added 2 commits May 4, 2023 15:17

Move API tracing tests to e2e

92de152

Fix typo

Move TestPrestateTracerTransfer to e2e test to avoid circular import

4308c7a

eelanagaraj force-pushed the feature/go-native-trace branch from e82bdb2 to 4308c7a Compare May 4, 2023 13:18

Merge branch 'master' into feature/go-native-trace

2480a96

eelanagaraj requested review from palango and carterqw2 May 4, 2023 13:19

eelanagaraj approved these changes May 4, 2023

View reviewed changes

palango added 4 commits June 16, 2023 15:23

Merge branch 'master' into feature/go-native-trace

8b5d2f8

Merge branch 'master' into feature/go-native-trace

493743e

Reorder tracer tests to minimize diff

52e96c1

Merge branch 'master' into feature/go-native-trace

7f2410b

palango mentioned this pull request Jun 22, 2023

Merge native tracer #2137

Merged

palango closed this Jun 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go native tracer #1978

Go native tracer #1978

jsvisa commented Oct 31, 2022 •

edited

jsvisa commented Nov 1, 2022

This comment was marked as outdated.

palango commented Nov 7, 2022

hbandura commented Nov 7, 2022 •

edited

jsvisa commented Nov 8, 2022

palango commented Feb 20, 2023

palango commented Feb 20, 2023

jsvisa commented Feb 21, 2023

palango commented Mar 9, 2023

palango commented Mar 16, 2023

jsvisa commented Mar 16, 2023

eelanagaraj commented Mar 24, 2023

jsvisa commented Apr 21, 2023

jsvisa commented Apr 21, 2023

eelanagaraj commented Apr 21, 2023 •

edited

jsvisa commented Apr 21, 2023

jsvisa commented Apr 21, 2023

eelanagaraj commented Apr 24, 2023

eelanagaraj commented Apr 28, 2023

codecov bot commented May 3, 2023 •

edited

eelanagaraj commented May 4, 2023

eelanagaraj left a comment

palango commented Jun 22, 2023

Go native tracer #1978

Go native tracer #1978

Conversation

jsvisa commented Oct 31, 2022 • edited

jsvisa commented Nov 1, 2022

This comment was marked as outdated.

palango commented Nov 7, 2022

hbandura commented Nov 7, 2022 • edited

jsvisa commented Nov 8, 2022

palango commented Feb 20, 2023

palango commented Feb 20, 2023

jsvisa commented Feb 21, 2023

palango commented Mar 9, 2023

palango commented Mar 16, 2023

jsvisa commented Mar 16, 2023

eelanagaraj commented Mar 24, 2023

jsvisa commented Apr 21, 2023

jsvisa commented Apr 21, 2023

eelanagaraj commented Apr 21, 2023 • edited

jsvisa commented Apr 21, 2023

jsvisa commented Apr 21, 2023

eelanagaraj commented Apr 24, 2023

eelanagaraj commented Apr 28, 2023

codecov bot commented May 3, 2023 • edited

Codecov Report

eelanagaraj commented May 4, 2023

eelanagaraj left a comment

Choose a reason for hiding this comment

palango commented Jun 22, 2023

jsvisa commented Oct 31, 2022 •

edited

hbandura commented Nov 7, 2022 •

edited

eelanagaraj commented Apr 21, 2023 •

edited

codecov bot commented May 3, 2023 •

edited