From bec888da19efd97e8041f6c6a3c2b9e1b0d55601 Mon Sep 17 00:00:00 2001 From: Andrew Camilleri Date: Sat, 23 Jul 2022 13:26:13 +0200 Subject: [PATCH] Payjoin label fixes (#3986) * Payjoin label fixes * When a payjoin label was applied, coin selection filter would not work * When a payjoin happened with a receive address wallet, the payjoin label was not applied * Coin selection shows when a utxo is currently reserved for a payjoin. Applies both to UI and to GF API * remove reserved label * Update BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs --- .../Contracts/IUTXOLocker.cs | 13 +++++++ BTCPayServer.Tests/PayJoinTests.cs | 8 ++--- ...GreenfieldStoreOnChainWalletsController.cs | 7 +++- .../Controllers/UIWalletsController.cs | 29 +++++---------- .../TransactionLabelMarkerHostedService.cs | 10 ------ .../Payments/Bitcoin/NBXplorerListener.cs | 8 ++--- .../PayJoin/PayJoinEndpointController.cs | 35 +++++++++++-------- .../Payments/PayJoin/PayJoinExtensions.cs | 3 +- .../Payments/PayJoin/PayJoinRepository.cs | 26 ++++++++------ .../PayJoin/PayjoinReceiverContext.cs | 8 ++--- BTCPayServer/Services/Labels/LabelFactory.cs | 6 +++- 11 files changed, 82 insertions(+), 71 deletions(-) create mode 100644 BTCPayServer.Abstractions/Contracts/IUTXOLocker.cs diff --git a/BTCPayServer.Abstractions/Contracts/IUTXOLocker.cs b/BTCPayServer.Abstractions/Contracts/IUTXOLocker.cs new file mode 100644 index 000000000..6fa9a1d07 --- /dev/null +++ b/BTCPayServer.Abstractions/Contracts/IUTXOLocker.cs @@ -0,0 +1,13 @@ +using System.Collections.Generic; +using System.Threading.Tasks; +using NBitcoin; + +namespace BTCPayServer.Payments.PayJoin; + +public interface IUTXOLocker +{ + Task TryLock(OutPoint outpoint); + Task TryUnlock(params OutPoint[] outPoints); + Task TryLockInputs(OutPoint[] outPoints); + Task> FindLocks(OutPoint[] outpoints); +} diff --git a/BTCPayServer.Tests/PayJoinTests.cs b/BTCPayServer.Tests/PayJoinTests.cs index 7d8b09059..c1fa68aae 100644 --- a/BTCPayServer.Tests/PayJoinTests.cs +++ b/BTCPayServer.Tests/PayJoinTests.cs @@ -69,7 +69,7 @@ namespace BTCPayServer.Tests using var tester = CreateServerTester(); await tester.StartAsync(); var network = tester.NetworkProvider.GetNetwork("BTC"); - var repo = tester.PayTester.GetService(); + var repo = tester.PayTester.GetService(); var outpoint = RandomOutpoint(); // Should not be locked @@ -166,7 +166,7 @@ namespace BTCPayServer.Tests using var tester = CreateServerTester(); await tester.StartAsync(); var broadcaster = tester.PayTester.GetService(); - var payjoinRepository = tester.PayTester.GetService(); + var payjoinRepository = tester.PayTester.GetService(); broadcaster.Disable(); var network = tester.NetworkProvider.GetNetwork("BTC"); var btcPayWallet = tester.PayTester.GetService().GetWallet(network); @@ -633,7 +633,7 @@ namespace BTCPayServer.Tests { await tester.StartAsync(); var broadcaster = tester.PayTester.GetService(); - var payjoinRepository = tester.PayTester.GetService(); + var payjoinRepository = tester.PayTester.GetService(); broadcaster.Disable(); var network = tester.NetworkProvider.GetNetwork("BTC"); var btcPayWallet = tester.PayTester.GetService().GetWallet(network); @@ -1155,7 +1155,7 @@ retry: Assert.True(invoiceEntity.GetPayments(false).All(p => !p.Accounted)); ourOutpoint = invoiceEntity.GetAllBitcoinPaymentData(false).First().PayjoinInformation.ContributedOutPoints[0]; }); - var payjoinRepository = tester.PayTester.GetService(); + var payjoinRepository = tester.PayTester.GetService(); // The outpoint should now be available for next pj selection Assert.False(await payjoinRepository.TryUnlock(ourOutpoint)); } diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldStoreOnChainWalletsController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldStoreOnChainWalletsController.cs index d8730d432..087c4cfb3 100644 --- a/BTCPayServer/Controllers/GreenField/GreenfieldStoreOnChainWalletsController.cs +++ b/BTCPayServer/Controllers/GreenField/GreenfieldStoreOnChainWalletsController.cs @@ -53,6 +53,7 @@ namespace BTCPayServer.Controllers.Greenfield private readonly WalletReceiveService _walletReceiveService; private readonly IFeeProviderFactory _feeProviderFactory; private readonly LabelFactory _labelFactory; + private readonly UTXOLocker _utxoLocker; public GreenfieldStoreOnChainWalletsController( IAuthorizationService authorizationService, @@ -68,7 +69,8 @@ namespace BTCPayServer.Controllers.Greenfield EventAggregator eventAggregator, WalletReceiveService walletReceiveService, IFeeProviderFactory feeProviderFactory, - LabelFactory labelFactory + LabelFactory labelFactory, + UTXOLocker utxoLocker ) { _authorizationService = authorizationService; @@ -85,6 +87,7 @@ namespace BTCPayServer.Controllers.Greenfield _walletReceiveService = walletReceiveService; _feeProviderFactory = feeProviderFactory; _labelFactory = labelFactory; + _utxoLocker = utxoLocker; } [Authorize(Policy = Policies.CanModifyStoreSettings, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] @@ -317,9 +320,11 @@ namespace BTCPayServer.Controllers.Greenfield var walletId = new WalletId(storeId, cryptoCode); var walletTransactionsInfoAsync = await _walletRepository.GetWalletTransactionsInfo(walletId); var utxos = await wallet.GetUnspentCoins(derivationScheme.AccountDerivation); + return Ok(utxos.Select(coin => { walletTransactionsInfoAsync.TryGetValue(coin.OutPoint.Hash.ToString(), out var info); + var labels = info?.Labels ?? new Dictionary(); return new OnChainWalletUTXOData() { Outpoint = coin.OutPoint, diff --git a/BTCPayServer/Controllers/UIWalletsController.cs b/BTCPayServer/Controllers/UIWalletsController.cs index 4050fbad2..e12c39d00 100644 --- a/BTCPayServer/Controllers/UIWalletsController.cs +++ b/BTCPayServer/Controllers/UIWalletsController.cs @@ -65,11 +65,7 @@ namespace BTCPayServer.Controllers private readonly PayjoinClient _payjoinClient; private readonly LabelFactory _labelFactory; private readonly PullPaymentHostedService _pullPaymentHostedService; - private readonly ApplicationDbContextFactory _dbContextFactory; - private readonly BTCPayNetworkJsonSerializerSettings _jsonSerializerSettings; - private readonly PullPaymentHostedService _pullPaymentService; - private readonly IEnumerable _payoutHandlers; - private readonly NBXplorerConnectionFactory _connectionFactory; + private readonly UTXOLocker _utxoLocker; private readonly WalletHistogramService _walletHistogramService; readonly CurrencyNameTable _currencyTable; @@ -79,10 +75,8 @@ namespace BTCPayServer.Controllers CurrencyNameTable currencyTable, BTCPayNetworkProvider networkProvider, UserManager userManager, - MvcNewtonsoftJsonOptions mvcJsonOptions, NBXplorerDashboard dashboard, WalletHistogramService walletHistogramService, - NBXplorerConnectionFactory connectionFactory, RateFetcher rateProvider, IAuthorizationService authorizationService, ExplorerClientProvider explorerProvider, @@ -94,12 +88,9 @@ namespace BTCPayServer.Controllers DelayedTransactionBroadcaster broadcaster, PayjoinClient payjoinClient, LabelFactory labelFactory, - ApplicationDbContextFactory dbContextFactory, - BTCPayNetworkJsonSerializerSettings jsonSerializerSettings, - PullPaymentHostedService pullPaymentService, - IEnumerable payoutHandlers, IServiceProvider serviceProvider, - PullPaymentHostedService pullPaymentHostedService) + PullPaymentHostedService pullPaymentHostedService, + UTXOLocker utxoLocker) { _currencyTable = currencyTable; Repository = repo; @@ -119,12 +110,8 @@ namespace BTCPayServer.Controllers _payjoinClient = payjoinClient; _labelFactory = labelFactory; _pullPaymentHostedService = pullPaymentHostedService; - _dbContextFactory = dbContextFactory; - _jsonSerializerSettings = jsonSerializerSettings; - _pullPaymentService = pullPaymentService; - _payoutHandlers = payoutHandlers; + _utxoLocker = utxoLocker; ServiceProvider = serviceProvider; - _connectionFactory = connectionFactory; _walletHistogramService = walletHistogramService; } @@ -625,15 +612,15 @@ namespace BTCPayServer.Controllers vm.InputsAvailable = utxos.Select(coin => { walletTransactionsInfoAsync.TryGetValue(coin.OutPoint.Hash.ToString(), out var info); + var labels = info?.Labels == null + ? new List() + : _labelFactory.ColorizeTransactionLabels(walletBlobAsync, info, Request).ToList(); return new WalletSendModel.InputSelectionOption() { Outpoint = coin.OutPoint.ToString(), Amount = coin.Value.GetValue(network), Comment = info?.Comment, - Labels = - info == null - ? null - : _labelFactory.ColorizeTransactionLabels(walletBlobAsync, info, Request), + Labels = labels, Link = string.Format(CultureInfo.InvariantCulture, network.BlockExplorerLink, coin.OutPoint.Hash.ToString()), Confirmations = coin.Confirmations diff --git a/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs b/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs index 595453ea1..ac1fa9763 100644 --- a/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs +++ b/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs @@ -14,8 +14,6 @@ using BTCPayServer.Services.Apps; using BTCPayServer.Services.Labels; using BTCPayServer.Services.PaymentRequests; using NBitcoin; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; namespace BTCPayServer.HostedServices { @@ -48,14 +46,6 @@ namespace BTCPayServer.HostedServices { UpdateTransactionLabel.InvoiceLabelTemplate(invoiceEvent.Invoice.Id) }; - - if (invoiceEvent.Invoice.GetPayments(invoiceEvent.Payment.GetCryptoCode(), false).Any(entity => - entity.GetCryptoPaymentData() is BitcoinLikePaymentData pData && - pData.PayjoinInformation?.CoinjoinTransactionHash == transactionId)) - { - labels.Add(UpdateTransactionLabel.PayjoinLabelTemplate()); - } - foreach (var paymentId in PaymentRequestRepository.GetPaymentIdsFromInternalTags(invoiceEvent.Invoice)) { labels.Add(UpdateTransactionLabel.PaymentRequestLabelTemplate(paymentId)); diff --git a/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs b/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs index 97b899121..f58c7c800 100644 --- a/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs +++ b/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs @@ -27,7 +27,7 @@ namespace BTCPayServer.Payments.Bitcoin public class NBXplorerListener : IHostedService { readonly EventAggregator _Aggregator; - private readonly PayJoinRepository _payJoinRepository; + private readonly UTXOLocker _utxoLocker; readonly ExplorerClientProvider _ExplorerClients; private readonly PaymentService _paymentService; readonly InvoiceRepository _InvoiceRepository; @@ -38,7 +38,7 @@ namespace BTCPayServer.Payments.Bitcoin BTCPayWalletProvider wallets, InvoiceRepository invoiceRepository, EventAggregator aggregator, - PayJoinRepository payjoinRepository, + UTXOLocker payjoinRepository, PaymentService paymentService, Logs logs) { @@ -48,7 +48,7 @@ namespace BTCPayServer.Payments.Bitcoin _InvoiceRepository = invoiceRepository; _ExplorerClients = explorerClients; _Aggregator = aggregator; - _payJoinRepository = payjoinRepository; + _utxoLocker = payjoinRepository; _paymentService = paymentService; } @@ -343,7 +343,7 @@ namespace BTCPayServer.Payments.Bitcoin // reuse our outpoint for another PJ (originalPJBroadcastable is false && !cjPJBroadcasted)) { - await _payJoinRepository.TryUnlock(payjoinInformation.ContributedOutPoints); + await _utxoLocker.TryUnlock(payjoinInformation.ContributedOutPoints); } await _paymentService.UpdatePayments(updatedPaymentEntities); diff --git a/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs b/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs index 642fb12d3..f33789b54 100644 --- a/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs +++ b/BTCPayServer/Payments/PayJoin/PayJoinEndpointController.cs @@ -83,7 +83,7 @@ namespace BTCPayServer.Payments.PayJoin private readonly InvoiceRepository _invoiceRepository; private readonly ExplorerClientProvider _explorerClientProvider; private readonly BTCPayWalletProvider _btcPayWalletProvider; - private readonly PayJoinRepository _payJoinRepository; + private readonly UTXOLocker _utxoLocker; private readonly EventAggregator _eventAggregator; private readonly NBXplorerDashboard _dashboard; private readonly DelayedTransactionBroadcaster _broadcaster; @@ -97,7 +97,7 @@ namespace BTCPayServer.Payments.PayJoin public PayJoinEndpointController(BTCPayNetworkProvider btcPayNetworkProvider, InvoiceRepository invoiceRepository, ExplorerClientProvider explorerClientProvider, BTCPayWalletProvider btcPayWalletProvider, - PayJoinRepository payJoinRepository, + UTXOLocker utxoLocker, EventAggregator eventAggregator, NBXplorerDashboard dashboard, DelayedTransactionBroadcaster broadcaster, @@ -111,7 +111,7 @@ namespace BTCPayServer.Payments.PayJoin _invoiceRepository = invoiceRepository; _explorerClientProvider = explorerClientProvider; _btcPayWalletProvider = btcPayWalletProvider; - _payJoinRepository = payJoinRepository; + _utxoLocker = utxoLocker; _eventAggregator = eventAggregator; _dashboard = dashboard; _broadcaster = broadcaster; @@ -148,7 +148,7 @@ namespace BTCPayServer.Payments.PayJoin }); } - await using var ctx = new PayjoinReceiverContext(_invoiceRepository, _explorerClientProvider.GetExplorerClient(network), _payJoinRepository, Logs); + await using var ctx = new PayjoinReceiverContext(_invoiceRepository, _explorerClientProvider.GetExplorerClient(network), _utxoLocker, Logs); ObjectResult CreatePayjoinErrorAndLog(int httpCode, PayjoinReceiverWellknownErrors err, string debug) { ctx.Logs.Write($"Payjoin error: {debug}", InvoiceEventData.EventSeverity.Error); @@ -322,7 +322,7 @@ namespace BTCPayServer.Payments.PayJoin } - if (!await _payJoinRepository.TryLockInputs(ctx.OriginalTransaction.Inputs.Select(i => i.PrevOut).ToArray())) + if (!await _utxoLocker.TryLockInputs(ctx.OriginalTransaction.Inputs.Select(i => i.PrevOut).ToArray())) { // We do not broadcast, since we might double spend a delayed transaction of a previous payjoin ctx.DoNotBroadcast(); @@ -502,18 +502,23 @@ namespace BTCPayServer.Payments.PayJoin _eventAggregator.Publish(new InvoiceEvent(invoice, InvoiceEvent.ReceivedPayment) { Payment = payment }); } - await _btcPayWalletProvider.GetWallet(network).SaveOffchainTransactionAsync(ctx.OriginalTransaction); + var labels = selectedUTXOs.GroupBy(pair => pair.Key.Hash).Select(utxo => + new KeyValuePair>(utxo.Key, + new List<(string color, Label label)>() + { + UpdateTransactionLabel.PayjoinExposedLabelTemplate(invoice?.Id) + })) + .ToDictionary(pair => pair.Key, pair => pair.Value); + + labels.Add(originalPaymentData.PayjoinInformation.CoinjoinTransactionHash, new List<(string color, Label label)>() + { + UpdateTransactionLabel.PayjoinLabelTemplate() + }); _eventAggregator.Publish(new UpdateTransactionLabel() { WalletId = walletId, - TransactionLabels = selectedUTXOs.GroupBy(pair => pair.Key.Hash).Select(utxo => - new KeyValuePair>(utxo.Key, - new List<(string color, Label label)>() - { - UpdateTransactionLabel.PayjoinExposedLabelTemplate(invoice?.Id) - })) - .ToDictionary(pair => pair.Key, pair => pair.Value) + TransactionLabels = labels }); ctx.Success(); // BTCPay Server support PSBT set as hex @@ -608,7 +613,7 @@ namespace BTCPayServer.Payments.PayJoin { continue; } - if (await _payJoinRepository.TryLock(availableUtxo.Outpoint)) + if (await _utxoLocker.TryLock(availableUtxo.Outpoint)) { return (new[] { availableUtxo }, PayjoinUtxoSelectionType.HeuristicBased); } @@ -620,7 +625,7 @@ namespace BTCPayServer.Payments.PayJoin { if (currentTry >= maxTries) break; - if (await _payJoinRepository.TryLock(utxo.Outpoint)) + if (await _utxoLocker.TryLock(utxo.Outpoint)) { return (new[] { utxo }, PayjoinUtxoSelectionType.Ordered); } diff --git a/BTCPayServer/Payments/PayJoin/PayJoinExtensions.cs b/BTCPayServer/Payments/PayJoin/PayJoinExtensions.cs index 5e718dee1..45dbfc532 100644 --- a/BTCPayServer/Payments/PayJoin/PayJoinExtensions.cs +++ b/BTCPayServer/Payments/PayJoin/PayJoinExtensions.cs @@ -13,7 +13,8 @@ namespace BTCPayServer.Payments.PayJoin { services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(provider => provider.GetRequiredService()); services.AddSingleton(); services.AddSingleton(); services.AddTransient(); diff --git a/BTCPayServer/Payments/PayJoin/PayJoinRepository.cs b/BTCPayServer/Payments/PayJoin/PayJoinRepository.cs index 98d7091a8..c81a3c771 100644 --- a/BTCPayServer/Payments/PayJoin/PayJoinRepository.cs +++ b/BTCPayServer/Payments/PayJoin/PayJoinRepository.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using BTCPayServer.Data; using Microsoft.EntityFrameworkCore; @@ -6,21 +7,19 @@ using NBitcoin; namespace BTCPayServer.Payments.PayJoin { - public class PayJoinRepository + public class UTXOLocker : IUTXOLocker { private readonly ApplicationDbContextFactory _dbContextFactory; - public PayJoinRepository(ApplicationDbContextFactory dbContextFactory) + public UTXOLocker(ApplicationDbContextFactory dbContextFactory) { _dbContextFactory = dbContextFactory; } + public async Task TryLock(OutPoint outpoint) { using var ctx = _dbContextFactory.CreateContext(); - ctx.PayjoinLocks.Add(new PayjoinLock() - { - Id = outpoint.ToString() - }); + ctx.PayjoinLocks.Add(new PayjoinLock() {Id = outpoint.ToString()}); try { return await ctx.SaveChangesAsync() == 1; @@ -36,11 +35,9 @@ namespace BTCPayServer.Payments.PayJoin using var ctx = _dbContextFactory.CreateContext(); foreach (OutPoint outPoint in outPoints) { - ctx.PayjoinLocks.Remove(new PayjoinLock() - { - Id = outPoint.ToString() - }); + ctx.PayjoinLocks.Remove(new PayjoinLock() {Id = outPoint.ToString()}); } + try { return await ctx.SaveChangesAsync() == outPoints.Length; @@ -63,6 +60,7 @@ namespace BTCPayServer.Payments.PayJoin Id = "K-" + outPoint.ToString() }); } + try { return await ctx.SaveChangesAsync() == outPoints.Length; @@ -72,5 +70,13 @@ namespace BTCPayServer.Payments.PayJoin return false; } } + + public async Task> FindLocks(OutPoint[] outpoints) + { + var outPointsStr = outpoints.Select(o => o.ToString()); + await using var ctx = _dbContextFactory.CreateContext(); + return (await ctx.PayjoinLocks.Where(l => outPointsStr.Contains(l.Id)).ToArrayAsync()) + .Select(l => OutPoint.Parse(l.Id)).ToHashSet(); + } } } diff --git a/BTCPayServer/Payments/PayJoin/PayjoinReceiverContext.cs b/BTCPayServer/Payments/PayJoin/PayjoinReceiverContext.cs index a5278737e..ea4e99031 100644 --- a/BTCPayServer/Payments/PayJoin/PayjoinReceiverContext.cs +++ b/BTCPayServer/Payments/PayJoin/PayjoinReceiverContext.cs @@ -14,14 +14,14 @@ namespace BTCPayServer.Payments.PayJoin { private readonly InvoiceRepository _invoiceRepository; private readonly ExplorerClient _explorerClient; - private readonly PayJoinRepository _payJoinRepository; + private readonly UTXOLocker _utxoLocker; private readonly BTCPayServer.Logging.Logs BTCPayLogs; - public PayjoinReceiverContext(InvoiceRepository invoiceRepository, ExplorerClient explorerClient, PayJoinRepository payJoinRepository, BTCPayServer.Logging.Logs logs) + public PayjoinReceiverContext(InvoiceRepository invoiceRepository, ExplorerClient explorerClient, UTXOLocker utxoLocker, BTCPayServer.Logging.Logs logs) { this.BTCPayLogs = logs; _invoiceRepository = invoiceRepository; _explorerClient = explorerClient; - _payJoinRepository = payJoinRepository; + _utxoLocker = utxoLocker; } public Invoice Invoice { get; set; } public NBitcoin.Transaction OriginalTransaction { get; set; } @@ -40,7 +40,7 @@ namespace BTCPayServer.Payments.PayJoin } if (!success && LockedUTXOs != null) { - disposing.Add(_payJoinRepository.TryUnlock(LockedUTXOs)); + disposing.Add(_utxoLocker.TryUnlock(LockedUTXOs)); } try { diff --git a/BTCPayServer/Services/Labels/LabelFactory.cs b/BTCPayServer/Services/Labels/LabelFactory.cs index c28ca6ceb..016b3fa0d 100644 --- a/BTCPayServer/Services/Labels/LabelFactory.cs +++ b/BTCPayServer/Services/Labels/LabelFactory.cs @@ -56,6 +56,7 @@ namespace BTCPayServer.Services.Labels { Text = uncoloredLabel.Text, Color = color, + Tooltip = "", TextColor = TextColor(color) }; @@ -108,7 +109,10 @@ namespace BTCPayServer.Services.Labels ? null : _linkGenerator.PayoutLink(payoutLabel.WalletId, null, PayoutState.Completed, request.Scheme, request.Host, request.PathBase); - + } + else if (uncoloredLabel.Text == "payjoin") + { + coloredLabel.Tooltip = $"This UTXO was part of a PayJoin transaction."; } return coloredLabel; }