From ee3d93576543ae721a44f1f602d2d197c862e089 Mon Sep 17 00:00:00 2001 From: Sujal Salekar Date: Sun, 20 Jul 2025 17:36:26 +0000 Subject: [PATCH] Refactor: Replace magic numbers with named constants and enums for clarity and maintainability --- libp2p/relay/circuit_v2/config.py | 20 ++++++++++++++------ libp2p/relay/circuit_v2/discovery.py | 24 +++++++++++++----------- libp2p/relay/circuit_v2/resources.py | 8 ++++++-- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/libp2p/relay/circuit_v2/config.py b/libp2p/relay/circuit_v2/config.py index 3315c74f1..b635e9669 100644 --- a/libp2p/relay/circuit_v2/config.py +++ b/libp2p/relay/circuit_v2/config.py @@ -18,6 +18,14 @@ RelayLimits, ) +# === Constants for relay configuration === +DEFAULT_MIN_RELAYS = 3 +DEFAULT_MAX_RELAYS = 20 +DEFAULT_DISCOVERY_INTERVAL = 300 # seconds +DEFAULT_RESERVATION_TTL = 3600 # seconds +DEFAULT_MAX_CIRCUIT_DURATION = 3600 # seconds +DEFAULT_MAX_CIRCUIT_BYTES = 1024 * 1024 * 1024 # 1GB + @dataclass class RelayConfig: @@ -33,14 +41,14 @@ class RelayConfig: # Discovery configuration bootstrap_relays: list[PeerInfo] = field(default_factory=list) - min_relays: int = 3 - max_relays: int = 20 - discovery_interval: int = 300 # seconds + min_relays: int = DEFAULT_MIN_RELAYS + max_relays: int = DEFAULT_MAX_RELAYS + discovery_interval: int = DEFAULT_DISCOVERY_INTERVAL # seconds # Connection configuration - reservation_ttl: int = 3600 # seconds - max_circuit_duration: int = 3600 # seconds - max_circuit_bytes: int = 1024 * 1024 * 1024 # 1GB + reservation_ttl: int = DEFAULT_RESERVATION_TTL # seconds + max_circuit_duration: int = DEFAULT_MAX_CIRCUIT_DURATION # seconds + max_circuit_bytes: int = DEFAULT_MAX_CIRCUIT_BYTES # 1GB def __post_init__(self) -> None: """Initialize default values.""" diff --git a/libp2p/relay/circuit_v2/discovery.py b/libp2p/relay/circuit_v2/discovery.py index b1310d8df..f2efcf5e5 100644 --- a/libp2p/relay/circuit_v2/discovery.py +++ b/libp2p/relay/circuit_v2/discovery.py @@ -43,10 +43,11 @@ logger = logging.getLogger("libp2p.relay.circuit_v2.discovery") -# Constants +# === Constants for relay discovery === MAX_RELAYS_TO_TRACK = 10 DEFAULT_DISCOVERY_INTERVAL = 60 # seconds STREAM_TIMEOUT = 10 # seconds +PEER_PROTOCOL_TIMEOUT = 5 # seconds # Extended interfaces for type checking @@ -166,19 +167,19 @@ async def discover_relays(self) -> None: continue # Check if peer supports the relay protocol - with trio.move_on_after(5): # Don't wait too long for protocol info + with trio.move_on_after(PEER_PROTOCOL_TIMEOUT): # Don't wait too long for protocol info if await self._supports_relay_protocol(peer_id): await self._add_relay(peer_id) # Limit number of relays we track - if len(self._discovered_relays) > self.max_relays: + if len(self._discovered_relays) > MAX_RELAYS_TO_TRACK: # Sort by last seen time and keep only the most recent ones sorted_relays = sorted( self._discovered_relays.items(), key=lambda x: x[1].last_seen, reverse=True, ) - to_remove = sorted_relays[self.max_relays :] + to_remove = sorted_relays[MAX_RELAYS_TO_TRACK :] for peer_id, _ in to_remove: del self._discovered_relays[peer_id] @@ -234,7 +235,8 @@ async def _check_via_peerstore(self, peer_id: ID) -> bool | None: if not callable(proto_getter): return None - + if peer_id not in peerstore.peer_ids(): + return None try: # Try to get protocols proto_result = proto_getter(peer_id) @@ -283,8 +285,6 @@ async def _check_via_mux(self, peer_id: ID) -> bool | None: return None mux = self.host.get_mux() - if not hasattr(mux, "protocols"): - return None peer_protocols = set() # Get protocols from mux with proper type safety @@ -293,11 +293,13 @@ async def _check_via_mux(self, peer_id: ID) -> bool | None: # Get protocols with proper typing mux_protocols = mux.get_protocols() if isinstance(mux_protocols, (list, tuple)): - available_protocols = list(mux_protocols) + available_protocols = [ + p for p in mux.get_protocols() if p is not None + ] for protocol in available_protocols: try: - with trio.fail_after(2): # Quick check + with trio.fail_after(PEER_PROTOCOL_TIMEOUT): # Quick check # Ensure we have a proper protocol object # Use string representation since we can't use isinstance is_tprotocol = str(type(protocol)) == str(type(TProtocol)) @@ -313,7 +315,7 @@ async def _check_via_mux(self, peer_id: ID) -> bool | None: self._protocol_cache[peer_id] = peer_protocols protocol_str = str(PROTOCOL_ID) - for protocol in peer_protocols: + for protocol in map(TProtocol, peer_protocols): if protocol == protocol_str: return True return False @@ -462,7 +464,7 @@ async def _cleanup_expired(self) -> None: for peer_id, relay_info in self._discovered_relays.items(): # Check if relay hasn't been seen in a while (3x discovery interval) - if now - relay_info.last_seen > self.discovery_interval * 3: + if now - relay_info.last_seen > DEFAULT_DISCOVERY_INTERVAL * 3: to_remove.append(peer_id) continue diff --git a/libp2p/relay/circuit_v2/resources.py b/libp2p/relay/circuit_v2/resources.py index 4da67ec6a..9468be8bd 100644 --- a/libp2p/relay/circuit_v2/resources.py +++ b/libp2p/relay/circuit_v2/resources.py @@ -19,6 +19,10 @@ # Import the protobuf definitions from .pb.circuit_pb2 import Reservation as PbReservation +# === Constants for resource management === +RANDOM_BYTES_LENGTH = 16 # 128 bits of randomness +TIMESTAMP_MULTIPLIER = 1000000 # To convert seconds to microseconds + @dataclass class RelayLimits: @@ -68,8 +72,8 @@ def _generate_voucher(self) -> bytes: # - Peer ID to bind it to the specific peer # - Timestamp for uniqueness # - Hash everything for a fixed size output - random_bytes = os.urandom(16) # 128 bits of randomness - timestamp = str(int(self.created_at * 1000000)).encode() + random_bytes = os.urandom(RANDOM_BYTES_LENGTH) # 128 bits of randomness + timestamp = str(int(self.created_at * TIMESTAMP_MULTIPLIER)).encode() peer_bytes = self.peer_id.to_bytes() # Combine all elements and hash them