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, sedited

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.

sedited approved

sedited commented at 1:26 PM on September 14, 2026: contributor

ACK f14569f97d081c6d34353bdf22e901a9baf92ef3

sedited merged this on Sep 14, 2026

sedited closed this on Sep 14, 2026

Sjors deleted the branch on Sep 14, 2026