Skip to content

QA: Fix race condition in wallet_encryption test#16420

Merged
fanquake merged 1 commit intobitcoin:masterfrom
jonasschnelli:2019/07/wallet_enc_test_fix
Jul 19, 2019
Merged

QA: Fix race condition in wallet_encryption test#16420
fanquake merged 1 commit intobitcoin:masterfrom
jonasschnelli:2019/07/wallet_enc_test_fix

Conversation

@jonasschnelli
Copy link
Copy Markdown
Contributor

There is some imprecision probably in the internal HTTPRPCTimer class (haven't exactly figured out where).
But we can't expect that waiting excatly 2 seconds right after calling walletpassphrase(2) will result in a locked wallet due to the nature how we internally handle threads/timers.

The wallet_encryption test fails regularely in CIs.

Here is a logged session:

�[0;34m node0 2019-07-18T18:51:22.569739Z [] ThreadRPCServer method=walletpassphrase user=__cookie__ �[0m
�[0;34m node0 2019-07-18T18:51:22.628656Z [] queue run of timer lockwallet() in 2 seconds (using HTTP) �[0m
�[0;34m node0 2019-07-18T18:51:22.629002Z [] Received a POST request for / from 127.0.0.1:46898 �[0m
�[0;34m node0 2019-07-18T18:51:22.629081Z [] ThreadRPCServer method=dumpprivkey user=__cookie__ �[0m
�[0;34m node0 2019-07-18T18:51:24.445620Z [] Flushing wallet.dat �[0m
�[0;34m node0 2019-07-18T18:51:24.451421Z [] Flushed wallet.dat 6ms �[0m
�[0;34m node0 2019-07-18T18:51:24.631703Z [] Received a POST request for / from 127.0.0.1:46898 �[0m
�[0;34m node0 2019-07-18T18:51:24.631737Z [] ThreadRPCServer method=dumpprivkey user=__cookie__ �[0m
�[0;36m test  2019-07-18T18:51:24.632000Z TestFramework (ERROR): Assertion failed �[0m
�[0;36m                                   Traceback (most recent call last):�[0m
�[0;36m                                     File "/home/ubuntu/src/test/functional/test_framework/test_framework.py", line 193, in main�[0m
�[0;36m                                       self.run_test()�[0m
�[0;36m                                     File "/home/ubuntu/src/test/functional/wallet_encryption.py", line 53, in run_test�[0m
�[0;36m                                       assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].dumpprivkey, address)�[0m

@promag
Copy link
Copy Markdown
Contributor

promag commented Jul 18, 2019

ACK 024ecd7, simple fix, one second shouldn't hurt.

An alternative is to lower to 1 second the lock timeout in line 48:

self.nodes[0].walletpassphrase(passphrase, 1)

A more robust alternative is to:

  • wait_until with a predicate that returns True when walletpassphrase raises an exception;
  • and make wait_until return the elapsed time so that the test could assert it is greater than 2 seconds.

@maflcko
Copy link
Copy Markdown
Member

maflcko commented Jul 18, 2019

ACK 024ecd7

Copy link
Copy Markdown
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 024ecd7

@fanquake fanquake merged commit 024ecd7 into bitcoin:master Jul 19, 2019
fanquake added a commit that referenced this pull request Jul 19, 2019
024ecd7 QA: Fix race condition in wallet_encryption test (Jonas Schnelli)

Pull request description:

  There is some imprecision probably in the internal HTTPRPCTimer class (haven't exactly figured out where).
  But we can't expect that waiting excatly 2 seconds right after calling `walletpassphrase(2)` will result in a locked wallet due to the nature how we internally handle threads/timers.

  The wallet_encryption test fails regularely in CIs.

  Here is a logged session:
  ```shell
  �[0;34m node0 2019-07-18T18:51:22.569739Z [] ThreadRPCServer method=walletpassphrase user=__cookie__ �[0m
  �[0;34m node0 2019-07-18T18:51:22.628656Z [] queue run of timer lockwallet() in 2 seconds (using HTTP) �[0m
  �[0;34m node0 2019-07-18T18:51:22.629002Z [] Received a POST request for / from 127.0.0.1:46898 �[0m
  �[0;34m node0 2019-07-18T18:51:22.629081Z [] ThreadRPCServer method=dumpprivkey user=__cookie__ �[0m
  �[0;34m node0 2019-07-18T18:51:24.445620Z [] Flushing wallet.dat �[0m
  �[0;34m node0 2019-07-18T18:51:24.451421Z [] Flushed wallet.dat 6ms �[0m
  �[0;34m node0 2019-07-18T18:51:24.631703Z [] Received a POST request for / from 127.0.0.1:46898 �[0m
  �[0;34m node0 2019-07-18T18:51:24.631737Z [] ThreadRPCServer method=dumpprivkey user=__cookie__ �[0m
  �[0;36m test  2019-07-18T18:51:24.632000Z TestFramework (ERROR): Assertion failed �[0m
  �[0;36m                                   Traceback (most recent call last):�[0m
  �[0;36m                                     File "/home/ubuntu/src/test/functional/test_framework/test_framework.py", line 193, in main�[0m
  �[0;36m                                       self.run_test()�[0m
  �[0;36m                                     File "/home/ubuntu/src/test/functional/wallet_encryption.py", line 53, in run_test�[0m
  �[0;36m                                       assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].dumpprivkey, address)�[0m
  ```

ACKs for top commit:
  promag:
    ACK 024ecd7, simple fix, one second shouldn't hurt.
  MarcoFalke:
    ACK 024ecd7
  fanquake:
    ACK 024ecd7

Tree-SHA512: 0cda1b8969b084bb765d2b35e90a8611c565ee458a7be1f2dde675f8ddbd9b9e421514547a7683f836e2c996e0538eb66b8c5b935b5a81e9319fb2be27624374
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
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.

4 participants