N1029676 commented 4 years ago

Summary: I used the CLI Ledger application to send HNT back to my phone and a different address appeared instead.

I haven't had a change to really dig into the source code but I found a pretty substantial issue and I'm not sure if it is user error, an innocent bug, or some nefarious issue here. I just used the helium-ledger-app v1.0.2 with my Ledger NanoX to succesfully receive some HNT and before I load it up with the bulk of the token from my phone I wanted to make sure I could get it back out. Unfortunately the Ledger application displayed a similar (but different) address to the one I entered to send funds to. I sent a small transaction to test and sure enough it went to the incorrect address instead of what I entered...

Addresses

Ledger: 14xxxxxxxxRWnG

Phone: 14xxxxxxxxZ4BBr Faulty: 14xxxxxxxxgCihq

Faulty transaction

PS C:\Users\xxx> .\helium-ledger-app.exe pay 14xxxxxxxxZ4BBr hnt 1
Communicating with Ledger - follow prompts on screen
Creating transaction for:
      1.00000000 HNT
        =
      100000000 Bones
+-----------------------------------------------------+------------+-------+----------------------------------------------+
| Payee                                               | Amount HNT | Nonce | Hash                                         |
+-----------------------------------------------------+------------+-------+----------------------------------------------+
| 14xxxxxxxxgCihq                                     | 1.00000000 | 1     | xxxxxxxxxx=                                  |
+-----------------------------------------------------+------------+-------+----------------------------------------------+

Balance command

PS C:\Users\xxx> .\helium-ledger-app.exe balance --qr
Communicating with Ledger - follow prompts on screen
+-----------------------------------------------------+--------------+--------------+-----------------+
| Address                                             | Balance HNT  | Data Credits | Security Tokens |
+-----------------------------------------------------+--------------+--------------+-----------------+
| 14xxxxxxxxRWnG                                      | 171.28424078 | 0            | 0               |
+-----------------------------------------------------+--------------+--------------+-----------------+

Ledger

Model: Ledger Nano X Firmware: 1.2.4-1 Helium version: 1.0.0 (40 Kb)

Helium CLI App

Version: v1.0.2 (among others) SHA256: 8989D8C4E5E582DB9D208C6A08BC8F308C5A2F68583B94E88DA54D4AD11B2E75

N1029676 commented 4 years ago

I just confirmed the same thing happens on:

helium-ledger-app 1.0.2
helium-ledger-app 1.0.1
helium-ledger-app 1.0.0

lthiery commented 4 years ago

Thanks for the report.

Just to be clear, we haven't tested or validated on the Nano X at all -- I am surprised the app opens up at all on that platform. The main README only mentions Nano S but I will update it to be more explicit about lack of Nano X suppport.

Did you install an unsigned binary that you built from this repo? Or does Ledger Live allow you to install on Nano X?

A few other questions about your report:

I understand that the balance command and the pay command claims different addresses ("Ledger" and "Faulty"), but what is your phone address? Is that just the payer of the test transaction?
What address is displayed on the screen when you checked your balance?

N1029676 commented 4 years ago

Everything installed just fine and without warning. I figured documentation just wasnt fully updated or tested yet, so that's what I get...

The phone address is what I entered on CLI and where I wanted the transaction to be sent to; but faulty address is what appeared on screen and ledger and ultimately where it went.

The address of the ledger properly appears on screen (CLI) and ledger.

I'm willing to help troubleshoot and develop but I'm unfamiliar with this so if you have a moment to spin me up I'm here to help. Especially since I stupidly sent 120 to the Ledger already before testing.

lthiery commented 4 years ago

I tested on a Nano S and I have good news for you, bad news for me.

The good news is that I believe your 120 HNT are recoverable. If you use the companion app somewhere other than Windows, the send address should output properly. In other words, this only affect sending from Windows.

The bad news for me is that I think this is a bug in the Windows version of the utility, so it affects Nano S as well. I'm pretty sure there's a difference in how the USB is being handle and the payee address is being truncated. I was able to replicate with Nano S on Windows.

I've pulled the Windows releases for now and am working on a fix.

N1029676 commented 4 years ago

Confirmed, it works fine when run from Ubuntu. Mild panic averted!

Let me know if you want assistance testing on Windows or Ledger Nano X.

