From f03eafd3f5071201e054f5a09c3998ce0b911252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:34:14 +0200 Subject: [PATCH 1/7] Exclude dust UTXOs from coin selection Add MINIMUM_UTXO_VALUE_SATS constant (default 546 sats, configurable via environment variable) and use it to filter dust UTXOs out of coin selection both server-side in NodeGuardService and in the Wallets UI. - Introduce MINIMUM_UTXO_VALUE_SATS in Constants with env-var override - Server-side: append dust outpoints to ignoreOutpoints so selection strategies don't count them toward requested amounts (which would otherwise result in short selections after local stripping) - UI: mark dust UTXOs with a warning "dust" badge and disable the freeze toggle for them, since they are auto-excluded --- src/Helpers/Constants.cs | 9 + src/Pages/Wallets.razor | 7 +- src/Rpc/NodeGuardService.cs | 17 +- src/Services/CoinSelectionService.cs | 5 + .../E2E/DustUtxoWithdrawalE2ETests.cs | 200 ++++++++++++++++++ test/NodeGuard.Tests/E2E/E2ECollection.cs | 30 +++ .../E2E/GetNewWalletAddressE2ETests.cs | 1 + test/NodeGuard.Tests/E2E/RebalanceE2ETests.cs | 1 + .../Rpc/NodeGuardServiceTests.cs | 85 ++++++++ .../Services/CoinSelectionServiceTests.cs | 141 ++++++++++++ 10 files changed, 493 insertions(+), 3 deletions(-) create mode 100644 test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs create mode 100644 test/NodeGuard.Tests/E2E/E2ECollection.cs create mode 100644 test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs diff --git a/src/Helpers/Constants.cs b/src/Helpers/Constants.cs index 2f30f28d..155e4ac0 100644 --- a/src/Helpers/Constants.cs +++ b/src/Helpers/Constants.cs @@ -107,6 +107,12 @@ public class Constants public static readonly int SESSION_TIMEOUT_MILLISECONDS = 3_600_000; public static readonly Money BITCOIN_DUST = new Money(0.00000546m, MoneyUnit.BTC); // 546 satoshi in BTC + /// + /// UTXOs with value less than or equal to this are excluded from coin selection (dust-attack protection). + /// Can be configured via the MINIMUM_UTXO_VALUE_SATS environment variable. + /// + public static readonly long MINIMUM_UTXO_VALUE_SATS = 546; + /// /// Minimum swap out size in BTC for automatic liquidity management (Swap Out). /// Can be configured via MINIMUM_SWAP_OUT_SIZE_BTC environment variable. @@ -495,6 +501,9 @@ static Constants() var minSweepTransactionAmount = Environment.GetEnvironmentVariable("MINIMUM_SWEEP_TRANSACTION_AMOUNT_SATS"); if (minSweepTransactionAmount != null) MINIMUM_SWEEP_TRANSACTION_AMOUNT_SATS = long.Parse(minSweepTransactionAmount); + var minimumUtxoValueSats = Environment.GetEnvironmentVariable("MINIMUM_UTXO_VALUE_SATS"); + if (minimumUtxoValueSats != null) MINIMUM_UTXO_VALUE_SATS = long.Parse(minimumUtxoValueSats); + DEFAULT_DERIVATION_PATH = GetEnvironmentalVariableOrThrowIfNotTesting("DEFAULT_DERIVATION_PATH") ?? DEFAULT_DERIVATION_PATH; diff --git a/src/Pages/Wallets.razor b/src/Pages/Wallets.razor index aa21e4cb..77ba3612 100644 --- a/src/Pages/Wallets.razor +++ b/src/Pages/Wallets.razor @@ -581,6 +581,7 @@ { foreach (var (_, utxo, tags, isFrozen) in _detailsUTXOs) { + var isDust = ((Money)utxo.Value).Satoshi <= Constants.MINIMUM_UTXO_VALUE_SATS; @utxo.Confirmations @($"{utxo.Value} BTC") + @if (isDust) + { + dust + } @if (tags.Any()) { foreach (var data in tags.Select((tag, i) => new { tag, i })) @@ -598,7 +603,7 @@ } - diff --git a/src/Rpc/NodeGuardService.cs b/src/Rpc/NodeGuardService.cs index 7319020c..8cbb704f 100644 --- a/src/Rpc/NodeGuardService.cs +++ b/src/Rpc/NodeGuardService.cs @@ -1057,8 +1057,17 @@ public override async Task GetAvailableUtxos(GetAvailableUtxos walletUtxos.Unconfirmed.UTXOs.Any(u => u.Outpoint.ToString() == utxo)) .ToList(); + // Ignore dust UTXOs server-side too, so they are not counted towards the requested amount + // by any selection strategy and then stripped locally, returning a short selection + var listDust = walletUtxos.Confirmed.UTXOs + .Concat(walletUtxos.Unconfirmed.UTXOs) + .Where(utxo => ((Money)utxo.Value).Satoshi <= Constants.MINIMUM_UTXO_VALUE_SATS) + .Select(utxo => utxo.Outpoint.ToString()) + .ToList(); + ignoreOutpoints.AddRange(listLocked); ignoreOutpoints.AddRange(listFrozen); + ignoreOutpoints.AddRange(listDust); var utxos = await _nbXplorerService.GetUTXOsByLimitAsync( derivationStrategy, @@ -1069,13 +1078,17 @@ public override async Task GetAvailableUtxos(GetAvailableUtxos ignoreOutpoints ); - var confirmedUtxos = utxos.Confirmed.UTXOs.Select(utxo => new Utxo() + var confirmedUtxos = utxos.Confirmed.UTXOs + .Where(utxo => ((Money)utxo.Value).Satoshi > Constants.MINIMUM_UTXO_VALUE_SATS) + .Select(utxo => new Utxo() { Amount = (Money)utxo.Value, Outpoint = utxo.Outpoint.ToString(), Address = utxo.Address.ToString() }); - var unconfirmedUtxos = utxos.Unconfirmed.UTXOs.Select(utxo => new Utxo() + var unconfirmedUtxos = utxos.Unconfirmed.UTXOs + .Where(utxo => ((Money)utxo.Value).Satoshi > Constants.MINIMUM_UTXO_VALUE_SATS) + .Select(utxo => new Utxo() { Amount = (Money)utxo.Value, Outpoint = utxo.Outpoint.ToString(), diff --git a/src/Services/CoinSelectionService.cs b/src/Services/CoinSelectionService.cs index ad2daee1..3173964a 100644 --- a/src/Services/CoinSelectionService.cs +++ b/src/Services/CoinSelectionService.cs @@ -167,6 +167,11 @@ private async Task> FilterLockedFrozenUTXOs(UTXOChanges? utxoChanges) { _logger.LogInformation("Removing UTXO: {Utxo} from UTXO set as it is locked", utxo.Outpoint.ToString()); } + else if (((Money)utxo.Value).Satoshi <= Constants.MINIMUM_UTXO_VALUE_SATS) + { + _logger.LogInformation("Removing UTXO: {Utxo} from UTXO set as it is dust ({Sats} sats <= {MinSats} sats)", + utxo.Outpoint.ToString(), ((Money)utxo.Value).Satoshi, Constants.MINIMUM_UTXO_VALUE_SATS); + } else { availableUTXOs.Add(utxo); diff --git a/test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs b/test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs new file mode 100644 index 00000000..7cbbb896 --- /dev/null +++ b/test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs @@ -0,0 +1,200 @@ +/* + * NodeGuard + * Copyright (C) 2023 Elenpay + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +using System.Net; +using FluentAssertions; +using Grpc.Core; +using Grpc.Net.Client; +using NBitcoin; +using NBitcoin.RPC; +using Nodeguard; +using Xunit.Abstractions; + +namespace NodeGuard.Tests.E2E; + +/// +/// End-to-end coverage for the dust UTXO protection (MINIMUM_UTXO_VALUE_SATS): a confirmed +/// 546-sat output sent to a NodeGuard hot wallet must be hidden from GetAvailableUtxos and must +/// never be auto-selected as an input of a withdrawal, even though the coin selection picks the +/// newest confirmed UTXO first (which the freshly-mined dust output would otherwise be). +/// Exercised against a LIVE NodeGuard instance + bitcoind. +/// Gated by (RUN_E2E_TESTS=1). Connection via env: +/// NODEGUARD_GRPC_ENDPOINT default http://localhost:50051 (h2c) +/// NODEGUARD_API_TOKEN default the dev "Liquidator" token +/// BITCOIND_RPC_URL/USER/PASS/WALLET default http://localhost:18443 / polaruser / polarpass / default +/// E2E_HOT_WALLET_ID NodeGuard hot wallet to withdraw from (default 3) +/// +[Trait("Category", "E2E")] +[Collection("E2E")] +public class DustUtxoWithdrawalE2ETests +{ + private const string DefaultDevToken = "8rvSsUGeyXXdDQrHctcTey/xtHdZQEn945KHwccKp9Q="; + private const long DustAmountSats = 546; + // The custom NBXplorer selectutxos backend behind GetAvailableUtxos picks UTXOs toward a + // target amount (amount=0 always yields an empty selection), so every call must request one. + private const long ProbeAmountSats = 2_000_000; + + private readonly ITestOutputHelper _output; + + public DustUtxoWithdrawalE2ETests(ITestOutputHelper output) + { + _output = output; + AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true); + } + + [E2EFact] + public async Task RequestWithdrawal_DoesNotSelectDustUtxo() + { + var client = CreateClient(out var headers); + var rpc = CreateBitcoindRpc(); + var walletId = int.Parse(Env("E2E_HOT_WALLET_ID", "3")); + + // 0. Wait for NodeGuard to be up and for the dev hot wallet to have a spendable UTXO + // (DbInitializer funds it with 20 BTC; another E2E test may have it locked temporarily). + await RetryAsync(async () => + { + var resp = await client.GetNodesAsync(new GetNodesRequest(), headers); + if (resp.Nodes.Count == 0) throw new InvalidOperationException("no nodes seeded yet"); + return true; + }, attempts: 90, delay: TimeSpan.FromSeconds(4), what: "GetNodes (NodeGuard readiness)"); + + await RetryAsync(async () => + { + var available = await client.GetAvailableUtxosAsync( + new GetAvailableUtxosRequest { WalletId = walletId, Amount = ProbeAmountSats }, headers); + if (available.Confirmed.Sum(u => u.Amount) < ProbeAmountSats) + throw new InvalidOperationException("hot wallet has no spendable UTXO yet"); + return true; + }, attempts: 60, delay: TimeSpan.FromSeconds(4), what: "GetAvailableUtxos (hot wallet funded)"); + + // 1. Send a 546-sat dust output to a fresh address of the wallet and confirm it. + var addressResponse = await client.GetNewWalletAddressAsync( + new GetNewWalletAddressRequest { WalletId = walletId, Skip = 0, Reserve = true }, headers); + var dustAddress = BitcoinAddress.Create(addressResponse.Address, Network.RegTest); + var dustTxId = await rpc.SendToAddressAsync(dustAddress, Money.Satoshis(DustAmountSats)); + await MineAsync(rpc, 6); + + var dustFundingTx = await rpc.GetRawTransactionAsync(dustTxId); + var dustVout = dustFundingTx.Outputs.AsIndexedOutputs() + .Single(o => o.TxOut.ScriptPubKey == dustAddress.ScriptPubKey).N; + var dustOutpoint = new OutPoint(dustTxId, dustVout); + _output.WriteLine($"dust UTXO: {dustOutpoint}"); + + // 2. The dust UTXO must show up in the raw GetUtxos listing (NBXplorer indexed it) + // but must be hidden from the available-for-coin-selection listing. + await RetryAsync(async () => + { + var all = await client.GetUtxosAsync(new GetUtxosRequest(), headers); + if (all.Confirmed.All(u => u.Outpoint != dustOutpoint.ToString())) + throw new InvalidOperationException("dust UTXO not indexed by NBXplorer yet"); + return true; + }, attempts: 30, delay: TimeSpan.FromSeconds(4), what: "GetUtxos (dust UTXO indexed)"); + + // No selection strategy may surface the dust UTXO. SmallestFirst would rank it first, and + // ClosestToTargetFirst targeting exactly 546 sats is the most adversarial case: the dust + // UTXO would be the top candidate if it were not filtered out. + foreach (var strategy in new[] + { + COIN_SELECTION_STRATEGY.SmallestFirst, + COIN_SELECTION_STRATEGY.BiggestFirst, + COIN_SELECTION_STRATEGY.ClosestToTargetFirst, + COIN_SELECTION_STRATEGY.UpToAmount, + }) + { + var availableUtxos = await client.GetAvailableUtxosAsync(new GetAvailableUtxosRequest + { + WalletId = walletId, + Strategy = strategy, + Amount = ProbeAmountSats, + ClosestTo = DustAmountSats, + }, headers); + availableUtxos.Confirmed.Should().NotBeEmpty( + $"the wallet's non-dust UTXOs must still be available with strategy {strategy}"); + availableUtxos.Confirmed.Select(u => u.Outpoint).Should().NotContain(dustOutpoint.ToString(), + $"dust UTXOs must not be offered for coin selection with strategy {strategy}"); + } + + // 3. Request an automatic withdrawal: no explicit outpoints, so coin selection runs. + var destination = await rpc.GetNewAddressAsync(); + var withdrawal = await client.RequestWithdrawalAsync(new RequestWithdrawalRequest + { + WalletId = walletId, + Description = "E2E dust protection test", + Destinations = { new Destination { Address = destination.ToString(), AmountSats = 1_000_000 } }, + MempoolFeeRate = FEES_TYPE.CustomFee, + CustomFeeRate = 2, + }, headers); + _output.WriteLine($"withdrawal request {withdrawal.RequestId} → txid {withdrawal.Txid}"); + withdrawal.IsHotWallet.Should().BeTrue(); + + // 4. The hot wallet signs and broadcasts asynchronously (PerformWithdrawalJob); wait for + // the tx to hit the mempool and assert the dust outpoint is not among its inputs. + var withdrawalTx = await RetryAsync(async () => + { + var tx = await rpc.GetRawTransactionAsync(uint256.Parse(withdrawal.Txid), throwIfNotFound: false); + return tx ?? throw new InvalidOperationException("withdrawal tx not broadcast yet"); + }, attempts: 30, delay: TimeSpan.FromSeconds(4), what: "GetRawTransaction (withdrawal broadcast)"); + + withdrawalTx.Inputs.Select(i => i.PrevOut).Should().NotContain(dustOutpoint, + "a freshly-confirmed dust UTXO would be the first pick of SelectUTXOsByOldest if it were not filtered out"); + + // 5. The dust UTXO must remain unspent (and unlocked) after the withdrawal. + var allUtxos = await client.GetUtxosAsync(new GetUtxosRequest(), headers); + allUtxos.Confirmed.Select(u => u.Outpoint).Should().Contain(dustOutpoint.ToString(), + "the dust UTXO must be left untouched in the wallet"); + } + + // ---- helpers ----------------------------------------------------------------------------- + + private async Task MineAsync(RPCClient rpc, int blocks) + { + var addr = await rpc.GetNewAddressAsync(); + await rpc.GenerateToAddressAsync(blocks, addr); + } + + private async Task RetryAsync(Func> action, int attempts, TimeSpan delay, string what) + { + Exception? last = null; + for (var i = 0; i < attempts; i++) + { + try { return await action(); } + catch (Exception ex) { last = ex; _output.WriteLine($"{what} attempt {i + 1}/{attempts} failed: {ex.Message}"); } + await Task.Delay(delay); + } + throw new InvalidOperationException($"{what} did not succeed after {attempts} attempts", last); + } + + private static NodeGuardService.NodeGuardServiceClient CreateClient(out Metadata headers) + { + var endpoint = Env("NODEGUARD_GRPC_ENDPOINT", "http://localhost:50051"); + headers = new Metadata { { "auth-token", Env("NODEGUARD_API_TOKEN", DefaultDevToken) } }; + return new NodeGuardService.NodeGuardServiceClient(GrpcChannel.ForAddress(endpoint)); + } + + private static RPCClient CreateBitcoindRpc() + { + var url = Env("BITCOIND_RPC_URL", "http://localhost:18443"); + var cred = new NetworkCredential(Env("BITCOIND_RPC_USER", "polaruser"), Env("BITCOIND_RPC_PASS", "polarpass")); + var rpc = new RPCClient(cred, new Uri(url), Network.RegTest); + return rpc.SetWalletContext(Env("BITCOIND_RPC_WALLET", "default")); + } + + private static string Env(string name, string fallback) + => Environment.GetEnvironmentVariable(name) is { Length: > 0 } v ? v : fallback; +} diff --git a/test/NodeGuard.Tests/E2E/E2ECollection.cs b/test/NodeGuard.Tests/E2E/E2ECollection.cs new file mode 100644 index 00000000..1e5a7687 --- /dev/null +++ b/test/NodeGuard.Tests/E2E/E2ECollection.cs @@ -0,0 +1,30 @@ +/* + * NodeGuard + * Copyright (C) 2023 Elenpay + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +namespace NodeGuard.Tests.E2E; + +/// +/// All E2E test classes share this collection so they run sequentially: they compete for the +/// same seeded dev wallets and mine blocks on the same regtest chain, so running them in +/// parallel makes them flaky. +/// +[CollectionDefinition("E2E", DisableParallelization = true)] +public class E2ECollection +{ +} diff --git a/test/NodeGuard.Tests/E2E/GetNewWalletAddressE2ETests.cs b/test/NodeGuard.Tests/E2E/GetNewWalletAddressE2ETests.cs index 91f90e6a..bfc99dd8 100644 --- a/test/NodeGuard.Tests/E2E/GetNewWalletAddressE2ETests.cs +++ b/test/NodeGuard.Tests/E2E/GetNewWalletAddressE2ETests.cs @@ -34,6 +34,7 @@ namespace NodeGuard.Tests.E2E; /// E2E_HOT_WALLET_ID NodeGuard hot wallet to query (default 3) /// [Trait("Category", "E2E")] +[Collection("E2E")] public class GetNewWalletAddressE2ETests { private const string DefaultDevToken = "8rvSsUGeyXXdDQrHctcTey/xtHdZQEn945KHwccKp9Q="; diff --git a/test/NodeGuard.Tests/E2E/RebalanceE2ETests.cs b/test/NodeGuard.Tests/E2E/RebalanceE2ETests.cs index 90f33b35..7fdf93b5 100644 --- a/test/NodeGuard.Tests/E2E/RebalanceE2ETests.cs +++ b/test/NodeGuard.Tests/E2E/RebalanceE2ETests.cs @@ -43,6 +43,7 @@ namespace NodeGuard.Tests.E2E; /// E2E_HOT_WALLET_ID NodeGuard hot wallet to fund the channel (default 3) /// [Trait("Category", "E2E")] +[Collection("E2E")] public class RebalanceE2ETests { private const string DefaultDevToken = "8rvSsUGeyXXdDQrHctcTey/xtHdZQEn945KHwccKp9Q="; diff --git a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs index 3df4e2cd..c9270832 100644 --- a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs +++ b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs @@ -35,6 +35,7 @@ using Nodeguard; using Quartz; using Quartz.Impl; +using NodeGuard.TestHelpers; using Channel = NodeGuard.Data.Models.Channel; using Key = NodeGuard.Data.Models.Key; using LiquidityRule = NodeGuard.Data.Models.LiquidityRule; @@ -1751,5 +1752,89 @@ public async Task GetRebalances_ResolvesNodePubkeyAndForwardsFilters() response.Rebalances[0].RebalanceId.Should().Be(1); response.Rebalances[0].NodePubkey.Should().Be("pubkey1"); } + + [Theory] + [InlineData(COIN_SELECTION_STRATEGY.SmallestFirst)] + [InlineData(COIN_SELECTION_STRATEGY.BiggestFirst)] + [InlineData(COIN_SELECTION_STRATEGY.ClosestToTargetFirst)] + [InlineData(COIN_SELECTION_STRATEGY.UpToAmount)] + public async Task GetAvailableUtxos_ExcludesDustUtxos(COIN_SELECTION_STRATEGY strategy) + { + // Arrange + var wallet = CreateWallet.SingleSig(CreateWallet.CreateInternalWallet()); + var walletRepository = new Mock(); + walletRepository.Setup(x => x.GetById(It.IsAny())).ReturnsAsync(wallet); + + var address = BitcoinAddress.Create("bcrt1q590shaxaf5u08ml8jwlzghz99dup3z9592vxal", Network.RegTest); + var dustUtxo = new UTXO() + { + Outpoint = new OutPoint(new uint256(1), 0), + Value = new Money(Constants.MINIMUM_UTXO_VALUE_SATS), + Address = address + }; + var availableUtxo = new UTXO() + { + Outpoint = new OutPoint(new uint256(2), 0), + Value = new Money(10_000L), + Address = address + }; + var unconfirmedDustUtxo = new UTXO() + { + Outpoint = new OutPoint(new uint256(3), 0), + Value = new Money(100L), + Address = address + }; + + var nbxplorerService = new Mock(); + nbxplorerService.Setup(x => x.GetUTXOsAsync(It.IsAny(), default)) + .ReturnsAsync(new UTXOChanges() + { + Confirmed = new UTXOChange() { UTXOs = new List() { dustUtxo, availableUtxo } }, + Unconfirmed = new UTXOChange() { UTXOs = new List() { unconfirmedDustUtxo } } + }); + List? ignoredOutpoints = null; + nbxplorerService.Setup(x => x.GetUTXOsByLimitAsync(It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny>(), default)) + .Callback?, + CancellationToken>((_, _, _, _, _, ignore, _) => ignoredOutpoints = ignore) + .ReturnsAsync(new UTXOChanges() + { + // The fork would not return ignored dust, but keep it here to prove the local + // defense-in-depth filter strips it from the response either way + Confirmed = new UTXOChange() { UTXOs = new List() { dustUtxo, availableUtxo } }, + Unconfirmed = new UTXOChange() { UTXOs = new List() { unconfirmedDustUtxo } } + }); + + var fmutxoRepository = new Mock(); + fmutxoRepository.Setup(x => x.GetLockedUTXOsByWalletId(It.IsAny())) + .ReturnsAsync(new List()); + + var coinSelectionService = new Mock(); + coinSelectionService.Setup(x => x.GetFrozenUTXOs()).ReturnsAsync(new List()); + + var service = CreateNodeGuardService( + walletRepository: walletRepository.Object, + nbXplorerService: nbxplorerService.Object, + fmutxoRepository: fmutxoRepository.Object, + coinSelectionService: coinSelectionService.Object); + + // Act + var response = await service.GetAvailableUtxos( + new GetAvailableUtxosRequest { WalletId = 1, Strategy = strategy, Amount = 10_000 }, + TestServerCallContext.Create()); + + // Assert + response.Confirmed.Should().ContainSingle(); + response.Confirmed[0].Outpoint.Should().Be(availableUtxo.Outpoint.ToString()); + response.Unconfirmed.Should().BeEmpty(); + + // Both dust UTXOs must also be ignored server-side so the fork does not count them + // towards the requested amount + ignoredOutpoints.Should().NotBeNull(); + ignoredOutpoints.Should().Contain(dustUtxo.Outpoint.ToString()); + ignoredOutpoints.Should().Contain(unconfirmedDustUtxo.Outpoint.ToString()); + ignoredOutpoints.Should().NotContain(availableUtxo.Outpoint.ToString()); + } } } diff --git a/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs b/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs new file mode 100644 index 00000000..04a046dc --- /dev/null +++ b/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs @@ -0,0 +1,141 @@ +/* + * NodeGuard + * Copyright (C) 2023 Elenpay + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ +using AutoMapper; +using FluentAssertions; +using NodeGuard.Data.Models; +using NodeGuard.Data.Repositories.Interfaces; +using NodeGuard.TestHelpers; +using Microsoft.Extensions.Logging; +using NBitcoin; +using NBXplorer.DerivationStrategy; +using NBXplorer.Models; + +namespace NodeGuard.Services; + +public class CoinSelectionServiceTests +{ + private readonly ILogger _logger = new Mock>().Object; + private readonly InternalWallet _internalWallet = CreateWallet.CreateInternalWallet(); + + private static UTXO CreateUtxo(uint index, long satoshis) + { + return new UTXO + { + Outpoint = new OutPoint(new uint256(index), 0), + Value = new Money(satoshis) + }; + } + + private CoinSelectionService CreateCoinSelectionService(List confirmedUtxos, List? lockedUtxos = null) + { + var fmutxoRepository = new Mock(); + var nbXplorerService = new Mock(); + var utxoTagRepository = new Mock(); + var mapper = new Mock(); + + nbXplorerService + .Setup(x => x.GetUTXOsAsync(It.IsAny(), default)) + .ReturnsAsync(new UTXOChanges() + { + Confirmed = new UTXOChange() + { + UTXOs = confirmedUtxos + } + }); + fmutxoRepository + .Setup(x => x.GetLockedUTXOs(null, null)) + .ReturnsAsync(lockedUtxos ?? new List()); + utxoTagRepository + .Setup(x => x.GetByKeyValue(It.IsAny(), It.IsAny())) + .ReturnsAsync(new List()); + + return new CoinSelectionService(_logger, mapper.Object, fmutxoRepository.Object, nbXplorerService.Object, null, null, utxoTagRepository.Object); + } + + [Fact] + public async Task GetAvailableUTXOsAsync_ExcludesDustUTXOs() + { + // Arrange + var derivationStrategy = CreateWallet.SingleSig(_internalWallet).GetDerivationStrategy(); + var coinSelectionService = CreateCoinSelectionService(new List() + { + CreateUtxo(1, 100), + CreateUtxo(2, Constants.MINIMUM_UTXO_VALUE_SATS), // 546, boundary: excluded (inclusive comparison) + CreateUtxo(3, Constants.MINIMUM_UTXO_VALUE_SATS + 1), + CreateUtxo(4, 10_000) + }); + + // Act + var availableUTXOs = await coinSelectionService.GetAvailableUTXOsAsync(derivationStrategy); + + // Assert + availableUTXOs + .Select(utxo => ((Money)utxo.Value).Satoshi) + .Should() + .BeEquivalentTo(new[] { Constants.MINIMUM_UTXO_VALUE_SATS + 1, 10_000L }); + } + + [Theory] + [InlineData(CoinSelectionStrategy.SmallestFirst)] + [InlineData(CoinSelectionStrategy.BiggestFirst)] + [InlineData(CoinSelectionStrategy.ClosestToTargetFirst)] + [InlineData(CoinSelectionStrategy.UpToAmount)] + public async Task GetAvailableUTXOsAsync_WithStrategy_ExcludesDustUTXOs(CoinSelectionStrategy strategy) + { + // Arrange + var derivationStrategy = CreateWallet.SingleSig(_internalWallet).GetDerivationStrategy(); + var dustUtxo = CreateUtxo(1, Constants.MINIMUM_UTXO_VALUE_SATS); + var availableUtxo = CreateUtxo(2, 10_000); + var coinSelectionService = CreateCoinSelectionService(new List() { dustUtxo, availableUtxo }); + + // Act + var availableUTXOs = await coinSelectionService.GetAvailableUTXOsAsync( + derivationStrategy, strategy, 0, 10_000, Constants.MINIMUM_UTXO_VALUE_SATS); + + // Assert + availableUTXOs.Should().ContainSingle(); + availableUTXOs[0].Outpoint.Should().Be(availableUtxo.Outpoint); + } + + [Fact] + public async Task GetAvailableUTXOsAsync_ExcludesDustAndLockedUTXOs() + { + // Arrange + var derivationStrategy = CreateWallet.SingleSig(_internalWallet).GetDerivationStrategy(); + var dustUtxo = CreateUtxo(1, 546); + var lockedUtxo = CreateUtxo(2, 20_000); + var availableUtxo = CreateUtxo(3, 30_000); + var lockedFmutxo = new FMUTXO() + { + TxId = lockedUtxo.Outpoint.Hash.ToString(), + OutputIndex = lockedUtxo.Outpoint.N, + SatsAmount = 20_000 + }; + var coinSelectionService = CreateCoinSelectionService( + new List() { dustUtxo, lockedUtxo, availableUtxo }, + new List() { lockedFmutxo }); + + // Act + var availableUTXOs = await coinSelectionService.GetAvailableUTXOsAsync(derivationStrategy); + + // Assert + availableUTXOs.Should().ContainSingle(); + availableUTXOs[0].Outpoint.Should().Be(availableUtxo.Outpoint); + } +} From fe30227d4ff3e69b9ab9cc4e1bb471fe7e554938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:44:56 +0200 Subject: [PATCH 2/7] Fix e2e --- .../E2E/DustUtxoWithdrawalE2ETests.cs | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs b/test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs index 7cbbb896..b8fa523b 100644 --- a/test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs +++ b/test/NodeGuard.Tests/E2E/DustUtxoWithdrawalE2ETests.cs @@ -109,19 +109,23 @@ await RetryAsync(async () => // No selection strategy may surface the dust UTXO. SmallestFirst would rank it first, and // ClosestToTargetFirst targeting exactly 546 sats is the most adversarial case: the dust // UTXO would be the top candidate if it were not filtered out. - foreach (var strategy in new[] - { - COIN_SELECTION_STRATEGY.SmallestFirst, - COIN_SELECTION_STRATEGY.BiggestFirst, - COIN_SELECTION_STRATEGY.ClosestToTargetFirst, - COIN_SELECTION_STRATEGY.UpToAmount, - }) + // UpToAmount selects UTXOs whose sum stays UNDER the target, so it gets a ceiling above + // the wallet balance: with a small target the dust UTXO would be the only one that fits + // (and the correct result once it is filtered out is an empty selection). + var strategyProbes = new (COIN_SELECTION_STRATEGY Strategy, long AmountSats)[] + { + (COIN_SELECTION_STRATEGY.SmallestFirst, ProbeAmountSats), + (COIN_SELECTION_STRATEGY.BiggestFirst, ProbeAmountSats), + (COIN_SELECTION_STRATEGY.ClosestToTargetFirst, ProbeAmountSats), + (COIN_SELECTION_STRATEGY.UpToAmount, Money.Coins(25m).Satoshi), + }; + foreach (var (strategy, amountSats) in strategyProbes) { var availableUtxos = await client.GetAvailableUtxosAsync(new GetAvailableUtxosRequest { WalletId = walletId, Strategy = strategy, - Amount = ProbeAmountSats, + Amount = amountSats, ClosestTo = DustAmountSats, }, headers); availableUtxos.Confirmed.Should().NotBeEmpty( @@ -130,6 +134,17 @@ await RetryAsync(async () => $"dust UTXOs must not be offered for coin selection with strategy {strategy}"); } + // With a target below any real UTXO, UpToAmount's only fitting candidate is the dust + // UTXO — the selection must come back empty rather than surface it. + var upToDustOnly = await client.GetAvailableUtxosAsync(new GetAvailableUtxosRequest + { + WalletId = walletId, + Strategy = COIN_SELECTION_STRATEGY.UpToAmount, + Amount = ProbeAmountSats, + }, headers); + upToDustOnly.Confirmed.Should().BeEmpty( + "no UTXO other than the (filtered) dust fits an UpToAmount target below the smallest real UTXO"); + // 3. Request an automatic withdrawal: no explicit outpoints, so coin selection runs. var destination = await rpc.GetNewAddressAsync(); var withdrawal = await client.RequestWithdrawalAsync(new RequestWithdrawalRequest From 2a3ffb0f3c35740a5aab0e25e8c1890c6f1338d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:58:24 +0200 Subject: [PATCH 3/7] fix(coin-selection): pass ignored outpoints to custom NBXplorer backend When `NBXPLORER_ENABLE_CUSTOM_BACKEND` is enabled, locked, frozen and dust UTXOs were counted by the backend towards the requested amount and only stripped by the local filter afterwards, producing selections that fell short of the target. - Extract `GetLockedFrozenOutpoints` from `FilterLockedFrozenUTXOs` so the outpoint list can be reused. - Fetch all UTXOs, compute dust outpoints (<= `MINIMUM_UTXO_VALUE_SATS`) and merge them with locked/frozen outpoints, then forward the combined list to `GetUTXOsByLimitAsync` as `ignoreOutpoints`. - Make `Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND` mutable so tests can toggle the custom-backend path. --- src/Helpers/Constants.cs | 2 +- src/Services/CoinSelectionService.cs | 26 +++++- .../Services/CoinSelectionServiceTests.cs | 83 +++++++++++++++++++ 3 files changed, 106 insertions(+), 5 deletions(-) diff --git a/src/Helpers/Constants.cs b/src/Helpers/Constants.cs index 155e4ac0..44b49bb6 100644 --- a/src/Helpers/Constants.cs +++ b/src/Helpers/Constants.cs @@ -28,7 +28,7 @@ public class Constants public static readonly bool ENABLE_REMOTE_SIGNER; public static readonly bool PUSH_NOTIFICATIONS_ONESIGNAL_ENABLED; public static readonly bool ENABLE_HW_SUPPORT; - public static readonly bool NBXPLORER_ENABLE_CUSTOM_BACKEND = false; + public static bool NBXPLORER_ENABLE_CUSTOM_BACKEND = false; // Not readonly so we can change it in tests /// /// Allow simultaneous channel opening operations using the same source and destination nodes /// diff --git a/src/Services/CoinSelectionService.cs b/src/Services/CoinSelectionService.cs index 3173964a..ddf4daba 100644 --- a/src/Services/CoinSelectionService.cs +++ b/src/Services/CoinSelectionService.cs @@ -148,15 +148,21 @@ public async Task> GetLockedUTXOsForRequest(IBitcoinRequest bitcoinRe return utxos.Confirmed.UTXOs.Where(utxo => lockedUTXOsList.Contains(utxo.Outpoint.ToString())).ToList(); } - private async Task> FilterLockedFrozenUTXOs(UTXOChanges? utxoChanges) + private async Task> GetLockedFrozenOutpoints() { var lockedUTXOs = await _fmutxoRepository.GetLockedUTXOs(); - var listLocked = lockedUTXOs.Select(utxo => $"{utxo.TxId}-{utxo.OutputIndex}").ToList(); + var listLocked = lockedUTXOs.Select(utxo => $"{utxo.TxId}-{utxo.OutputIndex}").ToList(); var listFrozen = await GetFrozenUTXOs(); var frozenAndLockedOutpoints = new List(); frozenAndLockedOutpoints.AddRange(listLocked); frozenAndLockedOutpoints.AddRange(listFrozen); - + return frozenAndLockedOutpoints; + } + + private async Task> FilterLockedFrozenUTXOs(UTXOChanges? utxoChanges) + { + var frozenAndLockedOutpoints = await GetLockedFrozenOutpoints(); + utxoChanges.RemoveDuplicateUTXOs(); var availableUTXOs = new List(); @@ -211,7 +217,19 @@ public async Task> GetAvailableUTXOsAsync(DerivationStrategyBase deri UTXOChanges utxoChanges; if (Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND) { - utxoChanges = await _nbXplorerService.GetUTXOsByLimitAsync(derivationStrategy, strategy, limit, amount, closestTo); + // Tell the backend which UTXOs to skip (locked, frozen and dust), otherwise it counts + // them towards the requested amount and the local filter below strips them afterwards, + // returning a selection that falls short of that amount + var ignoreOutpoints = await GetLockedFrozenOutpoints(); + var allUtxos = await _nbXplorerService.GetUTXOsAsync(derivationStrategy); + allUtxos.RemoveDuplicateUTXOs(); + var dustOutpoints = allUtxos.Confirmed.UTXOs + .Concat(allUtxos.Unconfirmed.UTXOs) + .Where(utxo => ((Money)utxo.Value).Satoshi <= Constants.MINIMUM_UTXO_VALUE_SATS) + .Select(utxo => utxo.Outpoint.ToString()); + ignoreOutpoints.AddRange(dustOutpoints); + + utxoChanges = await _nbXplorerService.GetUTXOsByLimitAsync(derivationStrategy, strategy, limit, amount, closestTo, ignoreOutpoints); } else { diff --git a/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs b/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs index 04a046dc..e48a21bc 100644 --- a/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs +++ b/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs @@ -113,6 +113,89 @@ public async Task GetAvailableUTXOsAsync_WithStrategy_ExcludesDustUTXOs(CoinSele availableUTXOs[0].Outpoint.Should().Be(availableUtxo.Outpoint); } + [Fact] + public async Task GetAvailableUTXOsAsync_WithCustomBackend_IgnoresLockedFrozenAndDustServerSide() + { + var previousCustomBackend = Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND; + Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND = true; + try + { + // Arrange + var derivationStrategy = CreateWallet.SingleSig(_internalWallet).GetDerivationStrategy(); + var dustUtxo = CreateUtxo(1, Constants.MINIMUM_UTXO_VALUE_SATS); + var lockedUtxo = CreateUtxo(2, 20_000); + var frozenUtxo = CreateUtxo(3, 30_000); + var availableUtxo = CreateUtxo(4, 40_000); + + var fmutxoRepository = new Mock(); + fmutxoRepository + .Setup(x => x.GetLockedUTXOs(null, null)) + .ReturnsAsync(new List() + { + new() + { + TxId = lockedUtxo.Outpoint.Hash.ToString(), + OutputIndex = lockedUtxo.Outpoint.N, + SatsAmount = 20_000 + } + }); + + var utxoTagRepository = new Mock(); + utxoTagRepository + .Setup(x => x.GetByKeyValue(Constants.IsFrozenTag, "true")) + .ReturnsAsync(new List() { new() { Outpoint = frozenUtxo.Outpoint.ToString() } }); + utxoTagRepository + .Setup(x => x.GetByKeyValue(Constants.IsManuallyFrozenTag, It.IsAny())) + .ReturnsAsync(new List()); + + var nbXplorerService = new Mock(); + nbXplorerService + .Setup(x => x.GetUTXOsAsync(It.IsAny(), default)) + .ReturnsAsync(new UTXOChanges() + { + Confirmed = new UTXOChange() + { + UTXOs = new List() { dustUtxo, lockedUtxo, frozenUtxo, availableUtxo } + } + }); + List? ignoredOutpoints = null; + nbXplorerService + .Setup(x => x.GetUTXOsByLimitAsync(It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny>(), default)) + .Callback?, + CancellationToken>((_, _, _, _, _, ignore, _) => ignoredOutpoints = ignore) + .ReturnsAsync(new UTXOChanges() + { + Confirmed = new UTXOChange() { UTXOs = new List() { availableUtxo } } + }); + + var mapper = new Mock(); + var coinSelectionService = new CoinSelectionService(_logger, mapper.Object, fmutxoRepository.Object, + nbXplorerService.Object, null, null, utxoTagRepository.Object); + + // Act + var availableUTXOs = await coinSelectionService.GetAvailableUTXOsAsync( + derivationStrategy, CoinSelectionStrategy.SmallestFirst, 0, 40_000, 0); + + // Assert + availableUTXOs.Should().ContainSingle(); + availableUTXOs[0].Outpoint.Should().Be(availableUtxo.Outpoint); + + // Locked, frozen and dust UTXOs must all be ignored server-side so the backend does + // not count them towards the requested amount and return a short selection + ignoredOutpoints.Should().NotBeNull(); + ignoredOutpoints.Should().Contain(dustUtxo.Outpoint.ToString()); + ignoredOutpoints.Should().Contain($"{lockedUtxo.Outpoint.Hash}-{lockedUtxo.Outpoint.N}"); + ignoredOutpoints.Should().Contain(frozenUtxo.Outpoint.ToString()); + ignoredOutpoints.Should().NotContain(availableUtxo.Outpoint.ToString()); + } + finally + { + Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND = previousCustomBackend; + } + } + [Fact] public async Task GetAvailableUTXOsAsync_ExcludesDustAndLockedUTXOs() { From f4e65842d49d64af3ec021a709165ffd00032798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:04:41 +0200 Subject: [PATCH 4/7] feat(coin-selection): implement method to gather ignored outpoints for coin selection --- src/Services/CoinSelectionService.cs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Services/CoinSelectionService.cs b/src/Services/CoinSelectionService.cs index ddf4daba..0272588d 100644 --- a/src/Services/CoinSelectionService.cs +++ b/src/Services/CoinSelectionService.cs @@ -159,6 +159,22 @@ private async Task> GetLockedFrozenOutpoints() return frozenAndLockedOutpoints; } + /// + /// Outpoints that must never be offered for coin selection: locked, frozen and dust UTXOs. + /// Locked/frozen are state lookups (database/tags); dust is a value predicate over the + /// wallet's current UTXO set, so the caller provides the already-fetched set instead of this + /// method issuing its own NBXplorer query. + /// + private async Task> GetIgnoredOutpoints(UTXOChanges utxos) + { + var ignoredOutpoints = await GetLockedFrozenOutpoints(); + ignoredOutpoints.AddRange(utxos.Confirmed.UTXOs + .Concat(utxos.Unconfirmed.UTXOs) + .Where(utxo => ((Money)utxo.Value).Satoshi <= Constants.MINIMUM_UTXO_VALUE_SATS) + .Select(utxo => utxo.Outpoint.ToString())); + return ignoredOutpoints; + } + private async Task> FilterLockedFrozenUTXOs(UTXOChanges? utxoChanges) { var frozenAndLockedOutpoints = await GetLockedFrozenOutpoints(); @@ -220,14 +236,9 @@ public async Task> GetAvailableUTXOsAsync(DerivationStrategyBase deri // Tell the backend which UTXOs to skip (locked, frozen and dust), otherwise it counts // them towards the requested amount and the local filter below strips them afterwards, // returning a selection that falls short of that amount - var ignoreOutpoints = await GetLockedFrozenOutpoints(); var allUtxos = await _nbXplorerService.GetUTXOsAsync(derivationStrategy); allUtxos.RemoveDuplicateUTXOs(); - var dustOutpoints = allUtxos.Confirmed.UTXOs - .Concat(allUtxos.Unconfirmed.UTXOs) - .Where(utxo => ((Money)utxo.Value).Satoshi <= Constants.MINIMUM_UTXO_VALUE_SATS) - .Select(utxo => utxo.Outpoint.ToString()); - ignoreOutpoints.AddRange(dustOutpoints); + var ignoreOutpoints = await GetIgnoredOutpoints(allUtxos); utxoChanges = await _nbXplorerService.GetUTXOsByLimitAsync(derivationStrategy, strategy, limit, amount, closestTo, ignoreOutpoints); } From 45be231b32aa8b74ea0abbec4b66808adf31680f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:24:06 +0200 Subject: [PATCH 5/7] Fall back to plain UTXO listing on backend failure, as there's a concern of maximum amount of ignored outpoints. When `NBXPLORER_ENABLE_CUSTOM_BACKEND` is enabled and the custom UTXO selection endpoint throws, callers previously got a hard failure. Wrap the custom-backend path in a try/catch and degrade to the plain `GetUTXOsAsync` listing on error, matching the code path used when the custom backend is disabled. The local filter still strips locked, frozen and dust UTXOs, so none can leak through. A warning is logged with the requested strategy for observability. Adds a unit test covering the failure path to ensure the fallback returns only the available UTXO and excludes locked and dust ones. --- src/Services/CoinSelectionService.cs | 28 ++++++-- .../Services/CoinSelectionServiceTests.cs | 65 +++++++++++++++++++ 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/Services/CoinSelectionService.cs b/src/Services/CoinSelectionService.cs index 0272588d..6c939d3d 100644 --- a/src/Services/CoinSelectionService.cs +++ b/src/Services/CoinSelectionService.cs @@ -233,14 +233,28 @@ public async Task> GetAvailableUTXOsAsync(DerivationStrategyBase deri UTXOChanges utxoChanges; if (Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND) { - // Tell the backend which UTXOs to skip (locked, frozen and dust), otherwise it counts - // them towards the requested amount and the local filter below strips them afterwards, - // returning a selection that falls short of that amount - var allUtxos = await _nbXplorerService.GetUTXOsAsync(derivationStrategy); - allUtxos.RemoveDuplicateUTXOs(); - var ignoreOutpoints = await GetIgnoredOutpoints(allUtxos); + try + { + // Tell the backend which UTXOs to skip (locked, frozen and dust), otherwise it + // counts them towards the requested amount and the local filter below strips them + // afterwards, returning a selection that falls short of that amount + var allUtxos = await _nbXplorerService.GetUTXOsAsync(derivationStrategy); + allUtxos.RemoveDuplicateUTXOs(); + var ignoreOutpoints = await GetIgnoredOutpoints(allUtxos); - utxoChanges = await _nbXplorerService.GetUTXOsByLimitAsync(derivationStrategy, strategy, limit, amount, closestTo, ignoreOutpoints); + utxoChanges = await _nbXplorerService.GetUTXOsByLimitAsync(derivationStrategy, strategy, limit, amount, closestTo, ignoreOutpoints); + } + catch (Exception e) + { + // Skip the custom backend entirely and degrade to the plain UTXO listing, same as + // when NBXPLORER_ENABLE_CUSTOM_BACKEND is off: the strategy/amount are no longer + // applied server-side, but the local filter below still strips locked, frozen and + // dust UTXOs, so none of them can leak through + _logger.LogWarning(e, + "UTXO selection through the custom NBXplorer backend failed for strategy {Strategy}, falling back to the plain UTXO listing", + strategy); + utxoChanges = await _nbXplorerService.GetUTXOsAsync(derivationStrategy); + } } else { diff --git a/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs b/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs index e48a21bc..a9e9bae8 100644 --- a/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs +++ b/test/NodeGuard.Tests/Services/CoinSelectionServiceTests.cs @@ -196,6 +196,71 @@ public async Task GetAvailableUTXOsAsync_WithCustomBackend_IgnoresLockedFrozenAn } } + [Fact] + public async Task GetAvailableUTXOsAsync_WithCustomBackendFailing_FallsBackToPlainListingAndLocalFilter() + { + var previousCustomBackend = Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND; + Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND = true; + try + { + // Arrange + var derivationStrategy = CreateWallet.SingleSig(_internalWallet).GetDerivationStrategy(); + var dustUtxo = CreateUtxo(1, Constants.MINIMUM_UTXO_VALUE_SATS); + var lockedUtxo = CreateUtxo(2, 20_000); + var availableUtxo = CreateUtxo(3, 30_000); + + var fmutxoRepository = new Mock(); + fmutxoRepository + .Setup(x => x.GetLockedUTXOs(null, null)) + .ReturnsAsync(new List() + { + new() + { + TxId = lockedUtxo.Outpoint.Hash.ToString(), + OutputIndex = lockedUtxo.Outpoint.N, + SatsAmount = 20_000 + } + }); + + var utxoTagRepository = new Mock(); + utxoTagRepository + .Setup(x => x.GetByKeyValue(It.IsAny(), It.IsAny())) + .ReturnsAsync(new List()); + + var nbXplorerService = new Mock(); + nbXplorerService + .Setup(x => x.GetUTXOsAsync(It.IsAny(), default)) + .ReturnsAsync(new UTXOChanges() + { + Confirmed = new UTXOChange() + { + UTXOs = new List() { dustUtxo, lockedUtxo, availableUtxo } + } + }); + nbXplorerService + .Setup(x => x.GetUTXOsByLimitAsync(It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny>(), default)) + .ThrowsAsync(new HttpRequestException("custom backend unavailable")); + + var mapper = new Mock(); + var coinSelectionService = new CoinSelectionService(_logger, mapper.Object, fmutxoRepository.Object, + nbXplorerService.Object, null, null, utxoTagRepository.Object); + + // Act + var availableUTXOs = await coinSelectionService.GetAvailableUTXOsAsync( + derivationStrategy, CoinSelectionStrategy.SmallestFirst, 0, 30_000, 0); + + // Assert: the plain listing is used and locked/dust UTXOs are still filtered locally + availableUTXOs.Should().ContainSingle(); + availableUTXOs[0].Outpoint.Should().Be(availableUtxo.Outpoint); + } + finally + { + Constants.NBXPLORER_ENABLE_CUSTOM_BACKEND = previousCustomBackend; + } + } + [Fact] public async Task GetAvailableUTXOsAsync_ExcludesDustAndLockedUTXOs() { From 30840a4f606886369c7d2c970f10e627d05a5278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:31:38 +0200 Subject: [PATCH 6/7] fix(rpc): preserve empty-selection contract on custom backend failure Restore the previous GetAvailableUtxos behavior where a failing custom NBXplorer backend yields an empty UTXO selection instead of surfacing an error to the caller. NBXplorerService.GetUTXOsByLimitAsync now correctly throws on non-success responses (replacing the unreachable empty return and fixing `throw e` stack rewrite), so NodeGuardService wraps the call in try/catch, logs a warning, and falls back to an empty UTXOChanges. Adds a unit test covering the custom-backend failure path. --- src/Rpc/NodeGuardService.cs | 29 ++++++++++---- src/Services/NBXplorerService.cs | 7 ++-- .../Rpc/NodeGuardServiceTests.cs | 39 +++++++++++++++++++ 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/Rpc/NodeGuardService.cs b/src/Rpc/NodeGuardService.cs index 8cbb704f..a42079fb 100644 --- a/src/Rpc/NodeGuardService.cs +++ b/src/Rpc/NodeGuardService.cs @@ -1069,14 +1069,27 @@ public override async Task GetAvailableUtxos(GetAvailableUtxos ignoreOutpoints.AddRange(listFrozen); ignoreOutpoints.AddRange(listDust); - var utxos = await _nbXplorerService.GetUTXOsByLimitAsync( - derivationStrategy, - coinSelectionStrategy, - request.Limit, - request.Amount, - request.ClosestTo, - ignoreOutpoints - ); + UTXOChanges utxos; + try + { + utxos = await _nbXplorerService.GetUTXOsByLimitAsync( + derivationStrategy, + coinSelectionStrategy, + request.Limit, + request.Amount, + request.ClosestTo, + ignoreOutpoints + ); + } + catch (Exception e) + { + // Preserve this RPC's previous contract: a failing custom backend yields an empty + // selection instead of an error + _logger.LogWarning(e, + "UTXO selection through the custom NBXplorer backend failed for wallet {WalletId}, returning an empty selection", + request.WalletId); + utxos = new UTXOChanges(); + } var confirmedUtxos = utxos.Confirmed.UTXOs .Where(utxo => ((Money)utxo.Value).Satoshi > Constants.MINIMUM_UTXO_VALUE_SATS) diff --git a/src/Services/NBXplorerService.cs b/src/Services/NBXplorerService.cs index 08f45ad6..fed85114 100644 --- a/src/Services/NBXplorerService.cs +++ b/src/Services/NBXplorerService.cs @@ -179,14 +179,15 @@ public async Task GetUTXOsByLimitAsync(DerivationStrategyBase deriv return client.Serializer.ToObject(await response.Content.ReadAsStringAsync().ConfigureAwait(false)); } + + throw new HttpRequestException( + $"selectutxos request failed with status code {(int)response.StatusCode}: {await response.Content.ReadAsStringAsync(cancellation).ConfigureAwait(false)}"); } catch (Exception e) { _logger.LogError(e.ToString()); - throw e; + throw; } - - return new UTXOChanges(); } public async Task GetFeeRateAsync(int blockCount, FeeRate fallbackFeeRate, diff --git a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs index c9270832..806c4806 100644 --- a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs +++ b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs @@ -1836,5 +1836,44 @@ public async Task GetAvailableUtxos_ExcludesDustUtxos(COIN_SELECTION_STRATEGY st ignoredOutpoints.Should().Contain(unconfirmedDustUtxo.Outpoint.ToString()); ignoredOutpoints.Should().NotContain(availableUtxo.Outpoint.ToString()); } + + [Fact] + public async Task GetAvailableUtxos_CustomBackendFails_ReturnsEmptySelection() + { + // Arrange + var wallet = CreateWallet.SingleSig(CreateWallet.CreateInternalWallet()); + var walletRepository = new Mock(); + walletRepository.Setup(x => x.GetById(It.IsAny())).ReturnsAsync(wallet); + + var nbxplorerService = new Mock(); + nbxplorerService.Setup(x => x.GetUTXOsAsync(It.IsAny(), default)) + .ReturnsAsync(new UTXOChanges()); + nbxplorerService.Setup(x => x.GetUTXOsByLimitAsync(It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny>(), default)) + .ThrowsAsync(new HttpRequestException("selectutxos request failed with status code 400")); + + var fmutxoRepository = new Mock(); + fmutxoRepository.Setup(x => x.GetLockedUTXOsByWalletId(It.IsAny())) + .ReturnsAsync(new List()); + + var coinSelectionService = new Mock(); + coinSelectionService.Setup(x => x.GetFrozenUTXOs()).ReturnsAsync(new List()); + + var service = CreateNodeGuardService( + walletRepository: walletRepository.Object, + nbXplorerService: nbxplorerService.Object, + fmutxoRepository: fmutxoRepository.Object, + coinSelectionService: coinSelectionService.Object); + + // Act + var response = await service.GetAvailableUtxos( + new GetAvailableUtxosRequest { WalletId = 1, Amount = 10_000 }, + TestServerCallContext.Create()); + + // Assert: the RPC keeps its previous contract and returns an empty selection + response.Confirmed.Should().BeEmpty(); + response.Unconfirmed.Should().BeEmpty(); + } } } From b8243660cc2c3cd3ad4eea4b6158db1f021fd5e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:42:22 +0200 Subject: [PATCH 7/7] refactor(coin-selection): address review comments, trim docs and drop redundant RPC dust filter Co-Authored-By: Claude Fable 5 --- src/Helpers/Constants.cs | 1 - src/Rpc/NodeGuardService.cs | 8 ++------ src/Services/CoinSelectionService.cs | 3 --- test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs | 6 ++---- 4 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/Helpers/Constants.cs b/src/Helpers/Constants.cs index 44b49bb6..d03f17bc 100644 --- a/src/Helpers/Constants.cs +++ b/src/Helpers/Constants.cs @@ -109,7 +109,6 @@ public class Constants /// /// UTXOs with value less than or equal to this are excluded from coin selection (dust-attack protection). - /// Can be configured via the MINIMUM_UTXO_VALUE_SATS environment variable. /// public static readonly long MINIMUM_UTXO_VALUE_SATS = 546; diff --git a/src/Rpc/NodeGuardService.cs b/src/Rpc/NodeGuardService.cs index a42079fb..fd1ac3a7 100644 --- a/src/Rpc/NodeGuardService.cs +++ b/src/Rpc/NodeGuardService.cs @@ -1091,17 +1091,13 @@ public override async Task GetAvailableUtxos(GetAvailableUtxos utxos = new UTXOChanges(); } - var confirmedUtxos = utxos.Confirmed.UTXOs - .Where(utxo => ((Money)utxo.Value).Satoshi > Constants.MINIMUM_UTXO_VALUE_SATS) - .Select(utxo => new Utxo() + var confirmedUtxos = utxos.Confirmed.UTXOs.Select(utxo => new Utxo() { Amount = (Money)utxo.Value, Outpoint = utxo.Outpoint.ToString(), Address = utxo.Address.ToString() }); - var unconfirmedUtxos = utxos.Unconfirmed.UTXOs - .Where(utxo => ((Money)utxo.Value).Satoshi > Constants.MINIMUM_UTXO_VALUE_SATS) - .Select(utxo => new Utxo() + var unconfirmedUtxos = utxos.Unconfirmed.UTXOs.Select(utxo => new Utxo() { Amount = (Money)utxo.Value, Outpoint = utxo.Outpoint.ToString(), diff --git a/src/Services/CoinSelectionService.cs b/src/Services/CoinSelectionService.cs index 6c939d3d..927065b0 100644 --- a/src/Services/CoinSelectionService.cs +++ b/src/Services/CoinSelectionService.cs @@ -161,9 +161,6 @@ private async Task> GetLockedFrozenOutpoints() /// /// Outpoints that must never be offered for coin selection: locked, frozen and dust UTXOs. - /// Locked/frozen are state lookups (database/tags); dust is a value predicate over the - /// wallet's current UTXO set, so the caller provides the already-fetched set instead of this - /// method issuing its own NBXplorer query. /// private async Task> GetIgnoredOutpoints(UTXOChanges utxos) { diff --git a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs index 806c4806..b96d1ba1 100644 --- a/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs +++ b/test/NodeGuard.Tests/Rpc/NodeGuardServiceTests.cs @@ -1800,10 +1800,8 @@ public async Task GetAvailableUtxos_ExcludesDustUtxos(COIN_SELECTION_STRATEGY st CancellationToken>((_, _, _, _, _, ignore, _) => ignoredOutpoints = ignore) .ReturnsAsync(new UTXOChanges() { - // The fork would not return ignored dust, but keep it here to prove the local - // defense-in-depth filter strips it from the response either way - Confirmed = new UTXOChange() { UTXOs = new List() { dustUtxo, availableUtxo } }, - Unconfirmed = new UTXOChange() { UTXOs = new List() { unconfirmedDustUtxo } } + // The backend honors the ignore list, so the dust UTXOs never come back + Confirmed = new UTXOChange() { UTXOs = new List() { availableUtxo } } }); var fmutxoRepository = new Mock();