Repository: bitcoin/bitcoin
File: test/functional/interface_rpc.py (Python)

Code before:
        self.setup_clean_chain = True
        self.supports_cli = False

    def test_getrpcinfo(self):
        self.log.info("Testing getrpcinfo...")

        info = self.nodes[0].getrpcinfo()
        assert_equal(len(info['active_commands']), 1)

        command = info['active_commands'][0]
        assert_equal(command['method'], 'getrpcinfo')
        assert_greater_than_or_equal(command['duration'], 0)
        assert_equal(info['logpath'], os.path.join(self.nodes[0].chain_path, 'debug.log'))

    def test_batch_request(self):
        self.log.info("Testing basic JSON-RPC batch request...")

        results = self.nodes[0].batch([
            # A basic request that will work fine.
            {"method": "getblockcount", "id": 1},
            # Request that will fail.  The whole batch request should still
            # work fine.
            {"method": "invalidmethod", "id": 2},
            # Another call that should succeed.
            {"method": "getblockhash", "id": 3, "params": [0]},
        ])

        result_by_id = {}
        for res in results:
            result_by_id[res["id"]] = res

        assert_equal(result_by_id[1]['error'], None)
        assert_equal(result_by_id[1]['result'], 0)

        assert_equal(result_by_id[2]['error']['code'], -32601)
        assert_equal(result_by_id[2]['result'], None)

        assert_equal(result_by_id[3]['error'], None)
        assert result_by_id[3]['result'] is not None

    def test_http_status_codes(self):
        self.log.info("Testing HTTP status codes for JSON-RPC requests...")

        expect_http_status(404, -32601, self.nodes[0].invalidmethod)
        expect_http_status(500, -8, self.nodes[0].getblockhash, 42)

    def test_work_queue_exceeded(self):
        self.log.info("Testing work queue exceeded...")
        self.restart_node(0, ['-rpcworkqueue=1', '-rpcthreads=1'])
        got_exceeded_error = []
        threads = []

Review comment (bug):
In commit "test: enable JSON-RPC 2.0 calls in tests and verify expected behavior" (65c2f4448dc556e7a922ebac4abed7ddfc8d7745)

This test code is broken because the requests do not have unique ids. So when the responses come back, the later responses override earlier ones in the `result_by_id` array, the existing test cases without a version fields will not be checked, and any result they return will be treated as valid.

I also think copying and pasting existing test cases for version 1.1 and version 2.0 makes this code harder to understand and less maintainable. The differences between versions should be small so IMO it would be better to just add a version argument (or version options like https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1430169572) to the test_batch_request method and call it 3 times. The other advantage of making the version variable is that it will make it more obvious what the differences between versions actually are. For example it is easy to miss the fact that 1.1 and 2.0 test cases are actually passing the version number in two different fields.

Code after:
        request.update(jsonrpc="2.0")
    elif options.version is not None:
        raise NotImplementedError(f"Unknown JSONRPC version {options.version}")
    if not options.notification:
        request.update(id=idx)
    request.update(fields)
    if options.request_fields:
        request.update(options.request_fields)
    return request


def format_response(options, idx, fields):
    if options.version == 2 and options.notification:
        return None
    response = {}
    if not options.notification:
        response.update(id=idx)
    if options.version == 2:
        response.update(jsonrpc="2.0")
    else:
        response.update(result=None, error=None)
    response.update(fields)
    if options.response_fields:
        response.update(options.response_fields)
    return response


def send_raw_rpc(node, raw_body: bytes) -> tuple[object, int]:
    return node._request("POST", "/", raw_body)


def send_json_rpc(node, body: object) -> tuple[object, int]:
    raw = json.dumps(body).encode("utf-8")
    return send_raw_rpc(node, raw)


def expect_http_rpc_status(expected_http_status, expected_rpc_error_code, node, method, params, version=1, notification=False):
    req = format_request(BatchOptions(version, notification), 0, {"method": method, "params": params})
    response, status = send_json_rpc(node, req)

    if expected_rpc_error_code is not None:
        assert_equal(response["error"]["code"], expected_rpc_error_code)

    assert_equal(status, expected_http_status)


def test_work_queue_getblock(node, got_exceeded_error):
    while not got_exceeded_error:
        try:
            node.cli("waitfornewblock", "500").send_cli()
        except subprocess.CalledProcessError as e: