test: wallet_backup.py, fix intermittent failure in "restore using dumped wallet" #28325

1. 
   furszy commented at 3:17 PM on August 22, 2023: member

   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.

2. 
   DrahtBot commented at 3:18 PM on August 22, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--021abf342d371248e50ceaed478a90ca-->

   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.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   No conflicts as of last run.

3. DrahtBot added the label Tests on Aug 22, 2023
4. test: wallet_backup.py, fix intermittent failure in "restore using dumped 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.

   c4929cfa50
5. in test/functional/wallet_backup.py:232 in c9cea1af52 outdated

    224 | @@ -230,7 +225,11 @@ def run_test(self):
    225 |              shutil.rmtree(os.path.join(self.nodes[2].chain_path, 'chainstate'))
    226 |  
    227 |              self.start_three(["-nowallet"])
    228 | -            self.init_three()
    229 | +            # Create new wallets for the three nodes.
    230 | +            # Provide 'import_deterministic_coinbase_key=False' to not import the deterministic
    231 | +            # coinbase key. We will use this empty wallets to test the 'importwallet()' RPC command below.
    232 | +            for node_num in range(3):
    233 | +                self.init_wallet(node=node_num, import_deterministic_coinbase_key=False)
   

   ---

   ---

   

   maflcko commented at 9:02 AM on August 23, 2023:

                   self.nodes[node_num].createwallet(wallet_name=self.default_wallet_name, descriptors=self.options.descriptors, load_on_startup=True)
   

   self.default_wallet_name is already used/hard-coded elsewhere in this test, so it seems clearer to use it here as well. This would also allow to drop the changes you made to the init_wallet function?

   ---

   

   furszy commented at 12:42 PM on August 23, 2023:

   sure. Pushed.

6. furszy force-pushed on Aug 23, 2023
7. maflcko approved
8. 
   maflcko commented at 12:51 PM on August 23, 2023: member

   lgtm ACK c4929cfa50ddb12943198a7f45723eedbd087d8f

9. maflcko added this to the milestone 26.0 on Aug 23, 2023
10. fanquake merged this on Aug 24, 2023
11. fanquake closed this on Aug 24, 2023

    ---

12. furszy deleted the branch on Aug 24, 2023
13. Frank-GER referenced this in commit 104055b102 on Sep 8, 2023
14. Fabcien referenced this in commit 2996a50a6e on Aug 14, 2024
15. bitcoin locked this on Aug 23, 2024

Contributors

furszyDrahtBotmaflcko

Milestone
26.0