Fix usage of Wasmer Module when recompiling Wasm (regression bug 1.3.x -> 1.4.0) #1907
Conversation
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
chipshort
reviewed
Oct 5, 2023
webmaster128
force-pushed
the
fix-module-usage
branch
2 times, most recently
from
f651771
to
ed6f9d4
Compare
webmaster128
changed the title
Before this fix, the compiling engine was attached to the Module instance (indirectly via the artifact). However, the Store used was created from a different engine.
webmaster128
force-pushed
the
fix-module-usage
branch
from
5ed1c73
to
7774c74
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The upgrade CosmWasm 1.3 -> 1.4.0 contains a big upgrade of Wasmer from 2.3 to version 4. This led to large refactorings in our usage of the Wasmer API. To a large degree this improved the integration. E.g. cached modules in memory are now much smaller because we don't need to cache a Store anymore. However, we made one mistake in some code paths when using Wasmer: When we recompile a module from the Wasm file to the node's architecture we use an engine with a singlepass compiler attached. But for running modules we use a different engine that does not have a compiler. This is nice as the engine running contracts is lightweight and reusable. Now the regression was introduced by creating a Wasmer
Moduleinstance with one engine but run it with a different engine. The type system and Wasmer API allows you to do that, but it seems to be an invalid usage of Wasmer due to how Modules, Artifacts and Engines work internally. The resulting error message we get is:This PR fixes the issue by ensuring the compiling engine and the Module instance are not used to instantiate and execute the Module. Instead the Module is serialized to the file system cache and read from there.
We also increase our test coverage for the affected cases. Before we just checked the instance was created. Now we also test that the created instances can be executed.