Skip to content

feat: announce dns p2p addresses#1196

Merged
ndrpp merged 2 commits intomainfrom
feat/announce-dns-p2p-addresses
Feb 6, 2026
Merged

feat: announce dns p2p addresses#1196
ndrpp merged 2 commits intomainfrom
feat/announce-dns-p2p-addresses

Conversation

@ndrpp
Copy link
Member

@ndrpp ndrpp commented Feb 6, 2026

Fixes # .

Changes proposed in this PR:

  • announce dns addresses p2p

@ndrpp ndrpp marked this pull request as ready for review February 6, 2026 09:10
@ndrpp
Copy link
Member Author

ndrpp commented Feb 6, 2026

/run-security-scan

Copy link
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

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

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This PR enhances the P2P address announcement logic within OceanP2P by explicitly handling DNS-based multiaddrs (dns, dns4, dns6, dnsaddr). It ensures that such addresses are announced if they do not resolve to a loopback address (localhost or 127.0.0.1). This change correctly prioritizes and allows public DNS P2P addresses, aligning with the feature of announcing DNS-p2p addresses.

Comments:
• [INFO][style] The addressString = maddr.nodeAddress().address line is duplicated and calculated in both the new DNS block and the subsequent generic IP block. While this is a minor detail and doesn't affect correctness, you could consider extracting this calculation to a single variable at the beginning of the shouldAnnounce method to slightly improve readability and avoid potential redundant calls to maddr.nodeAddress().

@alexcos20 alexcos20 self-requested a review February 6, 2026 10:05
Copy link
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

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

lgtm

@ndrpp ndrpp merged commit 3885e44 into main Feb 6, 2026
18 of 20 checks passed
@ndrpp ndrpp deleted the feat/announce-dns-p2p-addresses branch February 6, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants