Use the bank module to keep track of scope value owners #2140
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
…ed anywhere and anything trying to use the metadata keeper should define their own interface.
…eded when a scope is deleted.
…DOs for re-implementation.
…used when needed.
…eValueOwner endpoints to make it easier to change the stuff called in there.
… slice with nils removed (if it's got nils in it to begin with).
…DLinks since I couldn't actually use them.
…because the new way was sometimes returning an error when it shouldn't.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…e GetScopeWithValueOwner that gets the scope with the value owner populated. Update the comment on GetScope to indicate that it no longer has the value owner field populated.
…teScopes and GetScopeWithValueOwner.
… owner populated.
…ce it's used in there.
… it stop at the first error, check for nil entries, and also make sure the AccAddr isn't empty. Delete the WithNilsRemoved method. Add TODO to tweak MetadataAddress.String() to not panic when the address is invalid.
…e in a few places.
…in an error in the UpdateValueOwnersCmd.
…f (and only works on a single scope), and refactor SetScopeValueOwners to require that all of the coins already exist.
…face and in the MockBankKeeper, impelement BlockedAddr, MintCoins BurnCoins and SendCoins.
…g the newValueOwner to prevent that lookup when the value owner is bad or blocked and include the burn error if there is one.
…rChangeToProposed, validateScopeValueOwnerChangeFromExisting, and ValidateScopeValueOwnerUpdate methods since the latter isn't used anymore, and the others were only used by that flow. Create IsMarkerAddr and use it to not require markers to sign for value owner chagnes since the send restiction will check for permissions among the signers.
…and replace it with GetScopesForValueOwner.
…ne place and its doing any needed checks.
…ValidateScopeValueOwnersSigners. The new one has most of the content of ValidateUpdateValueOwners. Make all of WriteScope, DeleteScope, UpdateValueOwners, and MigrateValueOwner all use this same verification for value owner changes. Only provide the validatd signers as transfer agents.
…scope and change the TODO on it to decide if we want to require it.
…iple transfer agents.
…oes the check without having to get the account from auth.
…switch to the marker module's version which is more efficient.
…ts regardless of whether any of the existing accounts are marker accounts. It was either that or have it check the proposed too to see see if they might be needed.
…hat logic is now being tested on ValidateScopeValueOwnersSigners.
…ut of the keeper and into types since they might be needed by people trying to use the metadata keeper. Also create the provutils package for some generic stuff that'd probably be handy in other places.
…r utils that didn't have any yet.
…g owners, still process the signers and return them appropriately.
…stValidateWriteScope to fix all the stuff that has broken and add some new cases.
…g and could use some added cases and stuff.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR takes the value owner records out of the scopes and uses the bank module to keep track of them by minting a coin with the denom "nft/".
closes: #2137
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments..changelog/unreleased(see Adding Changes).Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.