Skip to content

Commit

Permalink
Fix: Payjoin wasn't always properly choosing utxo for optimal change
Browse files Browse the repository at this point in the history
  • Loading branch information
@NicolasDorier
NicolasDorier committed Feb 6, 2023
1 parent 94f2cd4 commit 967f76d
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 33 deletions.
31 changes: 27 additions & 4 deletions BTCPayServer.Tests/PayJoinTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,12 @@ public async Task ChooseBestUTXOsForPayjoin()
var network = tester.NetworkProvider.GetNetwork<BTCPayNetwork>("BTC");
var controller = tester.PayTester.GetService<PayJoinEndpointController>();

//Only one utxo, so obvious result
var utxos = new[] { FakeUTXO(1.0m) };
var utxos = new[] { FakeUTXO(1m) };
var paymentAmount = 0.5m;
var otherOutputs = new[] { 0.5m };
var inputs = new[] { 1m };
var result = await controller.SelectUTXO(network, utxos, inputs, paymentAmount, otherOutputs);
Assert.Equal(PayJoinEndpointController.PayjoinUtxoSelectionType.Ordered, result.selectionType);
Assert.Equal(PayJoinEndpointController.PayjoinUtxoSelectionType.HeuristicBased, result.selectionType);
Assert.Contains(result.selectedUTXO, utxo => utxos.Contains(utxo));

//no matter what here, no good selection, it seems that payment with 1 utxo generally makes payjoin coin selection unperformant
Expand All @@ -124,15 +123,39 @@ public async Task ChooseBestUTXOsForPayjoin()
otherOutputs = new[] { 0.5m };
inputs = new[] { 1m };
result = await controller.SelectUTXO(network, utxos, inputs, paymentAmount, otherOutputs);
Assert.Equal(PayJoinEndpointController.PayjoinUtxoSelectionType.Ordered, result.selectionType);
Assert.Equal(PayJoinEndpointController.PayjoinUtxoSelectionType.HeuristicBased, result.selectionType);

//when there is no change, anything works
utxos = new[] { FakeUTXO(1), FakeUTXO(0.1m), FakeUTXO(0.001m), FakeUTXO(0.003m) };
paymentAmount = 0.5m;
otherOutputs = new decimal[0];
inputs = new[] { 0.03m, 0.07m };
result = await controller.SelectUTXO(network, utxos, inputs, paymentAmount, otherOutputs);


// We want to make a transaction such that
// min(out) < min(in)

// Original transaction is:
// 0.5 -> 0.3 , 0.1
// When chosing a new utxo x, we have the modified tx
// 0.5 , x -> 0.3 , (0.1+x)
// We need:
// min(0.3, 0.1+x) < min(0.5, x)
// Any x > 0.3 should be fine
utxos = new[] { FakeUTXO(0.2m), FakeUTXO(0.3m), FakeUTXO(0.31m) };
paymentAmount = 0.1m;
otherOutputs = new decimal[] { 0.3m };
inputs = new[] { 0.5m };
result = await controller.SelectUTXO(network, utxos, inputs, paymentAmount, otherOutputs);
Assert.Equal(PayJoinEndpointController.PayjoinUtxoSelectionType.HeuristicBased, result.selectionType);
Assert.Equal(0.31m, result.selectedUTXO[0].Value.GetValue(network));

// If the 0.31m wasn't available, no selection heuristic based
utxos = new[] { FakeUTXO(0.2m), FakeUTXO(0.3m) };
result = await controller.SelectUTXO(network, utxos, inputs, paymentAmount, otherOutputs);
Assert.Equal(PayJoinEndpointController.PayjoinUtxoSelectionType.Ordered, result.selectionType);
Assert.Equal(0.2m, result.selectedUTXO[0].Value.GetValue(network));
}


