Consider type-level distinction for PeerKeyLocation with known vs unknown address

[sanity]

Problem

PeerKeyLocation currently has a peer_addr: PeerAddr field where PeerAddr can be either Known(SocketAddr) or Unknown. This flexibility is intentional - peers behind NAT don't know their external address until told by the network.

However, this creates a class of bugs where code assumes an address is known when it isn't. Example: #2207 where ConnectResponse.acceptor was created with Unknown address even though the responder must have a known address.

Current Mitigations

1. Runtime debug_assert at construction sites (PR refactor: complete connection-based routing migration (remove PeerId) #2208)
2. Unit tests that verify address is known in responses
3. Defensive handling in places that receive messages (e.g., tracing module)

Proposed Enhancement

Use the type system to distinguish between:

/// Peer location where address may be unknown (for request initiators)
pub struct PeerKeyLocation {
    pub pub_key: TransportPublicKey,
    pub peer_addr: PeerAddr,  // Can be Unknown or Known
}

/// Peer location where address is guaranteed known (for response senders)
pub struct KnownPeerLocation {
    pub pub_key: TransportPublicKey,
    addr: SocketAddr,  // Always known - private field enforced by constructor
}

impl KnownPeerLocation {
    /// Only way to construct - requires known address
    pub fn new(pub_key: TransportPublicKey, addr: SocketAddr) -> Self {
        Self { pub_key, addr }
    }
    
    pub fn socket_addr(&self) -> SocketAddr {
        self.addr
    }
}

impl From<KnownPeerLocation> for PeerKeyLocation {
    fn from(known: KnownPeerLocation) -> Self {
        PeerKeyLocation {
            pub_key: known.pub_key,
            peer_addr: PeerAddr::Known(known.addr),
        }
    }
}

Then message types can specify which they require:

// Joiner initiates - may not know own address
pub struct ConnectRequest {
    pub joiner: PeerKeyLocation,  // Can be Unknown
    // ...
}

// Acceptor responds - must have known address
pub struct ConnectResponse {
    pub acceptor: KnownPeerLocation,  // Compile-time guarantee
}

Benefits

1. Compile-time safety: Can't create ConnectResponse without a known address
2. Self-documenting: Message types express their requirements clearly
3. No runtime overhead: Zero-cost abstraction, same as current approach
4. Gradual migration: Can introduce KnownPeerLocation incrementally

Drawbacks

1. Requires reviewing all message types to determine which needs which
2. Adds another type to the codebase
3. Some messages might legitimately accept either (though this is rare)

Related

• PeerKeyLocation addr() panics when address is unknown #2207 - Bug caused by missing known address requirement
• refactor: complete connection-based routing migration (remove PeerId) #2208 - Added runtime protections and tests

Scope

This is a refactoring improvement, not urgent. The runtime protections in #2208 provide adequate safety. This issue tracks a potential future enhancement for stronger compile-time guarantees.

[AI-assisted - Claude]

[github-actions]

🤖 Auto-labeled

Applied labels:

• T-enhancement (99% confidence)
• E-medium (90% confidence)
• A-networking (95% confidence)
• A-developer-xp (80% confidence)
• S-needs-design (90% confidence)

Reasoning: The issue proposes a refactor to add a new type (KnownPeerLocation) to provide compile-time guarantees about peer addresses — this is an improvement to existing code rather than a bug or new feature, so 'T-enhancement' fits. The change touches message types and construction sites across networking code, requiring review and moderate effort to migrate, so 'E-medium' is appropriate. It clearly concerns peer/address handling and message structures (networking), and also impacts developer experience and API clarity, so 'A-networking' and 'A-developer-xp' apply. Because this is an architectural/typing design change that benefits from discussion and review before implementation, 'S-needs-design' is suggested.

Previous labels: none

If these labels are incorrect, please update them. This helps improve the auto-labeling system.