The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See 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.

Code review ACK 5bd2010

I had just written the same fix essentially. The stopatheight node shuts down before the list of wait_until checks in connect_nodes succeeds. I guess in the python execution is slower in CI but the node and sync is still comparatively fast.

Concept ACK.

lgtm ACK 5bd2010

Even with latest HEAD, still seeing an apparent race. Still investigating.

2023-10-04T15:24:47.476000Z TestFramework (INFO): Restarted node before snapshot validation completed, reloading...
2023-10-04T15:24:47.782000Z TestFramework (INFO): Ensuring snapshot chain syncs to tip. (399)
2023-10-04T15:25:47.828000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
            lambda: n1.getchainstates().get('snapshot', {}).get('blocks') == FINAL_HEIGHT)
'''
2023-10-04T15:25:47.828000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/james/src/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
    self.run_test()
  File "/home/james/src/bitcoin/./test/functional/feature_assumeutxo.py", line 162, in run_test
    wait_until_helper(
  File "/home/james/src/bitcoin/test/functional/test_framework/util.py", line 276, in wait_until_helper
    raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
AssertionError: Predicate ''''
            lambda: n1.getchainstates().get('snapshot', {}).get('blocks') == FINAL_HEIGHT)
''' not true after 60.0 seconds
2023-10-04T15:25:47.879000Z TestFramework (INFO): Stopping nodes

lgtm ACK 7e40032

Alright, I'm guessing we're fixed for real this time; 7e40032 accounts for the late change that we made on @ryanofsky's recommendation to list what was the snapshot chainstate as the normal chainstate once background validation completes. There was a race in the test during the first time we check for the final height, which may be done after background validation completed.

I'm going to keep running the test in a loop on my end and will post back in a bit to confirm that I think we're good to go.

Code review ACK 7e40032

ACK 7e40032

Gonna let this run in a loop for a bit before merging.

Test has been running on my machine in a loop without failure for about an hour now.