Tung/add wasm client wasmvm v2.0.0 #168
Merged
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
|
Important Auto Review SkippedReview was skipped due to path filters Files ignored due to path filters (2)
WalkthroughThis update introduces a multi-stage Docker build for a Golang application, enhancing the project with wasm functionality for improved performance and flexibility. Key changes include the integration of the ibc wasm client, adjustments in keeper initialization, and updates to script commands for starting local nodes, ensuring the application is lightweight and efficient. Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sum
Files selected for processing (4)
- Dockerfile (1 hunks)
- app/app.go (14 hunks)
- app/upgrades.go (2 hunks)
- scripts/start_local_node.sh (1 hunks)
Check Runs (1)
lint completed (5)
- app/app.go: 38-38: SA1019: "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" is deprecated: The legacy v1beta1 gov types maintained in this file are deprecated and will be removed in a future release. Please use MsgIBCSoftwareUpgrade and MsgRecoverClient in favour of the legacy v1beta1 gov proposal types. (staticcheck)
- app/upgrades.go: 10-10: File is not
gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) -s prefix(github.com/CosmWasm/wasmd) --custom-order (gci) - app/upgrades.go: 23-23: File is not
gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) -s prefix(github.com/CosmWasm/wasmd) --custom-order (gci)
Additional comments: 16
Dockerfile (7)
- 1-1: The base image
golang:1.21-alpine3.18is a good choice for building Go applications, providing a balance between size and the necessary build tools. However, ensure that this version aligns with the project's compatibility requirements.- 3-7: Using build arguments
LIBWASM_VERSIONandLIBWASM_CHECKSUMwith subsequent checks ensures that the build process is parameterized and secure. This approach allows for flexibility and verification of the wasmvm library version being used.- 9-9: Adding necessary build dependencies is crucial for the compilation process. However, consider documenting why each package is needed, especially for less obvious ones like
libusb-dev, to improve maintainability.- 20-23: Copying local directories (
internal,testing,modules) and theLICENSEfile before runninggo mod downloadis a good practice to ensure that any local overrides are respected. This step is crucial for builds that depend on local modules not yet available in remote repositories.- 28-30: Setting the working directory to
/go/modules/light-clients/08-wasmbefore runninggo mod downloadis specific to the project structure. Ensure that this path accurately reflects the location of the Go module that needs to be built.- 32-32: The build command is comprehensive, including various build tags and linker flags to optimize the binary. Ensure that all flags and build tags (
netgo,ledger,muslc) are necessary and correctly configured for the project's requirements.- 34-38: Switching to a lightweight
alpine:3.18image for the final stage and copying the built binary is a good practice for optimizing the image size. Ensure that the runtime dependencies of the binary (if any) are satisfied by the Alpine image.scripts/start_local_node.sh (3)
- 5-5: The definition of
EVEDwith the--homeflag is a good practice for specifying the home directory. Ensure that theEVE_HOMEpath is correctly set and that theevedexecutable path is accurate.- 5-5: The script appears to be well-structured and follows good practices for initializing and configuring a local node. However, ensure that the
evedcommand and its flags are up-to-date with the latest version of the software being used.- 5-5: The commented-out section at the end of the script related to starting the daemon, adding a governator, and bringing the daemon back to the foreground suggests that this part of the script is either under development or optional. Consider clarifying its purpose or removing it if it's no longer needed.
app/upgrades.go (1)
- 10-11: The addition of imports for
v2 "github.com/CosmWasm/wasmd/x/wasm/migrations/v2"andwasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"is crucial for supporting WASM migrations and types. Ensure that these imports are used within the file and that their versions are compatible with the rest of the project.app/app.go (5)
- 12-22: The addition of imports related to WASM functionality, including
wasmvm,wasm08,wasm08keeper, andwasm08types, is crucial for integrating wasmvm v2.0.0-rc2. These imports enable the application to utilize the latest WASM light client capabilities and contract handling.- 153-157: The introduction of the
ContractMemoryLimitconstant is a significant change. It sets a memory limit for each contract execution, ensuring that all nodes run with the same limit and thus maintain consistency across the network. This is a critical parameter for managing resource usage in contract execution.- 248-248: The inclusion of a new
Wasm08Keeperentity is a key part of integrating wasmvm v2.0.0-rc2. This keeper is essential for managing the lifecycle and execution of WASM contracts under the new version, ensuring that the application can effectively utilize the enhanced capabilities of wasmvm.- 506-513: Creating a new WASM VM instance with
wasmvm.NewVMis a critical step in integrating wasmvm v2.0.0-rc2. The configuration, including the directory for WASM binaries, capabilities, memory limit, and other parameters, is essential for the proper functioning of WASM contracts within the application. This setup ensures that the application is equipped to handle WASM contracts with the updated wasmvm version.- 528-535: The initialization of
Wasm08KeeperwithNewKeeperWithVMis an important change. It configures the keeper with the newly created WASM VM instance, enabling the application to manage and execute WASM contracts using the latest wasmvm version. This adjustment is crucial for leveraging the enhanced capabilities of wasmvm v2.0.0-rc2.
app/app.go
Outdated
| @@ -31,7 +35,7 @@ | |||
| ibctransferkeeper "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper" | |||
| ibctransfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" | |||
| ibc "github.com/cosmos/ibc-go/v8/modules/core" | |||
| ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" //nolint:staticcheck | |||
| ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" | |||
There was a problem hiding this comment.
The import github.com/cosmos/ibc-go/v8/modules/core/02-client/types is marked as deprecated. It's recommended to use MsgIBCSoftwareUpgrade and MsgRecoverClient in favor of the legacy v1beta1 gov proposal types to ensure future compatibility and leverage the latest IBC functionalities.
- import ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
+ // Consider replacing deprecated types with their updated counterpartsCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" | |
| // Consider replacing deprecated types with their updated counterparts |
Comment on lines
+14
to
+16
| ADD https://github.com/CosmWasm/wasmvm/releases/download/${LIBWASM_VERSION}/libwasmvm_muslc.x86_64.a /lib/libwasmvm_muslc.x86_64.a | ||
| RUN sha256sum /lib/libwasmvm_muslc.x86_64.a | grep ${LIBWASM_CHECKSUM} | ||
| RUN cp /lib/libwasmvm_muslc.x86_64.a /lib/libwasmvm_muslc.a |
There was a problem hiding this comment.
Downloading and verifying the libwasmvm_muslc.x86_64.a library directly from GitHub ensures that the specific version of wasmvm is used. This is a secure practice as it uses checksum verification. However, consider handling potential download failures or checksum mismatches more gracefully.
app/upgrades.go
Outdated
Comment on lines
10
to
11
| v2 "github.com/CosmWasm/wasmd/x/wasm/migrations/v2" | ||
| wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" |
There was a problem hiding this comment.
The file's import order does not follow the Go conventions, which typically group standard library imports separately from third-party packages. Consider reordering the imports to improve readability and maintainability.
import (
"fmt"
"github.com/eve-network/eve/app/upgrades"
upgradetypes "cosmossdk.io/x/upgrade/types"
+ v2 "github.com/CosmWasm/wasmd/x/wasm/migrations/v2"
+ wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
"github.com/cosmos/cosmos-sdk/baseapp"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types"
distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper"
paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| v2 "github.com/CosmWasm/wasmd/x/wasm/migrations/v2" | |
| wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" | |
| import ( | |
| "fmt" | |
| "github.com/eve-network/eve/app/upgrades" | |
| upgradetypes "cosmossdk.io/x/upgrade/types" | |
| v2 "github.com/CosmWasm/wasmd/x/wasm/migrations/v2" | |
| wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" | |
| "github.com/cosmos/cosmos-sdk/baseapp" | |
| authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" | |
| banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" | |
| crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types" | |
| distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" | |
| govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" | |
| govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" | |
| minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" | |
| paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper" | |
| paramstypes "github.com/cosmos/cosmos-sdk/x/params/types" | |
| slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" | |
| stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" | |
| ) |
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.
Closes: #122
This pull request implements the WASM light client with wasmvm v2.0.0-rc2. Close dublicate pull request #153
Summary by CodeRabbit