fix incorrect dust value computations in UTXO selection

[conduition]

Fixes bugs described here: d1bc93d#r145217554 (originally from this issue)

This still leaves two bugs:

• If I set feePerByte to some large fee rate, e.g. 100 sat/vb, this will (incorrectly) drop change outputs less than 148 * 100 = 14800 sats. That's a considerable amount of money and shouldn't be thrown away. Instead of using opts.feePerByte, you should have some dustRelayFeeRate constant (or option) which is distinct from the fee rate the user is asking for on their transaction. In core, it defaults to 3 sat/vb and most nodes will also use that number.
• this.dust is still set to default to 148 bytes, which is incorrect. See the comment in core here. 148 is the minimum input size needed to spend a non-segwit UTXO. But core also accounts for the size of the UTXO itself when computing the dust threshold, so actually you want dust = 148 + 34.

[conduition]

@paulmillr Tests fail on this and I'm not sure why. I spent a bit too much time trying to decode how the UTXO selection algorithms work and i'm having a tough go of it. Perhaps you might be able to help me figure out why the tests fail?

UTXO Select
├─estimate size: ☆
│ estimate size: ✓
├─estimating size of custom scripts: ☆
│ estimating size of custom scripts: ✓
├─estimator: ☆
│ estimator: ✓
├─accumulate: ☆
│ accumulate: ☓
node:internal/process/promises:391
    triggerUncaughtException(err, true /* fromPromise */);
    ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

  [
+   512n
-   16777216n
  ]
    at Object.test (scure-btc-signer/test/utxo-select.test.js:704:5)
    at run (scure-btc-signer/node_modules/micro-should/index.js:43:29)
    at scure-btc-signer/node_modules/micro-should/index.js:174:13 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: [ 512n ],
  expected: [ 16777216n ],
  operator: 'deepStrictEqual'
}

Node.js v20.16.0

There are other cases which also fail, but I can't tell if the problem is the tests themselves (i.e. the UTXO selection procedure is wrong, or my changes).

I can get tests to pass by doing this, but i'm not sure if that's a correct approach.

diff --git a/test/utxo-select.test.js b/test/utxo-select.test.js
index 9e28e9f..772c64e 100644
--- a/test/utxo-select.test.js
+++ b/test/utxo-select.test.js
@@ -666,6 +666,7 @@ describe('UTXO Select', () => {
     const FEE = 1n;
     const est = new btc._Estimator(inputs, [{ ...OUTPUTS[0], amount: 100n }], {
       feePerByte: FEE,
+      dustRelayFeeRate: FEE,
       changeAddress: '2MshuFeVGhXVdRv77UcJYvRBi2JyTNwgSR2',
       network: regtest,
       allowLegacyWitnessUtxo: true,
@@ -819,6 +820,7 @@ describe('UTXO Select', () => {
     const t2 = (strategy, amount) => {
       const est = new btc._Estimator(inputs, [{ ...OUTPUTS[0], amount }], {
         feePerByte: FEE,
+        dustRelayFeeRate: FEE,
         changeAddress: '2MshuFeVGhXVdRv77UcJYvRBi2JyTNwgSR2',
         network: regtest,
         allowLegacyWitnessUtxo: true,
@@ -972,9 +974,10 @@ describe('UTXO Select', () => {
     });
 
     const t3 = (strategy) => {
-      const FEE = 0n // no fee to test exact amounts
+      const FEE = 0n; // no fee to test exact amounts
       const est = new btc._Estimator(inputs, [{ ...OUTPUTS[0], amount: inputsTotalAmount }], {
         feePerByte: FEE,
+        dustRelayFeeRate: FEE,
         changeAddress: '2MshuFeVGhXVdRv77UcJYvRBi2JyTNwgSR2',
         network: regtest,
         allowLegacyWitnessUtxo: true,

[paulmillr]

b38a853

[paulmillr]

thanks