core: cmd: invert disableMemory #23558

MariusVanDerWijden · 2021-09-09T14:37:24Z

Both ReturnData and Memory are pretty big and should not be part of traces by default.
This inverts the behavior of DisableMemory and DisableReturnData.

rjl493456442 · 2021-09-10T06:50:42Z

cmd/evm/internal/t8ntool/flags.go

-	TraceDisableMemoryFlag = cli.BoolFlag{
-		Name:  "trace.nomemory",
-		Usage: "Disable full memory dump in traces",
+	TraceEnableMemoryFlag = cli.BoolFlag{


You can preserve this flag TraceDisableMemoryFlag but change the type as cli.TBoolFlag. So that the default value is True. I think we should keep the compatibility if possible.

I agree with this for all flags.

(but please keep the existing change to LogConfig)

rjl493456442 · 2021-09-10T06:52:08Z

Any particular reason to only change the ReturnData and Memory? What about Stack and Storage?
If they are not part of the standard tracing output, should we also disable them by default?

holiman · 2021-09-10T07:22:12Z

Stack is nearly always used/necessary. Storage though... I doesn't blow up output like the others, but do slow down execution. It's an expensive op, though, so it's not as bad as memory - which can easily be a mb per line

tests/state_test.go

holiman · 2021-09-10T11:24:44Z

cmd/evm/internal/t8ntool/flags.go

-	TraceDisableMemoryFlag = cli.BoolFlag{
-		Name:  "trace.nomemory",
-		Usage: "Disable full memory dump in traces",
+	TraceEnableMemoryFlag = cli.BoolFlag{


I agree with this for all flags.

(but please keep the existing change to LogConfig)

MariusVanDerWijden · 2021-09-13T13:48:43Z

Alright, preserving the flags now

holiman

LGTM (two minor nits)

core/vm/logger.go

Co-authored-by: Martin Holst Swende <martin@swende.se>

* core: cmd: invert disableMemory * core: fix missed inversion * cmd/evm: preserve Flags but change default value * Apply suggestions from code review Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Martin Holst Swende <martin@swende.se>

implementing PR ethereum#23558 from ethereum/go-ethereum

* eth/tracers: implement debug.intermediateRoots (ethereum#23594) This PR implements a new debug method, which I've talked briefly about to some other client developers. It allows the caller to obtain the intermediate state roots for a block (which might be either a canon block or a 'bad' block). Signed-off-by: wenbiao <delweng@gmail.com> * core, rpc: disable memory output by default in traces (ethereum#23558) * core: cmd: invert disableMemory * core: fix missed inversion * cmd/evm: preserve Flags but change default value * Apply suggestions from code review Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Martin Holst Swende <martin@swende.se> Signed-off-by: wenbiao <delweng@gmail.com> * eth/tracers: abort evm execution when trace is aborted (ethereum#23580) Signed-off-by: wenbiao <delweng@gmail.com> * eth/tracers: avoid unsyncronized mutations on trie database (ethereum#23632) This PR fixes an issue in traceChain, where the statedb Commit operation was performed asynchronously with dereference-operations agains the underlying trie.Database instance. Due to how the reference counting works within the trie database (where parent count is recursively updated when new parents are added), doing dereferencing in the middle of Commit can cause the refcount to become wrong, leading to an inconsistent state. This was fixed by doing Commit/Deref from the same routine. Signed-off-by: wenbiao <delweng@gmail.com> * core,eth: call frame tracing (ethereum#23087) This change introduces 2 new optional methods; `enter()` and `exit()` for js tracers, and makes `step()` optiona. The two new methods are invoked when entering and exiting a call frame (but not invoked for the outermost scope, which has it's own methods). Currently these are the data fields passed to each of them: enter: type (opcode), from, to, input, gas, value exit: output, gasUsed, error The PR also comes with a re-write of the callTracer. As a backup we keep the previous tracing script under the name `callTracerLegacy`. Behaviour of both tracers are equivalent for the most part, although there are some small differences (improvements), where the new tracer is more correct / has more information. Signed-off-by: wenbiao <delweng@gmail.com> * eth/tracers: re-write of 4byte tracer using enter/exit (ethereum#23622) * eth/tracers: add re-write of 4byte tracer using enter/exit * eth/tracers: fix 4byte indent Signed-off-by: wenbiao <delweng@gmail.com> * eth/tracers: tx.BaseFee not implemented Signed-off-by: wenbiao <delweng@gmail.com> * eth/tracers: do the JSON serialization via .js to capture C faults Signed-off-by: wenbiao <delweng@gmail.com> * eth/tracers: fix callTracer fault handling (ethereum#23667) * eth/tracers: fix calltracer fault handling * eth/tracers: fix calltracer indentation Signed-off-by: wenbiao <delweng@gmail.com> * eth/tracers: invoke enter/exit on 0-value calls to inex accounts (ethereum#23828) Signed-off-by: wenbiao <delweng@gmail.com> * eth: make traceChain avoid OOM on long-running tracing (ethereum#23736) 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: wenbiao <delweng@gmail.com> * eth/tracers: expose contextual infos (block hash, tx hash, tx index) Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: redefine Context Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: support for golang tracers + add golang callTracer (ethereum#23708) * 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 01bb908. * eth/tracers: linter nits * eth/tracers: fix output on err Co-authored-by: Martin Holst Swende <martin@swende.se> Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: make native calltracer default (ethereum#23867) Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: package restructuring (ethereum#23857) * eth/tracers: restructure tracer package * core/vm/runtime: load js tracers * eth/tracers: mv bigint js code to own file * eth/tracers: add method docs for native tracers * eth/tracers: minor doc fix * core,eth: cancel evm on nativecalltracer stop * core/vm: fix failing test Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com> Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: ethapi.TransactionArgs was not merged Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: fix the api_test with ErrInsufficientFunds to ErrInsufficientFundsForTransfer Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: check posa before statedb.Prepare in IntermiateRoots api Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: make js calltracer default, compatible with old version Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: fix the default callTrace name of callTracerJs Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * Revert "eth/tracers: fix the default callTrace name of callTracerJs" This reverts commit 62a3bc215d9f07e422a4c659289bb3ba4f9ed2fa. Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * Revert "eth/tracers: make js calltracer default, compatible with old version" This reverts commit 85ef42c0ea651f0b228d4209b1b2598b24e12f1f. Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> * eth/tracers: fix the variable race condition Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io> Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de> Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com> Co-authored-by: Péter Szilágyi <peterke@gmail.com> Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>

newtoncx · 2022-03-02T09:31:08Z

There are some docs which need update here. For example:
https://geth.ethereum.org/docs/rpc/ns-debug#debug_tracetransaction
I was quite confused why I wasn't seeing memory contents in the traces. Appreciated!

rjl493456442 · 2022-03-03T08:16:00Z

@newtoncx thanks for catching it! I have opened an issue for it.

* core: cmd: invert disableMemory * core: fix missed inversion * cmd/evm: preserve Flags but change default value * Apply suggestions from code review Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Martin Holst Swende <martin@swende.se>

core: cmd: invert disableMemory

b0368fa

MariusVanDerWijden requested review from holiman, karalabe and rjl493456442 as code owners September 9, 2021 14:37

core: fix missed inversion

51c772f

rjl493456442 reviewed Sep 10, 2021

View reviewed changes

holiman reviewed Sep 10, 2021

View reviewed changes

cmd/evm: preserve Flags but change default value

ccd003d

holiman approved these changes Sep 13, 2021

View reviewed changes

core/vm/logger.go Outdated Show resolved Hide resolved

core/vm/logger.go Outdated Show resolved Hide resolved

Apply suggestions from code review

e540700

Co-authored-by: Martin Holst Swende <martin@swende.se>

holiman merged commit b8d7c66 into ethereum:master Sep 13, 2021

holiman added this to the 1.10.9 milestone Sep 13, 2021

rjl493456442 mentioned this pull request Sep 23, 2021

Untraceable transaction using debug_traceTransaction #23552

Closed

This was referenced Sep 23, 2021

Cannot run trace.block() on block 2396379 on archival node etclabscore/core-geth#412

Open

implementing PR #23558 from ethereum/go-ethereum etclabscore/core-geth#416

Merged

MariusVanDerWijden deleted the enable-memory branch November 30, 2021 15:29

haltman-at mentioned this pull request Dec 22, 2021

Traces as of 1.10.9 do not include memory (or storage as of 1.0.5) #24146

Closed

AusIV pushed a commit to NoteGio/go-ethereum that referenced this pull request Jan 18, 2022

implementing PR ethereum#23558 from ethereum/go-ethereum

578065e

AusIV pushed a commit to NoteGio/go-ethereum that referenced this pull request Jan 18, 2022

Merge pull request ethereum#416 from afmsavage/cmd-disable-memoryReturn

84b3b37

implementing PR ethereum#23558 from ethereum/go-ethereum

rjl493456442 mentioned this pull request Mar 3, 2022

doc: the default tracing behavior in documentation should be updated. #24497

Closed

fvictorio mentioned this pull request Jun 5, 2023

debug_traceTransaction's disableMemory flag is different from geth NomicFoundation/hardhat#3972

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: cmd: invert disableMemory #23558

core: cmd: invert disableMemory #23558

MariusVanDerWijden commented Sep 9, 2021

rjl493456442 Sep 10, 2021

holiman Sep 10, 2021

rjl493456442 commented Sep 10, 2021

holiman commented Sep 10, 2021 via email

holiman Sep 10, 2021

MariusVanDerWijden commented Sep 13, 2021

holiman left a comment

newtoncx commented Mar 2, 2022

rjl493456442 commented Mar 3, 2022

core: cmd: invert disableMemory #23558

core: cmd: invert disableMemory #23558

Conversation

MariusVanDerWijden commented Sep 9, 2021

rjl493456442 Sep 10, 2021

Choose a reason for hiding this comment

holiman Sep 10, 2021

Choose a reason for hiding this comment

rjl493456442 commented Sep 10, 2021

holiman commented Sep 10, 2021 via email

holiman Sep 10, 2021

Choose a reason for hiding this comment

MariusVanDerWijden commented Sep 13, 2021

holiman left a comment

Choose a reason for hiding this comment

newtoncx commented Mar 2, 2022

rjl493456442 commented Mar 3, 2022