Expand Down
58 changes: 29 additions & 29 deletions BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,62 +585,62 @@ public enum PayjoinUtxoSelectionType
Ordered
}
[NonAction]
public async Task<(UTXO[] selectedUTXO, PayjoinUtxoSelectionType selectionType)> SelectUTXO(BTCPayNetwork network, UTXO[] availableUtxos, decimal[] otherInputs, decimal mainPaymentOutput,
public async Task<(UTXO[] selectedUTXO, PayjoinUtxoSelectionType selectionType)> SelectUTXO(BTCPayNetwork network, UTXO[] availableUtxos, decimal[] otherInputs, decimal originalPaymentOutput,
IEnumerable<decimal> otherOutputs)
{
if (availableUtxos.Length == 0)
return (Array.Empty<UTXO>(), PayjoinUtxoSelectionType.Unavailable);
// Assume the merchant wants to get rid of the dust
HashSet<OutPoint> locked = new HashSet<OutPoint>();
// We don't want to make too many db roundtrip which would be inconvenient for the sender
int maxTries = 30;
int currentTry = 0;
// UIH = "unnecessary input heuristic", basically "a wallet wouldn't choose more utxos to spend in this scenario".
//
// "UIH1" : one output is smaller than any input. This heuristically implies that that output is not a payment, and must therefore be a change output.
//
// "UIH2": one input is larger than any output. This heuristically implies that no output is a payment, or, to say it better, it implies that this is not a normal wallet-created payment, it's something strange/exotic.
//src: https://gist.github.com/AdamISZ/4551b947789d3216bacfcb7af25e029e#gistcomment-2796539

// BlockSci UIH1 and UIH2:
// if min(out) < min(in) then UIH1 else UIH2
// https://eprint.iacr.org/2022/589.pdf


var origMinOut = otherOutputs.Concat(new[] { decimal.MaxValue }).Min();
var origMinIn = otherInputs.Concat(new[] { decimal.MaxValue }).Min();

foreach (var availableUtxo in availableUtxos)
{
if (currentTry >= maxTries)
if (locked.Count >= maxTries)
break;

var invalid = false;
foreach (var input in otherInputs.Concat(new[] { availableUtxo.Value.GetValue(network) }))
var utxoValue = availableUtxo.Value.GetValue(network);
var newPaymentOutput = originalPaymentOutput + utxoValue;
var minOut = Math.Min(origMinOut, newPaymentOutput);
var minIn = Math.Min(origMinIn, utxoValue);

if (minOut < minIn)
{
var computedOutputs =
otherOutputs.Concat(new[] { mainPaymentOutput + availableUtxo.Value.GetValue(network) });
if (computedOutputs.Any(output => input > output))
// UIH1: Optimal change, smallest output is the change address.
if (await _utxoLocker.TryLock(availableUtxo.Outpoint))
{
//UIH 1 & 2
invalid = true;
break;
return (new[] { availableUtxo }, PayjoinUtxoSelectionType.HeuristicBased);
}
else
{
locked.Add(availableUtxo.Outpoint);
continue;
}
}

if (invalid)
else
{
// UIH2: Unnecessary input, let's try to get an optimal change by using a different output
continue;
}
if (await _utxoLocker.TryLock(availableUtxo.Outpoint))
{
return (new[] { availableUtxo }, PayjoinUtxoSelectionType.HeuristicBased);
}

locked.Add(availableUtxo.Outpoint);
currentTry++;
}

foreach (var utxo in availableUtxos.Where(u => !locked.Contains(u.Outpoint)))
{
if (currentTry >= maxTries)
if (locked.Count >= maxTries)
break;
if (await _utxoLocker.TryLock(utxo.Outpoint))
{
return (new[] { utxo }, PayjoinUtxoSelectionType.Ordered);
}
currentTry++;
locked.Add(utxo.Outpoint);
}
return (Array.Empty<UTXO>(), PayjoinUtxoSelectionType.Unavailable);
}
Expand Down

0 comments on commit 967f76d

Please sign in to comment.