Sync up with bitcoinjs-lib 6 #2054

brandonblack · 2022-02-26T00:27:29Z

This is a pretty major change:

TransactionBuilder, classify, and related are no longer part of bitcoinjs-lib, so they're moved into utxo-lib.
tiny-secp256k1 no longer works in browser w/o WASM support, so switched crypto implementation to noble-secp256k1
- Schnorr signatures changed because lack of extra nonce data is now empty buffer instead of skipped
- I made some moderately extensive changes to noble-secp256k1 Flesh out Schnorr/BIP340 functionality paulmillr/noble-secp256k1#50

OttoAllmendinger

looks good overall

please use https://www.conventionalcommits.org/en/v1.0.0/ for commits and mention which ref or tag you pulled the TransactionBuilder code from in the upstream repo

OttoAllmendinger · 2022-02-26T11:00:01Z

modules/utxo-lib/src/networks.ts

-  bech32?: string;
+  bech32: string;


why do we need this?

To match the Network type from bitocinjs-lib I'd love a better idea if you have one. I considered changing the upstream type in my "BitGo specific" commit, but it looked like it would become a maintenance pain.

OttoAllmendinger · 2022-02-26T11:00:37Z

modules/utxo-lib/test/bitgo/fixtures/signature/bitcoin/p2tr/parsed-backup-bitgo-fullSigned.json

-    "9f74390b6c7f5988772a909fc689a3e2dd680ceb8d54d4caffdd5907132bc415d7d964f4b25ce02690938cd4b62057f80b929aca4155c64db205a336b2e6ce79",
-    "69a969c8df93bc7894d045bc5e5d73f50d815fed7f703f11080f6b46561220156a0eed3382ec0eddbfa2737a29a7b07ba572880ae26f05f56ebca40bd7a5a987"
+    "a2074b68b115e970c8c4bee9cf75c03b82bd3e3086d58801a4adb7b31b9b5f2cea4a5615a4feef74b61db6d765bb814feaf27331fad128dda55cfe106d25d44e",
+    "6bace9d49a0a7617e8ab40cd5361257829c0f09809d6f70ce130a70d7af3de1bf7409cbe379481e0494b32531b955fff67825cd969a3e95fc853c048692feda1"


why did this change?

See the PR description:

Schnorr signatures changed because lack of extra nonce data is now empty buffer instead of skipped`

OttoAllmendinger · 2022-02-26T11:02:08Z

modules/utxo-lib/test/networks.ts

-        assert.strictEqual(typeof (network as any).bech32, bech32Coins(network) ? 'string' : 'undefined');
+        assert.strictEqual(typeof (network as any).bech32,  'string');
+        if (!bech32Coins(network)) {
+          assert.strictEqual((network as any).bech32.length, 0);
+        }


IMO undefined is better than zero-length string here

No disagreement, but that's not what the upstream type has.

changed it so we do not use the upstream type anymore: 6f553c0

brandonblack · 2022-02-27T15:13:41Z

Improved commit history, formatted code, and tidied up noble_ecc.ts

codecov · 2022-02-28T15:45:20Z

Codecov Report

Merging #2054 (3392077) into master (737deab) will increase coverage by 85.92%.
The diff coverage is n/a.

❗ Current head 3392077 differs from pull request most recent head f0a9102. Consider uploading reports for the commit f0a9102 to get more accurate results

@@             Coverage Diff             @@
##           master    #2054       +/-   ##
===========================================
+ Coverage        0   85.92%   +85.92%     
===========================================
  Files           0      169      +169     
  Lines           0     8504     +8504     
  Branches        0     1275     +1275     
===========================================
+ Hits            0     7307     +7307     
- Misses          0      781      +781     
- Partials        0      416      +416

Flag	Coverage Δ
unit	`85.92% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
modules/account-lib/src/coin/algo/transaction.ts	`82.56% <0.00%> (ø)`
modules/account-lib/src/coin/xrp/utils.ts	`60.00% <0.00%> (ø)`
modules/account-lib/src/coin/hbar/keyPair.ts	`100.00% <0.00%> (ø)`
modules/account-lib/src/coin/celo/stakingUtils.ts	`100.00% <0.00%> (ø)`
...odules/account-lib/src/coin/celo/stakingBuilder.ts	`93.33% <0.00%> (ø)`
...unt-lib/src/coin/hbar/transactionBuilderFactory.ts	`82.35% <0.00%> (ø)`
modules/account-lib/src/coin/dot/txnSchema.ts	`100.00% <0.00%> (ø)`
...les/account-lib/src/coin/stx/transactionBuilder.ts	`85.13% <0.00%> (ø)`
...account-lib/src/coin/sol/stakingActivateBuilder.ts	`97.67% <0.00%> (ø)`
modules/account-lib/src/coin/cspr/constants.ts	`100.00% <0.00%> (ø)`
... and 159 more

