diff --git a/packages/wallet/src/integration/disconnect-delivery.test.ts b/packages/wallet/src/integration/disconnect-delivery.test.ts new file mode 100644 index 0000000..bd67336 --- /dev/null +++ b/packages/wallet/src/integration/disconnect-delivery.test.ts @@ -0,0 +1,98 @@ +// Copyright (C) 2026 Whiterun LLC, +// This software is licensed under the GNU Lesser General Public License (LGPL), version 3.0 or later. +// A copy of the license can be found in the LICENSE file or at https://www.gnu.org/licenses/lgpl-3.0.html + +/** + * Wallet-initiated disconnect must actually reach the dapp. + * + * disconnect.test.ts covers the other direction — the dapp sends `disconnect` + * and the wallet reacts. Nothing covered wallet → dapp, which is how this got + * shipped broken: doDisconnect fired the courtesy message without awaiting it and + * then tore the relay connection down, so the publish died mid-flight and the + * dapp went on believing the wallet was connected until its own liveness + * timeout. Downstream wallets were carrying a patch for it. + * + * These tests are worth their runtime because the failure is invisible locally — + * the wallet's own state is correct either way, and only the peer notices. + */ + +import { describe, it, expect, afterEach } from "vitest"; +import { RelayMsgAction, DisconnectReason } from "@wizardconnect/core"; +import type { DisconnectMessage, ProtocolMessage } from "@wizardconnect/core"; + +import { setupConnection, waitFor, type ConnectionHandles } from "./helpers.js"; + +let handles: ConnectionHandles | null = null; + +afterEach(() => { + handles?.cleanup(); + handles = null; +}); + +/** Disconnect messages the dapp actually received off the relay. */ +function disconnectsSeenByDapp(h: ConnectionHandles): DisconnectMessage[] { + return h.dapp.messages.filter( + (msg: ProtocolMessage) => msg.action === RelayMsgAction.Disconnect, + ) as DisconnectMessage[]; +} + +describe("wallet-initiated disconnect", () => { + it("delivers the courtesy disconnect to the dapp", async () => { + handles = await setupConnection(); + expect(disconnectsSeenByDapp(handles)).toHaveLength(0); + + handles.wallet.manager.disconnect(handles.wallet.connectionId); + + // The whole point: this arrives over a real relay, which it cannot do if the + // connection is torn down while the publish is still in flight. + await waitFor(() => disconnectsSeenByDapp(handles!).length > 0, { + timeoutMs: 20000, + what: "disconnect message received by the dapp", + }); + + expect(disconnectsSeenByDapp(handles)[0].reason).toBe( + DisconnectReason.UserDisconnect, + ); + }); + + it("removes the connection immediately, without waiting for delivery", async () => { + // Teardown is deferred, but the registry must not be: a caller that + // disconnects and then inspects state should never see the dying connection. + handles = await setupConnection(); + const { manager, connectionId } = handles.wallet; + expect(Object.keys(manager.getConnections())).toContain(connectionId); + + manager.disconnect(connectionId); + + expect(Object.keys(manager.getConnections())).not.toContain(connectionId); + }); + + it("delivers a disconnect for every connection in disconnectAll", async () => { + handles = await setupConnection(); + + handles.wallet.manager.disconnectAll(); + + await waitFor(() => disconnectsSeenByDapp(handles!).length > 0, { + timeoutMs: 20000, + what: "disconnect message from disconnectAll", + }); + expect(Object.keys(handles.wallet.manager.getConnections())).toHaveLength( + 0, + ); + }); + + it("lets the dapp reconnect on the same URI afterwards", async () => { + // Deferring teardown must not leave the URI unusable — connect() returns an + // existing connection for a URI, so a stale entry would be handed back. + handles = await setupConnection(); + const { manager, connectionId } = handles.wallet; + + manager.disconnect(connectionId); + const reconnectedId = manager.connect(handles.dapp.uri); + + expect(reconnectedId).not.toBe(connectionId); + expect(Object.keys(manager.getConnections())).toContain(reconnectedId); + + manager.disconnect(reconnectedId); + }); +}); diff --git a/packages/wallet/src/wallet-connection-manager.ts b/packages/wallet/src/wallet-connection-manager.ts index dd189b1..d95f09b 100644 --- a/packages/wallet/src/wallet-connection-manager.ts +++ b/packages/wallet/src/wallet-connection-manager.ts @@ -45,6 +45,15 @@ export interface PendingSignRequest { request: SignTransactionRequest; } +/** + * How long doDisconnect waits for the courtesy `disconnect` message to be + * published before tearing the relay connection down anyway. + * + * Generous enough for a slow relay, short enough that an unreachable one cannot + * hold the socket open indefinitely. + */ +const DISCONNECT_PUBLISH_TIMEOUT_MS = 5000; + interface ActiveConnection { id: string; uri: string; @@ -213,37 +222,62 @@ export class WalletConnectionManager extends EventEmitter { - for (const seq of conn.signSequences) { - this.activeSignSequences.delete(seq); - } - clearInterval(conn.notificationProcessor ?? undefined); - conn.cleanup(); - this.connections.delete(connectionId); - this.emit("connectionsChanged"); + if (!sendMessage || !conn.client) { + conn.cleanup(); + return; } - if (sendMessage && conn.client) { - const disconnectMsg = { - action: RelayMsgAction.Disconnect, - reason: DisconnectReason.UserDisconnect, - time: Math.floor(Date.now() / 1000), - }; - conn.client.relay(disconnectMsg) - .then(() => { - clean() - }) - .catch(() => clean() ); + const disconnectMsg: DisconnectMessage = { + action: RelayMsgAction.Disconnect, + reason: DisconnectReason.UserDisconnect, + time: Math.floor(Date.now() / 1000), + }; - } else { - clean() - } + // Tear down only once the courtesy message has actually gone out. + // + // relay() resolves after `Promise.allSettled(pool.publish(...))` — a real + // round trip to every configured relay. Calling conn.cleanup() straight + // after firing it closed the pool underneath the in-flight publish, so the + // disconnect usually never reached the relay and the dapp went on believing + // the wallet was connected until its own liveness timeout fired. Downstream + // wallets were patching this out of the published package. + // + // Bounded, because "the publish never settles" is exactly the case where a + // relay is unreachable, and a socket that is never closed is worse than a + // courtesy message that is never delivered. + let torndown = false; + const teardown = () => { + if (torndown) return; + torndown = true; + conn.cleanup(); + }; + + const timer = setTimeout(teardown, DISCONNECT_PUBLISH_TIMEOUT_MS); + conn.client + .relay(disconnectMsg) + .catch(() => { + // Nothing to do: we are disconnecting either way. + }) + .finally(() => { + clearTimeout(timer); + teardown(); + }); } /**