Skip to content

test: fix wallet_transactiontime_rescan.py --descriptors and add to test runner #23596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
maflcko merged 3 commits into from
Dec 6, 2021
Merged

test: fix wallet_transactiontime_rescan.py --descriptors and add to test runner #23596

maflcko merged 3 commits into from
Dec 6, 2021

Conversation

@theStack
Copy link
Contributor

@theStack theStack commented Nov 25, 2021

The functional test wallet_transactiontime_rescan.py currently fails on master branch, if descriptor wallets are used (argument --descriptors). This is due to the fact that in this case, the test framework maps the importaddress RPC calls to the importdescriptors RPC (rescan=False -> timestamp='now'), which always rescans blocks of the past 2 hours, based on the current MTP timestamp. In order to avoid importing the last address (wo3), we generate 10 more blocks with advanced time, to ensure that the balance after importing is zero:

bitcoin/test/functional/wallet_transactiontime_rescan.py

Lines 125 to 134 in 681b25e

self.log.info('Restore user wallet on another node without rescan')
restorenode.createwallet(wallet_name='wo', disable_private_keys=True)
restorewo_wallet = restorenode.get_wallet_rpc('wo')
restorewo_wallet.importaddress(wo1, rescan=False)
restorewo_wallet.importaddress(wo2, rescan=False)
restorewo_wallet.importaddress(wo3, rescan=False)
# check user has 0 balance and no transactions
assert_equal(restorewo_wallet.getbalance(), 0)

Calling this test with descriptor wallets is also added to test runner. Fixes #23562.

@DrahtBot DrahtBot added the Tests label Nov 25, 2021
brunoerg
brunoerg approved these changes Nov 26, 2021
View reviewed changes
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

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

tACK e4a54af

@Sjors
Copy link
Member

Sjors commented Nov 28, 2021 •
edited
Loading

On macOS I was getting this failure consistency when compiling without BDB. Thanks for fixing.

tACK e4a54af

Though another could be to fix the wrapper in test_node.py to use a timestamp slightly in the future when rescan is false, or to use importdescriptors directly.

@maflcko maflcko merged commit 32d9f37 into bitcoin:master Dec 6, 2021
@theStack theStack deleted the 202111-test-fix-wallet_transactiontime_rescan-for-descriptor-wallets branch December 6, 2021 11:39
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 7, 2021
Merge bitcoin#23596: test: fix `wallet_transactiontime_rescan.py --de…
24fd1ed 
…scriptors` and add to test runner

e4a54af  test: add wallet_transactiontime_rescan.py --descriptors to test_runner.py (Sebastian Falbesoner)
b60e02e test: fix test wallet_transactiontime_rescan.py for descriptor wallets (Sebastian Falbesoner)
a905ed1 test: refactor: use `set_node_times` helper in wallet_transactiontime_rescan.py (Sebastian Falbesoner)

Pull request description:

  The functional test wallet_transactiontime_rescan.py currently fails on master branch, if descriptor wallets are used (argument `--descriptors`). This is due to the fact that in this case, the test framework maps the importaddress RPC calls to the importdescriptors RPC (rescan=False -> timestamp='now'), which always rescans blocks of the past 2 hours, based on the current MTP timestamp. In order to avoid importing the last address (wo3), we generate 10 more blocks with advanced time, to ensure that the balance after importing is zero:
  https://github.com/bitcoin/bitcoin/blob/681b25e3cd7d084f642693152322ed9a40f33ba0/test/functional/wallet_transactiontime_rescan.py#L125-L134

  Calling this test with descriptor wallets is also added to test runner. Fixes bitcoin#23562.

ACKs for top commit:
  Sjors:
    tACK e4a54af
  brunoerg:
    tACK e4a54af

Tree-SHA512: 9fd8e298d48dd7947b1218d61a1a66c1241b3dbb14451b0ec7cd30caa74ee540e7ee5a7bd10d421b9e3b6e549fa5c3e85bd02496436128b433b328118642f600
RandyMcMillan pushed a commit to RandyMcMillan/mempool-tab that referenced this pull request Dec 23, 2021
Merge bitcoin/bitcoin#23596: test: fix `wallet_transactiontime_rescan…
9d13e87 
….py --descriptors` and add to test runner

a59b185  test: add wallet_transactiontime_rescan.py --descriptors to test_runner.py (Sebastian Falbesoner)
d56bbf8 test: fix test wallet_transactiontime_rescan.py for descriptor wallets (Sebastian Falbesoner)
e7ac976 test: refactor: use `set_node_times` helper in wallet_transactiontime_rescan.py (Sebastian Falbesoner)

Pull request description:

  The functional test wallet_transactiontime_rescan.py currently fails on master branch, if descriptor wallets are used (argument `--descriptors`). This is due to the fact that in this case, the test framework maps the importaddress RPC calls to the importdescriptors RPC (rescan=False -> timestamp='now'), which always rescans blocks of the past 2 hours, based on the current MTP timestamp. In order to avoid importing the last address (wo3), we generate 10 more blocks with advanced time, to ensure that the balance after importing is zero:
  https://github.com/bitcoin/bitcoin/blob/0fc4444fc0eea0ae88ac2f87434bf2c21f5b21da/test/functional/wallet_transactiontime_rescan.py#L125-L134

  Calling this test with descriptor wallets is also added to test runner. Fixes #23562.

ACKs for top commit:
  Sjors:
    tACK a59b185
  brunoerg:
    tACK a59b185

Tree-SHA512: 9fd8e298d48dd7947b1218d61a1a66c1241b3dbb14451b0ec7cd30caa74ee540e7ee5a7bd10d421b9e3b6e549fa5c3e85bd02496436128b433b328118642f600
@bitcoin bitcoin locked and limited conversation to collaborators Dec 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@achow101 achow101 Awaiting requested review from achow101

@ryanofsky ryanofsky Awaiting requested review from ryanofsky

1 more reviewer

@brunoerg brunoerg brunoerg approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test: wallet_transactiontime_rescan.py failure with --without-bdb

5 participants

@theStack @Sjors @brunoerg @maflcko @DrahtBot