Consensus: Policy: Move CFeeRate out of consensus module and create CPolicy interface #7820

1. 
   jtimon commented at 7:41 pm on April 5, 2016: contributor

   This moves the CFeeRate and dust code out of libconsensus and the consensus module. Before that, it prepares an empty CPolicy interface and implementation where the dust methods are moved to. It also encapsulates the global minRelayTxFee behind the CPolicy interface. Since this may be too disruptive for one PR, I was very careful to separate things in smaller commits. The first commit, for example, may be useful for @jonasschnelli ’s work encapsulating the wallet.

   This is also intended to replace @morcos’ #7557, #6068 and #5114.

2. 
   dcousens commented at 0:42 am on April 6, 2016: contributor

   concept ACK
3. laanwj added the label Refactoring on Apr 6, 2016
4. jtimon force-pushed on Apr 14, 2016
5. 
   jtimon commented at 3:58 pm on April 14, 2016: contributor

   Rebased(1). Also, fixed the error that was making python ./qa/pull-tester/rpc-tests.py mempool_limit fail.
6. jtimon force-pushed on Aug 30, 2016
7. jtimon force-pushed on Oct 14, 2016
8. jtimon force-pushed on Oct 14, 2016
9. jtimon force-pushed on Oct 14, 2016
10. 
    jtimon commented at 6:07 pm on October 14, 2016: contributor

    Rebased and fixed one error, but it is still failing. Still not sure what to do with AmountErrMsg and ParseAmountFromArgs. For the latter to use the former, both need to be put together in either util.o or ui_interface.o. I pushed an alternative simpler option without creating the policy interface nor removing the global minRelayTxFee in https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-dust-out-minimal.
11. 
    NicolasDorier commented at 2:06 am on October 15, 2016: contributor

    Concept ACK will review, can you check the failing travis ?
12. 
    jtimon commented at 3:08 pm on October 15, 2016: contributor

    I’m sorry, after sewgit’s rebase this did never passed travis (but this time I removed the known past mistakes, so it’s weird). That;s why I suggested https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-dust-out-minimal which seems to work (locally). At the same I promised @btcdrak I wouldn’t open parallel mutually exclusive PRs to see which one fails on travis or for people to compare and choose…

    The travis error seemed related to CAmount becoming undefined for windows, but on the last push it also failed in another build. I will keep investigating, but travis is what I meant by “it is still failing”.

13. 
    jtimon commented at 0:12 am on November 4, 2016: contributor

    EDIT: needed rebased and rebased https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-dust-out-minimal ping @btcdrak since the two branches are kind of competing and he advised me against doing that with 2 open PRs because people tend to get confused.
14. jtimon force-pushed on Nov 4, 2016
15. jtimon force-pushed on Nov 23, 2016
16. jtimon force-pushed on Nov 23, 2016
17. Consensus: Minimal way to move dust out of consensus b49e3876d1
18. jtimon force-pushed on Dec 2, 2016
19. Consensus: Policy: MOVEONLY: Move CFeeRate out of the consensus module

    ...from amount.o to policy/feerate.o
    
    Policy, because it moves policy code to the policy directory (common module)

    25c6b82b08
20. Globals: Allow to parse arguments without implicitly using the argMap global 7e2e6fe582
21. Policy: Introduce empty CPolicy interface and CDefaultPolicy implementation 04f546d64a
22. Globals: Policy: mempool and filterRounder are initialized before minRelayTxFee

    Therefore the default value before init::AppInit2() is being used always.

    7cf8970e14
23. Policy: Encapsulate ::minRelayTxFee initialization behind CDefaultPolicy::InitFromArgs() 9185a86c2a
24. ERROR-unittest: Consensus: Policy: MOVEONLY-ish: Rename dust methods from CTxOut to CPolicy

    Consensus, because it moves non-consensus code out of the consensus module
    Policy, because it moves policy code to the policy directory (server module)
    
    Method Renames:
    - s/CTxOut::GetDustThreshold(const CFeeRate&)/CDefaultPolicy::GetDustThreshold(const CTxOut&)/
    - s/CTxOut::IsDust(const CFeeRate&)/CDefaultPolicy::AcceptDust(const CTxOut&)/

    93f8f016e2
25. ERROR-unit-test_IsStandard: Policy: Introduce CPolicy::GetMinRelayFee() and use it everywhere except for dust a56cbadab1
26. Globals: Cleanly remove ::minRelayTxFee daabe730db
27. jtimon force-pushed on Dec 2, 2016
28. 
    jtimon commented at 5:45 am on December 5, 2016: contributor

    Apart from being failing tests and needing rebase, this is going to conflict with #9243. Replaced with the simpler branch I was suggesting that only moves CFeeRate without creating any policy class or interface: #9279
29. jtimon closed this on Dec 5, 2016

    ---

30. DrahtBot locked this on Sep 8, 2021

Contributors
jtimon dcousens NicolasDorier

Labels
Refactoring