fix: SessionStore race conditions and async initialization (WAPI-1118)

apps/integration-tests/src/end-to-end.integration.test.ts

@@ -94,8 +94,8 @@ t.describe("E2E Integration Test", () => {
 	t.beforeEach(async () => {
 		dappKvStore = new InMemoryKVStore();
 		walletKvStore = new InMemoryKVStore();
-		dappSessionStore = new SessionStore(dappKvStore);
-		walletSessionStore = new SessionStore(walletKvStore);
+		dappSessionStore = await SessionStore.create(dappKvStore);
+		walletSessionStore = await SessionStore.create(walletKvStore);
 
 		const dappTransport = await WebSocketTransport.create({ url: RELAY_URL, kvstore: dappKvStore, websocket: WebSocket });
 		const walletTransport = await WebSocketTransport.create({ url: RELAY_URL, kvstore: walletKvStore, websocket: WebSocket });
@@ -251,8 +251,8 @@ t.describe("E2E Integration Test via Proxy", () => {
 
 		dappKvStore = new InMemoryKVStore();
 		walletKvStore = new InMemoryKVStore();
-		dappSessionStore = new SessionStore(dappKvStore);
-		walletSessionStore = new SessionStore(walletKvStore);
+		dappSessionStore = await SessionStore.create(dappKvStore);
+		walletSessionStore = await SessionStore.create(walletKvStore);
 
 		// DApp connects directly, Wallet connects through proxy
 		const dappTransport = await WebSocketTransport.create({ url: RELAY_URL, kvstore: dappKvStore, websocket: WebSocket });

apps/load-tests/src/client/session-pair.ts

@@ -93,8 +93,8 @@ export async function createSessionPair(options: CreateSessionPairOptions): Prom
 	// Create isolated stores for each client
 	const dappKvStore = new InMemoryKVStore();
 	const walletKvStore = new InMemoryKVStore();
-	const dappSessionStore = new SessionStore(dappKvStore);
-	const walletSessionStore = new SessionStore(walletKvStore);
+	const dappSessionStore = await SessionStore.create(dappKvStore);
+	const walletSessionStore = await SessionStore.create(walletKvStore);
 
 	// Use MockKeyManager - no real crypto (see key-manager.ts for rationale)
 	const keyManager = new MockKeyManager();

apps/load-tests/src/scenarios/async-delivery-helpers.ts

@@ -32,8 +32,8 @@ export interface AsyncDeliverySession {
 export async function setupAsyncDeliverySession(url: string, timeoutMs = 60000): Promise<AsyncDeliverySession | null> {
 	const dappKvStore = new InMemoryKVStore();
 	const walletKvStore = new InMemoryKVStore();
-	const dappSessionStore = new SessionStore(dappKvStore);
-	const walletSessionStore = new SessionStore(walletKvStore);
+	const dappSessionStore = await SessionStore.create(dappKvStore);
+	const walletSessionStore = await SessionStore.create(walletKvStore);
 	const keyManager = new MockKeyManager();
 
 	let dappClient: DappClient | null = null;
@@ -93,7 +93,7 @@ export async function setupAsyncDeliverySession(url: string, timeoutMs = 60000):
 export async function testAsyncRecovery(session: AsyncDeliverySession, timeoutMs = 30000): Promise<AsyncDeliveryTestResult> {
 	const { sessionId, url, dappClient, walletKvStore } = session;
 	const keyManager = new MockKeyManager();
-	const walletSessionStore = new SessionStore(walletKvStore);
+	const walletSessionStore = await SessionStore.create(walletKvStore);
 
 	let walletClient: WalletClient | null = null;
 	let timeoutHandle: ReturnType<typeof setTimeout> | null = null;

apps/rn-demo/context/WalletContext.tsx

@@ -62,7 +62,7 @@ export function WalletProvider({ children }: { children: ReactNode }) {
 				setIsInitializing(true);
 
 				const kvstore = new AsyncStorageKVStore("wallet-");
-				const sessionstore = new SessionStore(kvstore);
+				const sessionstore = await SessionStore.create(kvstore);
 
 				manager = new SessionManager(sessionstore, RELAY_URL);
 				setSessionManager(manager);

apps/web-demo/src/components/BasicDemo.tsx

@@ -27,8 +27,8 @@ export default function BasicDemo() {
 				const dappKvStore = new LocalStorageKVStore("dapp-");
 				const walletKvStore = new LocalStorageKVStore("wallet-");
 
-				const dappSessionStore = new SessionStore(dappKvStore);
-				const walletSessionStore = new SessionStore(walletKvStore);
+				const dappSessionStore = await SessionStore.create(dappKvStore);
+				const walletSessionStore = await SessionStore.create(walletKvStore);
 
 				// Create transports (in a real app, these would be separate instances)
 				const dappTransport = await WebSocketTransport.create({

apps/web-demo/src/components/MetaMaskMobileDemo.tsx

@@ -178,7 +178,7 @@ export default function MetaMaskMobileDemo() {
 			addDappLog("system", "Creating dApp client...");
 
 			const dappKvStore = new LocalStorageKVStore("metamask-mobile-demo-dapp/");
-			const sessionStore = new SessionStore(dappKvStore);
+			const sessionStore = await SessionStore.create(dappKvStore);
 			setDappSessionStore(sessionStore);
 
 			const dappTransport = await WebSocketTransport.create({

apps/web-demo/src/components/TrustedDemo.tsx

@@ -172,7 +172,7 @@ export default function TrustedDemo() {
 			addDappLog("system", "Creating dApp client...");
 
 			const dappKvStore = new LocalStorageKVStore("trusted-demo-dapp-");
-			const dappSessionStore = new SessionStore(dappKvStore);
+			const dappSessionStore = await SessionStore.create(dappKvStore);
 
 			const dappTransport = await WebSocketTransport.create({
 				url: RELAY_URL,
@@ -316,7 +316,7 @@ export default function TrustedDemo() {
 			addWalletLog("system", "Creating wallet client...");
 
 			const walletKvStore = new LocalStorageKVStore("trusted-demo-wallet-");
-			const walletSessionStore = new SessionStore(walletKvStore);
+			const walletSessionStore = await SessionStore.create(walletKvStore);
 
 			const walletTransport = await WebSocketTransport.create({
 				url: RELAY_URL,

apps/web-demo/src/components/UntrustedDemo.tsx

@@ -177,7 +177,7 @@ export default function UntrustedDemo() {
 			addDappLog("system", "Creating dApp client...");
 
 			const dappKvStore = new LocalStorageKVStore("untrusted-demo-dapp-");
-			const dappSessionStore = new SessionStore(dappKvStore);
+			const dappSessionStore = await SessionStore.create(dappKvStore);
 
 			const dappTransport = await WebSocketTransport.create({
 				url: RELAY_URL,
@@ -350,7 +350,7 @@ export default function UntrustedDemo() {
 			addWalletLog("system", "Creating wallet client...");
 
 			const walletKvStore = new LocalStorageKVStore("untrusted-demo-wallet-");
-			const walletSessionStore = new SessionStore(walletKvStore);
+			const walletSessionStore = await SessionStore.create(walletKvStore);
 
 			const walletTransport = await WebSocketTransport.create({
 				url: RELAY_URL,

packages/core/CHANGELOG.md

@@ -10,9 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Add `validatePeerKey` method to `IKeyManager` interface for peer public key validation at handshake and resume time ([#70](https://github.com/MetaMask/mobile-wallet-protocol/pull/70))
+- **BREAKING:** `SessionStore` constructor is now private; use `await SessionStore.create(kvstore)` ([#71](https://github.com/MetaMask/mobile-wallet-protocol/pull/71))
 
 ### Fixed
 
+- Fix `SessionStore` race conditions and fire-and-forget garbage collection ([#71](https://github.com/MetaMask/mobile-wallet-protocol/pull/71))
 - Guard against `NaN` in session expiry timestamps ([#70](https://github.com/MetaMask/mobile-wallet-protocol/pull/70))
 
 ## [0.3.1]

packages/core/package.json

@@ -28,6 +28,7 @@
 		"registry": "https://registry.npmjs.org/"
 	},
 	"dependencies": {
+		"async-mutex": "^0.5.0",
 		"centrifuge": "^5.3.5",
 		"eventemitter3": "^5.0.1",
 		"uuid": "^11.1.0"

packages/core/src/base-client.integration.test.ts

@@ -101,8 +101,8 @@ t.describe("BaseClient", () => {
 		const kvstoreA = new InMemoryKVStore();
 		const kvstoreB = new InMemoryKVStore();
 
-		sessionStoreA = new SessionStore(kvstoreA);
-		sessionStoreB = new SessionStore(kvstoreB);
+		sessionStoreA = await SessionStore.create(kvstoreA);
+		sessionStoreB = await SessionStore.create(kvstoreB);
 
 		const transportA = await WebSocketTransport.create({ url: WEBSOCKET_URL, kvstore: kvstoreA, websocket: WebSocket });
 		const transportB = await WebSocketTransport.create({ url: WEBSOCKET_URL, kvstore: kvstoreB, websocket: WebSocket });
@@ -292,7 +292,7 @@ t.describe("BaseClient", () => {
 		// 3. Create a third client to publish garbage data
 		const kvstoreC = new InMemoryKVStore();
 		const transportC = await WebSocketTransport.create({ url: WEBSOCKET_URL, kvstore: kvstoreC, websocket: WebSocket });
-		const clientC = new TestClient(transportC, new KeyManager(), new SessionStore(kvstoreC));
+		const clientC = new TestClient(transportC, new KeyManager(), await SessionStore.create(kvstoreC));
 		await clientC["transport"].connect();
 
 		// 4. Listen for error events on client B

packages/core/src/session-store/index.test.ts

@@ -41,9 +41,9 @@ t.describe("SessionStore", () => {
 		expiresAt,
 	});
 
-	t.beforeEach(() => {
+	t.beforeEach(async () => {
 		kvstore = new MockKVStore();
-		sessionstore = new SessionStore(kvstore);
+		sessionstore = await SessionStore.create(kvstore);
 	});
 
 	t.test("should set and get a session", async () => {
@@ -135,25 +135,47 @@ t.describe("SessionStore", () => {
 		t.expect(retrieved).toBeNull();
 	});
 
-	t.test("should garbage collect expired sessions", async () => {
-		const valid = createSession("valid", Date.now() + 10000);
-		const expired = createSession("expired", Date.now() - 10000);
-
-		// Manually set sessions in the kvstore
-		await sessionstore.set(valid);
-		// Manually setting an expired session by bypassing the `set` method's check
-		const expiredKey = `session:${expired.id}`;
-		await kvstore.set(expiredKey, JSON.stringify(expired));
-		await (kvstore as MockKVStore)["store"].set("sessions:master-list", JSON.stringify(["valid", "expired"]));
-
-		// Manually trigger garbage collection
-		await (sessionstore as any).garbageCollect();
-
-		const list = await sessionstore.list();
+	t.test("should complete GC before the first public method returns", async () => {
+		const freshKvstore = new MockKVStore();
+		const validSession = createSession("valid", Date.now() + 10000);
+		const expiredSession = createSession("expired", Date.now() - 10000);
+
+		// Seed store with one valid and one expired session
+		await freshKvstore.set(
+			"session:valid",
+			JSON.stringify({
+				...validSession,
+				keyPair: { publicKeyB64: Buffer.from(validSession.keyPair.publicKey).toString("base64"), privateKeyB64: Buffer.from(validSession.keyPair.privateKey).toString("base64") },
+				theirPublicKeyB64: Buffer.from(validSession.theirPublicKey).toString("base64"),
+			}),
+		);
+		await freshKvstore.set(
+			"session:expired",
+			JSON.stringify({
+				...expiredSession,
+				keyPair: {
+					publicKeyB64: Buffer.from(expiredSession.keyPair.publicKey).toString("base64"),
+					privateKeyB64: Buffer.from(expiredSession.keyPair.privateKey).toString("base64"),
+				},
+				theirPublicKeyB64: Buffer.from(expiredSession.theirPublicKey).toString("base64"),
+			}),
+		);
+		await freshKvstore.set("sessions:master-list", JSON.stringify(["valid", "expired"]));
+
+		const store = await SessionStore.create(freshKvstore);
+		const list = await store.list();
 		t.expect(list).toHaveLength(1);
 		t.expect(list[0].id).toBe("valid");
+	});
+
+	t.test("should not lose entries when multiple sessions are set concurrently", async () => {
+		const promises = [];
+		for (let i = 0; i < 10; i++) {
+			promises.push(sessionstore.set(createSession(`concurrent-${i}`, Date.now() + 10000)));
+		}
+		await Promise.all(promises);
 
-		const rawExpired = await kvstore.get(expiredKey);
-		t.expect(rawExpired).toBeNull();
+		const list = await sessionstore.list();
+		t.expect(list).toHaveLength(10);
 	});
 });

packages/core/src/session-store/index.ts

@@ -1,3 +1,4 @@
+import { Mutex } from "async-mutex";
 import { ErrorCode, SessionError } from "../domain/errors";
 import type { IKVStore } from "../domain/kv-store";
 import type { Session } from "../domain/session";
@@ -31,10 +32,20 @@ export class SessionStore implements ISessionStore {
 	private static readonly MASTER_LIST_KEY = "sessions:master-list";
 
 	private readonly kvstore: IKVStore;
+	private readonly mutex = new Mutex();
 
-	constructor(kvstore: IKVStore) {
+	/**
+	 * Creates a new SessionStore instance and runs initial garbage collection.
+	 * Use this instead of the constructor to ensure GC completes before use.
+	 */
+	static async create(kvstore: IKVStore): Promise<SessionStore> {
+		const store = new SessionStore(kvstore);
+		await store.garbageCollect();
+		return store;
+	}
+
+	private constructor(kvstore: IKVStore) {
 		this.kvstore = kvstore;
-		this.garbageCollect(); // Run garbage collection once on startup
 	}
 
 	/**
@@ -169,20 +180,24 @@ export class SessionStore implements ISessionStore {
 	 * @param id - The ID of the session to add.
 	 */
 	private async addToMasterList(id: string): Promise<void> {
-		const list = await this.getMasterList();
-		if (!list.includes(id)) {
-			list.push(id);
-			await this.kvstore.set(SessionStore.MASTER_LIST_KEY, JSON.stringify(list));
-		}
+		await this.mutex.runExclusive(async () => {
+			const list = await this.getMasterList();
+			if (!list.includes(id)) {
+				list.push(id);
+				await this.kvstore.set(SessionStore.MASTER_LIST_KEY, JSON.stringify(list));
+			}
+		});
 	}
 
 	/**
 	 * Removes a session ID from the master list.
 	 * @param id - The ID of the session to remove.
 	 */
 	private async removeFromMasterList(id: string): Promise<void> {
-		const list = await this.getMasterList();
-		const filtered = list.filter((sessionId) => sessionId !== id);
-		await this.kvstore.set(SessionStore.MASTER_LIST_KEY, JSON.stringify(filtered));
+		await this.mutex.runExclusive(async () => {
+			const list = await this.getMasterList();
+			const filtered = list.filter((sessionId) => sessionId !== id);
+			await this.kvstore.set(SessionStore.MASTER_LIST_KEY, JSON.stringify(filtered));
+		});
 	}
 }

packages/dapp-client/src/client.integration.test.ts

@@ -48,7 +48,7 @@ t.describe("DappClient Integration Tests", () => {
 
 	t.beforeEach(async () => {
 		const dappKvStore = new InMemoryKVStore();
-		const dappSessionStore = new SessionStore(dappKvStore);
+		const dappSessionStore = await SessionStore.create(dappKvStore);
 		const dappTransport = await WebSocketTransport.create({ url: RELAY_URL, kvstore: dappKvStore, websocket: WebSocket });
 		dappClient = new DappClient({ transport: dappTransport, sessionstore: dappSessionStore, keymanager: new KeyManager() });
 	});

packages/wallet-client/src/client.integration.test.ts

@@ -51,7 +51,7 @@ t.describe("WalletClient Integration Tests", () => {
 
 	t.beforeEach(async () => {
 		const walletKvStore = new InMemoryKVStore();
-		const walletSessionStore = new SessionStore(walletKvStore);
+		const walletSessionStore = await SessionStore.create(walletKvStore);
 		const walletTransport = await WebSocketTransport.create({ url: RELAY_URL, kvstore: walletKvStore, websocket: WebSocket });
 		walletClient = new WalletClient({ transport: walletTransport, sessionstore: walletSessionStore, keymanager: new KeyManager() });
 
@@ -77,7 +77,7 @@ t.describe("WalletClient Integration Tests", () => {
 
 		// Create fresh client for second test to avoid state issues
 		const walletKvStore2 = new InMemoryKVStore();
-		const walletSessionStore2 = new SessionStore(walletKvStore2);
+		const walletSessionStore2 = await SessionStore.create(walletKvStore2);
 		const walletTransport2 = await WebSocketTransport.create({ url: RELAY_URL, kvstore: walletKvStore2, websocket: WebSocket });
 		const walletClient2 = new WalletClient({ transport: walletTransport2, sessionstore: walletSessionStore2, keymanager: new KeyManager() });
 

yarn.lock

@@ -2764,6 +2764,7 @@ __metadata:
     "@metamask/auto-changelog": ^5.0.2
     "@types/uuid": ^10.0.0
     "@types/ws": ^8.18.1
+    async-mutex: ^0.5.0
     centrifuge: ^5.3.5
     eciesjs: ^0.4.15
     eventemitter3: ^5.0.1
@@ -5421,6 +5422,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"async-mutex@npm:^0.5.0":
+  version: 0.5.0
+  resolution: "async-mutex@npm:0.5.0"
+  dependencies:
+    tslib: ^2.4.0
+  checksum: be1587f4875f3bb15e34e9fcce82eac2966daef4432c8d0046e61947fb9a1b95405284601bc7ce4869319249bc07c75100880191db6af11d1498931ac2a2f9ea
+  languageName: node
+  linkType: hard
+
 "asynckit@npm:^0.4.0":
   version: 0.4.0
   resolution: "asynckit@npm:0.4.0"