refactor: Add and use EnsureConnman in rpc code #21719

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2104-netConnman changing 8 files +104 −85
  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: 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:

    • #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 12: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).

    --- a/src/rpc/net.cpp
    +++ b/src/rpc/net.cpp
    @@ -41,7 +41,7 @@ const std::vector<std::string> CONNECTION_TYPE_DOC{
     
     CConnman& EnsureConnman(const NodeContext& node)
     {
    -    if (!node.connman) {
    +    if (true) {
             throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
         }
         return *node.connman;
    
    
  15. DrahtBot cross-referenced this on Apr 18, 2021 from issue rpc: getblockfrompeer by Sjors
  16. theStack commented at 12:09 PM on April 18, 2021: contributor

    Concept ACK

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

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

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

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

    Thanks, done

  20. theStack approved
  21. theStack commented at 12:19 PM on April 19, 2021: contributor

    ACK fafb68add5e16e8bd5b9428bcffcaee2639747cf

  22. jarolrod commented at 3:09 AM on April 20, 2021: member

    re-ACK fafb68add5e16e8bd5b9428bcffcaee2639747cf

  23. ryanofsky approved
  24. ryanofsky commented at 10:51 PM on April 20, 2021: contributor

    Code review ACK fafb68add5e16e8bd5b9428bcffcaee2639747cf

  25. MarcoFalke merged this on Apr 21, 2021
  26. MarcoFalke closed this on Apr 21, 2021

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

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC