test: fix intermittent error in getblockfrompeer.py #27784
Conversation
This fixes an intermittent error, caused by blocks arriving out of order due to how compact block relay may revert to headers processing when the tip hasn't caught up, and resulting in slightly different pruning behavior. Making sure that all blocks from the previous tests are synced before generating more blocks makes this impossible. See Issue bitcoin#27749 for more details.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
There was a problem hiding this comment.
ACK 9fe9074
Unfortunately there seems to be no reliable way to reproduce the issue on master (which could then be also ran on this PR to ensure that it is indeed fixed), but the explanation in #27749 (comment) makes sense to me.
|
9fe9074 looks harmless, but I haven't delved into the underlying issue. Can the original be reproduced by running the test in a loop?? CI failure seems spurious. |
I haven't been able to reproduce it, I think it requires a constellation with a very special timing which should make it very rare even in the CI. One thing I did is check in the log is that node2 doesn't use |
If you want to give that approach a try, the following shell script (which I have been running for days already, without any issue) should do exactly that: #!/usr/bin/env bash
set -e
while true; do ./test/functional/rpc_getblockfrompeer.py; done |
|
lgtm ACK 9fe9074 |
9fe9074 test: add block sync to getblockfrompeer.py (Martin Zumsande) Pull request description: This adds an additional `sync_blocks` call, fixing an intermittent error caused by blocks arriving out of order due to how compact block relay may revert to headers processing when the tip hasn't caught up, and resulting in slightly different pruning behavior. Making sure that all blocks from the previous tests are synced before generating more blocks makes this impossible. See bitcoin#27749 (comment) and bitcoin#27749 (comment) for a more detailed analysis. bitcoin#27770 is a more long-term approach to avoid having to deal with magic pruneheight numbers in the first place, but that PR introduces a new RPC and needs more discussion. Fixes bitcoin#27749. ACKs for top commit: MarcoFalke: lgtm ACK 9fe9074 theStack: ACK 9fe9074 Tree-SHA512: f3de1ea68725429aeef448c351ea812b805fa216912b112d7db9aceeddb1f2381b705c2577734b0d308e78ec5e0c4d26dc65fc2171f6e21f13061fc71d48216c
This adds an additional
sync_blockscall, fixing an intermittent error caused by blocks arriving out of order due to how compact block relay may revert to headers processing when the tip hasn't caught up, and resulting in slightly different pruning behavior.Making sure that all blocks from the previous tests are synced before generating more blocks makes this impossible.
See #27749 (comment) and #27749 (comment) for a more detailed analysis.
#27770 is a more long-term approach to avoid having to deal with magic pruneheight numbers in the first place, but that PR introduces a new RPC and needs more discussion.
Fixes #27749.