Skip to content

test: Add missing syncwithvalidationinterfacequeue to wallet_balance test #16302

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
merged 1 commit into from
Jun 30, 2019
Merged

test: Add missing syncwithvalidationinterfacequeue to wallet_balance test #16302

merged 1 commit into from
Jun 30, 2019

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jun 28, 2019 •
edited
Loading

A syncwithvalidationinterfacequeue should be sufficient.

Fixes #16020

@maflcko maflcko modified the milestones: 0.18.1, 0.19.0 Jun 28, 2019
@fanquake
Copy link
Member

cc @IlyasRidhuan

@fanquake fanquake added the Tests label Jun 28, 2019
@maflcko maflcko force-pushed the 1906-testWalletBal branch 2 times, most recently from 6993e5b to fa81525 Compare June 28, 2019 15:19
@promag
Copy link
Contributor

promag commented Jun 29, 2019

Still not using sync_all as OP suggests, although I think sync_blocks and syncwithvalidationinterfacequeue is enough. If you keep current commit please update PR.

Just to be clear about the problem, there's a race between getbalance and CWallet::BlockConnected.

ACK fa81525. This can be tested by adding sleep in CWallet::BlockConnected just before LOCK(cs_wallet) - master will always fail while this PR will succeed.

@maflcko maflcko changed the title test: Add missing sync_all to wallet_balance test test: Add missing syncwithvalidationinterfacequeue to wallet_balance test Jun 29, 2019
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jun 30, 2019
Merge bitcoin#16302: test: Add missing syncwithvalidationinterfaceque…
e7f3450 
…ue to wallet_balance test

fa81525 test: Add missing sync_all to wallet_balance test (MarcoFalke)

Pull request description:

  A `syncwithvalidationinterfacequeue` should be sufficient.

  Fixes  bitcoin#16020

ACKs for top commit:
  promag:
    ACK fa81525. This can be tested by adding sleep in `CWallet::BlockConnected` just before `LOCK(cs_wallet)` - master will always fail while this PR will succeed.

Tree-SHA512: 07e067c698627f90f0b9848f921b7067adc70c27105db3258e056384197e50dbee055c87839d238cc11bde11179d3f5879b39e1c8e15465f8f07558c694b677d
@maflcko maflcko merged commit fa81525 into bitcoin:master Jun 30, 2019
@maflcko maflcko deleted the 1906-testWalletBal branch June 30, 2019 15:27
maflcko pushed a commit that referenced this pull request Mar 12, 2020
Merge #18228: test: Add missing syncwithvalidationinterfacequeue
e2d3663 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in #12217 and to wallet_balance.py in #16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 13, 2020
Merge bitcoin#18228: test: Add missing syncwithvalidationinterfacequeue
b553196 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jul 16, 2020
@jasonbcox
Merge #16302: test: Add missing syncwithvalidationinterfacequeue to w…
7a820a7 
…allet_balance test

Summary:
fa815255c70d32809aac640db4a8762c7d71e8db test: Add missing sync_all to wallet_balance test (MarcoFalke)

Pull request description:

  A `syncwithvalidationinterfacequeue` should be sufficient.

  Fixes  #16020

ACKs for top commit:
  promag:
    ACK fa81525. This can be tested by adding sleep in `CWallet::BlockConnected` just before `LOCK(cs_wallet)` - master will always fail while this PR will succeed.

Tree-SHA512: 07e067c698627f90f0b9848f921b7067adc70c27105db3258e056384197e50dbee055c87839d238cc11bde11179d3f5879b39e1c8e15465f8f07558c694b677d

Backport of Core [[bitcoin/bitcoin#16302 | PR16302]]

Test Plan: `test_runner.py wallet_balance`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D6954
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
Merge bitcoin#18228: test: Add missing syncwithvalidationinterfacequeue
e8599a1 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2021
@PastaPastaPasta
Merge bitcoin#18228: test: Add missing syncwithvalidationinterfacequeue
39136e6 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
@PastaPastaPasta
Merge bitcoin#18228: test: Add missing syncwithvalidationinterfacequeue
d6fadb2 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
@PastaPastaPasta
Merge bitcoin#18228: test: Add missing syncwithvalidationinterfacequeue
35bb037 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
@PastaPastaPasta
Merge bitcoin#18228: test: Add missing syncwithvalidationinterfacequeue
785e7b9 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
@PastaPastaPasta
Merge bitcoin#18228: test: Add missing syncwithvalidationinterfacequeue
b8676dd 
faf6f15 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)

Pull request description:

  The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.

ACKs for top commit:
  jonatack:
    ACK faf6f15 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in bitcoin#12217 and to wallet_balance.py in bitcoin#16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.

Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
@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.
Reviewers
No reviews
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
0.19.0
Development

Successfully merging this pull request may close these issues.

Failure in wallet_balance.py
3 participants
@maflcko @fanquake @promag