Repository navigation
test(iroh): verify custom relay round trips - #8482
Conversation
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static func requiredSecretKey(_ name: String) throws -> CmxIrohSecretKey { | ||
| let value = try required(name) | ||
| guard value.utf8.count == 64 else { | ||
| throw EnvironmentError.invalid(name) | ||
| } | ||
| var bytes = Data(capacity: 32) | ||
| var index = value.startIndex | ||
| while index < value.endIndex { | ||
| let next = value.index(index, offsetBy: 2) | ||
| guard let byte = UInt8(value[index ..< next], radix: 16) else { | ||
| throw EnvironmentError.invalid(name) | ||
| } | ||
| bytes.append(byte) | ||
| index = next | ||
| } | ||
| return try CmxIrohSecretKey(bytes: bytes) | ||
| } |
There was a problem hiding this comment.
String.index(_:offsetBy:) without limitedBy: traps instead of throwing when the string has fewer than 64 grapheme clusters but exactly 64 UTF-8 bytes (e.g., 62 ASCII hex chars + one two-byte UTF-8 character). The last loop iteration calls value.index(index, offsetBy: 2) past endIndex, producing a fatal error rather than the expected EnvironmentError.invalid. Using limitedBy: lets the guard catch it cleanly.
| static func requiredSecretKey(_ name: String) throws -> CmxIrohSecretKey { | |
| let value = try required(name) | |
| guard value.utf8.count == 64 else { | |
| throw EnvironmentError.invalid(name) | |
| } | |
| var bytes = Data(capacity: 32) | |
| var index = value.startIndex | |
| while index < value.endIndex { | |
| let next = value.index(index, offsetBy: 2) | |
| guard let byte = UInt8(value[index ..< next], radix: 16) else { | |
| throw EnvironmentError.invalid(name) | |
| } | |
| bytes.append(byte) | |
| index = next | |
| } | |
| return try CmxIrohSecretKey(bytes: bytes) | |
| } | |
| static func requiredSecretKey(_ name: String) throws -> CmxIrohSecretKey { | |
| let value = try required(name) | |
| guard value.utf8.count == 64 else { | |
| throw EnvironmentError.invalid(name) | |
| } | |
| var bytes = Data(capacity: 32) | |
| var index = value.startIndex | |
| while index < value.endIndex { | |
| guard let next = value.index(index, offsetBy: 2, limitedBy: value.endIndex) else { | |
| throw EnvironmentError.invalid(name) | |
| } | |
| guard let byte = UInt8(value[index ..< next], radix: 16) else { | |
| throw EnvironmentError.invalid(name) | |
| } | |
| bytes.append(byte) | |
| index = next | |
| } | |
| return try CmxIrohSecretKey(bytes: bytes) | |
| } |
| #expect(firstAddress.pathHints.map(\.value) == [relayURL]) | ||
| #expect(secondAddress.pathHints.map(\.value) == [relayURL]) |
There was a problem hiding this comment.
relayURL. If the FFI layer surfaces any additional hint alongside the relay URL (a secondary relay URL, a loopback address, etc.), these fail even though relay selection is correct. The polling helper relayAddress already uses contains(where:) to confirm relay presence; the subsequent strict equality may overconstrain the assertion and produce spurious failures in environments where the endpoint discovers extra hints.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| let deadline = Date().addingTimeInterval(CmxIrohCustomRelayLiveEnvironment.timeout) | ||
| while Date() < deadline { | ||
| let address = await endpoint.address() | ||
| if address.pathHints.contains(where: { | ||
| $0.kind == .relayURL && $0.value == relayURL | ||
| }) { | ||
| return address | ||
| } | ||
| if !(await endpoint.isHealthy()) { | ||
| throw LiveTestError.endpointClosed | ||
| } | ||
| try await Task.sleep(for: .milliseconds(100)) | ||
| } | ||
| throw LiveTestError.relayAddressTimedOut |
There was a problem hiding this comment.
Date()-based deadline tracking is inconsistent with the rest of the timeout strategy. relayAddress and relayPath both compute their deadline via Date().addingTimeInterval(...) which reflects wall-clock time and can jump backward on an NTP correction. connectPair already uses ContinuousClock().sleep, which is monotonic. A backward clock jump during a test would cause the loop to exit immediately with a timeout error rather than waiting the intended duration.
1 participant
Adds opt-in live gates that:
Verification:
To run authenticated endpoint-bound coverage, provide
CMUX_IROH_CUSTOM_RELAY_BOUND_URL,CMUX_IROH_CUSTOM_RELAY_FIRST_SECRET_KEY_HEX,CMUX_IROH_CUSTOM_RELAY_FIRST_TOKEN,CMUX_IROH_CUSTOM_RELAY_SECOND_SECRET_KEY_HEX, andCMUX_IROH_CUSTOM_RELAY_SECOND_TOKEN, plusCMUX_IROH_CUSTOM_RELAY_LIVE=1.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds opt-in live tests that verify custom relay round trips in
CmuxIrohTransport. Tests assert exact relay selection and a bidirectional request/response over one stream across unauthenticated, static-token, and endpoint-bound modes.New Features
Migration
CMUX_IROH_CUSTOM_RELAY_LIVE=1(optionalCMUX_IROH_CUSTOM_RELAY_TIMEOUT, default 10s).CMUX_IROH_CUSTOM_RELAY_STATIC_URL,CMUX_IROH_CUSTOM_RELAY_STATIC_TOKEN.CMUX_IROH_CUSTOM_RELAY_NO_TOKEN_URL.CMUX_IROH_CUSTOM_RELAY_BOUND_URL,CMUX_IROH_CUSTOM_RELAY_FIRST_SECRET_KEY_HEX,CMUX_IROH_CUSTOM_RELAY_FIRST_TOKEN,CMUX_IROH_CUSTOM_RELAY_SECOND_SECRET_KEY_HEX,CMUX_IROH_CUSTOM_RELAY_SECOND_TOKEN.Written for commit b63de38. Summary will update on new commits.