These files were removed from bitcoinjs-lib in version 6, but we're not ready to give them up quite yet. Imported from BitGo/bitcionjs-lib/165f26ae18082158e531b607fcc7245ad641e263 Issue: BG-41154

Issue: BG-41154

* Schnorr signatures changed because lack of extra nonce data is now empty buffer instead of skipped * tiny-secp256k1 no longer works in browser w/o WASM support, so switched crypto implementation to noble-secp256k1 * Required some moderately extensive changes to noble-secp256k1 paulmillr/noble-secp256k1#50 Issue: BG-41154

Let's ignore this rule for now since it makes importing bitcoinjs-lib code more difficult. Issue: BG-41154

Mostly braces around `if` blocks Issue: BG-41154

Since TransactionBuilder now lives in `utxo-lib`, we can change the interface and remove the dependency on `bitcoinjs-lib.Network` Issue: BG-41154

Not required for updated utxolib/bitcoinjs-lib Issue: BG-41154

Ideally dependent packages should define those themselves but we will use this shortcut for now so we do not have to depend on the custom secp256k1 package everywhere. Issue: BG-41154

* bump `bip32` package to v3 * use `bip32: BIP32API` type from `@bitgo/utxo-lib` * use `bip32: BIP32Interface` type from `bip32` package This has the unfortunate downside that we are depending on `@bitgo/utxo-lib` for bip32 again, after we removed this dependency in earlier commits. We should fix this by instantiating `bip32: BIP32API` per package. However this is not easy to do here since we are depending on a slightly exotic secp256k1 library in `@bitgo/utxo-lib` and I don't want to depend on that directly in `BitGoJS` yet. Issue: BG-41154

P2TR/Schorr transaction changes due to change in extra nonce data See f185efebfe330f1ff786694b2703d76ebb87e71a for details Issue: BG-41154

Issue: BG-41154

P2TR/Schorr transaction changes due to change in extra nonce data See f185efebfe330f1ff786694b2703d76ebb87e71a for details Issue: BG-41154

Issues: BG-41154

github-actions · 2022-05-26T02:15:55Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2022-06-06T02:17:46Z

This PR was closed because it has been stalled for 10 days with no activity.

brandonblack requested review from OttoAllmendinger and sangaman February 26, 2022 00:27

brandonblack requested a review from tylerlevine as a code owner February 26, 2022 00:27

OttoAllmendinger requested changes Feb 26, 2022

View reviewed changes

brandonblack force-pushed the bitcoinjs_lib_6_sync branch from bbfb6cf to f185efe Compare February 27, 2022 15:12

brandonblack requested review from CapnMigraine and argjv as code owners February 28, 2022 14:33

OttoAllmendinger force-pushed the bitcoinjs_lib_6_sync branch 2 times, most recently from 0b62e1f to e30785c Compare February 28, 2022 15:37

OttoAllmendinger self-requested a review February 28, 2022 15:42

OttoAllmendinger previously approved these changes Feb 28, 2022

View reviewed changes

brandonblack dismissed OttoAllmendinger’s stale review via b9184b8 March 1, 2022 17:18

