test: Bump walletpassphrase timeout to avoid intermittent issue #28089

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2307-test-int-time- changing 1 files +1 −1

maflcko commented at 1:46 PM on July 17, 2023: member

There is no risk increasing the timeout, but it avoids intermittent issues (when running in valgrind) of the form:

 node1 2023-07-15T06:17:37.827609Z [http] [httpserver.cpp:255] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:45950 
 node1 2023-07-15T06:17:37.833168Z [httpworker.3] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=signrawtransactionwithwallet user=__cookie__ 
 test  2023-07-15T06:17:38.037000Z TestFramework (ERROR): JSONRPC error 
                                   Traceback (most recent call last):
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
                                       self.run_test()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_fundrawtransaction.py", line 137, in run_test
                                       self.test_many_inputs_send()
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_fundrawtransaction.py", line 687, in test_many_inputs_send
                                       fundedAndSignedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex'])
                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 50, in __call__
                                       return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                     File "/root/b-c-ci/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 129, in __call__
                                       raise JSONRPCException(response['error'], status)
                                   test_framework.authproxy.JSONRPCException: Error: Please enter the wallet passphrase with walletpassphrase first. (-13)

test: Bump walletpassphrase timeout to avoid intermittent issue fa257a727c
DrahtBot commented at 1:46 PM on July 17, 2023: contributor


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 ishaanam

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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #28139 (test: create wallet specific for test_locked_wallet case by furszy)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot added the label Tests on Jul 17, 2023
in test/functional/wallet_fundrawtransaction.py:637 in fa257a727c
```
 630 | @@ -631,7 +631,7 @@ def test_locked_wallet(self):
 631 |          fundedTx = self.nodes[1].fundrawtransaction(rawtx)
 632 |  
 633 |          # Now we need to unlock.
 634 | -        self.nodes[1].walletpassphrase("test", 600)
 635 | +        self.nodes[1].walletpassphrase("test", 999999)
 636 |          signedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex'])
 637 |          self.nodes[1].sendrawtransaction(signedTx['hex'])
 638 |          self.generate(self.nodes[1], 1)
```
furszy commented at 2:14 PM on July 17, 2023:

This looks fishy. It seems that we unlock the wallet inside a specific test case, and use the unlocked period for other test cases.

A better approach would be to relock the wallet at the end of this test case ( self.nodes[1].walletlock()). And unlock-lock the wallet in the other cases when it is needed.

maflcko commented at 2:19 PM on July 17, 2023:

Sounds good. An alternative might be to not use a passphrase after this test. Do you want to submit either as a pull request? Happy to close mine then. Otherwise, I'll leave the feedback for a follow-up, as the goal here is to not change any logic, only the constant.

furszy commented at 2:44 PM on July 17, 2023:

An alternative might be to not use a passphrase after this test

Yeah. Would be even better to create a wallet specific to this test and unload it at the end of it.

Do you want to submit either as a pull request? Happy to close mine then. Otherwise, I'll leave the feedback for a follow-up, as the goal here is to not change any logic, only the constant.

Will try to do it later today. Shouldn't take me much. Still, all good if this one gets merged before it.
maflcko added this to the milestone 26.0 on Jul 19, 2023
ishaanam commented at 4:55 AM on July 23, 2023: contributor

ACK fa257a727c27a85cffaed94d1a241e631b2afda3

I would also ACK a PR that creates a wallet specific to this test as discussed above.
fanquake commented at 10:47 AM on July 25, 2023: member

I think this PR can now be closed given #28139?
maflcko commented at 11:11 AM on July 25, 2023: member

Thanks, yes.
maflcko closed this on Jul 25, 2023
maflcko deleted the branch on Jul 25, 2023
fanquake referenced this in commit f033a981ed on Jul 26, 2023
sidhujag referenced this in commit 747a4072fc on Aug 9, 2023
bitcoin locked this on Jul 24, 2024

Type	Reviewers
ACK	ishaanam

Contributors

Labels

Milestone
26.0

Linked (view graph)

#28139 test: create wallet specific for test_locked_wallet case

test: Bump walletpassphrase timeout to avoid intermittent issue #28089

Reviews

Conflicts