Skip to content

Commit

Permalink
Merge pull request #4136 from BitGo/BTC-721-override-signing
Browse files Browse the repository at this point in the history
feat: add flag to override signing for non-segwit transactions
  • Loading branch information
davidkaplanbitgo authored Dec 11, 2023
2 parents 6e8b84c + 2bcdaf0 commit b578210
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
22 changes: 19 additions & 3 deletions modules/abstract-utxo/src/abstractUtxoCoin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ type UtxoBaseSignTransactionOptions<TNumber extends number | bigint = number> =
* When false, creates half-signed transaction with placeholder signatures.
*/
isLastSignature?: boolean;
/**
* If true, allows signing a non-segwit input with a witnessUtxo instead requiring a previous
* transaction (nonWitnessUtxo)
*/
allowNonSegwitSigningWithoutPrevTx?: boolean;
};

export type SignTransactionOptions<TNumber extends number | bigint = number> = UtxoBaseSignTransactionOptions<TNumber> &
Expand Down Expand Up @@ -1164,13 +1169,21 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
return signerKeychain;
};

const setSignerMusigNonceWithOverride = (
psbt: utxolib.bitgo.UtxoPsbt,
signerKeychain: utxolib.BIP32Interface,
nonSegwitOverride: boolean
) => {
utxolib.bitgo.withUnsafeNonSegwit(psbt, () => psbt.setAllInputsMusig2NonceHD(signerKeychain), nonSegwitOverride);
};

let signerKeychain: utxolib.BIP32Interface | undefined;

if (tx instanceof bitgo.UtxoPsbt && isTxWithKeyPathSpendInput) {
switch (params.signingStep) {
case 'signerNonce':
signerKeychain = getSignerKeychain();
tx.setAllInputsMusig2NonceHD(signerKeychain);
setSignerMusigNonceWithOverride(tx, signerKeychain, !!params.allowNonSegwitSigningWithoutPrevTx);
AbstractUtxoCoin.PSBT_CACHE.set(tx.getUnsignedTx().getId(), tx);
return { txHex: tx.toHex() };
case 'cosignerNonce':
Expand All @@ -1191,7 +1204,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
// this instance is not an external signer
assert(txPrebuild.walletId, 'walletId is required for MuSig2 bitgo nonce');
signerKeychain = getSignerKeychain();
tx.setAllInputsMusig2NonceHD(signerKeychain);
setSignerMusigNonceWithOverride(tx, signerKeychain, !!params.allowNonSegwitSigningWithoutPrevTx);
const response = await this.signPsbt(tx.toHex(), txPrebuild.walletId);
tx.combine(bitgo.createPsbtFromHex(response.psbt, this.network));
break;
Expand All @@ -1214,7 +1227,10 @@ export abstract class AbstractUtxoCoin extends BaseCoin {

let signedTransaction: bitgo.UtxoTransaction<bigint> | bitgo.UtxoPsbt;
if (tx instanceof bitgo.UtxoPsbt) {
signedTransaction = signAndVerifyPsbt(tx, signerKeychain, { isLastSignature });
signedTransaction = signAndVerifyPsbt(tx, signerKeychain, {
isLastSignature,
allowNonSegwitSigningWithoutPrevTx: params.allowNonSegwitSigningWithoutPrevTx,
});
} else {
if (tx.ins.length !== txPrebuild.txInfo?.unspents?.length) {
throw new Error('length of unspents array should equal to the number of transaction inputs');
Expand Down
19 changes: 16 additions & 3 deletions modules/abstract-utxo/src/sign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ export class TransactionSigningError<TNumber extends number | bigint = number> e
export function signAndVerifyPsbt(
psbt: utxolib.bitgo.UtxoPsbt,
signerKeychain: utxolib.BIP32Interface,
{ isLastSignature }: { isLastSignature: boolean }
{
isLastSignature,
allowNonSegwitSigningWithoutPrevTx,
}: { isLastSignature: boolean; allowNonSegwitSigningWithoutPrevTx?: boolean }
): utxolib.bitgo.UtxoPsbt | utxolib.bitgo.UtxoTransaction<bigint> {
const txInputs = psbt.txInputs;
const outputIds: string[] = [];
Expand All @@ -83,7 +86,11 @@ export function signAndVerifyPsbt(
}

try {
psbt.signInputHD(inputIndex, signerKeychain);
utxolib.bitgo.withUnsafeNonSegwit(
psbt,
() => psbt.signInputHD(inputIndex, signerKeychain),
!!allowNonSegwitSigningWithoutPrevTx
);
debug('Successfully signed input %d of %d', inputIndex + 1, psbt.data.inputs.length);
} catch (e) {
return new InputSigningError<bigint>(inputIndex, { id: outputId }, e);
Expand All @@ -105,7 +112,13 @@ export function signAndVerifyPsbt(

const outputId = outputIds[inputIndex];
try {
if (!psbt.validateSignaturesOfInputHD(inputIndex, signerKeychain)) {
if (
!utxolib.bitgo.withUnsafeNonSegwit(
psbt,
() => psbt.validateSignaturesOfInputHD(inputIndex, signerKeychain),
!!allowNonSegwitSigningWithoutPrevTx
)
) {
return new InputSigningError(inputIndex, { id: outputId }, new Error(`invalid signature`));
}
} catch (e) {
Expand Down
3 changes: 3 additions & 0 deletions modules/sdk-core/src/bitgo/wallet/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,9 @@ export class Wallet implements IWallet {
assert(k.pub);
return k.pub;
}),
// Building PSBTs with the bulk flag does not include the previous transaction for non-segwit inputs.
// Manually override the signing and validating to not fail.
allowNonSegwitSigningWithoutPrevTx: !!params.bulk,
};

const txPrebuilds = Array.isArray(buildResponse) ? buildResponse : [buildResponse];
Expand Down

0 comments on commit b578210

Please sign in to comment.