test: ipc should reject invalid json #36093

Sjors commented at 3:51 PM on August 26, 2026: member

Suggested (by me) as a followup for #36088: #36088 (review)

Reverting that PR will cause the test to fail.

The first refactor commit changes assert_capnp_failed to assert_capnp_raises to reduce repetition.

DrahtBot added the label Tests on Aug 26, 2026

DrahtBot commented at 3:51 PM on August 26, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36093.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	hodlinator, jeanpablojp

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

template.submitSolution(ctx, 0, 0, 0, b"\x00") in test/functional/interface_ipc_mining.py

fanquake requested review from ryanofsky on Aug 26, 2026

maflcko commented at 7:31 PM on August 26, 2026: member

Hmm, corecheck doesn't detect new coverage. Maybe it should compile with the dev-mode preset? cc @m3dwards

in test/functional/interface_ipc.py:67 in abd24ace1b

  62 | +            self.log.debug("Invalid JSON in an executeRpc request must be rejected")
  63 | +            try:
  64 | +                await rpc.executeRpc(ctx, "invalid json", "/", "")
  65 | +                raise AssertionError("executeRpc unexpectedly succeeded")
  66 | +            except capnp.KjException as e:
  67 | +                assert_capnp_failed(e, "remote exception: std::exception: invalid JSON received over IPC")

hodlinator commented at 1:01 PM on August 27, 2026:

This pattern would become repeated in 5 places. Maybe assert_capnp_failed() could be turned into assert_capnp_raises()?

diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 119591a675..3e2f64fcac 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -10,7 +10,7 @@ from contextlib import ExitStack
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 from test_framework.ipc_util import (
-    assert_capnp_failed,
+    assert_capnp_raises,
     load_capnp_modules,
     make_capnp_init_ctx,
     make_mining_ctx,
@@ -60,11 +60,8 @@ class IPCInterfaceTest(BitcoinTestFramework):
             self.log.debug("Create Rpc proxy object")
             rpc = init.makeRpc(ctx).result
             self.log.debug("Invalid JSON in an executeRpc request must be rejected")
-            try:
-                await rpc.executeRpc(ctx, "invalid json", "/", "")
-                raise AssertionError("executeRpc unexpectedly succeeded")
-            except capnp.KjException as e:
-                assert_capnp_failed(e, "remote exception: std::exception: invalid JSON received over IPC")
+            await assert_capnp_raises(lambda: rpc.executeRpc(ctx, "invalid json", "/", ""),
+                                      "remote exception: std::exception: invalid JSON received over IPC")
             self.log.debug("The connection still works after the failed call")
             request = json.dumps({"method": "getblockcount", "params": [], "id": 1})
             response = json.loads((await rpc.executeRpc(ctx, request, "/", "")).result)
diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py
index c36d73ab0a..05dc75a532 100755
--- a/test/functional/interface_ipc_mining.py
+++ b/test/functional/interface_ipc_mining.py
@@ -37,7 +37,7 @@ from test_framework.util import (
 from test_framework.wallet import MiniWallet
 from test_framework.p2p import P2PInterface
 from test_framework.ipc_util import (
-    assert_capnp_failed,
+    assert_capnp_raises,
     assert_create_new_block_fails,
     destroying,
     load_capnp_modules,
@@ -523,11 +523,8 @@ class IPCMiningTest(BitcoinTestFramework):
                 assert_equal(submitted, False)
 
                 self.log.debug("Submit solution that can't be deserialized")
-                try:
-                    await template.submitSolution(ctx, 0, 0, 0, b"\x00")
-                    raise AssertionError("submitSolution unexpectedly succeeded")
-                except capnp.lib.capnp.KjException as e:
-                    assert_capnp_failed(e, "remote exception: std::exception: SpanReader::read(): end of data:")
+                await assert_capnp_raises(lambda: template.submitSolution(ctx, 0, 0, 0, b"\x00"),
+                                          "remote exception: std::exception: SpanReader::read(): end of data:")
 
                 self.log.debug("Submit a block with a bad version")
                 block.nVersion = 0
@@ -712,18 +709,12 @@ class IPCMiningTest(BitcoinTestFramework):
                 )
 
             self.log.debug("Submit a malformed complete block")
-            try:
-                await mining2.submitBlock(ctx2, block.serialize()[:-15])
-                raise AssertionError("submitBlock unexpectedly succeeded")
-            except capnp.lib.capnp.KjException as e:
-                assert_capnp_failed(e, "remote exception: std::exception: SpanReader::read(): end of data:")
+            await assert_capnp_raises(lambda: mining2.submitBlock(ctx2, block.serialize()[:-15]),
+                                      "remote exception: std::exception: SpanReader::read(): end of data:")
 
             self.log.debug("Submit empty block data")
-            try:
-                await mining2.submitBlock(ctx2, b"")
-                raise AssertionError("submitBlock unexpectedly succeeded")
-            except capnp.lib.capnp.KjException as e:
-                assert_capnp_failed(e, "remote exception: std::exception: SpanReader::read(): end of data:")
+            await assert_capnp_raises(lambda: mining2.submitBlock(ctx2, b""),
+                                      "remote exception: std::exception: SpanReader::read(): end of data:")
             assert_equal(self.nodes[2].is_node_stopped(), False)
 
         asyncio.run(capnp.run(async_routine()))
diff --git a/test/functional/test_framework/ipc_util.py b/test/functional/test_framework/ipc_util.py
index a4ebd091e8..4a221ed7d7 100644
--- a/test/functional/test_framework/ipc_util.py
+++ b/test/functional/test_framework/ipc_util.py
@@ -159,15 +159,17 @@ async def make_mining_ctx(self, node_index=0):
     mining = init.makeMining(ctx).result
     return ctx, mining
 
-def assert_capnp_failed(e, description_prefix):
-    assert e.description.startswith(description_prefix), f"Expected description starting with '{description_prefix}', got '{e.description}'"
-    assert_equal(e.type, "FAILED")
+
+async def assert_capnp_raises(fun, description_prefix):
+    try:
+        await fun()
+        raise AssertionError(f"Function unexpectedly succeeded without raising {description_prefix!r}")
+    except capnp.lib.capnp.KjException as e:
+        assert e.description.startswith(description_prefix), f"Expected description starting with '{description_prefix}', got '{e.description}'"
+        assert_equal(e.type, "FAILED")
 
 
 async def assert_create_new_block_fails(ctx, mining, opts, expected_msg):
     """Assert that mining.createNewBlock fails with the expected remote exception."""
-    try:
-        await mining.createNewBlock(ctx, opts)
-        raise AssertionError("createNewBlock unexpectedly succeeded")
-    except capnp.lib.capnp.KjException as e:
-        assert_capnp_failed(e, f"remote exception: std::exception: {expected_msg}")
+    await assert_capnp_raises(lambda: mining.createNewBlock(ctx, opts),
+                              f"remote exception: std::exception: {expected_msg}")

</details>

Sjors commented at 7:32 AM on August 28, 2026:

Taken.

hodlinator approved

hodlinator commented at 1:07 PM on August 27, 2026: contributor

ACK abd24ace1bbc146789593959af15258b854932a9

m3dwards commented at 2:52 PM on August 27, 2026: contributor

Hmm, corecheck doesn't detect new coverage. Maybe it should compile with the dev-mode preset? cc @m3dwards

I've added the pycapnp package to corecheck to enable the interface_ipc.py functional test and can see it being run and now appearing in the coverage checking. Do we think this is enough?

2026-08-27T14:44:59.692Z 256/299 - interface_ipc.py passed, Duration: 1 s

test: refactor Cap'n Proto exception assertions

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>

858814146d

test: ipc should reject invalid json

Suggested as a followup for #36088:
https://github.com/bitcoin/bitcoin/pull/36088#discussion_r3864044274

f14569f97d

Sjors force-pushed on Aug 28, 2026

Sjors commented at 7:33 AM on August 28, 2026: member

Added @hodlinator's suggested refactor as a prep commit, see #36093 (review).

maflcko commented at 9:50 AM on August 28, 2026: member

Nice. corecheck now shows a green line in "Gained baseline coverage"

hodlinator approved

hodlinator commented at 11:12 AM on August 28, 2026: contributor

re-ACK f14569f97d081c6d34353bdf22e901a9baf92ef3

Thanks for taking the test refactor suggestion. Re-ran functional tests.

jeanpablojp commented at 2:11 PM on August 29, 2026: contributor

tACK f14569f97d081c6d34353bdf22e901a9baf92ef3

I reverted #36088 and the test fails. It also fails with only the throw removed, where all unit tests still pass. And changing the SpanReader message makes the rewritten assertions fail too.