bitcoinjs / coinselect

An unspent transaction output (UTXO) selection module for bitcoin.
MIT License
176 stars 101 forks source link

Inputs nor outputs are returned when a certain condition is met #75

Open sondreb opened 1 year ago

sondreb commented 1 year ago

If I understand this correctly, I shouldn't rely on the fee value returned unless either inputs or outputs are returned? Here is from the README example:

// .inputs and .outputs will be undefined if no solution was found
if (!inputs || !outputs) return

There is only a single UTXO provided to the coinselect and it runs to the following line, which makes it continue out of the loop and go to the bottom where only fee is returned:

https://github.com/bitcoinjs/coinselect/blob/1f7cd3766e2c891909a4d72afb472b54b70fc239/blackjack.js#L22

That results in both inputs and outputs being empty, which are only supplied if finalize is ever called, which in the case you have a single UTXO that has enough for output and fee.

https://github.com/bitcoinjs/coinselect/blob/1f7cd3766e2c891909a4d72afb472b54b70fc239/blackjack.js#L34

Is this expected behavior, that inputs and outputs will be empty in a scenario like this and is the example misleading, or do I have other issues with the provided input data?

Overtorment commented 1 year ago

single UTXO provided

whats the purpose of using coinselect then?

sondreb commented 1 year ago

single UTXO provided

whats the purpose of using coinselect then?

For fee calculation and output preparation for Psbt. Did another test with another wallet that had two UTXOs, this time it continued on the "waste" check and hit this:

https://github.com/bitcoinjs/coinselect/blob/1f7cd3766e2c891909a4d72afb472b54b70fc239/accumulative.js#L32

And hit "waste" the second time, which it returns the fee only.

Overtorment commented 1 year ago

so i would suggest that solution was indeed not found, to satisfy the requested feerate.

provide a test vector so we can check this hypothesis

sondreb commented 1 year ago

I was attempting to use blackjack and based upon writing tests for it, is it correct that blackjack will only be successful if there fee available within a certain threshold (upper and lower bounds)?

That's what it sounds like from the documentation, but it was not very clear to me that it will always fail if not within this threshold, maybe a clearer instruction that using blackjack should only be done if one already knows what the threshold beforehand?

Blackjack - accumulates inputs until the target value (+fees) is matched, does not accumulate inputs that go over the target value (within a threshold)

The require('coinselect') will attempt to do blackjack first, it will not happen very often (in a normal wallet user scenario) where an input is found that is very close match to output (+ fee threshold), so the optimization you get from blackjack, which I presume is to attempt to find an UTXOs that is within the range of the output, is most often wasted resources?

Using blackjack:

var feeRate = 10

// Works (return fee and inputs/outputs):
coinSelectBlackjack([{ vout: 0, value: 303400 }], [{ value: 300000 }], feeRate)
coinSelectBlackjack([{ vout: 0, value: 301920 }], [{ value: 300000 }], feeRate)

// "Fails" and returns only { fee: 1920 } (which is required for it to work)
coinSelectBlackjack([{ vout: 0, value: 301919 }], [{ value: 300000 }], feeRate)

// "Fails" and returns only { fee: 440 }. This is the "problematic" scenario that I've been debugging.
// Consequence is that `blackjack` cannot be used as the sole method of coinselect.
coinSelectBlackjack([{ vout: 0, value: 303401 }], [{ value: 300000 }], feeRate)

Using accumulative:

// Works (return fee and inputs/outputs):
coinSelectAccumulative([{ vout: 0, value: 303400 }], [{ value: 300000 }], feeRate)
coinSelectAccumulative([{ vout: 0, value: 301920 }], [{ value: 300000 }], feeRate)

// "Fails" and returns only { fee: 1920 } (which is required for it to work)
coinSelectAccumulative([{ vout: 0, value: 301919 }], [{ value: 300000 }], feeRate)

// Works and returns { fee: 3401 } + input/output
coinSelectAccumulative([{ vout: 0, value: 303401 }], [{ value: 300000 }], feeRate)

// Works and returns { fee: 3740 } and single output
coinSelectAccumulative([{ vout: 0, value: 303740 }], [{ value: 300000 }], feeRate)

// Works and returns { fee: 2260 } and double output (change address is above dust threshold)
coinSelectAccumulative([{ vout: 0, value: 303741 }], [{ value: 300000 }], feeRate)

So is the advice to avoid duplicate runs of coin select to always prefer using the require('coinselect/accumulative') method?

And do the main one if you want sort order + a potential lucky (hence the name?) optimization that blackjack does?