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

Add support for vesting messages (MsgCreateVestingAccount) #1074

Closed
webmaster128 opened this issue Mar 6, 2022 · 5 comments · Fixed by #1114
Closed

Add support for vesting messages (MsgCreateVestingAccount) #1074

webmaster128 opened this issue Mar 6, 2022 · 5 comments · Fixed by #1114

Comments

@webmaster128
Copy link
Member

There seems to be one message only: https://github.com/cosmos/cosmos-sdk/blob/v0.45.1/x/auth/vesting/types/msgs.go

This came up in #1062 but the author decided to just be selfish and fork away instead of creating a trivial PR to add this type to the codebase.

@dreamcodez
Copy link

dreamcodez commented Mar 28, 2022

@webmaster128 this 'selfish' author wanted to test that his approach worked before sending a PR upstream. Also, I have no idea how to get the checked in node_modules build chain working. Please note the number of files changed.

I'm happy to submit these changes upstream when I understand the codebase better and I can clean it up.

@webmaster128
Copy link
Member Author

webmaster128 commented Mar 29, 2022

Hey @dreamcodez, thank you! For CosmJS maintenance we use yarn 3 for package management. This makes the setup a bit tricky. See https://github.com/cosmos/cosmjs/blob/main/HACKING.md for a starting guide how to work in CosmJS. Let me know if something is missing or unclear.

@webmaster128
Copy link
Member Author

@dreamcodez do you want to give this ticket a try? In cosmos/cosmos-multisig-ui#54 you see how the message type is used and the Amino JSON converters are implemented when this is done on the user's side. But it would be great to get it in here to be easily available to every user.

@webmaster128
Copy link
Member Author

webmaster128 commented Apr 7, 2022

In #1114 we have the message type working with sign mode direct.

Unfortunately Amino JSON signing is causing trouble. Looking at cosmos/cosmos-sdk#11019 (review) it seems like the message type was not properly registered in Cosmos SDK 0.44/0.45. However, people report that signing with the Go CLI client and Ledger works, which indicates the same bug we found for authz messages in https://medium.com/p/8754a0dc2a88. Further debugging would be needed which requires getting the raw JSON sign doc during the signing process from the Go CLI.

Is there any value for users to get the direct sign mode merged without Amino JSON support?

@webmaster128
Copy link
Member Author

See #1115 for the follow-up ticket for Amino JSON support

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