brandonblack force-pushed the bitcoinjs_lib_6_sync branch 2 times, most recently from 009762f to 3392077 Compare March 1, 2022 19:37

reardencode and others added 14 commits March 30, 2022 14:20

feat: import transaction_builder, classify, etc.

ff3f9e2

These files were removed from bitcoinjs-lib in version 6, but we're not ready to give them up quite yet. Imported from BitGo/bitcionjs-lib/165f26ae18082158e531b607fcc7245ad641e263 Issue: BG-41154

fix: Format imported files

7bc77ca

Issue: BG-41154

style: disable no-duplicate-imports

f2f891d

Let's ignore this rule for now since it makes importing bitcoinjs-lib code more difficult. Issue: BG-41154

fix: apply eslint --fix

357599d

Mostly braces around `if` blocks Issue: BG-41154

fix: remove references to bitcoinjs.Network

bef96bc

Since TransactionBuilder now lives in `utxo-lib`, we can change the interface and remove the dependency on `bitcoinjs-lib.Network` Issue: BG-41154

fix(unspents): remove bitcoinjs-lib.d.ts

a3174ed

Not required for updated utxolib/bitcoinjs-lib Issue: BG-41154

feat(utxo-lib): export ECPairAPI, BIP32API

c601a50

Ideally dependent packages should define those themselves but we will use this shortcut for now so we do not have to depend on the custom secp256k1 package everywhere. Issue: BG-41154

fix(core): regenerate p2tr test vectors

af46b95

P2TR/Schorr transaction changes due to change in extra nonce data See f185efebfe330f1ff786694b2703d76ebb87e71a for details Issue: BG-41154

fix(unspents): use new bip32/BIP32Interface in tests

5d3359a

Issue: BG-41154

fix(utxo-bin): fix test fixtures

29f6851

P2TR/Schorr transaction changes due to change in extra nonce data See f185efebfe330f1ff786694b2703d76ebb87e71a for details Issue: BG-41154

chore: update deps

c25d8ed

Issues: BG-41154

fix(core): Fix Avalanch for new BIP32

f0a9102

Issues: BG-41154

brandonblack force-pushed the bitcoinjs_lib_6_sync branch from 3392077 to f0a9102 Compare March 30, 2022 21:28

github-actions bot added the stale label May 26, 2022

github-actions bot closed this Jun 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync up with bitcoinjs-lib 6 #2054

Sync up with bitcoinjs-lib 6 #2054

brandonblack commented Feb 26, 2022

OttoAllmendinger left a comment

OttoAllmendinger Feb 26, 2022

brandonblack Feb 27, 2022

OttoAllmendinger Feb 26, 2022

brandonblack Feb 27, 2022

OttoAllmendinger Feb 26, 2022

brandonblack Feb 27, 2022

OttoAllmendinger Feb 28, 2022

brandonblack commented Feb 27, 2022

codecov bot commented Feb 28, 2022 •

edited

Loading

github-actions bot commented May 26, 2022

github-actions bot commented Jun 6, 2022

Sync up with bitcoinjs-lib 6 #2054

Sync up with bitcoinjs-lib 6 #2054

Conversation

brandonblack commented Feb 26, 2022

OttoAllmendinger left a comment

Choose a reason for hiding this comment

OttoAllmendinger Feb 26, 2022

Choose a reason for hiding this comment

brandonblack Feb 27, 2022

Choose a reason for hiding this comment

OttoAllmendinger Feb 26, 2022

Choose a reason for hiding this comment

brandonblack Feb 27, 2022

Choose a reason for hiding this comment

OttoAllmendinger Feb 26, 2022

Choose a reason for hiding this comment

brandonblack Feb 27, 2022

Choose a reason for hiding this comment

OttoAllmendinger Feb 28, 2022

Choose a reason for hiding this comment

brandonblack commented Feb 27, 2022

codecov bot commented Feb 28, 2022 • edited Loading

Codecov Report

github-actions bot commented May 26, 2022

github-actions bot commented Jun 6, 2022

codecov bot commented Feb 28, 2022 •

edited

Loading