-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove old provwasm bindings #2119
Conversation
WalkthroughThe recent changes streamline the application's interaction with CosmWasm by removing outdated Changes
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 Configuration File (
|
f9bb3cb
to
80b08a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- app/app.go (4 hunks)
- internal/provwasm/query_plugins.go (3 hunks)
Additional comments not posted (5)
internal/provwasm/query_plugins.go (3)
Line range hint
58-75
:
No changes toStargateQuerier
.The function remains unchanged and appears to handle queries correctly with appropriate error handling. No issues found.
Line range hint
77-94
:
No changes toGrpcQuerier
.The function remains unchanged and appears to handle queries correctly with appropriate error handling. No issues found.
Line range hint
48-56
:
Simplification ofQueryPlugins
function.The removal of the
registry
parameter simplifies the function signature, focusing on standardized querying mechanisms. However, this change might impact any existing functionality that relied on custom queriers. Ensure that this change aligns with the overall architectural goals and does not inadvertently remove necessary functionality.app/app.go (1)
651-651
: Simplification ofWasmKeeper
initialization.The removal of
encoderRegistry
andquerierRegistry
simplifies the instantiation ofWasmKeeper
. This change suggests a shift in managing encoders and queriers, potentially impacting how the application interacts with CosmWasm features. Verify that this aligns with the intended architectural changes and does not affect required functionalities.Verification successful
Simplification of
WasmKeeper
initialization verified.The removal of
encoderRegistry
andquerierRegistry
does not adversely affect the current usage ofWasmKeeper
in the codebase. The initialization and interactions withWasmKeeper
remain consistent with the intended architectural changes. No issues found with the simplification.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing `encoderRegistry` and `querierRegistry` in `WasmKeeper` initialization. # Test: Search for any usage of `WasmKeeper` initialization in the codebase to ensure compatibility. rg --type go -A 5 $'WasmKeeper'Length of output: 5473
CHANGELOG.md (1)
41-43
: Changelog entry looks good.The entry for the removal of old
provwasm
bindings is clear and follows the standard format for documenting breaking changes. It includes a link to the related pull request for additional context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- internal/provwasm/query_plugins.go (3 hunks)
Additional comments not posted (4)
internal/provwasm/query_plugins.go (4)
Line range hint
56-75
:
LGTM! Effective handling of Stargate queries.The function effectively dispatches whitelisted Stargate queries and handles errors. The conversion to JSON is beneficial for client-side processing.
Line range hint
77-95
:
LGTM! Efficient handling of gRPC queries.The function efficiently dispatches whitelisted gRPC queries and manages errors. It ensures only authorized queries are processed.
Line range hint
97-112
:
LGTM! Robust conversion from proto to JSON.The function effectively converts proto messages to JSON, ensuring data integrity and facilitating client-side processing. The use of
protoResponseType.Reset()
is a good practice.
Line range hint
45-54
:
Verify impact of removing theregistry
parameter.The removal of the
registry
parameter and custom plugins simplifies the function. Ensure that this change does not negatively impact other parts of the codebase that might rely on custom query handling.
74d530e
to
684ceab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .changelog/unreleased/client-breaking/2119-remove-deprecated-bindings.md (1 hunks)
- app/app.go (4 hunks)
- internal/provwasm/query_plugins.go (3 hunks)
Files skipped from review due to trivial changes (1)
- .changelog/unreleased/client-breaking/2119-remove-deprecated-bindings.md
Files skipped from review as they are similar to previous changes (1)
- app/app.go
Additional comments not posted (1)
internal/provwasm/query_plugins.go (1)
Line range hint
45-54
:
Simplification ofQueryPlugins
function.The removal of the
registry
parameter simplifies the function signature and aligns with the goal of standardizing query handling. However, this change reduces flexibility by removing support for custom queries. Ensure that all necessary custom queries are accommodated elsewhere or are no longer needed.Verification successful
Simplification of
QueryPlugins
function is verified.The removal of the
registry
parameter and thecustomPlugins
function inQueryPlugins
does not affect any dependencies related to custom query handling. The function is used inapp/app.go
withwasmkeeper.WithQueryPlugins
, and no other dependencies were found. This change aligns with the goal of simplifying and standardizing query handling.
app/app.go
: Usage withwasmkeeper.WithQueryPlugins
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing custom query handling. # Test: Search for the usage of the `QueryPlugins` function. Expect: No dependencies on custom query handling. rg --type go -A 5 $'QueryPlugins'Length of output: 1258
Description
This PR removes the old pre 2.0 ProvWasm custom bindings. These older bindings provided the initial wasm support, however, they were very limited in functionality and mostly are inoperable at this time.
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passesSummary by CodeRabbit
New Features
WasmKeeper
.Bug Fixes
provwasm
bindings affecting user implementations.Refactor
QueryPlugins
function by eliminating theregistry
parameter for a more standardized querying mechanism.