LedgerHQ / app-ont

MIT License
2 stars 3 forks source link

BIP44 not compatible with existing NEO Ledger addresses #1

Closed nickfujita closed 6 years ago

nickfujita commented 6 years ago

So I was looking into the new ONT ledger app, and it seems that it will not be compatible with existing NEO addresses where users have their mainnet ONT stored either from the second airdrop, or via swap using neon-wallet.

https://github.com/ontio/OWallet/blob/master/src/core/ontLedger.js#L40

It appears the reason is that the SLIP44 value was updated to Ontology

https://github.com/satoshilabs/slips/blob/master/slip-0044.md

The problem with this is that if you change the unique address format produced by BIP44

https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki

Then the public key produced by the ledger will be different.

So I tested the ledger app using the Ontology SLIP44 - 80000400, and it seems to be able to connect to the device and generate the public key. However this address is different from the public key generated by the device when using the NEO SLIP44 - 80000378 So I tried swapping this value back to the NEO SLIP44 value, but the ledger app does not appear to accept connections to create the correct public address.(edited) It just errors out with Ledger device: UNKNOWN_ERROR (0x6804) This should not be an issue for new ledger users, but rather an issue for existing NEO ledger users, as their mainnet ONT/ONG will be unaccessible.

Can this app be updated to accept SLIP44 values for both NEO - 80000378 and Ontology - 80000400?

nickfujita commented 6 years ago

https://medium.com/ontologynetwork/owallet-faq-7f4f96784253

Regarding the proposed solution for users with existing NEO Ledger addresses. This does not appear to be a viable solution in the near or far term for the following reasons.

In the near term, the ask from Ontology is that all Ledger users that hold ONT completely reset their Ledger wallets. This reset does not only affect NEO and ONT, but any and all wallets a user may have on their device. Meaning that if a Ledger user has Bitcoin, Bitcoin Cash, Litecoin, Ethereum, so on and so forth, users need to transfer all funds out of these addresses, into temporary addresses, and then back into a newly generated set of Ledger addresses for each coin. Not only is this be time consuming, but it poses some risk to users as they are moving funds out of their secure device into an exchange (assumed to incur withdrawal fees) or paper wallets. Furthermore, the whole premies of owning a Ledger is that all private keys are stored in the device, and shall never leave. The one exception is the mnemonic phrase generated on first usage. As such, users are encouraged to protect this phrase for dear life, and some have gone through great lengths to secure these phrases or even portions of these phrases in multiple secure locations. This ask appears to be far to great.

The long term impact is that the creation of the Ontology SLIP-44: 0x80000400, is that the address sets between NEO and Ontology have and will more so become fragmented. Although the public and private keys between the two are in the same format, there will be a Ledger divide where seemingly compatible addresses have become no longer compatible. This will go beyond the recovery of ONT funds from the token swap or airdrop round 2. In the event that a user is sent ONT funds to their NEO created Ledger address, they will be forced to reset their Ledger for all wallets every time. While this should not be an every day practice, having seen the mass of support questions come for even just managing NEO addresses, this is bound to happen.

In conclusion, the root cause of this is that the Ontology SLIP-44: 0x80000400, in my opinion, should never have been created in the first place. Since the decision was to keep the address schema for Ontology the same as NEO, the same methodology should be applied when creating deterministic addresses with BIP-44.

This issue appears to be similar to how there are different ledger apps for Bitcoin and Bitcoin Cash. When using the Ledger Bitcoin wallet or Ledger Live, either the Bitcoin or Bitcoin Cash Ledger apps can be used. I would assume that this same issue exists between the 2, as they both have different SLIP-44 values. This means that they either use the same transaction signing format or have adjusted both apps to accommodate different transaction signing formats. Ideally if Ledger would allow Ontology to publish the Ontology Ledger app with just the NEO SLIP-44 value to exist along side the existing NEO Ledger app, since the signing logic is different for transactions. However, if this is not possible another solution would be to update the existing NEO Ledger app to handle transaction signing for both NEO and ONT.

nickfujita commented 6 years ago

It's curious that for ETH you are able to use the same Ledger app in myetherwallet to have different BIP32 values:

image

Some of which are accounted for in the Makefile for the ETH Ledger app: https://github.com/LedgerHQ/blue-app-eth/blob/master/Makefile

Would it be possible for us to do the same for this ONT app? Or at the very least just change the SLIP-44 value used in this app?

https://github.com/LedgerHQ/ledger-app-ont/blob/master/Makefile#L27

Update to:

APP_LOAD_PARAMS = --path "44'/888'" --appFlags 0x40 --apdu $(COMMON_LOAD_PARAMS)
TamtamHero commented 6 years ago

Fixed in commit https://github.com/LedgerHQ/ledger-app-ont/commit/76356de89f4f10106a8c11727e4200b7bb9551b8 The fix will be deployed in the coming days.

nickfujita commented 6 years ago

@TamtamHero Awesome! Thanks for adding this 👍

nickfujita commented 6 years ago

@TamtamHero When can expect to have a new version of this app deployed to Ledger Live? I know you mentioned "the coming days", but is it possible to be more specific?

jeroenski74 commented 5 years ago

I see that the derivation path is added, but I can't find it, probably I'm not looking at the right place. @nickfujita can you tell me what it is? I'm probably have to reset my Ledger, which is fine, but I just want to store my old (empty) addresses with key in case of a future drop I'm not aware of.

With kind regards!