From de3753d04ea6a748dd7510d004ef946195554117 Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Sun, 17 May 2020 22:21:35 +0900 Subject: [PATCH] Rename payjoin optional parameters, implement sender minFeeRate --- BTCPayServer.Tests/PayJoinTests.cs | 47 ++++++++++++++++++- .../PayJoin/PayJoinEndpointController.cs | 24 +++++----- BTCPayServer/Services/PayjoinClient.cs | 39 +++++++++------ 3 files changed, 83 insertions(+), 27 deletions(-) diff --git a/BTCPayServer.Tests/PayJoinTests.cs b/BTCPayServer.Tests/PayJoinTests.cs index 0e72be229..83c4b536c 100644 --- a/BTCPayServer.Tests/PayJoinTests.cs +++ b/BTCPayServer.Tests/PayJoinTests.cs @@ -402,8 +402,8 @@ namespace BTCPayServer.Tests var requesting = pjClient.RequestPayjoin(fakeServer.ServerUri, derivationSchemeSettings, psbt, default); var request = await fakeServer.GetNextRequest(); Assert.Equal("1", request.Request.Query["v"][0]); - Assert.Equal(changeIndex.ToString(), request.Request.Query["feebumpindex"][0]); - Assert.Equal("3000", request.Request.Query["maxfeebumpcontribution"][0]); + Assert.Equal(changeIndex.ToString(), request.Request.Query["additionalfeeoutputindex"][0]); + Assert.Equal("3000", request.Request.Query["maxadditionalfeecontribution"][0]); Logs.Tester.LogInformation("The payjoin receiver tries to make us pay lots of fee"); @@ -438,6 +438,19 @@ namespace BTCPayServer.Tests ex = await Assert.ThrowsAsync(async () => await requesting); Assert.Contains("increased the fee rate", ex.Message); + Logs.Tester.LogInformation("The payjoin receiver can't decrease the fee rate too much"); + pjClient.MinimumFeeRate = new FeeRate(50m); + requesting = pjClient.RequestPayjoin(fakeServer.ServerUri, derivationSchemeSettings, psbt, default); + request = await fakeServer.GetNextRequest(); + originalPSBT = await ParsePSBT(request); + proposalTx = originalPSBT.GetGlobalTransaction(); + proposalTx.Outputs[changeIndex].Value -= Money.Satoshis(3000); + await request.Response.WriteAsync(PSBT.FromTransaction(proposalTx, Network.RegTest).ToBase64(), Encoding.UTF8); + fakeServer.Done(); + ex = await Assert.ThrowsAsync(async () => await requesting); + Assert.Contains("a too low fee rate", ex.Message); + pjClient.MinimumFeeRate = null; + Logs.Tester.LogInformation("Make sure the receiver implementation do not take more fee than allowed"); var bob = tester.NewAccount(); await bob.GrantAccessAsync(); @@ -472,6 +485,36 @@ namespace BTCPayServer.Tests var proposal = await pjClient.RequestPayjoin(endpoint, derivationSchemeSettings, psbt, default); Assert.True(proposal.TryGetFee(out var newFee)); Assert.Equal(Money.Satoshis(3001 + 50), newFee); + proposal = proposal.SignAll(derivationSchemeSettings.AccountDerivation, alice.GenerateWalletResponseV.AccountHDKey, signingAccount.GetRootedKeyPath()); + proposal.Finalize(); + await tester.ExplorerNode.SendRawTransactionAsync(proposal.ExtractTransaction()); + await notifications.NextEventAsync(); + + Logs.Tester.LogInformation("Abusing minFeeRate should give not enough money error"); + invoice = bob.BitPay.CreateInvoice( + new Invoice() { Price = 0.1m, Currency = "BTC", FullNotifications = true }); + invoiceBIP21 = new BitcoinUrlBuilder(invoice.CryptoInfo.First().PaymentUrls.BIP21, + tester.ExplorerClient.Network.NBitcoinNetwork); + psbt = (await nbx.CreatePSBTAsync(alice.DerivationScheme, new CreatePSBTRequest() + { + Destinations = + { + new CreatePSBTDestination() + { + Amount = invoiceBIP21.Amount, + Destination = invoiceBIP21.Address + } + }, + FeePreference = new FeePreference() + { + ExplicitFee = Money.Satoshis(3001) + } + })).PSBT; + psbt.SignAll(derivationSchemeSettings.AccountDerivation, alice.GenerateWalletResponseV.AccountHDKey, signingAccount.GetRootedKeyPath()); + endpoint = TestAccount.GetPayjoinEndpoint(invoice, Network.RegTest); + pjClient.MinimumFeeRate = new FeeRate(100_000_000.2m); + var ex2 = await Assert.ThrowsAsync(async () => await pjClient.RequestPayjoin(endpoint, derivationSchemeSettings, psbt, default)); + Assert.Equal(PayjoinReceiverWellknownErrors.NotEnoughMoney, ex2.WellknownError); } } diff --git a/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs b/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs index 6ce47cbd3..2eb5b5841 100644 --- a/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs +++ b/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs @@ -117,8 +117,9 @@ namespace BTCPayServer.Payments.PayJoin [MediaTypeConstraint("text/plain")] [RateLimitsFilter(ZoneLimits.PayJoin, Scope = RateLimitsScope.RemoteAddress)] public async Task Submit(string cryptoCode, - long maxfeebumpcontribution = -1, - int feebumpindex = -1, + long maxadditionalfeecontribution = -1, + int additionalfeeoutputindex = -1, + decimal minfeerate = -1.0m, int v = 1) { if (v != 1) @@ -130,7 +131,8 @@ namespace BTCPayServer.Payments.PayJoin new JProperty("message", "This version of payjoin is not supported.") }); } - Money allowedFeeBumpContribution = Money.Satoshis(maxfeebumpcontribution >= 0 ? maxfeebumpcontribution : long.MaxValue); + FeeRate senderMinFeeRate = minfeerate < 0.0m ? null : new FeeRate(minfeerate); + Money allowedSenderFeeContribution = Money.Satoshis(maxadditionalfeecontribution >= 0 ? maxadditionalfeecontribution : long.MaxValue); var network = _btcPayNetworkProvider.GetNetwork(cryptoCode); if (network == null) { @@ -345,10 +347,10 @@ namespace BTCPayServer.Payments.PayJoin var ourNewOutput = newTx.Outputs[originalPaymentOutput.Index]; HashSet isOurOutput = new HashSet(); isOurOutput.Add(ourNewOutput); - TxOut preferredFeeBumpOutput = feebumpindex >= 0 - && feebumpindex < newTx.Outputs.Count - && !isOurOutput.Contains(newTx.Outputs[feebumpindex]) - ? newTx.Outputs[feebumpindex] : null; + TxOut preferredFeeBumpOutput = additionalfeeoutputindex >= 0 + && additionalfeeoutputindex < newTx.Outputs.Count + && !isOurOutput.Contains(newTx.Outputs[additionalfeeoutputindex]) + ? newTx.Outputs[additionalfeeoutputindex] : null; var rand = new Random(); int senderInputCount = newTx.Inputs.Count; foreach (var selectedUTXO in selectedUTXOs.Select(o => o.Value)) @@ -434,7 +436,7 @@ namespace BTCPayServer.Payments.PayJoin } // The rest, we take from user's change - for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero && allowedFeeBumpContribution > Money.Zero; i++) + for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero && allowedSenderFeeContribution > Money.Zero; i++) { if (preferredFeeBumpOutput is TxOut && preferredFeeBumpOutput != newTx.Outputs[i]) @@ -444,10 +446,10 @@ namespace BTCPayServer.Payments.PayJoin var outputContribution = Money.Min(additionalFee, newTx.Outputs[i].Value); outputContribution = Money.Min(outputContribution, newTx.Outputs[i].Value - newTx.Outputs[i].GetDustThreshold(minRelayTxFee)); - outputContribution = Money.Min(outputContribution, allowedFeeBumpContribution); + outputContribution = Money.Min(outputContribution, allowedSenderFeeContribution); newTx.Outputs[i].Value -= outputContribution; additionalFee -= outputContribution; - allowedFeeBumpContribution -= outputContribution; + allowedSenderFeeContribution -= outputContribution; } } @@ -457,7 +459,7 @@ namespace BTCPayServer.Payments.PayJoin // we are not under the relay fee, it should be OK. var newVSize = txBuilder.EstimateSize(newTx, true); var newFeePaid = newTx.GetFee(txBuilder.FindSpentCoins(newTx)); - if (new FeeRate(newFeePaid, newVSize) < minRelayTxFee) + if (new FeeRate(newFeePaid, newVSize) < (senderMinFeeRate ?? minRelayTxFee)) { await UnlockUTXOs(); await BroadcastNow(); diff --git a/BTCPayServer/Services/PayjoinClient.cs b/BTCPayServer/Services/PayjoinClient.cs index 658a5f69d..b8b936041 100644 --- a/BTCPayServer/Services/PayjoinClient.cs +++ b/BTCPayServer/Services/PayjoinClient.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Configuration; +using System.Globalization; using System.Linq; using System.Net.Http; using System.Text; @@ -43,8 +44,9 @@ namespace BTCPayServer.Services public class PayjoinClientParameters { - public Money MaxFeeBumpContribution { get; set; } - public int? FeeBumpIndex { get; set; } + public Money MaxAdditionalFeeContribution { get; set; } + public FeeRate MinFeeRate { get; set; } + public int? AdditionalFeeOutputIndex { get; set; } public int Version { get; set; } = 1; } @@ -72,6 +74,7 @@ namespace BTCPayServer.Services } public Money MaxFeeBumpContribution { get; set; } + public FeeRate MinimumFeeRate { get; set; } public async Task RequestPayjoin(Uri endpoint, DerivationSchemeSettings derivationSchemeSettings, PSBT originalTx, CancellationToken cancellationToken) @@ -94,7 +97,7 @@ namespace BTCPayServer.Services var changeOutput = originalTx.Outputs.CoinsFor(derivationSchemeSettings.AccountDerivation, signingAccount.AccountKey, signingAccount.GetRootedKeyPath()) .FirstOrDefault(); if (changeOutput is PSBTOutput o) - clientParameters.FeeBumpIndex = (int)o.Index; + clientParameters.AdditionalFeeOutputIndex = (int)o.Index; var sentBefore = -originalTx.GetBalance(derivationSchemeSettings.AccountDerivation, signingAccount.AccountKey, signingAccount.GetRootedKeyPath()); @@ -102,8 +105,9 @@ namespace BTCPayServer.Services if (!originalTx.TryGetEstimatedFeeRate(out var originalFeeRate) || !originalTx.TryGetVirtualSize(out var oldVirtualSize)) throw new ArgumentException("originalTx should have utxo information", nameof(originalTx)); var originalFee = originalTx.GetFee(); - - clientParameters.MaxFeeBumpContribution = MaxFeeBumpContribution is null ? originalFee : MaxFeeBumpContribution; + clientParameters.MaxAdditionalFeeContribution = MaxFeeBumpContribution is null ? originalFee : MaxFeeBumpContribution; + if (MinimumFeeRate is FeeRate v) + clientParameters.MinFeeRate = v; var cloned = originalTx.Clone(); cloned.Finalize(); @@ -220,20 +224,25 @@ namespace BTCPayServer.Services if (ourInputCount < originalTx.Inputs.Count) throw new PayjoinSenderException("The payjoin receiver removed some of our inputs"); + if (!newPSBT.TryGetEstimatedFeeRate(out var newFeeRate) || !newPSBT.TryGetVirtualSize(out var newVirtualSize)) + throw new PayjoinSenderException("The payjoin receiver did not included UTXO information to calculate fee correctly"); + + if (clientParameters.MinFeeRate is FeeRate minFeeRate) + { + if (newFeeRate < minFeeRate) + throw new PayjoinSenderException("The payjoin receiver created a payjoin with a too low fee rate"); + } + var sentAfter = -newPSBT.GetBalance(derivationSchemeSettings.AccountDerivation, signingAccount.AccountKey, signingAccount.GetRootedKeyPath()); if (sentAfter > sentBefore) { var overPaying = sentAfter - sentBefore; - - if (!newPSBT.TryGetEstimatedFeeRate(out var newFeeRate) || !newPSBT.TryGetVirtualSize(out var newVirtualSize)) - throw new PayjoinSenderException("The payjoin receiver did not included UTXO information to calculate fee correctly"); - var additionalFee = newPSBT.GetFee() - originalFee; if (overPaying > additionalFee) throw new PayjoinSenderException("The payjoin receiver is sending more money to himself"); - if (overPaying > clientParameters.MaxFeeBumpContribution) + if (overPaying > clientParameters.MaxAdditionalFeeContribution) throw new PayjoinSenderException("The payjoin receiver is making us pay too much fee"); // Let's check the difference is only for the fee and that feerate @@ -255,10 +264,12 @@ namespace BTCPayServer.Services requestUri = requestUri.Substring(0, i); List parameters = new List(3); parameters.Add($"v={clientParameters.Version}"); - if (clientParameters.FeeBumpIndex is int feeBumpIndex) - parameters.Add($"feebumpindex={feeBumpIndex}"); - if (clientParameters.MaxFeeBumpContribution is Money maxFeeBumpContribution) - parameters.Add($"maxfeebumpcontribution={maxFeeBumpContribution.Satoshi}"); + if (clientParameters.AdditionalFeeOutputIndex is int additionalFeeOutputIndex) + parameters.Add($"additionalfeeoutputindex={additionalFeeOutputIndex.ToString(CultureInfo.InvariantCulture)}"); + if (clientParameters.MaxAdditionalFeeContribution is Money maxAdditionalFeeContribution) + parameters.Add($"maxadditionalfeecontribution={maxAdditionalFeeContribution.Satoshi.ToString(CultureInfo.InvariantCulture)}"); + if (clientParameters.MinFeeRate is FeeRate minFeeRate) + parameters.Add($"minfeerate={minFeeRate.SatoshiPerByte.ToString(CultureInfo.InvariantCulture)}"); endpoint = new Uri($"{requestUri}?{string.Join('&', parameters)}"); return endpoint; }