transport/api/src/config.rs (3)

168-174: Add documentation for the session-related constants.

The constants effectively replace magic numbers and have reasonable values. Consider adding documentation to explain the rationale behind these values and any constraints they must satisfy.

Add rustdoc comments for each constant:

+/// Default session idle timeout (3 minutes).
 const DEFAULT_SESSION_IDLE_TIMEOUT: Duration = Duration::from_secs(180);

+/// Minimum allowed session idle timeout (1 minute).
 const SESSION_IDLE_MIN_TIMEOUT: Duration = Duration::from_secs(60);

+/// Default delay between session establishment retry attempts.
 const DEFAULT_SESSION_ESTABLISH_RETRY_DELAY: Duration = Duration::from_secs(2);

+/// Default maximum number of session establishment retry attempts.
 const DEFAULT_SESSION_ESTABLISH_MAX_RETRIES: u32 = 3;

---

Line range hint 189-193: Enhance the validation error message.

The error message could be more descriptive by including the minimum allowed value.

-        Err(ValidationError::new("session idle timeout is too low"))
+        Err(ValidationError::new(&format!(
+            "session idle timeout must be at least {} seconds",
+            SESSION_IDLE_MIN_TIMEOUT.as_secs()
+        )))

---

Line range hint 205-224: Extract validation range limits as constants.

For consistency with other constants and to make the limits more maintainable, consider extracting the validation range for establish_max_retries.

+/// Minimum allowed value for session establishment retries.
+const MIN_SESSION_ESTABLISH_RETRIES: u32 = 0;
+
+/// Maximum allowed value for session establishment retries.
+const MAX_SESSION_ESTABLISH_RETRIES: u32 = 20;
+
 #[validate(range(min = 0, max = 20))]
 #[default(DEFAULT_SESSION_ESTABLISH_MAX_RETRIES)]
 #[serde(default = "default_session_establish_max_retries")]

Then update the validation:

-#[validate(range(min = 0, max = 20))]
+#[validate(range(min = "MIN_SESSION_ESTABLISH_RETRIES", max = "MAX_SESSION_ESTABLISH_RETRIES"))]

common/network-types/src/session/protocol.rs (3)

311-315: Consider enhancing documentation for MAX_MESSAGE_SIZE

The new constant MAX_MESSAGE_SIZE has been introduced. Providing additional details in the documentation comments about its purpose and how it relates to Segment::MAXIMUM_SIZE would improve code readability and help other developers understand its significance.

---

428-429: Define the reserved bits mask as a constant for clarity

The mask 0b111111_0000000000 used to extract reserved bits could be defined as a named constant (e.g., RESERVED_BITS_MASK). This would improve code readability and make future maintenance easier.

---

501-501: Explain the magic number in the test assertion

In the test assertion assert!(SessionMessage::<0>::MAX_MESSAGE_SIZE < 2048);, the magic number 2048 is used without context. Consider defining it as a constant or adding a comment to explain its significance, enhancing the clarity of the test.

Configuration used: .coderabbit.yaml
Review profile: CHILL

common/network-types/src/session/protocol.rs (1)

500-501: Consider adding specific test cases for size validation.

While the basic size constraint is tested, consider adding test cases that specifically verify:

1. Messages exactly at MAX_MESSAGE_SIZE
2. Messages exceeding MAX_MESSAGE_SIZE
3. Messages with non-zero reserved bits

#[test]
fn session_message_should_validate_size_and_reserved_bits() {
    // Test maximum allowed size
    let msg = create_message_with_size(SessionMessage::<0>::MAX_MESSAGE_SIZE);
    assert!(SessionMessage::try_from(&msg[..]).is_ok());

    // Test exceeding maximum size
    let msg = create_message_with_size(SessionMessage::<0>::MAX_MESSAGE_SIZE + 1);
    assert!(matches!(
        SessionMessage::try_from(&msg[..]),
        Err(SessionError::IncorrectMessageLength)
    ));

    // Test non-zero reserved bits
    let mut msg = create_valid_message();
    set_reserved_bits(&mut msg, 0b100000_0000000000);
    assert!(matches!(
        SessionMessage::try_from(&msg[..]),
        Err(SessionError::ParseError)
    ));
}

Configuration used: .coderabbit.yaml
Review profile: CHILL

common/network-types/src/session/protocol.rs (1)

500-501: Consider documenting the significance of 2048.

While the test verifies that MAX_MESSAGE_SIZE is less than 2048, it would be helpful to document why this specific value was chosen as the upper bound.

-        assert!(SessionMessage::<0>::MAX_MESSAGE_SIZE < 2048);
+        // Ensure MAX_MESSAGE_SIZE fits within 11 bits (< 2048) since we reserve 
+        // the upper 6 bits of the u16 message length field
+        assert!(SessionMessage::<0>::MAX_MESSAGE_SIZE < 2048);

Configuration used: .coderabbit.yaml
Review profile: CHILL