# TODO Analysis: Circuit v2 Relay Issues

[acul71]

TODO Analysis: Circuit v2 Relay Issues

Last review: August 2025

Overview

This document analyzes four TODO/FIXME issues in the circuit_v2 relay module:

1. Signature generation for production - libp2p/relay/circuit_v2/resources.py:97 ✅ RESOLVED
2. Voucher and signature verification - libp2p/relay/circuit_v2/resources.py:193 ✅ RESOLVED
3. Sophisticated relay selection - libp2p/relay/circuit_v2/transport.py:222 ❌ PENDING
4. Reservation storage and refresh mechanism - libp2p/relay/circuit_v2/transport.py:279 ❌ PENDING

---

Issue 1: Signature Generation for Production ✅ RESOLVED

Issue Location

File: libp2p/relay/circuit_v2/resources.py:97
Method: to_proto()
Status: ✅ RESOLVED by PR #698

Resolution

PR #698 (link) implemented proper signature generation for production use. The implementation:

• Uses the host's private key for signing reservation vouchers
• Implements domain separation with RELAY_VOUCHER_DOMAIN_SEP for security
• Signs comprehensive data including voucher, expiration time, and domain separator
• Includes proper error handling and logging
• Maintains backward compatibility with fallback to empty signatures

Current Implementation

def to_proto(self) -> PbReservation:
    """Convert the reservation to its protobuf representation."""
    signature = b""
    
    # Sign the voucher if we have a host with a private key
    if self.host is not None:
        try:
            private_key = self.host.get_private_key()
            data_to_sign = self.get_data_to_sign()
            signature = private_key.sign(data_to_sign)
        except Exception as e:
            logger.warning("Failed to sign reservation voucher: %s", str(e))
            signature = b""
    
    return PbReservation(
        expire=int(self.expires_at),
        voucher=self.voucher,
        signature=signature,
    )

---

Issue 2: Voucher and Signature Verification ✅ RESOLVED

Issue Location

File: libp2p/relay/circuit_v2/resources.py:193
Method: verify_reservation()
Status: ✅ RESOLVED by PR #698

Resolution

PR #698 (link) implemented comprehensive voucher and signature verification. The implementation:

• Verifies voucher matches the stored reservation
• Validates signature using the relay's public key
• Implements proper domain separation for signature verification
• Includes detailed logging for debugging
• Handles edge cases and provides clear error messages

Current Implementation

def verify_reservation(self, peer_id: ID, proto_res: PbReservation) -> bool:
    """Verify a reservation from a protobuf message."""
    # Check reservation exists and hasn't expired
    reservation = self._reservations.get(peer_id)
    if not reservation or reservation.is_expired():
        return False
    
    # Verify voucher matches
    if proto_res.voucher != reservation.voucher:
        return False
    
    # Verify signature using relay's public key
    if not proto_res.signature:
        return False
    
    data_to_sign = self._get_data_to_sign(proto_res.voucher, proto_res.expire)
    return self._verify_signature_with_relay_key(data_to_sign, proto_res.signature)

---

Issue 3: Sophisticated Relay Selection

Issue Location

File: libp2p/relay/circuit_v2/transport.py:222
Method: _select_relay()
TODO Comment: # TODO: Implement more sophisticated relay selection

Current Status Analysis

Current Implementation

The _select_relay() method currently uses a simple "first available" strategy:

async def _select_relay(self, peer_info: PeerInfo) -> ID | None:
    """Select an appropriate relay for the given peer."""
    # Try to find a relay
    attempts = 0
    while attempts < self.client_config.max_auto_relay_attempts:
        # Get a relay from the list of discovered relays
        relays = self.discovery.get_relays()
        if relays:
            # TODO: Implement more sophisticated relay selection
            # For now, just return the first available relay
            return relays[0]
        
        # Wait and try discovery
        await trio.sleep(1)
        attempts += 1
    
    return None

The Problem

• No Load Balancing: Always picks the first relay, potentially overloading it
• No Performance Consideration: Doesn't consider latency or bandwidth
• No Reliability: Doesn't consider relay health or availability
• No Optimization: Doesn't use historical performance data

Cross-Implementation Analysis

Go Implementation Status

• Current Strategy: Random selection with TODO for ping-based improvement
• Location: /p2p/host/autorelay/relay_finder.go:753-770
• Status: Also has TODO for better selection strategy
• Comment: // TODO: better relay selection strategy; this just selects random relays, but we should probably use ping latency as the selection metric

JavaScript Implementation Status

• Current Strategy: Discovery-based with queue management
• Location: /packages/transport-circuit-relay-v2/src/transport/discovery.ts
• Status: Uses topology registration, no sophisticated selection
• Approach: Event-driven discovery rather than complex selection algorithms

What Must Be Done

Required Changes

1. Relay Scoring: Implement a scoring system for relays
2. Load Balancing: Distribute connections across multiple relays
3. Performance Metrics: Consider latency, bandwidth, and reliability
4. Health Monitoring: Track relay availability and performance

Implementation Options

Option A: Simple Round-Robin Selection (Recommended)

async def _select_relay(self, peer_info: PeerInfo) -> ID | None:
    """Select an appropriate relay for the given peer."""
    attempts = 0
    while attempts < self.client_config.max_auto_relay_attempts:
        relays = self.discovery.get_relays()
        if relays:
            # Simple round-robin selection for load balancing
            selected_index = attempts % len(relays)
            return relays[selected_index]
        
        await trio.sleep(1)
        attempts += 1
    
    return None

Option B: Reservation-Priority Selection

async def _select_relay(self, peer_info: PeerInfo) -> ID | None:
    """Select an appropriate relay for the given peer."""
    attempts = 0
    while attempts < self.client_config.max_auto_relay_attempts:
        relays = self.discovery.get_relays()
        if relays:
            # Prioritize relays with active reservations
            relays_with_reservations = []
            other_relays = []
            
            for relay_id in relays:
                relay_info = self.discovery.get_relay_info(relay_id)
                if relay_info and relay_info.has_reservation:
                    relays_with_reservations.append(relay_id)
                else:
                    other_relays.append(relay_id)
            
            # Return first available relay with reservation, or fallback to others
            if relays_with_reservations:
                return relays_with_reservations[attempts % len(relays_with_reservations)]
            elif other_relays:
                return other_relays[attempts % len(other_relays)]
        
        await trio.sleep(1)
        attempts += 1
    
    return None

Option C: Future Enhancement - Ping-Based Selection

async def _select_relay(self, peer_info: PeerInfo) -> ID | None:
    """Select an appropriate relay for the given peer."""
    attempts = 0
    while attempts < self.client_config.max_auto_relay_attempts:
        relays = self.discovery.get_relays()
        if relays:
            # Measure ping latency for each relay
            relay_latencies = []
            for relay_id in relays:
                try:
                    latency = await self._measure_relay_latency(relay_id)
                    relay_latencies.append((relay_id, latency))
                except Exception:
                    # Skip relays that don't respond to ping
                    continue
            
            if relay_latencies:
                # Sort by latency and return the fastest
                relay_latencies.sort(key=lambda x: x[1])
                return relay_latencies[0][0]
        
        await trio.sleep(1)
        attempts += 1
    
    return None

async def _measure_relay_latency(self, relay_id: ID) -> float:
    """Measure the latency to a relay using ping."""
    # Implementation would use libp2p ping protocol
    # This is a placeholder for future enhancement
    pass

Impact Analysis

• Performance: Medium - Better load distribution
• Load Distribution: High - Prevents relay overload
• Complexity: Low-Medium - Simple to implement
• Maintenance: Low - Minimal ongoing maintenance required

Recommendation

Start with Option A (Round-Robin) because:

1. Simple to implement - Minimal code changes required
2. Effective load balancing - Distributes connections across available relays
3. Consistent with other implementations - Go and JavaScript also use simple strategies
4. Future-proof - Can be enhanced later with ping-based selection
5. Fallback strategy - Can implement fallback mechanisms if scored selection fails

Priority: LOW - This is an optimization issue, not a critical functionality problem. Both Go and JavaScript implementations also use simple selection strategies and have TODOs for future improvements.

Developer Feedback: The PR developer suggests implementing fallback mechanisms (e.g., random selection) if more sophisticated selection strategies fail, which aligns with the progressive enhancement approach.

---

Issue 4: Reservation Storage and Refresh Mechanism

Issue Location

File: libp2p/relay/circuit_v2/transport.py:279
Method: _make_reservation()
TODO Comment: # TODO: Implement reservation storage and refresh mechanism

Current Status Analysis

Current Implementation

The _make_reservation() method currently doesn't store or manage reservations:

async def _make_reservation(self, stream: INetStream, relay_peer_id: ID) -> bool:
    """Make a reservation with a relay."""
    try:
        # Send reservation request
        reserve_msg = HopMessage(
            type=HopMessage.RESERVE,
            peer=self.host.get_id().to_bytes(),
        )
        await stream.write(reserve_msg.SerializeToString())
        
        # Read response
        resp_bytes = await stream.read()
        resp = HopMessage()
        resp.ParseFromString(resp_bytes)
        
        # Check status
        status_code = getattr(resp.status, "code", StatusCode.OK)
        if status_code != StatusCode.OK:
            return False
        
        # Store reservation info
        # TODO: Implement reservation storage and refresh mechanism
        return True
        
    except Exception as e:
        logger.error("Error making reservation: %s", str(e))
        return False

The Problem

• No Persistence: Reservations are lost after restart
• No Refresh: Reservations expire without renewal
• No Tracking: Can't monitor reservation status
• No Optimization: Can't reuse existing reservations
• Specification Violation: Doesn't comply with Circuit Relay v2 spec requirements

Cross-Implementation Analysis

Go Implementation Status

• FULLY IMPLEMENTED - Comprehensive reservation management
• Location: /p2p/host/autorelay/relay_finder.go
• Key Features:
  • Persistent storage with map[peer.ID]*circuitv2.Reservation
  • Scheduled background refresh with rsvpRefreshInterval = time.Minute
  • Connection protection with autorelayTag
  • Parallel refresh with errgroup.Group
  • Event-driven architecture

JavaScript Implementation Status

• FULLY IMPLEMENTED - Event-driven reservation management
• Location: /packages/transport-circuit-relay-v2/src/transport/reservation-store.ts
• Key Features:
  • Persistent storage with PeerMap<RelayEntry>
  • Timeout-based refresh with setTimeout
  • Connection protection with KEEP_ALIVE_TAG
  • Queue-based concurrency control
  • Comprehensive event system

What Must Be Done

Required Changes (Based on Spec and Other Implementations)

1. Reservation Storage: Store reservation details persistently
2. Automatic Refresh: Implement background refresh mechanism
3. Connection Protection: Prevent accidental connection closure
4. Error Recovery: Handle failed reservations and retries
5. Event System: Implement event-driven architecture

Implementation Options

Option A: Go-Style Implementation (Recommended)

class CircuitV2Transport(ITransport):
    def __init__(self, host: IHost, protocol: CircuitV2Protocol, config: RelayConfig):
        # ... existing code ...
        self._reservations: dict[ID, ReservationInfo] = {}
        self._refresh_task: trio.Task | None = None
        self._refresh_interval = 60  # 1 minute, like Go implementation

class ReservationInfo:
    def __init__(self, relay_id: ID, expires_at: int, voucher: bytes, signature: bytes):
        self.relay_id = relay_id
        self.expires_at = expires_at
        self.voucher = voucher
        self.signature = signature
        self.created_at = time.time()
        self.connection_id: str | None = None

async def _make_reservation(self, stream: INetStream, relay_peer_id: ID) -> bool:
    """Make a reservation with a relay."""
    try:
        # ... existing request code ...
        
        if status_code == StatusCode.OK:
            # Store reservation information
            if resp.HasField("reservation"):
                reservation_info = ReservationInfo(
                    relay_id=relay_peer_id,
                    expires_at=resp.reservation.expire,
                    voucher=resp.reservation.voucher,
                    signature=resp.reservation.signature,
                )
                reservation_info.connection_id = stream.id
                self._reservations[relay_peer_id] = reservation_info
                
                # Protect the connection (like Go's autorelayTag)
                await self._protect_connection(relay_peer_id)
                
                # Start refresh task if not already running
                if not self._refresh_task or self._refresh_task.done():
                    self._refresh_task = trio.lowlevel.spawn_system_task(
                        self._refresh_reservations
                    )
            
            return True
        
        return False
        
    except Exception as e:
        logger.error("Error making reservation: %s", str(e))
        return False

