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

feat: add XCS registry permissioning and docs #4553

Merged
merged 37 commits into from
Mar 15, 2023

Conversation

czarcas7ic
Copy link
Member

@czarcas7ic czarcas7ic commented Mar 9, 2023

Closes: #4442

What is the purpose of the change

This PR introduces updates to the XCS Registry feature of Osmosis. The changes made in this branch aim to improve the security and usability of the XCS Registry by adding permissions and documentation.

Specifically, this branch introduces a new permission system for the XCS Registry. With these permissions, the contract admin can now restrict access to the registry, enabling them to control who can write, modify, enable, and disable the contents of the registry. This new feature enhances the security of the registry by reducing the risk of unauthorized access.

This PR also introduces a basic README.

Brief Changelog

  • allows the contract governor to make any registry modifications
  • allows the contract governor to set an authorized address for a specific source_chain
    • that address can make any registry modifications to their specified source_chain, but nothing else
  • starts a README.md for the crosschain registry contract
  • refactors the previous execute.rs tests to
    • include contract calls from Authorized, Unauthorized, and Contract Owner addresses
    • combines success and failure test cases to prevent repeating boilerplate

Testing and Verifying

Modified tests to attempt to run CRUD operations on Authorized, Unauthorized, and Contract Owner addresses

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (yes / no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
  • How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

@czarcas7ic czarcas7ic added the V:state/compatible/no_backport State machine compatible PR, depends on prior breaks label Mar 9, 2023
@czarcas7ic czarcas7ic marked this pull request as ready for review March 10, 2023 02:24
@nicolaslara
Copy link
Contributor

nicolaslara commented Mar 10, 2023

This looks good to me, but we should deal with not being able to directly modify existing items. That can be another PR, and I think it can either be disallowing changes or just requiring a time delay for gov to cancel them if necessary.

Same with deletions.


### UnwrapDenom

The `UnwrapDenom` query allows a caller to retrieve the denom trace for a given IBC denom and then unwrap it into its constituent coins on the originating blockchain.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we want to expose this at all. If so we may want to change the name, as "unwrap" should refer to actually sending the message

Copy link
Member Author

Choose a reason for hiding this comment

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

True, we really have no reason to expose this. If you agree then I can remove it here!

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed in 618a6d6

Copy link
Contributor

@nicolaslara nicolaslara left a comment

Choose a reason for hiding this comment

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

@czarcas7ic made some small changes when removing the unwrap query. This should be ready for merge if you don't have strong opinions about my changes

@czarcas7ic czarcas7ic merged commit 66d5eaa into main Mar 15, 2023
@czarcas7ic czarcas7ic deleted the adam/xcs-registry-perms-and-doc branch March 15, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Routing] Permissions for registries
3 participants