Skip to content

fix(pulse-core): reference-count shared subscriptions - #1000

Merged
determined-001 merged 1 commit into
mainfrom
fix/watcher-refcount
Aug 10, 2026
Merged

fix(pulse-core): reference-count shared subscriptions#1000
determined-001 merged 1 commit into
mainfrom
fix/watcher-refcount

Conversation

@determined-001

Copy link
Copy Markdown
Owner

Stacked on #999. Merge #997#998#999 → this.

The bug

subscribe() and subscribeContract() memoise by key, so concurrent callers asking for the same address or contract receive the same Watcher object. unsubscribe() called stop() on it unconditionally.

Watcher.stop() sets _stopped, calls removeAllListeners(), and makes emit() return false without dispatching — so the first caller to leave silently killed every other caller's event flow.

Proven against pulse-core:

✓ expect(wA).toBe(wB)                               // both callers share ONE Watcher
✓ wB live before A disconnects                      // bGot.length === 1
✓ engine.unsubscribe(ADDR) → wB.stopped === true    // A's teardown stops B's watcher
✓ wB.emit(...) === false, bGot.length still 1       // B silently dead

Why it was invisible

In apps/web both SSE routes key on data that is not per-connectionaddress, and `contract:${contractId}` — and unsubscribe on teardown. So when one visitor closed their tab, every other visitor watching the same contract kept an open connection, kept receiving heartbeats, and never received another event. Indistinguishable from a quiet contract.

Cheap to weaponise: connect to a popular contract, disconnect, repeat, and every other viewer's stream goes silent.

The fix

A namespaced refcount map (addr: / contract: / config:, since the three registries have independent key spaces). subscribe*() retains, unsubscribe*() releases, and the watcher stops only on the last release.

Details worth review:

  • Each watcher's stop handler clears its own refcount entry, so a consumer calling watcher.stop() directly cannot strand a count and leave the next subscription for that key permanently unstoppable.
  • release() on an unknown key returns true (stop). An already-torn-down entry must not keep a watcher alive.
  • unsubscribeAll() / unsubscribeAllContracts() are teardown and deliberately ignore counts. unsubscribeAll() now iterates a snapshot, because the stop handlers mutate the registry it was iterating.

Single-subscriber behaviour is unchanged: one subscribe, one unsubscribe, stopped immediately.

Verification

Beyond unit tests — verified end-to-end against a live server. Two clients streaming mainnet USDC (CCW67TSZ...), first disconnected:

=== client B, after A left ===
12551 bytes
data: {"type":"contract.emitted","contractId":"CCW67TSZ...","ledger":63854040,...}
data: {"type":"contract.emitted","contractId":"CCW67TSZ...","ledger":63854040,...}

Client B went on receiving real contract.emitted events rather than going silent.

12 new tests; all 603 existing pulse-core tests still pass.

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
orbital-dev Ready Ready Preview Aug 10, 2026 12:23am

`subscribe()` and `subscribeContract()` memoise by key, so concurrent callers
asking for the same address or contract receive the *same* Watcher object.
`unsubscribe()` called `stop()` on it unconditionally. `Watcher.stop()` sets
`_stopped`, calls `removeAllListeners()`, and makes `emit()` return false
without dispatching — so the first caller to leave silently killed every other
caller's event flow.

The failure is invisible from the outside. In apps/web both SSE routes key on
data that is not per-connection (`address`, `contract:${contractId}`) and
unsubscribe on teardown, so when one visitor closed their tab, every other
visitor watching the same contract kept an open connection, kept receiving
heartbeats, and never received another event — indistinguishable from a quiet
contract.

Adds a namespaced refcount map (`addr:` / `contract:` / `config:`, since the
three registries have independent key spaces). `subscribe*()` retains,
`unsubscribe*()` releases, and the watcher stops only on the last release.

Details worth noting:
- Each watcher's stop handler clears its own refcount entry, so a consumer
  calling `watcher.stop()` directly cannot strand a count and leave the next
  subscription for that key permanently unstoppable.
- `release()` on an unknown key returns true (stop). An already-torn-down
  entry should not keep a watcher alive.
- `unsubscribeAll()` / `unsubscribeAllContracts()` are teardown and
  deliberately ignore counts; `unsubscribeAll()` now iterates a snapshot
  because the stop handlers mutate the registry it was iterating.

Single-subscriber behaviour is unchanged: one subscribe, one unsubscribe,
stopped immediately.

Verified end to end against a live server: two clients streaming mainnet USDC
(CCW67TSZ...), first disconnected, second went on receiving real
`contract.emitted` events rather than going silent.

12 new tests; all 603 existing pulse-core tests still pass.
@determined-001
determined-001 merged commit 03bb6b7 into main Aug 10, 2026
27 checks passed
@determined-001
determined-001 deleted the fix/watcher-refcount branch August 10, 2026 00:25
determined-001 pushed a commit that referenced this pull request Aug 10, 2026
The two repositories are independent - neither is a fork of the other - and
their mains had diverged both ways: 42 commits here, 22 there (contributor
PRs #33-#53: CAP-67 unified ingestion, abi-registry attestation and semantic
taxonomy, the SEP-48 gap memo). This is a real two-way merge rather than a
push, so every contributor commit on the chalkeion-stellar side keeps its
authorship and its merge attribution.

Conflicts and how they were resolved:

- packages/abi-registry/package.json - union of both bin entries
  (abi-registry, abi-registry-generate, orbital-codegen); kept the newer
  @stellar/stellar-sdk ^16.1.0 and added yargs, which the verify CLI needs.
- packages/abi-registry/src/index.ts - both sides only appended exports;
  kept both blocks.
- packages/abi-registry/test/ChainedAbiRegistryClient.test.ts - both sides
  appended a describe block at end of file; kept both.
- packages/pulse-core/src/EventEngine.ts - chalkeion-stellar extracted
  withTimestampDate into src/timestampDate.ts while this side still defined
  it locally, which collided with the new import (TS2440). Dropped the local
  copy - the extracted one is identical - and kept the refcount keys from
  #1000.
- docs/COOKBOOK.md - both sides added recipes over the same numbering.
  Kept this side's 10-18 and renumbered the incoming unified-ingestion
  recipe to 19.
- pnpm-lock.yaml - regenerated from the merged manifests rather than
  hand-resolved.

One integration failure the merge itself produced: openData.test.ts derives
the taxonomy from pulse-core's exhaustive describeEvent switch, and the
incoming CAP-67 work added asset.clawback and fee.incurred without
corresponding open-data records. Added both to generate-open-data.mjs and
regenerated the artifacts (46 records).

Verified on the merged tree: pnpm build, lint, format:check, typecheck of
all five projects, Horizon type-drift check, and the suites for pulse-core
(698), abi-registry (456), pulse-notify (77), anchor-sdk (72),
orbital-indexer (8) and apps/web (40).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants