From b838a0e3b672eb875047acdf3449e65702f5c0ee Mon Sep 17 00:00:00 2001 From: unniznd Date: Tue, 12 Aug 2025 21:50:10 +0530 Subject: [PATCH 1/9] added none type to return value of negotiate and changed caller handles to handle none. Added newsfragment. --- libp2p/host/basic_host.py | 3 +++ libp2p/protocol_muxer/multiselect.py | 2 +- libp2p/security/security_multistream.py | 7 ++++++- libp2p/stream_muxer/muxer_multistream.py | 7 ++++++- newsfragments/837.fix.rst | 1 + 5 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 newsfragments/837.fix.rst diff --git a/libp2p/host/basic_host.py b/libp2p/host/basic_host.py index 70e41953..008fe7e5 100644 --- a/libp2p/host/basic_host.py +++ b/libp2p/host/basic_host.py @@ -288,6 +288,9 @@ class BasicHost(IHost): protocol, handler = await self.multiselect.negotiate( MultiselectCommunicator(net_stream), self.negotiate_timeout ) + if protocol is None: + await net_stream.reset() + raise StreamFailure("No protocol selected") except MultiselectError as error: peer_id = net_stream.muxed_conn.peer_id logger.debug( diff --git a/libp2p/protocol_muxer/multiselect.py b/libp2p/protocol_muxer/multiselect.py index 8d311391..e58c0981 100644 --- a/libp2p/protocol_muxer/multiselect.py +++ b/libp2p/protocol_muxer/multiselect.py @@ -53,7 +53,7 @@ class Multiselect(IMultiselectMuxer): self, communicator: IMultiselectCommunicator, negotiate_timeout: int = DEFAULT_NEGOTIATE_TIMEOUT, - ) -> tuple[TProtocol, StreamHandlerFn | None]: + ) -> tuple[TProtocol | None, StreamHandlerFn | None]: """ Negotiate performs protocol selection. diff --git a/libp2p/security/security_multistream.py b/libp2p/security/security_multistream.py index 193cc092..d15dbbd9 100644 --- a/libp2p/security/security_multistream.py +++ b/libp2p/security/security_multistream.py @@ -26,6 +26,9 @@ from libp2p.protocol_muxer.multiselect_client import ( from libp2p.protocol_muxer.multiselect_communicator import ( MultiselectCommunicator, ) +from libp2p.transport.exceptions import ( + SecurityUpgradeFailure, +) """ Represents a secured connection object, which includes a connection and details about @@ -104,7 +107,7 @@ class SecurityMultistream(ABC): :param is_initiator: true if we are the initiator, false otherwise :return: selected secure transport """ - protocol: TProtocol + protocol: TProtocol | None communicator = MultiselectCommunicator(conn) if is_initiator: # Select protocol if initiator @@ -114,5 +117,7 @@ class SecurityMultistream(ABC): else: # Select protocol if non-initiator protocol, _ = await self.multiselect.negotiate(communicator) + if protocol is None: + raise SecurityUpgradeFailure("No protocol selected") # Return transport from protocol return self.transports[protocol] diff --git a/libp2p/stream_muxer/muxer_multistream.py b/libp2p/stream_muxer/muxer_multistream.py index 76699c67..d96820a4 100644 --- a/libp2p/stream_muxer/muxer_multistream.py +++ b/libp2p/stream_muxer/muxer_multistream.py @@ -30,6 +30,9 @@ from libp2p.stream_muxer.yamux.yamux import ( PROTOCOL_ID, Yamux, ) +from libp2p.transport.exceptions import ( + MuxerUpgradeFailure, +) class MuxerMultistream: @@ -73,7 +76,7 @@ class MuxerMultistream: :param conn: conn to choose a transport over :return: selected muxer transport """ - protocol: TProtocol + protocol: TProtocol | None communicator = MultiselectCommunicator(conn) if conn.is_initiator: protocol = await self.multiselect_client.select_one_of( @@ -81,6 +84,8 @@ class MuxerMultistream: ) else: protocol, _ = await self.multiselect.negotiate(communicator) + if protocol is None: + raise MuxerUpgradeFailure("No protocol selected") return self.transports[protocol] async def new_conn(self, conn: ISecureConn, peer_id: ID) -> IMuxedConn: diff --git a/newsfragments/837.fix.rst b/newsfragments/837.fix.rst new file mode 100644 index 00000000..47919c23 --- /dev/null +++ b/newsfragments/837.fix.rst @@ -0,0 +1 @@ +Added multiselect type consistency in negotiate method. Updates all the usages of the method. From 1ecff5437ce8bbd6c2edf66f12f02466d5d3ad7c Mon Sep 17 00:00:00 2001 From: unniznd Date: Thu, 14 Aug 2025 07:29:06 +0530 Subject: [PATCH 2/9] fixed newsfragment filename issue. --- newsfragments/{837.fix.rst => 837.bugfix.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{837.fix.rst => 837.bugfix.rst} (100%) diff --git a/newsfragments/837.fix.rst b/newsfragments/837.bugfix.rst similarity index 100% rename from newsfragments/837.fix.rst rename to newsfragments/837.bugfix.rst From 621469734949df6e7b9abecfb4edc585f97766d2 Mon Sep 17 00:00:00 2001 From: unniznd Date: Mon, 25 Aug 2025 23:01:35 +0530 Subject: [PATCH 3/9] removed redundant imports --- libp2p/security/security_multistream.py | 3 --- libp2p/stream_muxer/muxer_multistream.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/libp2p/security/security_multistream.py b/libp2p/security/security_multistream.py index 9b341ed7..a9c4b19c 100644 --- a/libp2p/security/security_multistream.py +++ b/libp2p/security/security_multistream.py @@ -29,9 +29,6 @@ from libp2p.protocol_muxer.multiselect_client import ( from libp2p.protocol_muxer.multiselect_communicator import ( MultiselectCommunicator, ) -from libp2p.transport.exceptions import ( - SecurityUpgradeFailure, -) """ Represents a secured connection object, which includes a connection and details about diff --git a/libp2p/stream_muxer/muxer_multistream.py b/libp2p/stream_muxer/muxer_multistream.py index 4a07b261..322db912 100644 --- a/libp2p/stream_muxer/muxer_multistream.py +++ b/libp2p/stream_muxer/muxer_multistream.py @@ -33,9 +33,6 @@ from libp2p.stream_muxer.yamux.yamux import ( PROTOCOL_ID, Yamux, ) -from libp2p.transport.exceptions import ( - MuxerUpgradeFailure, -) class MuxerMultistream: From c08007feda758dfc16efb29940f153e751d8922c Mon Sep 17 00:00:00 2001 From: unniznd Date: Wed, 27 Aug 2025 21:54:05 +0530 Subject: [PATCH 4/9] improve error message in basic host --- libp2p/host/basic_host.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libp2p/host/basic_host.py b/libp2p/host/basic_host.py index 1ef5dda2..ee1bb04d 100644 --- a/libp2p/host/basic_host.py +++ b/libp2p/host/basic_host.py @@ -290,7 +290,9 @@ class BasicHost(IHost): ) if protocol is None: await net_stream.reset() - raise StreamFailure("No protocol selected") + raise StreamFailure( + "Failed to negotiate protocol: no protocol selected" + ) except MultiselectError as error: peer_id = net_stream.muxed_conn.peer_id logger.debug( From 9f80dbae12920622416cd774b6db1198965cb718 Mon Sep 17 00:00:00 2001 From: unniznd Date: Wed, 27 Aug 2025 22:05:19 +0530 Subject: [PATCH 5/9] added the testcase for StreamFailure --- tests/core/host/test_basic_host.py | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/core/host/test_basic_host.py b/tests/core/host/test_basic_host.py index ed21ad80..635f2863 100644 --- a/tests/core/host/test_basic_host.py +++ b/tests/core/host/test_basic_host.py @@ -1,3 +1,10 @@ +from unittest.mock import ( + AsyncMock, + MagicMock, +) + +import pytest + from libp2p import ( new_swarm, ) @@ -10,6 +17,9 @@ from libp2p.host.basic_host import ( from libp2p.host.defaults import ( get_default_protocols, ) +from libp2p.host.exceptions import ( + StreamFailure, +) def test_default_protocols(): @@ -22,3 +32,30 @@ def test_default_protocols(): # NOTE: comparing keys for equality as handlers may be closures that do not compare # in the way this test is concerned with assert handlers.keys() == get_default_protocols(host).keys() + + +@pytest.mark.trio +async def test_swarm_stream_handler_no_protocol_selected(monkeypatch): + key_pair = create_new_key_pair() + swarm = new_swarm(key_pair) + host = BasicHost(swarm) + + # Create a mock net_stream + net_stream = MagicMock() + net_stream.reset = AsyncMock() + net_stream.muxed_conn.peer_id = "peer-test" + + # Monkeypatch negotiate to simulate "no protocol selected" + async def fake_negotiate(comm, timeout): + return None, None + + monkeypatch.setattr(host.multiselect, "negotiate", fake_negotiate) + + # Now run the handler and expect StreamFailure + with pytest.raises( + StreamFailure, match="Failed to negotiate protocol: no protocol selected" + ): + await host._swarm_stream_handler(net_stream) + + # Ensure reset was called since negotiation failed + net_stream.reset.assert_awaited() From 3c52b859baca1af6ade433569fbd57d083d8f432 Mon Sep 17 00:00:00 2001 From: unniznd Date: Fri, 29 Aug 2025 11:30:17 +0530 Subject: [PATCH 6/9] improved the error message --- libp2p/security/security_multistream.py | 4 +++- libp2p/stream_muxer/muxer_multistream.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libp2p/security/security_multistream.py b/libp2p/security/security_multistream.py index a9c4b19c..f7c81de1 100644 --- a/libp2p/security/security_multistream.py +++ b/libp2p/security/security_multistream.py @@ -118,6 +118,8 @@ class SecurityMultistream(ABC): # Select protocol if non-initiator protocol, _ = await self.multiselect.negotiate(communicator) if protocol is None: - raise MultiselectError("fail to negotiate a security protocol") + raise MultiselectError( + "fail to negotiate a security protocol: no protocl selected" + ) # Return transport from protocol return self.transports[protocol] diff --git a/libp2p/stream_muxer/muxer_multistream.py b/libp2p/stream_muxer/muxer_multistream.py index 322db912..76689c17 100644 --- a/libp2p/stream_muxer/muxer_multistream.py +++ b/libp2p/stream_muxer/muxer_multistream.py @@ -85,7 +85,9 @@ class MuxerMultistream: else: protocol, _ = await self.multiselect.negotiate(communicator) if protocol is None: - raise MultiselectError("fail to negotiate a stream muxer protocol") + raise MultiselectError( + "fail to negotiate a stream muxer protocol: no protocol selected" + ) return self.transports[protocol] async def new_conn(self, conn: ISecureConn, peer_id: ID) -> IMuxedConn: From 31040931ea7543e3d993662ddb9564bd77f40c04 Mon Sep 17 00:00:00 2001 From: acul71 Date: Sat, 30 Aug 2025 23:44:49 +0200 Subject: [PATCH 7/9] fix: remove unused upgrade_listener function (Issue 2 from #726) --- libp2p/transport/upgrader.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libp2p/transport/upgrader.py b/libp2p/transport/upgrader.py index 8b47fff4..40ba5321 100644 --- a/libp2p/transport/upgrader.py +++ b/libp2p/transport/upgrader.py @@ -1,9 +1,7 @@ from libp2p.abc import ( - IListener, IMuxedConn, IRawConnection, ISecureConn, - ITransport, ) from libp2p.custom_types import ( TMuxerOptions, @@ -43,10 +41,6 @@ class TransportUpgrader: self.security_multistream = SecurityMultistream(secure_transports_by_protocol) self.muxer_multistream = MuxerMultistream(muxer_transports_by_protocol) - def upgrade_listener(self, transport: ITransport, listeners: IListener) -> None: - """Upgrade multiaddr listeners to libp2p-transport listeners.""" - # TODO: Figure out what to do with this function. - async def upgrade_security( self, raw_conn: IRawConnection, From d620270eafa1b1858874f77b05e51f3dcf6e3a45 Mon Sep 17 00:00:00 2001 From: acul71 Date: Sun, 31 Aug 2025 00:10:15 +0200 Subject: [PATCH 8/9] docs: add newsfragment for issue 883 - remove unused upgrade_listener function --- newsfragments/883.internal.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 newsfragments/883.internal.rst diff --git a/newsfragments/883.internal.rst b/newsfragments/883.internal.rst new file mode 100644 index 00000000..a9ca3a0e --- /dev/null +++ b/newsfragments/883.internal.rst @@ -0,0 +1,5 @@ +Remove unused upgrade_listener function from transport upgrader + +- Remove unused `upgrade_listener` function from `libp2p/transport/upgrader.py` (Issue 2 from #726) +- Clean up unused imports related to the removed function +- Improve code maintainability by removing dead code From 7d6eb28d7c8fa30468de635890a6d194d56014f5 Mon Sep 17 00:00:00 2001 From: unniznd Date: Mon, 1 Sep 2025 09:48:08 +0530 Subject: [PATCH 9/9] message inconsistency fixed --- libp2p/security/security_multistream.py | 2 +- libp2p/stream_muxer/muxer_multistream.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libp2p/security/security_multistream.py b/libp2p/security/security_multistream.py index f7c81de1..ee8d4475 100644 --- a/libp2p/security/security_multistream.py +++ b/libp2p/security/security_multistream.py @@ -119,7 +119,7 @@ class SecurityMultistream(ABC): protocol, _ = await self.multiselect.negotiate(communicator) if protocol is None: raise MultiselectError( - "fail to negotiate a security protocol: no protocl selected" + "Failed to negotiate a security protocol: no protocol selected" ) # Return transport from protocol return self.transports[protocol] diff --git a/libp2p/stream_muxer/muxer_multistream.py b/libp2p/stream_muxer/muxer_multistream.py index 76689c17..ef90fac0 100644 --- a/libp2p/stream_muxer/muxer_multistream.py +++ b/libp2p/stream_muxer/muxer_multistream.py @@ -86,7 +86,7 @@ class MuxerMultistream: protocol, _ = await self.multiselect.negotiate(communicator) if protocol is None: raise MultiselectError( - "fail to negotiate a stream muxer protocol: no protocol selected" + "Fail to negotiate a stream muxer protocol: no protocol selected" ) return self.transports[protocol]