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 #69

Merged
merged 30 commits into from
Jul 25, 2023
Merged

Conversation

ryanchristo
Copy link
Member

@ryanchristo ryanchristo commented May 23, 2023

Closes: #10

This pull request adds support for amino signing.

  • MsgCreateGroupPolicy
  • MsgCreateGroupWithPolicy (policyAsAdmin set to true)
  • MsgCreateGroupWithPolicy (policyAsAdmin set to false)
  • MsgExec
  • MsgSubmitProposal (no messages)
  • MsgSubmitProposal (with MsgSend)
  • MsgSubmitProposal (with MsgDelegate)
  • MsgSubmitProposal (with MsgRedelegate)
  • MsgSubmitProposal (with MsgUndelegate)
  • MsgSubmitProposal (with MsgWithdrawDelegatorReward)
  • MsgSubmitProposal (with MsgUpdateGroupMetadata)
  • MsgSubmitProposal (with MsgUpdateGroupMembers)
  • MsgSubmitProposal (with MsgUpdateDecisionPolicy)
  • MsgUpdateGroupMetadata
  • MsgUpdateGroupMembers
  • MsgUpdateDecisionPolicy
  • MsgVote (exec set to 1)
  • MsgVote (exec set to 0)

Deploy Preview: https://deploy-preview-69--regen-groups-ui.netlify.app/

@netlify
Copy link

netlify bot commented May 23, 2023

Deploy Preview for regen-groups-ui ready!

Name Link
🔨 Latest commit fa6f45d
🔍 Latest deploy log https://app.netlify.com/sites/regen-groups-ui/deploys/64bfe93b3a7c250008a768ec
😎 Deploy Preview https://deploy-preview-69--regen-groups-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ryanchristo
Copy link
Member Author

ryanchristo commented Jun 13, 2023

The current setup using getOfflineSignerOnlyAmino works with messages that do not have embedded types but it does not work with creating and updating group policies and submitting proposals. Still investigating.

@ryanchristo
Copy link
Member Author

ryanchristo commented Jun 13, 2023

MsgCreateGroupWithPolicy (using new helpers)

Broadcasting transaction failed with code 4 (codespace: sdk). Log: signature verification failed; please verify account number (1), sequence (13) and chain-id (regenlocal): unauthorized

MsgSubmitProposal (not using new helpers)

Broadcasting transaction failed with code 7 (codespace: sdk). Log: msg 0: invalid from address: empty address string is not allowed: invalid address

Updating group name and metadata works as expected.

@ryanchristo
Copy link
Member Author

MsgSubmitProposal (not using any helpers and no message)

Broadcasting transaction failed with code 4 (codespace: sdk). Log: signature verification failed; please verify account number (1), sequence (3) and chain-id (regenlocal): unauthorized

Same error as MsgCreateGroupWithPolicy with helpers.

package.json Outdated Show resolved Hide resolved
@ryanchristo
Copy link
Member Author

ryanchristo commented Jun 15, 2023

MsgCreateGroupWithPolicy is working after updating metadata fields to not be empty and only if groupPolicyAsAdmin is set to true. It seems the encoding of zero-values is not working correctly. Also, MsgSubmitProposalEncoded type does not exist in the telescope generated code so will need to figure out why and how to proceed.

@ryanchristo ryanchristo changed the title feat: #10 add amino signing support feat: add amino signing support Jun 16, 2023
@ryanchristo ryanchristo requested a review from blushi June 16, 2023 19:16
src/api/group.messages.ts Outdated Show resolved Hide resolved
@ryanchristo ryanchristo force-pushed the ryan/10-amino-signing-support branch from 96f220c to f5c3bea Compare June 29, 2023 04:55
@ryanchristo ryanchristo changed the base branch from main to dev July 3, 2023 19:04
@ryanchristo
Copy link
Member Author

This is currently working with all group messages but the project won't build due to typescript errors.

@ryanchristo
Copy link
Member Author

ryanchristo commented Jul 12, 2023

I think we should manually fix the typescript errors upstream (regen-network/regen-js#79) and tag a new version of the api package that we can use until we resolve the issues in telescope and/or regen-js.

I think it's ok if the changes get overwritten as we can always pull them back in if we need to tag a new release of the api package that will be used in the groups ui. I'd like to at least have a version we know can use for the MVP and I think rewriting the code we are importing from @regen-network/api/src is too much to reproduce in this repo.

@ryanchristo ryanchristo requested a review from a team July 12, 2023 23:29
@ryanchristo
Copy link
Member Author

If we decide to merge regen-network/regen-js#79, I'll tag another release and update and then open for review.

@ryanchristo ryanchristo marked this pull request as ready for review July 13, 2023 16:52
@ryanchristo
Copy link
Member Author

This is now ready to review.

@ryanchristo
Copy link
Member Author

ryanchristo commented Jul 13, 2023

And follow up issue created #105.

@blushi
Copy link
Member

blushi commented Jul 18, 2023

I'm getting an error when editing group:
Broadcasting transaction failed with code 4 (codespace: sdk). Log: signature verification failed; please verify account number (12), sequence (1866) and chain-id (regen-redwood-1): unauthorized

@ryanchristo
Copy link
Member Author

Oof. Looking into it. Also noticed the update group messages need to be added. Will fix and add.

@ryanchristo ryanchristo marked this pull request as draft July 18, 2023 19:55
@ryanchristo
Copy link
Member Author

The new messages require a new file from the api src directory so we may need to manually resolve and release a version with the manual fixes upstream...

@ryanchristo
Copy link
Member Author

Also running into issues now with submitting proposals with the following messages:

  • MsgUpdateGroupMembers
  • MsgUpgradeGroupPolicyDecisionPolicy

@ryanchristo ryanchristo marked this pull request as ready for review July 24, 2023 20:38
@blushi
Copy link
Member

blushi commented Jul 25, 2023

Working now with a MsgUpdateGroupMembers proposal.

But I still get the following error when trying to create a proposal with MsgUpdateGroupDecisionPolicy as soon as I have a percentage decision policy:

Proposal could not be created:
Broadcasting transaction failed with code 10 (codespace: math). Log: msg 0: decision policy: threshold: parse mantissa: : invalid decimal string: invalid decimal string

Here's a fix: #116

Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

pre-approving, good to merge after final testing

@ryanchristo
Copy link
Member Author

pre-approving, good to merge after final testing

Ran through submit proposal messages one more time with each supported message (and no messages) on redwood:

https://deploy-preview-69--regen-groups-ui.netlify.app/37

@ryanchristo ryanchristo merged commit 9dead2b into dev Jul 25, 2023
@ryanchristo ryanchristo deleted the ryan/10-amino-signing-support branch July 25, 2023 16:42
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 this pull request may close these issues.

Set up & test amino types & signing
2 participants