Skip to content
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

cosmwasm v2 updates #172

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

mintthemoon
Copy link

I got this a bit farther than #160 (and redid a bit of PFC's work when I should have merged it in 🙈 but there's plenty new), still a few outstanding concerns so I'll keep this as a draft.

Outstanding issues:

  1. All migration and backwards compatibility tests have been removed, the upgrade to v2 introduces incompatibilities with the old contract versions and it isn't clear to me how to resolve this cleanly.
  2. Submessage/reply structure has changed in ways that aren't obvious how to handle, I made some attempt but some are still failing. This is only an issue for cw721-fixed-price.
  3. This is pointing at my fork of cw-plus-plus pending merge of add multi-owner cw-ownable with cosmwasm v2 larry0x/cw-plus-plus#25

Other than the ones mentioned, all tests (that I could find) are passing. I likely won't have bandwidth to look into any of these issues for at least the rest of this week so if anyone wants to take a stab at them feel free.

@mintthemoon
Copy link
Author

Looking at those test runs, couple more updates I missed. Every single instance of Addr::unchecked and String::from or string literals as addresses in messages must be changed or we get those decode errors. Bit of a pain eh?

Comment on lines +28 to +30
# TODO: switch to official https://github.com/larry0x/cw-plus-plus
# requires merge of https://github.com/larry0x/cw-plus-plus/pull/25
cw-ownable = { git = "https://github.com/mintthemoon/cw-plus-plus", branch = "dev", package = "cw-ownable" }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that mentioned PR got merged 👀

Suggested change
# TODO: switch to official https://github.com/larry0x/cw-plus-plus
# requires merge of https://github.com/larry0x/cw-plus-plus/pull/25
cw-ownable = { git = "https://github.com/mintthemoon/cw-plus-plus", branch = "dev", package = "cw-ownable" }
cw-ownable = { version = "^2.1" }

@Buckram123
Copy link

Hey what is the progress on upgrade? We would like to use cw721 that supports cosmwasm v2!

Outstanding issues:
2. Submessage/reply structure has changed in ways that aren't obvious how to handle, I made some attempt but some are still failing. This is only an issue for cw721-fixed-price.

Related to this issue CosmWasm/cosmwasm#2253 and for testing: CosmWasm/cw-multi-test#206

@ash-burnt
Copy link

We are also looking to use cw721 in a cosmwasm v2 environment. Is there anything we can do to help?

@taitruong
Copy link
Collaborator

I'll have a look at it, until then pls resolve conflicts and rebase main branch

@hard-nett
Copy link

@taitruong bumping here again! would be useful to have v2 support so standard nft dependencies can be used within the most recent cosmwasm version 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants