Adapt payjoin implementation to the BIP (#1569)

This commit is contained in:
Nicolas Dorier
2020-05-17 05:07:24 +09:00
committed by GitHub
parent e9bda50054
commit 8d1ff01ee8
7 changed files with 302 additions and 72 deletions

View File

@@ -0,0 +1,74 @@
using System.Threading.Channels;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Hosting.Server.Features;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
namespace BTCPayServer.Tests
{
public class FakeServer : IDisposable
{
IWebHost webHost;
SemaphoreSlim semaphore;
CancellationTokenSource cts = new CancellationTokenSource();
public FakeServer()
{
_channel = Channel.CreateUnbounded<HttpContext>();
semaphore = new SemaphoreSlim(0);
}
Channel<HttpContext> _channel;
public async Task Start()
{
webHost = new WebHostBuilder()
.UseKestrel()
.UseUrls("http://127.0.0.1:0")
.Configure(appBuilder =>
{
appBuilder.Run(async ctx =>
{
await _channel.Writer.WriteAsync(ctx);
await semaphore.WaitAsync(cts.Token);
});
})
.Build();
await webHost.StartAsync();
var port = new Uri(webHost.ServerFeatures.Get<IServerAddressesFeature>().Addresses.First(), UriKind.Absolute)
.Port;
ServerUri = new Uri($"http://127.0.0.1:{port}/");
}
public Uri ServerUri { get; set; }
public void Done()
{
semaphore.Release();
}
public async Task Stop()
{
await webHost.StopAsync();
}
public void Dispose()
{
cts.Dispose();
webHost?.Dispose();
semaphore.Dispose();
}
public async Task<HttpContext> GetNextRequest()
{
return await _channel.Reader.ReadAsync();
}
}
}

View File

@@ -19,6 +19,7 @@ using BTCPayServer.Services.Invoices;
using BTCPayServer.Services.Wallets;
using BTCPayServer.Tests.Logging;
using BTCPayServer.Views.Wallets;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
@@ -27,6 +28,7 @@ using NBitcoin;
using NBitcoin.Altcoins;
using NBitcoin.Payment;
using NBitpayClient;
using NBXplorer.DerivationStrategy;
using NBXplorer.Models;
using Newtonsoft.Json.Linq;
using OpenQA.Selenium;
@@ -196,7 +198,8 @@ namespace BTCPayServer.Tests
if (unsupportedFormats.Contains(receiverAddressType))
{
errorCode = "unsupported-inputs";
}else if (receiverAddressType != senderAddressType)
}
else if (receiverAddressType != senderAddressType)
{
errorCode = "out-of-utxos";
}
@@ -359,6 +362,126 @@ namespace BTCPayServer.Tests
}
}
[Fact]
[Trait("Integration", "Integration")]
public async Task CanUsePayjoin2()
{
using (var tester = ServerTester.Create())
{
await tester.StartAsync();
var pjClient = tester.PayTester.GetService<PayjoinClient>();
var nbx = tester.PayTester.GetService<ExplorerClientProvider>().GetExplorerClient("BTC");
var notifications = await nbx.CreateWebsocketNotificationSessionAsync();
var alice = tester.NewAccount();
await alice.RegisterDerivationSchemeAsync("BTC", ScriptPubKeyType.Segwit, true);
await notifications.ListenDerivationSchemesAsync(new[] { alice.DerivationScheme });
var address = (await nbx.GetUnusedAsync(alice.DerivationScheme, DerivationFeature.Deposit)).Address;
tester.ExplorerNode.SendToAddress(address, Money.Coins(1.0m));
await notifications.NextEventAsync();
var psbt = (await nbx.CreatePSBTAsync(alice.DerivationScheme, new CreatePSBTRequest()
{
Destinations =
{
new CreatePSBTDestination()
{
Amount = Money.Coins(0.5m),
Destination = new Key().PubKey.GetAddress(ScriptPubKeyType.Legacy, Network.RegTest)
}
},
FeePreference = new FeePreference()
{
ExplicitFee = Money.Satoshis(3000)
}
})).PSBT;
var derivationSchemeSettings = alice.GetController<WalletsController>().GetDerivationSchemeSettings(new WalletId(alice.StoreId, "BTC"));
var signingAccount = derivationSchemeSettings.GetSigningAccountKeySettings();
psbt.SignAll(derivationSchemeSettings.AccountDerivation, alice.GenerateWalletResponseV.AccountHDKey, signingAccount.GetRootedKeyPath());
var changeIndex = Array.FindIndex(psbt.Outputs.ToArray(), (PSBTOutput o) => o.ScriptPubKey.IsScriptType(ScriptType.P2WPKH));
using var fakeServer = new FakeServer();
await fakeServer.Start();
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]);
Logs.Tester.LogInformation("The payjoin receiver tries to make us pay lots of fee");
var originalPSBT = await ParsePSBT(request);
var proposalTx = originalPSBT.GetGlobalTransaction();
proposalTx.Outputs[changeIndex].Value -= Money.Satoshis(3001);
await request.Response.WriteAsync(PSBT.FromTransaction(proposalTx, Network.RegTest).ToBase64(), Encoding.UTF8);
fakeServer.Done();
var ex = await Assert.ThrowsAsync<PayjoinSenderException>(async () => await requesting);
Assert.Contains("too much fee", ex.Message);
Logs.Tester.LogInformation("The payjoin receiver tries to send money to himself");
requesting = pjClient.RequestPayjoin(fakeServer.ServerUri, derivationSchemeSettings, psbt, default);
request = await fakeServer.GetNextRequest();
originalPSBT = await ParsePSBT(request);
proposalTx = originalPSBT.GetGlobalTransaction();
proposalTx.Outputs.Where((o, i) => i != changeIndex).First().Value += Money.Satoshis(1);
proposalTx.Outputs[changeIndex].Value -= Money.Satoshis(1);
await request.Response.WriteAsync(PSBT.FromTransaction(proposalTx, Network.RegTest).ToBase64(), Encoding.UTF8);
fakeServer.Done();
ex = await Assert.ThrowsAsync<PayjoinSenderException>(async () => await requesting);
Assert.Contains("money to himself", ex.Message);
Logs.Tester.LogInformation("The payjoin receiver can't increase the fee rate too much");
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("increased the fee rate", ex.Message);
Logs.Tester.LogInformation("Make sure the receiver implementation do not take more fee than allowed");
var bob = tester.NewAccount();
await bob.GrantAccessAsync();
await bob.RegisterDerivationSchemeAsync("BTC", ScriptPubKeyType.Segwit, true);
await notifications.ListenDerivationSchemesAsync(new[] { bob.DerivationScheme });
address = (await nbx.GetUnusedAsync(bob.DerivationScheme, DerivationFeature.Deposit)).Address;
tester.ExplorerNode.SendToAddress(address, Money.Coins(1.1m));
await notifications.NextEventAsync();
bob.ModifyStore(s => s.PayJoinEnabled = true);
var invoice = bob.BitPay.CreateInvoice(
new Invoice() { Price = 0.1m, Currency = "BTC", FullNotifications = true });
var 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());
var endpoint = TestAccount.GetPayjoinEndpoint(invoice, Network.RegTest);
pjClient.MaxFeeBumpContribution = Money.Satoshis(50);
var proposal = await pjClient.RequestPayjoin(endpoint, derivationSchemeSettings, psbt, default);
Assert.True(proposal.TryGetFee(out var newFee));
Assert.Equal(Money.Satoshis(3001 + 50), newFee);
}
}
private static async Task<PSBT> ParsePSBT(Microsoft.AspNetCore.Http.HttpContext request)
{
var bytes = await request.Request.Body.ReadBytesAsync(int.Parse(request.Request.Headers["Content-Length"].First()));
var str = Encoding.UTF8.GetString(bytes);
return PSBT.Parse(str, Network.RegTest);
}
[Fact]
[Trait("Integration", "Integration")]
public async Task CanUsePayjoinFeeCornerCase()

