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

add multi-owner cw-ownable with cosmwasm v2 #25

Merged
merged 7 commits into from
Aug 14, 2024

Conversation

mintthemoon
Copy link
Contributor

Includes work by @taitruong from #21

Unblocks public-awesome/cw-nfts#160 and indirectly DA0-DA0/dao-contracts#840

taitruong and others added 5 commits August 12, 2024 13:30
…singleton. But can be re-use e.g. by defining `CREATOR: OwnershipStore = OwnershipStore::new("creator")`
* move all logic to `OwnershipStore`. This way `cw-ownable` is still a singleton. But can be re-use e.g. by defining `CREATOR: OwnershipStore = OwnershipStore::new("creator")`

* cargo fmt

* cargo clippy --tests

* use api and storage (instead of DepsMut) for being less restrictive

* fix ownable

* remove unneccessary lifetime (clippy warning)

---------

Co-authored-by: mr-t <[email protected]>
Copy link
Collaborator

@CyberHoward CyberHoward left a comment

Choose a reason for hiding this comment

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

I think it makes sense to expose the OwnershipStore object but we should aim to keep the remaining API the same, following the cw2 ways of doing things.

packages/ownable/src/lib.rs Outdated Show resolved Hide resolved
packages/ownable/src/lib.rs Outdated Show resolved Hide resolved
packages/ownable/src/lib.rs Show resolved Hide resolved
@mintthemoon
Copy link
Contributor Author

I think it makes sense to expose the OwnershipStore object but we should aim to keep the remaining API the same, following the cw2 ways of doing things.

That all makes sense to me. I'm not too familiar with the context and overall architecture of this whole thing; my main goal here is to enable the cw-nfts cosmwasm v2 upgrade (public-awesome/cw-nfts#172) and I don't expect any of those edits would be an issue for it, maybe some minor changes.

mintthemoon and others added 2 commits August 13, 2024 17:03
* update ownable exports and revert API breaking change

* fully revert update_ownership breaking API change and fix relevant tests
@CyberHoward CyberHoward merged commit 4620865 into larry0x:main Aug 14, 2024
@CyberHoward
Copy link
Collaborator

Thanks for contributing! I exported the Item's key so you can still construct the OWNER struct yourself if you prefer that.

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.

3 participants