Consistent UTXO Handling for CWC Transactions - #4213
Conversation
…tcore-node and bitcore-lib style transactions
kajoseph
left a comment
There was a problem hiding this comment.
Hey @MicahMaphet , thanks for the PR and nice catch. I think a better way to approach this is to have a standardizeUtxo function that looks something like this:
standardizeUtxo(utxo) {
return {
txid: utxo.txid || utxo.mintTxid,
vout: utxo.vout || utxo.mintIndex,
...etc...
};
}then, we can just do
utxos = utxos.map(this.standardizeUtxo);There was a problem hiding this comment.
🟡 Changes recommended
Unresolved issues remain in UTXO validation, public typings, address normalization, and API exposure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Standardizes UTXO handling across BTC, BCH, DOGE, and LTC transaction operations.
Changes:
- Supports bitcore-node and bitcore-lib UTXO formats.
- Updates transaction creation, signing, and sighash handling.
- Adds cross-chain UTXO compatibility tests.
File summaries
| File | Summary |
|---|---|
packages/crypto-wallet-core/test/transactions.test.ts |
Tests UTXO format compatibility across chains. |
packages/crypto-wallet-core/src/transactions/ltc/index.ts |
Integrates normalized UTXOs for Litecoin transactions. |
packages/crypto-wallet-core/src/transactions/doge/index.ts |
Integrates normalized UTXOs for Dogecoin transactions. |
packages/crypto-wallet-core/src/transactions/btc/index.ts |
Adds UTXO normalization and updated transaction handling. |
packages/crypto-wallet-core/src/transactions/bch/index.ts |
Integrates normalized UTXOs for Bitcoin Cash transactions. |
Review details
Suppressed comments (9)
packages/crypto-wallet-core/src/transactions/bch/index.ts:7
- This override narrows recipient amounts to
number, while the base provider and the implementation explicitly supportnumber | string(Number(recipient.amount)). String amounts therefore fail type checking for BCH even though they work at runtime; keep this override's public type consistent with the base.
recipients: Array<{ address: string; amount: number }>;
packages/crypto-wallet-core/src/transactions/btc/index.ts:49
- The new docstring misspells “internaly”; please correct it to “internally”.
*/
packages/crypto-wallet-core/src/transactions/btc/index.ts:285
- The new type comment uses “were” where “where” is required, making the description ungrammatical.
* Utxo type for functions were the received utxo type is unknown.
packages/crypto-wallet-core/src/transactions/btc/index.ts:218
- This changes the provider's
getSigningAddressesimplementation, but the exportedTransactionsProxystill has nogetSigningAddressesforwarding method, soTransactions.getSigningAddresses(...)remains unavailable despite being listed as a supported API in the PR description. Add the proxy method (and its public typing) or remove this as a public API promise.
getSigningAddresses(params: {
tx: TransactionType;
utxos: EveryUtxoType[];
}): (string | undefined)[] {
const { tx, utxos } = params;
const bitcoreTx = new this.lib.Transaction(tx);
const btcUtxos = utxos.map(utxo => this.standardizeUtxo(utxo));
const applicableUtxos = this.getRelatedUtxos({
outputs: bitcoreTx.inputs,
utxos: btcUtxos
});
return applicableUtxos.map(utxo => utxo.address);
packages/crypto-wallet-core/src/transactions/btc/index.ts:54
- Defaulting a missing index to
0silently turns an invalid UTXO into a reference to output 0. A node record with an absentmintIndexcan therefore be selected and signed as<txid>:0, whereas the underlyingUnspentOutputvalidation would reject it. Validate that a supported index field is present instead of applying this fallback.
outputIndex: Number(utxo.outputIndex ?? utxo.mintIndex ?? utxo.vout ?? 0),
packages/crypto-wallet-core/src/transactions/btc/index.ts:271
- The new documentation uses “use” where “used” is required.
* Standard utxo type use for internal processing.
packages/crypto-wallet-core/src/transactions/btc/index.ts:52
- The
?? 0fallback fabricates a valid zero-satoshi UTXO when all supported amount fields are absent. InisSweepmode coin selection is bypassed, so malformed input can be serialized as a spendable-looking input instead of being rejected. Require a value field and let invalid UTXOs fail validation rather than defaulting to zero.
satoshis: Number(utxo.satoshis ?? utxo.value ?? this.lib.Unit.fromBTC(utxo.amount ?? 0).toSatoshis()),
packages/crypto-wallet-core/src/transactions/doge/index.ts:7
- This override narrows recipient amounts to
number, while the base provider and the implementation explicitly supportnumber | string(Number(recipient.amount)). String amounts therefore fail type checking for DOGE even though they work at runtime; keep this override's public type consistent with the base.
recipients: Array<{ address: string; amount: number }>;
packages/crypto-wallet-core/src/transactions/ltc/index.ts:7
- This override narrows recipient amounts to
number, while the base provider and the implementation explicitly supportnumber | string(Number(recipient.amount)). String amounts therefore fail type checking for LTC even though they work at runtime; keep this override's public type consistent with the base.
recipients: Array<{ address: string; amount: number }>;
- Files reviewed: 5/5 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const unspentOutputUtxos = [ | ||
| { | ||
| txId: '643ec66d6c4cad4cbdb8ed2166b8078975e0af9bb7ff7e30d394f43b0d9f18ab', | ||
| outputIndex: 1, | ||
| satoshis: 90_000, | ||
| script: '76a9144e744a19a009a9dd43a23a7c12045c83e82ac9d288ac' |
crypto-wallet-core handle empty ("") address for utxo conversion
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description 🗒️
Previously
crypto-wallet-core'sCWC.Transactionshad inconsistent handling of utxos..createand.signrequiredbitcore-nodeutxos. Bybitcore-nodeutxos I mean howbitcore-nodestores utxos in its database. These have mintTxid, spentTxid, mintIndex, and value; whilebitcore-libstyle utxos have txid, outputIndex, and satoshis..getSighashrequiredbitcore-libutxos.Now
CWC.Transactionsaccepts both kinds of utxos.Motivation
I need these changes for
bitcore-hardwarein order to sign transactions. I need to construct a transaction with.createand pass those utxos into a [a given hardware wallet object].sign. To sign with hardware wallets, I need to give the the sighash via.getSighash. The current version ofcrypto-wallet-coredoes not allow this because.createand.getSighashtake different utxo types. This problem could be most easily solved be simply making.getSighashconvertbitcore-nodeutxos intobitcore-libutxos. However, we want consistent handling of utxos. This PR instead solves the core problem by making all transaction methods in thecryto-wallet-coreutxo chains accept bothbitcore-nodeandbitcore-libutxos.Changelog 🪵
functionality 🔨
EveryUtxoTypewhere the properties are unknown due to it being received outside the Transactions class. Can contain any type of utxo data:UnspentOutput,UnspentOutput.toObject, and utxos from thebitcore-nodedatabase. Hard to work with because all the properties need to be checked individually.UtxoTypefor internal usage. Nice to work with because the properties don't need to be checked. Uses property naming fromUnspentOutput.Transactions.create,Transactions.sign,Transactions.getSighash, andTransactions.getSigningAddressesacceptEveryUtxoTypeTransactions.createsilently ignores utxomintHeightsorting when supplied withbitcore-libutxosorganization 🎶
Number()is used instead ofparseInt()getRelatedUtxosacceptsUtxoTyperather thanbitcore-nodeutxo typesTesting Notes 🥼 🧪
crypto-wallet-coretests all work, but I can add more to test the new functionality.The following cases would previously break, but they now work.
.createand.getSighashpreviously use different utxo types but now accept either.bitcore-lib utxos
bitcore-node utxos
Checklist ✅ 🗒️