fix(deploy): persist session key only after map_account succeeds (closes #94) (#106)

Author: EnderOfWorlds007

.changeset/fix-session-persist-after-map.md

@@ -0,0 +1,5 @@
+---
+"playground-cli": patch
+---
+
+Fixed an intermittent `Revive::AccountUnmapped` failure during contract deploys. The per-deploy session key is now persisted to `~/.config/dot/accounts.json` only AFTER its `map_account` extrinsic is confirmed on chain. Previously the persist happened first, so a failing `map_account` (e.g. nonce race, transient chain error) left the on-disk state lying — the retry would find the existing key, skip the mapping step, and fail at the dry-run with AccountUnmapped. Fixes #94.

src/utils/deploy/run.test.ts

@@ -106,6 +106,7 @@ vi.mock("./contracts.js", () => ({
 }));
 vi.mock("./session-account.js", () => ({
     getOrCreateSessionAccount: getOrCreateSessionAccountMock,
+    persistSessionAccount: vi.fn(async () => {}),
     SESSION_MIN_BALANCE: 5_000_000_000n,
     SESSION_FUND_AMOUNT: 50_000_000_000n,
 }));

src/utils/deploy/run.ts

@@ -20,6 +20,7 @@ import { publishToPlayground, normalizeDomain } from "./playground.js";
 import { runContractsPhase, type ContractsPhaseEvent } from "./contracts.js";
 import {
     getOrCreateSessionAccount,
+    persistSessionAccount,
     SESSION_FUND_AMOUNT,
     SESSION_MIN_BALANCE,
 } from "./session-account.js";
@@ -278,6 +279,7 @@ async function maybeRunContracts(
                     client.assetHub.tx.Revive.map_account(),
                     session.account.signer,
                 );
+                await persistSessionAccount(session);
             }
 
             const result = await runContractsPhase({

src/utils/deploy/session-account.test.ts

@@ -2,7 +2,13 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest";
 import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { getOrCreateSessionAccount, readSessionAccount } from "./session-account.js";
+import {
+    getOrCreateSessionAccount,
+    persistSessionAccount,
+    readSessionAccount,
+} from "./session-account.js";
+
+// ── getOrCreateSessionAccount ────────────────────────────────────────────────
 
 describe("getOrCreateSessionAccount", () => {
     let tmp: string;
@@ -23,46 +29,132 @@ describe("getOrCreateSessionAccount", () => {
         rmSync(tmp, { recursive: true, force: true });
     });
 
-    it("generates a new key on first call and persists the mnemonic", async () => {
+    it("returns created=true and a valid key on first call", async () => {
         const { info, created } = await getOrCreateSessionAccount();
 
         expect(created).toBe(true);
         expect(info.mnemonic.split(" ").length).toBeGreaterThanOrEqual(12);
         expect(info.account.ss58Address).toMatch(/^[1-9A-HJ-NP-Za-km-z]+$/);
         expect(info.account.h160Address).toMatch(/^0x[0-9a-fA-F]{40}$/);
         expect(typeof info.account.signer.signTx).toBe("function");
+    });
+
+    it("does NOT write accounts.json before persistSessionAccount is called", async () => {
+        await getOrCreateSessionAccount();
 
         const path = join(tmp, "accounts.json");
-        expect(existsSync(path)).toBe(true);
-        const stored = JSON.parse(readFileSync(path, "utf8"));
-        expect(stored.default).toBe(info.mnemonic);
+        expect(existsSync(path)).toBe(false);
     });
 
-    it("returns the same key on subsequent calls with created=false", async () => {
+    it("returns created=false and existing key when file already has a key", async () => {
+        // Seed the file via persist on the first creation.
         const first = await getOrCreateSessionAccount();
+        await persistSessionAccount(first.info);
+
         const second = await getOrCreateSessionAccount();
 
-        expect(first.created).toBe(true);
         expect(second.created).toBe(false);
         expect(second.info.mnemonic).toBe(first.info.mnemonic);
         expect(second.info.account.ss58Address).toBe(first.info.account.ss58Address);
     });
 
-    it("ignores garbage in the store and regenerates a valid key", async () => {
+    it("returns created=true (new key) on retry when file was never written (map_account failed)", async () => {
+        // Simulate: first create, map_account throws → no persist → retry.
+        const first = await getOrCreateSessionAccount();
+        expect(first.created).toBe(true);
+
+        // map_account failed; we never called persistSessionAccount.
+        // On retry getOrCreateSessionAccount should mint a fresh key.
+        const retry = await getOrCreateSessionAccount();
+        expect(retry.created).toBe(true);
+        // A fresh mnemonic is generated each time.
+        expect(retry.info.mnemonic).not.toBe(first.info.mnemonic);
+    });
+
+    it("ignores garbage in the store and returns a fresh key", async () => {
         const path = join(tmp, "accounts.json");
-        await getOrCreateSessionAccount();
-        const { writeFileSync } = await import("node:fs");
+        // Write garbage into the file to simulate corruption.
+        const { writeFileSync, mkdirSync } = await import("node:fs");
+        const { dirname } = await import("node:path");
+        mkdirSync(dirname(path), { recursive: true });
         writeFileSync(path, JSON.stringify({ default: { not: "a string" } }));
 
         const { info, created } = await getOrCreateSessionAccount();
         expect(created).toBe(true);
         expect(info.mnemonic.split(" ").length).toBeGreaterThanOrEqual(12);
 
+        // The garbage file is still there; it should not have been overwritten.
         const stored = JSON.parse(readFileSync(path, "utf8"));
-        expect(typeof stored.default).toBe("string");
+        expect(typeof stored.default).not.toBe("string");
     });
 });
 
+// ── persistSessionAccount ────────────────────────────────────────────────────
+
+describe("persistSessionAccount", () => {
+    let tmp: string;
+    let originalRoot: string | undefined;
+
+    beforeEach(() => {
+        tmp = mkdtempSync(join(tmpdir(), "pg-session-persist-"));
+        originalRoot = process.env.POLKADOT_ROOT;
+        process.env.POLKADOT_ROOT = tmp;
+    });
+
+    afterEach(() => {
+        if (originalRoot === undefined) {
+            delete process.env.POLKADOT_ROOT;
+        } else {
+            process.env.POLKADOT_ROOT = originalRoot;
+        }
+        rmSync(tmp, { recursive: true, force: true });
+    });
+
+    it("writes accounts.json with the mnemonic under the 'default' key", async () => {
+        const { info } = await getOrCreateSessionAccount();
+        await persistSessionAccount(info);
+
+        const path = join(tmp, "accounts.json");
+        expect(existsSync(path)).toBe(true);
+        const stored = JSON.parse(readFileSync(path, "utf8"));
+        expect(stored.default).toBe(info.mnemonic);
+    });
+
+    it("after persist, getOrCreateSessionAccount returns created=false", async () => {
+        const { info } = await getOrCreateSessionAccount();
+        await persistSessionAccount(info);
+
+        const second = await getOrCreateSessionAccount();
+        expect(second.created).toBe(false);
+        expect(second.info.mnemonic).toBe(info.mnemonic);
+    });
+
+    it("map_account-fails scenario: no persist → retry mints new key and re-maps", async () => {
+        // Step 1: create key in memory (map_account would be attempted here).
+        const attempt1 = await getOrCreateSessionAccount();
+        expect(attempt1.created).toBe(true);
+
+        // Step 2: map_account throws — do NOT call persistSessionAccount.
+        // (No action needed — file is untouched.)
+
+        // Step 3: retry — should produce a fresh key with created=true.
+        const attempt2 = await getOrCreateSessionAccount();
+        expect(attempt2.created).toBe(true);
+        // It's a different key, so the retry path would re-attempt map_account.
+        expect(attempt2.info.mnemonic).not.toBe(attempt1.info.mnemonic);
+
+        // Step 4: map_account succeeds — now persist.
+        await persistSessionAccount(attempt2.info);
+
+        // Step 5: subsequent call loads from disk.
+        const loaded = await getOrCreateSessionAccount();
+        expect(loaded.created).toBe(false);
+        expect(loaded.info.mnemonic).toBe(attempt2.info.mnemonic);
+    });
+});
+
+// ── readSessionAccount ────────────────────────────────────────────────────────
+
 describe("readSessionAccount", () => {
     let tmp: string;
     let originalRoot: string | undefined;
@@ -87,10 +179,12 @@ describe("readSessionAccount", () => {
     });
 
     it("returns the persisted key without creating a new one", async () => {
-        const { info: created } = await getOrCreateSessionAccount();
+        const { info } = await getOrCreateSessionAccount();
+        await persistSessionAccount(info);
+
         const loaded = await readSessionAccount();
-        expect(loaded?.mnemonic).toBe(created.mnemonic);
-        expect(loaded?.account.ss58Address).toBe(created.account.ss58Address);
+        expect(loaded?.mnemonic).toBe(info.mnemonic);
+        expect(loaded?.account.ss58Address).toBe(info.account.ss58Address);
     });
 
     it("does not create a file when the store is empty (read-only)", async () => {

src/utils/deploy/session-account.ts

@@ -77,6 +77,32 @@ class FileKvStore implements KvStore {
     }
 }
 
+/** In-memory KvStore — used to mint a session key without touching disk. */
+class InMemoryKvStore implements KvStore {
+    private readonly data: Record<string, string> = {};
+
+    async get(key: string): Promise<string | null> {
+        return this.data[key] ?? null;
+    }
+
+    async set(key: string, value: string): Promise<void> {
+        this.data[key] = value;
+    }
+
+    async remove(key: string): Promise<void> {
+        delete this.data[key];
+    }
+
+    async getJSON<T>(key: string): Promise<T | null> {
+        const raw = await this.get(key);
+        return raw === null ? null : (JSON.parse(raw) as T);
+    }
+
+    async setJSON(key: string, value: unknown): Promise<void> {
+        await this.set(key, JSON.stringify(value));
+    }
+}
+
 /** Read the persisted session key; returns null on a miss (does not mint). */
 export async function readSessionAccount(): Promise<SessionKeyInfo | null> {
     const store = new FileKvStore(accountsPath());
@@ -85,17 +111,36 @@ export async function readSessionAccount(): Promise<SessionKeyInfo | null> {
 }
 
 /**
- * Load the persisted session key, or mint + save a fresh one on first call.
- * `created` is true only on the minting call — callers use it to gate the
- * one-time `Revive.map_account` bootstrap.
+ * Load the persisted session key, or mint a fresh one in memory on first call.
+ *
+ * `created` is true only on the minting call — callers MUST:
+ * 1. Submit `Revive.map_account` on-chain (gated by `created === true`).
+ * 2. Call `persistSessionAccount(info)` ONLY AFTER the extrinsic is confirmed.
+ *
+ * Keeping persist separate from create prevents the file from recording a key
+ * whose on-chain mapping was never established.  If `map_account` fails, the
+ * file is untouched so the next retry mints a fresh key and re-attempts mapping.
  */
 export async function getOrCreateSessionAccount(): Promise<{
     info: SessionKeyInfo;
     created: boolean;
 }> {
-    const store = new FileKvStore(accountsPath());
-    const manager = new SessionKeyManager({ store });
-    const existing = await manager.get();
+    const fileStore = new FileKvStore(accountsPath());
+    const fileManager = new SessionKeyManager({ store: fileStore });
+    const existing = await fileManager.get();
     if (existing) return { info: existing, created: false };
-    return { info: await manager.create(), created: true };
+
+    // Mint in memory — do NOT write to disk yet. The caller must call
+    // `persistSessionAccount` after `map_account` succeeds.
+    const memManager = new SessionKeyManager({ store: new InMemoryKvStore() });
+    return { info: await memManager.create(), created: true };
+}
+
+/**
+ * Write a session key to disk.  Call this ONLY after the on-chain
+ * `Revive.map_account` extrinsic for `info` has been confirmed.
+ */
+export async function persistSessionAccount(info: SessionKeyInfo): Promise<void> {
+    const store = new FileKvStore(accountsPath());
+    await store.set("default", info.mnemonic);
 }