Support JSON-RPC 2.0 when requested by client #27101

pinheadmz · 2023-02-14T19:13:42Z

Bitcoin Core's JSONRPC server behaves with a special blend of 1.0, 1.1 and 2.0 behaviors. This introduces compliance issues with more strict clients. There are the major misbehaviors that I found:

returning non-200 HTTP codes for RPC errors like "Method not found" (this is not a server error or an HTTP error)
returning both "error" and "result" fields together in a response object.
different error-handling behavior for single and batched RPC requests (batches contain errors in the response but single requests will actually throw HTTP errors)

#15495 added regression tests after a discussion in #15381 to kinda lock in our RPC behavior to preserve backwards compatibility.

#12435 was an attempt to allow strict 2.0 compliance behind a flag, but was abandoned.

The approach in this PR is not strict and preserves backwards compatibility in a familiar bitcoin-y way: all old behavior is preserved, but new rules are applied to clients that opt in. One of the rules in the JSON RPC 2.0 spec is that the kv pair "jsonrpc": "2.0" must be present in the request. Well, let's just use that to trigger strict 2.0 behavior! When that kv pair is included in a request object, the response will adhere to strict JSON-RPC 2.0 rules, essentially:

always return HTTP 200 "OK" unless there really is a server error or malformed request
either return "error" OR "result" but never both
same behavior for single and batch requests

If this is merged next steps can be:

Refactor bitcoin-cli to always use strict 2.0
Refactor the python test framework to always use strict 2.0 for everything
Begin deprecation process for 1.0/1.1 behavior (?)

If we can one day remove the old 1.0/1.1 behavior we can clean up the rpc code quite a bit.

DrahtBot · 2023-02-14T19:13:45Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	cbergqvist, ryanofsky, tdb3
Concept ACK	ajtowns, fanquake, sipa, stickies-v, vincenzopalazzo
Stale ACK	willcl-ark

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29946 (JSON-RPC request Content-Type is application/json by luke-jr)

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.

ajtowns

Concept ACK

src/rpc/request.h

fanquake · 2023-02-20T17:41:13Z

Concept ACK

sipa · 2023-02-27T21:33:38Z

Concept ACK. Migrating to strict JSON-RPC 2.0 for bitcoin-cli and whatever opts into 2.0 is cool, even if it means keeping support for kinda-sorta-JSON-RPC 1.0 around on the server side.

willcl-ark · 2023-02-28T12:31:16Z

Concept ACK.

Might be worth updating a few other things at the same time if you continue to move ahead:

This comment: https://github.com/bitcoin/bitcoin/pull/27101/files#diff-188b88992c9e08a108e852e7f69a2c2c6569d4b1212c9d05c46fdfd7fcb778d7R21-R27
HelpExampleRPC() to json 2.0
Could warrant a release note to document new behaviours when using json 2.0

pinheadmz · 2023-03-01T19:08:45Z

@willcl-ark thanks, I added comments and release notes.

I also wrote a tiny testing package using libjson-rpc-cpp to check against an "outside" JSON-RPC 2.0 implementation. The package is https://github.com/pinheadmz/jsonrpc-bitcoin and seems to like the 2.0 implementation so far.

stickies-v

Concept ACK

pinheadmz · 2023-03-20T15:04:55Z

This PR is ready for code review if any of you fine handsome concept-ACKers have the time ❤️

willcl-ark

ACK 2da8231

Left a few comments which could be addressed here, in a followup or not at all. None of them materially affect the current implementation of this feature which works well.

Although, I would be curious to know why we are still responding with http 500 errors for invalid json after the move to json 2.0.

willcl-ark · 2023-03-20T21:22:53Z

test/functional/interface_rpc.py

+        AuthServiceProxy.jsonrpc20 = True
+        # All 200!
+        expect_http_status(200, -32601, self.nodes[0].invalidmethod)    # RPC_METHOD_NOT_FOUND
+        expect_http_status(200, -8,     self.nodes[0].getblockhash, 42) # RPC_INVALID_PARAMETER


Should this not be a status: 200, error: -32602 under json 2.0?

Hm yeah good point, a few error codes like this are specified. One problem with this is that our application error RPC_INVALID_PARAMETER (-8) is coded ~186 times in RPC functions everywhere. I suppose we could re-map those error codes back to the jsonrpc 2.0 spec in JSONRPCExecOne()? Only when the request indicates jsonrpc 2.0

test/functional/interface_rpc.py

doc/release-notes-27101.md

test/functional/interface_rpc.py

pinheadmz · 2023-03-28T18:17:06Z

Added a scripted-diff to completely replace all occurrences of the application-defined RPC_INVALID_PARAMETER (-8) with the JSONRPC 2.0 spec-defined RPC_INVALID_PARAMS (-32602)

pinheadmz · 2024-05-01T19:39:40Z

@ryanofsky thanks so much, again, for your help. Especially in understanding the std::move semantics. I've reviewed each commit in your branch and force pushed it to mine with the only change being specifying json request version in JSONRPCReplyObj() calls in bitcoin-cli.cpp

cbergqvist

reACK 8583c2f

Ran most functional tests including interface_rpc.py without any failures. Ran make check without failures or skips.

Happy that the version parameters to JSONRPCReplyObj() etc are now explicit.

cbergqvist · 2024-05-08T12:40:09Z

src/httprpc.cpp

+                    jreq.parse(valRequest[i]);
+                    response = JSONRPCExec(jreq);
+                } catch (UniValue& e) {
+                    response = JSONRPCReplyObj(NullUniValue, e, jreq.id, jreq.m_json_version);


Here the caught UniValue& is not explicitly std::moved. But on line 272 it is. Is that intentional?

Good catch thanks. I'll need @ryanofsky to weigh in here because I keep misunderstanding c++ move semantics. But here's what I think is happening:

In JSONRPCReplyObj() in request.cpp the really import move-instead-of-copy is when we call reply.pushKV(...): https://github.com/pinheadmz/bitcoin/blob/8583c2f93d4eb020bf1e3b74860b691ec12e5eee/src/rpc/request.cpp#L64

That makes me think that the std::move() currently in JSONErrorReply() in httprpc.cpp is unnecessary: https://github.com/esotericnonsense/bitcoin/blob/8583c2f93d4eb020bf1e3b74860b691ec12e5eee/src/httprpc.cpp#L90

...and also unnecessary is the one you see on line 272.

So if I'm finally understanding, which is unlikely, then there are two places where std::move can be removed but actually this line 244 right here is OK, because the import move-instead-of-copy happens inside the function being called.

re: #27101 (comment)

I think @cbergqvist is right here and e should be replaced by std::move(e) on line 244.

The JSONRPCReplyObj signature is:

UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONVersion json_version);

So without std::move(e), the error parameter will be initialized by making a copy of e instead of moving from e. This is because e is an lvalue, while std::move(e) is an rvalue.

In JSONRPCReplyObj() in request.cpp the really import move-instead-of-copy is when we call reply.pushKV(...): https://github.com/pinheadmz/bitcoin/blob/8583c2f93d4eb020bf1e3b74860b691ec12e5eee/src/rpc/request.cpp#L64

That is true. The signature for pushKV is void pushKV(std::string key, UniValue val), so without std::move(error), on that line the function parameter val would be initialized by copying not moving.

That makes me think that the std::move() currently in JSONErrorReply() in httprpc.cpp is unnecessary: https://github.com/esotericnonsense/bitcoin/blob/8583c2f93d4eb020bf1e3b74860b691ec12e5eee/src/httprpc.cpp#L90

This is not true. std::move is needed to avoid copy-initializing the JSONRPCReplyObj error parameter there, just like on line 272 and on line

So if I'm finally understanding, which is unlikely, then there are two places where std::move can be removed but actually this line 244 right here is OK, because the import move-instead-of-copy happens inside the function being called.

Yeah this is a little off. What you are saying is true if the function signature accepts parameters by reference. For example if you have function that looks like:

