Repository: bitcoin/bitcoin
File: test/functional/wallet_assumeutxo.py (Python)

Code before:
        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]])

        # 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")
        # 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 PAUSE_HEIGHT or higher
        # (it may have synced more blocks before the stopatheight triggered)
        snapshot_height = chainstates[-1]['blocks']
        assert_greater_than_or_equal(snapshot_height, PAUSE_HEIGHT)

        # After restart, wallets that existed before cannot be loaded because
        # the wallet loading code checks if required blocks are available for
        # rescanning. During assumeutxo background sync, blocks before the
        # snapshot are not available, so wallet loading fails.
        # This applies to both wallet "w" (restored from backup) and wallet "w1"
        # (watch-only created at snapshot height).
        assert_raises_rpc_error(-4, "Wallet loading failed. Error loading wallet. Wallet requires blocks to be downloaded", n1.loadwallet, "w")
        assert_raises_rpc_error(-4, "Wallet loading failed. Error loading wallet. Wallet requires blocks to be downloaded", n1.loadwallet, wallet_name)

        # Verify backup from before snapshot height still can't be restored
        assert_raises_rpc_error(-4, "Wallet loading failed. Error loading wallet. Wallet requires blocks to be downloaded", n1.restorewallet, "w2_test", "backup_w2.dat")

        self.complete_background_validation(n1)

        self.log.info("Ensuring wallet can be restored from a backup that was created before the snapshot height")
        n1.restorewallet("w2", "backup_w2.dat")
        # Check balance of w2 wallet
        assert_equal(n1.getbalance(), 340)

        # Check balance of w wallet after node is synced
        n1.loadwallet("w")
        w = n1.get_wallet_rpc("w")
        assert_equal(w.getbalance(), 34)


Review comment (suggestion):
Since you are checking the length of `chainstates`, maybe it is clear to put the index `1` instead of `-1`. That way, I believe it is easier to see that you are accessing the second chainstate.

Are you sure the `assert_greater_than_or_equal` is needed? I don't see why `snapshot_height` would be higher. I have tried it several times to be sure, and it is always equal to `PAUSE_HEIGHT`. Maybe an `assert_equal` would be enough?

Since snapshot_height is just used here maybe we can put `chainstates[-1]['blocks']` directly into the assert.

```suggestion
        assert_equal(chainstates[1]['blocks'], PAUSE_HEIGHT)
```

Code after:

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

        # 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")
        # 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)

        # After restart, wallets that existed before cannot be loaded because
        # the wallet loading code checks if required blocks are available for
        # rescanning. During assumeutxo background sync, blocks before the
        # snapshot are not available, so wallet loading fails.
        # After restart, the required height is SNAPSHOT_BASE_HEIGHT + 1 for all wallets.
        assert_raises_rpc_error(-4, loading_error(SNAPSHOT_BASE_HEIGHT + 1), n1.loadwallet, "w")
        assert_raises_rpc_error(-4, loading_error(SNAPSHOT_BASE_HEIGHT + 1), n1.loadwallet, wallet_name)

        # Verify backup from before snapshot height still can't be restored
        assert_raises_rpc_error(-4, loading_error(SNAPSHOT_BASE_HEIGHT + 1), n1.restorewallet, "w2_test", "backup_w2.dat")

        self.complete_background_validation(n1)

        self.log.info("Ensuring wallet can be restored from a backup that was created before the snapshot height")
        n1.restorewallet("w2", "backup_w2.dat")
        # Check balance of w2 wallet
        assert_equal(n1.getbalance(), 340)

        # Check balance of w wallet after node is synced
        n1.loadwallet("w")
        w = n1.get_wallet_rpc("w")