namada icon indicating copy to clipboard operation
namada copied to clipboard

Ledger (hardware wallet) updates

Open murisi opened this issue 2 years ago • 14 comments

The following tasks pertain to the hardware wallet and the maintenance of the test-vectors that are used to check its correctness:

  • [x] Finish review of the replay protection changes to transaction signing
  • [x] Make a request to Zondax to change signing algorithm to accommodate above replay protection changes
  • [x] Make test vector generator cover more variations of each transaction type and increase quantity of vectors
  • [ ] Test the hardware wallet implementation of multisig either on the web or CLI client
  • [x] Check whether it is now possible to use other key derivation paths aside from the default one

Non-essential tasks to increase the maintainability of hardware wallet support:

  • Write code that attempts to parse a test vector JSON file in order to more quickly determine whether the serializations have compatibility breaking changes
  • Consider rewriting the test vector generator shell script in Rust (maybe using the SDK) and integrating into codebase to avoid desynchronization issues

murisi avatar Oct 20 '23 18:10 murisi

Hi, I am testing the ledger (hardware wallet - S Plus Device) code path for transfers and am consistently getting an error that I wanted to tag onto this open ticket:

$ namada wallet derive --use-device --alias testing-ledger-nano-S-plus
<address successfully imported from ledger device>
$ namada client transfer --node https://rpc.luminara.icu --source <redacted> --target <redacted> --token tnam1q87wtaqqtlwkw927gaff34hgda36huk0kgry692a --amount 1 --use-device
Submitting a tx to reveal the public key for address <redacted>...
...
Error:
   0: Ledger | App Error: | 27012 apdu parameters invalid

I am able to import my address and create a valid ledger connection to import my address, however, the ledger connection consistently fails for transfer transactions

Noting that I've tracked down the failing line to this: https://github.com/anoma/namada/blob/main/crates/apps/src/lib/client/tx.rs#L106 which is a call to sign from the zondax built Namada ledger app. Still investigating the root issue on my side

dan-u410 avatar Apr 22 '24 15:04 dan-u410

Hi @dan-u410 - thanks for reporting! For our debugging purposes, can you specify which version(s) of the Namada software and Ledger app you used for this test? This will help us investigate the issue.

cwgoes avatar Apr 23 '24 07:04 cwgoes

Hi @cwgoes, thank you for the speedy reply!

  • Namada Ledger App Version: 0.0.16
  • Namada CLI Version: v0.32.0 (should be reproducible on v0.33.0 as well)
  • I'm guessing the transfer transaction being pushed to the ledger from Nam CLI is failing to be parsed & causing the 0x6984 data invalid error

Full log:

