mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-03-05 19:26:17 +00:00
Merge bitcoin/bitcoin#34728: test: Fix intermittent issue in wallet_assumeutxo.py
faa68ed4bdce6540f99d23c9b200ca575794a1b2 test: Fix intermittent issue in wallet_assumeutxo.py (MarcoFalke)
Pull request description:
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
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
ACKs for top commit:
kevkevinpal:
ACK [faa68ed](faa68ed4bd)
achow101:
ACK faa68ed4bdce6540f99d23c9b200ca575794a1b2
w0xlt:
ACK faa68ed4bdce6540f99d23c9b200ca575794a1b2
Tree-SHA512: 6fcd52b6f6a5fa5a5e41e7b3cf5c733af62af9c60271e7d22c266aca90f2af67f91ffe80a3ed8b8d1a91d001700870f493207998bac988c4e3671a3a0dba7ba7
This commit is contained in:
commit
4c40a923f0
@ -721,6 +721,16 @@ def find_vout_for_address(node, txid, addr):
|
||||
raise RuntimeError("Vout not found for address: txid=%s, addr=%s" % (txid, addr))
|
||||
|
||||
|
||||
def dumb_sync_blocks(*, src, dst, height=None):
|
||||
"""Sync blocks between `src` and `dst` nodes via RPC submitblock up to height."""
|
||||
height = height or src.getblockcount()
|
||||
for i in range(dst.getblockcount() + 1, height + 1):
|
||||
block_hash = src.getblockhash(i)
|
||||
block = src.getblock(blockhash=block_hash, verbose=0)
|
||||
dst.submitblock(block)
|
||||
assert_equal(dst.getblockcount(), height)
|
||||
|
||||
|
||||
def sync_txindex(test_framework, node):
|
||||
test_framework.log.debug("Waiting for node txindex to sync")
|
||||
sync_start = int(time.time())
|
||||
|
||||
@ -12,8 +12,8 @@ from test_framework.messages import COIN
|
||||
from test_framework.util import (
|
||||
assert_equal,
|
||||
assert_greater_than,
|
||||
assert_greater_than_or_equal,
|
||||
assert_raises_rpc_error,
|
||||
dumb_sync_blocks,
|
||||
ensure_for,
|
||||
)
|
||||
from test_framework.wallet import MiniWallet
|
||||
@ -42,8 +42,8 @@ class AssumeutxoTest(BitcoinTestFramework):
|
||||
def setup_network(self):
|
||||
"""Start with the nodes disconnected so that one can generate a snapshot
|
||||
including blocks the other hasn't yet seen."""
|
||||
self.add_nodes(4)
|
||||
self.start_nodes(extra_args=self.extra_args)
|
||||
self.add_nodes(self.num_nodes, self.extra_args)
|
||||
self.start_nodes()
|
||||
|
||||
def import_descriptor(self, node, wallet_name, key, timestamp):
|
||||
import_request = [{"desc": descsum_create("pkh(" + key.pubkey + ")"),
|
||||
@ -227,32 +227,18 @@ class AssumeutxoTest(BitcoinTestFramework):
|
||||
|
||||
PAUSE_HEIGHT = FINAL_HEIGHT - 40
|
||||
|
||||
self.log.info("Restarting node to stop at height %d", PAUSE_HEIGHT)
|
||||
self.restart_node(1, extra_args=[
|
||||
f"-stopatheight={PAUSE_HEIGHT}", *self.extra_args[1]])
|
||||
self.log.info(f"Unload wallets and sync node up to height {PAUSE_HEIGHT}")
|
||||
n1.unloadwallet("w")
|
||||
n1.unloadwallet(wallet_name)
|
||||
dumb_sync_blocks(src=n0, dst=n1, height=PAUSE_HEIGHT)
|
||||
|
||||
# Finally connect the nodes and let them sync.
|
||||
#
|
||||
# Set `wait_for_connect=False` to avoid a race between performing connection
|
||||
# assertions and the -stopatheight tripping.
|
||||
self.connect_nodes(0, 1, wait_for_connect=False)
|
||||
|
||||
n1.wait_until_stopped(timeout=5)
|
||||
|
||||
self.log.info(
|
||||
"Restarted node before snapshot validation completed, reloading...")
|
||||
self.restart_node(1, extra_args=self.extra_args[1])
|
||||
|
||||
self.log.info("Verify node state after restart during background sync")
|
||||
self.log.info("Verify node state during background sync")
|
||||
# Verify there are still two chainstates (background validation not complete)
|
||||
chainstates = n1.getchainstates()['chainstates']
|
||||
assert_equal(len(chainstates), 2)
|
||||
# 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)
|
||||
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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user