Change sync_blocks to pick smarter maxheight (on top of #9196) #9139

pull ryanofsky wants to merge 2 commits into bitcoin:master from ryanofsky:sync-height changing 2 files +9 −6

ryanofsky commented at 10:07 PM on November 11, 2016: member

This PR changes the sync_blocks test function to make it more reliable.

The change is based on #9196 to prevent a test failure that would otherwise start to happen.

Send tip change notification from invalidateblock

This change is needed to prevent sync_blocks timeouts in the mempool_reorg
test after the sync_blocks update in the upcoming commit
"[qa] Change sync_blocks to pick smarter maxheight".

This change was initially suggested by Suhas Daftuar <sdaftuar@chaincode.com>
in https://github.com/bitcoin/bitcoin/pull/8680#r78209060

67c6326abd

[qa] Change sync_blocks to pick smarter maxheight

Instead of syncing to max height returned by the waitforblockheight RPC, sync
to the max height returned by the getblockcount RPC.

This change was suggested by Suhas Daftuar <sdaftuar@chaincode.com>.

1126c853d9

fanquake added the label Tests on Nov 12, 2016
ryanofsky force-pushed on Nov 14, 2016
ryanofsky renamed this:
~~Change sync_blocks to pick smarter maxheight~~
Change sync_blocks to pick smarter maxheight (on top of #9136)
on Nov 14, 2016
ryanofsky force-pushed on Nov 14, 2016
ryanofsky renamed this:
~~Change sync_blocks to pick smarter maxheight (on top of #9136)~~
Change sync_blocks to pick smarter maxheight
on Nov 14, 2016
ryanofsky force-pushed on Nov 15, 2016
ryanofsky commented at 3:11 PM on November 15, 2016: member

Rearranged if-conditions as suggested offline by @morcos.
in qa/rpc-tests/test_framework/util.py:None in b0e57bde96 outdated
```
 128 | @@ -129,17 +129,19 @@ def sync_blocks(rpc_connections, *, wait=1, timeout=60):
 129 |      one node already synced to the latest, stable tip, otherwise there's a
 130 |      chance it might return before all nodes are stably synced.
```
MarcoFalke commented at 3:27 PM on November 15, 2016:

Does this comment still apply, considering that you switched from 0 to getblockcount?

ryanofsky commented at 3:39 PM on November 15, 2016:

This change might make the problem less likely to happen, but it still could happen. If none of the nodes in the set are fully synced when the sync_blocks call is made, then maxheight will be set to some height less than the final stable height, and the call could return before the nodes are all synced.
MarcoFalke commented at 3:28 PM on November 15, 2016: member

Concept ACK
morcos commented at 9:09 PM on November 15, 2016: member

ACK
MarcoFalke commented at 10:41 PM on November 19, 2016: member

@ryanofsky The 60cf51c patch looks like a separate bugfix, not strictly related to tests. Would you mind submitting this as another pull request?
ryanofsky force-pushed on Nov 21, 2016
ryanofsky renamed this:
~~Change sync_blocks to pick smarter maxheight~~
Change sync_blocks to pick smarter maxheight (on top of #9196)
on Nov 21, 2016
MarcoFalke closed this on Nov 23, 2016
MarcoFalke reopened this on Nov 23, 2016
MarcoFalke merged this on Nov 23, 2016
MarcoFalke closed this on Nov 23, 2016
MarcoFalke referenced this in commit 0de7fd36de on Nov 23, 2016
codablock referenced this in commit 646ebd344a on Jan 16, 2018
codablock referenced this in commit 09245eda70 on Jan 16, 2018
codablock referenced this in commit f831710f46 on Jan 17, 2018
andvgal referenced this in commit 6b27fec32e on Jan 6, 2019
CryptoCentric referenced this in commit 8baadb154b on Feb 25, 2019
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#9196 Send tip change notification from invalidateblock