eth/tracers: add txHash field on txTraceResult

[sjlee1125]

This PR modifies the interface for the results of debug_traceBlock and debug_traceCall by adding the txHash, allowing users to identify which transaction's trace result corresponds to. This was previously discussed in issue #20629, but there has been no further discussion since then.

[holiman]

What @s1na wrote on that ticket was

result() has access to the transaction hash (ctx.txHash) now. I'm guessing that should be enough for the use-case.

Meaning, without putting the hash on the outer result-container, the inner tracer has access to the hash and can put it on the result, if desired. Is that not sufficient?

[holiman]

Does omitEmpty work as intended on the empty-hash?

[sjlee1125]

Thank you for the review. I realized that common.Hash is of type [32]byte, so it cannot have an empty value in a byte array. I will modify that.

[sjlee1125]

fixed it!

[holiman]

In general, LGTM (if we want this feature)

[sjlee1125]

What @s1na wrote on that ticket was

result() has access to the transaction hash (ctx.txHash) now. I'm guessing that should be enough for the use-case.

Meaning, without putting the hash on the outer result-container, the inner tracer has access to the hash and can put it on the result, if desired. Is that not sufficient?

How can the hash be inserted into the inner trace? It doesn't seem possible to retrieve the txHash when making a debug_traceBlock RPC call.

[holiman]

How can the hash be inserted into the inner trace?

So then it would be up to the tracer that is used -- and NewStructLogger is the default:

	// Default tracer is the struct logger
	tracer = logger.NewStructLogger(config.Config)
	if config.Tracer != nil {
		tracer, err = DefaultDirectory.New(*config.Tracer, txctx, config.TracerConfig)
		if err != nil {
			return nil, err
		}
	}

However, the TxHash is available through the txctx, which is passed to the non-default tracer constructor. It is not passed to the NewStructLogger constructor. If it were, it would have been possible for the structlogger to include it into the ExecutionResult:

type ExecutionResult struct {
	Gas         uint64         `json:"gas"`
	Failed      bool           `json:"failed"`
	ReturnValue string         `json:"returnValue"`
	StructLogs  []StructLogRes `json:"structLogs"`
}

As it is, we can either fix it "everywhere" with the way this PR does it, or we can fix it only for the structlogger by modifying the constructor.
I don't have any hard preference at this point. @s1na WDYT?

[s1na]

Tracers are used in multiple contexts:

• traceCall which operates on a set of tx parameters. Here txHash would be nil.
• traceTransaction which takes in a txHash as parameter, hence the user already knows the hash.
• traceBlock and traceChain output the trace for multiple mined (or possibly pending) txes.

Because of this it might make sense to add this info directly to the result of traceBlock or traceChain. However the txHash of every trace is easily inferable via an eth_getBlockByNumber/Hash. Why aren't you using that?

[holiman]

I'm leaning towards accepting this PR as is, based on your comments above.

(However:)

However the txHash of every trace is easily inferable via an eth_getBlockByNumber/Hash.

Do you mean he should obtain the traces, then afterwards load the block body and correlate? Sounds clunky.

[s1na]

Can you please fix the failing tests?

[holiman]

LGTM