core, eth, internal, cmd: rework EVM constructor #30745
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,18 +74,18 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg | |
| ) | ||
|
|
||
| // Apply pre-execution system calls. | ||
| var tracingStateDB = vm.StateDB(statedb) | ||
| if hooks := cfg.Tracer; hooks != nil { | ||
| tracingStateDB = state.NewHookedState(statedb, hooks) | ||
| } | ||
| context = NewEVMBlockContext(header, p.chain, nil) | ||
| evm := vm.NewEVM(context, tracingStateDB, p.config, cfg) | ||
|
|
||
| if beaconRoot := block.BeaconRoot(); beaconRoot != nil { | ||
| ProcessBeaconBlockRoot(*beaconRoot, evm, tracingStateDB) | ||
| } | ||
| if p.config.IsPrague(block.Number(), block.Time()) { | ||
| ProcessParentBlockHash(block.ParentHash(), evm, tracingStateDB) | ||
| } | ||
|
|
||
| // Iterate over and process the individual transactions | ||
|
|
@@ -96,7 +96,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg | |
| } | ||
| statedb.SetTxContext(tx.Hash(), i) | ||
|
|
||
| receipt, err := ApplyTransactionWithEVM(msg, p.config, gp, statedb, blockNumber, blockHash, tx, usedGas, evm) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not apply tx %d [%v]: %w", i, tx.Hash().Hex(), err) | ||
| } | ||
|
|
@@ -113,10 +113,10 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg | |
| } | ||
| requests = append(requests, depositRequests) | ||
| // EIP-7002 withdrawals | ||
| withdrawalRequests := ProcessWithdrawalQueue(evm, tracingStateDB) | ||
| requests = append(requests, withdrawalRequests) | ||
| // EIP-7251 consolidations | ||
| consolidationRequests := ProcessConsolidationQueue(evm, tracingStateDB) | ||
| requests = append(requests, consolidationRequests) | ||
| } | ||
|
|
||
|
|
@@ -135,9 +135,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg | |
| // and uses the input parameters for its environment similar to ApplyTransaction. However, | ||
| // this method takes an already created EVM instance as input. | ||
| func ApplyTransactionWithEVM(msg *Message, config *params.ChainConfig, gp *GasPool, statedb *state.StateDB, blockNumber *big.Int, blockHash common.Hash, tx *types.Transaction, usedGas *uint64, evm *vm.EVM) (receipt *types.Receipt, err error) { | ||
| if hooks := evm.Config.Tracer; hooks != nil { | ||
| if hooks.OnTxStart != nil { | ||
| hooks.OnTxStart(evm.GetVMContext(), tx, msg.From) | ||
| } | ||
|
|
@@ -148,7 +146,7 @@ func ApplyTransactionWithEVM(msg *Message, config *params.ChainConfig, gp *GasPo | |
|
|
||
| // Create a new context to be used in the EVM environment. | ||
| txContext := NewEVMTxContext(msg) | ||
| evm.SetTxContext(txContext) | ||
holiman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Apply the transaction to the current state (included in the env). | ||
| result, err := ApplyMessage(evm, msg, gp) | ||
|
|
@@ -159,7 +157,7 @@ func ApplyTransactionWithEVM(msg *Message, config *params.ChainConfig, gp *GasPo | |
| // Update the state with pending changes. | ||
| var root []byte | ||
| if config.IsByzantium(blockNumber) { | ||
| evm.StateDB.Finalise(true) | ||
|
There was a problem hiding this comment. I think we should use tracingStateDB here. Finalise is hooked in some cases. There was a problem hiding this comment. The EVM is constructed with 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 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. There was a problem hiding this comment.
Yeah. One way to do it would be, to 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 As it is right now, we pass the statedb struct in order to
and 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 There was a problem hiding this comment. Initially, I did unify them, by expanding the There was a problem hiding this comment. I would like to move the accessEvent into the EVM itself. So we can remove it from the statedb. |
||
| } else { | ||
| root = statedb.IntermediateRoot(config.IsEIP158(blockNumber)).Bytes() | ||
| } | ||
|
|
@@ -210,16 +208,13 @@ func MakeReceipt(evm *vm.EVM, result *ExecutionResult, statedb *state.StateDB, b | |
| // and uses the input parameters for its environment. It returns the receipt | ||
| // for the transaction, gas used and an error if the transaction failed, | ||
| // indicating the block was invalid. | ||
| func ApplyTransaction(config *params.ChainConfig, evm *vm.EVM, gp *GasPool, statedb *state.StateDB, header *types.Header, tx *types.Transaction, usedGas *uint64) (*types.Receipt, error) { | ||
| msg, err := TransactionToMessage(tx, types.MakeSigner(config, header.Number, header.Time), header.BaseFee) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| // Create a new context to be used in the EVM environment | ||
| return ApplyTransactionWithEVM(msg, config, gp, statedb, header.Number, header.Hash(), tx, usedGas, evm) | ||
| } | ||
|
|
||
| // ProcessBeaconBlockRoot applies the EIP-4788 system call to the beacon block root | ||
|
|
@@ -242,7 +237,7 @@ func ProcessBeaconBlockRoot(beaconRoot common.Hash, vmenv *vm.EVM, statedb vm.St | |
| To: ¶ms.BeaconRootsAddress, | ||
| Data: beaconRoot[:], | ||
| } | ||
| vmenv.SetTxContext(NewEVMTxContext(msg)) | ||
| statedb.AddAddressToAccessList(params.BeaconRootsAddress) | ||
| _, _, _ = vmenv.Call(vm.AccountRef(msg.From), *msg.To, msg.Data, 30_000_000, common.U2560) | ||
| statedb.Finalise(true) | ||
|
|
@@ -268,7 +263,7 @@ func ProcessParentBlockHash(prevHash common.Hash, vmenv *vm.EVM, statedb vm.Stat | |
| To: ¶ms.HistoryStorageAddress, | ||
| Data: prevHash.Bytes(), | ||
| } | ||
| vmenv.SetTxContext(NewEVMTxContext(msg)) | ||
| statedb.AddAddressToAccessList(params.HistoryStorageAddress) | ||
| _, _, _ = vmenv.Call(vm.AccountRef(msg.From), *msg.To, msg.Data, 30_000_000, common.U2560) | ||
| statedb.Finalise(true) | ||
|
|
@@ -304,7 +299,7 @@ func processRequestsSystemCall(vmenv *vm.EVM, statedb vm.StateDB, requestType by | |
| GasTipCap: common.Big0, | ||
| To: &addr, | ||
| } | ||
| vmenv.SetTxContext(NewEVMTxContext(msg)) | ||
| statedb.AddAddressToAccessList(addr) | ||
| ret, _, _ := vmenv.Call(vm.AccountRef(msg.From), *msg.To, msg.Data, 30_000_000, common.U2560) | ||
| statedb.Finalise(true) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 upgetTracerFn. I'll take a lookThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yeah it's not quite that simple. Maybe let's leave it for a different PR