Skip to content

Commit 39002c1

Browse files
committed
cr suggestions
1 parent 83f553b commit 39002c1

File tree

7 files changed

+74
-41
lines changed

7 files changed

+74
-41
lines changed

claude/utxo-expert-development-agent.md

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -203,15 +203,21 @@ class UTXOManager {
203203
targetAmount: number,
204204
feeRate: number
205205
): { inputs: UTXO[]; fee: number; change: number } {
206-
// Implementation using coinselect algorithm
207-
const accumulative = require('coinselect/accumulative')
206+
// Implementation using first-party UtxoSelector with multi-strategy approach
207+
const { UtxoSelector } = require('@xchainjs/xchain-utxo')
208+
const selector = new UtxoSelector()
208209

209-
const targets = [{ address: 'target-address', value: targetAmount }]
210-
const { inputs, outputs, fee } = accumulative(availableUTXOs, targets, feeRate)
211-
212-
const change = outputs.find(output => !output.address)?.value || 0
213-
214-
return { inputs: inputs || [], fee, change }
210+
try {
211+
const result = selector.selectOptimal(availableUTXOs, targetAmount, feeRate)
212+
return {
213+
inputs: result.inputs,
214+
fee: result.fee,
215+
change: result.changeAmount
216+
}
217+
} catch (error) {
218+
// Handle selection failure
219+
throw new Error(`UTXO selection failed: ${error.message}`)
220+
}
215221
}
216222

217223
private async getCurrentBlockHeight(): Promise<number> {
@@ -262,11 +268,12 @@ class TransactionBuilder {
262268
index: utxo.index,
263269
witnessUtxo: utxo.witnessUtxo
264270
})
265-
} else if (utxo.nonWitnessUtxo) {
271+
} else if (utxo.txHex) {
272+
// Use full transaction hex when witnessUtxo is not available
266273
psbt.addInput({
267274
hash: utxo.hash,
268275
index: utxo.index,
269-
nonWitnessUtxo: utxo.nonWitnessUtxo
276+
nonWitnessUtxo: Buffer.from(utxo.txHex, 'hex')
270277
})
271278
}
272279
}

