Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

Ethereum app not working with api #9

Open
mtleliever opened this issue Jun 25, 2018 · 4 comments
Open

Ethereum app not working with api #9

mtleliever opened this issue Jun 25, 2018 · 4 comments

Comments

@mtleliever
Copy link

Hi there.

I've noticed that the api runs into errors when trying to run the sample code with the Ethereum app opened instead of Bitcoin.

First off, I noticed that LedgerClient.GetHIDLedgers() returns a 0 length IEnumerable when the "Browser Support" setting is enabled on the Ledger Ethereum App. When Browser Support is disabled, I do end up getting a proper result with LedgerClient.GetHIDLedgers().First().

However, any call I make after that gets me a LedgerWalletException: INS not supported. Even a simple call where I can't really make any mistakes, such as ledger.GetFirmwareVersion() runs into the same exception.

When running this same code on the Bitcoin app for example, I get the proper firmware version and don't run into any exceptions.

This was tested using the latest firmware version of the Ledger.

@NicolasDorier
Copy link
Contributor

I don't know about how the Ethereum app is working, but it has probably different API than Bitcoin.

@mtleliever
Copy link
Author

I decided to look more deeply into this to determine more accurately what was going on, and discovered a few things in the process.

It is not only the Ethereum app which runs into the LedgerWalletException: INS not supported, but it is several apps. Ethereum, Ripple, Stellar, Nano, and Ark all run into the same error when performing any call, such as ledger.GetFirmwareVersion() for example. The one thing these apps all have in common is they are the only apps (that I found) on the Ledger which have Settings in the app. The Bitcoin app, Dogecoin, Dash, and many others, have no Settings in their app, and they all function fine with the ledger.GetFirmwareVersion() call. The contents of Settings can vary. Some will have the "Browser Support" option, while others have different ones, but they all run into the error no matter what if they have Settings.

I decided to sniff around in the code a bit to see where things actually started changing. In HIDTransportBase, in the Read method, on line 182, when the data is read from the HID, is where the data starts to differentiate. The packet byte[] with the Bitcoin app is the same as every other app without Settings, while the packet byte[] with the Ethereum app is the same as every other app with Settings. So then finally, when the command is unwrapped on line 185, the result is incorrect and completely different, which is what ends up causing the error I think.

Do you have any plans on looking into this and potentially providing a fix for it? If not, do you have any idea how I might go about approaching a fix for this?

@NicolasDorier
Copy link
Contributor

I will not look into this myself, but I would be happy to merge any PR you would make as long as it does not break Bitcoin related currencies.

@MelbourneDeveloper
Copy link
Contributor

@ThatSlyGuy I am starting to wade through the code here: https://github.com/LedgerHQ/blue-app-eth . But, it's not exactly pretty. You are right about these issues. It seems that there is no standard across the Ledger 3rd party apps. It appears that each app will have its own logic and messaging system.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants