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. DrahtBot cross-referenced this on Jul 8, 2022 from issue [kernel 3c/n] Decouple validation cache initialization from `ArgsManager` by dongcarl
15. DrahtBot cross-referenced this on Jul 8, 2022 from issue assumeutxo: add init and completion logic by jamesob
16. DrahtBot cross-referenced this on Jul 8, 2022 from issue p2p: Erlay support signaling by naumenkogs
17. DrahtBot cross-referenced this on Jul 13, 2022 from issue [kernel 3b/n] Decouple `{Dump,Load}Mempool` from `ArgsManager` by dongcarl
18. fanquake requested review from instagibbs on Jul 13, 2022
19. DrahtBot cross-referenced this on Jul 14, 2022 from issue refactor: Introduce EvictionManager and use it for the inbound eviction logic by dergoegge
20. DrahtBot cross-referenced this on Jul 14, 2022 from issue refactor: Introduce EvictionManager by dergoegge
21. DrahtBot cross-referenced this on Jul 17, 2022 from issue fuzz: Fix assert bug in txorphan target by chinggg
22. chinggg approved
23. 
    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
24. DrahtBot added the label Needs rebase on Jul 18, 2022
25. MarcoFalke force-pushed on Jul 18, 2022
26. 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
27. Move DEFAULT_MAX_ORPHAN_TRANSACTIONS to node/txorphanage.h

    Also, sort file list after rename

    fa999c314a
28. MarcoFalke force-pushed on Jul 19, 2022
29. fanquake requested review from ryanofsky on Jul 19, 2022
30. DrahtBot removed the label Needs rebase on Jul 19, 2022
31. 
    MarcoFalke commented at 10:27 AM on July 19, 2022: member

    Rebased

32. shaavan approved
33. 
    shaavan commented at 12:44 PM on July 19, 2022: contributor

    reACK fa999c314a916d842e50a0d58d12a260f5732dbf

    Changes since my last review:

    • Rebased over master.
34. DrahtBot cross-referenced this on Jul 20, 2022 from issue refactor: Nuke policy/fees->mempool circular dependencies by hebasto
35. DrahtBot cross-referenced this on Jul 21, 2022 from issue assumeutxo: snapshot initialization by jamesob
36. 
    MarcoFalke commented at 3:08 PM on July 26, 2022: member

    Closing for now to open review for other pulls.

37. MarcoFalke closed this on Jul 26, 2022

    ---

38. MarcoFalke deleted the branch on Jul 26, 2022
39. bitcoin locked this on Jul 26, 2023

Contributors

MarcoFalkejonatackshaavanDrahtBotglozowchinggg

Review Requested
instagibbsryanofsky