Skip to content

Fix incorrect peer port autodetection and substream limit exceeded errors#27

Merged
teor2345 merged 5 commits intosubspace-v9from
conn-stream-fixes
Apr 7, 2025
Merged

Fix incorrect peer port autodetection and substream limit exceeded errors#27
teor2345 merged 5 commits intosubspace-v9from
conn-stream-fixes

Conversation

@teor2345
Copy link
Copy Markdown

@teor2345 teor2345 commented Mar 24, 2025

This PR cherry-picks these 3 network connection/stream bug fixes to our substrate fork:

Peer address autodetection fix, which works around a libp2p bug that pollutes the peer set with ephemeral outbound ports:

Peer connection substream fixes, which correctly detect and react to I/O errors on connections by closing substreams:

Part of the fix for:
autonomys/subspace#3450

dmitry-markin and others added 4 commits March 24, 2025 09:39
…al addresses (paritytech#7338)

Instead of using libp2p-provided external address candidates,
susceptible to address translation issues, use litep2p-backend approach
based on confirming addresses observed by multiple peers as external.

Fixes paritytech#7207.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…ech#7640) - conflicts resolved using --strategy-option=theirs

This PR updates litep2p to version 0.9.1. The yamux config is entirely
removed to mirror the libp2p yamux upstream version.
While at it, I had to bump indexmap and URL as well.

This release enhances compatibility between litep2p and libp2p by using
the latest Yamux upstream version. Additionally, it includes various
improvements and fixes to boost the stability and performance of the
WebSocket stream and the multistream-select protocol.

- yamux: Switch to upstream implementation while keeping the controller
API ([paritytech#320](paritytech/litep2p#320))
- req-resp: Replace SubstreamSet with FuturesStream
([paritytech#321](paritytech/litep2p#321))
- cargo: Bring up to date multiple dependencies
([paritytech#324](paritytech/litep2p#324))
- build(deps): bump hickory-proto from 0.24.1 to 0.24.3
([paritytech#323](paritytech/litep2p#323))
- build(deps): bump openssl from 0.10.66 to 0.10.70
([paritytech#322](paritytech/litep2p#322))

- websocket/stream: Fix unexpected EOF on `Poll::Pending` state
poisoning ([paritytech#327](paritytech/litep2p#327))
- websocket/stream: Avoid memory allocations on flushing
([paritytech#325](paritytech/litep2p#325))
- multistream-select: Enforce `io::error` instead of empty protocols
([paritytech#318](paritytech/litep2p#318))
- multistream: Do not wait for negotiation in poll_close
([paritytech#319](paritytech/litep2p#319))

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
…n `std::io::Errors` (paritytech#7724) - conflicts resolved using --strategy-option=theirs

This PR handles a case where we called the `poll_next` on an outbound
substream notification to check if the stream is closed. It is entirely
possible that the `poll_next` would return an `io::error`, for example
end of file.

This PR ensures that we make the distinction between unexpected incoming
data, and error originated from `poll_next`.

While at it, the bulk of the PR change propagates the PeerID from the
network behavior, through the notification handler, to the notification
outbound stream for logging purposes.

cc @paritytech/networking

Part of: paritytech#7722

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@teor2345 teor2345 added the bug Something isn't working label Mar 24, 2025
@teor2345 teor2345 self-assigned this Mar 24, 2025
@teor2345 teor2345 changed the title Conn stream fixes Fix incorrect peer port autodetection and substream limit exceeded errors Mar 24, 2025
@teor2345
Copy link
Copy Markdown
Author

I pushed a logging fix to this PR, it's just a harmless warning, it might not be worth updating subspace yet.

Copy link
Copy Markdown

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

LGTM

.with_websocket(WebSocketTransportConfig {
listen_addresses: websocket.into_iter().flatten().map(Into::into).collect(),
yamux_config: yamux_config.clone(),
yamux_config: litep2p::yamux::Config::default(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this uses litep2p config, is this expected ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yep, it's in the Litep2pNetworkBackend impl, so it needs to use that config type.

@teor2345 teor2345 merged commit 2be8c00 into subspace-v9 Apr 7, 2025
65 of 143 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants