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

CLI gov deposit JSON errors with ledger #9055

Closed
4 tasks
Tracked by #13327 ...
fedekunze opened this issue Apr 6, 2021 · 9 comments
Closed
4 tasks
Tracked by #13327 ...

CLI gov deposit JSON errors with ledger #9055

fedekunze opened this issue Apr 6, 2021 · 9 comments
Assignees
Labels
C:CLI C:Ledger Issues and features related Ledger integration and functionality C:x/gov

Comments

@fedekunze
Copy link
Collaborator

fedekunze commented Apr 6, 2021

Summary of Bug

Deposit JSON can't be signed by ledger due to whitespace

Version

Gaia v4.2.0
Ledger Cosmos v2.18.0

Steps to Reproduce

gaiad tx gov deposit <proposal> <amount> <flags> --ledger

confirm transaction before signing and broadcasting [y/N]: y
Error: JSON Contains whitespace in the corpus

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@fedekunze fedekunze added C:CLI C:x/gov C:Ledger Issues and features related Ledger integration and functionality labels Apr 6, 2021
@fedekunze
Copy link
Collaborator Author

also fails with gaiad tx sign <file>

@fedekunze
Copy link
Collaborator Author

cc @jleni

@amaury1093
Copy link
Contributor

Where in the JSON are there whitespaces? Could you --generate-only to see the JSON?

@jleni
Copy link
Member

jleni commented Apr 6, 2021

same question. Could you post the payload you are sending?
if there is a bug, we should be able to reproduce it by adding an additional unit test to the parser

@fedekunze
Copy link
Collaborator Author

{"body":{"messages":[{"@type":"/cosmos.gov.v1beta1.MsgDeposit","proposal_id":"44","depositor":"cosmos1849m9wncrqp6v4tkss6a3j8uzvuv0cp7wcgvqa","amount":[{"denom":"uatom","amount":"255000000"}]}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[{"denom":"uatom","amount":"54"}],"gas_limit":"106309","payer":"","granter":""}},"signatures":[]}

@amaury1093
Copy link
Contributor

amaury1093 commented Apr 6, 2021

Ah, my bad, --generate-only doesn't print the SignDoc (the signing payload), just the proto JSON of the full tx.

I added manually a fmt.Println of the amino SignDoc while testing on a local chain (explicity with spaces in the memo), and it printed:

{"account_number":"8","sequence":"2","chain_id":"my-chain","memo":"A B C","fee":{"amount":[],"gas":"200000"},"msgs":[{"type":"cosmos-sdk/MsgDeposit","value":{"amount":[{"amount":"10","denom":"stake"}],"depositor":"cosmos1xl2256vdh0j68khz9wq88hnyqcq0f5f4za2480","proposal_id":"1"}}]}

which seems correct. I also managed to deposit, so I couldn't reproduce the initial error.

BTW, afaiu the --ledger flag is useless here, since the CLI read from the --from and determines if it's a ledger key or not from the keyring. That flag is used only in keys add command.

@fedekunze Could you provide more details about the reproduction steps?

@fedekunze
Copy link
Collaborator Author

ok this is super weird but this only fails if the amount is ≥ 100 atoms (100000000uatom)

@fedekunze
Copy link
Collaborator Author

prob a parsing issue?

@ftheirs
Copy link

ftheirs commented Jan 2, 2023

Added test for parser and integration! This is working fine!
Related PR: cosmos/ledger-cosmos#77

@ftheirs ftheirs closed this as completed Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:CLI C:Ledger Issues and features related Ledger integration and functionality C:x/gov
Projects
None yet
Development

No branches or pull requests

5 participants