Fix/unchained wallets integration #354
Conversation
There was a problem hiding this comment.
I ran test suite against ledger bitcoin 2.0.6 and it passed for all applicable tests.
I ran test suite against ledger bitcoin 2.1.0 (test version because I don't have the production version). Lots of failures, and not just due to testnet vs mainnet stuff. This used to pass.
I ran test suite against leger bitcoin 2.1.3 and it had some errors and a failure. I'm not sure the scope of this PR as it's missing a description, but are these supposed to be fixed?
Test 27-29 Error
Confirm the wallet policy
Ledger device: Invalid data received (0x6a80)
Test 30
Confirm the wallet policy: "mainnet P2SH 2-of-2 multisig address"e;
Name: 2-of-2 P2SH mainnet wallet
Expected token: ebe7154ae75d4117d9da0525a6e134881f4b171671724a24ce00d7569be9a16c
REGISTERING QUORUM
Key 0
xpub: xpub6Ca5CwTgRASgkXbXE5TeddTP9mPCbYHreCpmGt9dhz9y6femstHGCoFESHHKKRcm414xMKnuLjP9LDS7TwaJC9n5gxua6XB1rwPcC6hqDub
xfp: 00000004
Path: m/45'/0'/100'
Key 1
xpub: xpub6CCHViYn5VzPfSR7baop9FtGcbm3UnqHwa54Z2eNvJnRFCJCdo9HtCYoLJKZCoATMLUowDDA1BMGfQGauY3fDYU3HyMzX4NDkoLYCSkLpbH
xfp: f57ec65d
Path: m/45'/0'/100'
Test failed
package.json
Outdated
| @@ -144,7 +148,7 @@ | |||
| "redux-thunk": "^2.3.0", | |||
| "reselect": "^4.0.0", | |||
| "sass": "^1.56.2", | |||
| "unchained-bitcoin": "^1.0.1", | |||
| "unchained-bitcoin": "^1.1.0", | |||
There was a problem hiding this comment.
This can stay as-is, but has to be merged before #358 else update this to v1.2.0
There was a problem hiding this comment.
I think the errors you're seeing are because one more uc-bitcoin bump is needed that updates the fixtures. I'll add that in.
There was a problem hiding this comment.
I can get test 30 to pass now, but 27-29 still error.
| optimizeDeps: { | ||
| include: [ | ||
| /* | ||
| Add packages you want to develop locally with caravan here | ||
| for example, uncomment the ones below if working on either package | ||
| */ | ||
| // "unchained-bitcoin", | ||
| // "unchained-wallets", | ||
| ], | ||
| }, |
There was a problem hiding this comment.
This also requires a counterpart in:
commonjsOptions: {
include: [/unchained-bitcoin/, /unchained-wallets/],
},
There was a problem hiding this comment.
It's weird that I was able to get things to work without that.
bucko13
force-pushed
the
fix/unchained-wallets-integration
branch
from
0e8fe89 to
8c826b6
Compare
Ok, these should be resolved now with the uc-bitcoin version bump |
Still seeing hardware transport errors on 27-29 using v2.1.3. |
Are you using the bitcoin testnet app or mainnet? I also get transport errors attempting to use the bitcoin mainnet app but the testnet app works. |
Mainnet 2.0.6 works for them, Mainnet 2.1.3 fails. I don't have results for Testnet 2.1.0 on-hand as I stopped running tests when it was displaying significant failures earlier on. |
|
The following test results are for ledger testnet 2.2.0-beta and mainnet 2.1.3. I cleared out node_modules and reinstalled everything. The one failing test is expected as ledger no longer accepts the weirdly long paths. In the future we want to add support in the test suite for noting these types of exceptions.
|
Tests 27-29 are for testnet. So it's kind of weird actually that 2.0.6 was working for them, which means I think this is expected behavior now (maybe a bug in the older ledger app). |
I get the same results |
Description
Does this PR introduce a breaking change?
Does this PR fix an open issue?