Skip to content

Commit 3910b3b

Browse files
davidkaplanbitgodavidkaplanbitgo
authored
Merge pull request #6436 from BitGo/BTC-2286
Bump BitGo's fork for bitcoinjs-lib and improve the functionality for wrapping safe signing
2 parents 77950e6 + ce2cc90 commit 3910b3b

File tree

10 files changed

+33
-100
lines changed

10 files changed

+33
-100
lines changed

‎modules/abstract-utxo/src/sign.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export function signAndVerifyPsbt(
6464
signerKeychain: utxolib.BIP32Interface,
6565
{
6666
isLastSignature,
67+
/** deprecated */
6768
allowNonSegwitSigningWithoutPrevTx,
6869
}: { isLastSignature: boolean; allowNonSegwitSigningWithoutPrevTx?: boolean }
6970
): utxolib.bitgo.UtxoPsbt | utxolib.bitgo.UtxoTransaction<bigint> {
@@ -85,11 +86,7 @@ export function signAndVerifyPsbt(
8586
}
8687

8788
try {
88-
utxolib.bitgo.withUnsafeNonSegwit(
89-
psbt,
90-
() => psbt.signInputHD(inputIndex, signerKeychain),
91-
!!allowNonSegwitSigningWithoutPrevTx
92-
);
89+
psbt.signInputHD(inputIndex, signerKeychain);
9390
debug('Successfully signed input %d of %d', inputIndex + 1, psbt.data.inputs.length);
9491
} catch (e) {
9592
return new InputSigningError<bigint>(inputIndex, { id: outputId }, e);
@@ -111,13 +108,7 @@ export function signAndVerifyPsbt(
111108

112109
const outputId = outputIds[inputIndex];
113110
try {
114-
if (
115-
!utxolib.bitgo.withUnsafeNonSegwit(
116-
psbt,
117-
() => psbt.validateSignaturesOfInputHD(inputIndex, signerKeychain),
118-
!!allowNonSegwitSigningWithoutPrevTx
119-
)
120-
) {
111+
if (!psbt.validateSignaturesOfInputHD(inputIndex, signerKeychain)) {
121112
return new InputSigningError(inputIndex, { id: outputId }, new Error(`invalid signature`));
122113
}
123114
} catch (e) {

‎modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export async function signTransaction<TNumber extends number | bigint>(
2727
txInfo: { unspents?: utxolib.bitgo.Unspent<TNumber>[] } | undefined;
2828
isLastSignature: boolean;
2929
signingStep: 'signerNonce' | 'cosignerNonce' | 'signerSignature' | undefined;
30+
/** deprecated */
3031
allowNonSegwitSigningWithoutPrevTx: boolean;
3132
pubs: string[] | undefined;
3233
cosignerPub: string | undefined;
@@ -47,19 +48,11 @@ export async function signTransaction<TNumber extends number | bigint>(
4748
isLastSignature = params.isLastSignature;
4849
}
4950

50-
const setSignerMusigNonceWithOverride = (
51-
psbt: utxolib.bitgo.UtxoPsbt,
52-
signerKeychain: utxolib.BIP32Interface,
53-
nonSegwitOverride: boolean
54-
) => {
55-
utxolib.bitgo.withUnsafeNonSegwit(psbt, () => psbt.setAllInputsMusig2NonceHD(signerKeychain), nonSegwitOverride);
56-
};
57-
5851
if (tx instanceof bitgo.UtxoPsbt && isTxWithKeyPathSpendInput) {
5952
switch (params.signingStep) {
6053
case 'signerNonce':
6154
assert(signerKeychain);
62-
setSignerMusigNonceWithOverride(tx, signerKeychain, params.allowNonSegwitSigningWithoutPrevTx);
55+
tx.setAllInputsMusig2NonceHD(signerKeychain);
6356
PSBT_CACHE.set(tx.getUnsignedTx().getId(), tx);
6457
return { txHex: tx.toHex() };
6558
case 'cosignerNonce':
@@ -80,7 +73,7 @@ export async function signTransaction<TNumber extends number | bigint>(
8073
// this instance is not an external signer
8174
assert(params.walletId, 'walletId is required for MuSig2 bitgo nonce');
8275
assert(signerKeychain);
83-
setSignerMusigNonceWithOverride(tx, signerKeychain, params.allowNonSegwitSigningWithoutPrevTx);
76+
tx.setAllInputsMusig2NonceHD(signerKeychain);
8477
const response = await coin.signPsbt(tx.toHex(), params.walletId);
8578
tx.combine(bitgo.createPsbtFromHex(response.psbt, coin.network));
8679
break;
@@ -102,7 +95,6 @@ export async function signTransaction<TNumber extends number | bigint>(
10295
assert(signerKeychain);
10396
signedTransaction = signAndVerifyPsbt(tx, signerKeychain, {
10497
isLastSignature,
105-
allowNonSegwitSigningWithoutPrevTx: params.allowNonSegwitSigningWithoutPrevTx,
10698
});
10799
} else {
108100
if (tx.ins.length !== params.txInfo?.unspents?.length) {

‎modules/utxo-bin/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
"@noble/curves": "1.8.1",
3636
"archy": "^1.0.0",
3737
"bech32": "^2.0.0",
38-
"bitcoinjs-lib": "npm:@bitgo-forks/[email protected].2",
38+
"bitcoinjs-lib": "npm:@bitgo-forks/[email protected].11",
3939
"bs58": "^5.0.0",
4040
"bs58check": "^2.1.2",
4141
"cashaddress": "^1.1.0",

‎modules/utxo-lib/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
"bip174": "npm:@bitgo-forks/[email protected]",
5555
"bip32": "^3.0.1",
5656
"bitcoin-ops": "^1.3.0",
57-
"bitcoinjs-lib": "npm:@bitgo-forks/[email protected].9",
57+
"bitcoinjs-lib": "npm:@bitgo-forks/[email protected].11",
5858
"bs58check": "^2.1.2",
5959
"cashaddress": "^1.1.0",
6060
"create-hash": "^1.2.0",

‎modules/utxo-lib/src/bitgo/PsbtUtil.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,14 @@ export function toPsbtBuffer(data: Buffer | string): Buffer {
133133
* This function allows signing or validating a psbt with non-segwit inputs those do not contain nonWitnessUtxo.
134134
*/
135135
export function withUnsafeNonSegwit<T>(psbt: Psbt, fn: () => T, unsafe = true): T {
136+
const prevValue = (psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT;
137+
const prevValueWarn = (psbt as any).__CACHE.__WARN_UNSAFE_SIGN_NONSEGWIT;
136138
(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT = unsafe;
137139
(psbt as any).__CACHE.__WARN_UNSAFE_SIGN_NONSEGWIT = !unsafe;
138140
try {
139141
return fn();
140142
} finally {
141-
(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT = false;
142-
(psbt as any).__CACHE.__WARN_UNSAFE_SIGN_NONSEGWIT = true;
143+
(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT = prevValue;
144+
(psbt as any).__CACHE.__WARN_UNSAFE_SIGN_NONSEGWIT = prevValueWarn;
143145
}
144146
}

‎modules/utxo-lib/src/bitgo/zcash/ZcashPsbt.ts‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ import {
44
getDefaultVersionGroupIdForVersion,
55
ZcashTransaction,
66
} from './ZcashTransaction';
7-
import { Network, PsbtTransaction, Signer } from '../../';
7+
import { Network, PsbtTransaction } from '../../';
88
import { Psbt as PsbtBase } from 'bip174';
99
import * as types from 'bitcoinjs-lib/src/types';
10-
import { ValidateSigFunction } from 'bitcoinjs-lib/src/psbt';
11-
import { ProprietaryKeySubtype, PSBT_PROPRIETARY_IDENTIFIER, withUnsafeNonSegwit } from '../PsbtUtil';
10+
import { ProprietaryKeySubtype, PSBT_PROPRIETARY_IDENTIFIER } from '../PsbtUtil';
1211
const typeforce = require('typeforce');
1312

1413
const CONSENSUS_BRANCH_ID_KEY = Buffer.concat([
@@ -124,17 +123,6 @@ export class ZcashPsbt extends UtxoPsbt<ZcashTransaction<bigint>> {
124123
this.tx.consensusBranchId = getDefaultConsensusBranchIdForVersion(network, version);
125124
}
126125

127-
// For Zcash transactions, we do not have to have non-witness UTXO data for non-segwit
128-
// transactions because zcash hashes the value directly. Thus, it is unnecessary to have
129-
// the previous transaction hash on the unspent.
130-
signInput(inputIndex: number, keyPair: Signer, sighashTypes?: number[]): this {
131-
return withUnsafeNonSegwit(this, super.signInput.bind(this, inputIndex, keyPair, sighashTypes));
132-
}
133-
134-
validateSignaturesOfInput(inputIndex: number, validator: ValidateSigFunction, pubkey?: Buffer): boolean {
135-
return withUnsafeNonSegwit(this, super.validateSignaturesOfInput.bind(this, inputIndex, validator, pubkey));
136-
}
137-
138126
private setPropertyCheckSignatures(propName: keyof ZcashTransaction<bigint>, value: unknown) {
139127
if (this.tx[propName] === value) {
140128
return;

‎modules/utxo-lib/src/testutil/psbt.ts‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import {
2424
UtxoPsbt,
2525
UtxoTransaction,
2626
verifySignatureWithUnspent,
27-
withUnsafeNonSegwit,
2827
} from '../bitgo';
2928
import { Network } from '../networks';
3029
import { mockReplayProtectionUnspent, mockWalletUnspent } from './mock';
@@ -115,32 +114,21 @@ export function signPsbtInput(
115114
params?: {
116115
signers?: { signerName: KeyName; cosignerName?: KeyName };
117116
deterministic?: boolean;
117+
// For backwards compatibility keep this here.
118118
skipNonWitnessUtxo?: boolean;
119119
}
120120
): void {
121-
function signPsbt(psbt: UtxoPsbt, signFunc: () => void, skipNonWitnessUtxo?: boolean) {
122-
if (skipNonWitnessUtxo) {
123-
withUnsafeNonSegwit(psbt, signFunc);
124-
} else {
125-
signFunc();
126-
}
127-
}
128-
129-
const { signers, deterministic, skipNonWitnessUtxo } = params ?? {};
121+
const { signers, deterministic } = params ?? {};
130122
const { signerName, cosignerName } = signers ? signers : getSigners(input.scriptType);
131123
if (sign === 'halfsigned') {
132124
if (input.scriptType === 'p2shP2pk') {
133-
signPsbt(psbt, () => psbt.signInput(inputIndex, rootWalletKeys[signerName]), skipNonWitnessUtxo);
125+
psbt.signInput(inputIndex, rootWalletKeys[signerName]);
134126
} else {
135-
signPsbt(psbt, () => psbt.signInputHD(inputIndex, rootWalletKeys[signerName]), skipNonWitnessUtxo);
127+
psbt.signInputHD(inputIndex, rootWalletKeys[signerName]);
136128
}
137129
}
138130
if (sign === 'fullsigned' && cosignerName && input.scriptType !== 'p2shP2pk') {
139-
signPsbt(
140-
psbt,
141-
() => psbt.signInputHD(inputIndex, rootWalletKeys[cosignerName], { deterministic }),
142-
skipNonWitnessUtxo
143-
);
131+
psbt.signInputHD(inputIndex, rootWalletKeys[cosignerName], { deterministic });
144132
}
145133
}
146134

@@ -156,12 +144,13 @@ export function signAllPsbtInputs(
156144
params?: {
157145
signers?: { signerName: KeyName; cosignerName?: KeyName };
158146
deterministic?: boolean;
147+
// For backwards compatibility keep this here.
159148
skipNonWitnessUtxo?: boolean;
160149
}
161150
): void {
162-
const { signers, deterministic, skipNonWitnessUtxo } = params ?? {};
151+
const { signers, deterministic } = params ?? {};
163152
inputs.forEach((input, inputIndex) => {
164-
signPsbtInput(psbt, input, inputIndex, rootWalletKeys, sign, { signers, deterministic, skipNonWitnessUtxo });
153+
signPsbtInput(psbt, input, inputIndex, rootWalletKeys, sign, { signers, deterministic });
165154
});
166155
}
167156

‎modules/utxo-lib/test/bitgo/psbt/Psbt.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import {
3232
clonePsbtWithoutNonWitnessUtxo,
3333
deleteWitnessUtxoForNonSegwitInputs,
3434
getPsbtInputScriptType,
35-
withUnsafeNonSegwit,
3635
getTransactionAmountsFromPsbt,
3736
WalletUnspent,
3837
getDefaultSigHash,
@@ -289,18 +288,14 @@ function runBuildSignSendFlowTest(
289288
hex = psbtWithoutPrevTx.toHex();
290289

291290
psbtAtHsm = createPsbtFromHex(hex, network);
292-
withUnsafeNonSegwit(psbtAtHsm, () => {
293-
testutil.signAllPsbtInputs(psbtAtHsm, inputs, rootWalletKeys, 'fullsigned', {
294-
signers: {
295-
signerName: 'user',
296-
cosignerName: 'bitgo',
297-
},
298-
deterministic: true,
299-
});
300-
});
301-
withUnsafeNonSegwit(psbtAtHsm, () => {
302-
assertValidate(psbtAtHsm);
291+
testutil.signAllPsbtInputs(psbtAtHsm, inputs, rootWalletKeys, 'fullsigned', {
292+
signers: {
293+
signerName: 'user',
294+
cosignerName: 'bitgo',
295+
},
296+
deterministic: true,
303297
});
298+
assertValidate(psbtAtHsm);
304299
hexAtHsm = psbtAtHsm.toHex();
305300

306301
psbtFromHsm = createPsbtFromHex(hexAtHsm, network);

‎modules/utxo-lib/test/bitgo/psbt/ZcashPsbt.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,11 @@ describe('Zcash PSBT', function () {
4545
describe('should be able to sign the transaction', function () {
4646
it('can sign the inputs', async function () {
4747
psbt.signAllInputsHD(rootWalletKeys.user);
48-
assert(!(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT);
4948
psbt.signAllInputsHD(rootWalletKeys.bitgo);
50-
assert(!(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT);
5149
});
5250

5351
it('can validate the signatures on the unspents', async function () {
5452
psbt.validateSignaturesOfAllInputs();
55-
assert(!(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT);
5653
});
5754

5855
it('can finalize and extract the transaction', async function () {

‎yarn.lock‎

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7459,11 +7459,6 @@ bip174@=2.1.1, bip174@^2.1.1:
74597459
resolved "https://registry.npmjs.org/bip174/-/bip174-2.1.1.tgz#ef3e968cf76de234a546962bcf572cc150982f9f"
74607460
integrity sha512-mdFV5+/v0XyNYXjBS6CQPLo9ekCx4gtKZFnJm5PMto7Fs9hTTDpkkzOB7/FtluRI6JbUUAu+snTYfJRgHLZbZQ==
74617461

7462-
"bip174@npm:@bitgo-forks/[email protected]":
7463-
version "3.0.0-rc.1"
7464-
resolved "https://registry.npmjs.org/@bitgo-forks/bip174/-/bip174-3.0.0-rc.1.tgz#6ca32dc75513744a9ae914255f915494056541b4"
7465-
integrity sha512-eGi5die7Q7O3yPtkcGF1gD7qLlJLiLnYI4DpFTF6tUhUo71gy3RoXAAeeJA2fLpnVoJofXnLdLfpcO6OEZAsvw==
7466-
74677462
"bip174@npm:@bitgo-forks/[email protected]":
74687463
version "3.1.0-master.4"
74697464
resolved "https://registry.npmjs.org/@bitgo-forks/bip174/-/bip174-3.1.0-master.4.tgz#0c8a750cc98097120169e753e7fccc23b1cb9eaa"
@@ -7528,26 +7523,10 @@ bitcoinjs-lib@^6.1.5, bitcoinjs-lib@^6.1.7:
75287523
typeforce "^1.11.3"
75297524
varuint-bitcoin "^1.1.2"
75307525

7531-
"bitcoinjs-lib@npm:@bitgo-forks/[email protected]":
7532-
version "7.1.0-master.2"
7533-
resolved "https://registry.npmjs.org/@bitgo-forks/bitcoinjs-lib/-/bitcoinjs-lib-7.1.0-master.2.tgz#8b5760fd7d7f6ca526f1fbee93bc74a6ea75ac6d"
7534-
integrity sha512-D62U1pWej8M+7gROykLGGPvFf0zFal10kAAbuaHuy1ohtwKLNHRiUz8dpdjEZBzNkF+pU+GQhItdiEJTslK4/A==
7535-
dependencies:
7536-
bech32 "^2.0.0"
7537-
bip174 "npm:@bitgo-forks/[email protected]"
7538-
bs58check "^2.1.2"
7539-
create-hash "^1.1.0"
7540-
fastpriorityqueue "^0.7.1"
7541-
json5 "^2.2.3"
7542-
ripemd160 "^2.0.2"
7543-
typeforce "^1.11.3"
7544-
varuint-bitcoin "^1.1.2"
7545-
wif "^2.0.1"
7546-
7547-
"bitcoinjs-lib@npm:@bitgo-forks/[email protected]":
7548-
version "7.1.0-master.9"
7549-
resolved "https://registry.npmjs.org/@bitgo-forks/bitcoinjs-lib/-/bitcoinjs-lib-7.1.0-master.9.tgz#e2fa6b9a12106980772899f7409754244ec0fe35"
7550-
integrity sha512-iWDwsRZK30tW7jtBiOlOJhuJzZ2THIvdvREBHM2cHG669nGQxNPxpYXmk0Lgm5x4jG4GN1dM6dPR6Car6EtDsA==
7526+
"bitcoinjs-lib@npm:@bitgo-forks/[email protected]":
7527+
version "7.1.0-master.11"
7528+
resolved "https://registry.npmjs.org/@bitgo-forks/bitcoinjs-lib/-/bitcoinjs-lib-7.1.0-master.11.tgz#49b9c31b5a973b08c34c9dc468ac241c9416e18a"
7529+
integrity sha512-Yyh67I26iI7FGqPBY7rxqHZ9FM9JuouAsViQocrr7URhRpuZEWVsM/oMTNbMnRw2cPFj4jWKhRDLadgrUk2HEQ==
75517530
dependencies:
75527531
bech32 "^2.0.0"
75537532
bip174 "npm:@bitgo-forks/[email protected]"

0 commit comments

Comments
 (0)