Consensus: Rename: Move consensus code to the consensus directory #8328

1. 
   jtimon commented at 1:11 pm on July 11, 2016: contributor

   It seems to me that version.h shouldn’t be used by libconsensus, so I left that out. amount.cpp is not used by libconsensus (see #7820 ), also left out of the rename for now.

   Although pow.o and versionbits.o still depend on chain.o (dependent on storage) and can’t be moved to the consensus package, they can be moved to the directory already, so they are moved.

   Although @laanwj believes that the best time to do this kind of refactoring would be right after forking 0.13, I believe that doing it right before has negligible risks and would simplify future backports of consensus code to the 0.13 branch.

2. Consensus: Rename: Move consensus code to the consensus directory 1e1db2f26c
3. 
   dcousens commented at 3:03 pm on July 11, 2016: contributor

   utACK 1e1db2f
4. jonasschnelli added the label Refactoring on Jul 12, 2016
5. jtimon referenced this in commit a2c0badcf9 on Jul 13, 2016
6. Consensus: Rename: consensus/utilmoneystr.cpp belongs in libconsensus because

   FormatMoney()
   
   - My opinion: there's only satoshis (aka units) in libconsensus

   c078fd157a
7. 
   jtimon commented at 12:42 pm on July 13, 2016: contributor

   Updated adding utilmoneystr, which I forgot will be needed in libconsensus for this line: https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L1961 (although if many people agree with me, I would prefer to keep it out and just print satoshis).
8. jtimon closed this on Jul 13, 2016

   ---

9. jtimon deleted the branch on Jul 13, 2016
10. jtimon restored the branch on Jul 13, 2016
11. jtimon reopened this on Jul 13, 2016

    ---

12. 
    afk11 commented at 4:42 pm on July 14, 2016: contributor

    ACK c078fd1
13. 
    maaku commented at 5:02 pm on July 14, 2016: contributor

    I really don’t think that we should be moving everything that libconsensus is code-dependent on into the consensus directory. That does not help clarity of the code base. Things like serialization and basic data structures in primitives probably deserve to be their own library, which libconsensus links against. Moving them all into the consensus directory doesn’t help IMHO.
14. 
    jtimon commented at 8:54 pm on July 14, 2016: contributor

    I’m all ears for suggestions on other internal packages for the consensus one to depend on besides crypto and libsecp256k1. I think this approach does help the clarity of the codebase by explicitly putting together all the files included (or that are planned to be included) in the same building package (consensus). For example, when I ask “please don’t include this header from a consensus file”, it would simplify things (not everybody looks at src/Makefile.am). Also if we plan for libconsensus to become its own repository eventually, I see this as the most straightforward path. Some people on IRC have expressed that maybe there could be some intermediary libraries that both libconsensus and bitcoin core depend on. So it seems there’s mixing feelings about this and in any case this can be done at any time (in fact, it was phase 4 in my newer published plan). Therefore I’m closing the PR.

    Regarding the separation of utilmoneystr in a different commit, it was because I believe it shouldn’t be part of libconsensus. At least @sipa seems to agree with me there, so at some point before exposing verifyTx in libconsensus I will create a PR removing the one line dependency and ping you both.

15. jtimon closed this on Jul 14, 2016

    ---

16. DrahtBot locked this on Sep 8, 2021

Contributors
jtimon dcousens afk11 maaku

Labels
Refactoring