View File

@@ -145,6 +145,10 @@ namespace BTCPayServer.Tests
public async Task CreateStoreAsync()
{
if (UserId is null)
{
await RegisterAsync();
}
var store = this.GetController<UserStoresController>();
await store.CreateStore(new CreateStoreViewModel() {Name = "Test Store"});
StoreId = store.CreatedStoreId;
@@ -161,6 +165,8 @@ namespace BTCPayServer.Tests
public async Task<WalletId> RegisterDerivationSchemeAsync(string cryptoCode, ScriptPubKeyType segwit = ScriptPubKeyType.Legacy,
bool importKeysToNBX = false)
{
if (StoreId is null)
await CreateStoreAsync();
SupportedNetwork = parent.NetworkProvider.GetNetwork<BTCPayNetwork>(cryptoCode);
var store = parent.PayTester.GetController<StoresController>(UserId, StoreId);
GenerateWalletResponseV = await parent.ExplorerClient.GenerateWalletAsync(new GenerateWalletRequest()

View File

@@ -1023,7 +1023,7 @@ namespace BTCPayServer.Controllers
}
}
private DerivationSchemeSettings GetDerivationSchemeSettings(WalletId walletId)
internal DerivationSchemeSettings GetDerivationSchemeSettings(WalletId walletId)
{
var paymentMethod = CurrentStore
.GetSupportedPaymentMethods(NetworkProvider)

View File

@@ -117,7 +117,7 @@ namespace BTCPayServer.Payments.PayJoin
[MediaTypeConstraint("text/plain")]
[RateLimitsFilter(ZoneLimits.PayJoin, Scope = RateLimitsScope.RemoteAddress)]
public async Task<IActionResult> Submit(string cryptoCode,
bool noadjustfee = false,
long maxfeebumpcontribution = -1,
int feebumpindex = -1,
int v = 1)
{
@@ -130,6 +130,7 @@ namespace BTCPayServer.Payments.PayJoin
new JProperty("message", "This version of payjoin is not supported.")
});
}
Money allowedFeeBumpContribution = Money.Satoshis(maxfeebumpcontribution >= 0 ? maxfeebumpcontribution : long.MaxValue);
var network = _btcPayNetworkProvider.GetNetwork<BTCPayNetwork>(cryptoCode);
if (network == null)
{
@@ -415,7 +416,7 @@ namespace BTCPayServer.Payments.PayJoin
Money expectedFee = txBuilder.EstimateFees(newTx, originalFeeRate);
Money actualFee = newTx.GetFee(txBuilder.FindSpentCoins(newTx));
Money additionalFee = expectedFee - actualFee;
if (additionalFee > Money.Zero && !noadjustfee)
if (additionalFee > Money.Zero)
{
// If the user overpaid, taking fee on our output (useful if sender dump a full UTXO for privacy)
for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero && due < Money.Zero; i++)
@@ -433,7 +434,7 @@ namespace BTCPayServer.Payments.PayJoin
}
// The rest, we take from user's change
for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero; i++)
for (int i = 0; i < newTx.Outputs.Count && additionalFee > Money.Zero && allowedFeeBumpContribution > Money.Zero; i++)
{
if (preferredFeeBumpOutput is TxOut &&
preferredFeeBumpOutput != newTx.Outputs[i])
@@ -443,8 +444,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);
newTx.Outputs[i].Value -= outputContribution;
additionalFee -= outputContribution;
allowedFeeBumpContribution -= outputContribution;
}
}

View File

@@ -41,6 +41,13 @@ namespace BTCPayServer.Services
}
}
public class PayjoinClientParameters
{
public Money MaxFeeBumpContribution { get; set; }
public int? FeeBumpIndex { get; set; }
public int Version { get; set; } = 1;
}
public class PayjoinClient
{
public const string PayjoinOnionNamedClient = "payjoin.onion";
@@ -64,6 +71,8 @@ namespace BTCPayServer.Services
_httpClientFactory = httpClientFactory;
}
public Money MaxFeeBumpContribution { get; set; }
public async Task<PSBT> RequestPayjoin(Uri endpoint, DerivationSchemeSettings derivationSchemeSettings,
PSBT originalTx, CancellationToken cancellationToken)
{
@@ -75,13 +84,17 @@ namespace BTCPayServer.Services
throw new ArgumentNullException(nameof(originalTx));
if (originalTx.IsAllFinalized())
throw new InvalidOperationException("The original PSBT should not be finalized.");
var clientParameters = new PayjoinClientParameters();
var type = derivationSchemeSettings.AccountDerivation.ScriptPubKeyType();
if (!SupportedFormats.Contains(type))
{
throw new PayjoinSenderException($"The wallet does not support payjoin");
}
var signingAccount = derivationSchemeSettings.GetSigningAccountKeySettings();
var changeOutput = originalTx.Outputs.CoinsFor(derivationSchemeSettings.AccountDerivation, signingAccount.AccountKey, signingAccount.GetRootedKeyPath())
.FirstOrDefault();
if (changeOutput is PSBTOutput o)
clientParameters.FeeBumpIndex = (int)o.Index;
var sentBefore = -originalTx.GetBalance(derivationSchemeSettings.AccountDerivation,
signingAccount.AccountKey,
signingAccount.GetRootedKeyPath());
@@ -89,11 +102,10 @@ 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;
var cloned = originalTx.Clone();
if (!cloned.TryFinalize(out var errors))
{
return null;
}
cloned.Finalize();
// We make sure we don't send unnecessary information to the receiver
foreach (var finalized in cloned.Inputs.Where(i => i.IsFinalized()))
@@ -107,6 +119,8 @@ namespace BTCPayServer.Services
}
cloned.GlobalXPubs.Clear();
endpoint = ApplyOptionalParameters(endpoint, clientParameters);
using HttpClient client = CreateHttpClient(endpoint);
var bpuresponse = await client.PostAsync(endpoint,
new StringContent(cloned.ToHex(), Encoding.UTF8, "text/plain"), cancellationToken);
@@ -206,11 +220,6 @@ namespace BTCPayServer.Services
if (ourInputCount < originalTx.Inputs.Count)
throw new PayjoinSenderException("The payjoin receiver removed some of our inputs");
// We limit the number of inputs the receiver can add
var addedInputs = newPSBT.Inputs.Count - originalTx.Inputs.Count;
if (addedInputs == 0)
throw new PayjoinSenderException("The payjoin receiver did not added any input");
var sentAfter = -newPSBT.GetBalance(derivationSchemeSettings.AccountDerivation,
signingAccount.AccountKey,
signingAccount.GetRootedKeyPath());
@@ -224,8 +233,8 @@ 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 > originalFee)
throw new PayjoinSenderException("The payjoin receiver is making us pay more than twice the original fee");
if (overPaying > clientParameters.MaxFeeBumpContribution)
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
// did not changed that much
@@ -239,6 +248,21 @@ namespace BTCPayServer.Services
return newPSBT;
}
private static Uri ApplyOptionalParameters(Uri endpoint, PayjoinClientParameters clientParameters)
{
var requestUri = endpoint.AbsoluteUri;
if (requestUri.IndexOf('?', StringComparison.OrdinalIgnoreCase) is int i && i != -1)
requestUri = requestUri.Substring(0, i);
List<string> parameters = new List<string>(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}");
endpoint = new Uri($"{requestUri}?{string.Join('&', parameters)}");
return endpoint;
}
private HttpClient CreateHttpClient(Uri uri)
{
if (uri.IsOnion())

View File

@@ -144,11 +144,11 @@
{
var feeRateOption = Model.RecommendedSatoshiPerByte[index];
<button type="button" class="btn btn-sm btn-outline-primary crypto-fee-link" value="@feeRateOption.FeeRate" data-toggle="tooltip" title="@feeRateOption.FeeRate sat/b">
<input type="hidden" asp-for="RecommendedSatoshiPerByte[index].Target" />
<input type="hidden" asp-for="RecommendedSatoshiPerByte[index].FeeRate" />
@feeRateOption.Target.TimeString()
</button>
<input type="hidden" asp-for="RecommendedSatoshiPerByte[index].Target" />
<input type="hidden" asp-for="RecommendedSatoshiPerByte[index].FeeRate" />
}
</div>
</div>