Move DEFAULT_MAX_ORPHAN_TRANSACTIONS to node/txorphanage.h #25564

1. 
   MarcoFalke commented at 12:43 PM on July 7, 2022: member

   It is not meaninful to run txorphanage without a node (validation, chainstate, mempool, rpc, ...). The module is in the node library and won't be added to the kernel library. Thus, it should be moved to src/node.

   Also, move DEFAULT_MAX_ORPHAN_TRANSACTIONS to node/txorphanage.h, as it logically belongs to TxOrphanage::m_orphans and not to PeerManager.

2. MarcoFalke added the label Refactoring on Jul 7, 2022
3. in src/node/txorphanage.h:14 in fa17d03a77 outdated

      9 | @@ -10,6 +10,9 @@
     10 |  #include <primitives/transaction.h>
     11 |  #include <sync.h>
     12 |  
     13 | +/** Default for -maxorphantx, maximum number of orphan transactions kept in memory */
     14 | +static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100;
   

   ---

   ---

   

   jonatack commented at 12:52 PM on July 7, 2022:

   Maybe make constexpr while touching this line.

   ---

   

   MarcoFalke commented at 4:30 PM on July 7, 2022:

   Thx, done

4. MarcoFalke force-pushed on Jul 7, 2022
5. DrahtBot added the label Needs rebase on Jul 7, 2022
6. shaavan approved
7. 
   shaavan commented at 6:17 PM on July 7, 2022: contributor

   Code Review ACK dddd207b093a2c44f8f12bcc2ee6244fe5143f0d

   • Verified that the scripted diff script works correctly, and the consequent diff is the same as the first commit.
   • Verified that the files are correctly sorted after the renaming.
   • I agree with converting const -> constexpr, which makes the variable a compile-time constant.
8. MarcoFalke force-pushed on Jul 8, 2022
9. DrahtBot removed the label Needs rebase on Jul 8, 2022
10. shaavan approved
11. 
    shaavan commented at 9:19 AM on July 8, 2022: contributor

    reACK fa6e0734f5c2acc9e9b8de287c0da0f84fe87373

    Changes since my last review:

    • Rebased over current master
12. 
    DrahtBot commented at 9:43 AM on July 8, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #25667 (assumeutxo: snapshot initialization by jamesob)
    • #25527 ([kernel 3c/n] Decouple validation cache initialization from ArgsManager by dongcarl)
    • #17786 (refactor: Nuke policy/fees->mempool circular dependencies by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

13. 
    glozow commented at 9:43 AM on July 8, 2022: member

    Concept ACK, light code review looks correct to me

14. fanquake requested review from instagibbs on Jul 13, 2022
15. chinggg approved
16. 
    chinggg commented at 6:10 AM on July 17, 2022: contributor

    Concept ACK https://github.com/bitcoin/bitcoin/commit/fa6e0734f5c2acc9e9b8de287c0da0f84fe87373

    • Verified txorphan fuzz target works properly
    • I think it can make writing fuzz target easier by modularizing txorphanage
17. DrahtBot added the label Needs rebase on Jul 18, 2022
18. MarcoFalke force-pushed on Jul 18, 2022
19. scripted-diff: Move txorphanage to src/node

    -BEGIN VERIFY SCRIPT-
     # Move module
     git mv src/txorphanage.cpp src/node/
     git mv src/txorphanage.h   src/node/
     # Replacements
     sed -i 's:txorphanage\.h:node/txorphanage.h:g'     $(git grep -l txorphanage)
     sed -i 's:txorphanage\.cpp:node/txorphanage.cpp:g' $(git grep -l txorphanage)
     sed -i 's:TXORPHANAGE_H:NODE_TXORPHANAGE_H:g'      $(git grep -l TXORPHANAGE_H)
    -END VERIFY SCRIPT-

    fa6fca37ec
20. Move DEFAULT_MAX_ORPHAN_TRANSACTIONS to node/txorphanage.h

    Also, sort file list after rename

    fa999c314a
21. MarcoFalke force-pushed on Jul 19, 2022
22. fanquake requested review from ryanofsky on Jul 19, 2022
23. DrahtBot removed the label Needs rebase on Jul 19, 2022
24. 
    MarcoFalke commented at 10:27 AM on July 19, 2022: member

    Rebased

25. shaavan approved
26. 
    shaavan commented at 12:44 PM on July 19, 2022: contributor

    reACK fa999c314a916d842e50a0d58d12a260f5732dbf

    Changes since my last review:

    • Rebased over master.
27. 
    MarcoFalke commented at 3:08 PM on July 26, 2022: member

    Closing for now to open review for other pulls.

28. MarcoFalke closed this on Jul 26, 2022

    ---

29. MarcoFalke deleted the branch on Jul 26, 2022
30. bitcoin locked this on Jul 26, 2023

Contributors

MarcoFalkejonatackshaavanDrahtBotglozowchinggg

Review Requested
instagibbsryanofsky