void MyFunction(SomeType& param); void MyFunction(const SomeType& param); void MyFunction(SomeType&& param);

In this case, no matter whether you call MyFunction with std::move or without it, the parameter param is going to be a reference not a value, so no copy will be created just by calling the function. There could copies of param made inside the function, depending on what it does internally, as you say. But merely calling the function will not copy the parameter.

However, if you have a function that takes a non-reference value parameter like:

void MyFunction(SomeType param);

No matter what the function does internally, just calling the function is going to copy-construct param variable or move-construct it. And unless you use std::move or the argument is already an rvalue, the argument will be copied.

Had to check whether UniValue even had a move-constructor. It seems like one should be generated implicitly if my readings of https://en.cppreference.com/w/cpp/language/move_constructor#Implicitly-declared_move_constructor and univalue.h are correct.

@ryanofsky's explanation rings true with my long underutilized C++11 neurons.

cbergqvist · 2024-05-08T13:28:37Z

src/httprpc.cpp

-        req->WriteReply(HTTP_OK, strReply);
-    } catch (const UniValue& objError) {
-        JSONErrorReply(req, objError, jreq.id);
+        req->WriteReply(HTTP_OK, reply.write() + "\n");


Might be more correct & compatible to append "\r\n". See https://stackoverflow.com/questions/5757290/http-header-line-break-style

That's a reasonable comment but I kept the single character which is how it worked on master (see below). I see a few examples of \r\n in the codebase but its rare and I'm not sure how it's decided.

bitcoin/src/rpc/request.cpp

Lines 52 to 56 in 63d0b93

std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id)

{

UniValue reply = JSONRPCReplyObj(result, error, id);

return reply.write() + "\n";

}

In commit "rpc: refactor single/batch requests" (a64a2b7)

re: #27101 (comment)

HTTP headers use \r\n, but this line is generating the JSON object in the body of the response, after the headers. And the JSON could be written with either \r\n or \n or no trailing line break at all. I think it would probably make sense not to include any trailing line break, but existing code uses \n, so in a refactoring commit it seems best not to change this.

ryanofsky

Code review ACK 8583c2f. Looks very good! I left several comments, but none are critical and they could be followed up later.

ryanofsky · 2024-05-08T15:13:09Z

src/httprpc.cpp

-        req->WriteReply(HTTP_OK, strReply);
-    } catch (const UniValue& objError) {
-        JSONErrorReply(req, objError, jreq.id);
+        req->WriteReply(HTTP_OK, reply.write() + "\n");


In commit "rpc: refactor single/batch requests" (a64a2b7)

re: #27101 (comment)

HTTP headers use \r\n, but this line is generating the JSON object in the body of the response, after the headers. And the JSON could be written with either \r\n or \n or no trailing line break at all. I think it would probably make sense not to include any trailing line break, but existing code uses \n, so in a refactoring commit it seems best not to change this.

ryanofsky · 2024-05-08T15:24:44Z

src/rpc/request.cpp

+        if (!valJsonRPC.isStr()) {
+            throw JSONRPCError(RPC_INVALID_REQUEST, "jsonrpc field must be a string");
+        }
+        if (valJsonRPC.get_str() == "1.0") {


In commit "rpc: identify JSON-RPC 2.0 requests" (6d0be75)

Might be good to add a comment noting that the "jsonrpc" version field was added in the JSON-RPC 2.0 spec, so value "1.0" is nonstandard. But it is accepted for backwards compatibility, because some old documentation (incorrectly) suggested adding it.

ryanofsky · 2024-05-08T15:26:30Z

src/rpc/request.cpp

@@ -167,6 +167,22 @@ void JSONRPCRequest::parse(const UniValue& valRequest)
    // Parse id now so errors from here on will have the id
    id = request.find_value("id");

+    // Check for JSON-RPC 2.0 (default 1.1)
+    m_json_version = JSONVersion::JSON_1_BTC;
+    const UniValue& valJsonRPC = request.find_value("jsonrpc");


In commit "rpc: identify JSON-RPC 2.0 requests" (6d0be75)

Variable name valJsonRPC doesn't really follow the suggested coding style which calls for snake_case. Could rename it to "jsonrpc_version"

ryanofsky · 2024-05-08T15:27:59Z

src/rpc/request.h

@@ -11,6 +11,11 @@

 #include <univalue.h>

+enum class JSONVersion {


In commit "rpc: identify JSON-RPC 2.0 requests" (6d0be75)

Technically JSON-RPC version would be more accurate the JSON version. could s/JSON/JSONRPC/ throughout this enum

Is JSON/JSONRPC really necessary for the values themselves if the enum is of the type enum class so that one always has to prepend the type? - JSONRPCVersion::JSONRPC_1_BTC vs JSONRPCVersion::1_BTC.

re: #27101 (comment)

Is JSON/JSONRPC really necessary for the values themselves if the enum is of the type enum class so that one always has to prepend the type? - JSONRPCVersion::JSONRPC_1_BTC vs JSONRPCVersion::1_BTC.

Good point. I'd probably call the constants V1_LEGACY and V2 if renaming.

src/httprpc.cpp

ryanofsky · 2024-05-08T15:49:41Z

src/rpc/server.cpp

+            return JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version);
+        }
+    } else {
+        // Legacy Bitcoin JSONRPC 1.0/1.2 behavior:


In commit "rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests" (56b6c33)

This is probably supposed to say 1.1 not 1.2

ryanofsky · 2024-05-08T16:06:44Z

src/rpc/server.cpp

+        } catch (UniValue& e) {
+            return JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version);
+        } catch (const std::exception& e) {
+            return JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version);