async def _refresh_reservations(self) -> None:
    """Background task to refresh expiring reservations (Go-style)."""
    while True:
        try:
            now = time.time()
            to_refresh = []
            
            # Find reservations that need refresh (2 minutes before expiry, like Go)
            for relay_id, reservation in self._reservations.items():
                time_until_expiry = reservation.expires_at - now
                if time_until_expiry < 120:  # 2 minutes before expiry
                    to_refresh.append(relay_id)
            
            # Refresh reservations in parallel (like Go's errgroup)
            if to_refresh:
                async with trio.open_nursery() as nursery:
                    for relay_id in to_refresh:
                        nursery.start_soon(self._refresh_reservation, relay_id)
            
            # Sleep for refresh interval (1 minute, like Go)
            await trio.sleep(self._refresh_interval)
            
        except Exception as e:
            logger.error("Error in reservation refresh: %s", str(e))
            await trio.sleep(self._refresh_interval)

async def _protect_connection(self, relay_id: ID) -> None:
    """Protect connection to relay (like Go's autorelayTag)."""
    # Implementation would use connection manager to protect the connection
    # This prevents accidental closure of relay connections
    pass

Option B: JavaScript-Style Implementation

class ReservationStore:
    def __init__(self, host: IHost, config: RelayConfig):
        self.host = host
        self.config = config
        self._reservations: dict[ID, RelayEntry] = {}
        self._refresh_timeouts: dict[ID, trio.Task] = {}
        
        # Constants from JavaScript implementation
        self.REFRESH_WINDOW = 10 * 60 * 1000  # 10 minutes
        self.REFRESH_TIMEOUT = 5 * 60 * 1000   # 5 minutes
        self.REFRESH_TIMEOUT_MIN = 30 * 1000   # 30 seconds

class RelayEntry:
    def __init__(self, relay_id: ID, reservation: PbReservation, connection_id: str):
        self.relay_id = relay_id
        self.reservation = reservation
        self.connection_id = connection_id
        self.timeout_task: trio.Task | None = None

async def add_relay(self, relay_id: ID) -> bool:
    """Add a relay reservation (JavaScript-style)."""
    try:
        # Check existing reservation
        existing = self._reservations.get(relay_id)
        if existing:
            expiration_ms = (existing.reservation.expire - time.time()) * 1000
            if expiration_ms > self.REFRESH_WINDOW:
                return True  # Still valid
        
        # Create new reservation
        reservation = await self._create_reservation(relay_id)
        connection_id = await self._get_connection_id(relay_id)
        
        # Calculate timeout duration (like JavaScript)
        expiration_ms = (reservation.expire - time.time()) * 1000
        timeout_duration = min(
            max(expiration_ms - self.REFRESH_TIMEOUT, self.REFRESH_TIMEOUT_MIN),
            2**31 - 1  # JavaScript setTimeout limit
        )
        
        # Schedule refresh timeout
        timeout_task = trio.lowlevel.spawn_system_task(
            self._schedule_refresh, relay_id, timeout_duration / 1000
        )
        
        # Store reservation
        entry = RelayEntry(relay_id, reservation, connection_id)
        entry.timeout_task = timeout_task
        self._reservations[relay_id] = entry
        
        # Protect connection with tag (like JavaScript's KEEP_ALIVE_TAG)
        await self._protect_connection_with_tag(relay_id, expiration_ms / 1000)
        
        return True
        
    except Exception as e:
        logger.error("Error adding relay %s: %s", relay_id, str(e))
        return False

async def _schedule_refresh(self, relay_id: ID, delay_seconds: float) -> None:
    """Schedule a refresh timeout (JavaScript-style)."""
    await trio.sleep(delay_seconds)
    
    try:
        logger.debug("Refreshing reservation for relay %s", relay_id)
        await self.add_relay(relay_id)  # Refresh by re-adding
    except Exception as e:
        logger.error("Could not refresh reservation for relay %s: %s", relay_id, str(e))
        await self._remove_reservation(relay_id)

Option C: Hybrid Approach (Best of Both)

class ReservationManager:
    def __init__(self, host: IHost, config: RelayConfig):
        self.host = host
        self.config = config
        self._reservations: dict[ID, ReservationInfo] = {}
        self._refresh_task: trio.Task | None = None
        
        # Go-style constants
        self.REFRESH_INTERVAL = 60  # 1 minute
        self.EXPIRATION_SLACK = 120  # 2 minutes
        
        # JavaScript-style constants
        self.REFRESH_WINDOW = 600  # 10 minutes
        self.REFRESH_TIMEOUT = 300  # 5 minutes

    async def add_reservation(self, relay_id: ID, reservation: PbReservation) -> None:
        """Add a reservation with both Go and JavaScript features."""
        # Store reservation
        info = ReservationInfo(relay_id, reservation)
        self._reservations[relay_id] = info
        
        # Protect connection (Go-style)
        await self._protect_connection(relay_id)
        
        # Schedule refresh (JavaScript-style timeout + Go-style background task)
        await self._schedule_refresh_timeout(relay_id, reservation.expire)
        
        # Start background refresh task (Go-style)
        if not self._refresh_task or self._refresh_task.done():
            self._refresh_task = trio.lowlevel.spawn_system_task(
                self._background_refresh
            )

    async def _background_refresh(self) -> None:
        """Go-style background refresh task."""
        while True:
            try:
                now = time.time()
                to_refresh = []
                
                for relay_id, info in self._reservations.items():
                    if now + self.EXPIRATION_SLACK >= info.expires_at:
                        to_refresh.append(relay_id)
                
                # Parallel refresh (Go-style)
                if to_refresh:
                    async with trio.open_nursery() as nursery:
                        for relay_id in to_refresh:
                            nursery.start_soon(self._refresh_reservation, relay_id)
                
                await trio.sleep(self.REFRESH_INTERVAL)
                
            except Exception as e:
                logger.error("Background refresh error: %s", str(e))
                await trio.sleep(self.REFRESH_INTERVAL)

Impact Analysis

• Reliability: High - Prevents reservation expiration (spec compliance)
• Performance: High - Reduces connection setup time
• Complexity: Medium - Requires background task management
• Resource Usage: Low-Medium - Minimal overhead

Recommendation

Start with Option A (Go-Style) because:

1. Spec Compliance: Meets Circuit Relay v2 specification requirements
2. Proven Approach: Based on Go's production-ready implementation
3. Simple to Implement: Straightforward background task approach
4. Effective: Parallel refresh and connection protection
5. Developer Feedback: PR developer confirms this approach aligns with Go and Rust implementations

Priority: CRITICAL - This is a specification compliance issue. The Circuit Relay v2 spec explicitly requires reservation refresh, and current implementation violates this requirement. Both Go and JavaScript have fully implemented this feature.

Developer Note: The PR developer has confirmed that Go and Rust implementations do not use complex database storage, validating the simple in-memory approach.

---

Summary and Recommendations

Priority Order

1. ✅ COMPLETED: Signature generation and verification (Issues 1 & 2)

   • Status: Resolved by PR fix: Fix voucher verification and data transfer limits in Circuit Relay v2 #698
   • Impact: Production-ready security implementation

2. 🔴 CRITICAL: Reservation storage and refresh (Issue 4)

   • Status: Specification compliance issue
   • Impact: Required for Circuit Relay v2 compliance

3. 🟡 LOW: Sophisticated relay selection (Issue 3)

   • Status: Optimization opportunity
   • Impact: Nice-to-have performance improvement

Implementation Strategy

1. ✅ Phase 1: Implement basic signature generation and verification (COMPLETED)
2. 🔴 Phase 2: Add reservation storage with automatic refresh (CRITICAL)
3. 🟡 Phase 3: Implement relay selection optimization (LOW PRIORITY)
4. 🟡 Phase 4: Add comprehensive health monitoring (LOW PRIORITY)

