Put the correct RPC port (default for the selected chain or set explicitly with -rpcport) into the RPC help text "curl" example. Previously, the port there was always reported as 8332 (default mainnet port). With this change, it will be correct for testnet/regtest or with an explicit -rpcport, so that the example can be used as-is in these situations.
This is a simplified version of #15158, following the discussion with @laanwj.
568 | @@ -567,8 +569,17 @@ std::string HelpExampleCli(const std::string& methodname, const std::string& arg 569 | 570 | std::string HelpExampleRpc(const std::string& methodname, const std::string& args) 571 | { 572 | - return "> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\":\"curltest\", " 573 | - "\"method\": \"" + methodname + "\", \"params\": [" + args + "] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n"; 574 | + const int rpcPort = gArgs.GetArg("-rpcport", BaseParams().RPCPort()); 575 | + 576 | + std::ostringstream help;
Please choose another variable name to avoid shadowing function help :-)
Fixed, another good catch! Do you have an automated script to check for those shadowing instances? :)
Put the correct RPC port (default for the selected chain or set explicitly
with -rpcport) into the RPC help text "curl" example. Previously, the
port there was always reported as 8332 (default mainnet port). With this
change, it will be correct for testnet/regtest or with an explicit
-rpcport, so that the example can be used as-is in these situations.
I'm really sorry to say, but again this introduces an unwanted dependency.
rpcserver is supposed to be more or less independent of bitcoin (or -coin specific stuff), which includes the chain.
I'm really sorry to say, but again this introduces an unwanted dependency.
rpcserveris supposed to be more or less independent of bitcoin (or -coin specific stuff), which includes the chain.
Well, then I guess there is no way at all to do what I'm trying to do here.
Why do you think such a (simple) code dependency is so bad, considering that there is already a semantic dependency anyway (by having that curl example in the first place)?
Why do you think such a (simple) code dependency is so bad, considering that there is already a semantic dependency anyway (by having that curl example in the first place)?
Look, I'm not going to back down on keeping the architecture clean here. It should in principle be possible to copy rpc/server.cpp to another project and have it still work. A curl example would still be valid as long as there is a http frontend, but it might use a different way to determine the port to listen on than bitcoin does.
I think it is possible to do this without introducing any new dependencies by making httprpc, which knows about both HTTP and RPC, request the port number from http and then tell the rpc_server module about which port number to use in the help.
I think it is possible to do this without introducing any new dependencies by making
httprpc, which knows about both HTTP and RPC, request the port number fromhttpand then tell therpc_servermodule about which port number to use in the help.
That seems like overengineering to me - but I'm happy to do whatever you prefer. I guess we have two remaining options:
Do you like 2), or is that too much complexity for little benefit? I think this is something that would be nice to fix, but I'm perfectly happy to go with 1) as well if that's what you and others prefer.
That seems like overengineering to me - but I'm happy to do whatever you prefer. I guess we have two remaining options:
IMO it's not over-engineering if it's simply working within the constraints of the architecture. It will become a mess if we start to add dependencies from every single thing to everything else, certainly for what is only a slight documentation improvement.
I'm okay with both options.