refactor: wire protocol cleanup and NAT routing improvements #2191
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Consolidates approved changes from PRs #2169 and #2171: - Remove ObservedAddr in favor of std::net::SocketAddr - Add PeerAddr enum to explicitly represent known/unknown addresses - Update PeerKeyLocation to use PeerAddr for NAT scenarios - Clean up wire protocol types for address handling - Remove unused transient_manager module This is part of the larger #2164 effort to simplify peer identity. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
sanity
added a commit
that referenced
this pull request
Dec 2, 2025
Consolidates changes from PRs #2172, #2174, and #2175: This builds on PR #2191 (wire protocol cleanup) and adds: - Fix seeding/subscribe operations to handle PeerAddr::Unknown for NAT scenarios - Gateway properly fills in observed addresses from packet source - Improved subscriber address tracking in seeding manager - Update live_tx and connection tests for new address model NOTE: This PR requires review - previous PRs (#2174, #2175) had CHANGES_REQUESTED from Nacho. Submitting consolidated changes for fresh review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The previous stacked PRs (#2169 → #2171 → #2172 → #2174 → #2175) ran into git history divergence issues after squash-merging #2167. This made it impossible to cleanly merge the subsequent approved changes.
This Solution
This PR consolidates the approved changes from PRs #2169 and #2171 (both approved by Nacho) into a single clean PR against main:
Changes from PR #2169 (wire protocol cleanup):
ObservedAddrin favor ofstd::net::SocketAddrtransient_managermoduleChanges from PR #2171 (NAT routing improvements):
PeerAddrenum to explicitly represent known/unknown addressesPeerKeyLocationto usePeerAddrfor NAT scenariosContext
This is part of issue #2164 (peer identity simplification). The original stacked PR approach was blocked because:
This PR recreates the approved changes by applying the diff from the intermediate branch to a fresh branch based on main.
Testing
Related
[AI-assisted - Claude]