use MessageRunContext struct #2779
Conversation
cla-bot
bot
added
the
s
tsahee
added
the
after-next-version
eljobe
removed
the
after-next-version
| if err := PopulateStylusTargetCache(targetConfig); err != nil { | ||
| return fmt.Errorf("error populating stylus target cache: %w", err) | ||
| } | ||
| s.wasmTargets = targetConfig.WasmTargets() |
There was a problem hiding this comment.
not right now - but I wonder if we can move the targetConfig to a different place.. though we'll still need that Initialize call
|
|
||
| func createNonL1BlockChainWithStackConfig( | ||
| t *testing.T, info *BlockchainTestInfo, dataDir string, chainConfig *params.ChainConfig, arbOSInit *params.ArbOSInit, initMessage *arbostypes.ParsedInitMessage, stackConfig *node.Config, execConfig *gethexec.Config, wasmCacheTag uint32, useFreezer bool, | ||
| t *testing.T, info *BlockchainTestInfo, dataDir string, chainConfig *params.ChainConfig, arbOSInit *params.ArbOSInit, initMessage *arbostypes.ParsedInitMessage, stackConfig *node.Config, execConfig *gethexec.Config, useFreezer bool, |
There was a problem hiding this comment.
This could be important.
By default, tests did not use wasmCache. Now there is no way to turn it off.
We might want/need to add cachetag as a parameter to both ExecutionEngine and NewMessageCommitContext.
It only affects system tests, though, which seem to be passing, and there are very few stylus contracts in tests so I'm certainly not worried about mem usage..
still, somethin remember/watch
There was a problem hiding this comment.
I agree, we discussed it briefly before: #2779 (comment)
but looking into future, any interactions between different tests that change their execution might make the tests unreliable/indeterministic and we should avoid it. The global cache of deserialized stylus modules changes slightly the execution, either we call wasmer deserialization function or we get the result from cache - the exec difference doesn't seem to be huge, and if I am correct it is contained within the deserialization of module, so maybe not that bad.
Currently it seems that all program_test tests have DontParallelise set, so eitherway that shouldn't be an issue currently, only other tests that use stylus programs might be affected (still in limited way of only having shared wasmer deserialization cache)
There was a problem hiding this comment.
In future, we could either support overwriting wasm cache tag to not use the global cache, or maybe we could refactor the cache to not be a global cache and support multiple instances of the cache for test purposes (although I am not sure if that's worth extra complexity)
3 participants
Resolves NIT-2827
Pulls OffchainLabs/go-ethereum#373