Testing Requirements

• ✅ Unit tests for signature generation/verification (COMPLETED)
• 🔴 Integration tests for reservation lifecycle (CRITICAL)
• 🟡 Performance tests for relay selection algorithms (LOW PRIORITY)
• 🔴 Stress tests for reservation refresh mechanisms (CRITICAL)

Documentation Needs

• ✅ Security considerations for signature implementation (COMPLETED)
• 🔴 Configuration options for reservation management (CRITICAL)
• 🟡 Troubleshooting guide for relay selection (LOW PRIORITY)
• 🔴 Performance tuning for reservation refresh (CRITICAL)

---

Issue Comparison Summary

Issue	Status	Priority	Spec Compliance	Go Implementation	JS Implementation	Python Status	Recommendation
Issue 1: Signature Generation	✅ RESOLVED	N/A	✅ Required	✅ Full	✅ Full	✅ PR #698	Production ready
Issue 2: Voucher Verification	✅ RESOLVED	N/A	✅ Required	✅ Full	✅ Full	✅ PR #698	Production ready
Issue 3: Relay Selection	❌ PENDING	🟡 LOW	❌ Optional	🟡 Simple + TODO	🟡 Simple + TODO	❌ Not implemented	Round-robin approach
Issue 4: Reservation Storage	❌ PENDING	🔴 CRITICAL	✅ Required	✅ Full	✅ Full	❌ Not implemented	Go-style implementation

Cross-Implementation Analysis

Go libp2p Implementation

• Issues 1 & 2: ✅ Fully implemented with proper cryptographic signing
• Issue 3: 🟡 Simple random selection with TODO for ping-based improvement
• Issue 4: ✅ Comprehensive reservation management with background refresh

JavaScript libp2p Implementation

• Issues 1 & 2: ✅ Fully implemented with proper cryptographic signing
• Issue 3: 🟡 Discovery-based selection with TODO for ping-based improvement
• Issue 4: ✅ Event-driven reservation management with timeout-based refresh

Python libp2p Implementation

• Issues 1 & 2: ✅ Resolved by PR fix: Fix voucher verification and data transfer limits in Circuit Relay v2 #698 (production ready)
• Issue 3: ❌ Not implemented (low priority optimization)
• Issue 4: ❌ Not implemented (critical spec compliance issue)

Key Findings

1. Specification Compliance: Issues 1, 2, and 4 are required by the Circuit Relay v2 specification
2. Implementation Maturity: Go and JavaScript have production-ready implementations for all critical issues
3. Python Progress: Python has resolved the security-critical issues (1 & 2) but still lacks reservation management
4. Optimization Opportunities: Issue 3 is an optimization that all implementations plan to improve in the future

Next Steps

1. 🔴 IMMEDIATE: Implement Issue 4 (Reservation Storage) using Go-style approach
2. 🟡 FUTURE: Consider Issue 3 (Relay Selection) optimization when resources permit
3. ✅ MONITOR: Ensure Issues 1 & 2 remain stable in production

Developer Feedback Summary

From PR #698 Developer (guha-rahul):

• Issues 1 & 2: ✅ Confirmed as implemented in PR fix: Fix voucher verification and data transfer limits in Circuit Relay v2 #698
• Issue 3: Suggests implementing fallback mechanisms if scored selection fails (e.g., random selection from available relays)
• Issue 4: Confirms that Go and Rust implementations do not use complex database storage, validating the simple in-memory approach (Option A)

Key Insights:

• The developer's feedback aligns with the cross-implementation analysis
• Simple approaches are preferred over complex solutions
• Fallback mechanisms are important for reliability
• The Go-style implementation for Issue 4 is the right choice

[guha-rahul]

Well since isssue 1 and 2 is already being implemented in #698 , moving on directly to issue 3 is it a good idea to have fallback mechanisms if scored selection fails, try simpler strategies like random selection from available relays. And for issue 4 , my rationale for choosing the simple A is because rust and go impls do not use them

[acul71]

I've update the discussion with current status and your suggestions. See if it's ok.