vmConfig check for invalid tracer.

[0xTylerHolmes]

When we create a new blockchain we supply a vmConfig object. If we set debug mode to true without supplying aTracer we have the following nil pointer deref.

go-ethereum/core/state_transition.go

Line 328 in 7ca4f60

st.evm.Config.Tracer.CaptureTxStart(st.initialGas)

If we specify debug without adding a tracer we will get the nil pointer deref here. To mitigate this the constructor for blockchain checks that if the vmConfig has debug mode enabled it also checks to make sure the tracer is non-nil.

[rjl493456442]

I think we can do it in an alternative approach. We can remove the Debug field in vm.Config, but define Tracing() function(or some other better name) instead. If the tracer is configured, then we dump the execution statistics into tracer.

// Config are the configuration options for the Interpreter
type Config struct {
	// Debug                   bool      // Enables debugging
	Tracer                  EVMLogger // Opcode logger
	NoBaseFee               bool      // Forces the EIP-1559 baseFee to 0 (needed for 0 price calls)
	EnablePreimageRecording bool      // Enables recording of SHA3/keccak preimages
	ExtraEips               []int     // Additional EIPS that are to be enabled
}

// Tracing returns an indicator if evm execution tracer is configured.
func (config *Config) Tracing() bool {
	return config.Tracer != nil
}

[holiman]

I was thinking we could just disable Debug and print a warning, but OTOH this is a situation where you probably would rather abort and retry than continue and see a warning.
I'm fine with this. The solution from @rjl493456442 is "not great" IMO, because the Debug check happens in a lot ot hot-spots, where we don't want to incur a method call.

If we switched to use proper constructor-invocations, then it would be easier to solve it by having the Debug flag become set implicitly rather than explicitly.

TLDR; SGTM for now

[s1na]

I'm slightly in favor of dropping Debug as Gary suggested. We can check vmConfig.Tracer != nil to avoid the method invocation overhead.

It seems so far tracer is the only use-case for Debug. Even if it were other cases (e.g. mocking caller as per #26414) they won't be necessarily invoked in the same place as tracing. E.g. see:

https://github.com/ethereum/go-ethereum/pull/26414/files#diff-64508d317d86e7a4a5294d76455a5e74148e26883da9e8b4ffc9bbfa3cc8550eR289-R290

[holiman]

Ok, I'm fine with @rjl493456442 's idea, but then we need to turn it into a local variable in the interpreter loop

[holiman]

I think we can do it in an alternative approach.

Implemented in #27048, PTAL

[holiman]

Closing in favour of #27048