Wire CIP-103 push events through DappSyncProvider#1814
Open
alleneubank wants to merge 7 commits into
Open
Conversation
Send Connect (and other CIP-103 browser extensions) push txChanged /
accountsChanged / statusChanged / connected events to the dApp window
as `{type: 'SPLICE_WALLET_EVENT', event, payload}` postMessage frames.
DappAsyncProvider already forwards these via its EventSource listeners
into AbstractProvider.emit(); DappSyncProvider had no equivalent and
silently dropped every push. The result was the Joel-reported symptom
on wallet-gateway/examples/ping: the user signs in the wallet popup,
the tx lands on the ledger (PQS-confirmed), but the dApp's
sdk.onTxChanged(listener) never fires so the UI never updates.
Changes (all additive, no behavior change for HTTP/SSE flow):
- core/types: add WalletEvent.SPLICE_WALLET_EVENT and a matching
SpliceMessage discriminated-union variant carrying
{event, payload, target?}, so isSpliceMessageEvent() recognizes
the push envelope instead of failing Zod validation.
- core/rpc-transport: add optional RpcTransport.onEvent. Implement
on WindowTransport with a single shared message listener (one DOM
listener per transport, fanned out to subscribers via a Set) gated
on the WindowTransport `target` so a dApp connected to extension A
doesn't see frames posted by extension B.
- core/provider-dapp: in DappSyncProvider's constructor, subscribe
to transport.onEvent (when present) and forward each (event,
payload) into AbstractProvider.emit(event, payload). This is what
the Sync path needed for parity with DappAsyncProvider's existing
EventSource → emit wiring. Adds teardown() to release the
subscription.
- docs/fork-only-dapp-sync-event-channel.md: full trace of the
symptom, root cause, the CIP-103 spec gap (the spec defines these
four as OpenRPC methods, not as a documented push channel for the
extension transport), and the three open questions for any
eventual upstream conversation (envelope shape, RpcTransport.onEvent
optionality, OpenRPC subscription concept).
Fork-only because CIP-103 doesn't yet specify a wire envelope for the
extension push channel — the SPLICE_WALLET_EVENT shape used here
matches what Send Connect emits, but upstream may pick a different
shape (e.g. JSON-RPC notifications in the existing
SPLICE_WALLET_REQUEST envelope). The patch is intentionally minimal
so it survives any of those resolutions.
Verified locally against wallet-gateway/examples/ping (this repo,
rebased onto wallet-gateway/main as of today) with Send Connect webext
v0.3.2-dev unpacked and testnet selected: clicking `create Ping
contract`, signing, and waiting now renders `Total transactions: 3`
with the pending → signed → executed lifecycle visible. PQS confirms
the underlying create on testnet.
Signed-off-by: Allen <allen@send.it>
Two findings from `rl review` against `wallet-gateway/main..HEAD`: - major-2: removed the try/catch in `WindowTransport.installEventDispatcher` that swallowed listener exceptions and only `console.error`'d them. This was new error suppression introduced by the previous commit and was inconsistent with `AbstractProvider.emit` (`core/splice-provider/src/index.ts:54`), which does not isolate listener throws. With only one practical subscriber per transport (`DappSyncProvider`), the isolation was theoretical and the net effect was hiding real listener bugs in the console. - major-1: documented the upstream lifecycle gap in `docs/fork-only-dapp-sync-event-channel.md`. `ExtensionAdapter.teardown()` is a no-op upstream and `DiscoveryClient.disconnect()` only reaches the adapter, so the new `DappSyncProvider.teardown()` is dead code under the current SDK control flow. The gap is upstream's (the patch can't fix it without expanding scope into `sdk/dapp-sdk` and `core/wallet-discovery`). Doc now cites the two upstream files+lines, explains the GC retention / stale-listener consequence on intra-page reconnect, and proposes two minimal upstream fixes. `teardown()` ships ready to be called. Signed-off-by: Allen <allen@send.it>
Cycle-2 `rl review` findings against `bb/dapp-sync-event-channel@d08c7e6`: - major-4 (fork-introduced, code fix): `WindowTransport.installEventDispatcher` was permissive on untargeted events — when `options.target` was set, an event frame with no `data.target` still reached the target-scoped transport. Because `SPLICE_WALLET_EVENT.target` is optional in the schema, any wallet broadcasting an untargeted event would drive listeners for the selected wallet. Tightened by dropping `data.target !== undefined` from the filter: a target-scoped transport now requires `data.target` to equal `options.target`. Untargeted transports (no `options.target`) continue to receive everything. - major-3 (pre-existing upstream, doc-only): added a paragraph to the "Known gap: `teardown()` is dead code" section reaffirming the scope decision. `ExtensionAdapter.teardown()` and `DiscoveryClient.disconnect()` are upstream-owned files unchanged by this fork; the lifecycle wiring is upstream's call. `DappSyncProvider.teardown()` ships ready to be wired. Signed-off-by: Allen <allen@send.it>
Cycle-3 `rl review` re-flagged the pre-existing upstream lifecycle gap as major-5, identical in substance to cycle-1's major-1 and cycle-2's major-3. The reviewer is adversarial-by-design and will not credit "decided not to fix" doc framing. Doc-only sharpening of the "Known gap" section to make the scope decision unambiguous to any future reader (and to any future review pass): - Retitle as "Scope: WONTFIX (upstream-owned)" to lead with the decision, not the bug. - Explicit "Status: known, intentional, not addressed by this PR" header. - Name the two upstream-owned files this PR does not touch: - sdk/dapp-sdk/src/adapter/extension-adapter.ts - core/wallet-discovery/src/client.ts - Cross-reference the JsCommands schema-stub fix scoping (PR canton-network/ wallet-gateway#1726) as the precedent for minimal upstream-eligible changes that don't refactor adjacent upstream surfaces. - Note all three review cycles re-flagged the gap and that the scope decision stands. No code changes. `DappSyncProvider.teardown()` continues to ship as a callable contract; upstream picks the caller. Signed-off-by: Allen <allen@send.it>
Adds a subsection to docs/fork-only-dapp-sync-event-channel.md that pre-empts the natural reviewer question: "why not reuse the existing SPLICE_WALLET_RESPONSE envelope for push events instead of inventing SPLICE_WALLET_EVENT?" Three independent reasons (per-request listener scope, self-removing listener, hijacking risk on id-reuse) are derived directly from upstream wallet-gateway/main code. Also clarifies that the Option B alternative (JSON-RPC notification inside SPLICE_WALLET_REQUEST) sidesteps two of the three reasons but still requires the same plumbing this PR introduces — making the choice of wire envelope a bikeshed independent of the structural fix. Signed-off-by: Allen <allen@send.it>
Adds an "EIP-1193 precedent" subsection to the push-event-channel doc citing eip-1193.md:24 (transport-agnostic) and :158-167 (EventEmitter mandate). Tabulates the universal MetaMask-pattern convention (single permanent inpage listener, JSON-RPC notifications demuxed by id-absence) against the upstream WindowTransport (per-request, response-only, self-removing) and this fork's implementation. Reframes the bug as "violates the EIP-1193 EventEmitter contract" rather than "missing SPLICE_WALLET_EVENT handler" -- a structural framing that doesn't depend on the envelope-naming bikeshed. Signed-off-by: Allen <allen@send.it>
Drops personal attribution and testnet-specific party IDs from the symptom section; replaces Send-internal CLI commands (sloki, sinfra) in the repro recipe with generic guidance. Technical content otherwise unchanged. Signed-off-by: Allen Eubank <allen@send.it>
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
3 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
Send Connect (and any other CIP-103 browser extension) pushes
txChanged/accountsChanged/statusChanged/connectedevents to the dApp window as{type: 'SPLICE_WALLET_EVENT', event, payload}postMessage frames.DappAsyncProvider(HTTP/SSE wallet-gateway flow) wires the equivalents intoAbstractProvider.emit()via its EventSource listeners;DappSyncProvider(postMessage flow used byExtensionAdapter) had no equivalent path and silently dropped every push.End-user symptom (reported in public Slack, repro'd here against
wallet-gateway/examples/ping):The transaction does land on the ledger (confirmed by querying the participant's transaction store). The dApp's
sdk.onTxChanged(listener)just never fires, souseTransactions()never sees the lifecycle events, so the UI never updates.Full diagnosis, captured wire frames, and the CIP-103 spec-gap discussion are in
docs/fork-only-dapp-sync-event-channel.md.Why this is
fork-only(for now)CIP-103 today only defines
accountsChanged/statusChanged/connected/txChangedas OpenRPC methods (pull). The docs label them as "Events" in the Sync API but stop short of specifying a wire envelope for pushing them over postMessage. The Async path has a de-facto SSE answer; the Sync path doesn't.This patch picks one envelope —
SPLICE_WALLET_EVENT(symmetric with the existingSPLICE_WALLET_REQUEST/_RESPONSE) — and wires it through. That is what Send Connect already emits. Upstream may prefer a different shape (e.g. JSON-RPC notifications inside the existingSPLICE_WALLET_REQUESTenvelope); the diff is intentionally minimal so that swap stays small.Filing on the staging fork as a working reference implementation for the structural fix. The upstream canon-form (envelope shape + whether to ship as
SPLICE_WALLET_EVENTor as a JSON-RPC notification on the existingSPLICE_WALLET_REQUESTenvelope) is a separate conversation tracked at canton-network/wallet#1815, and the upstream PR is open at canton-network/wallet#1814.Changes (all additive, no behavior change on the HTTP/SSE flow)
core/types/src/index.tsWalletEvent.SPLICE_WALLET_EVENTand a matchingSpliceMessagediscriminated-union variant carrying{event, payload, target?}. Without this,isSpliceMessageEventwould still reject the push envelope as malformed.core/rpc-transport/src/index.tsRpcTransport.onEvent. Implements it onWindowTransportwith a single shared DOMmessagelistener (lazily installed on first subscription, fanned out to subscribers via aSet) gated on the transport'stargetso multi-extension dApps don't cross the streams.core/provider-dapp/src/DappSyncProvider.tstransport.onEventexists, subscribe and forward each(event, payload)tothis.emit(event, payload). Adds ateardown()to release the subscription.docs/fork-only-dapp-sync-event-channel.mdonEventoptionality, OpenRPC subscription concept).Test plan
wallet-gateway/examples/ping(this repo, rebased ontowallet-gateway/maintoday) + Send Connect webext v0.3.2-dev unpacked + testnet. Clickcreate Ping contract, sign in approval popup. Tap captures threeSPLICE_WALLET_EVENTframes (txChangedpending → signed → executed). dApp UI never rendersTotal transactions: 1. The participant's transaction store confirms the create did land (templatecanton-builtin-admin-workflow-ping:Canton.Internal.Ping:Ping; commandId matches the post-message stream and resolves cleanly via the ledger API).Total transactions: 3with all three lifecycle JSON payloads visible (pending → signed → executed, latest first).commandIdmatches the post-message stream;updateIdmatches the participant's transaction store.WindowTransport.onEventhappy-path +targetgating incore/rpc-transport, (b)DappSyncProviderforwards push events viaAbstractProvider.emitincore/provider-dapp, (c) end-to-end ping E2E inexamples/ping/tests/.DappAsyncProvider/HttpTransport.RpcTransport.onEventis optional soHttpTransport(which doesn't implement it) is unaffected. Manual smoke against the 5N Loop Wallet (Devnet)LoopAdapterwould still be valuable; happy to add if reviewers want it before merge.Links
docs/fork-only-dapp-sync-event-channel.mdJsCommandsschema stub fix: fix: add JsCommands schema stub in openrpc-dapp-api.json #1726 (separate scope, intentionally not bundled here)