mirrors/btcpayserver
1
0
Fork 0
mirror of https://github.com/btcpayserver/btcpayserver.git synced 2025-01-18 21:32:27 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix: Payjoin wasn't always properly choosing utxo for optimal change (#4600)

Browse Source 
* Fix: Payjoin wasn't always properly choosing utxo for optimal change

* Update with timeout
This commit is contained in:
Nicolas Dorier 2023-02-07 16:43:31 +09:00 committed by GitHub
parent 219d03b8dd
commit 6a4d8f7404
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
BTCPayServer.Tests/PayJoinTests.cs
View File

@ -109,13 +109,12 @@ namespace BTCPayServer.Tests
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
@ -124,7 +123,7 @@ namespace BTCPayServer.Tests
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) };
@ -132,7 +131,31 @@ namespace BTCPayServer.Tests
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));
}

65
BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs
View File

@ -1,6 +1,7 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
@ -585,62 +586,64 @@ namespace BTCPayServer.Payments.PayJoin
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
TimeSpan timeout = TimeSpan.FromSeconds(30.0);
Stopwatch watch = new Stopwatch();
watch.Start();
// 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 (watch.Elapsed > timeout)
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 (watch.Elapsed > timeout)
break;
if (await _utxoLocker.TryLock(utxo.Outpoint))
{
return (new[] { utxo }, PayjoinUtxoSelectionType.Ordered);
}
currentTry++;
locked.Add(utxo.Outpoint);
}
return (Array.Empty<UTXO>(), PayjoinUtxoSelectionType.Unavailable);
}