hoprd/rest-api/src/session.rs (1)

84-89: Ensure robust error handling in FromStr implementation

When parsing the service ID, consider providing more informative error messages or logging the original input that caused the parse failure. This can aid in debugging.

transport/session/src/lib.rs (1)

75-76: Update documentation for SessionClientConfig

The documentation comment for the target field mentions "Contains target protocol and optionally encrypted target of the session." Since SessionTarget now encapsulates both protocol and target, consider rephrasing the comment for clarity. For example:

-    /// Contains target protocol and optionally encrypted target of the session.
+    /// Specifies the session target, including protocol and destination details.

hoprd/hoprd/src/exit.rs (1)

191-194: Improved error variable naming

Renaming the error variable from e to error enhances code readability and clarity in log messages.

tests/hopr.py (2)

462-462: Line exceeds maximum length

Line 462 exceeds the maximum allowed line length (156 > 120). To enhance readability and comply with style guidelines, consider breaking the function signature across multiple lines.

Apply this diff to fix the line length:

-    async def session_client(self, destination: str, path: str, protocol: str, target: str, listen_on: str = "127.0.0.1:0", capabilities=None, sealed_target=False, service=False):
+    async def session_client(
+        self,
+        destination: str,
+        path: str,
+        protocol: str,
+        target: str,
+        listen_on: str = "127.0.0.1:0",
+        capabilities=None,
+        sealed_target=False,
+        service=False,
+    ):

🧰 Tools

🪛 Ruff (0.8.0)

462-462: Line too long (156 > 120)

(E501)

---

477-477: Simplify conditional logic for actual_target assignment

The nested ternary operators in the assignment of actual_target can reduce readability. Refactoring to a clearer conditional structure would improve maintainability.

Apply this diff to simplify the logic:

-            actual_target = {
-                "Sealed": base64.b64encode(seal_with_peerid(destination, bytes(target,'utf-8'), 50)).decode('ascii')
-            } if sealed_target else { "Service": int(target) } if service else { "Plain": target }
+            if sealed_target:
+                actual_target = {
+                    "Sealed": base64.b64encode(
+                        seal_with_peerid(destination, bytes(target, 'utf-8'), 50)
+                    ).decode('ascii')
+                }
+            elif service:
+                actual_target = { "Service": int(target) }
+            else:
+                actual_target = { "Plain": target }

tests/test_session.py (3)

380-382: Enhance docstring to describe loopback service functionality.

The current docstring focuses on buffer size which is common to all UDP tests. Consider adding information about the loopback service being tested and its purpose.

-    """
-    HOPR UDP socket buffers are set to 462 bytes to mimic the underlying MTU of the HOPR protocol.
-    """
+    """
+    Tests the UDP loopback service (Service ID: 0) which allows the Exit node to send data
+    received on a session back to itself, eliminating the need for a separate echo service.
+    
+    Note: HOPR UDP socket buffers are set to 462 bytes to mimic the underlying MTU of the HOPR protocol.
+    """

---

394-401: Fix string formatting and remove stale comment.

Two minor issues to address:

1. The f-string for target doesn't need string interpolation
2. The commented logging line references an undefined variable dst_sock_port

-    src_sock_port = await src_peer.api.session_client(dest_peer.peer_id, path={"Hops": 0}, protocol='udp',
-                                                      target=f"0", service=True)
+    src_sock_port = await src_peer.api.session_client(dest_peer.peer_id, path={"Hops": 0}, protocol='udp',
+                                                      target="0", service=True)

-    #logging.info(f"session to {dst_sock_port} opened successfully")
+    #logging.info("session opened successfully")

🧰 Tools

🪛 Ruff (0.8.0)

397-397: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

403-426: Fix line length issue.

The code logic is correct, but there's a line that exceeds the maximum length of 120 characters.

-    assert await src_peer.api.session_close_client(protocol='udp', bound_ip='127.0.0.1', bound_port=src_sock_port) is True
+    assert await src_peer.api.session_close_client(
+        protocol='udp',
+        bound_ip='127.0.0.1',
+        bound_port=src_sock_port
+    ) is True

🧰 Tools

🪛 Ruff (0.8.0)

425-425: Line too long (122 > 120)

(E501)

Configuration used: .coderabbit.yaml
Review profile: CHILL

tests/test_session.py (2)

374-424: LGTM! Well-structured test for the loopback service.

The test function effectively verifies the UDP loopback service functionality by sending and receiving data through service ID 0.

Consider implementing a stress test variant of this function to verify the loopback service under high load conditions, as suggested in the previous review.

🧰 Tools

🪛 Ruff (0.8.0)

396-396: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

396-396: Remove unnecessary f-string prefix.

The string "0" doesn't contain any placeholders, so the f prefix can be removed.

-    src_sock_port = await src_peer.api.session_client(dest_peer.peer_id, path={"Hops": 0}, protocol='udp',
-                                                      target=f"0", service=True)
+    src_sock_port = await src_peer.api.session_client(dest_peer.peer_id, path={"Hops": 0}, protocol='udp',
+                                                      target="0", service=True)

🧰 Tools

🪛 Ruff (0.8.0)

396-396: f-string without any placeholders

Remove extraneous f prefix

(F541)

Configuration used: .coderabbit.yaml
Review profile: CHILL