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 amino signing support to tokenfactory messages #7139

Merged
merged 2 commits into from
Jan 2, 2024

Conversation

madrezaz
Copy link
Contributor

What is the purpose of the change

This PR changes the codec being used for getting sign bytes of tokenfactory module messages, in order to allow amino signing.

Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes?
  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

@github-actions github-actions bot added C:app-wiring Changes to the app folder C:x/tokenfactory labels Dec 17, 2023
@madrezaz madrezaz marked this pull request as ready for review December 17, 2023 07:25
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

uTACK, thanks so much !

Comment on lines +54 to +56
RegisterLegacyAminoCodec(authzcodec.Amino)
RegisterLegacyAminoCodec(govcodec.Amino)
RegisterLegacyAminoCodec(groupcodec.Amino)
Copy link
Member

Choose a reason for hiding this comment

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

annoying that we need to do this :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I hope we can remove these in newer versions with textual sign mode

@mattverse mattverse added V:state/breaking State machine breaking PR A:no-changelog labels Dec 18, 2023
@madrezaz
Copy link
Contributor Author

Considering you tagged the PR as state breaking, should I move the changelog entry to another section?

Copy link
Contributor

github-actions bot commented Jan 2, 2024

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

@github-actions github-actions bot added the Stale label Jan 2, 2024
@mattverse mattverse removed the Stale label Jan 2, 2024
@mattverse
Copy link
Member

Considering you tagged the PR as state breaking, should I move the changelog entry to another section?

Should be fine :) thanks!

@mattverse mattverse merged commit 2a64b0b into osmosis-labs:main Jan 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:no-changelog C:app-wiring Changes to the app folder C:x/tokenfactory V:state/breaking State machine breaking PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants