core, eth, internal, cmd: rework EVM constructor

[rjl493456442]

This pull request refactors the EVM constructor by removing the TxContext parameter.

The EVM object is frequently overused. Ideally, only a single EVM instance should be created and reused throughout the entire state transition of a block, with the transaction context switched as needed by calling evm.SetTxContext.

Unfortunately, in some parts of the code, the EVM object is repeatedly created, resulting in unnecessary complexity. This pull request is the first step towards gradually improving and simplifying this setup.

[holiman]

Very good initiative!

[holiman]

So, the tracer can be set earlier, on the vmconfig before it is passed to the evm. The thing that changes between transactions is the traceOutput. So we would need to split up getTracerFn. I'll take a look

[holiman]

Hm, yeah it's not quite that simple. Maybe let's leave it for a different PR

[holiman]

Looks good to me! Did we run it on benches anything?

[rjl493456442]

Not yet! Is 7 or 8 available? Thanks and Best regards Gary rong Martin HS ***@***.***>于2024年11月18日 周一下午5:56写道：

…

***@***.**** approved this pull request. Looks good to me! Did we run it on benches anything? — Reply to this email directly, view it on GitHub <#30745 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNO6OKSZMLJBJBGDRCHIGT2BG2UPAVCNFSM6AAAAABRTL53XKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINBSGA2TAOJRG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[holiman]

Not yet! Is 7 or 8 available?

I think so -- I'm not using them, at least

[rjl493456442]

It's deployed on 7/8. I will try to test the tracing functionalities tomorrow.

[s1na]

I think we should use tracingStateDB here. Finalise is hooked in some cases.

[rjl493456442]

The EVM is constructed with tracingStateDB.

But I agree it's a bit confusing here for have two stateDB instances, (a) the one in the EVM object (b) the supplied one for calling IntermediateRoot and making receipts.

The next step is to unify these two instances into a single one. The ideal status is we always use the evm.StateDB for interacting throughout the entire state transition.

[holiman]

The next step is to unify these two instances into a single one. The ideal status is we always use the evm.StateDB for interacting throughout the entire state transition.

Yeah. One way to do it would be, to instead of IntermediateState/Finalize, have func TxEnd(isEip158, isByzantium bool) common.Hash and BlockEnd(isEip158 bool) common.Hash. And instead of

			var root []byte
			if chainConfig.IsByzantium(vmContext.BlockNumber) {
				statedb.Finalise(true)
			} else {
				root = statedb.IntermediateRoot(chainConfig.IsEIP158(vmContext.BlockNumber)).Bytes()
			}

we would do

			isEip158 := chainConfig.IsEIP158(vmContext.BlockNumber)
			isByzantium := chainConfig.IsByzantium(vmContext.BlockNumber)
			root := statedb.TxEnd(isEip158, isByzantium)

---

As it is right now, we pass the statedb struct in order to

1. Calculate root, via IntermediateRoot which is not exposed in vm.StateDB interface,
2. To pass to receipt-building, which invokes the following methods on it

	if statedb.GetTrie().IsVerkle() {
		statedb.AccessEvents().Merge(evm.AccessEvents)
	}

and
statedb.GetLogs(tx.Hash(), blockNumber.Uint64(), blockHash) and statedb.TxIndex()

So, in order to unify them into one, we need to do something clever about the receipt-building too. They way this PR works now is ok to me, as Gary pointed out -- the evm already uses the hookedstate, so Finalise does indeed operate on the hooked one.

[holiman]

Initially, I did unify them, by expanding the vm.StateDB interface quite a lot. It's not a great solution to do that, so @fjl instead suggested we do the conversion lower down. So now we have a bit of a compromise.

[rjl493456442]

if statedb.GetTrie().IsVerkle() {
		statedb.AccessEvents().Merge(evm.AccessEvents)
	}

I would like to move the accessEvent into the EVM itself. So we can remove it from the statedb.