feat(ocap-kernel): bound relay hints in OCAP URLs and relay pool size#929
Merged
Conversation
Cap relay hints embedded in OCAP URLs to 3 (MAX_URL_RELAY_HINTS) with priority selection (bootstrap first, then most recently seen). Bound the relay pool to 20 entries (MAX_KNOWN_RELAYS) with eviction of oldest non-bootstrap entries. This prevents unbounded URL growth as kernels exchange relay hints across the network. Closes #889 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…g, add tests - Avoid direct mutation of potentially-hardened entries in initRemoteIdentity bootstrap clearing loop (use spread instead) - Extract getRelayEntries as local function in makeKernelStore to avoid fragile this binding in getKnownRelayAddresses - Add test: init clears bootstrap flag on relays removed from bootstrap set - Add test: init enforces MAX_KNOWN_RELAYS pool cap Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sirtimid
force-pushed
the
sirtimid/bounded-relay-management
branch
from
6c18881 to
1690fd8
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1690fd8. Configure here.
…rable, fix review issues Extract relay and remote identity store methods from store/index.ts into store/methods/relay.ts following the existing methods pattern. Make MAX_URL_RELAY_HINTS and MAX_KNOWN_RELAYS configurable via RemoteCommsOptions (defaults: 3 and 20), threaded through the full RPC handler -> Kernel -> initRemoteIdentity options flow. Address all review findings: add write-path validation with superstruct RelayEntryStruct, wrap JSON.parse with contextual error, log migration and eviction events, warn when bootstrap count exceeds pool cap, document Date.now() SES assumption and default value rationale, add missing tests for sorting stability, bootstrap-exceeds-cap, mixed legacy format, and configurable caps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address review findings across the bounded relay management feature: tighten RelayEntryStruct to reject NaN/Infinity/negative/empty values, unify read/write validation paths, harden initRemoteIdentity return value, add defensive cap validation, prevent mnemonic leaking to platform services, simplify RPC handler and options passing, fix inaccurate SES/hardened-object comments and stale JSDoc references, and add missing test coverage for edge cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
grypez
previously requested changes
Apr 13, 2026
Contributor
grypez
left a comment
grypez
left a comment
There was a problem hiding this comment.
The code complexity seems high for what amounts to a FIFO queue on relay addresses. I think we are due a review of our connection logic. In context of the codebase, no issues. One nit.
Replace inline Object.fromEntries filter with the existing ifDefined utility from @metamask/kernel-utils, as suggested in PR review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
@grypez It's not a simple FIFO queue, the complexity exists because there are two tiers of relays with different eviction semantics: Bootstrap relays and Learned relays. But I agree, a holistic look at the connection layer could probably simplify things. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OCAP URLs embed relay multiaddrs as discovery hints. After #887 propagated these hints on redemption, the relay list in URLs (and in the KV store) grows unboundedly as kernels encounter new relays through URL exchange. This PR bounds both the URL hint count and the relay pool size.
After investigating whether a gossip protocol was needed (#889), the conclusion was that the problem is URL size, not relay discovery — passive propagation via URL exchange is sufficient at current scale. A gossip protocol can be added later if needed.
Changes
RelayEntrytype withaddr,lastSeen,isBootstrapmetadata for prioritized relay selectionMAX_URL_RELAY_HINTS(3), selecting bootstrap relays first, then most recently seenMAX_KNOWN_RELAYS(20) with eviction of oldest non-bootstrap entriesstring[]relay storage format toRelayEntry[]getRelayEntries()/setRelayEntries()/getKnownRelayAddresses()replacegetKnownRelays()/setKnownRelays()Testing
All changes are covered by unit tests. New tests verify URL hint capping, bootstrap relay priority in selection, pool cap enforcement (in both
addKnownRelaysandinitRemoteIdentity), eviction of oldest non-bootstrap entries,lastSeentimestamp updates on re-observation, bootstrap flag clearing on re-init with a different set, and legacy storage format auto-migration. Existing tests were updated for the new store API. All 82 tests pass, lint and build are clean.Closes #889
Note
Medium Risk
Changes relay persistence and selection logic used for OCAP URL issuance, including an on-read KV migration and eviction behavior that could affect connectivity if misconfigured or buggy.
Overview
OCAP URL issuance now limits embedded relay hints (default
3) by selecting relays from a prioritized pool (bootstrap first, then most recently seen).Relay storage is refactored from a raw
string[]toRelayEntry[](addr,lastSeen,isBootstrap), with a bounded relay pool (default20) that evicts oldest non-bootstrap entries and merges/updates bootstrap relays on init; legacyknownRelaysdata is auto-migrated on read.initRemoteComms/RPC addsmaxUrlRelayHintsandmaxKnownRelays, strips undefined params viaifDefined, and avoids passingmnemonicto platform services; tests are updated/expanded for capping, ordering, eviction, and migration.Reviewed by Cursor Bugbot for commit 9bbc7e9. Bugbot is set up for automated code reviews on this repo. Configure here.