test: Fix intermittent issue in wallet_assumeutxo.py #34728

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2603-test-wallet-assume-sync changing 2 files +19 −23

maflcko commented at 8:37 AM on March 4, 2026: member

The test has many issues:

It fails intermittently, due to the use of -stopatheight (https://github.com/bitcoin/bitcoin/issues/34710)
Using -stopatheight is expensive, because it requires two restarts, making the test slow
The overwritten def setup_network does not store the extra args via the add_nodes call, so if code is added in the future to restart a node, it may not pick up its global extra args.

Fix all issues by:

Adding and using a fast dumb_sync_blocks util helper to achieve what -stopatheight doesn't achieve
Calling self.add_nodes(self.num_nodes, self.extra_args) in the overridden setup_network

Can be tested via this diff:

diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp
index ab0e5ccb69..49384c10b8 100644
--- a/src/node/kernel_notifications.cpp
+++ b/src/node/kernel_notifications.cpp
@@ -61,2 +61,3 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state
     if (m_stop_at_height && index.nHeight >= m_stop_at_height) {
+        LogInfo("Send shutdown signal after reaching stop height");
         if (!m_shutdown_request()) {
diff --git a/src/util/tokenpipe.cpp b/src/util/tokenpipe.cpp
index c982fa6fc4..a5565ebd36 100644
--- a/src/util/tokenpipe.cpp
+++ b/src/util/tokenpipe.cpp
@@ -4,2 +4,3 @@
 #include <util/tokenpipe.h>
+#include <util/time.h>
 
@@ -60,2 +61,3 @@ int TokenPipeEnd::TokenRead()
         ssize_t result = read(m_fd, &token, 1);
+        UninterruptibleSleep(500ms);
         if (result < 0) {

On master: The test fails On this pull: The test passes

Fixes https://github.com/bitcoin/bitcoin/issues/34710

DrahtBot added the label Tests on Mar 4, 2026
DrahtBot commented at 8:38 AM on March 4, 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 w0xlt, kevkevinpal, achow101

Stale ACK optout21

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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #34425 (test: Fix all races after a socket is closed gracefully by maflcko)
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.
DrahtBot added the label CI failed on Mar 4, 2026
DrahtBot removed the label CI failed on Mar 4, 2026
maflcko added this to the milestone 31.0 on Mar 4, 2026
in test/functional/test_framework/util.py:727 in fadcf5b986 outdated
```
 720 | @@ -721,6 +721,16 @@ def find_vout_for_address(node, txid, addr):
 721 |      raise RuntimeError("Vout not found for address: txid=%s, addr=%s" % (txid, addr))
 722 |  
 723 |  
 724 | +def dumb_sync_blocks(*, src, dst, height=None):
 725 | +    """Sync blocks between `src` and `dst` nodes via RPC submitblock up to height."""
 726 | +    height = height or src.getblockcount()
 727 | +    for i in range(dst.getblockcount() + 1, height + 1):
```
optout21 commented at 11:33 AM on March 4, 2026:

Nit: This method assumes that dst.getblockcount() <= src.getblockcount(). If that's not the case, the for loop will be a no-op, and the last assert will fail. Maybe it's worth to add an assert for this precondition at the top.

maflcko commented at 12:13 PM on March 4, 2026:

I am confident a single assert is necessary and sufficient to surface such logic bugs reliably and clearly. So I'll leave this as-is for now.

However, I am happy to push any patch, if there is a reason for it. (Ideally with exact steps to reproduce and the output before/after)
optout21 commented at 11:34 AM on March 4, 2026: contributor

utACK fadcf5b9860306d10eed75b78c7fbc5fa34ff31a

Reproduced the failure on master reliably with the path in the description (extra sleep). On the PR branch could not reproduce the failure (with or without patch). Verified by code review and unit tests.

Type	Reviewers
ACK	w0xlt, kevkevinpal, achow101
Stale ACK	optout21

in test/functional/wallet_assumeutxo.py:245 in fadcf5b986

w0xlt commented at 4:23 PM on March 4, 2026:

With dumb_sync_blocks, the height should be exactly PAUSE_HEIGHT.

         # The background chainstate should still be at START_HEIGHT
         assert_equal(chainstates[0]['blocks'], START_HEIGHT)
-        # The snapshot chainstate should be at least PAUSE_HEIGHT. It may be
-        # higher because stopatheight may allow additional blocks to be
-        # processed during shutdown (per stopatheight documentation).
-        assert_greater_than_or_equal(chainstates[1]['blocks'], PAUSE_HEIGHT)
+        # The snapshot chainstate should be at PAUSE_HEIGHT
+        assert_equal(chainstates[1]['blocks'], PAUSE_HEIGHT)

         # After restart, wallets that existed before cannot be loaded because
         # the wallet loading code checks if required blocks are available for

maflcko commented at 4:43 PM on March 4, 2026:

Nice. Removed the outdated comment, thx!

test: Fix intermittent issue in wallet_assumeutxo.py faa68ed4bd
maflcko force-pushed on Mar 4, 2026
w0xlt commented at 4:45 PM on March 4, 2026: contributor

ACK faa68ed4bdce6540f99d23c9b200ca575794a1b2
DrahtBot requested review from optout21 on Mar 4, 2026
kevkevinpal commented at 4:46 PM on March 4, 2026: contributor

ACK faa68ed

I was able to observe the failure on the master branch and was unable to reproduce the error on faa68ed
fanquake commented at 5:20 PM on March 4, 2026: member

Can't seem to reproduce this on 30.x, with the supplied diff, so I don't think this needs backporting?
maflcko commented at 5:24 PM on March 4, 2026: member

Can't seem to reproduce this on 30.x, with the supplied diff, so I don't think this needs backporting?

This only happens after commit 5cd57943b8adc76ed0b8a75a83f27bc0f971cbef (in 31.x)
achow101 commented at 6:50 PM on March 4, 2026: member

ACK faa68ed4bdce6540f99d23c9b200ca575794a1b2
achow101 merged this on Mar 4, 2026
achow101 closed this on Mar 4, 2026
maflcko deleted the branch on Mar 4, 2026
fjahr commented at 8:20 PM on March 4, 2026: contributor

post-merge ACK faa68ed4bdce6540f99d23c9b200ca575794a1b2