diff --git a/BTCPayServer.Tests/Extensions.cs b/BTCPayServer.Tests/Extensions.cs index d6e71c6df..9452cbe79 100644 --- a/BTCPayServer.Tests/Extensions.cs +++ b/BTCPayServer.Tests/Extensions.cs @@ -170,12 +170,6 @@ retry: wait.Until(d => d.WaitForElement(By.CssSelector("#WalletTransactions[data-loaded='true']"))); } - public static async Task WaitWalletTransactionsLoaded(this IPage page) - { - await page.WaitForLoadStateAsync(LoadState.NetworkIdle); - await page.Locator("#WalletTransactions[data-loaded='true']").WaitForAsync(new() { State = WaitForSelectorState.Visible }); - } - public static IWebElement WaitForElement(this IWebDriver driver, By selector) { var wait = new WebDriverWait(driver, SeleniumTester.ImplicitWait); diff --git a/BTCPayServer.Tests/PlaywrightTester.cs b/BTCPayServer.Tests/PlaywrightTester.cs index 64d4fd112..03621246d 100644 --- a/BTCPayServer.Tests/PlaywrightTester.cs +++ b/BTCPayServer.Tests/PlaywrightTester.cs @@ -474,6 +474,7 @@ namespace BTCPayServer.Tests goto retry; } } + await Server.ExplorerNode.GenerateAsync(1); await Page.ReloadAsync(); await Page.Locator("#CancelWizard").ClickAsync(); return addressStr; @@ -541,5 +542,61 @@ namespace BTCPayServer.Tests await page.BringToFrontAsync(); return new SwitchDisposable(page, old, this, closeAfter); } + + public async Task GoToWalletTransactions(WalletId walletId = null) + { + await GoToWallet(walletId, navPages: WalletsNavPages.Transactions); + await Page.Locator("#WalletTransactions[data-loaded='true']").WaitForAsync(new() { State = WaitForSelectorState.Visible }); + return new WalletTransactionsPMO(Page); + } + +#nullable enable + public class WalletTransactionsPMO(IPage page) + { + public Task SelectAll() => page.SetCheckedAsync(".mass-action-select-all", true); + public async Task Select(params uint256[] txs) + { + foreach (var txId in txs) + { + await page.SetCheckedAsync($"{TxRowSelector(txId)} .mass-action-select", true); + } + } + + public Task BumpFeeSelected() => page.ClickAsync("#BumpFee"); + + public Task BumpFee(uint256? txId = null) => page.ClickAsync($"{TxRowSelector(txId)} .bumpFee-btn"); + static string TxRowSelector(uint256? txId = null) => txId is null ? ".transaction-row:first-of-type" : $".transaction-row[data-value=\"{txId}\"]"; + + public Task AssertHasLabels(string label) => AssertHasLabels(null, label); + public async Task AssertHasLabels(uint256? txId, string label) + { + await page.ReloadAsync(); + await page.Locator($"{TxRowSelector(txId)} .transaction-label[data-value=\"{label}\"]").WaitForAsync(); + } + + public async Task AssertNotFound(uint256 txId) + { + Assert.False(await page.Locator(TxRowSelector(txId)).IsVisibleAsync()); + } + } + + + public async Task GoToWalletSend(WalletId? walletId = null) + { + await GoToWallet(walletId, navPages: WalletsNavPages.Send); + return new(Page); + } + + public class SendWalletPMO(IPage page) + { + public Task FillAddress(BitcoinAddress address) => page.FillAsync("[name='Outputs[0].DestinationAddress']", + address.ToString()); + + public Task SweepBalance() => page.ClickAsync("#SweepBalance"); + + public Task Sign() => page.ClickAsync("#SignTransaction"); + + public Task SetFeeRate(decimal val) => page.FillAsync("[name=\"FeeSatoshiPerByte\"]", val.ToString(CultureInfo.InvariantCulture)); + } } } diff --git a/BTCPayServer.Tests/WalletTests.cs b/BTCPayServer.Tests/WalletTests.cs index 40fab4ed0..a0d6b9ddd 100644 --- a/BTCPayServer.Tests/WalletTests.cs +++ b/BTCPayServer.Tests/WalletTests.cs @@ -2,9 +2,6 @@ using System.Linq; using System.Threading.Tasks; using BTCPayServer.Abstractions.Models; -using BTCPayServer.Client; -using BTCPayServer.Client.Models; -using BTCPayServer.Views.Wallets; using NBitcoin; using Xunit; using Xunit.Abstractions; @@ -28,8 +25,8 @@ public class WalletTests(ITestOutputHelper helper) : UnitTestBase(helper) var txs = (await client.ShowOnChainWalletTransactions(s.StoreId, "BTC")).Select(t => t.TransactionHash).ToArray(); Assert.Equal(3, txs.Length); - await s.GoToWallet(navPages: WalletsNavPages.Transactions); - await ClickBumpFee(s, txs[0]); + var w = await s.GoToWalletTransactions(); + await w.BumpFee(txs[0]); // Because a single transaction is selected, we should be able to select CPFP only (Because no change are available, we can't do RBF) await s.Page.Locator("[name='txId']").WaitForAsync(); @@ -38,13 +35,13 @@ public class WalletTests(ITestOutputHelper helper) : UnitTestBase(helper) await s.ClickCancel(); // Same but using mass action - await SelectTransactions(s, txs[0]); + await w.Select(txs[0]); await s.Page.ClickAsync("#BumpFee"); await s.Page.Locator("[name='txId']").WaitForAsync(); await s.ClickCancel(); // Because two transactions are select we can only mass bump on CPFP - await SelectTransactions(s, txs[0], txs[1]); + await w.Select(txs[0], txs[1]); await s.Page.ClickAsync("#BumpFee"); Assert.False(await s.Page.Locator("[name='txId']").IsVisibleAsync()); Assert.Equal("disabled", await s.Page.GetAttributeAsync("#BumpMethod", "disabled")); @@ -58,12 +55,12 @@ public class WalletTests(ITestOutputHelper helper) : UnitTestBase(helper) // The CPFP tag should be applied to the new tx var cpfpTx = (await client.ShowOnChainWalletTransactions(s.StoreId, "BTC")).Select(t => t.TransactionHash).ToArray()[0]; - await AssertHasLabels(s, cpfpTx, "CPFP"); + await w.AssertHasLabels(cpfpTx, "CPFP"); // The CPFP should be RBF-able Assert.DoesNotContain(cpfpTx, txs); - await ClickBumpFee(s, cpfpTx); + await w.BumpFee(cpfpTx); Assert.Null(await s.Page.GetAttributeAsync("#BumpMethod", "disabled")); Assert.Equal("RBF", await s.Page.Locator("#BumpMethod option:checked").InnerTextAsync()); @@ -82,11 +79,25 @@ public class WalletTests(ITestOutputHelper helper) : UnitTestBase(helper) var rbfTx = (await client.ShowOnChainWalletTransactions(s.StoreId, "BTC")).Select(t => t.TransactionHash).ToArray()[0]; // CPFP has been replaced, so it should not be found - Assert.False(await s.Page.Locator(TxRowSelector(cpfpTx)).IsVisibleAsync()); + await w.AssertNotFound(cpfpTx); // However, the new transaction should have copied the CPFP tag from the transaction it replaced, and have a RBF label as well. - await AssertHasLabels(s, rbfTx, "CPFP"); - await AssertHasLabels(s, rbfTx, "RBF"); + await w.AssertHasLabels(rbfTx, "CPFP"); + await w.AssertHasLabels(rbfTx, "RBF"); + + // Now, we sweep all the UTXOs to a single destination. This should be RBF-able. (Fee deducted on the lone UTXO) + var send = await s.GoToWalletSend(); + await send.FillAddress(new Key().PubKey.GetAddress(ScriptPubKeyType.Legacy, Network.RegTest)); + await send.SweepBalance(); + await send.SetFeeRate(20m); + await send.Sign(); + await s.Page.ClickAsync("button[value=broadcast]"); + // Now we RBF the sweep + await w.BumpFee(); + Assert.Equal("RBF", await s.Page.Locator("#BumpMethod").InnerTextAsync()); + await s.ClickPagePrimary(); + await s.Page.ClickAsync("#BroadcastTransaction"); + await w.AssertHasLabels("RBF"); } private async Task CreateInvoices(PlaywrightTester tester) @@ -102,31 +113,6 @@ public class WalletTests(ITestOutputHelper helper) : UnitTestBase(helper) } } - private async Task AssertHasLabels(PlaywrightTester s, uint256 txId, string label) - { - await s.Page.ReloadAsync(); - await s.Page.Locator($"{TxRowSelector(txId)} .transaction-label[data-value=\"{label}\"]").WaitForAsync(); - } - private async Task AssertHasLabels(PlaywrightTester s, string label) - { - await s.Page.ReloadAsync(); - await s.Page.Locator($".transaction-label[data-value=\"{label}\"]").First.WaitForAsync(); - } - - static string TxRowSelector(uint256 txId) => $".transaction-row[data-value=\"{txId}\"]"; - - private async Task SelectTransactions(PlaywrightTester s, params uint256[] txs) - { - foreach (var txId in txs) - { - await s.Page.SetCheckedAsync($"{TxRowSelector(txId)} .mass-action-select", true); - } - } - - private async Task ClickBumpFee(PlaywrightTester s, uint256 txId) - { - await s.Page.ClickAsync($"{TxRowSelector(txId)} .bumpFee-btn"); - } [Fact] [Trait("Playwright", "Playwright")] @@ -156,16 +142,18 @@ public class WalletTests(ITestOutputHelper helper) : UnitTestBase(helper) Assert.Contains($"/stores/{s.StoreId}/invoices", s.Page.Url); await s.FindAlertMessage(StatusMessageModel.StatusSeverity.Error, partialText: "No UTXOs available"); - // But we should be able to bump from the wallet's page - await s.GoToWallet(navPages: WalletsNavPages.Transactions); - await s.Page.SetCheckedAsync(".mass-action-select-all", true); - await s.Page.ClickAsync("#BumpFee"); - await s.ClickPagePrimary(); - await s.Page.ClickAsync("#BroadcastTransaction"); - Assert.Contains($"/wallets/{s.WalletId}", s.Page.Url); - await s.FindAlertMessage(partialText: "Transaction broadcasted successfully"); + for (int i = 0; i < 5; i++) + { + var txs = await s.GoToWalletTransactions(); + await txs.SelectAll(); + await txs.BumpFeeSelected(); + await s.ClickPagePrimary(); + await s.Page.ClickAsync("#BroadcastTransaction"); + Assert.Contains($"/wallets/{s.WalletId}", s.Page.Url); + await s.FindAlertMessage(partialText: "Transaction broadcasted successfully"); - // The CPFP tag should be applied to the new tx - await AssertHasLabels(s, "CPFP"); + // The CPFP tag should be applied to the new tx + await txs.AssertHasLabels("CPFP"); + } } } diff --git a/BTCPayServer/Controllers/UIWalletsController.cs b/BTCPayServer/Controllers/UIWalletsController.cs index 95411e248..900f72885 100644 --- a/BTCPayServer/Controllers/UIWalletsController.cs +++ b/BTCPayServer/Controllers/UIWalletsController.cs @@ -284,7 +284,6 @@ namespace BTCPayServer.Controllers model.RecommendedSatoshiPerByte = recommendedFees.Where(option => option != null).ToList(); model.FeeSatoshiPerByte ??= recommendedFees.Skip(1).FirstOrDefault()?.FeeRate; - if (HttpContext.Request.Method != HttpMethods.Post) { model.Command = null; @@ -303,7 +302,7 @@ namespace BTCPayServer.Controllers var feeBumpUrl = Url.Action(nameof(WalletBumpFee), new { walletId, transactionId = bumpTarget.GetSingleTransactionId(), model.FeeSatoshiPerByte, model.BumpMethod, model.TransactionHashes, model.Outpoints })!; if (model.BumpMethod == "CPFP") { - var utxos = await explorer.GetUTXOsAsync(paymentMethod.AccountDerivation); + var utxos = await explorer.GetUTXOsAsync(paymentMethod.AccountDerivation, cancellationToken); List bumpableUTXOs = bumpTarget.GetMatchedOutpoints(utxos.GetUnspentUTXOs().Where(u => u.Confirmations == 0).Select(u => u.Outpoint)); if (bumpableUTXOs.Count == 0) @@ -350,12 +349,16 @@ namespace BTCPayServer.Controllers // RBF is only supported for a single tx var tx = txs[bumpTarget.GetSingleTransactionId()!]; var changeOutput = tx.Outputs.FirstOrDefault(o => o.Feature == DerivationFeature.Change); + if (changeOutput is null && + tx is { Transaction: { Outputs: [{ ScriptPubKey: {} singleAddress }] }}) + changeOutput = new() { ScriptPubKey = singleAddress, Index = 0 }; if (tx.Inputs.Count != tx.Transaction?.Inputs.Count || changeOutput is null) { this.ModelState.AddModelError(nameof(model.BumpMethod), StringLocalizer["This transaction can't be RBF'd"]); return View(nameof(WalletBumpFee), model); } + IActionResult ChangeTooSmall(WalletBumpFeeViewModel vm, Money? missing) { if (missing is not null) @@ -365,7 +368,14 @@ namespace BTCPayServer.Controllers return View(nameof(WalletBumpFee), vm); } - var bumpResult = bumpable[tx.TransactionId].ReplacementInfo!.CalculateBumpResult(targetFeeRate); + var bumpableTx = bumpable[tx.TransactionId].ReplacementInfo!; + if (targetFeeRate < bumpableTx.CalculateNewMinFeeRate()) + { + ModelState.AddModelError(nameof(model.FeeSatoshiPerByte), StringLocalizer["The selected fee rate is too small. The minimum is {0} sat/byte", bumpableTx.CalculateNewMinFeeRate().SatoshiPerByte]); + return View(nameof(WalletBumpFee), model); + } + + var bumpResult = bumpableTx.CalculateBumpResult(targetFeeRate); var createPSBT = new CreatePSBTRequest() { RBF = true, @@ -378,7 +388,12 @@ namespace BTCPayServer.Controllers { ExplicitFee = bumpResult.NewTxFee }, - ExplicitChangeAddress = changeOutput.Address, + ExplicitChangeAddress = changeOutput switch + { + { Address: {} addr } => PSBTDestination.Create(addr), + { ScriptPubKey: {} scriptPubKey } => PSBTDestination.Create(scriptPubKey), + _ => throw new InvalidOperationException("Invalid change output") + }, Destinations = tx.Transaction.Outputs.AsIndexedOutputs() .Select(o => new CreatePSBTDestination() { @@ -904,8 +919,7 @@ namespace BTCPayServer.Controllers { try { - var result = await feeProvider.GetFeeRateAsync( - (int)network.NBitcoinNetwork.Consensus.GetExpectedBlocksFor(time)); + var result = await feeProvider.GetFeeRateAsync((int)network.NBitcoinNetwork.Consensus.GetExpectedBlocksFor(time)); options.Add(new WalletSendModel.FeeRateOption() { Target = time, diff --git a/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs b/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs index 1d4bab847..c727e19bc 100644 --- a/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs +++ b/BTCPayServer/HostedServices/TransactionLabelMarkerHostedService.cs @@ -92,24 +92,20 @@ namespace BTCPayServer.HostedServices { if (transactionEvent.NewTransactionEvent.Replacing is not null) { - foreach (var replaced in transactionEvent.NewTransactionEvent.Replacing) { var replacedwoId = new WalletObjectId(wid, WalletObjectData.Types.Tx, replaced.ToString()); var replacedo = await _walletRepository.GetWalletObject(replacedwoId); var replacedLinks = replacedo?.GetLinks().Where(t => t.type != WalletObjectData.Types.Tx) ?? []; - if (replacedLinks.Count() != 0) + var rbf = new WalletObjectId(wid, WalletObjectData.Types.RBF, ""); + var label = WalletRepository.CreateLabel(rbf); + newObjs.Add(label.ObjectData); + links.Add(WalletRepository.NewWalletObjectLinkData(txWalletObject, label.Id)); + links.Add(WalletRepository.NewWalletObjectLinkData(txWalletObject, rbf, new JObject() { - var rbf = new WalletObjectId(wid, WalletObjectData.Types.RBF, ""); - var label = WalletRepository.CreateLabel(rbf); - newObjs.Add(label.ObjectData); - links.Add(WalletRepository.NewWalletObjectLinkData(txWalletObject, label.Id)); - links.Add(WalletRepository.NewWalletObjectLinkData(txWalletObject, rbf, new JObject() - { - ["txs"] = JArray.FromObject(new[] { replaced.ToString() }) - })); - } + ["txs"] = JArray.FromObject(new[] { replaced.ToString() }) + })); foreach (var link in replacedLinks) { links.Add(WalletRepository.NewWalletObjectLinkData(new WalletObjectId(walletObjectDatas.Key, link.type, link.id), txWalletObject, link.linkdata)); diff --git a/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs b/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs index 70adeeca2..d87245317 100644 --- a/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs +++ b/BTCPayServer/Payments/Bitcoin/NBXplorerListener.cs @@ -155,10 +155,7 @@ namespace BTCPayServer.Payments.Bitcoin if (evt.DerivationStrategy != null) { wallet.InvalidateCache(evt.DerivationStrategy); - var validOutputs = network.GetValidOutputs(evt).ToList(); - if (!validOutputs.Any()) - break; - foreach (var output in validOutputs) + foreach (var output in network.GetValidOutputs(evt)) { if (!output.matchedOutput.Value.IsCompatible(network)) continue; diff --git a/BTCPayServer/Services/Fees/NBxplorerFeeProvider.cs b/BTCPayServer/Services/Fees/NBxplorerFeeProvider.cs index 6ed2f9edb..2299ca59b 100644 --- a/BTCPayServer/Services/Fees/NBxplorerFeeProvider.cs +++ b/BTCPayServer/Services/Fees/NBxplorerFeeProvider.cs @@ -10,7 +10,7 @@ namespace BTCPayServer.Services.Fees { public async Task GetFeeRateAsync(int blockTarget = 20) { - return (await ExplorerClient.GetFeeRateAsync(blockTarget).ConfigureAwait(false)).FeeRate; + return (await ExplorerClient.GetFeeRateAsync(blockTarget).ConfigureAwait(false)).FeeRate; } } } diff --git a/BTCPayServer/Services/Wallets/BTCPayWallet.cs b/BTCPayServer/Services/Wallets/BTCPayWallet.cs index 981cd874f..53f0460ed 100644 --- a/BTCPayServer/Services/Wallets/BTCPayWallet.cs +++ b/BTCPayServer/Services/Wallets/BTCPayWallet.cs @@ -338,9 +338,9 @@ namespace BTCPayServer.Services.Wallets WHERE code = @code AND wallet_id=@walletId AND mempool IS TRUE AND replaced_by IS NULL AND blk_id IS NULL GROUP BY code, tx_id ) - SELECT tt.tx_id, u.raw, tt.input_count, tt.change_count, uu.unspent_count FROM unconfs u + SELECT tt.tx_id, u.raw, tt.input_count, tt.change_count, COALESCE(uu.unspent_count, 0) FROM unconfs u JOIN tracked_txs tt USING (code, tx_id) - JOIN unspent_utxos uu USING (code, tx_id); + LEFT JOIN unspent_utxos uu USING (code, tx_id); """, parameters: new { @@ -367,8 +367,11 @@ namespace BTCPayServer.Services.Wallets { continue; } - if ((state.MempoolInfo?.FullRBF is true || tx.RBF) && tx.Inputs.Count == r.input_count && - r.change_count > 0) + + var rbf = state.MempoolInfo?.FullRBF is true || Network.IsBTC || tx.RBF; + rbf &= tx.Inputs.Count == r.input_count; + rbf &= r.change_count > 0 || tx.Outputs.Count == 1; + if (rbf) { canRBF.Add(uint256.Parse(r.tx_id)); } diff --git a/BTCPayServer/Views/UIWallets/WalletBumpFee.cshtml b/BTCPayServer/Views/UIWallets/WalletBumpFee.cshtml index 9fdeed9cf..f1385b4fb 100644 --- a/BTCPayServer/Views/UIWallets/WalletBumpFee.cshtml +++ b/BTCPayServer/Views/UIWallets/WalletBumpFee.cshtml @@ -67,18 +67,10 @@ } } - @if (!ViewContext.ModelState.IsValid) - { -
    - @foreach (var errors in ViewData.ModelState.Where(pair => pair.Key == string.Empty && pair.Value.ValidationState == ModelValidationState.Invalid)) - { - foreach (var error in errors.Value.Errors) - { -
  • @error.ErrorMessage
    • - } - } -
