Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#30238: json-rpc 2.0 followups: docs, tests, cli
Browse files Browse the repository at this point in the history
1f6ab12 minor: remove unnecessary semicolons from RPC content type examples (Matthew Zipkin)
b225295 test: use json-rpc 2.0 in all functional tests by default (Matthew Zipkin)
391843b bitcoin-cli: use json-rpc 2.0 (Matthew Zipkin)
d39bdf3 test: remove unused variable in interface_rpc.py (Matthew Zipkin)
0ead71d doc: update and link for JSON-RPC 2.0 (Matthew Zipkin)

Pull request description:

  This is a follow-up to #27101.

  - Addresses [post-merge comments ](bitcoin/bitcoin#27101 (comment))
  - bitcoin-cli uses JSON-RPC 2.0
  - functional tests use JSON-RPC 2.0 by default (exceptions are in the regression tests added by #27101)

ACKs for top commit:
  tdb3:
    ACK 1f6ab12
  cbergqvist:
    ACK 1f6ab12

Tree-SHA512: 49bf14c70464081280216ece538a2f5ec810bac80a86a83ad3284f0f1b017edf755a1a74a45be279effe00218170cafde7c2de58aed07097a95c2c6b837a6b6c
  • Loading branch information
fanquake committed Jun 8, 2024
2 parents 2ad6e8e + 1f6ab12 commit a44b0f7
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 30 deletions.
8 changes: 4 additions & 4 deletions doc/JSON-RPC-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ requests when multiple wallets are in use.

```sh
# Get block count from the / endpoint when rpcuser=alice and rpcport=38332
$ curl --user alice --data-binary '{"jsonrpc": "1.0", "id": "0", "method": "getblockcount", "params": []}' -H 'content-type: application/json;' localhost:38332/
$ curl --user alice --data-binary '{"jsonrpc": "2.0", "id": "0", "method": "getblockcount", "params": []}' -H 'content-type: application/json' localhost:38332/

# Get balance from the /wallet/walletname endpoint when rpcuser=alice, rpcport=38332 and rpcwallet=desc-wallet
$ curl --user alice --data-binary '{"jsonrpc": "1.0", "id": "0", "method": "getbalance", "params": []}' -H 'content-type: application/json;' localhost:38332/wallet/desc-wallet
$ curl --user alice --data-binary '{"jsonrpc": "2.0", "id": "0", "method": "getbalance", "params": []}' -H 'content-type: application/json' localhost:38332/wallet/desc-wallet

```

Expand Down Expand Up @@ -80,15 +80,15 @@ The server recognizes [JSON-RPC v2.0](https://www.jsonrpc.org/specification) req
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.
protocol in v27.0 and prior releases.

|| 1.1 | 2.0 |
|-|-|-|
| Request marker | `"version": "1.1"` (or none) | `"jsonrpc": "2.0"` |
| Response marker | (none) | `"jsonrpc": "2.0"` |
| `"error"` and `"result"` fields in response | both present | only one is present |
| HTTP codes in response | `200` unless there is any kind of RPC error (invalid parameters, method not found, etc) | Always `200` unless there is an actual HTTP server error (request parsing error, endpoint not found, etc) |
| Notifications: requests that get no reply | (not supported) | Supported for requests that exclude the "id" field |
| Notifications: requests that get no reply | (not supported) | Supported for requests that exclude the "id" field. Returns HTTP status `204` "No Content" |

## Security

Expand Down
7 changes: 2 additions & 5 deletions doc/release-notes-27101.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,5 @@ JSON-RPC
--------

The JSON-RPC server now recognizes JSON-RPC 2.0 requests and responds with
strict adherence to the specification (https://www.jsonrpc.org/specification):

- 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.
strict adherence to the [specification](https://www.jsonrpc.org/specification).
See [JSON-RPC-interface.md](/doc/JSON-RPC-interface.md#json-rpc-11-vs-20) for details.
8 changes: 4 additions & 4 deletions src/bitcoin-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ class AddrinfoRequestHandler : public BaseRequestHandler
}
addresses.pushKV("total", total);
result.pushKV("addresses_known", std::move(addresses));
return JSONRPCReplyObj(std::move(result), NullUniValue, /*id=*/1, JSONRPCVersion::V1_LEGACY);
return JSONRPCReplyObj(std::move(result), NullUniValue, /*id=*/1, JSONRPCVersion::V2);
}
};

Expand Down Expand Up @@ -367,7 +367,7 @@ class GetinfoRequestHandler: public BaseRequestHandler
}
result.pushKV("relayfee", batch[ID_NETWORKINFO]["result"]["relayfee"]);
result.pushKV("warnings", batch[ID_NETWORKINFO]["result"]["warnings"]);
return JSONRPCReplyObj(std::move(result), NullUniValue, /*id=*/1, JSONRPCVersion::V1_LEGACY);
return JSONRPCReplyObj(std::move(result), NullUniValue, /*id=*/1, JSONRPCVersion::V2);
}
};

Expand Down Expand Up @@ -622,7 +622,7 @@ class NetinfoRequestHandler : public BaseRequestHandler
}
}

return JSONRPCReplyObj(UniValue{result}, NullUniValue, /*id=*/1, JSONRPCVersion::V1_LEGACY);
return JSONRPCReplyObj(UniValue{result}, NullUniValue, /*id=*/1, JSONRPCVersion::V2);
}

const std::string m_help_doc{
Expand Down Expand Up @@ -709,7 +709,7 @@ class GenerateToAddressRequestHandler : public BaseRequestHandler
UniValue result(UniValue::VOBJ);
result.pushKV("address", address_str);
result.pushKV("blocks", reply.get_obj()["result"]);
return JSONRPCReplyObj(std::move(result), NullUniValue, /*id=*/1, JSONRPCVersion::V1_LEGACY);
return JSONRPCReplyObj(std::move(result), NullUniValue, /*id=*/1, JSONRPCVersion::V2);
}
protected:
std::string address_str;
Expand Down
1 change: 1 addition & 0 deletions src/rpc/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params,
request.pushKV("method", strMethod);
request.pushKV("params", params);
request.pushKV("id", id);
request.pushKV("jsonrpc", "2.0");
return request;
}

Expand Down
1 change: 1 addition & 0 deletions src/rpc/request.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ enum class JSONRPCVersion {
V2
};

/** JSON-RPC 2.0 request, only used in bitcoin-cli **/
UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id);
UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONRPCVersion jsonrpc_version);
UniValue JSONRPCError(int code, const std::string& message);
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,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\": \"2.0\", \"id\": \"curltest\", "
"\"method\": \"" + methodname + "\", \"params\": [" + args + "]}' -H 'content-type: application/json;' http://127.0.0.1:8332/\n";
"\"method\": \"" + methodname + "\", \"params\": [" + args + "]}' -H 'content-type: application/json' http://127.0.0.1:8332/\n";
}

std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList& args)
Expand All @@ -187,7 +187,7 @@ std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList&
}

return "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", "
"\"method\": \"" + methodname + "\", \"params\": " + params.write() + "}' -H 'content-type: application/json;' http://127.0.0.1:8332/\n";
"\"method\": \"" + methodname + "\", \"params\": " + params.write() + "}' -H 'content-type: application/json' http://127.0.0.1:8332/\n";
}

// Converts a hex string to a public key if possible
Expand Down
6 changes: 3 additions & 3 deletions src/test/rpc_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ BOOST_AUTO_TEST_CASE(help_example)
// test different argument types
const RPCArgList& args = {{"foo", "bar"}, {"b", true}, {"n", 1}};
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", args), "> bitcoin-cli -named test foo=bar b=true n=1\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", args), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"foo\":\"bar\",\"b\":true,\"n\":1}}' -H 'content-type: application/json;' http://127.0.0.1:8332/\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", args), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"foo\":\"bar\",\"b\":true,\"n\":1}}' -H 'content-type: application/json' http://127.0.0.1:8332/\n");

// test shell escape
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", {{"foo", "b'ar"}}), "> bitcoin-cli -named test foo='b'''ar'\n");
Expand All @@ -565,15 +565,15 @@ BOOST_AUTO_TEST_CASE(help_example)
obj_value.pushKV("b", false);
obj_value.pushKV("n", 1);
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", {{"name", obj_value}}), "> bitcoin-cli -named test name='{\"foo\":\"bar\",\"b\":false,\"n\":1}'\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", obj_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":{\"foo\":\"bar\",\"b\":false,\"n\":1}}}' -H 'content-type: application/json;' http://127.0.0.1:8332/\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", obj_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":{\"foo\":\"bar\",\"b\":false,\"n\":1}}}' -H 'content-type: application/json' http://127.0.0.1:8332/\n");

// test array params
UniValue arr_value(UniValue::VARR);
arr_value.push_back("bar");
arr_value.push_back(false);
arr_value.push_back(1);
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", {{"name", arr_value}}), "> bitcoin-cli -named test name='[\"bar\",false,1]'\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", arr_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":[\"bar\",false,1]}}' -H 'content-type: application/json;' http://127.0.0.1:8332/\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", arr_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":[\"bar\",false,1]}}' -H 'content-type: application/json' http://127.0.0.1:8332/\n");

// test types don't matter for shell
BOOST_CHECK_EQUAL(HelpExampleCliNamed("foo", {{"arg", true}}), HelpExampleCliNamed("foo", {{"arg", "true"}}));
Expand Down
1 change: 0 additions & 1 deletion test/functional/interface_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import subprocess


RPC_INVALID_ADDRESS_OR_KEY = -5
RPC_INVALID_PARAMETER = -8
RPC_METHOD_NOT_FOUND = -32601
RPC_INVALID_REQUEST = -32600
Expand Down
35 changes: 24 additions & 11 deletions test/functional/test_framework/authproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
- HTTP connections persist for the life of the AuthServiceProxy object
(if server supports HTTP/1.1)
- sends protocol 'version', per JSON-RPC 1.1
- sends "jsonrpc":"2.0", per JSON-RPC 2.0
- sends proper, incrementing 'id'
- sends Basic HTTP authentication headers
- parses all JSON numbers that look like floats as Decimal
Expand Down Expand Up @@ -117,23 +117,36 @@ def get_request(self, *args, **argsn):
params = dict(args=args, **argsn)
else:
params = args or argsn
return {'version': '1.1',
return {'jsonrpc': '2.0',
'method': self._service_name,
'params': params,
'id': AuthServiceProxy.__id_count}

def __call__(self, *args, **argsn):
postdata = json.dumps(self.get_request(*args, **argsn), default=serialization_fallback, ensure_ascii=self.ensure_ascii)
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
if response['error'] is not None:
raise JSONRPCException(response['error'], status)
elif 'result' not in response:
raise JSONRPCException({
'code': -343, 'message': 'missing JSON-RPC result'}, status)
elif status != HTTPStatus.OK:
raise JSONRPCException({
'code': -342, 'message': 'non-200 HTTP status code but no JSON-RPC error'}, status)
# For backwards compatibility tests, accept JSON RPC 1.1 responses
if 'jsonrpc' not in response:
if response['error'] is not None:
raise JSONRPCException(response['error'], status)
elif 'result' not in response:
raise JSONRPCException({
'code': -343, 'message': 'missing JSON-RPC result'}, status)
elif status != HTTPStatus.OK:
raise JSONRPCException({
'code': -342, 'message': 'non-200 HTTP status code but no JSON-RPC error'}, status)
else:
return response['result']
else:
assert response['jsonrpc'] == '2.0'
if status != HTTPStatus.OK:
raise JSONRPCException({
'code': -342, 'message': 'non-200 HTTP status code'}, status)
if 'error' in response:
raise JSONRPCException(response['error'], status)
elif 'result' not in response:
raise JSONRPCException({
'code': -343, 'message': 'missing JSON-RPC 2.0 result and error'}, status)
return response['result']

def batch(self, rpc_call_list):
Expand All @@ -142,7 +155,7 @@ def batch(self, rpc_call_list):
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
if status != HTTPStatus.OK:
raise JSONRPCException({
'code': -342, 'message': 'non-200 HTTP status code but no JSON-RPC error'}, status)
'code': -342, 'message': 'non-200 HTTP status code'}, status)
return response

def _get_response(self):
Expand Down

0 comments on commit a44b0f7

Please sign in to comment.