require liquidity providers to provide liquidity

[adlai]

Certain malicious and/or incompetent liquidity providers send inputs totaling less than the coinjoin amount! Current code creates in this situation an empty change output, so the invalid transaction fails harmlessly (and the likely incompetent maker refuses to sign it).

Here, we fail earlier, with a clearer message.

[chris-belcher]

I agree with this but haven't tested.

[raedah]

Does the transaction creation need to be fixed so that if there is no change, then no change address is included in the transaction, instead of an invalid empty sized change output?

[chris-belcher]

@raedah coinjoin sending with no change address (i.e. sweeping) has never really been supported. It's not clear where the maker fees would go. Although it might be useful in the rare occasion when the maker gets an order for exactly the right coinjoin amount.

[raedah]

@chris-belcher Its quite normal for the orderbook to show someone serving zero fee offers. Looked just now and someone is doing so. So if an offer gets filled and the maker has utxos that are the correct sum, it would cause this crash then. Since some people serve offer sizes which are based on their mixdepth sizes, takers would just have to take the max order size where there is no fee to cause this crash. Another case is for when offers are created for mixdepth dust, where no change output is wanted. Where is the code for this?

[adlai]

@raedah makers know they must designate a change output, and ensure that its above their DUST_THRESHOLD; this happens in the UTXO selection.

Thinking further about this, maybe the negotiation process should include one side communicating this value to the other ("taker to makers" makes more sense to me), so the makers don't think they're complying but the taker actually has higher standards?

[raedah]

@adlai Look at how total_amount is used just above that. We select a mixdepth that has enough for the amount and txfee, but dont consider if there is enough for change. But yeah, I do see the Exception, 'dont have the required UTXOs' so it shouldnt make it as far as constructing a transaction.

[AdamISZ]

utACK

[adlai]

Github closed this because I opened the PR from my develop branch.

Github is silly, but so am I, because it's done this to me before.

Create new branches for each PR and learn from my mistakes...