CI failed on this condition here: https://github.com/bitcoin/bitcoin/actions/runs/19395398994/job/55494696022?pr=33878#step:9:3983
The check was added not too long ago in #33745 and the fix here switches the check to the node which actually produces the block. There are also some comments added to make the checks easier so understand.
Closes #33884
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33880.
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.
🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/19396238061/job/55496659251
LLM reason (✨ experimental): Lint failure: ruff reports undefined name wait_until in test/functional/interface_ipc.py.
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
255@@ -256,7 +256,7 @@ async def interrupt_wait():
256 assert_equal(res.result, True)
257
258 self.log.debug("Block should propagate")
259- assert_equal(self.nodes[1].getchaintips()[0]["height"], current_block_height + 1)
Good catch. The test needs to check two things:
submitBlock()So the check for current_block_height + 1 is important, but we should either do it for self.nodes[0] (no need to wait_until) or keep it for self.nodes[1], but after sync_all.
Only relying on sync_all is not enough, because it won’t catch a regression where the IPC code fails to update the node itself, so there’s no block to propagate and the nodes remained synced.
Good catch. The test needs to check two things:
Thanks for explaining, I have switched the check to node0 and kept it above the sync because that feels like the most natural flow given my improved understanding of the test now. I have also added some short comments based on your explanation.
255@@ -256,9 +256,11 @@ async def interrupt_wait():
256 assert_equal(res.result, True)
257
258 self.log.debug("Block should propagate")
259- assert_equal(self.nodes[1].getchaintips()[0]["height"], current_block_height + 1)
260+ # Check that the IPC node actually updates it own chain
261+ assert_equal(self.nodes[0].getchaintips()[0]["height"], current_block_height + 1)
it own -> its own [possessive pronoun "its" required; "it" is incorrect here]
drahtbot_id_5_m
