Skip to content

test: Add missing wait for version to be sent in add_outbound_p2p_connection#28822

Merged
fanquake merged 1 commit intobitcoin:masterfrom
maflcko:2311-test-sync-ver-
Nov 9, 2023
Merged

test: Add missing wait for version to be sent in add_outbound_p2p_connection#28822
fanquake merged 1 commit intobitcoin:masterfrom
maflcko:2311-test-sync-ver-

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Nov 8, 2023 •
edited
Loading

Can be tested with:

diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index b1ed97b794..eb4f72c6b6 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -205,6 +205,7 @@ class P2PConnection(asyncio.Protocol):
         assert not self._transport
         logger.debug("Connected & Listening: %s:%d" % (self.dstaddr, self.dstport))
         self._transport = transport
+        import time;time.sleep(.1);
         if self.on_connection_send_msg:
             self.send_message(self.on_connection_send_msg)
             self.on_connection_send_msg = None  # Never used again

Found and reported by mzumsande in #28782 (review)

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 8, 2023 •
edited
Loading

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK mzumsande

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Nov 8, 2023
@fanquake fanquake requested a review from mzumsande November 8, 2023 10:56
mzumsande
mzumsande reviewed Nov 8, 2023
View reviewed changes
Copy link
Contributor

@mzumsande mzumsande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK faa2ad8

@fanquake fanquake merged commit c2da8c5 into bitcoin:master Nov 9, 2023
@maflcko maflcko deleted the 2311-test-sync-ver- branch November 9, 2023 10:39
kwvg added a commit to kwvg/dash that referenced this pull request Oct 16, 2024
@kwvg
merge bitcoin#28822: Add missing wait for version to be sent in add_o…
d0804d4 
…utbound_p2p_connection
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 22, 2024
@PastaPastaPasta
Merge #6329: backport: merge bitcoin#20524, bitcoin#26553, bitcoin#27986
dd629cf 
, bitcoin#28645, bitcoin#28632, bitcoin#28782, bitcoin#28822, bitcoin#29006, bitcoin#29212, merge bitcoin-core/gui#754, partial bitcoin#23443, bitcoin#26448 (BIP324 backports: part 3)

aa5311d merge bitcoin#29212: Fix -netinfo backward compat with getpeerinfo pre-v26 (Kittywhiskers Van Gogh)
1a293c7 merge bitcoin#29006: fix v2 transport intermittent test failure (Kittywhiskers Van Gogh)
d0804d4 merge bitcoin#28822: Add missing wait for version to be sent in add_outbound_p2p_connection (Kittywhiskers Van Gogh)
c0b3062 merge bitcoin#28782: Add missing sync on send_version in peer_connect (Kittywhiskers Van Gogh)
35253cd merge bitcoin#28632: make python p2p not send getaddr on incoming connections (Kittywhiskers Van Gogh)
6a4ca62 merge bitcoin#28645: fix `assert_debug_log` call-site bugs, add type checks (Kittywhiskers Van Gogh)
deaee14 merge bitcoin-core/gui#754: Add BIP324-specific labels to peer details (Kittywhiskers Van Gogh)
fffe6e7 merge bitcoin#27986: remove race in the user-agent reception check (Kittywhiskers Van Gogh)
1bf135b merge bitcoin#26553: Fix intermittent failure in rpc_net.py (Kittywhiskers Van Gogh)
5bf245b partial bitcoin#26448: fix intermittent failure in p2p_sendtxrcncl.py (Kittywhiskers Van Gogh)
b7c0030 partial bitcoin#23443: Erlay support signaling (Kittywhiskers Van Gogh)
c709df7 merge bitcoin#20524: Move MIN_VERSION_SUPPORTED to p2p.py (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on #6324

  * Dependency for #6330

  ## How Has This Been Tested?

  * Changes to Qt client were validated by running the client

    <details>

    <summary>Screenshot</summary>

    ![Transport reporting in Qt](https://github.com/user-attachments/assets/0d551e19-f3a2-4ce7-83d6-5cb3d03b1765)

    </details>

  * Changes to `dash-cli` were validated by running it with different node versions

    **Against a node built on this PR**

    <details>

    <summary>Screenshot</summary>

    ![getinfo with node running the latest build](https://github.com/user-attachments/assets/8cda68cc-727a-4cf3-a4d8-dd6a33331d78)

    </details>

    **Against a node built running the last release**

    <details>

    <summary>Screenshot</summary>

    ![getinfo with node running the latest release](https://github.com/user-attachments/assets/0c6ff476-7cc9-4297-bae5-35d423aba480)

    </details>

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas (note: N/A)
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation (note: N/A)
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    ACK aa5311d
  PastaPastaPasta:
    utACK aa5311d

Tree-SHA512: 8cca324ac988a73c0590a4e9b318e81ce951ac55fb173cf507fa647cab01ab4981e6a06d4792376b4bfb44ff09d4811de05fadb9ba793dd00b4c7965b4b22654
@bitcoin bitcoin locked and limited conversation to collaborators Nov 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mzumsande mzumsande mzumsande left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maflcko @DrahtBot @mzumsande @fanquake