diff --git a/BTCPayServer.Tests/PayJoinTests.cs b/BTCPayServer.Tests/PayJoinTests.cs index 1b5802b0b..63a17698d 100644 --- a/BTCPayServer.Tests/PayJoinTests.cs +++ b/BTCPayServer.Tests/PayJoinTests.cs @@ -188,8 +188,8 @@ namespace BTCPayServer.Tests var receiverCoin = await receiverUser.ReceiveUTXO(Money.Satoshis(810), network); string lastInvoiceId = null; - var vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(700), Paid: Money.Satoshis(700), Fee: Money.Satoshis(110), ExpectLocked: false, ExpectedError: "not-enough-money"); - async Task RunVector() + var vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(700), Paid: Money.Satoshis(700), Fee: Money.Satoshis(110), InvoicePaid: true, ExpectedError: "not-enough-money"); + async Task RunVector(bool skipLockedCheck = false) { var coin = await senderUser.ReceiveUTXO(vector.SpentCoin, network); var invoice = receiverUser.BitPay.CreateInvoice(new Invoice() {Price = vector.InvoiceAmount.ToDecimal(MoneyUnit.BTC), Currency = "BTC", FullNotifications = true}); @@ -203,47 +203,71 @@ namespace BTCPayServer.Tests var psbt = txBuilder.BuildPSBT(false); psbt = await senderUser.Sign(psbt); var pj = await senderUser.SubmitPayjoin(invoice, psbt, vector.ExpectedError); - if (vector.ExpectLocked) + if (vector.ExpectedError is null) { - Assert.True(await payjoinRepository.TryUnlock(receiverCoin.Outpoint)); + Assert.Contains(pj.Inputs, o => o.PrevOut == receiverCoin.Outpoint); + if (!skipLockedCheck) + Assert.True(await payjoinRepository.TryUnlock(receiverCoin.Outpoint)); } else { - Assert.False(await payjoinRepository.TryUnlock(receiverCoin.Outpoint)); + Assert.Null(pj); + if (!skipLockedCheck) + Assert.False(await payjoinRepository.TryUnlock(receiverCoin.Outpoint)); + } + + if (vector.InvoicePaid) + { + await TestUtils.EventuallyAsync(async () => + { + invoice = await receiverUser.BitPay.GetInvoiceAsync(invoice.Id); + Assert.Equal("paid", invoice.Status); + }); } return pj; } - async Task LockNewReceiverCoin() + async Task LockAllButReceiverCoin() { var coins = await btcPayWallet.GetUnspentCoins(receiverUser.DerivationScheme); - foreach (var coin in coins.Where(c => c.OutPoint != receiverCoin.Outpoint)) + foreach (var coin in coins) { - await payjoinRepository.TryLock(coin.OutPoint); + if (coin.OutPoint != receiverCoin.Outpoint) + await payjoinRepository.TryLock(coin.OutPoint); + else + await payjoinRepository.TryUnlock(coin.OutPoint); } } Logs.Tester.LogInformation("Here we send exactly the right amount. This should fails as\n" + "there is not enough to pay the additional payjoin input. (going below the min relay fee" + "However, the original tx has been broadcasted!"); - vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(700), Paid: Money.Satoshis(700), Fee: Money.Satoshis(110), ExpectLocked: false, ExpectedError: "not-enough-money"); + vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(700), Paid: Money.Satoshis(700), Fee: Money.Satoshis(110), InvoicePaid: true, ExpectedError: "not-enough-money"); await RunVector(); - await LockNewReceiverCoin(); + await LockAllButReceiverCoin(); Logs.Tester.LogInformation("We don't pay enough"); - vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(700), Paid: Money.Satoshis(690), Fee: Money.Satoshis(110), ExpectLocked: false, ExpectedError: "invoice-not-fully-paid"); + vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(700), Paid: Money.Satoshis(690), Fee: Money.Satoshis(110), InvoicePaid: false, ExpectedError: "invoice-not-fully-paid"); await RunVector(); Logs.Tester.LogInformation("We pay correctly"); - vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(500), Fee: Money.Satoshis(110), ExpectLocked: true, ExpectedError: null as string); + vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(500), Fee: Money.Satoshis(110), InvoicePaid: true, ExpectedError: null as string); await RunVector(); + await LockAllButReceiverCoin(); + + Logs.Tester.LogInformation("We pay a little bit more the invoice with enough fees to support additional input\n" + + "The receiver should have added a fake output"); + vector = (SpentCoin: Money.Satoshis(910), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(700), Fee: Money.Satoshis(110), InvoicePaid: true, ExpectedError: null as string); + var proposedPSBT = await RunVector(); + Assert.Equal(2, proposedPSBT.Outputs.Count); + await LockAllButReceiverCoin(); Logs.Tester.LogInformation("We pay correctly, but no utxo\n" + "However, this has the side effect of having the receiver broadcasting the original tx"); await payjoinRepository.TryLock(receiverCoin.Outpoint); - vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(500), Fee: Money.Satoshis(110), ExpectLocked: true, ExpectedError: "out-of-utxos"); - await RunVector(); - await LockNewReceiverCoin(); + vector = (SpentCoin: Money.Satoshis(810), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(500), Fee: Money.Satoshis(110), InvoicePaid: true, ExpectedError: "out-of-utxos"); + await RunVector(true); + await LockAllButReceiverCoin(); var originalSenderUser = senderUser; retry: @@ -253,8 +277,8 @@ namespace BTCPayServer.Tests // The send pay remaining 86 sat from his pocket // So total paid by sender should be 86 + 510 + 200 so we should get 1090 - (86 + 510 + 200) == 294 back) Logs.Tester.LogInformation($"Check if we can take fee on overpaid utxo{(senderUser == receiverUser ? " (to self)" : "")}"); - vector = (SpentCoin: Money.Satoshis(1090), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(510), Fee: Money.Satoshis(200), ExpectLocked: true, ExpectedError: null as string); - var proposedPSBT = await RunVector(); + vector = (SpentCoin: Money.Satoshis(1090), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(510), Fee: Money.Satoshis(200), InvoicePaid: true, ExpectedError: null as string); + proposedPSBT = await RunVector(); Assert.Equal(2, proposedPSBT.Outputs.Count); Assert.Contains(proposedPSBT.Outputs, o => o.Value == Money.Satoshis(500) + receiverCoin.Amount); Assert.Contains(proposedPSBT.Outputs, o => o.Value == Money.Satoshis(294)); @@ -276,7 +300,7 @@ namespace BTCPayServer.Tests }); tester.ExplorerNode.Generate(1); receiverCoin = await receiverUser.ReceiveUTXO(Money.Satoshis(810), network); - + await LockAllButReceiverCoin(); if (senderUser != receiverUser) { Logs.Tester.LogInformation("Let's do the same, this time paying to ourselves"); @@ -292,7 +316,7 @@ namespace BTCPayServer.Tests // Same as above. Except the sender send one satoshi less, so the change // 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); + vector = (SpentCoin: Money.Satoshis(1089), InvoiceAmount: Money.Satoshis(500), Paid: Money.Satoshis(510), Fee: Money.Satoshis(200), InvoicePaid: true, ExpectedError: null as string); proposedPSBT = await RunVector(); Assert.Equal(2, proposedPSBT.Outputs.Count); // We should have our payment diff --git a/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs b/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs index 06d5012bd..0a6d256ec 100644 --- a/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs +++ b/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs @@ -25,6 +25,7 @@ using NBXplorer.Models; using Newtonsoft.Json.Linq; using NicolasDorier.RateLimits; using Microsoft.Extensions.Logging; +using NBXplorer.DerivationStrategy; namespace BTCPayServer.Payments.PayJoin { @@ -170,14 +171,12 @@ namespace BTCPayServer.Payments.PayJoin { await _payJoinRepository.TryUnlock(selectedUTXOs.Select(o => o.Key).ToArray()); } - PSBTOutput paymentOutput = null; + PSBTOutput originalPaymentOutput = null; BitcoinAddress paymentAddress = null; InvoiceEntity invoice = null; - int ourOutputIndex = -1; DerivationSchemeSettings derivationSchemeSettings = null; foreach (var output in psbt.Outputs) { - ourOutputIndex++; var key = output.ScriptPubKey.Hash + "#" + network.CryptoCode.ToUpperInvariant(); invoice = (await _invoiceRepository.GetInvoicesFromAddresses(new[] {key})).FirstOrDefault(); if (invoice is null) @@ -222,7 +221,7 @@ namespace BTCPayServer.Payments.PayJoin selectedUTXOs.Add(utxo.Outpoint, utxo); } - paymentOutput = output; + originalPaymentOutput = output; paymentAddress = paymentDetails.GetDepositAddress(network.NBitcoinNetwork); break; } @@ -247,7 +246,7 @@ namespace BTCPayServer.Payments.PayJoin "We do not have any UTXO available for making a payjoin for now")); } - var originalPaymentValue = paymentOutput.Value; + var originalPaymentValue = originalPaymentOutput.Value; await _broadcaster.Schedule(DateTimeOffset.UtcNow + TimeSpan.FromMinutes(1.0), originalTx, network); //check if wallet of store is configured to be hot wallet @@ -261,19 +260,44 @@ namespace BTCPayServer.Payments.PayJoin await BroadcastNow(); return StatusCode(500, CreatePayjoinError(500, "unavailable", $"This service is unavailable for now")); } - + + Money contributedAmount = Money.Zero; var newTx = originalTx.Clone(); - var ourOutput = newTx.Outputs[ourOutputIndex]; + var ourNewOutput = newTx.Outputs[originalPaymentOutput.Index]; + HashSet isOurOutput = new HashSet(); + isOurOutput.Add(ourNewOutput); foreach (var selectedUTXO in selectedUTXOs.Select(o => o.Value)) { - ourOutput.Value += (Money)selectedUTXO.Value; + contributedAmount += (Money)selectedUTXO.Value; newTx.Inputs.Add(selectedUTXO.Outpoint); } + ourNewOutput.Value += contributedAmount; + var minRelayTxFee = this._dashboard.Get(network.CryptoCode).Status.BitcoinStatus?.MinRelayTxFee ?? + new FeeRate(1.0m); + + // Probably receiving some spare change, let's add an output to make + // it looks more like a normal transaction + if (newTx.Outputs.Count == 1) + { + var change = await explorer.GetUnusedAsync(derivationSchemeSettings.AccountDerivation, DerivationFeature.Change); + var randomChangeAmount = RandomUtils.GetUInt64() % (ulong)contributedAmount.Satoshi; + var fakeChange = newTx.Outputs.CreateNewTxOut(randomChangeAmount, change.ScriptPubKey); + if (fakeChange.IsDust(minRelayTxFee)) + { + randomChangeAmount = fakeChange.GetDustThreshold(minRelayTxFee); + fakeChange.Value = randomChangeAmount; + } + if (randomChangeAmount < contributedAmount) + { + ourNewOutput.Value -= fakeChange.Value; + newTx.Outputs.Add(fakeChange); + isOurOutput.Add(fakeChange); + } + } var rand = new Random(); Utils.Shuffle(newTx.Inputs, rand); Utils.Shuffle(newTx.Outputs, rand); - ourOutputIndex = newTx.Outputs.IndexOf(ourOutput); // Remove old signatures as they are not valid anymore foreach (var input in newTx.Inputs) @@ -291,32 +315,30 @@ namespace BTCPayServer.Payments.PayJoin Money additionalFee = expectedFee - actualFee; if (additionalFee > Money.Zero) { - var minRelayTxFee = this._dashboard.Get(network.CryptoCode).Status.BitcoinStatus?.MinRelayTxFee ?? - new FeeRate(1.0m); - // If the user overpaid, taking fee on our output (useful if they dump a full UTXO for privacy) - if (due < Money.Zero) + for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero && due < Money.Zero; i++) { - ourFeeContribution = Money.Min(additionalFee, -due); - ourFeeContribution = Money.Min(ourFeeContribution, - ourOutput.Value - ourOutput.GetDustThreshold(minRelayTxFee)); - ourOutput.Value -= ourFeeContribution; - additionalFee -= ourFeeContribution; + if (isOurOutput.Contains(newTx.Outputs[i])) + { + var outputContribution = Money.Min(additionalFee, -due); + outputContribution = Money.Min(outputContribution, + newTx.Outputs[i].Value - newTx.Outputs[i].GetDustThreshold(minRelayTxFee)); + newTx.Outputs[i].Value -= outputContribution; + additionalFee -= outputContribution; + ourFeeContribution += outputContribution; + } } // The rest, we take from user's change - if (additionalFee > Money.Zero) + for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero; i++) { - for (int i = 0; i < newTx.Outputs.Count && additionalFee != Money.Zero; i++) + if (!isOurOutput.Contains(newTx.Outputs[i])) { - 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; - } + 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; } } @@ -352,8 +374,8 @@ namespace BTCPayServer.Payments.PayJoin // This will make the invoice paid even if the user do not // broadcast the payjoin. var originalPaymentData = new BitcoinLikePaymentData(paymentAddress, - paymentOutput.Value, - new OutPoint(originalTx.GetHash(), paymentOutput.Index), + originalPaymentOutput.Value, + new OutPoint(originalTx.GetHash(), originalPaymentOutput.Index), originalTx.RBF); originalPaymentData.ConfirmationCount = -1; originalPaymentData.PayjoinInformation = new PayjoinInformation()