[Cashtab] [BCH Deprecation] [HD Node] Pt 1/3 - Implement separate masterHDNode creation logic
ClosedPublic
Actions

Authored by emack on Nov 2 2022, 08:45.

Details

Reviewers

bytesofman

Group Reviewers

Restricted Project

Commits

rABC3f7074f7f76c: [Cashtab] [BCH Deprecation] [HD Node] Pt 1/3 - Implement separate masterHDNode…

Summary

T2730

This is part 1 of the 3 part diff to deprecate the use of BCH-JS' BCH.HDNode object for generating the masterHDNode and downstream parameters.

Pt 1/3 - Implement separate masterHDNode creation logic
Pt 2/3 - Localize downstream logic from masterHDNode (add and compare local outputs for fromSeed, derivePath, toPublicKey, toCashaddress & toWIF)
Pt 3/3 - Deprecate BCH.HDNode (fromSeed, derivePath, toPublicKey, toCashAddress & toWIF)

Relies on the bitcoincashjs-lib library (115KB) (bch fork of bitcoinjs-lib). I originally tried to implement this from the ground up but the end to end logic to generate masterHDNode involves ~15 separate libraries (ecurve, secp256k1, create-hash...etc) which wasn't a good ROI.
With bitcoincashjs-lib it at least enables us to continue with the BCH-JS (1.29MB) deprecation process and we can revisit when there's a better alternative.

Note: bitcoincashjs-lib and coininfo libs will also be used to deprecate BCH.TransactionBuilder() in an unrelated diff.

Test Plan

npm install
npm test
npm start
create a new wallet and observe the 'getWalletDetails(): BCH-JS masterHDNode matches localMasterHDNode from bitcoincashjs-lib' console log message

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

emack created this revision.Nov 2 2022, 08:45

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 2 2022, 08:45

emack requested review of this revision.Nov 2 2022, 08:45

Harbormaster completed remote builds in B20798: Diff 36120.Nov 2 2022, 08:48

emack edited the summary of this revision. (Show Details)Nov 2 2022, 11:03

bytesofman requested changes to this revision.Nov 5 2022, 04:51

bytesofman added inline comments.

web/cashtab/package.json
9 ↗	(On Diff #36120)	Seems like `https://github.com/bitcoinjs/bitcoinjs-lib` is much better maintained compared to `bitcoincashjs-lib`, though larger. Worth reviewing the specific function in each to see if one has any clear advantages or if we specifically need the `bitcoincashjs-lib` version for some reason. A background question to all of this is "what would make all of this easier for XEC devs going forward?" I don't think the approach used by bch-js (wrapping a bunch of dependencies and then throwing in an API) is the best answer for us. But it does seem like we can't immediately get away from a series of dependencies.
10 ↗	(On Diff #36120)	This seems overkill since it's just pulling in an object for one coin. We could just have that object in `Ticker.js` --- which would also allow us to make it XEC-specific instead of borrowing from another bch repo.
18 ↗	(On Diff #36120)	What is this used for?

This revision now requires changes to proceed.Nov 5 2022, 04:51

bytesofman mentioned this in D12433: [Cashtab] [proof of concept] Deprecating legacy address formats.Nov 8 2022, 10:54

Moved the coin object from psf/coininfo into app as a common component

re: bitcoinjs-lib, I tried to use it in place of bitcoincashjs-lib, however there are a few issues at play:

bitcoinjs-lib has a dependency on the bip32 library
bip32 now requires instantiation via the use of a BIP32Factory, which has an additional an additional dependency on 'tiny-secp256k1' (see https://github.com/bitcoinjs/bip32)
the 'tiny-secp256k1' library has issues with react apps (see https://stackoverflow.com/questions/72350903/error-using-react-native-using-bitcoinjs-lib-and-ecpair-tiny-secp256k1).
the suggestion as per page above is to use the react compatible fork of bitcoinjs-lib (https://github.com/novalabio/react-native-bitcoinjs-lib), which was last updated 6 years ago, which is even less maintained than bitcoincashjs-lib.
hence I'm proposing to stick with @psf/bitcoincashjs-lib