core: switch EVM tx context in ApplyMessage #30809
Conversation
rjl493456442
requested review from
lightclient,
s1na,
karalabe,
holiman and
MariusVanDerWijden
as code owners
| @@ -605,7 +605,7 @@ func (evm *EVM) GetVMContext() *tracing.VMContext { | |||
| BlockNumber: evm.Context.BlockNumber, | |||
| Time: evm.Context.Time, | |||
| Random: evm.Context.Random, | |||
| GasPrice: evm.TxContext.GasPrice, | |||
There was a problem hiding this comment.
@s1na it might be an incompatible change, but I feel like it's the right direction.
The effectiveGasPrice could be derived with BaseFee (block context) and tx gas price.
Originally, we must to invoke evm.SetTxContext before the TxStartHook, with this change, the requirement is gone
There was a problem hiding this comment.
Hmm.. Why not just move the tracer stuff a bit? We ought to be able to get the tx-specific stuff into OnTxStart?
There was a problem hiding this comment.
Wait a minute. The TxStart hook:
TxStartHook = func(vm *VMContext, tx *types.Transaction, from common.Address)
It has the tx. The tx has a GasPrice method. Why would we have it also in the VMContext? Gary's change makes total sense
There was a problem hiding this comment.
For info, in our case, we indeed take the gasPrice from the transaction and not from the *VMContext struct in our tracer.
There was a problem hiding this comment.
@maoueh Just want to mention that the GasPrice taken from the transaction object is the different with one in the *tracing.VMContext, the first one refers to the full gas price while the latter one refers to the effectiveGasPrice
There was a problem hiding this comment.
@rjl493456442 Yes thanks for the info. We are handling the different tx types to pick the correct gas values.
There was a problem hiding this comment.
I think we should bring it back in another tracing API version. The tracer implementation should not have to compute the effective gas price by itself. It should be handled by the tracing infrastructure.
|
@s1na May I get your approval? |
fjl
changed the title
This pull request relocates the EVM tx context switching to the ApplyMessage function.
With this change, we can remove a tons of EVM.SetTxContext call before the message
execution for simplification.
Tracing API changes
GasPricefield of theVMContextstruct withBaseFee. Users may instead take the effective gas price fromtx.EffectiveGasTipValue(env.BaseFee).