Skip to content

Conversation

@furszy
Copy link
Member

@furszy furszy commented Aug 22, 2023

Aiming to fix #25652.

The failure arises because the test expects init_wallet() (the test framework function) to create a wallet with no keys. However, the function also imports the deterministic private key used to receive the coinbase coins.

This causes a race within the "restore using dumped wallet" case, where we intend to have a new wallet (with no existing keys) to test the 'importwallet()' RPC result.
The reason why this failure is intermittent is that it depends on other peers delivering the chain right after node2 startup and prior to the test 'node2.getbalance()' call and also the synchronization of the validation queue.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 22, 2023

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 MarcoFalke

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

…mped wallet"

The failure arises because the test expects 'init_wallet()' (the test
framework function) creating a wallet with no keys. However, the function
also imports the deterministic private key used to receive the coinbase coins.

This causes a race within the "restore using dumped wallet" case, where we
intend to have a new wallet (with no existing keys) to test the
'importwallet()' RPC result.
The reason behind the intermittent failures might be other peers delivering
the chain right after node2 startup (sync of the validation queue included)
and prior to the 'node2.getbalance()' check.
@furszy furszy force-pushed the 2023_fix_race_wallet_backup_test branch from c9cea1a to c4929cf Compare August 23, 2023 12:42
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm ACK c4929cf

@maflcko maflcko added this to the 26.0 milestone Aug 23, 2023
@fanquake fanquake merged commit e8989f2 into bitcoin:master Aug 24, 2023
@furszy furszy deleted the 2023_fix_race_wallet_backup_test branch August 24, 2023 13:10
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Sep 8, 2023
… in "restore using dumped wallet"

c4929cf test: wallet_backup.py, fix intermittent failure in "restore using dumped wallet" (furszy)

Pull request description:

  Aiming to fix bitcoin#25652.

  The failure arises because the test expects `init_wallet()` (the test framework function) to create a wallet with no keys. However, the function also imports the deterministic private key used to receive the coinbase coins.

  This causes a race within the "restore using dumped wallet" case, where we intend to have a new wallet (with no existing keys) to test the 'importwallet()' RPC result.
  The reason why this failure is intermittent is that it depends on other peers delivering the chain right after node2 startup and prior to the test 'node2.getbalance()' call and also the synchronization of the validation queue.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK c4929cf

Tree-SHA512: 80faa590439305576086a7d6e328f2550c97b218771fc5eba0567feff78732a2605d028a30a368d50944ae3d25fdbd6d321fb97321791a356416f2b790999613
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Aug 14, 2024
…mped wallet"

Summary:
```
The failure arises because the test expects 'init_wallet()' (the test
framework function) creating a wallet with no keys. However, the function also imports the deterministic private key used to receive the coinbase coins.

This causes a race within the "restore using dumped wallet" case, where we intend to have a new wallet (with no existing keys) to test the 'importwallet()' RPC result.
The reason behind the intermittent failures might be other peers delivering the chain right after node2 startup (sync of the validation queue included) and prior to the 'node2.getbalance()' check.
```

Backport of [[bitcoin/bitcoin#28325 | core#28325]].

Test Plan:
  ./test/functional/test_runner.py wallet_backup

Reviewers: #bitcoin_abc, bytesofman

Reviewed By: #bitcoin_abc, bytesofman

Differential Revision: https://reviews.bitcoinabc.org/D16643
@bitcoin bitcoin locked and limited conversation to collaborators Aug 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wallet_backup.py fails with AssertionError: not(50 == 0) [assert_equal(self.nodes[2].getbalance(), 0)]

4 participants