mirror of
https://github.com/btcpayserver/btcpayserver.git
synced 2025-03-03 09:29:10 +01:00
Do not remove outputs in payjoin tx
This commit is contained in:
parent
9fc451c9ca
commit
ba2184e21a
4 changed files with 11 additions and 52 deletions
|
@ -18,7 +18,6 @@ using Microsoft.AspNetCore.Http;
|
|||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.AspNetCore.Mvc.Controllers;
|
||||
using Microsoft.AspNetCore.Mvc.Routing;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using NBitcoin;
|
||||
using NBXplorer;
|
||||
|
|
|
@ -290,12 +290,15 @@ namespace BTCPayServer.Tests
|
|||
|
||||
|
||||
// Same as above. Except the sender send one satoshi less, so the change
|
||||
// output get below dust and should be removed completely.
|
||||
// output would get below dust and would be removed completely.
|
||||
// So we remove as much fee as we can, and still accept the transaction because it is above minrelay fee
|
||||
vector = (SpentCoin: Money.Satoshis(1089), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(510), Fee: Money.Satoshis(200), ExpectLocked: true, ExpectedError: null as string);
|
||||
proposedPSBT = await RunVector();
|
||||
var output = Assert.Single(proposedPSBT.Outputs);
|
||||
// With the output removed, the user should have largely pay all the needed fee
|
||||
Assert.Equal(Money.Satoshis(510) + receiverCoin.Amount, output.Value);
|
||||
Assert.Equal(2, proposedPSBT.Outputs.Count);
|
||||
// We should have our payment
|
||||
Assert.Contains(proposedPSBT.Outputs, output => output.Value == Money.Satoshis(500) + receiverCoin.Amount);
|
||||
// Plus our other change output with value just at dust level
|
||||
Assert.Contains(proposedPSBT.Outputs, output => output.Value == Money.Satoshis(294));
|
||||
proposedPSBT = await senderUser.Sign(proposedPSBT);
|
||||
proposedPSBT = proposedPSBT.Finalize();
|
||||
explorerClient = tester.PayTester.GetService<ExplorerClientProvider>().GetExplorerClient(proposedPSBT.Network.NetworkSet.CryptoCode);
|
||||
|
|
|
@ -283,10 +283,6 @@ namespace BTCPayServer.Payments.PayJoin
|
|||
|
||||
Money ourFeeContribution = Money.Zero;
|
||||
// We need to adjust the fee to keep a constant fee rate
|
||||
var originalNewTx = newTx.Clone();
|
||||
bool isSecondPass = false;
|
||||
recalculateFee:
|
||||
ourOutput = newTx.Outputs[ourOutputIndex];
|
||||
var txBuilder = network.NBitcoinNetwork.CreateTransactionBuilder();
|
||||
txBuilder.AddCoins(psbt.Inputs.Select(i => i.GetCoin()));
|
||||
txBuilder.AddCoins(selectedUTXOs.Select(o => o.Value.AsCoin()));
|
||||
|
@ -316,49 +312,14 @@ namespace BTCPayServer.Payments.PayJoin
|
|||
if (i != ourOutputIndex)
|
||||
{
|
||||
var outputContribution = Money.Min(additionalFee, newTx.Outputs[i].Value);
|
||||
outputContribution = Money.Min(outputContribution,
|
||||
newTx.Outputs[i].Value - newTx.Outputs[i].GetDustThreshold(minRelayTxFee));
|
||||
newTx.Outputs[i].Value -= outputContribution;
|
||||
additionalFee -= outputContribution;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
List<int> dustIndices = new List<int>();
|
||||
for (int i = 0; i < newTx.Outputs.Count; i++)
|
||||
{
|
||||
if (newTx.Outputs[i].IsDust(minRelayTxFee))
|
||||
{
|
||||
dustIndices.Insert(0, i);
|
||||
}
|
||||
}
|
||||
|
||||
if (dustIndices.Count > 0)
|
||||
{
|
||||
if (isSecondPass)
|
||||
{
|
||||
// This should not happen
|
||||
await UnlockUTXOs();
|
||||
await BroadcastNow();
|
||||
return StatusCode(500,
|
||||
CreatePayjoinError(500, "unavailable",
|
||||
$"This service is unavailable for now (isSecondPass)"));
|
||||
}
|
||||
|
||||
foreach (var dustIndex in dustIndices)
|
||||
{
|
||||
newTx.Outputs.RemoveAt(dustIndex);
|
||||
}
|
||||
|
||||
ourOutputIndex = newTx.Outputs.IndexOf(ourOutput);
|
||||
newTx = originalNewTx.Clone();
|
||||
foreach (var dustIndex in dustIndices)
|
||||
{
|
||||
newTx.Outputs.RemoveAt(dustIndex);
|
||||
}
|
||||
ourFeeContribution = Money.Zero;
|
||||
isSecondPass = true;
|
||||
goto recalculateFee;
|
||||
}
|
||||
|
||||
if (additionalFee > Money.Zero)
|
||||
{
|
||||
// We could not pay fully the additional fee, however, as long as
|
||||
|
|
|
@ -170,7 +170,7 @@ namespace BTCPayServer.Services
|
|||
var additionalFee = newPSBT.GetFee() - originalFee;
|
||||
if (overPaying > additionalFee)
|
||||
throw new PayjoinSenderException("The payjoin receiver is sending more money to himself");
|
||||
if (overPaying > 2 * originalFee)
|
||||
if (overPaying > originalFee)
|
||||
throw new PayjoinSenderException("The payjoin receiver is making us pay more than twice the original fee");
|
||||
|
||||
// Let's check the difference is only for the fee and that feerate
|
||||
|
@ -178,11 +178,7 @@ namespace BTCPayServer.Services
|
|||
var expectedFee = originalFeeRate.GetFee(newVirtualSize);
|
||||
// Signing precisely is hard science, give some breathing room for error.
|
||||
expectedFee += originalFeeRate.GetFee(newPSBT.Inputs.Count * 2);
|
||||
|
||||
// If the payjoin is removing some dust, we may pay a bit more as a whole output has been removed
|
||||
var removedOutputs = Math.Max(0, originalTx.Outputs.Count - newPSBT.Outputs.Count);
|
||||
expectedFee += removedOutputs * Money.Satoshis(294);
|
||||
if (overPaying > expectedFee)
|
||||
if (overPaying > (expectedFee - originalFee))
|
||||
throw new PayjoinSenderException("The payjoin receiver increased the fee rate we are paying too much");
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue