This repository has been archived by the owner on Oct 16, 2023. It is now read-only.
Update cw-721 to 0.17.0 #137
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grod220
commented
May 2, 2023
Comment on lines
-50
to
-60
| // Parent::default().instantiate() copied below | ||
| // Cannot use given it overrides contract version | ||
| let info = ContractInfoResponse { | ||
| name: msg.name, | ||
| symbol: msg.symbol, | ||
| }; | ||
| Parent::default().contract_info.save(deps.storage, &info)?; | ||
| let minter = deps.api.addr_validate(&msg.minter)?; | ||
| Parent::default().minter.save(deps.storage, &minter)?; | ||
|
|
||
| Ok(Response::default()) |
There was a problem hiding this comment.
No longer an issue as the base contract doesn't set the contract version anymore
grod220
commented
May 2, 2023
Comment on lines
+18
to
+32
| #[test] | ||
| fn only_token_owner_can_burn() { | ||
| let mut mock = MockEnv::new().build().unwrap(); | ||
|
|
||
| let user = Addr::unchecked("user"); | ||
| let token_id = mock.mint(&user).unwrap(); | ||
| mock.set_health_response(&user, &token_id, &below_max_for_burn()); | ||
|
|
||
| let bad_guy = Addr::unchecked("bad_guy"); | ||
| let res = mock.burn(&bad_guy, &token_id); | ||
| let err: ContractError = res.unwrap_err().downcast().unwrap(); | ||
| assert_eq!(err, BaseError(Ownership(NotOwner))); | ||
|
|
||
| mock.burn(&user, &token_id).unwrap(); | ||
| } |
There was a problem hiding this comment.
Moved from other test file. Better fits here given this is the burn test suite.
f36c46a to
071ac22
Compare
071ac22 to
2a857f2
Compare
larry0x
reviewed
May 8, 2023
There was a problem hiding this comment.
@grod220 I think we need a migrate entry point on the NFT contract?
See the discussion in this PR: public-awesome/cw-nfts#119
213a974 to
fff2c09
Compare
fff2c09 to
4836c8e
Compare
larry0x
approved these changes
May 9, 2023
Comment on lines
+43
to
+44
| cw721 = { git = "https://github.com/CosmWasm/cw-nfts/", branch = "main" } | ||
| cw721-base = { git = "https://github.com/CosmWasm/cw-nfts/", branch = "main", features = ["library"] } |
There was a problem hiding this comment.
We should ask them to make a v0.17.1 release to include the features you need
There was a problem hiding this comment.
True. I assumed they'd cut a new release in the next few weeks or so and we can come back and pin this to that version.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This primarily affects how the minting role is handled. Now there is a separate message to manage Ownership using cw-ownable.