-
Notifications
You must be signed in to change notification settings - Fork 409
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
Upgrade to cosmos-sdk v0.45.0 #717
Conversation
Codecov Report
@@ Coverage Diff @@
## master #717 +/- ##
==========================================
+ Coverage 59.21% 59.99% +0.77%
==========================================
Files 48 48
Lines 5451 5592 +141
==========================================
+ Hits 3228 3355 +127
- Misses 1992 1995 +3
- Partials 231 242 +11
|
Thank you @alpe for this PR. Re the follow ups. This is a very important question before merging, or at least before tagging a release.
-> This is an important question we should work out in the design docs and make a decision there before implementing. This has been heavily discussed in #697 and changes were made to the wasmvm interface to expose all needed info to the runtime, but we need to document the behavior. These ones would be nice to have by 1.0 but require extending the CosmWasm interfaces in a backwards compatible way:
-> I added a tracking issue for this in CosmWasm CosmWasm/cosmwasm#1200 Please add more thoughts/comments on these to that issue |
@faddat @iramiller @ValarDragon @RiccardoM @litvintech I would love any comments here as you all have been working on 0.44 forks. |
Hi! a moment to collect thoughts :)
Should we add support for "authz" to cosmwasm? Should we integrate IBC version negotiation, Support IBC version negotiation in contracts wasmvm#269? Should we pass the IBC relayer address to the contract? Should we do the IBC receive package (n)ack error cases differently? |
All of those require extensions to the CosmWasm interface. |
Upgrade to SDK v0.45.0-rc1
This will be needed if we want to provide bonuses / fees / incentives to relayers in the future. It should not be used for auth |
This would mean that you could say provide partial account access to a contract (like let it stake on your behalf). Happy if you have more practical ideas here. But will take this into account as a wished feature, just need to clarify it |
Unless anyone has concerns, I feel this is good to merge |
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.
good stuff
go.mod
Outdated
github.com/confio/ics23/go => github.com/confio/ics23/go v0.6.6 | ||
github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.42.11 | ||
github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.45.0-rc1 |
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.
github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.45.0-rc1 | |
github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.45.0 |
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.
@alpe why is this even here?
github.com/cosmos/cosmos-sdk v0.45.0
is set above and we don't change repos.
I would remove it
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.
I think it's because of ibc-go which may be bringing in a different version of cosmos-sdk
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.
Turns out they were added here: https://github.com/CosmWasm/wasmd/pull/692/files
With a recent comment they are not a good idea: #692 (comment)
I fixed them with your hint, but maybe they need to be removed as well....
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.
ja, a transitive dependency. I did not look where it came from but just wanted to ensure we have the right sdk version set.
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.
good point
go.mod
Outdated
github.com/confio/ics23/go => github.com/confio/ics23/go v0.6.6 | ||
github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.42.11 | ||
github.com/cosmos/cosmos-sdk => github.com/cosmos/cosmos-sdk v0.45.0-rc1 |
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.
@alpe why is this even here?
github.com/cosmos/cosmos-sdk v0.45.0
is set above and we don't change repos.
I would remove it
lgtm 🎉 |
Resolves #501
This upgrade includes
Follow ups: