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

x/authz design should not require a cyclic dependency to function #16463

Closed
Tracked by #13025
kocubinski opened this issue Jun 8, 2023 · 3 comments · Fixed by #16509
Closed
Tracked by #13025

x/authz design should not require a cyclic dependency to function #16463

kocubinski opened this issue Jun 8, 2023 · 3 comments · Fixed by #16509
Assignees

Comments

@kocubinski
Copy link
Member

kocubinski commented Jun 8, 2023

Problem

The current x/authz design has modules x/bank and x/staking implementing authz.Authorization, which contains a reference the authz.AcceptResponse. This means that modules implementing Authorization must depend on x/authz which is already a bit of an anti-pattern.

CmdGrantAuthorization in x/authz depends on x/bank and /x/staking, forming the cyclic module relationship.

Possible Solutions

  1. Push AcceptResponse down into sdk/types, and change Updated field type to proto.Message. Refactor usages.
  2. ADR-33 style plugin system (details forthcoming)
@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Jun 8, 2023
@kocubinski kocubinski added T:Sprint and removed needs-triage Issue that needs to be triaged labels Jun 8, 2023
@github-project-automation github-project-automation bot moved this to 📝 Todo in Cosmos-SDK Jun 8, 2023
@aaronc
Copy link
Member

aaronc commented Jun 8, 2023

Should we just switch to autocli?

@kocubinski
Copy link
Member Author

Should we just switch to autocli?

Maybe, what's the timeline for that as viable production ready replacement though?

@aaronc
Copy link
Member

aaronc commented Jun 8, 2023

Should we just switch to autocli?

Maybe, what's the timeline for that as viable production ready replacement though?

No idea, we need to decide who is going to work on transaction support and when we prioritize that.

For authz cli we probably should just expect a json authorization instead of trying to cover so many use cases

@kocubinski kocubinski self-assigned this Jun 13, 2023
@github-project-automation github-project-automation bot moved this from 📝 Todo to 👏 Done in Cosmos-SDK Jun 13, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK Jul 18, 2023
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 a pull request may close this issue.

2 participants