replace MessageRunMode with a struct #373
Conversation
| @@ -213,7 +213,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, | |||
| rejectedTxs = append(rejectedTxs, &rejectedTx{i, errMsg}) | |||
| continue | |||
| } | |||
| msg, err := core.TransactionToMessage(tx, signer, pre.Env.BaseFee, core.MessageCommitMode) | |||
| msg, err := core.TransactionToMessage(tx, signer, pre.Env.BaseFee, core.NewMessageCommitContext(nil)) // TODO: do we need to specify wasm targets here if chainConfig.IsArbitrum == false? | |||
There was a problem hiding this comment.
do we need to specify wasm targets here if chainConfig.IsArbitrum == false?
There was a problem hiding this comment.
per other comments, I think we should allow init with nil to mean localTarget
Also, if stylus isn't active, it should't be a problem to really have nil there
| @@ -1167,19 +1167,19 @@ func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.S | |||
| } | |||
|
|
|||
| // Arbitrum: a tx can schedule another (see retryables) | |||
| result, err = runScheduledTxes(ctx, b, state, header, blockCtx, core.MessageGasEstimationMode, result) | |||
| result, err = runScheduledTxes(ctx, b, state, header, blockCtx, core.NewMessageGasEstimationContext(), result) // TODO: why do we use GasEstimationMode here? | |||
There was a problem hiding this comment.
flagging this TODO question
There was a problem hiding this comment.
@PlasmaPower it seems that here we set it to GasEstimationMode because in the function runScheduledTxes (just below this one) we only run scheduled Txes for gas estimation mode.. but I don't understand that condition itself (currently, we call runScheduledTxes from two places I found, both set runmode argument to GasEstimation so this condition isn't really doing anything)
| wasmTargets []rawdb.WasmTarget | ||
| } | ||
|
|
||
| func NewMessageCommitContext(wasmTargets []rawdb.WasmTarget) *MessageRunContext { |
There was a problem hiding this comment.
we always need loclaTarget.
Maybe make the argument extraWasmTargets, and then the function adds local if not already in there, and in most invocations you can just pass nil?
There was a problem hiding this comment.
another option is to special case len(wasmTargets) == 0 and in that case add localTarget
| @@ -213,7 +213,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, | |||
| rejectedTxs = append(rejectedTxs, &rejectedTx{i, errMsg}) | |||
| continue | |||
| } | |||
| msg, err := core.TransactionToMessage(tx, signer, pre.Env.BaseFee, core.MessageCommitMode) | |||
| msg, err := core.TransactionToMessage(tx, signer, pre.Env.BaseFee, core.NewMessageCommitContext(nil)) // TODO: do we need to specify wasm targets here if chainConfig.IsArbitrum == false? | |||
There was a problem hiding this comment.
per other comments, I think we should allow init with nil to mean localTarget
Also, if stylus isn't active, it should't be a problem to really have nil there
This PR:
MessageRunModeenum withMessageRunContextstruct, that apart from run mode now contains wasm cache tag and wasm targets listcachingDBandethdb.DatabaseResolves NIT-2827