rpc: JSON-RPC 2.0 should not respond to "notifications"

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.

1 files changed, 27 insertions, 4 deletions

diff --git a/src/httprpc.cpp b/src/httprpc.cpp
index 777ad32bbe..3eb34dbe6a 100644
--- a/src/httprpc.cpp
+++ b/src/httprpc.cpp
@@ -211,6 +211,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
             const bool catch_errors{jreq.m_json_version == JSONRPCVersion::V2};
             reply = JSONRPCExec(jreq, catch_errors);
 
+            if (jreq.IsNotification()) {
+                // Even though we do execute notifications, we do not respond to them
+                req->WriteReply(HTTP_NO_CONTENT);
+                return true;
+            }
+
         // array of requests
         } else if (valRequest.isArray()) {
             // Check authorization for each request's method
@@ -235,15 +241,32 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
             reply = UniValue::VARR;
             for (size_t i{0}; i < valRequest.size(); ++i) {
                 // Batches never throw HTTP errors, they are always just included
-                // in "HTTP OK" responses.
+                // in "HTTP OK" responses. Notifications never get any response.
+                UniValue response;
                 try {
                     jreq.parse(valRequest[i]);
-                    reply.push_back(JSONRPCExec(jreq, /*catch_errors=*/true));
+                    response = JSONRPCExec(jreq, /*catch_errors=*/true);
                 } catch (UniValue& e) {
-                    reply.push_back(JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version));
+                    response = 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));
+                    response = JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version);
                 }
+                if (!jreq.IsNotification()) {
+                    reply.push_back(std::move(response));
+                }
+            }
+            // Return no response for an all-notification batch, but only if the
+            // batch request is non-empty. Technically according to the JSON-RPC
+            // 2.0 spec, an empty batch request should also return no response,
+            // However, if the batch request is empty, it means the request did
+            // not contain any JSON-RPC version numbers, so returning an empty
+            // response could break backwards compatibility with old RPC clients
+            // relying on previous behavior. Return an empty array instead of an
+            // empty response in this case to favor being backwards compatible
+            // over complying with the JSON-RPC 2.0 spec in this case.
+            if (reply.size() == 0 && valRequest.size() > 0) {
+                req->WriteReply(HTTP_NO_CONTENT);
+                return true;
             }
         }
         else