Rename payjoin optional parameters, implement sender minFeeRate

This commit is contained in:
nicolas.dorier 2020-05-17 22:21:35 +09:00
parent 2a030fe7fb
commit de3753d04e
No known key found for this signature in database
GPG key ID: 6618763EF09186FE
3 changed files with 83 additions and 27 deletions

View file

@ -402,8 +402,8 @@ namespace BTCPayServer.Tests
var requesting = pjClient.RequestPayjoin(fakeServer.ServerUri, derivationSchemeSettings, psbt, default); var requesting = pjClient.RequestPayjoin(fakeServer.ServerUri, derivationSchemeSettings, psbt, default);
var request = await fakeServer.GetNextRequest(); var request = await fakeServer.GetNextRequest();
Assert.Equal("1", request.Request.Query["v"][0]); Assert.Equal("1", request.Request.Query["v"][0]);
Assert.Equal(changeIndex.ToString(), request.Request.Query["feebumpindex"][0]); Assert.Equal(changeIndex.ToString(), request.Request.Query["additionalfeeoutputindex"][0]);
Assert.Equal("3000", request.Request.Query["maxfeebumpcontribution"][0]); Assert.Equal("3000", request.Request.Query["maxadditionalfeecontribution"][0]);
Logs.Tester.LogInformation("The payjoin receiver tries to make us pay lots of fee"); 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<PayjoinSenderException>(async () => await requesting); ex = await Assert.ThrowsAsync<PayjoinSenderException>(async () => await requesting);
Assert.Contains("increased the fee rate", ex.Message); 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<PayjoinSenderException>(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"); Logs.Tester.LogInformation("Make sure the receiver implementation do not take more fee than allowed");
var bob = tester.NewAccount(); var bob = tester.NewAccount();
await bob.GrantAccessAsync(); await bob.GrantAccessAsync();
@ -472,6 +485,36 @@ namespace BTCPayServer.Tests
var proposal = await pjClient.RequestPayjoin(endpoint, derivationSchemeSettings, psbt, default); var proposal = await pjClient.RequestPayjoin(endpoint, derivationSchemeSettings, psbt, default);
Assert.True(proposal.TryGetFee(out var newFee)); Assert.True(proposal.TryGetFee(out var newFee));
Assert.Equal(Money.Satoshis(3001 + 50), 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<PayjoinReceiverException>(async () => await pjClient.RequestPayjoin(endpoint, derivationSchemeSettings, psbt, default));
Assert.Equal(PayjoinReceiverWellknownErrors.NotEnoughMoney, ex2.WellknownError);
} }
} }

View file

@ -117,8 +117,9 @@ namespace BTCPayServer.Payments.PayJoin
[MediaTypeConstraint("text/plain")] [MediaTypeConstraint("text/plain")]
[RateLimitsFilter(ZoneLimits.PayJoin, Scope = RateLimitsScope.RemoteAddress)] [RateLimitsFilter(ZoneLimits.PayJoin, Scope = RateLimitsScope.RemoteAddress)]
public async Task<IActionResult> Submit(string cryptoCode, public async Task<IActionResult> Submit(string cryptoCode,
long maxfeebumpcontribution = -1, long maxadditionalfeecontribution = -1,
int feebumpindex = -1, int additionalfeeoutputindex = -1,
decimal minfeerate = -1.0m,
int v = 1) int v = 1)
{ {
if (v != 1) if (v != 1)
@ -130,7 +131,8 @@ namespace BTCPayServer.Payments.PayJoin
new JProperty("message", "This version of payjoin is not supported.") 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<BTCPayNetwork>(cryptoCode); var network = _btcPayNetworkProvider.GetNetwork<BTCPayNetwork>(cryptoCode);
if (network == null) if (network == null)
{ {
@ -345,10 +347,10 @@ namespace BTCPayServer.Payments.PayJoin
var ourNewOutput = newTx.Outputs[originalPaymentOutput.Index]; var ourNewOutput = newTx.Outputs[originalPaymentOutput.Index];
HashSet<TxOut> isOurOutput = new HashSet<TxOut>(); HashSet<TxOut> isOurOutput = new HashSet<TxOut>();
isOurOutput.Add(ourNewOutput); isOurOutput.Add(ourNewOutput);
TxOut preferredFeeBumpOutput = feebumpindex >= 0 TxOut preferredFeeBumpOutput = additionalfeeoutputindex >= 0
&& feebumpindex < newTx.Outputs.Count && additionalfeeoutputindex < newTx.Outputs.Count
&& !isOurOutput.Contains(newTx.Outputs[feebumpindex]) && !isOurOutput.Contains(newTx.Outputs[additionalfeeoutputindex])
? newTx.Outputs[feebumpindex] : null; ? newTx.Outputs[additionalfeeoutputindex] : null;
var rand = new Random(); var rand = new Random();
int senderInputCount = newTx.Inputs.Count; int senderInputCount = newTx.Inputs.Count;
foreach (var selectedUTXO in selectedUTXOs.Select(o => o.Value)) 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 // 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 && if (preferredFeeBumpOutput is TxOut &&
preferredFeeBumpOutput != newTx.Outputs[i]) preferredFeeBumpOutput != newTx.Outputs[i])
@ -444,10 +446,10 @@ namespace BTCPayServer.Payments.PayJoin
var outputContribution = Money.Min(additionalFee, newTx.Outputs[i].Value); var outputContribution = Money.Min(additionalFee, newTx.Outputs[i].Value);
outputContribution = Money.Min(outputContribution, outputContribution = Money.Min(outputContribution,
newTx.Outputs[i].Value - newTx.Outputs[i].GetDustThreshold(minRelayTxFee)); newTx.Outputs[i].Value - newTx.Outputs[i].GetDustThreshold(minRelayTxFee));
outputContribution = Money.Min(outputContribution, allowedFeeBumpContribution); outputContribution = Money.Min(outputContribution, allowedSenderFeeContribution);
newTx.Outputs[i].Value -= outputContribution; newTx.Outputs[i].Value -= outputContribution;
additionalFee -= 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. // we are not under the relay fee, it should be OK.
var newVSize = txBuilder.EstimateSize(newTx, true); var newVSize = txBuilder.EstimateSize(newTx, true);
var newFeePaid = newTx.GetFee(txBuilder.FindSpentCoins(newTx)); var newFeePaid = newTx.GetFee(txBuilder.FindSpentCoins(newTx));
if (new FeeRate(newFeePaid, newVSize) < minRelayTxFee) if (new FeeRate(newFeePaid, newVSize) < (senderMinFeeRate ?? minRelayTxFee))
{ {
await UnlockUTXOs(); await UnlockUTXOs();
await BroadcastNow(); await BroadcastNow();

View file

@ -1,6 +1,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Configuration; using System.Configuration;
using System.Globalization;
using System.Linq; using System.Linq;
using System.Net.Http; using System.Net.Http;
using System.Text; using System.Text;
@ -43,8 +44,9 @@ namespace BTCPayServer.Services
public class PayjoinClientParameters public class PayjoinClientParameters
{ {
public Money MaxFeeBumpContribution { get; set; } public Money MaxAdditionalFeeContribution { get; set; }
public int? FeeBumpIndex { get; set; } public FeeRate MinFeeRate { get; set; }
public int? AdditionalFeeOutputIndex { get; set; }
public int Version { get; set; } = 1; public int Version { get; set; } = 1;
} }
@ -72,6 +74,7 @@ namespace BTCPayServer.Services
} }
public Money MaxFeeBumpContribution { get; set; } public Money MaxFeeBumpContribution { get; set; }
public FeeRate MinimumFeeRate { get; set; }
public async Task<PSBT> RequestPayjoin(Uri endpoint, DerivationSchemeSettings derivationSchemeSettings, public async Task<PSBT> RequestPayjoin(Uri endpoint, DerivationSchemeSettings derivationSchemeSettings,
PSBT originalTx, CancellationToken cancellationToken) PSBT originalTx, CancellationToken cancellationToken)
@ -94,7 +97,7 @@ namespace BTCPayServer.Services
var changeOutput = originalTx.Outputs.CoinsFor(derivationSchemeSettings.AccountDerivation, signingAccount.AccountKey, signingAccount.GetRootedKeyPath()) var changeOutput = originalTx.Outputs.CoinsFor(derivationSchemeSettings.AccountDerivation, signingAccount.AccountKey, signingAccount.GetRootedKeyPath())
.FirstOrDefault(); .FirstOrDefault();
if (changeOutput is PSBTOutput o) if (changeOutput is PSBTOutput o)
clientParameters.FeeBumpIndex = (int)o.Index; clientParameters.AdditionalFeeOutputIndex = (int)o.Index;
var sentBefore = -originalTx.GetBalance(derivationSchemeSettings.AccountDerivation, var sentBefore = -originalTx.GetBalance(derivationSchemeSettings.AccountDerivation,
signingAccount.AccountKey, signingAccount.AccountKey,
signingAccount.GetRootedKeyPath()); signingAccount.GetRootedKeyPath());
@ -102,8 +105,9 @@ namespace BTCPayServer.Services
if (!originalTx.TryGetEstimatedFeeRate(out var originalFeeRate) || !originalTx.TryGetVirtualSize(out var oldVirtualSize)) if (!originalTx.TryGetEstimatedFeeRate(out var originalFeeRate) || !originalTx.TryGetVirtualSize(out var oldVirtualSize))
throw new ArgumentException("originalTx should have utxo information", nameof(originalTx)); throw new ArgumentException("originalTx should have utxo information", nameof(originalTx));
var originalFee = originalTx.GetFee(); var originalFee = originalTx.GetFee();
clientParameters.MaxAdditionalFeeContribution = MaxFeeBumpContribution is null ? originalFee : MaxFeeBumpContribution;
clientParameters.MaxFeeBumpContribution = MaxFeeBumpContribution is null ? originalFee : MaxFeeBumpContribution; if (MinimumFeeRate is FeeRate v)
clientParameters.MinFeeRate = v;
var cloned = originalTx.Clone(); var cloned = originalTx.Clone();
cloned.Finalize(); cloned.Finalize();
@ -220,20 +224,25 @@ namespace BTCPayServer.Services
if (ourInputCount < originalTx.Inputs.Count) if (ourInputCount < originalTx.Inputs.Count)
throw new PayjoinSenderException("The payjoin receiver removed some of our inputs"); 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, var sentAfter = -newPSBT.GetBalance(derivationSchemeSettings.AccountDerivation,
signingAccount.AccountKey, signingAccount.AccountKey,
signingAccount.GetRootedKeyPath()); signingAccount.GetRootedKeyPath());
if (sentAfter > sentBefore) if (sentAfter > sentBefore)
{ {
var overPaying = 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; var additionalFee = newPSBT.GetFee() - originalFee;
if (overPaying > additionalFee) if (overPaying > additionalFee)
throw new PayjoinSenderException("The payjoin receiver is sending more money to himself"); 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"); 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 // 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); requestUri = requestUri.Substring(0, i);
List<string> parameters = new List<string>(3); List<string> parameters = new List<string>(3);
parameters.Add($"v={clientParameters.Version}"); parameters.Add($"v={clientParameters.Version}");
if (clientParameters.FeeBumpIndex is int feeBumpIndex) if (clientParameters.AdditionalFeeOutputIndex is int additionalFeeOutputIndex)
parameters.Add($"feebumpindex={feeBumpIndex}"); parameters.Add($"additionalfeeoutputindex={additionalFeeOutputIndex.ToString(CultureInfo.InvariantCulture)}");
if (clientParameters.MaxFeeBumpContribution is Money maxFeeBumpContribution) if (clientParameters.MaxAdditionalFeeContribution is Money maxAdditionalFeeContribution)
parameters.Add($"maxfeebumpcontribution={maxFeeBumpContribution.Satoshi}"); 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)}"); endpoint = new Uri($"{requestUri}?{string.Join('&', parameters)}");
return endpoint; return endpoint;
} }