-
-
Notifications
You must be signed in to change notification settings - Fork 107
Description
Problem
Currently, local client application subscriptions and network-wide subscriptions are conflated in the operations layer (particularly in crates/core/src/operations/subscribe.rs and crates/core/src/operations/update.rs), leading to increased coupling between the transport/websocket layer and the logical network operations.
As noted by @iduartgomez in #2064:
I think fundamentally there is a problem with conflating network subscriptions and local client application subscriptions. IMO we should distinguish them and handle them differently via different flows (e.g. sending internal event notifications for local ones)
This problem has been permeating across multiple bug fixes and changes and looks like we should address the root cause instead of keep working around it.
And in the general PR comment:
Taking an overall look at the PR I think we have successfully managed to couple local subscriptions/updates to network subscriptions/updates, which is ... NOT good. The goal long-term would be to separate them absolutely, we shouldn't have to worry about local connections inside ops/ module neither viceversa at the websocket/contract layer.
Current Architecture Issues
- Mixed concerns in operations layer: The
ops/module handles both network-wide subscription propagation and local client subscriptions in the same code paths. - Websocket/contract layer coupling: The websocket and contract handlers need to understand network operation semantics, when ideally they should be isolated.
- Complex workarounds: Bug fixes have increasingly added workarounds to handle the coupling (e.g., the
allow_selflogic inupdate.rs:690-716to handle local auto-subscribes during UPDATE propagation). - Handshake handler bypass: This stack "has nuked" the handshake handler's role in isolating logically-connected peers from those that are not, further eroding architectural boundaries.
Proposed Solution
Separate local and network subscriptions into distinct flows:
- Network subscriptions handled entirely in
ops/, concerned only with peer-to-peer subscription tree maintenance and routing. - Local subscriptions managed via internal event notifications isolated to the websocket/contract layer without involving network operations.
- Clear boundaries so
ops/no longer needs to know about local client connections and the websocket/contract layer does not need network routing details. - Restore handshake isolation so the handshake handler once again separates transport-level connections from logical network membership.
Benefits
- Simpler reasoning about subscription behavior
- Easier debugging (separate traces for local vs network events)
- Reduced coupling between layers
- Fewer edge cases and workarounds
- Better long-term maintainability
References
- PR feat: harden subscription routing #2064 (where this issue was identified)
crates/core/src/operations/subscribe.rscrates/core/src/operations/update.rs(subscriber broadcast logic withallow_selfworkaround)- websocket/contract handler integration points
- Related to issue Fail fast when peer has no ring location #2069 (pre-join routing semantics)
Priority
High — this architectural coupling is creating recurring bugs and costly workarounds, so we should address it soon after #2064 merges.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status