Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support trace API similar to OpenEthereum/Parity's one #211

Merged
merged 75 commits into from
Nov 5, 2020

Conversation

ziogaschr
Copy link
Member

@ziogaschr ziogaschr commented Oct 15, 2020

This PR adds a compatible trace API with OpenEthereum's one.

At the moment we have implemented:

  • trace_transaction
  • trace_block

For a node to enable this feature, it has to start with --vmdebug flag.

For the best performance node has to be synced in archive mode.
In console the trace API can be used.
Last, the trace RPC API is available, though it have to be enabled.

Fixes #100.


Work in Progress. Opened a PR to handle discussion over here.

…o OpenEthereum

# Conflicts:
#	eth/tracers/internal/tracers/assets.go
js/tracers: make calltracer report value in selfdestructs (#21549)

ethereum/go-ethereum@71c37d8
@ziogaschr
Copy link
Member Author

At this point we reached 100% compatibility with OpenEthereum tracers for trace_block and trace_transaction at Mordor testnet chain.

@meowsbits
Copy link
Contributor

meowsbits commented Nov 3, 2020

State tests are failing, looks like it could be an issue with CREATE gas. Possibly related to c32bcd6.

To re/produce: go test ./tests/

They look mostly like out of gas cases, maybe?

@ziogaschr ziogaschr changed the title [WiP] Support trace API similar to OpenEthereum's one Support trace API similar to OpenEthereum/Parity's one Nov 4, 2020
@@ -106,7 +106,7 @@ func (s *StructLog) ErrorString() string {
// if you need to retain them beyond the current call.
type Tracer interface {
CaptureStart(from common.Address, to common.Address, create bool, input []byte, gas uint64, value *big.Int) error
CaptureState(env *EVM, pc uint64, op OpCode, gas, cost uint64, memory *Memory, stack *Stack, rStack *ReturnStack, rData []byte, contract *Contract, depth int, err error) error
CaptureState(env *EVM, pc uint64, op OpCode, gas, availableGas, cost uint64, memory *Memory, stack *Stack, rStack *ReturnStack, rData []byte, contract *Contract, depth int, err error) error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ziogaschr I think you mentioned yesterday that this parameter may be removable since this value is equivalent to env.callGasTemp. If so, is it worth doing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point removed the extra availableGasCopy and use the in.evm.callGasTemp. This one can be removed if I make it public as in.evm.CallGasTemp. What do you think about it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK to make it public because that follows the pattern that we've already "allowed" with the introduction of EVM.CallErrorTemp, and IMO publicizing a field there is less "expensive" than adding a parameter to a shared interface.

consensus/ethash/consensus.go Outdated Show resolved Hide resolved
consensus/ethash/consensus.go Outdated Show resolved Hide resolved
This is an unnecessary condition since each uncle will always get one reward (and they're in a known and constant order), so len(uncleRewards) will always be len(uncles).

Signed-off-by: meows <b5c6@protonmail.com>
Copy link
Contributor

@meowsbits meowsbits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go test ./eth/tracers/... is failing

Eg. https://travis-ci.org/github/etclabscore/core-geth/jobs/741433701#L960

@ziogaschr ziogaschr dismissed meowsbits’s stale review November 5, 2020 15:17

Nice catch @meowsbits. I fixed the existing foundation call_tracer which I broke with the changes with regards gas. p.s.: Haven't checked the Travis failures as I noted them as broken most of the times a while back. Have we fixed them?

@ziogaschr ziogaschr requested a review from meowsbits November 5, 2020 15:40
@meowsbits
Copy link
Contributor

I'm going to merge this, but just noting the following CI suites failed at the same test, which is unusual. I could, however, not
reproduce this locally so I'm assuming this as unrelated issue, though one that we may need to follow up with.

This is a core-geth -specific integration test.

--- FAIL: TestGenerateBlockAndImport_CG1 (0.00s)

    --- FAIL: TestGenerateBlockAndImport_CG1/all-ethash (3.01s)

        worker_cg_test.go:77: timeout: 1

FAIL

@meowsbits meowsbits merged commit e447981 into master Nov 5, 2020
@meowsbits meowsbits deleted the feat/openethereum-trace-apis branch November 5, 2020 20:56
@ziogaschr ziogaschr restored the feat/openethereum-trace-apis branch November 10, 2020 14:29
@ziogaschr ziogaschr deleted the feat/openethereum-trace-apis branch November 10, 2020 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: I am a user from exchange, we want migrate from parity to core-get
4 participants