Merge bitcoin/bitcoin#27101: Support JSON-RPC 2.0 when requested by client

cbc6c440e3811d342fa570713702900b3e3e75b9 doc: add comments and release-notes for JSON-RPC 2.0 (Matthew Zipkin)
e7ee80dcf2b68684eae96070875ea13a60e3e7b0 rpc: JSON-RPC 2.0 should not respond to "notifications" (Matthew Zipkin)
bf1a1f1662427fbf1a43bb951364eface469bdb7 rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests (Matthew Zipkin)
466b90562f4785de74b548f7c4a256069e2aaf43 rpc: Add "jsonrpc" field and drop null "result"/"error" fields (Matthew Zipkin)
2ca1460ae3a7217eaa8c5972515bf622bedadfce rpc: identify JSON-RPC 2.0 requests (Matthew Zipkin)
a64a2b77e09bff784a2635ba19ff4aa6582bb5a5 rpc: refactor single/batch requests (Matthew Zipkin)
df6e3756d6feaf1856e7886820b70874209fd90b rpc: Avoid copies in JSONRPCReplyObj() (Matthew Zipkin)
09416f9ec445e4d6bb277400758083b0b4e8b174 test: cover JSONRPC 2.0 requests, batches, and notifications (Matthew Zipkin)
4202c170da37a3203e05a9f39f303d7df19b6d81 test: refactor interface_rpc.py (Matthew Zipkin)

Pull request description:

  Closes https://github.com/bitcoin/bitcoin/issues/2960

  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)

  https://github.com/bitcoin/bitcoin/pull/15495 added regression tests after a discussion in https://github.com/bitcoin/bitcoin/pull/15381 to kinda lock in our RPC behavior to preserve backwards compatibility.

  https://github.com/bitcoin/bitcoin/pull/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](https://www.jsonrpc.org/specification#request_object) 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](https://www.jsonrpc.org/specification#response_object), 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.

ACKs for top commit:
  cbergqvist:
    re ACK cbc6c440e3811d342fa570713702900b3e3e75b9
  ryanofsky:
    Code review ACK cbc6c440e3811d342fa570713702900b3e3e75b9. 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.
  tdb3:
    re ACK for cbc6c440e3811d342fa570713702900b3e3e75b9

Tree-SHA512: 0b702ed32368b34b29ad570d090951a7aeb56e3b0f2baf745bd32fdc58ef68fee6b0b8fad901f1ca42573ed714b150303829cddad4a34ca7ad847350feeedb36

6 files changed, 77 insertions, 48 deletions

diff --git a/src/rpc/protocol.h b/src/rpc/protocol.h
index 75e42e4c88..83a9010681 100644
--- a/src/rpc/protocol.h
+++ b/src/rpc/protocol.h
@@ -10,6 +10,7 @@
 enum HTTPStatusCode
 {
     HTTP_OK                    = 200,
+    HTTP_NO_CONTENT            = 204,
     HTTP_BAD_REQUEST           = 400,
     HTTP_UNAUTHORIZED          = 401,
     HTTP_FORBIDDEN             = 403,
diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp
index b7acd62ee3..d35782189e 100644
--- a/src/rpc/request.cpp
+++ b/src/rpc/request.cpp
@@ -26,6 +26,17 @@
  *
  * 1.0 spec: http://json-rpc.org/wiki/specification
  * 1.2 spec: http://jsonrpc.org/historical/json-rpc-over-http.html
+ *
+ * If the server receives a request with the JSON-RPC 2.0 marker `{"jsonrpc": "2.0"}`
+ * then Bitcoin will respond with a strictly specified response.
+ * It will only return an HTTP error code if an actual HTTP error is encountered
+ * such as the endpoint is not found (404) or the request is not formatted correctly (500).
+ * Otherwise the HTTP code is always OK (200) and RPC errors will be included in the
+ * response body.
+ *
+ * 2.0 spec: https://www.jsonrpc.org/specification
+ *
+ * Also see http://www.simple-is-better.org/rpc/#differences-between-1-0-and-2-0
  */
 
 UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id)
@@ -37,24 +48,25 @@ UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params,
     return request;
 }
 
-UniValue JSONRPCReplyObj(const UniValue& result, const UniValue& error, const UniValue& id)
+UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONRPCVersion jsonrpc_version)
 {
     UniValue reply(UniValue::VOBJ);
-    if (!error.isNull())
-        reply.pushKV("result", NullUniValue);
-    else
-        reply.pushKV("result", result);
-    reply.pushKV("error", error);
-    reply.pushKV("id", id);
+    // Add JSON-RPC version number field in v2 only.
+    if (jsonrpc_version == JSONRPCVersion::V2) reply.pushKV("jsonrpc", "2.0");
+
+    // Add both result and error fields in v1, even though one will be null.
+    // Omit the null field in v2.
+    if (error.isNull()) {
+        reply.pushKV("result", std::move(result));
+        if (jsonrpc_version == JSONRPCVersion::V1_LEGACY) reply.pushKV("error", NullUniValue);
+    } else {
+        if (jsonrpc_version == JSONRPCVersion::V1_LEGACY) reply.pushKV("result", NullUniValue);
+        reply.pushKV("error", std::move(error));
+    }
+    if (id.has_value()) reply.pushKV("id", std::move(id.value()));
     return reply;
 }
 