In commit "rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests" (56b6c33)

Returning RPC_PARSE_ERROR does not seem accurate if the request was parsed successfully but there was an error executing it. This was also a preexisting problem in HTTPReq_JSONRPC for batch requests, but now the behavior will happen for all version 2 single requests as well. Would suggest cleaning this up with a change like:

--- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -204,7 +204,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) return false; } - reply = JSONRPCExec(jreq); + // Legacy 1.0/1.1 behavior is for failed requests to throw + // exceptions which return HTTP errors and RPC errors to the client. + // 2.0 behavior is to catch exceptions and return HTTP success with + // RPC errors, as long as there is not HTTP server error. + const bool catch_errors{jreq.m_json_version == JSONVersion::JSON_2_0}; + reply = JSONRPCExec(jreq, catch_errors); // array of requests } else if (valRequest.isArray()) { @@ -233,12 +238,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) // in "HTTP OK" responses. try { jreq.parse(valRequest[i]); - reply.push_back(JSONRPCExec(jreq)); } catch (UniValue& e) { reply.push_back(JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version)); } catch (const std::exception& e) { reply.push_back(JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version)); } + reply.push_back(JSONRPCExec(jreq, /*catch_errors=*/true)); } } else --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -358,22 +358,18 @@ bool IsDeprecatedRPCEnabled(const std::string& method) return find(enabled_methods.begin(), enabled_methods.end(), method) != enabled_methods.end(); } -UniValue JSONRPCExec(const JSONRPCRequest& jreq) +UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors) { UniValue result; - if (jreq.m_json_version == JSONVersion::JSON_2_0) { - // JSONRPC 2.0 behavior: only throw HTTP error if the server is actually - // broken. Otherwise errors are sent back in "HTTP OK" responses. + if (catch_errors) { try { result = tableRPC.execute(jreq); } catch (UniValue& e) { return JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version); } catch (const std::exception& e) { - return JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version); + return JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_MISC_ERROR, e.what()), jreq.id, jreq.m_json_version); } } else { - // Legacy Bitcoin JSONRPC 1.0/1.2 behavior: - // Single requests may throw HTTP errors, handled by caller or client result = tableRPC.execute(jreq); } --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -181,7 +181,7 @@ extern CRPCTable tableRPC; void StartRPC(); void InterruptRPC(); void StopRPC(); -UniValue JSONRPCExec(const JSONRPCRequest& jreq); +UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors); // Drop witness when serializing for RPC? bool RPCSerializationWithoutWitness();

