Skip to content

Conversation

@patrick-ogrady
Copy link
Contributor

@patrick-ogrady patrick-ogrady commented Feb 6, 2021

Related: ava-labs/coreth#98

Problem

When invoking debug_traceTransaction with the default Blockscout tracing script, I noticed that the Tracer panics if called on a transaction that created a contract with no code. There is an example of such a transaction on the Avalanche testnet here.

Root Cause

The root cause of this issue is that the Tracer.dbWrapper.db is not populated when Tracer.GetResult is called at the end of traceTx. Specifically, note that GetResult calls result(ctx, db) and that the attached script attempts to access the code of any created contract from the provided db (which panics here as no dbWrapper.db was ever set).

@patrick-ogrady
Copy link
Contributor Author

patrick-ogrady commented Feb 6, 2021

I'm not sure if this is the best approach to fix this panic (I thought it was the least invasive) but believe the root cause is accurate.

@holiman
Copy link
Contributor

holiman commented Feb 9, 2021

I think this is 'semantically correct' -- it basically considers contract code to be an infinite fields of zeroes, and if no code is deployed, it just executes those, ending the execution on the first STOP.

However, in practice, there are problems with this. It will cause problems with differential fuzzing against other vms, and it will also add a performance penalty allocating stack and memory for plain value transfers to EOAs.

@karalabe
Copy link
Member

karalabe commented Feb 9, 2021

Could you perhaps make a similar transaction and post it here so we could play with it on Rinkeby or Goerli and repro?

@patrick-ogrady
Copy link
Contributor Author

I think this is 'semantically correct' -- it basically considers contract code to be an infinite fields of zeroes, and if no code is deployed, it just executes those, ending the execution on the first STOP.

However, in practice, there are problems with this. It will cause problems with differential fuzzing against other vms, and it will also add a performance penalty allocating stack and memory for plain value transfers to EOAs.

If the contract code was empty, I considered just setting Tracer.dbWrapper.db to be non-nil. However, I was concerned this approach may introduce some other bug by breaking the pattern established within the rest of this function. Maybe this approach may alleviate these concerns?

Could you perhaps make a similar transaction and post it here so we could play with it on Rinkeby or Goerli and repro?

I mimicked the empty contract creation (with a non-zero send to it) on Goerli: https://goerli.etherscan.io/tx/0x073e2af40916b3199d960814acbc08ec2e241fb7c53ffb7d19785be8a48b12f3

@karalabe
Copy link
Member

Here's crash stack trace and repro:

debug.traceTransaction("0xe8f921953addf0d7fdfa47c000b380cae10e0e072ee87b8e3e8539aacf04d51a", {tracer: '{ step: function() {}, fault: function() {}, result: function(ctx, db){ db.getBalance(ctx.to)} }'})

ERROR[02-16|11:51:09.372] RPC method debug_traceTransaction crashed: runtime error: invalid memory address or nil pointer dereference
goroutine 18418 [running]:
github.com/ethereum/go-ethereum/rpc.(*callback).call.func1(0xc02a2f89e0, 0x16, 0xc050e41d58)
	/work/src/github.com/ethereum/go-ethereum/rpc/service.go:200 +0xba
panic(0x11e1aa0, 0x1c5e540)
	/opt/google/go/src/runtime/panic.go:969 +0x1b9
github.com/ethereum/go-ethereum/eth/tracers.(*dbWrapper).pushObject.func1(0xc026ac24a0, 0x7f672801f940)
	/work/src/github.com/ethereum/go-ethereum/eth/tracers/tracer.go:200 +0xe5
gopkg.in/olebedev/go-duktape%2ev3.goFunctionCall(0x7f672806c260, 0xc010945270)
	/work/pkg/mod/gopkg.in/olebedev/[email protected]/duktape.go:157 +0xb8
gopkg.in/olebedev/go-duktape%2ev3._cgoexpwrap_f81c1a3ad1a4_goFunctionCall(0x7f672806c260, 0x1)
	_cgo_gotypes.go:3633 +0x2b
gopkg.in/olebedev/go-duktape%2ev3._Cfunc_duk_pcall_prop(0x7f672806c260, 0x200000000, 0x0)
	_cgo_gotypes.go:2341 +0x4d
gopkg.in/olebedev/go-duktape%2ev3.(*Context).PcallProp.func1(0xc026ac2480, 0x0, 0x2, 0x7f6728046350)
	/work/pkg/mod/gopkg.in/olebedev/[email protected]/api.go:786 +0x71
