-
Notifications
You must be signed in to change notification settings - Fork 607
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
refactor(wasmbindings): remove gamm and twap bindings #3766
Conversation
@@ -369,7 +369,6 @@ func (appKeepers *AppKeepers) InitNormalKeepers( | |||
// if we want to allow any custom callbacks | |||
supportedFeatures := "iterator,staking,stargate,osmosis,cosmwasm_1_1" | |||
|
|||
wasmOpts = append(owasm.RegisterCustomPlugins(appKeepers.GAMMKeeper, appKeepers.BankKeeper, appKeepers.TwapKeeper, appKeepers.TokenFactoryKeeper), wasmOpts...) |
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.
bank definitely still needs to be there. TokenFactory is moving to general bindings as well, so I think we should keep that for now.
ACK for gamm + twap
fefdd83
to
2d8ff60
Compare
2d8ff60
to
7096ecf
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.
LGTM
Planning to merge. No one has gotten back to me about using these on Telegram or Discord |
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.
ACK with the understanding that integrators will be without wasmbindings for swap messages if we do not implement the swaprouter wasmbindings by the next release
The only concern raised by the integrators is having the following queries whitelisted: osmosis/wasmbinding/stargate_whitelist.go Lines 87 to 88 in a5bc607
As a result, this should be good to be merged |
Closes: #XXX
What is the purpose of the change
Removing bindings gamm and twap. These have been causing additional overhead during refactors while not having production use cases.
In production, everyone should be using
osmosis-std
instead.Bank and tokenfactory bindings have use cases outside of Osmosis so these are kept for now.
An announcement about removal is to be made post-merge.
Testing and Verifying
This change is a trivial rework / code cleanup without any test coverage.
Documentation and Release Note
Unreleased
section inCHANGELOG.md
? yes