-
Notifications
You must be signed in to change notification settings - Fork 102
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
feat: QOL GetDefaultFeatures #413
Conversation
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.
There is no such thing as default capabilities in wasmvm. The chain using wasmvm need to support this the features. If I added fobar here but the chain does not support it, an wasmvm upgrade would break things.
If we add it, it should be done in wasmd. Wasmd already maintains such a list.
const ( | ||
TESTING_FEATURES = "staking,stargate,iterator,cosmwasm_1_1,cosmwasm_1_2" |
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.
We need to rename this variable to capabilities (see https://github.com/CosmWasm/cosmwasm/blob/main/docs/CAPABILITIES.md for more on that)
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.
Hmm, it's here but only as an example, not a default value: https://github.com/CosmWasm/wasmd/blob/v0.31.0/app/app.go#L517. There should be a default which you can expand. |
my idea here is that each wasmvm version as upgraded, would then add the cosmwasm_1_# to this. Then wasmd would call this like you mentioned with https://github.com/CosmWasm/wasmd/blob/v0.31.0/app/app.go#L517 (Since cosmwasm_1_2 is in reference to wasmvm, I really feel it makes sense to add here?) Then in wasmd you would do
yay / nay? |
nay, capabilities are always negotiated between chain and contract |
Feature
It would be nice to have
wasmvm.GetDefaultFeatures()
to return the default features. This is also used in the internal test.Reason
When upgrading, its common to forget this ( We did for juno v14). It would be nice for this to be upstream to ensure its handled when chains upgrade their wasmvm automatically for future 1.3+ wasmvm new features
Before & After
app.go