Thanks so much for the patch. I had to move this line back up to where it was, otherwise the test fails because after pushing the error into the batch reply, we would also then push a weird extra junk reply with the current id but the result from the last successful query

diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 0e3e66c515..777ad32bbe 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -238,12 +238,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) // in "HTTP OK" responses. try { jreq.parse(valRequest[i]); + reply.push_back(JSONRPCExec(jreq, /*catch_errors=*/true)); } catch (UniValue& e) { reply.push_back(JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version)); } catch (const std::exception& e) { reply.push_back(JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version)); } - reply.push_back(JSONRPCExec(jreq, /*catch_errors=*/true)); } } else

I'm trying to figure out how to write a test to catch one of these RPC_MISC_ERROR since they obviously weren't catching a RPC_PARSE_ERROR before adding your patch to the commit. If I understand correctly, that "misc" error would only throw if there was a bug in an RPC command because most of our try blocks try to catch a UniValue error first with a specific code.

Only for JSON-RPC 2.0 requests.

Avoid returning HTTP status errors for non-batch JSON-RPC 2.0 requests if the RPC method failed but the HTTP request was otherwise valid. Batch requests already did not return HTTP errors previously.

For JSON-RPC 2.0 requests we need to distinguish between a missing "id" field and "id":null. This is accomplished by making the JSONRPCRequest id property a std::optional<UniValue> with a default value of UniValue::VNULL. A side-effect of this change for non-2.0 requests is that request which do not specify an "id" field will no longer return "id": null in the response.

pinheadmz · 2024-05-14T17:28:00Z

force push to cbc6c44:

rename enum to JSONRPCVersion::{V1_LEGACY, V2}
use std::move() in httprpc
pass catch_errors argument to JSONRPCExec()
replace RPC_PARSE_ERROR with RPC_MISC_ERROR in JSONRPCExec()

thanks again for the reviews @cbergqvist @ryanofsky

cbergqvist · 2024-05-15T12:42:16Z

src/httprpc.cpp

@@ -73,8 +73,11 @@ static std::vector<std::vector<std::string>> g_rpcauth;
 static std::map<std::string, std::set<std::string>> g_rpc_whitelist;
 static bool g_rpc_whitelist_default = false;

-static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const UniValue& id)
+static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCRequest& jreq)


nit: JSONErrorReply -> JSONRPCErrorReply, although it could be argued that it actually does write a JSON object in the response.

cbergqvist

re ACK cbc6c44

All functional tests passed (with a few automatic skips), except feature_dbcrash - slow, unrelated => excluded, and feature_index_prune => timed out because rebase with bumped timeout has been held-off.

cbergqvist · 2024-05-15T12:46:27Z

src/rpc/server.cpp

-    {
-        rpc_result = JSONRPCReplyObj(NullUniValue,
-                                     JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
+UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors)


Would have gone the opposite way and called it throw_errors since it is an.. exception.. to maintain legacy behavior. Sorry for not catching that earlier.

re: #27101 (comment)

Would have gone the opposite way and called it throw_errors since it is an.. exception.. to maintain legacy behavior. Sorry for not catching that earlier.

I think either way is fine, but catch_errors does seem more literally correct since the argument is just controlling whether the exceptions will be caught. Errors will be still be thrown regardless.

ryanofsky

Code review ACK cbc6c44. Just suggested changes since the last review: changing uncaught exception error code from PARSE_ERROR to MISC_ERROR, renaming a few things, and adding comments.

ryanofsky · 2024-05-15T14:14:44Z

src/rpc/request.cpp

+        }
+        // The "jsonrpc" key was added in the 2.0 spec, but some older documentation
+        // incorrectly included {"jsonrpc":"1.0"} in a request object, so we
+        // maintain that for backwards compatibility.


In commit "rpc: identify JSON-RPC 2.0 requests" (2ca1460)

