refactor: Add and use EnsureConnman in rpc code #21719

1. 
   MarcoFalke commented at 6:22 pm on April 17, 2021: member

   This removes the 10 occurrences of throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); and replaces them with EnsureConnman.
2. refactor: Mark member functions const faabeb854a
3. DrahtBot added the label Build system on Apr 17, 2021
4. DrahtBot added the label Mining on Apr 17, 2021
5. DrahtBot added the label P2P on Apr 17, 2021
6. DrahtBot added the label Refactoring on Apr 17, 2021
7. DrahtBot added the label RPC/REST/ZMQ on Apr 17, 2021
8. MarcoFalke force-pushed on Apr 17, 2021
9. MarcoFalke removed the label Build system on Apr 17, 2021
10. MarcoFalke removed the label Mining on Apr 17, 2021
11. MarcoFalke removed the label P2P on Apr 17, 2021
12. 
    DrahtBot commented at 8:17 pm on April 17, 2021: 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:

    • #20295 (rpc: getblockfrompeer by Sjors)

    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. 
    practicalswift commented at 11:01 pm on April 17, 2021: contributor

    Concept ACK: neat cleanups!
14. 
    jarolrod commented at 0:34 am on April 18, 2021: member

    ACK fad37e6c380bf96fa430217d5e2f43322d4718f9

    This is a nice simplification. Tested the error message with the patch below. Used a similar patch with EnsurePeerman. Tested that, with this refactor, functionality remains the same by applying a similar patch to the occurrences of if(!node.connman).

     0--- a/src/rpc/net.cpp
     1+++ b/src/rpc/net.cpp
     2@@ -41,7 +41,7 @@ const std::vector<std::string> CONNECTION_TYPE_DOC{
     3 
     4 CConnman& EnsureConnman(const NodeContext& node)
     5 {
     6-    if (!node.connman) {
     7+    if (true) {
     8         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
     9     }
    10     return *node.connman;
    

15. 
    theStack commented at 12:09 pm on April 18, 2021: member

    Concept ACK

    There is one other instance in the ping RPC that can be tackled with EnsurePeerman:

     0diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
     1index 4d192ad95..fff7d3b60 100644
     2--- a/src/rpc/net.cpp
     3+++ b/src/rpc/net.cpp
     4@@ -92,12 +92,10 @@ static RPCHelpMan ping()
     5         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
     6 {
     7     NodeContext& node = EnsureAnyNodeContext(request.context);
     8-    if (!node.peerman) {
     9-        throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
    10-    }
    11+    PeerManager& peerman = EnsurePeerman(node);
    12
    13     // Request that each node send a ping during next message processing pass
    14-    node.peerman->SendPings();
    15+    peerman.SendPings();
    16     return NullUniValue;
    17 },
    18     };
    

    Also, including <any> isn’t needed in the new header src/rpc/net.h.

16. refactor: Add and use EnsureConnman in rpc code fafb68add5
17. MarcoFalke force-pushed on Apr 19, 2021
18. 
    MarcoFalke commented at 11:05 am on April 19, 2021: member

    Thanks, done
19. theStack approved
20. 
    theStack commented at 12:19 pm on April 19, 2021: member

    ACK fafb68add5e16e8bd5b9428bcffcaee2639747cf
21. 
    jarolrod commented at 3:09 am on April 20, 2021: member

    re-ACK fafb68add5e16e8bd5b9428bcffcaee2639747cf
22. ryanofsky approved
23. 
    ryanofsky commented at 10:51 pm on April 20, 2021: member

    Code review ACK fafb68add5e16e8bd5b9428bcffcaee2639747cf
24. MarcoFalke merged this on Apr 21, 2021
25. MarcoFalke closed this on Apr 21, 2021

    ---

26. MarcoFalke deleted the branch on Apr 21, 2021
27. sidhujag referenced this in commit 52a8f50ff8 on Apr 21, 2021
28. domob1812 referenced this in commit 0dbdd02f49 on Apr 28, 2021
29. domob1812 referenced this in commit c1c0ef1305 on Apr 28, 2021
30. gwillen referenced this in commit dd87547295 on Jun 1, 2022
31. DrahtBot locked this on Aug 18, 2022

Contributors
MarcoFalke DrahtBot practicalswift jarolrod theStack ryanofsky

Labels
Refactoring RPC/REST/ZMQ