feat:add buildMultisigTranx skeleton and eport it - #36
Conversation
RiH-137
commented
Mar 22, 2026
RiH-137
commented
- Added the new skeleton function with strong typing and guardrails in btc-controller/src/helper/buildMultiSigTransaction.ts.
- Exported it from the helper barrel in btc-controller/src/helper/index.ts.
- Exported it from the top-level module in btc-controller/src/index.ts.
- Build output generated matching compiled artifacts:
- btc-controller/src/helper/buildMultiSigTransaction.js
- btc-controller/src/helper/buildMultiSigTransaction.d.ts
There was a problem hiding this comment.
Pull request overview
Adds a new multisig transaction builder helper to the btc-controller package and wires it through the helper barrel and top-level module exports so downstream consumers can import it.
Changes:
- Introduces
buildMultiSigTransaction(PSBT-building skeleton) with new option/result types. - Re-exports the helper from
src/helper/index.*andsrc/index.*. - Adds compiled JS and
.d.tsartifacts for the new helper and updates existing compiled export barrels.
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| btc-controller/src/index.ts | Re-export buildMultiSigTransaction from the top-level module. |
| btc-controller/src/index.js | Compiled export surface updated to include buildMultiSigTransaction. |
| btc-controller/src/index.d.ts | Type export surface updated to include buildMultiSigTransaction. |
| btc-controller/src/helper/index.ts | Exports buildMultiSigTransaction from the helper barrel. |
| btc-controller/src/helper/index.js | Compiled helper barrel updated to export buildMultiSigTransaction. |
| btc-controller/src/helper/index.d.ts | Helper barrel typings updated to export buildMultiSigTransaction. |
| btc-controller/src/helper/buildMultiSigTransaction.ts | New multisig PSBT builder skeleton implementation and types. |
| btc-controller/src/helper/buildMultiSigTransaction.js | Compiled JS output for the new helper. |
| btc-controller/src/helper/buildMultiSigTransaction.d.ts | Generated typings for the new helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const totalInputValue = utxos.reduce((sum, utxo) => sum + utxo.value, 0); | ||
| const totalOutputValue = outputs.reduce((sum, output) => sum + output.value, 0); | ||
| const changeAmount = totalInputValue - totalOutputValue - feeInSats; | ||
|
|
||
| if (changeAmount < 0) { | ||
| throw new Error("Insufficient input value to cover outputs and fee"); | ||
| } | ||
|
|
||
| // Skeleton behavior: add change output only when a valid change target is supplied. | ||
| if (changeAddress && changeAmount > 0) { | ||
| psbt.addOutput({ address: changeAddress, value: changeAmount }); | ||
| } |
There was a problem hiding this comment.
If changeAmount > 0 and changeAddress is not provided, the PSBT will implicitly pay changeAmount as additional fee (on top of feeInSats). That can lead to unintentionally overpaying fees / losing funds. Consider requiring changeAddress whenever changeAmount > 0, or add an explicit option (e.g., allowExtraFee) to acknowledge this behavior, and throw otherwise.
| const { | ||
| network, | ||
| utxos, | ||
| outputs, | ||
| requiredSignatures, | ||
| totalSigners, | ||
| feeInSats = 0, | ||
| changeAddress, | ||
| } = options; | ||
|
|
||
| if (requiredSignatures < 1 || totalSigners < 1 || requiredSignatures > totalSigners) { | ||
| throw new Error("Invalid multisig signer configuration"); | ||
| } |
There was a problem hiding this comment.
feeInSats is not validated. A negative or non-integer fee will make changeAmount larger and can result in outputs exceeding inputs, producing an invalid transaction later. Recommend validating feeInSats is a finite, non-negative integer (and similarly validating utxo.value / output.value).
| outputs.forEach((output) => { | ||
| psbt.addOutput({ address: output.address, value: output.value }); | ||
| }); | ||
|
|
||
| const totalInputValue = utxos.reduce((sum, utxo) => sum + utxo.value, 0); | ||
| const totalOutputValue = outputs.reduce((sum, output) => sum + output.value, 0); | ||
| const changeAmount = totalInputValue - totalOutputValue - feeInSats; | ||
|
|
||
| if (changeAmount < 0) { | ||
| throw new Error("Insufficient input value to cover outputs and fee"); | ||
| } | ||
|
|
||
| // Skeleton behavior: add change output only when a valid change target is supplied. | ||
| if (changeAddress && changeAmount > 0) { | ||
| psbt.addOutput({ address: changeAddress, value: changeAmount }); | ||
| } | ||
|
|
||
| return { | ||
| psbt, | ||
| psbtBase64: psbt.toBase64(), | ||
| inputCount: utxos.length, | ||
| outputCount: psbt.txOutputs.length, | ||
| totalInputValue, | ||
| totalOutputValue, | ||
| changeAmount, | ||
| }; |
There was a problem hiding this comment.
totalOutputValue is computed from the caller-provided outputs only, but outputCount is derived from psbt.txOutputs.length (which includes the change output when added). This makes the returned totals inconsistent with the PSBT contents. Consider either (a) including the change output in totalOutputValue, or (b) renaming to clarify it excludes change, and/or returning both requested vs actual output totals.
| export interface BuildMultiSigTransactionOptions { | ||
| network: bitcoin.networks.Network; | ||
| utxos: MultiSigInputUTXO[]; | ||
| outputs: MultiSigOutput[]; | ||
| requiredSignatures: number; | ||
| totalSigners: number; | ||
| feeInSats?: number; | ||
| changeAddress?: string; | ||
| } |
There was a problem hiding this comment.
requiredSignatures and totalSigners are validated but otherwise unused; the function also doesn't construct or verify the multisig redeem/witness script from these values. This API can mislead callers into thinking the signer configuration affects the generated PSBT. Consider removing these fields until used, or using them to build/validate the witnessScript/redeemScript for each input.
| export function buildMultiSigTransaction( | ||
| options: BuildMultiSigTransactionOptions | ||
| ): BuildMultiSigTransactionResult { | ||
| const { | ||
| network, | ||
| utxos, | ||
| outputs, | ||
| requiredSignatures, | ||
| totalSigners, | ||
| feeInSats = 0, | ||
| changeAddress, | ||
| } = options; | ||
|
|
||
| if (requiredSignatures < 1 || totalSigners < 1 || requiredSignatures > totalSigners) { | ||
| throw new Error("Invalid multisig signer configuration"); | ||
| } | ||
|
|
||
| if (!Array.isArray(utxos) || utxos.length === 0) { | ||
| throw new Error("At least one UTXO is required to build a multisig transaction"); | ||
| } | ||
|
|
||
| if (!Array.isArray(outputs) || outputs.length === 0) { | ||
| throw new Error("At least one output is required to build a multisig transaction"); | ||
| } |
There was a problem hiding this comment.
This helper introduces new behavior but there are no accompanying tests. The repo already uses Mocha tests for helpers (e.g., test/signTransaction.js). Adding a focused test suite for buildMultiSigTransaction (guard rails, change output behavior, insufficient funds) would help prevent regressions.
| signTransaction, | ||
| utils, | ||
| getFeeAndInput, | ||
| getTransactionSize, | ||
| buildMultiSigTransaction, | ||
| TransactionVisualizer, |
There was a problem hiding this comment.
This change introduces tab-indented multi-line exports, which is inconsistent with the spacing/formatting used in other TS helpers (e.g., signTransaction.ts uses 2-space indentation). To keep diffs and style consistent, consider using the existing indentation style (spaces) here.
| signTransaction, | |
| utils, | |
| getFeeAndInput, | |
| getTransactionSize, | |
| buildMultiSigTransaction, | |
| TransactionVisualizer, | |
| signTransaction, | |
| utils, | |
| getFeeAndInput, | |
| getTransactionSize, | |
| buildMultiSigTransaction, | |
| TransactionVisualizer, |
