Add multiplication operator to CFeeRate #29037

1. 
   murchandamus commented at 6:57 pm on December 8, 2023: contributor

   Allows us to use coin_selection_params.m_long_term_feerate * 3 or 3 * coin_selection_params.m_long_term_feerate instead of
   CFeeRate{coin_selection_params.m_long_term_feerate.GetFee(3000)}

   inspired by #27877 (review)

2. 
   DrahtBot commented at 6:57 pm on December 8, 2023: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	ajtowns, kevkevinpal, ismaelsadeeq, achow101
   Stale ACK	kashifs

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   No conflicts as of last run.

3. murchandamus marked this as ready for review on Dec 8, 2023
4. Add multiplication operator to CFeeRate 1553c80786
5. in src/policy/feerate.h:75 in 7dacc17e9c outdated

   70@@ -71,6 +71,8 @@ class CFeeRate
   71     friend bool operator!=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK != b.nSatoshisPerK; }
   72     CFeeRate& operator+=(const CFeeRate& a) { nSatoshisPerK += a.nSatoshisPerK; return *this; }
   73     std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KVB) const;
   74+    friend CFeeRate operator*(const CFeeRate& f, int a) { return CFeeRate(a * f.nSatoshisPerK); }
   75+    friend CFeeRate operator*(int a, const CFeeRate& f) { return f * a; }
   

   ---

   ---

   

   kevkevinpal commented at 2:19 am on December 9, 2023:

   nit: To me it confused me for a moment cause I didn’t realize that f was using the operator defined on the line above, I think it would make more sense for a reader to just see the same thing above defined backwards like so

   friend CFeeRate operator*(int a, const CFeeRate& f) { return CFeeRate(f.nSatoshisPerK * a); }

   feel free to ignore this suggestion though

   ---

   

   murchandamus commented at 2:34 pm on December 9, 2023:

   Sure, I updated it to use the same formula in both lines, so that only the parameters appear in either order
6. murchandamus force-pushed on Dec 9, 2023
7. 
   kevkevinpal commented at 1:57 am on December 10, 2023: contributor

   ACK 1553c80
8. 
   kashifs commented at 2:52 pm on December 10, 2023: contributor

   tACK 1553c80

   I compiled from source, ran the test suite, and added some additional BOOST_CHECK tests on my local machine that all passed

9. 
   murchandamus commented at 2:24 pm on December 11, 2023: contributor

   Thanks both of you! @kashifs: If you open a PR against my PR or send me a patch by email, I’d be happy to add your commit with the additional tests to the PR.
10. test: Add tests for CFeeRate multiplication operator 1757452cc5
11. 
    kashifs commented at 10:05 pm on December 11, 2023: contributor

    Hi @murchandamus, I believe that I correctly opened a PR against this PR. Please let me know if this is the correct etiquette or if there is anything more that I should do.
12. murchandamus force-pushed on Dec 14, 2023
13. murchandamus force-pushed on Dec 14, 2023
14. 
    murchandamus commented at 9:15 pm on December 14, 2023: contributor

    Thanks @kashifs, I’ve merged your pull request into my pull request and pushed it.
15. 
    kashifs commented at 11:01 pm on December 14, 2023: contributor

    Okay, great! I’m slowly working my way through #27877
16. murchandamus requested review from kevkevinpal on Dec 15, 2023
17. in src/policy/feerate.h:75 in 1757452cc5

    70@@ -71,6 +71,8 @@ class CFeeRate
    71     friend bool operator!=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK != b.nSatoshisPerK; }
    72     CFeeRate& operator+=(const CFeeRate& a) { nSatoshisPerK += a.nSatoshisPerK; return *this; }
    73     std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KVB) const;
    74+    friend CFeeRate operator*(const CFeeRate& f, int a) { return CFeeRate(a * f.nSatoshisPerK); }
    75+    friend CFeeRate operator*(int a, const CFeeRate& f) { return CFeeRate(a * f.nSatoshisPerK); }
    

    ---

    ---

    

    ajtowns commented at 4:35 pm on December 18, 2023:

    Seems strange to have operator+= and operator* but no operator*= or operator+.
18. 
    ajtowns commented at 4:35 pm on December 18, 2023: contributor

    ACK 1757452cc55a6dacc62e4258043ee4d711fd281a ; lgtm
19. DrahtBot removed review request from kevkevinpal on Dec 18, 2023
20. DrahtBot requested review from kevkevinpal on Dec 18, 2023
21. DrahtBot removed review request from kevkevinpal on Dec 18, 2023
22. DrahtBot requested review from kevkevinpal on Dec 18, 2023
23. 
    kevkevinpal commented at 5:34 pm on December 18, 2023: contributor

    reACK 1757452
24. DrahtBot removed review request from kevkevinpal on Dec 18, 2023
25. ismaelsadeeq approved
26. 
    ismaelsadeeq commented at 4:48 pm on December 19, 2023: member

    ACK 1757452cc55a6dacc62e4258043ee4d711fd281a
27. in src/test/amount_tests.cpp:87 in 1757452cc5

    86@@ -87,10 +87,30 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
    87     CFeeRate(MAX_MONEY, std::numeric_limits<uint32_t>::max()).GetFeePerK();
    

    ---

    ---

    

    ismaelsadeeq commented at 4:49 pm on December 19, 2023:

    nit: will be nice to squashed this 1757452cc55a6dacc62e4258043ee4d711fd281a to the first commit
28. 
    achow101 commented at 0:24 am on December 20, 2023: member

    ACK 1757452cc55a6dacc62e4258043ee4d711fd281a
29. achow101 merged this on Dec 20, 2023
30. achow101 closed this on Dec 20, 2023

    ---

Contributors
murchandamus DrahtBot kevkevinpal kashifs ajtowns ismaelsadeeq achow101