-std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id)
-{
-    UniValue reply = JSONRPCReplyObj(result, error, id);
-    return reply.write() + "\n";
-}
-
 UniValue JSONRPCError(int code, const std::string& message)
 {
     UniValue error(UniValue::VOBJ);
@@ -171,7 +183,30 @@ void JSONRPCRequest::parse(const UniValue& valRequest)
     const UniValue& request = valRequest.get_obj();
 
     // Parse id now so errors from here on will have the id
-    id = request.find_value("id");
+    if (request.exists("id")) {
+        id = request.find_value("id");
+    } else {
+        id = std::nullopt;
+    }
+
+    // Check for JSON-RPC 2.0 (default 1.1)
+    m_json_version = JSONRPCVersion::V1_LEGACY;
+    const UniValue& jsonrpc_version = request.find_value("jsonrpc");
+    if (!jsonrpc_version.isNull()) {
+        if (!jsonrpc_version.isStr()) {
+            throw JSONRPCError(RPC_INVALID_REQUEST, "jsonrpc field must be a string");
+        }
+        // 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.
+        if (jsonrpc_version.get_str() == "1.0") {
+            m_json_version = JSONRPCVersion::V1_LEGACY;
+        } else if (jsonrpc_version.get_str() == "2.0") {
+            m_json_version = JSONRPCVersion::V2;
+        } else {
+            throw JSONRPCError(RPC_INVALID_REQUEST, "JSON-RPC version not supported");
+        }
+    }
 
     // Parse method
     const UniValue& valMethod{request.find_value("method")};
diff --git a/src/rpc/request.h b/src/rpc/request.h
index a682c58d96..e47f90af86 100644
--- a/src/rpc/request.h
+++ b/src/rpc/request.h
@@ -7,13 +7,18 @@
 #define BITCOIN_RPC_REQUEST_H
 
 #include <any>
+#include <optional>
 #include <string>
 
 #include <univalue.h>
 
+enum class JSONRPCVersion {
+    V1_LEGACY,
+    V2
+};
+
 UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id);
-UniValue JSONRPCReplyObj(const UniValue& result, const UniValue& error, const UniValue& id);
-std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id);
+UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONRPCVersion jsonrpc_version);
 UniValue JSONRPCError(int code, const std::string& message);
 
 /** Generate a new RPC authentication cookie and write it to disk */
@@ -28,7 +33,7 @@ std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in);
 class JSONRPCRequest
 {
 public:
-    UniValue id;
+    std::optional<UniValue> id = UniValue::VNULL;
     std::string strMethod;
     UniValue params;
     enum Mode { EXECUTE, GET_HELP, GET_ARGS } mode = EXECUTE;
@@ -36,8 +41,10 @@ public:
     std::string authUser;
     std::string peerAddr;
     std::any context;
+    JSONRPCVersion m_json_version = JSONRPCVersion::V1_LEGACY;
 
     void parse(const UniValue& valRequest);
+    [[nodiscard]] bool IsNotification() const { return !id.has_value() && m_json_version == JSONRPCVersion::V2; };
 };
 
 #endif // BITCOIN_RPC_REQUEST_H
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index a800451f4a..1ed406354a 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -360,36 +360,22 @@ bool IsDeprecatedRPCEnabled(const std::string& method)
     return find(enabled_methods.begin(), enabled_methods.end(), method) != enabled_methods.end();
 }
 
-static UniValue JSONRPCExecOne(JSONRPCRequest jreq, const UniValue& req)
-{
-    UniValue rpc_result(UniValue::VOBJ);
-
-    try {
-        jreq.parse(req);
-
-        UniValue result = tableRPC.execute(jreq);
-        rpc_result = JSONRPCReplyObj(result, NullUniValue, jreq.id);
-    }
-    catch (const UniValue& objError)
-    {
-        rpc_result = JSONRPCReplyObj(NullUniValue, objError, jreq.id);
-    }
-    catch (const std::exception& e)
-    {
-        rpc_result = JSONRPCReplyObj(NullUniValue,
-                                     JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
+UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors)
+{
+    UniValue result;
+    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_MISC_ERROR, e.what()), jreq.id, jreq.m_json_version);
+        }
+    } else {
+        result = tableRPC.execute(jreq);
     }
 
-    return rpc_result;
-}
-
-std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq)
-{
-    UniValue ret(UniValue::VARR);
-    for (unsigned int reqIdx = 0; reqIdx < vReq.size(); reqIdx++)
-        ret.push_back(JSONRPCExecOne(jreq, vReq[reqIdx]));
-
-    return ret.write() + "\n";
+    return JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id, jreq.m_json_version);
 }
 
 /**
diff --git a/src/rpc/server.h b/src/rpc/server.h
index b8348e4aa6..5735aff821 100644
--- a/src/rpc/server.h
+++ b/src/rpc/server.h
@@ -179,6 +179,6 @@ extern CRPCTable tableRPC;
 void StartRPC();
 void InterruptRPC();
 void StopRPC();
-std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq);
+UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors);
 
 #endif // BITCOIN_RPC_SERVER_H
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 9a7c731afe..f5a2e9eb63 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -175,7 +175,7 @@ std::string HelpExampleCliNamed(const std::string& methodname, const RPCArgList&
 
 std::string HelpExampleRpc(const std::string& methodname, const std::string& args)
 {
-    return "> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\": \"curltest\", "
+    return "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", "
         "\"method\": \"" + methodname + "\", \"params\": [" + args + "]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n";
 }
 
@@ -186,7 +186,7 @@ std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList&
         params.pushKV(param.first, param.second);
     }
 
-    return "> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\": \"curltest\", "
+    return "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", "
            "\"method\": \"" + methodname + "\", \"params\": " + params.write() + "}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n";
 }