[wallet] Refactor to use new MIN_CHANGE #6669

1. 
   MarcoFalke commented at 9:35 pm on September 13, 2015: member

   Introduce a new constant MIN_CHANGE and use it in appropriate places.
2. MarcoFalke force-pushed on Sep 18, 2015
3. MarcoFalke force-pushed on Sep 18, 2015
4. 
   MarcoFalke commented at 2:55 pm on September 18, 2015: member

   I will add some more tests later.
5. 
   MarcoFalke commented at 2:39 pm on September 20, 2015: member

   ping @dooglus: As you are one of the authors of the coin selection code and tests, would you mind giving a concept ACK or concept NACK on this pull?
6. 
   dooglus commented at 4:58 pm on September 20, 2015: contributor

   Looks good to me. It makes sense to have the minimum change not be hardcoded to 0.01.
7. laanwj added the label Wallet on Sep 23, 2015
8. 
   btcdrak commented at 7:27 am on September 30, 2015: contributor

   utACK
9. in src/wallet/wallet.h: in 1027fae01d outdated

   40@@ -41,8 +41,12 @@ extern bool fPayAtLeastCustomFee;
   41 static const CAmount DEFAULT_TRANSACTION_FEE = 0;
   42 //! -paytxfee will warn if called with a higher fee than this amount (in satoshis) per KB
   43 static const CAmount nHighTransactionFeeWarning = 0.01 * COIN;
   44+//! -mintxfee default
   45+static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000;
   46 //! -maxtxfee default
   47 static const CAmount DEFAULT_TRANSACTION_MAXFEE = 0.1 * COIN;
   48+//! minimum change amount
   49+static const CAmount nMinChange = CENT;
   

   ---

   ---

   

   sipa commented at 11:59 am on September 30, 2015:

   If it’s a constant, please use an all-upper-case name to make that clear, like MINIMUM_CHANGE.

   If you plan to turn it into a runtime variable later, it’s probably better to make it a runtime variable immediately now, and initialize it with a constant, for example DEFAULT_MINIMUM_CHANGE.

   ---

   

   MarcoFalke commented at 11:54 am on October 9, 2015:

   No plans yet to turn it into a runtime variable.
10. MarcoFalke force-pushed on Oct 9, 2015
11. 
    MarcoFalke commented at 11:54 am on October 9, 2015: member

    Addressed nit by @sipa in follow up commit c3c98ffcb6c27fb836754d54954d2f273928c164.
12. MarcoFalke force-pushed on Oct 24, 2015
13. MarcoFalke renamed this:
    [wallet] Refactor to use new nMinChange
    [wallet] Refactor to use new MIN_CHANGE
    on Oct 24, 2015
14. 
    wtogami commented at 2:11 am on October 27, 2015: contributor

    Add mention of the addition of DEFAULT_TRANSACTION_MINFEE to the commit message?
15. 
    MarcoFalke commented at 6:54 am on October 27, 2015: member

    @wtogami I think you intended to put the comment over at #6696?
16. 
    MarcoFalke commented at 6:55 am on October 27, 2015: member

    Anything holding this back? Does this need more review or additional test cases?
17. 
    wtogami commented at 9:30 pm on October 27, 2015: contributor

    @MarcoFalke oh? it appeared that the commits in this PR added it.
18. 
    sipa commented at 1:39 am on October 28, 2015: member

    Concept & light code review ACK.
19. [wallet] Refactor to use new MIN_CHANGE

    * Introduce new constant MIN_CHANGE and use it instead of the
    hardcoded "CENT"
    * Add test case for MIN_CHANGE
    * Introduce new constant for -mintxfee default:
      DEFAULT_TRANSACTION_MINFEE = 1000

    6b0e622c25
20. MarcoFalke force-pushed on Oct 28, 2015
21. [wallet] Add comments for doxygen a9c73a130e
22. MarcoFalke force-pushed on Oct 28, 2015
23. 
    MarcoFalke commented at 10:51 am on October 28, 2015: member

    @wtogami Done. (Fixed commit msg)
24. 
    laanwj commented at 12:37 pm on October 29, 2015: member

    Concept ACK. After this we could completely get rid of CENT.
25. in src/wallet/wallet.h: in a9c73a130e outdated

    40@@ -41,8 +41,12 @@ extern bool fPayAtLeastCustomFee;
    41 static const CAmount DEFAULT_TRANSACTION_FEE = 0;
    42 //! -paytxfee will warn if called with a higher fee than this amount (in satoshis) per KB
    43 static const CAmount nHighTransactionFeeWarning = 0.01 * COIN;
    44+//! -mintxfee default
    45+static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000;
    

    ---

    ---

    

    laanwj commented at 12:39 pm on October 29, 2015:

    Should use this new default constant in init.cpp as well (e.g. HelpMessage)

    ---

    

    MarcoFalke commented at 4:40 pm on October 29, 2015:

    @laanwj Done
26. 
    dcousens commented at 1:00 pm on October 29, 2015: contributor

    utACK
27. Init: Use DEFAULT_TRANSACTION_MINFEE in help message 6342a4889b
28. laanwj merged this on Nov 4, 2015
29. laanwj closed this on Nov 4, 2015

    ---

30. laanwj referenced this in commit 8a95a18562 on Nov 4, 2015
31. MarcoFalke deleted the branch on Nov 4, 2015
32. zkbot referenced this in commit 8713d73daf on Dec 18, 2019
33. zkbot referenced this in commit 2da77edbfe on Dec 18, 2019
34. zkbot referenced this in commit 577f7ef72a on Dec 18, 2019
35. DrahtBot locked this on Sep 8, 2021

Contributors
MarcoFalke dooglus btcdrak sipa wtogami laanwj dcousens

Labels
Wallet