./target/release/namada client transfer --node https://rpc.luminara.icu --source <redacted> --target <redacted> --token tnam1q87wtaqqtlwkw927gaff34hgda36huk0kgry692a --amount 1 --use-device
2024-04-22T21:02:22.538040Z  INFO log: [ 65] << 000101050000001a5701000015052c0000806d03008000000080000000800000008000000000000000000000000000000000000000000000000000000000000000
2024-04-22T21:02:22.761410Z  INFO log: [ 57] << 00c4144c1f7e77223936408dddb6b8940d34472d96aac918df105f431f12db6de7746e616d31717a347167336a7065646e6e65336d70787835
2024-04-22T21:02:22.767041Z  INFO log: [ 23] << 6e7479337839787a356478336379737374747730769000
2024-04-22T21:02:22.767263Z  INFO log: [ 65] << 000101050000001a5702000015052c0000806d03008000000080000000800000008000000000000000000000000000000000000000000000000000000000000000
2024-04-22T21:02:22.773077Z  INFO log: [  2] << 9000
2024-04-22T21:02:22.773207Z  INFO log: [ 65] << 00010105000000ff57020100fa1e0000006c756d696e6172612e3435666439346662356331346430646433303464610020000000323032342d30342d3232543231
2024-04-22T21:02:22.775033Z  INFO log: [ 65] << 0001010500013a30323a32322e3339343631312b30303a30305c8ec8e363f22d5de428707e055663e5395233aba4ebd5b03e541fb315d4903dc116d5cd044012ba
2024-04-22T21:02:22.777023Z  INFO log: [ 65] << 0001010500020e2441a38629fea231d81014323db22ee7997d27ffbcffac0000000000000000000000000000000000000000000000000000000000000000010100
2024-04-22T21:02:22.778999Z  INFO log: [ 65] << 0001010500030000000000000000000000000000000000000000000000000000000000000600fce5f4005fdd67155e475298d6e86f63abf2cfb200c4144c1f7e77
2024-04-22T21:02:22.780040Z  INFO log: [ 65] << 000101050004223936408dddb6b8940d34472d96aac918df105f430000000000000000000600fce5f4005fdd67155e475298d6e86f63abf2cfb200c4144c1f7e77
2024-04-22T21:02:22.786006Z  INFO log: [  2] << 9000
2024-04-22T21:02:22.786042Z  INFO log: [ 65] << 00010105000000cd57020200c81f12db6de74219000000000000a861000000000000000200000002eaf09d078f0100000020ce63594ce0dd531892a98ccecee95a
2024-04-22T21:02:22.788125Z  INFO log: [ 65] << 0001010500016a4b7f0e9e353183015bd4ea7ac3a429011000000074785f7472616e736665722e7761736d00ebf09d078f0100006200000001aa044641cb673cc7
2024-04-22T21:02:22.789997Z  INFO log: [ 65] << 0001010500026131a93592262985469a382401733582160ca036a20a63dbd4ff501cc8b341c75400fce5f4005fdd67155e475298d6e86f63abf2cfb240420f0000
2024-04-22T21:02:22.791002Z  INFO log: [ 65] << 00010105000300000000000000000000000000000000000000000000000000000006000041c75400fce5f4005fdd67155e475298d6e86f63abf2cfb240420f0000
2024-04-22T21:02:22.797110Z  INFO log: [ 25] << 556e7265636f676e697a6564206572726f7220636f64656984
Error:
   0: Ledger | App Error: | 27012 apdu parameters invalid

Location:
   /<>/namada/crates/apps/src/lib/cli/client.rs:66

Note: addresses I'm using are valid tnam1... addresses

dan-u410 avatar Apr 23 '24 12:04 dan-u410

@murisi Is this expected / do you expect your WIP changes to address this?

cwgoes avatar Apr 23 '24 15:04 cwgoes

@murisi Is this expected / do you expect your WIP changes to address this?

I suspect that this issue is due to Namada v0.33.0 not being supported by the Ledger app (which targets Namada v0.31.4). If the transaction formats have not changed much, I expect this to be quickly fixable.

murisi avatar Apr 24 '24 05:04 murisi

Tx format might change in this next release too BTW. If so, may want to base further ledger work on that (0.34.0)

brentstone avatar Apr 24 '24 06:04 brentstone

@dan-u410 Thanks for your report. I just realized that you're using Namada Ledger App version 0.0.16. Please try using Namada Ledger App version 0.0.18 ( https://github.com/Zondax/ledger-namada/releases/tag/v0.0.18 ) or later.

murisi avatar Apr 24 '24 06:04 murisi

@murisi I'm seeing v0.0.16 is the latest version of the Namada ledger app available in the Ledger Live App store. Could you please confirm? Might need to bump Zondax here to push new version to the Ledger app store. Noting Brent's msg, might just wait to bump until after v0.34.0 of the nam sdk release

I'll build and sideload the latest version of the Nam ledger app soon

dan-u410 avatar Apr 24 '24 15:04 dan-u410

hi @murisi || @cwgoes just wanted to give a heads up that ledger version v0.0.18 has not been published to the ledger live store

Ive also detailed a number of bugs in the offline signing code in the CLI here: https://github.com/anoma/namada/issues/3201

in the current state, I think most users will be totally blocked on using cold wallets if neither the ledger or offline CLI code path are fixed before mainnet

dan-u410 avatar Jul 09 '24 18:07 dan-u410

Hi @dan-u410 . Thanks for the heads up. The transaction format for Transfers changed in #3356 and also #3459 . So we've decided to publish to the ledger live store after a release has been cut to fix these issues.

murisi avatar Jul 11 '24 08:07 murisi

Hi @murisi do you have an ETA as to when that will be?

dan-u410 avatar Jul 17 '24 02:07 dan-u410