refactor: Add and use EnsureConnman in rpc code #21719
Conversation
maflcko
force-pushed
the
2104-netConnman
branch
from
fa72296 to
fad37e6
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
Concept ACK: neat cleanups! |
jarolrod
left a comment
jarolrod
left a comment
There was a problem hiding this comment.
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;
theStack
left a comment
theStack
left a comment
There was a problem hiding this comment.
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.
maflcko
force-pushed
the
2104-netConnman
branch
from
fad37e6 to
fafb68a
Compare
|
Thanks, done |
|
re-ACK fafb68a |
Use new EnsureConnman from bitcoin/bitcoin#21719 in the auxpow check code, and also use the request context chainstate there to get rid of globals.
Use new EnsureConnman from bitcoin/bitcoin#21719 in the namecoin RPC code.
6 participants
This removes the 10 occurrences of
throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");and replaces them withEnsureConnman.