From 20307f0ac0e17c6e478e2e11318dc7a9b18fc5d1 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 17 Sep 2026 00:27:38 +0200 Subject: [PATCH 1/2] wallet: don't double count discarded output groups with avoidpartialspends --- src/wallet/spend.cpp | 3 ++- test/functional/wallet_create_tx.py | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index cf5be440f732..f0cfcf5d3189 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -672,7 +672,8 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, filtered_groups[filter].Push(group, type, positive_only, /*insert_mixed=*/!positive_only); accepted = true; } - if (!accepted) ret_discarded_groups.emplace_back(group); + // The positive-only groups are a subset of the mixed ones, don't record them twice + if (!accepted && !positive_only) ret_discarded_groups.emplace_back(group); } } }; diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index a62759761f55..93ed0d9eaf76 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -35,6 +35,7 @@ def run_test(self): self.test_anti_fee_sniping() self.test_tx_size_too_large() self.test_create_too_long_mempool_chain() + self.test_too_long_mempool_chain_avoid_partial_spends() self.test_version3() def test_anti_fee_sniping(self): @@ -110,6 +111,32 @@ def test_create_too_long_mempool_chain(self): test_wallet.unloadwallet() + def test_too_long_mempool_chain_avoid_partial_spends(self): + self.log.info('Check that a discarded too-long-chain coin is not counted twice with avoidpartialspends') + df_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + + # avoid_reuse implies avoidpartialspends + self.nodes[0].createwallet(wallet_name="aps", avoid_reuse=True) + aps_wallet = self.nodes[0].get_wallet_rpc("aps") + + df_wallet.sendtoaddress(aps_wallet.getnewaddress(), 0.3) + self.generate(self.nodes[0], 1) + + # Spend the 0.3 coin to ourselves until it hits the ancestor limit. The chain is from us, so + # it is trusted and available for selection, but too long to be eligible, so it gets discarded. + for _ in range(25): + txid = aps_wallet.sendall([aps_wallet.getnewaddress()])['txid'] + assert_equal(self.nodes[0].getmempoolentry(txid)['ancestorcount'], 25) + + # Add a confirmed 0.7 coin at another address, leaving the chain in the mempool + txid = df_wallet.sendtoaddress(aps_wallet.getnewaddress(), 0.7) + self.generateblock(self.nodes[0], output=df_wallet.getnewaddress(), transactions=[txid]) + + # The confirmed 0.7 coin alone covers this payment + aps_wallet.sendtoaddress(df_wallet.getnewaddress(), 0.65) + + aps_wallet.unloadwallet() + def test_version3(self): self.log.info('Check wallet does not create transactions with version=3 yet') wallet_rpc = self.nodes[0].get_wallet_rpc(self.default_wallet_name) From 719324977a41ab5132699d9c81032f8b2687f599 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Fri, 18 Sep 2026 09:56:34 +0200 Subject: [PATCH 2/2] test: Check that ineligible output groups are discarded only once --- src/wallet/spend.h | 9 +++++++++ src/wallet/test/group_outputs_tests.cpp | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/src/wallet/spend.h b/src/wallet/spend.h index e64e73179cad..7df12ed299a0 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -120,6 +120,15 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, const CoinSelectionParams& coin_sel_params, const std::vector& filters); +/** + * Group coins by the provided filters, groups that pass no filter are appended to `ret_discarded_groups`. + */ +FilteredOutputGroups GroupOutputs(const CWallet& wallet, + const CoinsResult& coins, + const CoinSelectionParams& coin_sel_params, + const std::vector& filters, + std::vector& ret_discarded_groups); + /** * Attempt to find a valid input set that preserves privacy by not mixing OutputTypes. * `ChooseSelectionResult()` will be called on each OutputType individually and the best diff --git a/src/wallet/test/group_outputs_tests.cpp b/src/wallet/test/group_outputs_tests.cpp index 9b4d9bc8c81c..c1fe72bd74af 100644 --- a/src/wallet/test/group_outputs_tests.cpp +++ b/src/wallet/test/group_outputs_tests.cpp @@ -202,6 +202,11 @@ BOOST_AUTO_TEST_CASE(outputs_grouping_tests) /*expected_without_partial_spends_size=*/ 3, /*positive_only=*/ false); + // The two ineligible UTXOs must be discarded exactly once each + std::vector discarded_groups; + GroupOutputs(*wallet, group_verifier.coins_pool, makeSelectionParams(group_verifier.rand, /*avoid_partial_spends=*/true), {{BASIC_FILTER}}, discarded_groups); + BOOST_CHECK_EQUAL(discarded_groups.size(), 2U); + // ########################################################################################### // 7) Surpass the OUTPUT_GROUP_MAX_ENTRIES and verify that a second partial group gets created // ###########################################################################################