[RPC] Give RPC commands more information about the RPC request #8788

1. 
   jonasschnelli commented at 8:04 am on September 22, 2016: contributor

   Alternative solution for #8775. Looks more invasive than it is (mostly search & replace).

   This PR does replace the RPC call signature (UniValue &params, bool fHelp) with (JSONRequest &request) which allows the registered RPC calls to get more information about the request.

   The JSONRequest object contains the HTTP base auth username as well as the path of the called HTTP endpoint.

   This PR would allow wallet distinction based on HTTP auth username and or URL endpoints.

2. jonasschnelli added the label Refactoring on Sep 22, 2016
3. jonasschnelli added the label RPC/REST/ZMQ on Sep 22, 2016
4. 
   jonasschnelli commented at 8:08 am on September 22, 2016: contributor

   For the multiwallet support, each wallet RPC call could do something like CWallet *pwallet = CWallets::getWalletFromRequest(request) instead of accessing pwalletMain.
5. jonasschnelli force-pushed on Sep 22, 2016
6. 
   laanwj commented at 12:24 pm on September 22, 2016: member

   Concept ACK, the code churn is a bit unfortunate but passing a context structure into RPC methods had to be done at some point… (and it’s better to face this once than hacking in something like a thread local pointer…)
7. 
   jonasschnelli commented at 12:35 pm on September 22, 2016: contributor

   Yes. It’s larger then I initially though. But once we did the “large” migration to a structure/object-passing we will be flexible for further changes. Most changed lines are just a params. to request.params. which should be easy to verify/review.
8. in src/rpc/blockchain.cpp: in ff5b7914ca outdated

   446 
   447-UniValue getmempoolancestors(const UniValue& params, bool fHelp)
   448+UniValue getmempoolancestors(const JSONRequest& request)
   449 {
   450-    if (fHelp || params.size() < 1 || params.size() > 2) {
   451+    if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) {
   

   ---

   ---

   

   MarcoFalke commented at 10:52 pm on September 22, 2016:

   nit: You can put something like

   0UniValue params = request.params;
   

   in the first line of the method to make the diff smaller and less verbose?

   ---

   

   jonasschnelli commented at 7:50 am on September 23, 2016:

   I have considered this. But if we would have done this from the beginning, wouldn’t it be without the extra UniValue params delegation? I think this PRs solution is cleaner.

   We have to do this once and therefore probably accept a larger diff.

   ---

   

   laanwj commented at 9:40 am on September 25, 2016:

   No strong opinion on this. Indeed, doing const UniValue &params = request.params; at the beginning of each function would reduce diff, on the other hand, I don’t think prefixing request. everywhere is bad. It even makes the code a little bit more readable. “What params? The request params”. I’m fine with keeping it like this
9. 
   laanwj commented at 9:46 am on September 25, 2016: member

   This reminds me, I should have called that class JSONRPCRequest not JSONRequest. It’s a JSON-RPC server not a JSON server.

   If you agree I think it would make sense to rename it as a first commit (it only appears in 5 places) before it becomes an argument to every RPC call.

   Edit: GAH, JSONRPCRequest is already a function name in protocol.h. This could explain why I made the decision to name it like that. Ok… never mind the above.

10. 
    laanwj commented at 4:53 pm on September 29, 2016: member

    @jonasschnelli I’d suggest the following to get rid of the current JSONRPCRequest: https://github.com/laanwj/bitcoin/commit/ae6db2a5ff781ffa876f7d424438901d68529a8b, then rename the actual request object.
11. rpc: Change JSONRPCRequest to JSONRPCRequestObj

    This is more consistent with `JSONRPCReplyObj`.

    23c32a9694
12. [RPC] Give RPC commands more information about the RPC request 69d1c25768
13. jonasschnelli force-pushed on Oct 19, 2016
14. 
    jonasschnelli commented at 12:46 pm on October 19, 2016: contributor

    Rebased and added https://github.com/laanwj/bitcoin/commit/ae6db2a5ff781ffa876f7d424438901d68529a8b as first commit, renamed JSONRequest to JSONRPCRequest.
15. [RPC] pass HTTP basic authentication username to the JSONRequest object e7156ad61b
16. 
    laanwj commented at 1:16 pm on October 19, 2016: member

    utACK e7156ad
17. in src/rest.cpp: in e7156ad61b

    285@@ -286,6 +286,7 @@ static bool rest_chaininfo(HTTPRequest* req, const std::string& strURIPart)
    286     switch (rf) {
    287     case RF_JSON: {
    288         JSONRPCRequest jsonRequest;
    289+        jsonRequest.params = UniValue(UniValue::VARR);
    

    ---

    ---

    

    laanwj commented at 1:22 pm on October 19, 2016:

    This is kind of ugly, but it already was ugly, as we’re not supposed to call into RPC from the REST interface (or use any RPC stuff at all from REST). At some point these functions should be moved to a shared compilation unit. Out of scope for this pull ofcourse.
18. laanwj merged this on Oct 19, 2016
19. laanwj closed this on Oct 19, 2016

    ---

20. laanwj referenced this in commit 97c7f7362f on Oct 19, 2016
21. codablock referenced this in commit 3c59459264 on Sep 9, 2017
22. codablock referenced this in commit b4bd44b508 on Sep 11, 2017
23. UdjinM6 referenced this in commit 91d99fcd3f on Sep 11, 2017
24. codablock referenced this in commit d80b454215 on Sep 19, 2017
25. codablock referenced this in commit dd6b9ad20f on Jan 13, 2018
26. andvgal referenced this in commit 4ad681ab65 on Jan 6, 2019
27. CryptoCentric referenced this in commit 2976bb4adc on Feb 24, 2019
28. MarcoFalke locked this on Sep 8, 2021

Contributors
jonasschnelli laanwj MarcoFalke

Labels
Refactoring RPC/REST/ZMQ