I think it would be a little clearer to say "continue to accept that" instead of "maintain that." Otherwise it sounds like we are trying to maintain incorrectly including the field, not just allowing it if it is specified.

ryanofsky · 2024-05-15T14:19:18Z

src/rpc/server.cpp

-    {
-        rpc_result = JSONRPCReplyObj(NullUniValue,
-                                     JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
+UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors)


re: #27101 (comment)

Would have gone the opposite way and called it throw_errors since it is an.. exception.. to maintain legacy behavior. Sorry for not catching that earlier.

I think either way is fine, but catch_errors does seem more literally correct since the argument is just controlling whether the exceptions will be caught. Errors will be still be thrown regardless.

ryanofsky · 2024-05-15T14:31:02Z

@willcl-ark, @tdb3, any interest in re-acking? This seems like it could definitely be merged with another ack

tdb3 · 2024-05-15T14:48:51Z

@willcl-ark, @tdb3, any interest in re-acking? This seems like it could definitely be merged with another ack

Definitely. I'll plan to take a look tonight.

tdb3

re ACK for cbc6c44

Great work. Performed brief code review. Re-ran the tests described in #27101 (comment) again (actions 1 through 7, with the caveat that v27.0 was used as the baseline for comparison rather v26.0, and regtest was used). Everything worked as expected. Also ran all functional tests (passed).

maflcko · 2024-05-20T12:35:08Z

doc/JSON-RPC-interface.md

+and responds accordingly. A 2.0 request is identified by the presence of
+`"jsonrpc": "2.0"` in the request body. If that key + value is not present in a request,
+the legacy JSON-RPC v1.1 protocol is followed instead, which was the only available
+protocol in previous releases.


nit: Now that this is merged, it could say "in 27.0 and prior releases." Otherwise, on 29.x it will read as-if 28.0 had it missing.

maflcko · 2024-05-20T12:36:31Z

doc/release-notes-27101.md

+
+- Returning HTTP "204 No Content" responses to JSON-RPC 2.0 notifications instead of full responses.
+- Returning HTTP "200 OK" responses in all other cases, rather than 404 responses for unknown methods, 500 responses for invalid parameters, etc.
+- Returning either "result" fields or "error" fields in JSON-RPC responses, rather than returning both fields with one field set to null.


nit instead of duplicating the section from doc/JSON-RPC-interface.md here, it seems better to link/refer to it. Otherwise, if the section is updated, this may go stale or must be updated at the same time.

maflcko · 2024-05-20T13:45:30Z

test/functional/interface_rpc.py

-    except JSONRPCException as exc:
-        assert_equal(exc.error["code"], expected_rpc_code)
-        assert_equal(exc.http_status, expected_http_status)
+RPC_INVALID_ADDRESS_OR_KEY = -5


Looks like this is unused?

pinheadmz mentioned this pull request Feb 14, 2023

Support JSON-RPC 2.0 #2960

Closed

pinheadmz force-pushed the jsonrpc-2.0 branch from d21b25b to 440d6a2 Compare February 14, 2023 19:21

ajtowns reviewed Feb 16, 2023

View reviewed changes

src/rpc/request.h Outdated Show resolved Hide resolved

pinheadmz force-pushed the jsonrpc-2.0 branch 3 times, most recently from 97af16b to b139110 Compare February 27, 2023 21:24

pinheadmz force-pushed the jsonrpc-2.0 branch from b139110 to 814c3c1 Compare March 1, 2023 18:48

pinheadmz force-pushed the jsonrpc-2.0 branch 4 times, most recently from ec2b51b to d7a87a1 Compare March 1, 2023 21:25

stickies-v reviewed Mar 2, 2023

View reviewed changes

pinheadmz force-pushed the jsonrpc-2.0 branch from d7a87a1 to 2da8231 Compare March 2, 2023 15:29

pinheadmz marked this pull request as ready for review March 2, 2023 16:38

pinheadmz mentioned this pull request Mar 8, 2023

doc: document json rpc endpoints #27225

Merged

willcl-ark approved these changes Mar 21, 2023

View reviewed changes

pinheadmz force-pushed the jsonrpc-2.0 branch 2 times, most recently from 5388efb to c9a3862 Compare March 22, 2023 18:43

This was referenced Mar 24, 2023

Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet #27279

Merged

Move IsDeprecatedRPCEnabled to rpc/util, rm redundant rpcEnableDeprecated #27322

Closed

pinheadmz mentioned this pull request Mar 27, 2023

Inconsistent RPC error handling #19087

Open

pinheadmz force-pushed the jsonrpc-2.0 branch from c9a3862 to a754467 Compare March 28, 2023 18:16

pinheadmz force-pushed the jsonrpc-2.0 branch from a754467 to b583b39 Compare March 28, 2023 18:23

DrahtBot requested review from vincenzopalazzo and removed request for vincenzopalazzo March 8, 2024 01:24

scarletquasar approved these changes Mar 22, 2024

View reviewed changes

DrahtBot mentioned this pull request Apr 24, 2024

JSON-RPC request Content-Type is application/json #29946

Open

tdb3 mentioned this pull request Apr 28, 2024

RFC: Formal description of the RPC API #29912

Open

pinheadmz force-pushed the jsonrpc-2.0 branch from 58cb22c to 8583c2f Compare May 1, 2024 15:10

cbergqvist reviewed May 8, 2024

View reviewed changes

DrahtBot requested review from ryanofsky and tdb3 May 8, 2024 13:43

ryanofsky approved these changes May 8, 2024

View reviewed changes

pinheadmz added 5 commits May 14, 2024 10:32

rpc: identify JSON-RPC 2.0 requests

2ca1460

rpc: Add "jsonrpc" field and drop null "result"/"error" fields

466b905

Only for JSON-RPC 2.0 requests.

rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests

bf1a1f1

Avoid returning HTTP status errors for non-batch JSON-RPC 2.0 requests if the RPC method failed but the HTTP request was otherwise valid. Batch requests already did not return HTTP errors previously.

doc: add comments and release-notes for JSON-RPC 2.0

cbc6c44

pinheadmz force-pushed the jsonrpc-2.0 branch from 8583c2f to cbc6c44 Compare May 14, 2024 17:22

cbergqvist reviewed May 15, 2024

View reviewed changes

cbergqvist approved these changes May 15, 2024

View reviewed changes

DrahtBot requested a review from ryanofsky May 15, 2024 13:04

ryanofsky approved these changes May 15, 2024

View reviewed changes

tdb3 approved these changes May 16, 2024

View reviewed changes

ryanofsky merged commit 75118a6 into bitcoin:master May 16, 2024
16 checks passed

maflcko reviewed May 20, 2024

View reviewed changes

	std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id)
	{
	UniValue reply = JSONRPCReplyObj(result, error, id);
	return reply.write() + "\n";
	}

		@@ -11,6 +11,11 @@

		#include <univalue.h>

		enum class JSONVersion {

Support JSON-RPC 2.0 when requested by client #27101

Support JSON-RPC 2.0 when requested by client #27101

Conversation

pinheadmz commented Feb 14, 2023 • edited

DrahtBot commented Feb 14, 2023 • edited

Code Coverage

Reviews

Conflicts

ajtowns left a comment

Choose a reason for hiding this comment

fanquake commented Feb 20, 2023

sipa commented Feb 27, 2023

willcl-ark commented Feb 28, 2023

pinheadmz commented Mar 1, 2023

stickies-v left a comment

Choose a reason for hiding this comment

pinheadmz commented Mar 20, 2023

willcl-ark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinheadmz commented Mar 28, 2023

pinheadmz commented May 1, 2024

cbergqvist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanofsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinheadmz commented May 14, 2024

Choose a reason for hiding this comment

cbergqvist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanofsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanofsky commented May 15, 2024

tdb3 commented May 15, 2024

tdb3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinheadmz commented Feb 14, 2023 •

edited

DrahtBot commented Feb 14, 2023 •

edited