doc: Clarify that change_cost cannot be negative in GetSelectionWaste #22892

1. 
   benthecarman commented at 3:13 AM on September 5, 2021: contributor

   We assert that the change_cost must be positive so we should document it in the function's docs

   https://github.com/bitcoin/bitcoin/blob/4f5ad43b1e05cd7b403f87aae4c4d42e5aea810b/src/wallet/coinselection.cpp#L361

2. fanquake added the label Docs on Sep 5, 2021
3. in src/wallet/coinselection.h:177 in 2af5a46782 outdated

    173 | @@ -174,7 +174,7 @@ struct OutputGroup
    174 |   * change_cost = effective_feerate * change_output_size + long_term_feerate * change_spend_size
    175 |   *
    176 |   * @param[in] inputs The selected inputs
    177 | - * @param[in] change_cost The cost of creating change and spending it in the future. Only used if there is change. Must be 0 if there is no change.
    178 | + * @param[in] change_cost The cost of creating change and spending it in the future. Must be positive if there is change. Must be 0 if there is no change.
   

   ---

   ---

   

   jonatack commented at 11:22 AM on September 5, 2021:

   maybe keep both the "Only used if" and "must be positive" info

   - * [@param](/bitcoin-bitcoin/contributor/param/)[in] change_cost The cost of creating change and spending it in the future. Only used if there is change. Must be 0 if there is no change.
   + * [@param](/bitcoin-bitcoin/contributor/param/)[in] change_cost The cost of creating change and spending it in the future.
   + *                        Only used if there is change, in which case it must be positive.
   + *                        Must be 0 if there is no change.
   

   ---

   

   benthecarman commented at 5:56 PM on September 5, 2021:

   done

4. 
   jonatack commented at 11:23 AM on September 5, 2021: member

   Concept ACK

5. 
   practicalswift commented at 11:44 AM on September 5, 2021: contributor

   Concept ACK: preconditions should be documented

6. doc: Clarify that change_cost cannot be negative in GetSelectionWaste d2eccacd18
7. benthecarman force-pushed on Sep 5, 2021
8. 
   jonatack commented at 6:11 PM on September 5, 2021: member

   ACK d2eccacd188959c5aed661d0d5b39c119e7c7a32

9. fanquake requested review from achow101 on Sep 10, 2021
10. fanquake requested review from meshcollider on Sep 10, 2021
11. 
    jarolrod commented at 7:02 AM on September 13, 2021: member

    ACK d2eccacd188959c5aed661d0d5b39c119e7c7a32

    This should be documented properly. 🥃

12. shaavan approved
13. 
    shaavan commented at 1:51 PM on September 16, 2021: contributor

    ACK d2eccacd188959c5aed661d0d5b39c119e7c7a32

    It makes sense to document the condition that is later asserted in the code.

14. laanwj merged this on Sep 27, 2021
15. laanwj closed this on Sep 27, 2021

    ---

16. bitcoin locked this on Oct 30, 2022
17. benthecarman deleted the branch on Oct 10, 2025

Contributors

benthecarmanjonatackpracticalswiftjarolrodshaavan

Review Requested
meshcolliderachow101