[pull] master from bitcoin:master

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);
             }
         }
     };

src/wallet/spend.h

@@ -120,6 +120,15 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
                           const CoinSelectionParams& coin_sel_params,
                           const std::vector<SelectionFilter>& 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<SelectionFilter>& filters,
+                                  std::vector<OutputGroup>& 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

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<OutputGroup> 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
     // ###########################################################################################

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)