test: Add IPC wake-up test and reuse mining context

enirox001 commented at 11:59 pm on March 3, 2026: contributor

This is a follow-up to implement a couple of test improvements discussed in recent IPC PRs and issues.

adds a test to interface_ipc_mining.py to verify that createNewBlock wakes up immediately when the tip advances, rather than waiting for the cooldown timer to expire (https://github.com/bitcoin/bitcoin/pull/34184#discussion_r2842239399).
moves make_mining_ctx into ipc_util.py so it can be reused across the IPC tests instead of duplicating the setup code (https://github.com/bitcoin/bitcoin/pull/34422#discussion_r2852445430).
adds a test case to verify that providing an invalid coinbase to submitSolution returns a remote exception instead of crashing the node, closing the loop on the issue reported in #33341.
scales IPC wait timeouts using the test suite’s timeout_factor to prevent spurious failures in heavily loaded CI environments, capping extended waits to avoid test runner hangs (bitcoin-core/libmultiprocess#253 (comment)).

Closes #33341.

DrahtBot added the label Tests on Mar 3, 2026

DrahtBot commented at 11:59 pm on March 3, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	sedited, Sjors, achow101
Concept ACK	ismaelsadeeq
Stale ACK	ryanofsky, w0xlt

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34860 (mining: always pad scriptSig at low heights, drop include_dummy_extranonce by Sjors)
#34644 (mining: add submitBlock to IPC Mining interface by w0xlt)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

# Assert this only occurs on Darwin until fixed. -> # Assert that this only occurs on Darwin until fixed.
Rationale: The imperative “Assert” is missing the “that …” clause, making the sentence grammatically incomplete and slightly harder to parse.

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):

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

Possible places where comparison-specific test macros should replace generic comparisons:

[test/functional/test_framework/ipc_util.py] assert e.description.startswith(description_prefix), f"Expected description starting with '{description_prefix}', got '{e.description}'" -> Replace the bare assert ...startswith(...) with assert_equal(e.description.startswith(description_prefix), True, f"...") (using the provided comparison helper).

2026-03-20 18:56:12

DrahtBot added the label CI failed on Mar 4, 2026

enirox001 marked this as a draft on Mar 4, 2026

enirox001 force-pushed on Mar 4, 2026

enirox001 marked this as ready for review on Mar 4, 2026

DrahtBot removed the label CI failed on Mar 4, 2026

sedited requested review from Sjors on Mar 8, 2026

enirox001 force-pushed on Mar 9, 2026

DrahtBot added the label CI failed on Mar 9, 2026

enirox001 force-pushed on Mar 9, 2026

enirox001 commented at 12:13 pm on March 9, 2026: contributor

Added a new test in https://github.com/bitcoin/bitcoin/commit/3477c4300c00877b9183d11cdb717306fa928c5d to verify that submitSolution correctly throws an exception instead of crashing when given an invalid coinbase. This covers the edge case discussed in #33341. Thanks @ryanofsky for the test snippet.

Also updated the PR description to match changes

DrahtBot removed the label CI failed on Mar 9, 2026

in test/functional/interface_ipc_mining.py:211 in 2ebb25b83e

206+                    res = await mining.createNewBlock(ctx, self.default_block_create_options)
207+                    results['duration'] = time.time() - bg_start
208+                    results['success'] = res._has("result")
209+                task = asyncio.create_task(create_block_with_timing())
210+                await asyncio.sleep(0.5)
211+                assert not task.done(), "createNewBlock() returned early — cooldown did not engage"

ryanofsky commented at 4:11 pm on March 9, 2026:

In commit “test: verify createNewBlock wakes promptly when tip advances” (2ebb25b83e94cd0bfadbfa70413353e18cb3dc0d)

I think I’d suggest dropping these two lines. Few reasons:

The main purpose of the test is making sure createNewBlock() wakes up immediately after sync, but this is testing something else: that createNewBlock() does not return too early before the expected cooldown time. It would probably better to have a separate test for this.
This unconditionally sleeps for a half second, which would be short for a timeout, but is long for an unconditional delay. This is not too bad, but having many delays like this slows down tests so it is good to avoid them where possible. A better way of testing this could be to use mock times.
It’s not documented why a 0.5 second delay is used here and it seems like the choice is closely tied to the cooldown implementation, so if the implementation is updated this might have to change in the future.
If we want to keep a test that createNewBlock does not return too early, it might be better to make it a unit test instead of an IPC test so it can be closer to the code, use mock times, and access the same constants.

enirox001 commented at 5:02 am on March 11, 2026:

Done. I dropped these lines to keep the test strictly focused on the tip-advance wake-up, eliminating the unconditional sleep

in test/functional/interface_ipc_mining.py:369 in 3477c4300c

364+                    raise AssertionError("submitSolution unexpectedly succeeded")
365+                except capnp.lib.capnp.KjException as e:
366+                    assert_equal(e.type, "FAILED")
367+                    assert any(err in e.description for err in [
368+                        "SpanReader::read(): end of data",
369+                        "unknown non-KJ exception"

ryanofsky commented at 4:17 pm on March 9, 2026:

This “unknown non-KJ exception” case should not actually be expected, and it would be nice if this could this could be made explicit like the other case where it occurs. https://github.com/bitcoin/bitcoin/blob/d3056bc149f605225f22b1cc83b1a2d1cea64258/test/functional/interface_ipc_mining.py#L316

Maybe we could have a helper function like:

0def assert_capnp_failed(e, description):
1    if e.description == "remote exception: unknown non-KJ exception of type: kj::Exception":
2        # macOS + REDUCE_EXPORTS bug: Cap'n Proto fails to recognize
3        # its own exception type and returns a generic error instead.
4        # [#34422 (review)](/bitcoin-bitcoin/34422/#discussion_r2863852691)
5        # Assert this only occurs on Darwin until fixed.
6        assert_equal(platform.system(), "Darwin")
7    else:
8        assert_equal(e.description, description)
9    assert_equal(e.type, "FAILED")

enirox001 commented at 5:04 am on March 11, 2026:

Added assert_capnp_failed to ipc_util.py so it’s available for future tests, and I also updated the older blockReservedWeight test above to use it to clean up the inline macOS logic.

in test/functional/interface_ipc_mining.py:215 in 2ebb25b83e outdated

210+                await asyncio.sleep(0.5)
211+                assert not task.done(), "createNewBlock() returned early — cooldown did not engage"
212+                block_hex = self.nodes[1].getblock(node1_block_hash, False)
213+                self.nodes[0].submitblock(block_hex)
214+                try:
215+                    await asyncio.wait_for(task, timeout=10.0)

ryanofsky commented at 4:21 pm on March 9, 2026:

In commit “test: verify createNewBlock wakes promptly when tip advances” (2ebb25b83e94cd0bfadbfa70413353e18cb3dc0d)

Can you document where this 10 second timeout comes from? How much of the timeout is the actual amount of time cooldown is expected to take, and how much is extra padding to make the test more reliable?

Also wondering if there might be a way to use mock times to test this

Sjors commented at 3:19 pm on March 10, 2026:

I would definitely suggest using mock time.

enirox001 commented at 5:05 am on March 11, 2026:

Done. I implemented @Sjors’s suggestion below to use setmocktime to freeze the node’s clock. This guarantees that createNewBlock only wakes up due to the tip advance. Kept the 10s timeout purely as an asyncio safety fallback for CI and added a comment documenting this

ryanofsky approved

ryanofsky commented at 4:27 pm on March 9, 2026: contributor

Code review ACK 3477c4300c00877b9183d11cdb717306fa928c5d. Nice changes, and thanks for following up on the previous PRs! I’m a little unsure about the cost/benefit of the new createNewBlock cooldown test and asked some questions, and would like @Sjors to weigh in on. But it seems reasonable and the other test and refactoring changes both look good

Sjors commented at 3:20 pm on March 10, 2026: member

Concept ACK

enirox001 force-pushed on Mar 11, 2026

DrahtBot added the label CI failed on Mar 11, 2026

enirox001 force-pushed on Mar 11, 2026

enirox001 commented at 5:30 am on March 11, 2026: contributor

Thanks for the reviews, @ryanofsky and @Sjors

I’ve pushed an update addressing the recent review feedback.

Dropped the unconditional sleep and used setmocktime to freeze the node’s clock, ensuring createNewBlock wakes up strictly due to the tip advance. Added a comment clarifying that the 10s timeout is just CI padding.
Moved the macOS exception workaround into a reusable assert_capnp_failed helper in ipc_util.py, and refactored the existing blockReservedWeight test to use it as well.

DrahtBot removed the label CI failed on Mar 11, 2026

in test/functional/interface_ipc_mining.py:214 in 251c9fe6ff

209+                    res = await mining.createNewBlock(ctx, self.default_block_create_options)
210+                    results['duration'] = time.time() - bg_start
211+                    results['success'] = res._has("result")
212+                task = asyncio.create_task(create_block_with_timing())
213+                block_hex = self.nodes[1].getblock(node1_block_hash, False)
214+                self.nodes[0].submitblock(block_hex)

Sjors commented at 2:26 pm on March 11, 2026:

251c9fe6ffd0a974f74ab91eb5fb06ef95532ff2: omitting this line does not cause the test to fail.

ryanofsky commented at 3:31 pm on March 11, 2026:

In commit “test: verify createNewBlock wakes promptly when tip advances” (251c9fe6ffd0a974f74ab91eb5fb06ef95532ff2)

re: #34727 (review)

251c9fe: omitting this line does not cause the test to fail.

Not sure but maybe the test is forgetting to disconnect the nodes, so the block is getting synced even though it is not submitted?

The test also seems off in a few other ways. For one thing, it would seem nice if the test could use the wait_and_do helper. I would think the createNewBlock call go in the async wait_fn and submitblock and setmocktime could go in a do_fn.

The comment about a 10-second timeout as padding doesn’t seem right to me, because I would expect that after the block is submitted, there is still an additional cooldown period like 3 seconds or whatever before createNewBlock returns, and the point of the mocktime call would be to advance by 3 seconds so the test does not need to wait.

It also doesn’t seem like there is a point to the time.time() calls and the ‘duration’ value seems to be set and never used. The result dict could also be replaced by using python’s nonlocal keyword.

Sorry for all the suggestions, it just seems like there a number of things that could be improved

enirox001 commented at 4:54 am on March 12, 2026:

Thanks for the suggestions! I’ve completely refactored this:

Applied wait_and_do and nonlocal to clean up the structure.
Removed the confusing 10-second padding comment.
Regarding mocktime/duration: I tested it, and the internal 3-second wait relies on the real-time system clock, so setmocktime doesn’t bypass it. Because the node naturally returns after 3 seconds anyway, I kept the time.time() check to assert duration < 3.0

in test/functional/test_framework/ipc_util.py:167 in 730b93ae92

158@@ -155,3 +159,14 @@ async def make_mining_ctx(self):
159         self.log.debug("Create Mining proxy object")
160         mining = init.makeMining(ctx).result
161         return ctx, mining
162+
163+def assert_capnp_failed(e, description_prefix):
164+    if e.description == "remote exception: unknown non-KJ exception of type: kj::Exception":
165+        # macOS + REDUCE_EXPORTS bug: Cap'n Proto fails to recognize
166+        # its own exception type and returns a generic error instead.
167+        # https://github.com/bitcoin/bitcoin/pull/34422#discussion_r2863852691

ryanofsky commented at 3:19 pm on March 11, 2026:

In commit “test: verify IPC error handling for invalid coinbase” (730b93ae9256881eb2b2eddfd4cc4549e2e08bfe)

It’d be good to change the URL to #34723 (this issue was created after the original comment was written)

enirox001 commented at 4:52 am on March 12, 2026:

Done. Updated the URL to #34723 in the recent push.

ryanofsky commented at 3:32 pm on March 11, 2026: contributor

Code review 730b93ae9256881eb2b2eddfd4cc4549e2e08bfe. Thanks for applying previous suggestions!

enirox001 force-pushed on Mar 12, 2026

DrahtBot added the label CI failed on Mar 12, 2026

enirox001 commented at 4:59 am on March 12, 2026: contributor

Thanks for the detailed review again

I pushed an update addressing the recent review feedback.

Regarding the changes i really wanted to use mocktime here as you both suggested, but I ran into a hurdle when I tested it

I tried advancing setmocktime by 3 seconds inside the do_fn to instantly bypass the wait. However, because the polling mechanism seems to use a real-time system wait. advancing the mocktime didn’t seem to wake up the sleeping thread. The node still waits the 3 seconds before naturally returning.

Because of this, a standard timeout wouldn’t catch a failure if the submitblock interrupt breaks (it would just pass after 3s). To make the test completely deterministic, I refactored it using wait_and_do and added an assertion to verify the real-world duration is < 3.0s.

If there is a way to making setmocktime trigger the cv wake-up from the test that I missed, please let me know and I will happily update it. Otherwise, the duration check seems to be a feasible way to verify the prompt wake-up.

enirox001 commented at 9:03 am on March 12, 2026: contributor

CI failure is unrelated

in test/functional/interface_ipc_mining.py:216 in 3d81147226

211+                def do_fn():
212+                    block_hex = self.nodes[1].getblock(node1_block_hash, False)
213+                    self.nodes[0].submitblock(block_hex)
214+                await wait_and_do(wait_fn(), do_fn)
215+                assert_equal(success, True)
216+                assert duration < 3.0, f"createNewBlock took {duration:.2f}s, did not wake up promptly after tip advances"

Sjors commented at 10:27 am on March 12, 2026:

3d81147226e49fa4980f9e609afec3d1d162d6c2: on my M4 MacBook Pro duration was 0.118s on average, of which 0.1 is from wait_and_do. So we have 3 / (0.118 - 0.1) = 160x margin for a very slow CI machine. So the odds a false positive failure seem acceptably low.

sedited commented at 9:13 pm on March 19, 2026:

Mmh, I’m not sure, but this still feels a bit tight to me. Why not scale this with the timeout factor too?

I’m also bit confused by the mention of the cooldown in the commit message, aren’t we passing cooldown=False?

Sjors commented at 12:11 pm on March 20, 2026:

@sedited 38fdf3304e606024980acf69538b4e8a5fb5d52e test: verify createNewBlock wakes promptly when tip advances: the goal of this test is to check that cooldown is not happening. If it takes longer than 3 seconds we can’t tell if cooldown happened or the node was extremely slow.

Another approach could be skip this test if the timeout factor is > 1.

enirox001 commented at 5:50 pm on March 20, 2026:

I’m also bit confused by the mention of the cooldown in the commit message, aren’t we passing cooldown=False? @sedited about the commit message. In mining.capnp, createNewBlock actually defaults to cooldown: Bool = true. We don’t override it in the Python call, so the cooldown is active. This test verifies that the tip advance successfully interrupts it.

Mmh, I’m not sure, but this still feels a bit tight to me. Why not scale this with the timeout factor too?

If we scale the 3.0s assertion, a broken tip-advance would just hit the 3s fallback sleep and falsely pass on scaled runs

I will push an update that wraps the duration < 3.0 check in if self.options.timeout_factor == 1: to prevent spurious failures on slow CI runners

Sjors commented at 6:11 pm on March 20, 2026:

Is Python equality as reliable as Javascript or will == 1 work fine?

enirox001 commented at 6:58 pm on March 20, 2026:

Thankfully Python’s equality is much saner than JavaScript’s, however, i’ve updated the logic to use <= 1 instead. This ensures the strict 3s check still runs on standard or high-performance machines

DrahtBot requested review from ryanofsky on Mar 12, 2026

Sjors approved

Sjors commented at 10:34 am on March 12, 2026: member

ACK 9b6985c1ed4c806d554e8a977afd9104c7665368

DrahtBot removed the label CI failed on Mar 12, 2026

in test/functional/test_framework/ipc_util.py:161 in 9b6985c1ed

156+async def make_mining_ctx(self):
157+        """Create IPC context and Mining proxy object."""
158+        ctx, init = await make_capnp_init_ctx(self)
159+        self.log.debug("Create Mining proxy object")
160+        mining = init.makeMining(ctx).result
161+        return ctx, mining

w0xlt commented at 7:52 pm on March 12, 2026:

Why the extra indentation ?

0async def make_mining_ctx(self):
1    """Create IPC context and Mining proxy object."""
2    ctx, init = await make_capnp_init_ctx(self)
3    self.log.debug("Create Mining proxy object")
4    mining = init.makeMining(ctx).result
5    return ctx, mining

enirox001 commented at 10:37 pm on March 12, 2026:

Nice catch regarding the formatting. Fixed the indentation

in test/functional/test_framework/ipc_util.py:171 in 9b6985c1ed

166+        # its own exception type and returns a generic error instead.
167+        # https://github.com/bitcoin/bitcoin/pull/34723
168+        # Assert this only occurs on Darwin until fixed.
169+        assert_equal(platform.system(), "Darwin")
170+    else:
171+        assert description_prefix in e.description, f"Expected '{description_prefix}' in '{e.description}'"

w0xlt commented at 8:17 pm on March 12, 2026:

This can be stricter.

0        assert e.description.startswith(description_prefix), f"Expected description starting with '{description_prefix}', got '{e.description}'"

enirox001 commented at 10:38 pm on March 12, 2026:

You’re right, startswith is much safer here. Applied the suggestion thanks!

w0xlt commented at 8:35 pm on March 12, 2026: contributor

ACK 9b6985c1ed4c806d554e8a977afd9104c7665368 mod minor nits above

Sjors commented at 9:42 pm on March 12, 2026: member

If you feel like adding another followup commit, it would be good to scale waits (waitNext, etc) with the test suite timeout scaling factor. For background see https://github.com/bitcoin-core/libmultiprocess/pull/253#issuecomment-4049705804

enirox001 commented at 10:29 pm on March 12, 2026: contributor

If you feel like adding another followup commit, it would be good to scale waits (waitNext, etc) with the test suite timeout scaling factor. For background see bitcoin-core/libmultiprocess#253 (comment)

Sounds good, would work to add a followup commit with these changes

enirox001 force-pushed on Mar 12, 2026

enirox001 force-pushed on Mar 13, 2026

DrahtBot added the label CI failed on Mar 13, 2026

in test/functional/interface_ipc_mining.py:135 in 25e6fdcaf5

131@@ -132,7 +132,7 @@ async def async_routine():
132 
133             self.log.debug("interrupt() should abort waitTipChanged()")
134             async def wait_for_tip():
135-                long_timeout = 60000.0  # 1 minute
136+                long_timeout = 60000.0 * self.options.timeout_factor  # 1 minute

Sjors commented at 10:22 am on March 13, 2026:

This might be a bit much, depending on the maximum timeout factor used in CI - it might exceed the test runner max run time. If so, we could e.g. use the max of 1000 * timeout, 60000.

enirox001 commented at 10:41 am on March 13, 2026:

You’re right, a massive timeout factor would definitely nuke the CI runner with a really long wait. I’ve updated this to use max(timeout, 60000.0) so it defaults to a safe 1 minute. I also applied the same max() fix to the timeout * 60 line later in the file to make sure it doesn’t cause the same hang.

enirox001 force-pushed on Mar 13, 2026

enirox001 commented at 10:49 am on March 13, 2026: contributor

Thanks for the feedback and reviews. I’ve pushed an update addressing the recent reviews:

Fixed the indentation in make_mining_ctx and updated the error assertion to use startswith
Scaled IPC wait timeouts using self.options.timeout_factor to resolve the CI brittleness mentioned in https://github.com/bitcoin-core/libmultiprocess/pull/253#issuecomment-4049705804
Bounded the extended waits using max(timeout, 60000.0) in both places so large timeout factors don’t cause the CI runner to hang and exceed its max time limit

Updated PR description as well

Sjors commented at 11:02 am on March 13, 2026: member

Thanks!

ACK 9a71d533b422f15c975ca79230cc1cee6e9c4236

DrahtBot requested review from w0xlt on Mar 13, 2026

DrahtBot removed the label CI failed on Mar 13, 2026

w0xlt commented at 10:27 pm on March 13, 2026: contributor

reACK https://github.com/bitcoin/bitcoin/commit/9a71d533b422f15c975ca79230cc1cee6e9c4236

Sjors commented at 8:18 am on March 18, 2026: member

cc @ismaelsadeeq

test: verify createNewBlock wakes promptly when tip advances

This adds a complementary test to interface_ipc_mining.py to ensure
that createNewBlock() wakes up immediately once submitblock advances
the tip, rather than needlessly waiting for the cooldown timer to
expire on its own.

4ada575d6c

test: move make_mining_ctx to ipc_util.py

The async routines in both interface_ipc.py and interface_ipc_mining.py
contain redundant code to initialize the mining proxy object.

Move the make_mining_ctx helper into test_framework/ipc_util.py and
update both test files to use it. This removes the boilerplate and
prevents code duplication across the IPC test suite.

63684d6922

test: verify IPC error handling for invalid coinbase

Add a test case to interface_ipc_mining.py to verify that the IPC
server correctly handles and reports serialization errors rather than
crashing the node.

This covers the scenario where submitSolution is called with data
that cannot be deserialized, as discussed in #33341

Also introduces the assert_capnp_failed helper in ipc_util.py to
cleanly handle macOS-specific Cap'n Proto exception strings, and
refactors an existing block weight test to use it.

e7a918b69a

test: scale IPC mining wait timeouts by timeout_factor

The IPC mining tests (interface_ipc_mining.py) currently use
hardcoded timeouts (e.g., 1000ms, 60000ms) for operations like
waitTipChanged and waiting for block templates. In heavily
loaded CI environments, such as those running sanitizers with
high parallelism, these hardcoded timeouts can be too short,
leading to spurious test failures and brittleness.

This commit multiplies these timeout variables by the test
suite's global `self.options.timeout_factor`. This ensures that
the IPC wait conditions scale appropriately when the test suite
is run with a higher timeout factor, making the tests robust
against slow execution environments.

Addresses CI brittleness observed in bitcoin-core/libmultiprocess#253.

ad75b147b5

enirox001 force-pushed on Mar 20, 2026

enirox001 commented at 7:00 pm on March 20, 2026: contributor

Pushed a recent update in https://github.com/bitcoin/bitcoin/commit/ad75b147b5c3ab5eac268a1c1ced23894a8a79ba

Wrapped the strict duration < 3.0 check in if self.options.timeout_factor <= 1:. This ensures we still verify the prompt wake-up on standard or high-performance machines, but skip it on heavily loaded CI runners to avoid spurious failures.

sedited approved

sedited commented at 8:20 am on March 23, 2026: contributor

ACK ad75b147b5c3ab5eac268a1c1ced23894a8a79ba

DrahtBot requested review from w0xlt on Mar 23, 2026

DrahtBot requested review from Sjors on Mar 23, 2026

fanquake added this to the milestone 31.0 on Mar 23, 2026

Sjors commented at 10:07 am on March 23, 2026: member

ACK ad75b147b5c3ab5eac268a1c1ced23894a8a79ba

ismaelsadeeq commented at 6:59 am on March 24, 2026: member

Concept ACK

achow101 commented at 8:18 pm on March 24, 2026: member

ACK ad75b147b5c3ab5eac268a1c1ced23894a8a79ba

DrahtBot requested review from achow101 on Mar 24, 2026

DrahtBot requested review from ismaelsadeeq on Mar 24, 2026

achow101 merged this on Mar 24, 2026

achow101 closed this on Mar 24, 2026

achow101 added the label Needs Backport (31.x) on Mar 24, 2026

fanquake removed the label Needs Backport (31.x) on Mar 25, 2026

fanquake commented at 0:49 am on March 25, 2026: member

Backported to 31.x in #34800.

fanquake referenced this in commit acd7e3d061 on Mar 25, 2026

fanquake referenced this in commit 660947352c on Mar 25, 2026

fanquake referenced this in commit 39c87621d5 on Mar 25, 2026

fanquake referenced this in commit ac13aca72b on Mar 25, 2026

test: Add IPC wake-up test and reuse mining context #34727

Reviews

Conflicts

LLM Linter (✨ experimental)