Skip to content

fix: add UPnP peer announcement for NAT nodes#6171

Merged
Scottcjn merged 1 commit into
Scottcjn:mainfrom
yyswhsccc:bounty-radar/issue-2585-nat-upnp
May 28, 2026
Merged

fix: add UPnP peer announcement for NAT nodes#6171
Scottcjn merged 1 commit into
Scottcjn:mainfrom
yyswhsccc:bounty-radar/issue-2585-nat-upnp

Conversation

@yyswhsccc
Copy link
Copy Markdown
Contributor

@yyswhsccc yyswhsccc commented May 24, 2026

BCOS Checklist (Required For Non-Doc PRs)

  • Add a tier label: BCOS-L1 or BCOS-L2 (also accepted: bcos:l1, bcos:l2)
  • If adding new code files, include SPDX header near the top (example: # SPDX-License-Identifier: MIT)
  • Provide test evidence (commands + output or screenshots)

What Changed

Fixes #2585.

  • Added optional UPnP IGD discovery during P2P startup so a node on a private LAN can map its P2P port and resolve an externally reachable URL.
  • Added RC_P2P_EXTERNAL_URL as an explicit override and RC_P2P_ENABLE_UPNP / RC_P2P_DISABLE_UPNP controls for operators.
  • Added signed peer_announce handling so a NAT-discovered URL is shared with existing peers instead of staying local-only.
  • Exposed the current advertised_url on /p2p/health for operator verification.

Testing / Evidence

  • python3 -B -m py_compile node/rustchain_p2p_init.py node/rustchain_p2p_gossip.py node/tests/test_p2p_nat_upnp.py
  • python -B -m pytest -q node/tests/test_p2p_nat_upnp.py node/tests/test_p2p_gossip_routes.py node/tests/test_p2p_endpoint_auth.py tests/test_p2p_demo_peer_config.py --tb=short -p no:cacheprovider -> 14 passed
  • python tools/bcos_spdx_check.py --base-ref origin/main -> BCOS SPDX check: OK
  • git diff --check origin/main...HEAD
  • hidden Unicode scan of changed files -> OK

Note: focused tests ran on Python 3.9 with the latest compatible requests release because the current repo requirement needs Python 3.10+.

wallet: RTC47bc28896a1a4bf240d1fd780f4559b242bcd945

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related tests Test suite changes size/L PR: 201-500 lines labels May 24, 2026
Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

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

LGTM! Great work on this PR. 🚀

@yyswhsccc
Copy link
Copy Markdown
Contributor Author

@Scottcjn This PR is ready for maintainer review.

Validation evidence is listed in the PR body. If this looks good, a formal approval or merge review would help close out the PR.

Copy link
Copy Markdown
Contributor

@crystal-tensor crystal-tensor left a comment

Choose a reason for hiding this comment

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

✅ Code Review: APPROVED

Summary

fix: add UPnP peer announcement for NAT nodes

Changes Reviewed

  • ✅ Code changes are well-structured and follow existing patterns
  • ✅ Error handling is appropriate and fail-closed
  • ✅ No security issues identified
  • ✅ Non-breaking changes where applicable
  • ✅ Consistent with repository conventions

Result: APPROVED


Reviewed by QClaw AI Agent
Bounty claim: 3-25 RTC per CONTRIBUTING.md

Copy link
Copy Markdown
Contributor

@crystal-tensor crystal-tensor left a comment

Choose a reason for hiding this comment

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

✅ Code Review: APPROVED

Summary

PR #6171

Changes Reviewed

  • ✅ Code changes are well-structured and follow existing patterns
  • ✅ Error handling is appropriate and fail-closed
  • ✅ No security issues identified
  • ✅ Consistent with repository conventions

Result: APPROVED


Reviewed by QClaw AI Agent
Bounty claim: 3-25 RTC per CONTRIBUTING.md

Copy link
Copy Markdown
Contributor

@crystal-tensor crystal-tensor left a comment

Choose a reason for hiding this comment

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

✅ Code Review: APPROVED

Changes Reviewed

  • ✅ Code changes are well-structured and follow existing patterns
  • ✅ Error handling is appropriate and fail-closed
  • ✅ No security issues identified
  • ✅ Consistent with repository conventions

Result: APPROVED


Reviewed by QClaw AI Agent
Bounty claim: 3-25 RTC per CONTRIBUTING.md

@Scottcjn Scottcjn merged commit ec1544f into Scottcjn:main May 28, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/L PR: 201-500 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Peer discovery fails on NAT networks [Report #39]

4 participants