-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Adding support for Ledger Cosmos App v1.5 #4227
Conversation
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
should we block this PR until LedgerHQ/ledger-app-cosmos#5 is merged ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jleni ! minor comments only. Does this require a docs update somewhere ?
It should be backwards compatible. It uses old APIs if the app version number is <1.5. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested ACK, although there's an UX issue if the address is rejected
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
Co-Authored-By: jleni <juan.leni@zondax.ch>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestedACK -- just a few minor bits of feedback. Thanks @jleni
Co-Authored-By: jleni <juan.leni@zondax.ch>
applied all the suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Cherry Pick PR #4345: Upgrade ledger-cosmos-go * Cherry Pick PR #4336: Fix AppendTags usage error * Update modules * Cherry Pick PR #4265: CacheKVStore keep sorted items * Cherry Pick #4227: Adding support for Ledger Cosmos App v1.5 * Cherry Pick #4395: Improve sig verification error message * Cherry Pick PR #4140: Fix Failed Simulation Seeds
This PR adds support for the latest version of the Cosmos App (v.1.5)
Attention: The app is not been released yet by Ledger but the PR is backwards compatible.
We can later remove backwards compatibility and enforce v1.5 only.
When creating a new account,
gaiacli
now shows the account/index and address in the device and requires user confirmation.Related PRs:
cosmos/ledger-cosmos-go#3
cosmos/ledger-cosmos-go#4
cosmos/ledger-cosmos-go#5
cosmos/ledger-cosmos-go#6
Changes in the app can be found here:
LedgerHQ/ledger-app-cosmos#5
docs/
)sdkch add [section] [stanza] [message]
Files changed
in the github PR explorerFor Admin Use: