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

Amino support for x/authz & x/feegrant #9417

Closed
4 tasks
amaury1093 opened this issue May 28, 2021 · 3 comments · Fixed by #9457
Closed
4 tasks

Amino support for x/authz & x/feegrant #9417

amaury1093 opened this issue May 28, 2021 · 3 comments · Fixed by #9457
Assignees
Labels
C:Ledger Issues and features related Ledger integration and functionality C:x/authz C:x/feegrant
Milestone

Comments

@amaury1093
Copy link
Contributor

Summary

Should we add amino support for x/authz and x/feegrant Msgs ?

Problem

The current state of x/authz and x/feegrant is that their Msgs don't support legacy amino JSON signing.

The reasons for this were primarily:

So whereas the SDK as a whole still supports legacy amino JSON, some modules inside it don't. Ledger users won't be able to sign authz and feegrant Msgs.

Proposals

Either we fully support amino:
+ Ledger support
- more legacy code lying around

Or we fully document which modules in the SDK support legacy amino, and which don't
+ Less code change needed, v0.43 will be out sooner
- no ledger support


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@tac0turtle
Copy link
Member

I think more and more modules are being written with no amino. I have written a few that has removed or not added amino at all. I think saying the same for new modules makes sense.

We should just expedite the sign mode that fixes it

@aaronc
Copy link
Member

aaronc commented May 28, 2021

I think we should add for now but I also think we should expedite sign mode textual.

@aaronc aaronc added this to the v0.43 milestone May 28, 2021
@aaronc aaronc added the C:Ledger Issues and features related Ledger integration and functionality label May 28, 2021
@amaury1093
Copy link
Contributor Author

I'll take this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Ledger Issues and features related Ledger integration and functionality C:x/authz C:x/feegrant
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants