The payjoin receiver can inject a fake change

This commit is contained in:
nicolas.dorier 2020-04-07 18:14:31 +09:00
parent ba2184e21a
commit 9d2ab8b154
No known key found for this signature in database
GPG key ID: 6618763EF09186FE
2 changed files with 95 additions and 49 deletions

View file

@ -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<PSBT> 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<PSBT> 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

View file

@ -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<TxOut> isOurOutput = new HashSet<TxOut>();
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()