coinselection: Tiebreak SRD eviction by weight #33223

1. 
   murchandamus commented at 10:51 pm on August 19, 2025: contributor

   @yancyribbens pointed out that SRD would fail to find a possible solution if there are multiple UTXOs of the same effective value with diverse weight.

   This adds a tiebreaker that will make SRD succeed in such a scenario.

2. coinselection: Tiebreak SRD eviction by weight 0dee81b985
3. 
   DrahtBot commented at 10:51 pm on August 19, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33223.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   LLM Linter (✨ experimental)

   Possible typos and grammar issues:

   • have same effective value -> have the same effective value [missing definite article “the” makes the phrase less clear]

   drahtbot_id_4_m

4. in src/wallet/test/coinselector_tests.cpp:1245 in 0dee81b985

   1239@@ -1240,6 +1240,26 @@ BOOST_AUTO_TEST_CASE(srd_tests)
   1240         BOOST_CHECK(res);
   1241         BOOST_CHECK(res->GetWeight() <= max_selection_weight);
   1242     }
   1243+
   1244+    {
   1245+        // ################################################################################################################
   

   ---

   ---

   

   murchandamus commented at 10:51 pm on August 19, 2025:

   Move to coinselection_tests.cpp
5. 
   cedwies commented at 5:11 pm on August 21, 2025: none

   I ran the SRD tests, all pass as expected. To validate the comparator change, I also re-ran the fourth SRD test using the old comparator, and it fails (as expected). This confirms that the test setup is effective at catching the bug. The new comparator correctly enforces the weight limit, and the test demonstrates that the previous implementation of the comparator can only succeed by chance (~1 in a billion). Looks good to me.
6. in src/wallet/coinselection.cpp:531 in 0dee81b985

   528@@ -529,6 +529,9 @@ class MinOutputGroupComparator
   529 public:
   530     int operator() (const OutputGroup& group1, const OutputGroup& group2) const
   531     {
   

   ---

   ---

   

   yancyribbens commented at 6:09 pm on August 22, 2025:

   Since the goal here is to use the same sort as descending_effval_weight, L532 - L 535 can be replaced with this single line:

   return descending_effval_weight(group1, group2);

7. 
   yancyribbens commented at 7:33 pm on August 22, 2025: contributor

   Any reason this is in Draft?
8. 
   murchandamus commented at 9:35 pm on August 27, 2025: contributor

   I want to use the opportunity to move the SRD tests to the coinselection_tests, but haven’t gotten around to it yet.

Contributors
murchandamus DrahtBot cedwies yancyribbens