Merge 381593c906 into merged_master (Bitcoin PR bitcoin/bitcoin#24845)

src/wallet/spend.cpp

@@ -444,7 +444,8 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
 FilteredOutputGroups GroupOutputs(const CWallet& wallet,
                           const CoinsResult& coins,
                           const CoinSelectionParams& coin_sel_params,
-                          const std::vector<SelectionFilter>& filters)
+                          const std::vector<SelectionFilter>& filters,
+                          std::vector<OutputGroup>& ret_discarded_groups)
 {
     FilteredOutputGroups filtered_groups;
 
@@ -461,11 +462,14 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
                 group.Insert(std::make_shared<COutput>(output), ancestors, descendants);
 
                 // Each filter maps to a different set of groups
+                bool accepted = false;
                 for (const auto& sel_filter : filters) {
                     const auto& filter = sel_filter.filter;
                     if (!group.EligibleForSpending(filter)) continue;
                     filtered_groups[filter].Push(group, type, /*insert_positive=*/true, /*insert_mixed=*/true);
+                    accepted = true;
                 }
+                if (!accepted) ret_discarded_groups.emplace_back(group);
             }
         }
         return filtered_groups;
@@ -529,6 +533,7 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
                 const OutputGroup& group = *group_it;
 
                 // Each filter maps to a different set of groups
+                bool accepted = false;
                 for (const auto& sel_filter : filters) {
                     const auto& filter = sel_filter.filter;
                     if (!group.EligibleForSpending(filter)) continue;
@@ -541,7 +546,9 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
                     OutputType type = script.second;
                     // Either insert the group into the positive-only groups or the mixed ones.
                     filtered_groups[filter].Push(group, type, positive_only, /*insert_mixed=*/!positive_only);
+                    accepted = true;
                 }
+                if (!accepted) ret_discarded_groups.emplace_back(group);
             }
         }
     };
@@ -552,6 +559,15 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet,
     return filtered_groups;
 }
 
+FilteredOutputGroups GroupOutputs(const CWallet& wallet,
+                                  const CoinsResult& coins,
+                                  const CoinSelectionParams& params,
+                                  const std::vector<SelectionFilter>& filters)
+{
+    std::vector<OutputGroup> unused;
+    return GroupOutputs(wallet, coins, params, filters, unused);
+}
+
 // Returns true if the result contains an error and the message is not empty
 static bool HasErrorMsg(const util::Result<SelectionResult>& res) { return !util::ErrorString(res).empty(); }
 
@@ -782,7 +798,24 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
         }
 
         // Group outputs and map them by coin eligibility filter
-        FilteredOutputGroups filtered_groups = GroupOutputs(wallet, available_coins, coin_selection_params, ordered_filters);
+        std::vector<OutputGroup> discarded_groups;
+        FilteredOutputGroups filtered_groups = GroupOutputs(wallet, available_coins, coin_selection_params, ordered_filters, discarded_groups);
+
+        // Check if we still have enough balance after applying filters (some coins might be discarded)
+        CAmount total_discarded = 0;
+        CAmount total_unconf_long_chain = 0;
+        for (const auto& group : discarded_groups) {
+            total_discarded += group.GetSelectionAmount();
+            if (group.m_ancestors >= max_ancestors || group.m_descendants >= max_descendants) total_unconf_long_chain += group.GetSelectionAmount();
+        }
+
+        if (CAmount total_amount = available_coins.GetTotalAmount() - CAmountMap{{::policyAsset, total_discarded}} < value_to_select) { // ELEMENTS FIXME: check this
+            // Special case, too-long-mempool cluster.
+            if (CAmountMap{{::policyAsset, total_amount + total_unconf_long_chain}} > value_to_select) {
+                return util::Result<SelectionResult>({_("Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool")});
+            }
+            return util::Result<SelectionResult>(util::Error()); // General "Insufficient Funds"
+        }
 
         // Walk-through the filters until the solution gets found.
         // If no solution is found, return the first detailed error (if any).
@@ -801,8 +834,13 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
                 if (HasErrorMsg(res)) res_detailed_errors.emplace_back(res);
             }
         }
-        // Coin Selection failed.
-        return res_detailed_errors.empty() ? util::Result<SelectionResult>(util::Error()) : res_detailed_errors.front();
+
+        // Return right away if we have a detailed error
+        if (!res_detailed_errors.empty()) return res_detailed_errors.front();
+
+
+        // General "Insufficient Funds"
+        return util::Result<SelectionResult>(util::Error());
     }();
 
     return res;

test/functional/wallet_create_tx.py

@@ -32,6 +32,7 @@ class CreateTxWalletTest(BitcoinTestFramework):
 
         self.test_anti_fee_sniping()
         self.test_tx_size_too_large()
+        self.test_create_too_long_mempool_chain()
 
     def test_anti_fee_sniping(self):
         self.log.info('Check that we have some (old) blocks and that anti-fee-sniping is disabled')
@@ -83,6 +84,30 @@ class CreateTxWalletTest(BitcoinTestFramework):
         )
         self.nodes[0].settxfee(0)
 
+    def test_create_too_long_mempool_chain(self):
+        self.log.info('Check too-long mempool chain error')
+        df_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        self.nodes[0].createwallet("too_long")
+        test_wallet = self.nodes[0].get_wallet_rpc("too_long")
+
+        tx_data = df_wallet.send(outputs=[{test_wallet.getnewaddress(): 25}], options={"change_position": 0})
+        txid = tx_data['txid']
+        vout = 1
+
+        options = {"change_position": 0, "add_inputs": False}
+        for i in range(1, 25):
+            options['inputs'] = [{'txid': txid, 'vout': vout}]
+            tx_data = test_wallet.send(outputs=[{test_wallet.getnewaddress(): 25 - i}], options=options)
+            txid = tx_data['txid']
+
+        # Sending one more chained transaction will fail
+        options = {"minconf": 0, "include_unsafe": True, 'add_inputs': True}
+        assert_raises_rpc_error(-4, "Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool",
+                                test_wallet.send, outputs=[{test_wallet.getnewaddress(): 0.3}], options=options)
+
+        test_wallet.unloadwallet()
+
 
 if __name__ == '__main__':
     CreateTxWalletTest().main()