The verbose mempool json response can get very large. This adds an option to return the non-verbose response of just the txids. It is identical to the rpc response so the diff here is minimal. This also adds the mempool_sequence parameter for rpc consistency. Verbose defaults to true to remain backwards compatible.
It uses query parameters to be compatible with the efforts in #25752.
618+ return RESTERR(req, HTTP_BAD_REQUEST, "The \"mempool_sequence\" query parameter must be either \"true\" or \"false\".");
619+ }
620+ bool verbose = raw_verbose == "true";
621+ bool mempool_sequence = raw_mempool_sequence == "true";
622+ if (verbose && mempool_sequence) {
623+ return RESTERR(req, HTTP_BAD_REQUEST, "Verbose results cannot contain mempool sequence values. (hint: set \"verbose=false\")");
Concept ACK
You should update the doc (REST-interface.md) as well
607@@ -608,7 +608,20 @@ static bool rest_mempool(const std::any& context, HTTPRequest* req, const std::s
608 case RESTResponseFormat::JSON: {
609 std::string str_json;
610 if (param == "contents") {
611- str_json = MempoolToJSON(*mempool, true).write() + "\n";
612+ std::string raw_verbose = req->GetQueryParameter("verbose").value_or("true");
maybe use a const here?
0 const std::string raw_verbose = req->GetQueryParameter("verbose").value_or("true");
You should update the doc (REST-interface.md) as well @brunoerg would it make sense to update that file after there has been a release with this patch? I think that might confuse users who see those options in the doc but are unable to use them in any release.
120@@ -121,11 +121,13 @@ Returns various information about the transaction mempool.
121 Only supports JSON as output format.
122 Refer to the `getmempoolinfo` RPC help for details.
123
124-`GET /rest/mempool/contents.json`
125+`GET /rest/mempool/contents.json?verbose=false`
0`GET /rest/mempool/contents.json?verbose=<true|false>&mempool_sequence=<true|false>`
127 Returns the transactions in the mempool.
128 Only supports JSON as output format.
129-Refer to the `getrawmempool` RPC help for details.
130+Refer to the `getrawmempool` RPC help for details. Supports the `verbose` and
131+`mempool_sequence` arguments as query parameters. Defaults to setting
132+`verbose=true`.
getrawmempool RPC, as long as these arguments mirror the RPC I think we can leave out this extra documentation.
614+ return RESTERR(req, HTTP_BAD_REQUEST, "The \"verbose\" query parameter must be either \"true\" or \"false\".");
615+ }
616+ const std::string raw_mempool_sequence = req->GetQueryParameter("mempool_sequence").value_or("false");
617+ if (raw_mempool_sequence != "true" && raw_mempool_sequence != "false") {
618+ return RESTERR(req, HTTP_BAD_REQUEST, "The \"mempool_sequence\" query parameter must be either \"true\" or \"false\".");
619+ }
HTTPRequest::GetQueryParameter<bool> specialization that converts to and returns a bool, or throws an err if the string can’t be parsed. However, that would require some more fundamental changes to how we deal with errors (which unfortunately we return instead of throw) here. Flagging it here for follow-up, feel free to grab it if you’re keen.
HTTPRequest::GetQueryParameter<bool>, it would be more elegant than using string (“true”/“false”), but I know it would require a lot of more work and maybe could be done in a follow-up.
615+ }
616+ const std::string raw_mempool_sequence = req->GetQueryParameter("mempool_sequence").value_or("false");
617+ if (raw_mempool_sequence != "true" && raw_mempool_sequence != "false") {
618+ return RESTERR(req, HTTP_BAD_REQUEST, "The \"mempool_sequence\" query parameter must be either \"true\" or \"false\".");
619+ }
620+ const bool verbose = raw_verbose == "true";
{} list initialization is preferred (+ in other places):
0 const bool verbose{raw_verbose == "true"};
Concept ACK - bringing it further in line with the getrawmempool RPC seems reasonable.
would it make sense to update that file after there has been a release with this patch?
To minimize maintenance I’d suggest labeling this pull with the 25.0 milestone and adding a small disclaimer to the rest-interface.md documentation that these 2 parameters are available only from 25.0, similar to https://github.com/bitcoin/bitcoin/blob/3baa0f5a603cedd66442492346583f2b3d776db7/doc/REST-interface.md?plain=1#L56
That way when this PR doesn’t make it into 25.0 it’ll be easy to track and update the docs.
I’d suggest labeling this pull with the 25.0 milestone @stickies-v I don’t think I have permissions to add labels. Otherwise I have addressed all feedback from yourself and @brunoerg .
127+`GET /rest/mempool/contents.json?verbose=<true|false>&mempool_sequence=<true|false>`
128
129 Returns the transactions in the mempool.
130 Only supports JSON as output format.
131-Refer to the `getrawmempool` RPC help for details.
132+Refer to the `getrawmempool` RPC help for details. Defaults to setting
trailing whitespace is failing the linter:
0Refer to the `getrawmempool` RPC help for details. Defaults to setting
128
129 Returns the transactions in the mempool.
130 Only supports JSON as output format.
131-Refer to the `getrawmempool` RPC help for details.
132+Refer to the `getrawmempool` RPC help for details. Defaults to setting
133+`verbose=true`.
0`verbose=true` and `mempool_sequence=false`.
122@@ -123,11 +123,15 @@ Returns various information about the transaction mempool.
123 Only supports JSON as output format.
124 Refer to the `getmempoolinfo` RPC help for details.
125
126-`GET /rest/mempool/contents.json`
127+`GET /rest/mempool/contents.json?verbose=<true|false>&mempool_sequence=<true|false>`
Actually, since false is default, perhaps better to set that as the first option?
0`GET /rest/mempool/contents.json?verbose=<true|false>&mempool_sequence=<false|true>`
ACK c35c7d091 (for 25.0)
I don’t think I have permissions to add labels.
The maintainers should be able to add the 25.0 label when they see this, otherwise you can always send a message on #bitcoin-core-dev IRC. It’s not super urgent though, just would be a bit easier to track I think.
128
129 Returns the transactions in the mempool.
130 Only supports JSON as output format.
131-Refer to the `getrawmempool` RPC help for details.
132+Refer to the `getrawmempool` RPC help for details. Defaults to setting
133+`verbose=true` and `mempool_sequence=false.
0`verbose=true` and `mempool_sequence=false`.
re-ACK 1ff5d61
I verified that the only changes since c35c7d0 are suggested documentation updates to REST-interface.md.
tested ACK 1ff5d61dfdaf8987e5619162662e4c760af76a43.
I’ve performed manual tests getting the node started with -rest, comparing results between the rest api call using curl with the different combinations of params and values vs. calling the getrawtransacion rpc command (using bitcoin-cli), which is essentially what the python functional tests do.
It works as expected and it’s consistent with rpc.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | stickies-v, pablomartin4btc, achow101 |
| Concept ACK | brunoerg |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