packages/xchain-bitcoin/__e2e__/bitcoin-ledger-client.e2e.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe('Bitcoin Client Ledger', () => {
7474
fail()
7575
}
7676
})
77-
it('send max', async () => {
77+
it('send max ledger', async () => {
7878
try {
7979
const senderAddress = await btcClient.getAddressAsync(0)
8080
const recipientAddress = await btcClient.getAddressAsync(1)

packages/xchain-bitcoin/__e2e__/blockcypher.bitcoin-client.e2e.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AssetInfo, Network } from '@xchainjs/xchain-client'
2-
import { assetAmount, assetToBase, assetToString, baseAmount, baseToAsset } from '@xchainjs/xchain-util'
2+
import { assetAmount, assetToBase, assetToString, baseToAsset } from '@xchainjs/xchain-util'
33
import { UtxoClientParams } from '@xchainjs/xchain-utxo'
44

55
import { ClientKeystore as Client } from '../src/clientKeystore'
@@ -291,18 +291,17 @@ describe('Bitcoin Integration Tests for BlockCypher', () => {
291291

292292
// Optional: Actually broadcast the transaction (commented out for safety)
293293
// Uncomment the following to actually send the transaction
294-
295-
console.log('Broadcasting transaction...')
296-
const txHash = await btcClient.transfer({
297-
walletIndex: 0,
298-
asset: AssetBTC,
299-
recipient: recipientAddress,
300-
amount: baseAmount(sendMaxResult.maxAmount, BTC_DECIMAL),
301-
memo: 'max',
302-
feeRate: feeRate,
303-
})
304-
console.log('✅ Transaction broadcast successfully!')
305-
console.log('Transaction hash:', txHash)
294+
// console.log('Broadcasting transaction...')
295+
// const txHash = await btcClient.transfer({
296+
// walletIndex: 0,
297+
// asset: AssetBTC,
298+
// recipient: recipientAddress,
299+
// amount: baseAmount(sendMaxResult.maxAmount, BTC_DECIMAL),
300+
// memo: 'max',
301+
// feeRate: feeRate,
302+
// })
303+
// console.log('✅ Transaction broadcast successfully!')
304+
// console.log('Transaction hash:', txHash)
306305
} catch (err) {
307306
console.error('ERR running send max test:', err)
308307
if (err instanceof Error) {

packages/xchain-bitcoin/src/clientKeystore.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as ecc from '@bitcoin-js/tiny-secp256k1-asmjs'
22
import { FeeOption, FeeRate, TxHash, checkFeeBounds } from '@xchainjs/xchain-client'
33
import { getSeed } from '@xchainjs/xchain-crypto'
44
import { Address } from '@xchainjs/xchain-util'
5-
import { TxParams, UtxoClientParams } from '@xchainjs/xchain-utxo'
5+
import { TxParams, UtxoClientParams, UtxoSelectionPreferences } from '@xchainjs/xchain-utxo'
66
import { HDKey } from '@scure/bip32'
77
import * as Bitcoin from 'bitcoinjs-lib'
88
import { ECPairFactory, ECPairInterface } from 'ecpair'
@@ -108,7 +108,7 @@ class ClientKeystore extends Client {
108108
* @returns {Promise<TxHash|string>} A promise that resolves to the transaction hash or an error message.
109109
* @throws {"memo too long"} Thrown if the memo is longer than 80 characters.
110110
*/
111-
async transfer(params: TxParams & { feeRate?: FeeRate }): Promise<TxHash> {
111+
async transfer(params: TxParams & { feeRate?: FeeRate; utxoSelectionPreferences?: UtxoSelectionPreferences }): Promise<TxHash> {
112112
// Set the default fee rate to `fast`
113113
const feeRate = params.feeRate || (await this.getFeeRates())[FeeOption.Fast]
114114

@@ -121,16 +121,20 @@ class ClientKeystore extends Client {
121121
// Get the Bitcoin keys
122122
const btcKeys = this.getBtcKeys(this.phrase, fromAddressIndex)
123123

124+
// Merge default preferences with caller-provided preferences
125+
const mergedUtxoSelectionPreferences = {
126+
minimizeFee: true,
127+
avoidDust: true,
128+
minimizeInputs: false,
129+
...params.utxoSelectionPreferences,
130+
}
131+
124132
// Prepare the transaction
125133
const { rawUnsignedTx } = await this.prepareTxEnhanced({
126134
...params,
127135
sender: this.getAddress(fromAddressIndex),
128136
feeRate,
129-
utxoSelectionPreferences: {
130-
minimizeFee: true,
131-
avoidDust: true,
132-
minimizeInputs: false,
133-
},
137+
utxoSelectionPreferences: mergedUtxoSelectionPreferences,
134138
})
135139

136140
// Build the PSBT

packages/xchain-bitcoin/src/clientLedger.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import AppBtc from '@ledgerhq/hw-app-btc'
22
import type { Transaction } from '@ledgerhq/hw-app-btc/lib/types'
33
import { FeeOption, FeeRate, TxHash, checkFeeBounds } from '@xchainjs/xchain-client'
44
import { Address } from '@xchainjs/xchain-util'
5-
import { TxParams, UTXO, UtxoClientParams } from '@xchainjs/xchain-utxo'
5+
import { TxParams, UTXO, UtxoClientParams, UtxoSelectionPreferences } from '@xchainjs/xchain-utxo'
66
import * as Bitcoin from 'bitcoinjs-lib'
77

88
import { Client } from './client'
@@ -49,7 +49,9 @@ class ClientLedger extends Client {
4949
}
5050

5151
// Transfer BTC from Ledger
52-
async transfer(params: TxParams & { feeRate?: FeeRate }): Promise<TxHash> {
52+
async transfer(
53+
params: TxParams & { feeRate?: FeeRate; utxoSelectionPreferences?: UtxoSelectionPreferences },
54+
): Promise<TxHash> {
5355
const app = await this.getApp()
5456
const fromAddressIndex = params?.walletIndex || 0
5557
// Get fee rate
@@ -58,16 +60,24 @@ class ClientLedger extends Client {
5860
checkFeeBounds(this.feeBounds, feeRate)
5961
// Get sender address
6062
const sender = await this.getAddressAsync(fromAddressIndex)
63+
64+
// Create defaults and merge with caller-provided preferences
65+
const defaults = {
66+
minimizeFee: true,
67+
avoidDust: true,
68+
minimizeInputs: false,
69+
}
70+
const mergedUtxoSelectionPreferences = {
71+
...defaults,
72+
...params.utxoSelectionPreferences,
73+
}
74+
6175
// Prepare transaction using enhanced method with optimal UTXO selection
6276
const { rawUnsignedTx, inputs } = await this.prepareTxEnhanced({
6377
...params,
6478
sender,
6579
feeRate,
66-
utxoSelectionPreferences: {
67-
minimizeFee: true,
68-
avoidDust: true,
69-
minimizeInputs: false,
70-
},
80+
utxoSelectionPreferences: mergedUtxoSelectionPreferences,
7181
})
7282
const psbt = Bitcoin.Psbt.fromBase64(rawUnsignedTx)
7383
// Prepare Ledger inputs

packages/xchain-utxo/src/utxo-selector.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ export class UtxoSelector {
4444

4545
// Constants for calculations
4646
public static readonly DUST_THRESHOLD = 546 // satoshis
47-
public static readonly BYTES_PER_INPUT = 148 // approximate bytes per P2WPKH input
48-
public static readonly BYTES_PER_OUTPUT = 34 // bytes per P2WPKH output
47+
public static readonly BYTES_PER_INPUT = 68 // approximate vbytes per P2WPKH input
48+
public static readonly BYTES_PER_OUTPUT = 31 // vbytes per P2WPKH output
4949
public static readonly BASE_TX_SIZE = 10 // base transaction overhead
5050

5151
/**

packages/xchain-utxo/src/validators.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,20 @@ export class UtxoTransactionValidator {
303303

304304
private static containsControlCharacters(str: string): boolean {
305305
// Check for control characters that might cause issues
306-
return /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/.test(str)
306+
// Ranges: 0x00-0x08, 0x0B, 0x0C, 0x0E-0x1F, 0x7F
307+
for (let i = 0; i < str.length; i++) {
308+
const code = str.charCodeAt(i)
309+
if (
310+
(code >= 0x00 && code <= 0x08) || // \x00-\x08
311+
code === 0x0b || // \x0B (vertical tab)
312+
code === 0x0c || // \x0C (form feed)
313+
(code >= 0x0e && code <= 0x1f) || // \x0E-\x1F
314+
code === 0x7f // \x7F (DEL)
315+
) {
316+
return true
317+
}
318+
}
319+
return false
307320
}
308321

309322
private static validateBitcoinAddress(address: string, network: Network, allowedFormats?: string[]): void {

0 commit comments

Comments
 (0)