[Cashtab] Implementing bitgo/utxo-lib as POC
AbandonedPublic
Actions

Authored by bytesofman on Jan 4 2023, 17:36.

Details

Reviewers

emack

Group Reviewers

Restricted Project

Summary

T2893

This diff implements @bitgo/utxo-lib transaction builder. Exact raw txs of previous unit tests are recreated. Some shims are needed due to lack of full support in @bitgo/utxo-lib. However, @bitgo/utxo-lib is better maintained and tested than the current state of ecashjs-lib.

So, this diff should be the starting point for organizing a recommended dev stack.

This diff itself is the last proof of concept -- one that is actually working. It will need to be recast as a stacked diff to explain the various shims and steps that are necessary to get this to work. Also as part of the stack, existing ecashjs-lib and its dependencies should be deprecated in favor of @bitgo/utxo-lib

Related PR for adding this support to Bitgo library: https://github.com/BitGo/BitGoJS/pull/3160

Test Plan

No teamcity errors
npm start
Send XEC tx, etoken tx, message tx, encrypted msg tx

Diff Detail

Repository

rABC Bitcoin ABC

Branch

bitgo-implementation-take-3

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21696
Build 43032: Build Diff	cashtab-tests
Build 43031: arc lint + arc unit

Event Timeline

bytesofman created this revision.Jan 4 2023, 17:36

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 4 2023, 17:36

bytesofman requested review of this revision.Jan 4 2023, 17:36

Harbormaster completed remote builds in B21696: Diff 37459.Jan 4 2023, 17:39

In the description, would be useful to mention where the upstream source repo is. I had to google to find https://github.com/BitGo/BitGoJS/tree/master/modules/utxo-lib

bytesofman edited the summary of this revision. (Show Details)Jan 6 2023, 11:13

emack added a subscriber: emack.Jan 6 2023, 13:20

emack added inline comments.

web/cashtab/src/utils/__tests__/cashMethods.test.js
1036	I read through your PR #3160 (feat: support ecash transactions) in the bitgo repo and didn't see anything relating to native support of ecash addresses. I'm assuming this wouldn't be achieved through your subsequent stack but rather as a follow up PR to the bitgo repo? Otherwise needing to convert back to legacy formats again is a bit of a step backwards from the recent refactors.
web/cashtab/src/utils/cashMethods.js
704	what was the reason for removing this check? It's a separate validation of the inputUtxos length above.

bytesofman added inline comments.Jan 6 2023, 23:20

web/cashtab/src/utils/__tests__/cashMethods.test.js
1036	They do have `cashaddress` support in their library, so this probably can be added to the repo. In the meantime, we already had the shim in Cashtab's `TxBuilder.js`, so I didn't see moving it from their to `cashMethods.js` as too big a step back. imo worth it to lose the shim, though the goal should still be to get rid of it.
web/cashtab/src/utils/cashMethods.js
704	It probably should be preserved. The reason it was removed is because the new `TransactionBuilder` has a different format, and this check doesn't exist on it, so it just failed every time. If we end up implementing this library, I will amend to follow the correct format. However, based on the discussion in Bitgo, it's possible will upgrade Cashtab directly to the newer `PSBT` object instead of `TransactionBuilder`, which is being deprecated.

emack accepted this revision.Jan 15 2023, 23:37

This revision is now accepted and ready to land.Jan 15 2023, 23:37

marking this as changes planned since do not plan to land this in its current state. I will though use this as a starting point to test the PSBT (what replaced the transaction builder in bitcoinjs-lib) approach.

PSBT description, for reference: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki

Superseded by D13679