Allow exact match of target === input amount during accumulation. #106

ph101pp · 2024-07-24T14:51:23Z

Currently this example fails to create a valid selection even though the amount of sats available exactly matches the amount of sats needed.

const privKey = hex.decode('0101010101010101010101010101010101010101010101010101010101010101');
const pubKey = secp256k1.getPublicKey(privKey, true);
const spend = btc.p2wpkh(pubKey, regtest);
const utxo = [
  {
    ...spend, 
    txid: hex.decode('0af50a00a22f74ece24c12cd667c290d3a35d48124a69f4082700589172a3aa2'),
    index: 0,
    witnessUtxo: { script: spend.script, amount: 50_000n },  // same amount
  },
];
const outputs = [
  { address: '2MvpbAgedBzJUBZWesDwdM7p3FEkBEwq3n3', amount: 50_000n }, // same amount
];

const selected = btc.selectUTXO(utxo, outputs, 'default', {
  changeAddress: 'bcrt1pea3850rzre54e53eh7suwmrwc66un6nmu9npd7eqrhd6g4lh8uqsxcxln8',
  feePerByte: 0n, // no fee to remove variability
});

// result: selected === undefined
// expectation: valid selection of utxo[0] as input

This PR fixes the comparison so an exact match results in a valid selection.

paulmillr · 2024-07-24T14:57:01Z

Please add a test.

ph101pp · 2024-07-25T05:19:19Z

test/utxo-select.test.js

+
+    const t3 = (strategy) => {
+      const FEE = 0n // no fee to test exact amounts
+      const est = new btc._Estimator(inputs, [{ ...OUTPUTS[0], amount: inputsTotalAmount }], {
+        feePerByte: FEE,
+        changeAddress: '2MshuFeVGhXVdRv77UcJYvRBi2JyTNwgSR2',
+        network: regtest,
+        allowLegacyWitnessUtxo: true,
+        createTx: true,
+        allowSameUtxo: true,
+      });
+      const acc = est.result(strategy);
+      if (!acc) return;
+      const i = acc.inputs.map((i) => i.witnessUtxo.amount);
+      const o = acc.outputs.map((i) => i.amount);
+      const tx = acc.tx;
+      tx.sign(privKey1);
+      tx.finalize();
+      const expFee = BigInt(tx.vsize) * FEE;
+      return {
+        i,
+        o,
+        txFee: tx.fee,
+        expFee,
+        fee: acc.fee,
+        change: acc.change,
+      };
+    };
+    deepStrictEqual(t3('default'), {
+      i: inputs.map((i) => i.witnessUtxo.amount).reverse(),
+      o: [inputsTotalAmount],
+      txFee: 0n,
+      expFee: 0n,
+      fee: 0n,
+      change: false,
+    });
+    deepStrictEqual(t3('accumSmallest'), {
+      i: inputs.map((i) => i.witnessUtxo.amount),
+      o: [inputsTotalAmount],
+      txFee: 0n,
+      expFee: 0n,
+      fee: 0n,
+      change: false,
+    });
+    deepStrictEqual(t3('all'), {
+      i: inputs.map((i) => i.witnessUtxo.amount),
+      o: [inputsTotalAmount],
+      txFee: 0n,
+      expFee: 0n,
+      fee: 0n,
+      change: false,
+    });


New tests to verify that input amounts === output amounts with 0 fees results in all inputs selected. This is logically sound with regard to how accumulate should work imo.

ph101pp · 2024-07-25T05:22:16Z

test/utxo-select.test.js

      fee: 1691n,
    });
-    est.amount = 321n;
+    est.amount = 322n;


Step 1 outputs -> 2 outputs is now 1 sat higher since exact match doesn't result in extra input

ph101pp · 2024-07-25T05:22:55Z

test/utxo-select.test.js

    deepStrictEqual(t(est, est.biggest, true), {
      amounts: [512n],
-      change: 221n,
+      change: 0n,


seems right 321 + 191 = 512

ph101pp · 2024-07-25T05:25:05Z

test/utxo-select.test.js

@@ -677,7 +680,7 @@ describe('UTXO Select', () => {
      for (const idx of indices) {
        tx.addInput(inputs[idx]);
      }
-      tx.addOutputAddress(OUTPUTS[0].address, 100n, regtest);
+      tx.addOutputAddress(OUTPUTS[0].address, est.amount, regtest);


fixed hardcoded amount here

ph101pp · 2024-07-25T05:25:17Z

test/utxo-select.test.js

@@ -715,8 +718,8 @@ describe('UTXO Select', () => {
    est.amount = 256n;
    deepStrictEqual(t(est, est.biggest, true), {
      amounts: [512n],
-      change: 221n,
-      expFee: 191n,
+      change: 66n,


change explained by fix on L683

221 - ( 256 - 100) = 65

I'm not sure where the 1n fee difference comes from.

the difference comes from size of serialization:

length(varint(1)) != length(varint(2))

ph101pp · 2024-07-25T05:28:55Z

test/utxo-select.test.js

+      change: 106n,
+      expFee: 1685n,


change explained by fix on L683

261 - ( 256 - 100) = 105

I'm not sure where the 1n fee difference comes from.

ph101pp · 2024-07-25T05:33:31Z

test/utxo-select.test.js

    deepStrictEqual(t(est, est.biggest, true), {
      amounts: [512n, 256n],
-      change: 329n,
+      change: 107n,


change explained by fix on L683

(512 + 256) - (322 + 339) = 107

329 - ( 322 - 100) = 107

ph101pp · 2024-07-25T05:39:43Z

test/utxo-select.test.js

+      i: [256n],
+      o: [65n],
+      txFee: 191n,
+      expFee: 191n,
+      fee: 191n,
+      change: false,


seems right 65 + 191 = 256

ph101pp · 2024-07-31T05:40:54Z

Friendly bump on this, is there anything I can do to help getting this merged and published?

paulmillr · 2024-07-31T18:20:45Z

Thanks

Allow exact target === input amount match

635dbc5

ph101pp added 2 commits July 25, 2024 14:14

Allow exact target === input amount match

54c8e2c

Add accumulation tests with 0n fee

2ac4cc2

ph101pp commented Jul 25, 2024

View reviewed changes

paulmillr merged commit 9a29c88 into paulmillr:main Jul 31, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow exact match of target === input amount during accumulation. #106

Allow exact match of target === input amount during accumulation. #106

ph101pp commented Jul 24, 2024

paulmillr commented Jul 24, 2024

ph101pp Jul 25, 2024

ph101pp Jul 25, 2024 •

edited

Loading

ph101pp Jul 25, 2024 •

edited

Loading

ph101pp Jul 25, 2024

ph101pp Jul 25, 2024

paulmillr Jul 31, 2024

ph101pp Jul 25, 2024

ph101pp Jul 25, 2024

ph101pp Jul 25, 2024 •

edited

Loading

ph101pp commented Jul 31, 2024

paulmillr commented Jul 31, 2024

Allow exact match of target === input amount during accumulation. #106

Allow exact match of target === input amount during accumulation. #106

Conversation

ph101pp commented Jul 24, 2024

paulmillr commented Jul 24, 2024

ph101pp Jul 25, 2024

Choose a reason for hiding this comment

ph101pp Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

ph101pp Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

ph101pp Jul 25, 2024

Choose a reason for hiding this comment

ph101pp Jul 25, 2024

Choose a reason for hiding this comment

paulmillr Jul 31, 2024

Choose a reason for hiding this comment

ph101pp Jul 25, 2024

Choose a reason for hiding this comment

ph101pp Jul 25, 2024

Choose a reason for hiding this comment

ph101pp Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

ph101pp commented Jul 31, 2024

paulmillr commented Jul 31, 2024

ph101pp Jul 25, 2024 •

edited

Loading

ph101pp Jul 25, 2024 •

edited

Loading

ph101pp Jul 25, 2024 •

edited

Loading