test: Fix intermittent issue in p2p_sendtxrcncl.py #26381
Hidden character warning
test: Fix intermittent issue in p2p_sendtxrcncl.py #26381maflcko merged 3 commits intobitcoin:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
test/functional/p2p_sendtxrcncl.py
Outdated
There was a problem hiding this comment.
I don't think this is the log message we should expect here. I think should be txreconciliation protocol violation
Does it pass because this log message happened earlier?
There was a problem hiding this comment.
P2PInterface sends a verack on the version message, so this will disconnect for the verack, not for another reason. Looks like another bug in the test?
| peer.send_and_ping(msg_verack()) | ||
| peer.send_message(create_sendtxrcncl_msg()) | ||
| peer.wait_for_disconnect() | ||
| peer = self.nodes[0].add_p2p_connection(P2PInterface()) |
There was a problem hiding this comment.
nit: worth explicitly stating wait_for_verack=True?
There was a problem hiding this comment.
I don't really like the wait_for_verack use in this test. I think all tests here should wait for the verack. However this is currently not possible because waiting for the verack will also send a ping-pong, which obviously doesn't work. Maybe there could be another optional arg to add_p2p_connection to say skip_verack_ping_pong?
Previously it disconnected due to "sendtxrcncl received after verack", now it disconnects for the correct reason.
maflcko
force-pushed
the
2210-test-race-
branch
from
fafd020 to
fa3da83
Compare
|
Fixed another bug in the last push |
|
ACK fa3da83 |
fa3da83 test: Check debug log as well in p2p_sendtxrcncl.py (MacroFake) fae0439 test: Check correct disconnect reason in p2p_sendtxrcncl.py (MacroFake) fa590cf test: Fix intermittent issue in p2p_sendtxrcncl.py (MacroFake) Pull request description: Should fix bitcoin#26364 I can't reproduce this, but my guess would be that `PeerNoVerack::on_version`, which sends the `wtxidrelay` message, is executed in the event loop and thus may run after the main thread sending `msg_verack`. Also, fix another bug. Finally, add some `assert_debug_log` to ensure the right code branch is executed (and not some random, unrelated disconnect). ACKs for top commit: naumenkogs: ACK fa3da83 Tree-SHA512: ee2988372223ea22e94e9783ef6d37114a3945991b6d60f0157917f3850fb7077c566564c3771d915ee74ab28202a3b73c6d89ddec6e2c918462db9a3c02e6cf
4 participants
Should fix #26364
I can't reproduce this, but my guess would be that
PeerNoVerack::on_version, which sends thewtxidrelaymessage, is executed in the event loop and thus may run after the main thread sendingmsg_verack.Also, fix another bug.
Finally, add some
assert_debug_logto ensure the right code branch is executed (and not some random, unrelated disconnect).