refactor: Move mempool RPCs to rpc/mempool #24656

1. 
   MarcoFalke commented at 7:59 am on March 24, 2022: member

   This moves the remaining mempool RPCs to rpc/mempool. Previously all mempool RPCs from the blockchain category have been moved. This patch moves the ones from the rawtransactions category.

   In the future, as a follow-up to this refactoring patch, it could be considered whether a new mempool category should be introduced.

   Beside a clearer code organization, this pull request should also reduce the compile time and space of the rawtransactions.cpp file.

2. style: Add static keyword where possible in rpc/mempool fa0f666dd7
3. Move mempool RPCs to rpc/mempool

   Can be reviewed with
   --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

   fac5a51c47
4. MarcoFalke added the label Refactoring on Mar 24, 2022
5. MarcoFalke added the label RPC/REST/ZMQ on Mar 24, 2022
6. MarcoFalke added the label Mempool on Mar 24, 2022
7. MarcoFalke force-pushed on Mar 24, 2022
8. 
   promag commented at 8:32 am on March 24, 2022: member

   Code review ACK fac5a51c47fba21678fb35805e40d00fe7c891a0.
9. 
   DrahtBot commented at 9:34 am on March 24, 2022: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #24661 (refactor: Use clang-tidy syntax for C++ named arguments by fanquake)
   • #24513 (CChainState -> Chainstate by jamesob)
   • #21422 (Add feerate histogram to getmempoolinfo by kiminuo)

   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.

10. in src/rpc/mempool.cpp:678 in fac5a51c47

    673@@ -463,6 +674,8 @@ void RegisterMempoolRPCCommands(CRPCTable& t)
    674     static const CRPCCommand commands[]{
    675         // category     actor (function)
    676         // --------     ----------------
    677+        {"rawtransactions", &sendrawtransaction},
    678+        {"rawtransactions", &testmempoolaccept},
    

    ---

    ---

    

    shaavan commented at 12:14 pm on March 24, 2022:

    nit:

    I would suggest putting these two lines after the rest of the blockchain category functions to match the proper lexicographical ordering.

    ---

    

    MarcoFalke commented at 1:38 pm on March 24, 2022:

    I am expecting that everything will move to a new mempool category later, so I am leaving as-is for now.

    ---

    

    shaavan commented at 2:35 pm on March 27, 2022:

    I think that’s acceptable reasoning. Other than this, I can’t find any blockers for this PR, and hence I think it can be merged.

    ACK fac5a51c47fba21678fb35805e40d00fe7c891a0

11. 
    shaavan commented at 12:14 pm on March 24, 2022: contributor

    Concept ACK

    • The static keyword is used appropriately in rpc/mempool.cpp file. I checked for any other instances where a static keyword could be used and found none.
    • The moved code looks good to me. I was able to verify the move-only changes in the second commit successfully.
    • I was able to compile the PR successfully on Ubuntu 20.04
12. MarcoFalke merged this on Mar 28, 2022
13. MarcoFalke closed this on Mar 28, 2022

    ---

14. MarcoFalke deleted the branch on Mar 28, 2022
15. DrahtBot locked this on Mar 28, 2023

Contributors
MarcoFalke promag DrahtBot shaavan

Labels
Refactoring RPC/REST/ZMQ Mempool