rest: clean-up for `mempool` endpoints #25760

The functions rest_mempool_info and rest_mempool_contents are similar, the only difference between them is: rest_mempool_info uses MempoolInfoToJSON to get the mempool informations and rest_mempool_contents uses MempoolToJSON, for this reason this PR creates a new function to handle it and reduce duplicated code.

Also,

1. Rename strURIPart to str_uri_part.
2. Rename strJSON to str_json.

Concept ACK 029fad6

Alternatively, you could also only register /rest/mempool/ and inspect the str_uri_part? I think this would allow for slightly more code reduction.

<!--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:

• #25756 (rest: Remove support for a number of -deprecatedrest options by stickies-v)
• #25755 (rest: Use from_blockhash and txdetails query parameters by stickies-v)
• #25754 (rest: Extend HTTPRequest interface to support querying path (parameters) by stickies-v)
• #25753 (rest: Move format string from path-like parameter to query parameter by stickies-v)
• #25412 (rest: add /deploymentinfo endpoint by brunoerg)
• #21422 (Add feerate histogram to getmempoolinfo by kiminuo)

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.

Alternatively, you could also only register /rest/mempool/ and inspect the str_uri_part? I think this would allow for slightly more code reduction.

Nice, I had thought about it, but I think str_uri_part wouldn't get me the whole URI but only (in this case) the format, like: '.json'. Am I missing something?

http_request_cb passes everything after the endpoint prefix to str_uri_part. If you register the endpoints with prefix /rest/mempool/<info|content>, then indeed only .json would get passed to str_uri_part. If however you just register /rest/mempool/, then everything after that i.e. <info|content>.json would get passed. (plug: #25754 is trying to clean up this interface)

I don't think the current approach of registering both separately is unreasonable, so this is just a suggestion if you think that implementation makes sense - intuitively it would be my first approach.

Nice, @stickies-v. Force-pushed addressing your suggestion.

It is clearer having only /rest/mempool/, and then check the str_uri_part.

Nice! I've left a few more suggestions in comments, but I think that's pretty much it from me.

<details> <summary>Couldn't make suggested code, so here's the diff instead:</summary>

diff --git a/src/rest.cpp b/src/rest.cpp
index dd6fe8d8e..3f2ab98a8 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -594,20 +594,23 @@ static bool rest_mempool(const std::any& context, HTTPRequest* req, const std::s
 {
     if (!CheckWarmup(req))
         return false;
-    const CTxMemPool* mempool = GetMemPool(context, req);
-    if (!mempool) return false;
+
     std::string param;
     const RESTResponseFormat rf = ParseDataFormat(param, str_uri_part);
+    if (param != "contents" && param != "info") {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Error: invalid URI format. Expected /rest/mempool/<info|contents>.json");
+    }
+
+    const CTxMemPool* mempool = GetMemPool(context, req);
+    if (!mempool) return false;
 
     switch (rf) {
     case RESTResponseFormat::JSON: {
         std::string str_json;
-        if (str_uri_part == "contents.json") {
+        if (param == "contents") {
             str_json = MempoolToJSON(*mempool, true).write() + "\n";
-        } else if (str_uri_part == "info.json") {
-            str_json = MempoolInfoToJSON(*mempool).write() + "\n";
         } else {
-            return RESTERR(req, HTTP_BAD_REQUEST, "Error: invalid URI format");
+            str_json = MempoolInfoToJSON(*mempool).write() + "\n";
         }
 
         req->WriteHeader("Content-Type", "application/json");

</details>

Thanks, @stickies-v for your review! Force-pushed addressing the latest suggestions.

ACK da0c612c3d69164da19332167c38bfcd1f9777a8 - nice work on removing all the duplicate code!

Just 1 nit that I didn't see earlier, sorry.

Nice deduplication! 👌

Code-review ACK da0c612c3d69164da19332167c38bfcd1f9777a8

Happy to also re-ACK if you decide to take stickies' nit about removing the 'Error' prefix.

Force-pushed removing the Error: prefix. Thanks, @stickies-v.

re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b - verified that just the error message was updated since https://github.com/bitcoin/bitcoin/commit/da0c612c3d69164da19332167c38bfcd1f9777a8

re-ACK acbea66589100fe6ef726f4b2a92ec26132ef17b

Concept ACK