Add ledger wallet support #398
Conversation
There was a problem hiding this comment.
Please check this out @fyookball @kyuupichan
This is needed since the native Ledger app is kinda broken:
https://www.reddit.com/r/btc/comments/7n2t6o/ledger_cto_requests_suggestions_to_fix_their/
plugins/ledger/ledger.py
Outdated
| def perform_hw1_preflight(self): | ||
| try: | ||
| firmwareInfo = self.dongleObject.getFirmwareVersion() | ||
| firmware = firmwareInfo['version'].split(".") | ||
| self.multiOutputSupported = int(firmware[0]) >= 1 and int(firmware[1]) >= 1 and int(firmware[2]) >= 4 | ||
| bitcoinCashSupport = int(firmware[0]) >= 1 and int(firmware[1]) >= 1 and int(firmware[2]) >= 8 |
There was a problem hiding this comment.
what does the change from 4 to 8 signify? might be worth pulling this number out into a named variable for readability 👍
There was a problem hiding this comment.
AFAIK, that's the version for the Bitcoin Cash app in the Nano S.
The current version is 1.1.8 @bderose
There was a problem hiding this comment.
Correct, however the current version checking is a bit odd (ie. 1.1.9 would work but not 1.2.0), so a few things could be tweaked to provide backwards compatibility. Thoughts?
There was a problem hiding this comment.
One solution would be to use something like the semver package to do this comparison. I realize we may not want to introduce new dependencies, though. In that case, this SO question has some good suggestions.
There was a problem hiding this comment.
Ok, I've added the version comparison from distutils.version so no new dependencies are required, and it seems to work well.
plugins/ledger/ledger.py
Outdated
| def perform_hw1_preflight(self): | ||
| try: | ||
| firmwareInfo = self.dongleObject.getFirmwareVersion() | ||
| firmware = firmwareInfo['version'].split(".") | ||
| self.multiOutputSupported = int(firmware[0]) >= 1 and int(firmware[1]) >= 1 and int(firmware[2]) >= 4 | ||
| bitcoinCashSupport = int(firmware[0]) >= 1 and int(firmware[1]) >= 1 and int(firmware[2]) >= 8 |
There was a problem hiding this comment.
AFAIK, that's the version for the Bitcoin Cash app in the Nano S.
The current version is 1.1.8 @bderose
| inputIndex = inputIndex + 1 | ||
| if pin != 'paired': | ||
| firstTransaction = False | ||
| self.get_client().startUntrustedTransaction(True, inputIndex, |
There was a problem hiding this comment.
Only BIP143 signing is needed
| tmp += bfh(int_to_hex(utxo[1], 4)) | ||
| chipInputs.append({'value' : tmp, 'sequence' : sequence}) | ||
| redeemScripts.append(bfh(utxo[2])) | ||
| txtmp = bitcoinTransaction(bfh(utxo[0])) |
There was a problem hiding this comment.
Does this support both P2SH and non-P2SH transactions using the Nano S?
There was a problem hiding this comment.
I've only tested 2 cosigner multisig, but it should (since all transactions are BIP143, unlike core where its only segwit)
|
Awesome! You have no idea how useful this will be. Glad to see work being done on this issue, especially while Ledger's native software is having huge problems with low-fee transactions (see here). Great work! |
Fixes #153
This was tested using the Nano S - I can't verify it would work for the Blue or HW1.
I tested both standard and multisig wallets, along with multiple outputs on the Bitcoin Cash testnet and mainnet.
Most of the code needed for ledger wallet BIP143 transactions was sourced directly from the segwit update of spesmilo/electrum, so credit is due there.