- } + @if (!ViewContext.ModelState.IsValid) + { +
+ } @if (Model.GetBumpTarget().GetSingleTransactionId() is { } txId) {
diff --git a/BTCPayServer/Views/UIWallets/WalletSend.cshtml b/BTCPayServer/Views/UIWallets/WalletSend.cshtml index 77571610b..922c4c222 100644 --- a/BTCPayServer/Views/UIWallets/WalletSend.cshtml +++ b/BTCPayServer/Views/UIWallets/WalletSend.cshtml @@ -127,7 +127,7 @@
Your available balance is - @Model.CryptoCode. + @Model.CryptoCode. @if (Model.ImmatureBalance > 0) {
@StringLocalizer["Note: {0} are still immature and require additional confirmations.", $"{Model.ImmatureBalance} {Model.CryptoCode}"] @@ -205,7 +205,7 @@
- + @if (!string.IsNullOrEmpty(Model.PayJoinBIP21)) {
diff --git a/btcpayserver.sln.DotSettings b/btcpayserver.sln.DotSettings index bfdba3a71..de0cb89cd 100644 --- a/btcpayserver.sln.DotSettings +++ b/btcpayserver.sln.DotSettings @@ -5,6 +5,7 @@ LNURL NBX NBXplorer + PMO POS PSBT SSH