lthiery commented 4 years ago

Well it's very good to know that you have had any success with the Nano X!

I did just tag a new release which fixes the Windows problem, at least as far as I can tell. If you have a moment to test, it would be great to confirm. It should be available under releases in less than 15 minutes.

N1029676 commented 4 years ago

The prompts are correct and the balance query works but after approving the transaction on the NanoX, the CLI app responds with error: Could not find ledger. Is it disconnected or locked? immediately after approval.

CLI App

Version: v1.1.0 SHA256: BC06912E2568799AF0DC9F8FD31093BDF5A92B5B94B01CC456EFFB91F730C594

Balance query

PS C:\Users\xxx> .\helium-ledger-app.exe balance
Communicating with Ledger - follow prompts on screen
+-----------------------------------------------------+-------------+--------------+-----------------+
| Address                                             | Balance HNT | Data Credits | Security Tokens |
+-----------------------------------------------------+-------------+--------------+-----------------+
| 14xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxSRWnG | 20.00000000 | 0            | 0               |
+-----------------------------------------------------+-------------+--------------+-----------------+

Transaction attempt

PS C:\Users\xxx> .\helium-ledger-app.exe pay 14xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxZ4BBr hnt 1.0002
Communicating with Ledger - follow prompts on screen
Creating transaction for:
      1.00020000 HNT
        =
      100020000 Bones
error: Could not find ledger. Is it disconnected or locked?

Environment

Host Name:                 XXXXX
OS Name:                   Microsoft Windows 10 Pro for Workstations
OS Version:                10.0.18363 N/A Build 18363
OS Manufacturer:           Microsoft Corporation
OS Configuration:          Standalone Workstation
System Boot Time:          6/12/2020, 9:48:46 PM
System Manufacturer:       LENOVO
System Model:              30BECTO1WW
System Type:               x64-based PC
Processor(s):              1 Processor(s) Installed.
                           [01]: Intel64 Family 6 Model 85 Stepping 4 GenuineIntel ~3600 Mhz
BIOS Version:              LENOVO S03KT37A, 1/15/2020
Hotfix(s):                 12 Hotfix(s) Installed....
Network Card(s):           5 NIC(s) Installed....
Hyper-V Requirements:      A hypervisor has been detected. Features required for Hyper-V will not be displayed.

lthiery commented 4 years ago

On the Nano X, the display says "Waiting for commands" and only then can you send a fresh command.

Perhaps the screen which displays the Public Key is still pending on your screen when you issue the payment command?

N1029676 commented 4 years ago

"Waiting for commands" appears when I send the request.

It successfully says Transaction not confirmed when I decline the transaction on the NanoX, but still shows error: Could not find ledger. Is it disconnected or locked? immediately upon approval.

lthiery commented 4 years ago

Okay so this is probably Nano X specific then. This worked fine in Ubuntu with Nano X though?

N1029676 commented 4 years ago

It did with the previous version, haven't tried with the new one yet on Ubuntu.

Any chance or desire for NanoX support?

N1029676 commented 4 years ago

So v1.1.0 does not work with NanoX on Ubuntu either but v1.0.2 still does in the same configuration.

lthiery commented 4 years ago

We'd like to support Nano X. And it seems like the issues so far have not been related to Nano X.

I tested again and realized that my fix for the Windows write was ruined the read operation on all platforms. I didn't go so far as to submit the transaction - I only verified that the payee key was accurate.

I have a fix and I've submitted a payment from both Windows and Ubuntu. I'll have it tagged and available shortly as a release.

lthiery commented 4 years ago

Alright, give this a whirl: https://github.com/helium/helium-ledger-app/releases/tag/1.1.1

N1029676 commented 4 years ago

Confirmed! v1.1.1 worked on Windows10 and Ubuntu with the Ledger NanoX for the following actions:

[x] retrieving balance
[x] sending payment
[x] sending payment (while declining transaction)

I believe this issue is resolved!

lthiery commented 4 years ago

Glad to hear it! Thanks for your help

helium / helium-ledger-app

CLI App sent HNT to different address than the one entered #10

Addresses

Faulty transaction

Balance command

Ledger

Helium CLI App

CLI App

Balance query

Transaction attempt

Environment