Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Jul 2, 2020

Iterating all crypted keys might take time.

E.g.

 node0 2020-07-01T14:41:19.227367Z [httpworker.0] ThreadRPCServer method=walletpassphrase user=__cookie__ 
 node0 2020-07-01T14:41:24.377142Z [httpworker.0] queue run of timer lockwallet() in 100000000 seconds (using HTTP) 
...
 test  2020-07-01T14:41:24.379000Z TestFramework (ERROR): Assertion failed 
                                   Traceback (most recent call last):
                                     File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 117, in main
                                       self.run_test()
                                     File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_encryption.py", line 88, in run_test
                                       assert_greater_than(expected_time + 5, actual_time) # 5 second buffer
                                     File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 54, in assert_greater_than
                                       raise AssertionError("%s <= %s" % (str(thing1), str(thing2)))
                                   AssertionError: 1693614483 <= 1693614484

https://cirrus-ci.com/task/5322429885054976?command=ci#L4517

@fanquake fanquake added the Tests label Jul 2, 2020
@maflcko maflcko force-pushed the 2007-testFixInt branch from fa6e43d to fabd33b Compare July 2, 2020 00:08
@maflcko
Copy link
Member Author

maflcko commented Jul 2, 2020

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Concept ACK

expected_time = int(time.time()) + MAX_VALUE - 600
self.nodes[0].walletpassphrase(passphrase2, MAX_VALUE - 600)
# give buffer for walletpassphrase, since it iterates over all crypted keys
expected_time_with_buffer = time.time() + MAX_VALUE - 600
Copy link
Member

Choose a reason for hiding this comment

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

While touching this test, mind renaming MAX_VALUE to MAX_SLEEP_TIME (per src/wallet/rpcwallet.cpp:1884) and moving the definition up to the top of the file or using local variable naming, e.g. max_sleep_time.

Copy link
Member

Choose a reason for hiding this comment

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

There are several of both int(time.time()) and time.time() in the test. Standardize on one of the two?

Copy link
Member Author

Choose a reason for hiding this comment

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

They rounding needs to be exactly the way it is, otherwise the test wouldn't pass

@maflcko
Copy link
Member Author

maflcko commented Jul 14, 2020

@achow101 Mind taking a look here?

@achow101
Copy link
Member

ACK fabd33b

@maflcko maflcko merged commit f4de89e into bitcoin:master Jul 14, 2020
@maflcko maflcko deleted the 2007-testFixInt branch July 14, 2020 15:12
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 16, 2020
fabd33b test: Fix intermittent failure in wallet_encryption (MarcoFalke)

Pull request description:

  Iterating all crypted keys might take time.

  E.g.

  ```
   node0 2020-07-01T14:41:19.227367Z [httpworker.0] ThreadRPCServer method=walletpassphrase user=__cookie__
   node0 2020-07-01T14:41:24.377142Z [httpworker.0] queue run of timer lockwallet() in 100000000 seconds (using HTTP)
  ...
   test  2020-07-01T14:41:24.379000Z TestFramework (ERROR): Assertion failed
                                     Traceback (most recent call last):
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 117, in main
                                         self.run_test()
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_encryption.py", line 88, in run_test
                                         assert_greater_than(expected_time + 5, actual_time) # 5 second buffer
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 54, in assert_greater_than
                                         raise AssertionError("%s <= %s" % (str(thing1), str(thing2)))
                                     AssertionError: 1693614483 <= 1693614484
  ```

  https://cirrus-ci.com/task/5322429885054976?command=ci#L4517

ACKs for top commit:
  achow101:
    ACK fabd33b

Tree-SHA512: 7a3ccdfc0cdc05fef1f942d3167d100ed63422eb54c05405c884ed91162b7bdb5ce54cb5a981b99a6df2e4af1ea834ccd7d5156531c8c14ea13e735becd6b377
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
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