gopkg.in/olebedev/go-duktape%2ev3.(*Context).PcallProp(0xc026ac2480, 0x0, 0x2, 0x2)
	/work/pkg/mod/gopkg.in/olebedev/[email protected]/api.go:786 +0x3f
github.com/ethereum/go-ethereum/eth/tracers.(*Tracer).call(0xc00d42a630, 0x1369323, 0x6, 0xc050e40498, 0x2, 0x2, 0x0, 0x0, 0x0, 0x0, ...)
	/work/src/github.com/ethereum/go-ethereum/eth/tracers/tracer.go:516 +0x110
github.com/ethereum/go-ethereum/eth/tracers.(*Tracer).GetResult(0xc00d42a630, 0x15a46a0, 0xc01093d5c0, 0xc010945158, 0xc0398beae0, 0x0)
	/work/src/github.com/ethereum/go-ethereum/eth/tracers/tracer.go:656 +0x458
github.com/ethereum/go-ethereum/eth/tracers.(*API).traceTx(0xc000877d70, 0x1598820, 0xc041d68b40, 0x15a46a0, 0xc01093d5c0, 0x1445078, 0x1445088, 0xc0398be8a0, 0x365a9b8b56000000, 0xd84ee7427d76aa5e, ...)
	/work/src/github.com/ethereum/go-ethereum/eth/tracers/api.go:779 +0xa45
github.com/ethereum/go-ethereum/eth/tracers.(*API).TraceTransaction(0xc000877d70, 0x1598820, 0xc041d68b40, 0xd7f0dd3a9521f9e8, 0xca80b300c047fafd, 0x8e7be82e070e0ee1, 0x1ad504cfaa39853e, 0xc02ab9a060, 0x0, 0x0, ...)
	/work/src/github.com/ethereum/go-ethereum/eth/tracers/api.go:681 +0x2ed
reflect.Value.call(0xc004ded680, 0xc0005477c8, 0x13, 0x1366566, 0x4, 0xc01093d4a0, 0x4, 0x4, 0xc01093d4d0, 0xc0398be630, ...)
	/opt/google/go/src/reflect/value.go:476 +0x8c7
reflect.Value.Call(0xc004ded680, 0xc0005477c8, 0x13, 0xc01093d4a0, 0x4, 0x4, 0x16, 0x10, 0x0)
	/opt/google/go/src/reflect/value.go:337 +0xb9
github.com/ethereum/go-ethereum/rpc.(*callback).call(0xc004f02b40, 0x1598820, 0xc041d68b40, 0xc02a2f89e0, 0x16, 0xc0398be630, 0x2, 0x2, 0x0, 0x0, ...)
	/work/src/github.com/ethereum/go-ethereum/rpc/service.go:206 +0x2c5
github.com/ethereum/go-ethereum/rpc.(*handler).runMethod(0xc0002a6bd0, 0x1598820, 0xc041d68b40, 0xc03b6f5500, 0xc004f02b40, 0xc0398be630, 0x2, 0x2, 0x2)
	/work/src/github.com/ethereum/go-ethereum/rpc/handler.go:389 +0x8a
github.com/ethereum/go-ethereum/rpc.(*handler).handleCall(0xc0002a6bd0, 0xc0398be5d0, 0xc03b6f5500, 0x20300e)
	/work/src/github.com/ethereum/go-ethereum/rpc/handler.go:337 +0x294
github.com/ethereum/go-ethereum/rpc.(*handler).handleCallMsg(0xc0002a6bd0, 0xc0398be5d0, 0xc03b6f5500, 0x1587901)
	/work/src/github.com/ethereum/go-ethereum/rpc/handler.go:298 +0x1be
github.com/ethereum/go-ethereum/rpc.(*handler).handleMsg.func1(0xc0398be5d0)
	/work/src/github.com/ethereum/go-ethereum/rpc/handler.go:139 +0x46
github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc.func1(0xc0002a6bd0, 0xc02ab9a000)
	/work/src/github.com/ethereum/go-ethereum/rpc/handler.go:226 +0xd2
created by github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc
	/work/src/github.com/ethereum/go-ethereum/rpc/handler.go:222 +0x66
 
WARN [02-16|11:51:09.372] Served debug_traceTransaction            reqid=25 t=15.082766ms err="method handler crashed"

@karalabe
Copy link
Member

So, the fix here is not to do an extra CaptureState, rather to move the database initialization into CaptureStart. It will need a bit of interface refactoring throughout the code, but should be a lot cleaner and not entail any performance hits.

@holiman
Copy link
Contributor

holiman commented Mar 22, 2021

Closing in favou of #22333

@holiman holiman closed this Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants