Skip to content

Conversation

vasild
Copy link
Contributor

@vasild vasild commented Feb 29, 2024

We connect a new peer, wait for it using self.nodes[0].wait_for_new_peer() and then demand that "transport_protocol_type" is either "v1" or "v2". However wait_for_new_peer() may return too early - during the P2P handshake when "transport_protocol_type" is "detecting".

Fix this by waiting for the P2P handshake to complete and the protocol version to be established.

An example failure (equal fields removed for clarity):

AssertionError: not({
    ...
    'session_id': '',
    'transport_protocol_type': 'detecting',
} == {
    ...
    'session_id': '8fe5902166b892a57ac6849caadd02d2dff83d84f2e57ee50bf894f93cf2f62e',
    'transport_protocol_type': 'v2',
})

https://cirrus-ci.com/task/5839860502626304?logs=ci#L8326

We connect a new peer, wait for it using `self.nodes[0].wait_for_new_peer()`
and then demand that "transport_protocol_type" is either "v1" or "v2".
However `wait_for_new_peer()` may return too early - during the P2P handshake
when "transport_protocol_type" is "detecting".

Fix this by waiting for the P2P handshake to complete and the protocol
version to be established.

An example failure (equal fields removed for clarity):

```
AssertionError: not({
    ...
    'session_id': '',
    'transport_protocol_type': 'detecting',
} == {
    ...
    'session_id': '8fe5902166b892a57ac6849caadd02d2dff83d84f2e57ee50bf894f93cf2f62e',
    'transport_protocol_type': 'v2',
})
```
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 29, 2024

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.
A summary of reviews will appear here.

@DrahtBot DrahtBot added the Tests label Feb 29, 2024
@maflcko
Copy link
Member

maflcko commented Feb 29, 2024

Generally, it is good to check the open pull requests before opening a pull request, to avoid duplicate patches for the same fix.

@maflcko maflcko closed this Feb 29, 2024
@maflcko
Copy link
Member

maflcko commented Feb 29, 2024

#29511

@vasild vasild deleted the fix_race_in_rpc_net.py branch February 29, 2024 11:31
@bitcoin bitcoin locked and limited conversation to collaborators Feb 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants