Part 6 - p2p: replace upnpclient with async-upnp-client for NAT port mapping#970
Part 6 - p2p: replace upnpclient with async-upnp-client for NAT port mapping#970ping-ke wants to merge 14 commits intoupgrade/py313-baselinefrom
Conversation
upnpclient uses synchronous I/O and netifaces for interface discovery, both incompatible or unmaintained under Python 3.13. Switch to async-upnp-client which provides asyncio-native UPnP/IGD support. Rewrite UPnPService.discover() to use async_search() for device discovery and AiohttpSessionRequester for requests, removing the blocking ThreadPoolExecutor path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix AttributeError: replace undefined self.external_port/self.internal_port/self.protocol with self.port - Fix _discover timeout: wrap async_search with asyncio.wait_for instead of waiting after it completes - Fix _run: guard _add_port_mapping with self._service check to avoid AttributeError - Fix _delete_port_mapping: delete both TCP and UDP mappings to match AddPortMapping - Remove dead code: _refresh_task and _running fields that were never used
quarkchain/p2p/nat.py
Outdated
| session = aiohttp.ClientSession() | ||
| try: | ||
| await self._discover(session) | ||
| finally: |
There was a problem hiding this comment.
You may need to run the tests on a network with the router to verify that it works
| @@ -1,48 +1,21 @@ | |||
|
|
|||
| import aiohttp | |||
There was a problem hiding this comment.
This PR introduces a new runtime dependency on async-upnp-client (and directly imports aiohttp in nat.py), but the project dependencies still only list upnpclient and netifaces.
Right now requirements.txt / setup.py are not updated to install the new dependency set, so a clean environment will fail at import time before this code can run.
It looks like we should add async-upnp-client (and ensure aiohttp is available through project dependencies), while removing the old UPnP-specific dependencies that are no longer used.
There was a problem hiding this comment.
This will be resolved after merge other changes. Test can be run on branch upgrade-py3-13.
There was a problem hiding this comment.
I don’t think this is a good practice, as additional bugs may be introduced during the merge process.
Also, have you run the full test suite again on upgrade-py3-13?
There was a problem hiding this comment.
All changes will be made on the upgrade-py3-13 branch. After all tests pass, they will be cherry-picked to the target branch. Once the merge is completed, the full test suite will be executed again. I will also perform a comparison between the upgrade/base-line branch and the upgrade-py3-13 branch to ensure consistency and correctness.
upnpclient uses synchronous I/O and netifaces for interface discovery, both incompatible or unmaintained under Python 3.13. Switch to async-upnp-client which provides asyncio-native UPnP/IGD support. Rewrite UPnPService.discover() to use async_search() for device discovery and AiohttpSessionRequester for requests, removing the blocking ThreadPoolExecutor path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix AttributeError: replace undefined self.external_port/self.internal_port/self.protocol with self.port - Fix _discover timeout: wrap async_search with asyncio.wait_for instead of waiting after it completes - Fix _run: guard _add_port_mapping with self._service check to avoid AttributeError - Fix _delete_port_mapping: delete both TCP and UDP mappings to match AddPortMapping - Remove dead code: _refresh_task and _running fields that were never used
…rkchain into upgrade/p2p-nat
Summary
upnpclientuses synchronous/blocking I/O andnetifacesfor interface discovery; neither is well-maintained on Python 3.13.Rewrite
UPnPServiceinnat.pyto useasync-upnp-client:async_search()for UPnP IGD device discoveryAiohttpSessionRequester+UpnpFactoryfor async HTTP requestsThreadPoolExecutorblocking path andnetifacesdependencydiscover()coroutine (previouslyadd_nat_portmap()) thatreturns the mapped external IP string
p2p_server.pycall site accordinglyTest plan
None)