Heath Ledger: Add Ledger wallet support #205
Conversation
Feeling kinda documenting, might delete later.
The 0x API's are generally of a higher engineering quality. The exceptions encapsulate the error cases better, and they also manage the Ledger Transport lifecycle, which was a flaw of the previous implementation. We use them in keep-core too.
We use @0x/subproviders for the Ledger/Trezor integration. @ledgerhq/hw-app-eth is being upgraded as an older version was installed by accident.
"Stolen" from the Compound dApp. We'll leave these here as a placeholder, until we get the real UX in.
This introduces a custom LedgerSubprovider, which mitigates the existing 0x LedgerSubprovider's failure to produce the correct transaction signature values.
Colocate connector with other wallet info
We don't need this anymore - it was only supported by InjectedConnector and didn't appear to be useful for our case anyways.
liamzebedee
force-pushed
the
heath-ledger
branch
from
7858457 to
0abb63d
Compare
This is simpler and a bit more efficient.
liamzebedee
force-pushed
the
heath-ledger
branch
from
0abb63d to
52e8012
Compare
liamzebedee
force-pushed
the
heath-ledger
branch
from
52e8012 to
21c23d1
Compare
Removed unused variables, used a common functions.
Simplify default assignment and use specific type for param in jsdoc
Add Trezor wallet connector
|
Let's get this dooonnnnneeeee |
|
Added a |
|
@Shadowfiend, @ironng could you double-check if everything works, please? |
|
I probably won't have a chance to do it before merging (though I'll test-drive it on Ropsten). We do still need to do a tbtc.js version bump, which means we'll need to bump the dependency, which means to release this we'll also need a tbtc contract release. |
|
Yeah, I see. For local dev, we can use |
|
That will work with a local network, but the current version of tbtc.js is incompatible with the current version of Ropsten* tbtc contracts due to some function name changes. We're about to land a handful of additional such changes today or Monday, and will need to update tbtc.js one more time---then we can do a full deploy. |
Shadowfiend
dismissed
r-czajkowski’s stale review
I believe these have been attended to.
This PR implements Ledger hardware wallet support, cutting work out from #184. Due to 0x and ledgerjs API's not fully supporting chainId's greater than 255 (ie. our geth testnets), we had to implement a custom LedgerSubprovider for signing transactions. @r-czajkowski helped a lot with kickstarting the solution to this issue.
There's also some cleanup of the ChooseWalletDialog, based on previous feedback in #184.
Epic: #177
Depends on: keep-network/tbtc.js#55