search

namecoin / electrum-nmc

Namecoin port of Electrum Bitcoin client.
https://www.namecoin.org/
MIT License
29 stars 24 forks source link

Cherry-pick of recent Electrum fix for Ledger wallet Bitcoin app update #274

Closed maxweisspoker closed 3 years ago

maxweisspoker commented 3 years ago

This pull request allows Ledger hardware wallet to work. It is a cherry-picked commit from spesmilo/electrum. Please note that the pull request alters a small change in the cherry-picked commit due to Electrum-NMC not being up to date with Electrum. This means that possible future merges of Electrum may require manual intervention.

JeremyRand commented 3 years ago

Hi! Thanks for the PR. I'm really hesitant to cherry-pick individual fixes into master branch, as this makes maintenance a lot more painful long-term. I think the right approach is instead to catch up Electrum-NMC to upstream Electrum. I know it may look like that task is inactive, but in reality we are simply trying to get some of our changes upstreamed first, as that will cut down on maintenance effort. We will hopefully get some of that done soon, and then we can resume merging upstream commits.

That said, I'm curious: does Ledger actually work with Electrum-NMC if the upstream commits are cherry-picked? I wasn't actually sure if Ledger would work, as I don't think any of us tested it (unless I've forgotten some test results...).

maxweisspoker commented 3 years ago

So far I have only tested it with the "sign/verify message" feature. That works, although verification fails because of the displayed address format. When I convert the address to a Bitcoin format and try to validate it with Electrum (BTC), messages validate. So we know that it can communicate with the Ledger correctly. Therefore, I suspect that regular monetary (non-name) transactions will almost certainly work since Namecoin money-based transactions are identical to Bitcoin, (although the "addresses" displayed by Ledger for the tx preview may be Bitcoin addresses).

I have a few names that expire in just over 5k blocks, so sometime in the next month here, I am going to test it out. (Obviously, I will do tests with a new/test name -- I just have my NMC locked up in a way that is annoying/difficult to get at, so I'm waiting until I need to update my names anyway.) I'll report back with the results. Based on what I've read, I think it likely will not work, although I'm reasonably sure there could be a way to hack around it and somehow get the message signing feature to just sign a tx hash. (Although that would defeat at least some of the security purpose of the device.)

maxweisspoker commented 3 years ago

Apparently the Bitcoin Ledger app is locked into specific OP codes and thus will fail with a name transaction.

I'm therefore closing this pull request, since the purpose of a hardware wallet is to protect important assets, which on NMC is the names and not the coin (and Ledger doesn't support the names).