Skip to content

feat: add multi-session support - #2161

Merged
mjuchli-da merged 35 commits into
mainfrom
alex/multi-session
Aug 4, 2026
Merged

feat: add multi-session support#2161
mjuchli-da merged 35 commits into
mainfrom
alex/multi-session

Conversation

@alexmatson-da

Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI lite review requested due to automatic review settings July 29, 2026 17:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds origin-aware session/state handling to enable multiple concurrent wallet sessions (per dApp origin) across the frontend and backend, including API/schema updates to persist and expose session origin.

Changes:

  • Scope frontend stored auth/session state by dApp origin and introduce origin-detection via parent→popup messaging.
  • Extend user API + store layer to persist Session.origin and key session operations by accessToken.
  • Update OpenRPC specs / generated typings to require origin when adding sessions and to include it in session responses.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
wallet-gateway/remote/src/web/frontend/state-manager.ts Origin-scoped local/session storage keys for auth/session state.
wallet-gateway/remote/src/web/frontend/settings/index.ts Fetch settings data using origin-scoped access token.
wallet-gateway/remote/src/web/frontend/parties/index.ts Use origin-scoped access token/network id for parties flows.
wallet-gateway/remote/src/web/frontend/parties/add/index.ts Use origin-scoped access token/network id when creating wallets.
wallet-gateway/remote/src/web/frontend/networks/review/index.ts Use origin-scoped access token for network review actions.
wallet-gateway/remote/src/web/frontend/networks/index.ts Use origin-scoped access token when loading networks view.
wallet-gateway/remote/src/web/frontend/networks/add/index.ts Use origin-scoped access token for adding networks.
wallet-gateway/remote/src/web/frontend/login/login.ts Persist selected network/auth data under the detected origin.
wallet-gateway/remote/src/web/frontend/listeners.ts Add postMessage-based origin detection for popup flows.
wallet-gateway/remote/src/web/frontend/index.ts Make redirects/logout/auth flows origin-aware; share origin detection.
wallet-gateway/remote/src/web/frontend/identity-providers/review/index.ts Use origin-scoped access token for IdP review actions.
wallet-gateway/remote/src/web/frontend/identity-providers/index.ts Use origin-scoped access token when loading IdPs view.
wallet-gateway/remote/src/web/frontend/identity-providers/add/index.ts Use origin-scoped access token for adding IdPs.
wallet-gateway/remote/src/web/frontend/callback/index.ts Store token/expiry under origin and add session with origin.
wallet-gateway/remote/src/web/frontend/approve/index.ts Use origin-scoped access token for approve flows.
wallet-gateway/remote/src/web/frontend/api-keys/index.ts Use origin-scoped access token for API key listing/revocation.
wallet-gateway/remote/src/web/frontend/api-keys/add/index.ts Use origin-scoped access token for API key generation.
wallet-gateway/remote/src/user-api/rpc-gen/typings.ts Add Origin type + Session.origin + AddSessionParams.origin.
wallet-gateway/remote/src/user-api/controller.ts Persist session origin; change session ops to use accessToken keys.
wallet-gateway/remote/src/user-api/controller.test.ts Update tests for origin/session changes.
wallet-gateway/remote/src/middleware/sessionHandler.ts Validate session existence keyed by access token.
wallet-gateway/remote/src/middleware/apiKeyAuth.ts Create sessions for API-key auth including an origin value.
wallet-gateway/remote/src/ledger/wallet-sync-service.test.ts Update session fixtures to include origin.
wallet-gateway/remote/src/dapp-api/server.ts Use accessToken-keyed session lookup for SSE connections.
wallet-gateway/remote/src/dapp-api/controller.ts Use accessToken-keyed session lookup/removal.
wallet-gateway/remote/src/dapp-api/controller.test.ts Update tests for new session semantics.
core/wallet-user-rpc-client/src/openrpc.json Require origin for addSession; include origin in Session schema.
core/wallet-user-rpc-client/src/index.ts Update TS client types for origin changes.
core/wallet-ui-components/src/windows/popup.ts Broadcast parent origin to popup and await ACK.
core/wallet-store/src/Store.ts Change session API to be accessToken-keyed; add listSessions.
core/wallet-store-sql/src/store-sql.ts Implement accessToken-keyed session ops + origin-based uniqueness.
core/wallet-store-sql/src/schema.ts Add origin to SQL session schema mapping.
core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts Migration to add origin column to sessions.
core/wallet-store-inmemory/src/store-internal.ts In-memory store updated to support multiple sessions.
core/types/src/index.ts Add wallet origin broadcast message types/validation.
api-specs/openrpc-user-api.json Mirror OpenRPC changes for origin in user API.
Comments suppressed due to low confidence (7)

wallet-gateway/remote/src/web/frontend/index.ts:317

  • redirectToIntendedOrDefault is now async, but it’s invoked without awaiting in this async code path (floating promise).
        // Redirect to default page if on root path
        if ((getCurrentRoute(window.location.pathname) || '/') === '/') {
            redirectToIntendedOrDefault()
        }

wallet-gateway/remote/src/web/frontend/login/login.ts:117

  • redirectToIntendedOrDefault is now async, but this call site doesn’t await it. In an async handler, prefer awaiting to avoid floating promises and to keep control flow consistent after navigation is triggered.
        try {
            if (selectedIdp.type === 'self_signed') {
                await this.selfSign(selectedNetwork.id, clientId)
                redirectToIntendedOrDefault()
                return

wallet-gateway/remote/src/web/frontend/state-manager.ts:51

  • setWithStorage caches values under the bare key, which overwrites cached values across different origins. Cache using the same composite key used for storage (localStorageKey(key, origin)).
    wallet-gateway/remote/src/web/frontend/state-manager.ts:60
  • clearWithStorage deletes only the bare key from the in-memory cache, which can leave stale cached values for other origins (and/or fail to clear the correct cached entry). Delete using the composite storage key.
    wallet-gateway/remote/src/web/frontend/callback/index.ts:87
  • redirectToIntendedOrDefault is async now, but it’s called inside a .then(...) callback without being returned/awaited. Return the promise so errors propagate correctly and to avoid a floating promise.
                    .then(() => {
                        redirectToIntendedOrDefault()
                    })

core/types/src/index.ts:139

  • origin is validated with z.url() here, which is likely invalid for the Zod version used elsewhere in the repo. Use z.string().url() for consistency and compatibility.
    z.object({
        type: z.literal(WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN),
        origin: z.url(),
    }),

wallet-gateway/remote/src/dapp-api/server.ts:60

  • This debug block logs a hash derived from the access token (sha256(accessToken)) and uses a placeholder message. Even hashed, this can aid correlation and should not be logged in normal operation (SSE connects frequently).
                hash: crypto
                    .createHash('sha256')
                    .update(context.accessToken)
                    .digest('hex'),
            },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wallet-gateway/remote/src/web/frontend/index.ts
Comment thread wallet-gateway/remote/src/web/frontend/index.ts Outdated
Comment thread wallet-gateway/remote/src/web/frontend/index.ts
Comment thread wallet-gateway/remote/src/dapp-api/server.ts
Comment thread wallet-gateway/remote/src/web/frontend/listeners.ts
Comment thread core/types/src/index.ts
Comment thread core/wallet-ui-components/src/windows/popup.ts
Comment thread core/wallet-store/src/Store.ts
Comment thread wallet-gateway/remote/src/user-api/controller.ts Outdated
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings July 29, 2026 20:52
Comment thread wallet-gateway/remote/src/user-api/controller.ts Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 70 out of 71 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (15)

wallet-gateway/remote/src/web/frontend/state-manager.ts:17

  • The constructor deletes every localStorage entry that does not start with the current VERSION_PREFIX. Because localStorage is scoped per origin (not per app), this can wipe unrelated application data on the same origin, and it mutates localStorage while iterating it via for...in (non-deterministic). Limit cleanup to the wallet's legacy key prefix and iterate via localStorage.key(i).
    wallet-gateway/remote/src/web/frontend/index.ts:170
  • Debug logging and commented-out origin handling code were left in handleAuthRedirect(). This adds noise to production logs and keeps dead code paths around that reference removed state (e.g., sessionOrigin). Please remove before merging.
    wallet-gateway/remote/src/web/frontend/listeners.ts:18
  • handleMessage calls window.opener.postMessage(...) unconditionally. If the UI is opened directly (no opener) but still receives a matching message event, this will throw. Guard against window.opener being null/closed.
    wallet-gateway/remote/src/web/frontend/listeners.ts:28
  • detectCurrentOrigin() polls forever if the opener never sends the origin (or if the message is blocked), and even when the value is already available it waits for the next 100ms tick. Add an immediate fast-path and a timeout/fallback to avoid hanging callers.
    wallet-gateway/remote/src/user-api/controller.ts:864
  • listSessions currently calls getSession(authContext?.accessToken || '') and returns at most one session, even though the Store interface now includes listSessions(): Promise<Array<Session>> and the OpenRPC schema models sessions as a list. This will prevent clients from actually seeing multiple active sessions.
    wallet-gateway/remote/src/dapp-api/server.ts:57
  • This debug log includes an unprofessional message string and logs a SHA-256 of the access token. Even hashed token-derived values can be sensitive (and aren't needed for routine operation). Log only the sessionId (or remove the log) and keep the message professional.
    wallet-gateway/remote/src/dapp-api/server.ts:16
  • After removing the token-hash debug block, the crypto import becomes unused. Please remove it.
    core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts:11
  • The migration logs an incorrect message (it alters the sessions table, not networks) and adds origin as a nullable column. The updated Store interfaces and StoreSql logic require session.origin to be present, so existing rows would violate that. Add the column as NOT NULL with a default to backfill existing sessions, and remove the console log.
export async function up(db: Kysely<DB>): Promise<void> {
    console.log('Adding origin column to networks table')

    await db.schema.alterTable('sessions').addColumn('origin', 'text').execute()
}

wallet-gateway/remote/src/web/frontend/state-manager.ts:33

  • getWithStorage caches values by the bare key, but storage is now namespaced by origin. This can return the wrong cached value when switching origins. Use the computed storage key (which already includes the origin) as the cache key.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:47
  • setWithStorage/clearWithStorage update the in-memory cache using only key, which will collide across origins (and storage types). Store/delete using the same computed storage key that includes origin.
    core/wallet-ui-components/src/windows/popup.ts:88
  • handleMessage clears originPoller before it is guaranteed to be initialized (ACK could arrive quickly), and the polling loop never stops if the child never ACKs. Declare originPoller before registering the handler, remove console logging, and bound the polling attempts / stop when the window closes.
    core/wallet-store/src/Store.ts:157
  • Store.getSession() and Store.removeSession() now require an accessToken argument. There are still call sites using the old no-arg signature (e.g. core/wallet-store-sql/src/store-sql.test.ts:293, core/wallet-store-sql/src/store-sql.test.ts:298, core/wallet-store-inmemory/src/store-internal.test.ts:319, core/wallet-store-inmemory/src/store-internal.test.ts:321). These will fail to compile/run until updated to pass the relevant access token (and to use listSessions() where appropriate).
    // Session methods
    /**
     * getSession is keyed by the accessToken, which is unique per session. It retrieves the session associated with the provided accessToken.
     * @param accessToken The access token associated with the session to retrieve.
     * @returns A Promise that resolves to the Session object if found, or undefined if no session exists for the given accessToken.
     */
    getSession(accessToken: string): Promise<Session | undefined>

wallet-gateway/remote/src/user-api/controller.ts:406

  • setPrimaryWallet uses a hardcoded 'blahblahblah' access token fallback and then non-null asserts session!.id, which will throw when called without a valid auth context/session. Use assertConnected(authContext) and handle the missing session/notifier case safely.
    wallet-gateway/remote/src/web/frontend/index.ts:219
  • The commented-out handleSessionOrigin block should be removed rather than left in the codebase. It references stateManager.sessionOrigin (which no longer exists) and obscures the actual auth redirect logic.
    wallet-gateway/remote/src/dapp-api/controller.ts:130
  • Avoid context! in disconnect: store.getSession(context!.accessToken) will throw when context is undefined. Compute session conditionally so the existing if (!context || !sessionId) guard can work as intended.

Comment thread wallet-gateway/remote/src/signing/signing-worker.ts Outdated
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings July 30, 2026 12:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 44 out of 45 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (15)

wallet-gateway/remote/src/web/frontend/state-manager.ts:56

  • setWithStorage updates the in-memory cache with this.state.set(key, value), which ignores origin and can overwrite cached values for other origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:24
  • The constructor currently removes every localStorage entry that does not start with VERSION_PREFIX, which can wipe unrelated localStorage data for the same origin (including other apps running on the same domain). Limit cleanup to wallet-gateway keys only, and avoid mutating localStorage while iterating with for...in.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:36
  • getWithStorage caches values in this.state using only key, ignoring origin (and storage). With multi-session support, this can return the wrong value when the same key is read for different origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:65
  • clearWithStorage deletes from the in-memory cache using only key, which can unintentionally clear cached values for other origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:84
  • accessToken.get still writes/reads this.accessTokenCache (single value). After switching to an origin-keyed cache, update the decrypt path to store and return the token per-origin.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:97
  • accessToken.set updates a global cache value; with multi-session support it should cache per-origin.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:103
  • accessToken.clear clears a global cache value; with multi-session support it should clear the cache for the provided origin only.
    wallet-gateway/remote/src/web/frontend/listeners.ts:16
  • handleMessage assumes window.opener exists and posts an ACK without verifying the message actually came from the opener. In a non-popup context (or if another window posts this message), this can throw and/or let a non-opener influence currentOrigin. Guard on window.opener and event.source === window.opener before acknowledging.
    core/wallet-ui-components/src/windows/popup.ts:83
  • handleMessage references originPoller before it is initialized (it is declared later as a const). If the ACK message arrives quickly (between addEventListener and the setInterval assignment), this will throw a ReferenceError and break the popup flow. Also, the poller currently has no timeout and logs to console on every interval.
            const handleMessage = (event: MessageEvent) => {
                if (!isSpliceMessageEvent(event)) return
                if (
                    event.data.type !==
                    WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN_ACK

wallet-gateway/remote/src/signing/signing-worker.ts:190

  • Store.getSession is now keyed by accessToken, but this code passes userId. This will either fail to compile or always return the wrong session (and then crash on session!.id).
        const session = await this.options.store.getSession(userId)
        const sessionId = session!.id
        const notifier = this.options.notificationService.getNotifier(sessionId)

wallet-gateway/remote/src/web/frontend/listeners.ts:34

  • detectCurrentOrigin() polls forever if the opener never sends the origin broadcast (or if the message listener never runs). This can hang UI flows that await it. Add a timeout + fallback (or reject) so callers don't wait indefinitely.
    core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts:10
  • The migration log message references the wrong table ('networks'), and the new origin column is added without NOT NULL / default. StoreSql.setSession now requires session.origin, so leaving this nullable can cause null origin rows and runtime issues during rollout.
    console.log('Adding origin column to networks table')

    await db.schema.alterTable('sessions').addColumn('origin', 'text').execute()

wallet-gateway/remote/src/web/frontend/state-manager.ts:71

  • accessTokenCache is shared across all origins. After a token is cached for one origin, accessToken.get(otherOrigin) will incorrectly return the cached token for the first origin.

This issue also appears in the following locations of the same file:

  • line 80
  • line 97
  • line 103
    wallet-gateway/remote/src/user-api/controller.ts:406
  • setPrimaryWallet calls store.getSession(authContext?.accessToken || 'blahblahblah'). The placeholder token will never match a real session, and session!.id will throw. This method should either require a connected context or safely no-op when there's no session.
        setPrimaryWallet: async (params: SetPrimaryWalletParams) => {
            await store.setPrimaryWallet(params.partyId)
            const session = await store.getSession(
                authContext?.accessToken || 'blahblahblah'
            )
            const sessionId = session!.id

wallet-gateway/remote/src/dapp-api/controller.ts:134

  • disconnect dereferences context!.accessToken before checking whether context is defined. If context is missing, this will throw instead of returning null.
        disconnect: async () => {
            const session = await store.getSession(context!.accessToken)
            const sessionId = session?.id
            if (!context || !sessionId) {
                return null

Comment thread wallet-gateway/remote/src/web/frontend/parties/index.ts
Comment thread wallet-gateway/remote/src/web/frontend/parties/add/index.ts
Comment thread wallet-gateway/remote/src/middleware/apiKeyAuth.ts
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings July 30, 2026 12:39
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (12)

wallet-gateway/remote/src/web/frontend/state-manager.ts:38

  • StateManager caches values in this.state keyed only by key, but reads/writes storage using an origin-specific key. With multi-session/origin support this will return the wrong cached value when the same item (e.g. networkId) is requested for a different origin.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:56
  • setWithStorage writes the cached value under key only, which won’t match the origin-scoped lookup if getWithStorage is updated to cache by storage key (and even today it risks cross-origin contamination). Cache should use the same composed storage key as the underlying storage.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:65
  • clearWithStorage deletes from the in-memory cache by key only, which can leave behind (or delete) the wrong cached value when multiple origins are in use. Use the same composed storage key for cache operations as for storage operations.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:24
  • The StateManager constructor currently removes every localStorage entry that doesn’t start with VERSION_PREFIX. Since stateManager is instantiated at module load, this can wipe unrelated localStorage keys for the same origin (and for...in iteration while deleting can skip keys). Consider only removing wallet-owned keys from older versions.
    wallet-gateway/remote/src/web/frontend/listeners.ts:39
  • detectCurrentOrigin() polls sessionStorage forever when window.opener exists. If the parent never sends the broadcast (older dApp, blocked postMessage, etc.), this Promise never resolves and callers will hang. Add a timeout/fallback origin to avoid infinite waits.
    wallet-gateway/remote/src/web/frontend/index.ts:94
  • render() reads stateManager.networkId with this.currentOrigin || ''. Before connectedCallback() finishes, this will query/cache state under an empty origin, and with the current StateManager cache logic it can contaminate later reads for real origins. Avoid hitting StateManager until currentOrigin is available (or default to window.origin).
    wallet-gateway/remote/src/signing/signing-worker.ts:190
  • Store.getSession is now keyed by accessToken, but the signing worker calls it with userId and then dereferences session!.id. Also, service-account automation explicitly can run without a pre-existing stored session, so this will crash for those flows. Use the runContext auth accessToken when looking up a session and fall back to a user-level notifier ID when none exists.
        const session = await this.options.store.getSession(userId)
        const sessionId = session!.id
        const notifier = this.options.notificationService.getNotifier(sessionId)

core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts:11

  • Migration adds sessions.origin without a default and logs the wrong table name. Since the Store now requires origin, existing rows may end up with NULL and break reads/updates. Prefer adding the column with a default (and ideally NOT NULL) so existing rows are populated.
export async function up(db: Kysely<DB>): Promise<void> {
    console.log('Adding origin column to networks table')

    await db.schema.alterTable('sessions').addColumn('origin', 'text').execute()
}

core/wallet-test-utils/src/wallet-gateway.ts:101

  • When creating a Fireblocks wallet, the UI typically requires selecting a vault. The helper currently selects the signing provider and clicks Create without handling vaultName, which will likely break Fireblocks E2E flows.
        await this.page
            .getByLabel('Signing Provider')
            .selectOption(args.signingProvider)
        if (args.primary) {
            await this.page

wallet-gateway/remote/src/web/frontend/state-manager.ts:73

  • accessTokenCache is a single value shared across all origins, but accessToken.get(origin) is now origin-scoped. Once any origin’s token is cached, calls for other origins will return the wrong token.
    wallet-gateway/remote/src/user-api/controller.ts:410
  • setPrimaryWallet calls store.getSession(authContext?.accessToken || 'blahblahblah') and then dereferences session!.id. If authContext is missing/invalid this will query with a bogus token and crash; even with auth present, the placeholder should never ship.
        setPrimaryWallet: async (params: SetPrimaryWalletParams) => {
            await store.setPrimaryWallet(params.partyId)
            const session = await store.getSession(
                authContext?.accessToken || 'blahblahblah'
            )
            const sessionId = session!.id
            const notifier = notificationService.getNotifier(sessionId)

            const wallets = await store.getWallets()
            notifier.emit('accountsChanged', wallets)

wallet-gateway/remote/src/dapp-api/controller.ts:133

  • disconnect dereferences context! before checking !context, so unauthenticated calls will throw instead of returning null. Check context first, then look up the session.
        disconnect: async () => {
            const session = await store.getSession(context!.accessToken)
            const sessionId = session?.id
            if (!context || !sessionId) {
                return null

Comment thread core/wallet-test-utils/src/wallet-gateway.ts
Copilot AI review requested due to automatic review settings July 30, 2026 12:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (17)

wallet-gateway/remote/src/web/frontend/state-manager.ts:22

  • The constructor currently removes every localStorage entry that does not start with VERSION_PREFIX. This can wipe unrelated application data stored on the same origin (and mutating localStorage while iterating via for...in can also skip keys). Limit cleanup to prior wallet keys only and iterate using localStorage.key(i).
    wallet-gateway/remote/src/web/frontend/state-manager.ts:56
  • setWithStorage writes to the in-memory cache using only key, which will overwrite cached values for the same key across different origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:65
  • clearWithStorage deletes from the in-memory cache by key only, which can clear cached values for other origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:38
  • getWithStorage caches values in this.state by key only, ignoring origin (and storage type). With multi-session support this can return the wrong value when the same key is read for different origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:73
  • accessToken.get() returns accessTokenCache without checking origin, so a token decrypted for one origin can be incorrectly returned for another origin.
    wallet-gateway/remote/src/web/frontend/listeners.ts:34
  • detectCurrentOrigin() polls forever if the origin broadcast never arrives (e.g. popup opened directly, opener blocked, or postMessage fails). This can leave the UI hanging and leak an interval timer. Add a timeout fallback.
    wallet-gateway/remote/src/signing/signing-worker.ts:190
  • Store.getSession is now keyed by accessToken, but this code passes userId, so session will typically be undefined and session!.id will throw. The notifier id also becomes incorrect.
        const session = await this.options.store.getSession(userId)
        const sessionId = session!.id
        const notifier = this.options.notificationService.getNotifier(sessionId)

wallet-gateway/remote/src/user-api/controller.ts:407

  • setPrimaryWallet fetches the session using authContext?.accessToken || 'blahblahblah' and then assumes session!.id exists. This can crash and also hides unauthenticated calls. Use assertConnected(authContext) and handle missing sessions explicitly.
            const session = await store.getSession(
                authContext?.accessToken || 'blahblahblah'
            )
            const sessionId = session!.id
            const notifier = notificationService.getNotifier(sessionId)

wallet-gateway/remote/src/dapp-api/server.ts:61

  • This debug log includes a SHA-256 hash of the access token and an 'EEEEE' message. Even hashed token-derived values can aid correlation and should not be logged routinely; it also adds noisy, non-actionable logs.
        logger.debug(
            {
                sessionId: session?.id,
                hash: crypto
                    .createHash('sha256')
                    .update(context.accessToken)
                    .digest('hex'),
            },
            'EEEEE Retrieved session for SSE connection'

wallet-gateway/remote/src/dapp-api/server.ts:16

  • crypto is imported only for the removed/temporary debug logging (hash output). If the debug block is removed, this import becomes unused and will fail lint/build in TS projects.
import crypto from 'crypto'

core/wallet-ui-components/src/windows/popup.ts:79

  • The origin broadcast poller uses setInterval with no timeout and logs on every tick. If the child never ACKs (or is closed), this will spam logs and leak an interval + message listener.
            // due to the asynchronicity when sending the postMessage immediately after redirecting,
            // there is a chance that the child window has not yet loaded,
            // and does not an event listener established yet. Therefore,
            // we repeatedly poll until the child window sends back an acknowledgment message.
            const handleMessage = (event: MessageEvent) => {

core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts:11

  • This migration logs the wrong table name and adds sessions.origin without a NOT NULL/default. Existing rows would get NULL origin, but the code/schema treats origin as required, which can cause runtime issues.
export async function up(db: Kysely<DB>): Promise<void> {
    console.log('Adding origin column to networks table')

    await db.schema.alterTable('sessions').addColumn('origin', 'text').execute()
}

wallet-gateway/remote/src/web/frontend/index.ts:282

  • redirectToIntendedOrDefault() is async now, but it's invoked without await here. In strict setups this can trigger floating-promise lint errors and makes navigation timing nondeterministic relative to subsequent calls.
    wallet-gateway/remote/src/web/frontend/index.ts:312
  • redirectToIntendedOrDefault() is async now, but it's called without await when redirecting from '/'. This can lead to an unhandled promise (depending on lint/runtime) and races with other state updates.
    wallet-gateway/remote/src/web/frontend/callback/index.ts:90
  • The callback flow fires addUserSession(...).then(() => redirectToIntendedOrDefault()), but redirectToIntendedOrDefault is now async and is not awaited/returned. This can cause the redirect to race and makes errors harder to handle.
                addUserSession(
                    tokenResponse.access_token,
                    stateManager.networkId.get(origin) || ''
                )
                    .then(() => {
                        redirectToIntendedOrDefault()
                    })

wallet-gateway/remote/src/user-api/controller.ts:254

  • session!.id will throw if no session exists for the provided access token (e.g. store state drift). Avoid non-null assertions here and fail with a clear error instead.

This issue also appears on line 403 of the same file.

            const session = await store.getSession(connectedContext.accessToken)
            const sessionId = session!.id
            const notifier = notificationService.getNotifier(sessionId)

wallet-gateway/remote/src/dapp-api/controller.ts:133

  • disconnect uses context! before checking whether context exists. If the controller is created without an auth context, this will throw instead of returning null.
        disconnect: async () => {
            const session = await store.getSession(context!.accessToken)
            const sessionId = session?.id
            if (!context || !sessionId) {
                return null

Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings July 30, 2026 15:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 46 out of 47 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (18)

wallet-gateway/remote/src/web/frontend/state-manager.ts:36

  • StateManager's in-memory cache is keyed only by key, but all persisted values are keyed by (origin, key). This can cause cross-origin/session leakage (e.g., reading networkId for a different origin) and breaks multi-session isolation.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:22
  • The constructor currently removes every localStorage entry that does not start with VERSION_PREFIX. This can wipe unrelated application data stored under the same origin (and removing keys while iterating localStorage can also skip entries). Limit cleanup to legacy wallet keys only, and iterate over a snapshot of keys.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:56
  • setWithStorage writes to the in-memory cache under key only. With origin-scoped storage keys, this should also cache under an origin-scoped key to avoid collisions between sessions.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:65
  • clearWithStorage deletes the in-memory cache entry under key only. With origin-scoped storage keys, this should delete the origin-scoped cache key to avoid leaving stale values for other origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:95
  • accessToken.set/clear should update the per-origin cache (not a single shared value), otherwise the cache can still return a token from a different origin.
    wallet-gateway/remote/src/web/frontend/listeners.ts:16
  • handleMessage unconditionally calls window.opener.postMessage(...). Since this listener is installed even in non-popup/direct-tab contexts, window.opener can be null and this will throw. Guard the postMessage call.
    wallet-gateway/remote/src/web/frontend/listeners.ts:34
  • detectCurrentOrigin polls forever if the opener never broadcasts an origin (or if the message is blocked). This can hang pages that call it. Add a timeout and fall back to window.origin (or reject) to avoid unbounded polling.
    core/types/src/index.ts:137
  • z.url() is not a common Zod schema constructor in this codebase (other modules use z.string().url()). If z.url() is unavailable in the Zod version used, this will fail at runtime/compile time. Use z.string().url() for URL validation.
        type: z.literal(WalletEvent.SPLICE_WALLET_EXT_OPEN),
        url: z.url(),
        target: SpliceTarget.optional(),

core/types/src/index.ts:147

  • Same as above: use z.string().url() to validate URLs consistently; z.url() may not exist depending on the Zod version/typing.
    z.object({
        type: z.literal(WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN),
        origin: z.url(),
    }),

wallet-gateway/remote/src/web/frontend/parties/index.ts:170

  • origin is referenced in connectedCallback but is not defined in this scope, which will throw at runtime. Compute it via detectCurrentOrigin like other methods in this file.
    wallet-gateway/remote/src/web/frontend/parties/add/index.ts:93
  • origin is referenced in onSigningProviderChange but is not defined in this scope. This will throw at runtime when loading vaults. Fetch the origin (or persist it from loadContext) before reading the access token.
    core/wallet-test-utils/src/wallet-gateway.ts:62
  • This refactor removes WalletGateway helper methods (e.g. allocateWalletParty / getWalletExternalTxId) and drops support for selecting a Fireblocks vault, but the repo still calls these APIs (e.g. examples/ping/tests/external-signing-test-helpers.ts uses both methods and passes vaultName). This is a breaking change for internal consumers and will likely break tests/builds unless callers are updated or the helpers are reintroduced.
            | 'blockdaemon'
            | 'dfns'
            | 'fireblocks'
        primary?: boolean
    }): Promise<string> {

core/wallet-ui-components/src/windows/popup.ts:97

  • handleMessage calls clearInterval(originPoller) but originPoller is declared with const after the handler is defined. If the ACK arrives before originPoller is initialized, this can throw due to the temporal dead zone. Also, the polling interval/listener will leak if the child never ACKs (e.g. popup closed).
            const handleMessage = (event: MessageEvent) => {
                if (!isSpliceMessageEvent(event)) return
                if (
                    event.data.type !==
                    WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN_ACK

wallet-gateway/remote/src/web/frontend/settings/index.ts:28

  • This import omits the .js extension while most other relative imports in this frontend use explicit .js (ESM output). With NodeNext/ESM builds, extensionless relative imports can fail after transpilation. Align with the existing .js pattern.
    wallet-gateway/remote/src/web/frontend/callback/index.ts:12
  • This import omits the .js extension while other relative imports in this frontend use explicit .js (ESM output). Extensionless relative imports can fail after transpilation in NodeNext/ESM. Align with the existing .js pattern.
    wallet-gateway/remote/src/web/frontend/index.ts:26
  • These imports omit the .js extension while the rest of the frontend uses explicit .js for relative ESM imports. This can break NodeNext/ESM builds after transpilation.
    wallet-gateway/remote/src/middleware/apiKeyAuth.ts:78
  • Session.origin is described as a dApp origin/URL elsewhere (and may be URL-validated). Setting it to req.ip produces a non-URL string and can collide across requestors behind NAT/proxies. Consider using a dedicated scheme so it remains a valid origin-like identifier (or a separate field for API-key sessions).
            await authStore.setSession({
                id: v4(),
                origin: req.ip || 'unknown', // use the requestor's IP address as the origin for the session
                network: matchingKey.networkId,
                accessToken: hashedApiKey,

wallet-gateway/remote/src/dapp-api/controller.ts:133

  • disconnect() dereferences context!.accessToken before checking whether context is set. If disconnect is called without an auth context, this will throw instead of returning null. Check context first, then load the session.
        disconnect: async () => {
            const session = await store.getSession(context!.accessToken)
            const sessionId = session?.id
            if (!context || !sessionId) {
                return null

Comment thread wallet-gateway/remote/src/web/frontend/state-manager.ts Outdated
Comment thread wallet-gateway/remote/src/web/frontend/listeners.ts
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings August 3, 2026 16:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 71 out of 72 changed files in this pull request and generated no new comments.

Suppressed comments (8)

wallet-gateway/remote/src/web/frontend/state-manager.ts:46

  • getWithStorage caches values in this.state keyed only by key, but storage is now namespaced by origin (and can be localStorage vs sessionStorage). This causes cross-origin cache collisions (e.g., networkId.get(originA) can be returned for originB) and breaks multi-session behavior.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:65
  • setWithStorage/clearWithStorage update the in-memory cache using only key, so they don’t match a per-origin cache key (and they can also evict the wrong cached value when multiple origins are active). Cache writes/evictions should use the same origin+storage-scoped key as getWithStorage.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:24
  • The constructor currently removes every localStorage entry that doesn’t start with com.splice.wallet.v1. On a shared origin, this can wipe unrelated application data (and mutating localStorage while iterating with for..in can skip keys). Consider only deleting the legacy wallet keys (com.splice.wallet.) that belong to this app and are not in the current version namespace.
    wallet-gateway/remote/src/web/frontend/listeners.ts:41
  • detectCurrentOrigin() polls forever when window.opener exists but the origin broadcast never arrives (e.g., opener closed early, message blocked, or listener not installed). Because many call sites await this, it can hang the UI indefinitely. Add an immediate read path plus a timeout (with a clear fallback or explicit error).
    core/wallet-ui-components/src/windows/popup.ts:96
  • The origin broadcast poller runs indefinitely until an ACK is received. If the child window never loads/acks (popup blocked, navigated away, origin mismatch), this leaves a setInterval running and a message listener attached for the lifetime of the page. Add a max-attempts/timeout and stop polling if the popup is closed.
            const originPoller = setInterval(() => {
                win.postMessage(message, childOrigin)
            }, 500)

core/wallet-store-inmemory/src/store-internal.ts:256

  • StoreInternal.setSession keys sessions by accessToken but never evicts existing sessions for the same origin. If a dApp reconnects and rotates tokens, the in-memory store will accumulate multiple sessions for the same origin, diverging from StoreSql.setSession (which deletes by origin).
    async setSession(session: Session): Promise<void> {
        const storage = this.getStorage()
        storage.sessions.set(session.accessToken, session)
        this.updateStorage(storage)
    }

core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts:40

  • The index name sessions_one_session_per_origin_user suggests uniqueness per (user_id, origin), but the index is currently created on (network, user_id, origin). This doesn’t match StoreSql.setSession (which deletes by userId + origin, regardless of network) and allows multiple sessions per origin+user if the network differs.
    await sql`
            CREATE UNIQUE INDEX IF NOT EXISTS sessions_one_session_per_origin_user
            ON sessions(network, user_id, origin)
        `.execute(db)

wallet-gateway/remote/src/user-api/controller.ts:406

  • setPrimaryWallet uses authContext!.userId with a non-null assertion. Other handlers in this controller use assertConnected(authContext) to guarantee authentication and avoid runtime crashes if the controller is ever invoked without a connected context.

Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings August 4, 2026 13:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 72 out of 73 changed files in this pull request and generated 1 comment.

Suppressed comments (10)

wallet-gateway/remote/src/web/frontend/state-manager.ts:65

  • setWithStorage / clearWithStorage update this.state using only key, which conflicts across different origin values. This should use the same composite cache key as getWithStorage to avoid stale or incorrect values when multiple sessions are active.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:36
  • this.state caches values only by key, but storage is now namespaced by origin. This causes cross-origin leakage (e.g. calling networkId.get(originB) can return the cached value from originA). Key the in-memory cache by both origin and key (and keep clear consistent) so multi-session reads/writes stay isolated.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:24
  • The constructor currently deletes every localStorage entry that doesn't start with com.splice.wallet.v1, which can wipe unrelated application data on the same origin. Also, deleting while iterating with for..in over localStorage can skip keys. Restrict cleanup to the wallet's old key prefix and iterate via localStorage.length/key(i).
    wallet-gateway/remote/src/web/frontend/listeners.ts:20
  • handleMessage calls window.opener.postMessage(...) unconditionally. If the wallet is opened directly (no opener) but still receives a matching message event (e.g. from another window), this will throw. Guard window.opener before posting the ACK.
    core/types/src/index.ts:138
  • z.url() is not a Zod API in this codebase (and no other usage exists). This will fail at runtime/compile time. Use z.string().url() (optionally with protocol restrictions) for URL fields inside SpliceMessage.
    z.object({
        type: z.literal(WalletEvent.SPLICE_WALLET_EXT_OPEN),
        url: z.url(),
        target: SpliceTarget.optional(),
    }),

wallet-gateway/remote/src/web/frontend/callback/index.ts:91

  • redirectToIntendedOrDefault is now async. Calling it without awaiting/returning the promise can cause an unhandled rejection (and can race navigation). Return the promise from the .then(...) chain (or await inside an async callback).
    wallet-gateway/remote/src/web/frontend/index.ts:93
  • While currentOrigin is still null, networkId.get(this.currentOrigin || '') queries the state manager with an empty origin, which can collide with real origins and also interacts badly with the in-memory cache. Prefer not reading state until currentOrigin is known.
    wallet-gateway/remote/src/web/frontend/listeners.ts:41
  • detectCurrentOrigin() polls forever when window.opener exists but the origin broadcast never arrives (blocked postMessage, race, user closes opener, etc.). Add a timeout/fallback and ensure intervals/timeouts are always cleared to avoid leaking timers.
    wallet-gateway/remote/src/user-api/controller.ts:406
  • setPrimaryWallet uses authContext!.userId without first asserting a connected context. This can throw a confusing null-assertion error if the controller is ever constructed with authContext undefined. Align with the other methods by calling assertConnected(authContext) and using the returned userId.
    core/wallet-ui-components/src/windows/popup.ts:97
  • The origin poller runs indefinitely if the child window never ACKs (popup blocked, navigated away, listener not installed, etc.), leaking an interval + message listener. Add a max attempt count or timeout to stop polling and remove the listener.
            window.addEventListener('message', handleMessage)

            const originPoller = setInterval(() => {
                win.postMessage(message, childOrigin)
            }, 500)

Comment thread core/types/src/index.ts
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings August 4, 2026 14:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 72 out of 73 changed files in this pull request and generated no new comments.

Suppressed comments (11)

wallet-gateway/remote/src/web/frontend/state-manager.ts:38

  • getWithStorage caches values in this.state by key only. Since the storage key now includes origin (and sessionStorage is also used), different origins can read each other’s cached values, causing cross-session state leakage.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:24
  • The constructor removes every localStorage key that doesn’t start with the wallet prefix, which can delete unrelated application data for the same origin. Iterating with for...in over localStorage is also unreliable while deleting entries.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:56
  • setWithStorage writes to this.state using only key, so setting a value for one origin overwrites the cached value for other origins.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:65
  • clearWithStorage deletes from this.state using only key, which can clear the in-memory value for a different origin than the one being cleared.
    wallet-gateway/remote/src/web/frontend/listeners.ts:12
  • handleMessage can call window.opener.postMessage(...) even when the wallet is opened directly (no opener). In that case window.opener is null and this will throw on any matching message event.
    core/types/src/index.ts:137
  • z.url() is not a Zod API (the URL validator is z.string().url()). This will throw at runtime / fail typechecking depending on the Zod version.
        type: z.literal(WalletEvent.SPLICE_WALLET_EXT_OPEN),
        url: z.url(),
        target: SpliceTarget.optional(),

core/types/src/index.ts:147

  • z.url() is not a Zod API (the URL validator is z.string().url()). As written, the message schema for SPLICE_WALLET_BROADCAST_ORIGIN is invalid.
    z.object({
        type: z.literal(WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN),
        origin: z.url(),
    }),

wallet-gateway/remote/src/dapp-api/controller.ts:283

  • prepareExecute emits txChanged on the user-scoped notifier, but the SSE server subscribes to txChanged on the session-scoped notifier. This will prevent dApps from receiving transaction updates for the current session.
            const notifier = notificationService.getNotifier(context.userId)

            const commandId = params.commandId || v4()
            const transactionId = v4()

wallet-gateway/remote/src/middleware/sessionHandler.ts:44

  • When req.authContext is missing, this code calls withAuthContext(undefined).getSession(''), which can throw in store implementations that require an authenticated context. It should short-circuit to 401 before touching the store.
            logger.debug('Checking for active session for ' + context?.userId)
            const session = await store
                .withAuthContext(context)
                .getSession(context?.accessToken || '')
            if (!session) {

wallet-gateway/remote/src/web/frontend/listeners.ts:40

  • detectCurrentOrigin polls forever if the opener never sends the origin message (or if the listener fails), leaving a live interval and a Promise that never resolves.
    core/wallet-ui-components/src/windows/popup.ts:97
  • The origin broadcast poller runs indefinitely if the child window never acknowledges (e.g. popup blocked, navigation failure). This leaves a live interval + message listener in the parent page.
            // due to the asynchronicity when sending the postMessage immediately after redirecting,
            // there is a chance that the child window has not yet loaded,
            // and does not an event listener established yet. Therefore,
            // we repeatedly poll until the child window sends back an acknowledgment message.
            const handleMessage = (event: MessageEvent) => {

Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings August 4, 2026 15:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 72 out of 73 changed files in this pull request and generated no new comments.

Suppressed comments (11)

wallet-gateway/remote/src/web/frontend/state-manager.ts:23

  • The constructor currently removes every localStorage entry that doesn’t start with the new VERSION_PREFIX. This can delete unrelated application data stored on the same origin, and iterating for (const key in localStorage) while mutating storage can be unreliable. Consider limiting deletion to the previous wallet prefix only and iterating over a snapshot of keys.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:56
  • setWithStorage / clearWithStorage also update this.state using the unscoped key, which doesn’t match the new origin-based localStorage keys and perpetuates cross-origin cache collisions. These should use the same computed storage key as getWithStorage.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:38
  • getWithStorage caches values in this.state by key only. With origin-scoped storage keys, this causes collisions across different origins/sessions (e.g. reading networkId for origin B can return the cached value from origin A). Cache by the full computed storage key (and use that consistently when reading/writing).
    wallet-gateway/remote/src/web/frontend/listeners.ts:16
  • handleMessage can call window.opener.postMessage(...) even when window.opener is null (direct WG tab). Because the listener is registered unconditionally, any matching message event could throw at runtime. Guard early when there is no opener and only accept messages coming from the opener window.
    core/types/src/index.ts:138
  • z.url() is not a Zod API (Zod uses z.string().url()). As written this is likely a runtime/compile error in the SpliceMessage schema for SPLICE_WALLET_EXT_OPEN.
    z.object({
        type: z.literal(WalletEvent.SPLICE_WALLET_EXT_OPEN),
        url: z.url(),
        target: SpliceTarget.optional(),
    }),

core/types/src/index.ts:147

  • Same issue here: z.url() is not a standard Zod API. This schema should validate URLs via z.string().url() for the broadcast origin message.
    z.object({
        type: z.literal(WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN),
        origin: z.url(),
    }),

wallet-gateway/remote/src/middleware/sessionHandler.ts:44

  • If req.authContext is missing/undefined, this middleware still calls store.withAuthContext(context).getSession(''). Store implementations typically assertConnected() and can throw, turning an unauthenticated request into a 500. It’s safer to short-circuit to 401 when there’s no auth context or accessToken.
            logger.debug('Checking for active session for ' + context?.userId)
            const session = await store
                .withAuthContext(context)
                .getSession(context?.accessToken || '')
            if (!session) {

wallet-gateway/remote/src/web/frontend/listeners.ts:41

  • detectCurrentOrigin polls forever if the origin broadcast/ACK handshake never completes (e.g. popup blocked, postMessage target mismatch). This can hang callers and leak an interval timer. Consider adding a bounded timeout (and either reject or fall back to window.origin).
    core/wallet-ui-components/src/windows/popup.ts:97
  • The origin broadcast poller runs indefinitely until an ACK is received. If the child never ACKs (popup blocked, navigation error, origin mismatch), the interval and message listener will leak. Consider bounding retries / timing out and cleaning up listeners when the window closes.
            // due to the asynchronicity when sending the postMessage immediately after redirecting,
            // there is a chance that the child window has not yet loaded,
            // and does not an event listener established yet. Therefore,
            // we repeatedly poll until the child window sends back an acknowledgment message.
            const handleMessage = (event: MessageEvent) => {

wallet-gateway/remote/src/user-api/controller.ts:405

  • setPrimaryWallet uses authContext!.userId without first asserting connectivity. If authContext is ever undefined here (or if future refactors call this without middleware guarantees), this will throw. Other controller methods use assertConnected(authContext)—doing the same here makes the method safer and consistent.
    core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts:40
  • The index name sessions_one_session_per_origin_user suggests uniqueness by (user_id, origin), but the actual index is on (network, user_id, origin). This does not enforce “one session per origin per user” across networks, and it also doesn’t match the StoreSql.setSession behavior (which deletes by userId + origin, regardless of network). Consider making the index definition match the intended uniqueness.
    await sql`
            CREATE UNIQUE INDEX IF NOT EXISTS sessions_one_session_per_origin_user
            ON sessions(network, user_id, origin)
        `.execute(db)

Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Copilot AI review requested due to automatic review settings August 4, 2026 16:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 72 out of 73 changed files in this pull request and generated 1 comment.

Suppressed comments (6)

wallet-gateway/remote/src/web/frontend/state-manager.ts:23

  • The constructor currently removes all localStorage keys that don't start with the current VERSION_PREFIX. This can wipe unrelated data stored on the same origin (including any future keys not managed by StateManager). Also, iterating with for...in localStorage is unreliable for Storage.

Consider only removing previous wallet keys (e.g. com.splice.wallet.) that are not the current version, and iterate via localStorage.key(i).
wallet-gateway/remote/src/web/frontend/state-manager.ts:41

  • this.state is keyed only by key (e.g. "networkId"), but these values are now origin-scoped. If the UI interacts with multiple origins, reads can return the wrong origin’s cached value because the in-memory cache ignores origin (and the selected storage).
    wallet-gateway/remote/src/web/frontend/state-manager.ts:56
  • setWithStorage writes to the in-memory cache using just key, which collides across origins (and between localStorage/sessionStorage). This will corrupt cached values when multiple origins are in use.
    wallet-gateway/remote/src/web/frontend/state-manager.ts:65
  • clearWithStorage deletes from the in-memory cache using only key, which won’t clear the origin-scoped cache entries once the cache key includes origin/storage (and even today can collide across origins). This can leave stale cached values after a clear.
    wallet-gateway/remote/src/web/frontend/listeners.ts:14
  • handleMessage can run when window.opener is null (e.g. the WG is opened directly). In that case, window.opener.postMessage(...) will throw if a matching message is received. The listener should ignore messages unless there is an opener and the opener is the message source.
    core/wallet-store-inmemory/src/store-internal.ts:256
  • StoreInternal.setSession() keys sessions by access token only and never removes any existing session for the same origin. This diverges from the SQL store (and migration 015’s unique index), and in flows like API-key auth (which calls setSession twice with the same origin but different tokens) it can leave multiple active sessions for a single origin.
    async setSession(session: Session): Promise<void> {
        const storage = this.getStorage()
        storage.sessions.set(session.accessToken, session)
        this.updateStorage(storage)
    }

Comment thread wallet-gateway/remote/src/web/frontend/listeners.ts

@mjuchli-da mjuchli-da left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @alexmatson-da !

@mjuchli-da
mjuchli-da enabled auto-merge (squash) August 4, 2026 21:30
@mjuchli-da
mjuchli-da merged commit 59032eb into main Aug 4, 2026
51 of 53 checks passed
@mjuchli-da
mjuchli-da deleted the alex/multi-session branch August 4, 2026 22:16
mateuszpiatkowski-da pushed a commit that referenced this pull request Aug 10, 2026
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Co-authored-by: Marc Juchli <marc.juchli@digitalasset.com>
mateuszpiatkowski-da pushed a commit that referenced this pull request Aug 13, 2026
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Co-authored-by: Marc Juchli <marc.juchli@digitalasset.com>
mateuszpiatkowski-da pushed a commit that referenced this pull request Aug 13, 2026
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Co-authored-by: Marc Juchli <marc.juchli@digitalasset.com>
Signed-off-by: Mateusz Piątkowski <mateusz.piatkowski@digitalasset.com>
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.

3 participants