diff --git a/src/__tests__/reviewSignersCardKey.test.ts b/src/__tests__/reviewSignersCardKey.test.ts new file mode 100644 index 00000000..c55c5f9c --- /dev/null +++ b/src/__tests__/reviewSignersCardKey.test.ts @@ -0,0 +1,289 @@ +/** + * Regression test for Finding 1.1 (rocksolid/harden-pr-233): + * React key collision when multiple empty signer rows exist. + * + * The fix introduced a parallel `signerIds: string[]` array in + * useWalletFlowState / useMigrationWalletFlowState that is used as the + * React `key` on each signer row, instead of the (possibly empty, + * possibly duplicated) address string. + * + * This test pins the invariant on two layers: + * 1. The data-shape invariants of the React-key fix — exercised here + * by inline simulation of the hook's add/remove array logic. We + * cannot drive the hook itself in a node-environment jest run + * (the hook depends on next/router, zustand, tRPC, and toast + * providers), so this layer pins the *shape* the hook is + * contracted to maintain: synthetic ids stay distinct across + * empty-row collisions, and removal preserves index alignment + * across all five parallel arrays. + * 2. A source-level tripwire (separate describe block below) that + * catches anyone reverting the JSX back to address-as-key, or + * removing the parallel signerIds array from either hook. The + * React-key behavior itself — that the JSX consumes signerIds + * and that the hook exposes it — is pinned by that tripwire. + * + * Why no `renderHook`: `useWalletFlowState` depends on next/router, + * zustand, tRPC, and toast providers, none of which exist in jest's + * `node` test environment. Adding `@testing-library/react` + jsdom + * would be substantial scaffolding and out of scope for this change. + */ + +import fs from "fs"; +import path from "path"; +import { fileURLToPath } from "url"; +import { makeSignerId } from "@/components/pages/homepage/wallets/new-wallet-flow/shared/signerRows"; + +// ESM equivalent of CJS __dirname. Tests run under +// node --experimental-vm-modules so CJS globals are unavailable. +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +// Mirror the splice-based array logic that addSigner / removeSigner +// implement inside the two flow-state hooks. We cannot import the hooks +// directly because they pull in next/router, zustand, tRPC, and toast +// providers — none of which exist in jest's node test environment. So +// we replicate the data flow at the same level of abstraction the hook +// uses, and pin the invariant: after add/remove cycles, each parallel +// array stays index-aligned with the others. +type SignerRow = { + address: string; + description: string; + stakeKey: string; + drepKey: string; + id: string; +}; + +function applyAdd(rows: SignerRow[], newId: string): SignerRow[] { + // Mirrors addSigner: pushes empty fields with a fresh synthetic id. + return [ + ...rows, + { address: "", description: "", stakeKey: "", drepKey: "", id: newId }, + ]; +} + +function applyRemove(rows: SignerRow[], index: number): SignerRow[] { + // Mirrors removeSigner: splices the same index out of every parallel + // array. With address-as-key, two empty rows would collide on key="" + // and React could splice the wrong one — synthetic ids prevent that. + const next = rows.slice(); + next.splice(index, 1); + return next; +} + +describe("ReviewSignersCard signer-row key invariant", () => { + test("removing the middle of three signers keeps remaining rows aligned", () => { + const initial: SignerRow[] = [ + { + address: "addr1qx_creator", + description: "Alice", + stakeKey: "stake1_alice", + drepKey: "drep_alice", + id: "id-creator", + }, + ]; + + // User clicks "Add Signer" twice. Both new rows have address "" — + // the bug case. Synthetic ids must still differ. + const afterAdd1 = applyAdd(initial, "id-bob"); + const afterAdd2 = applyAdd(afterAdd1, "id-carol"); + + expect(afterAdd2).toHaveLength(3); + // Two empty addresses, but ids are distinct. + expect(afterAdd2[1]!.address).toBe(""); + expect(afterAdd2[2]!.address).toBe(""); + expect(afterAdd2[1]!.id).not.toBe(afterAdd2[2]!.id); + + // Fill them in so we can tell which row is which. + afterAdd2[1] = { ...afterAdd2[1]!, address: "addr1_bob", description: "Bob", stakeKey: "stake1_bob" }; + afterAdd2[2] = { ...afterAdd2[2]!, address: "addr1_carol", description: "Carol", stakeKey: "stake1_carol" }; + + // Remove Bob (index 1). + const afterRemove = applyRemove(afterAdd2, 1); + + expect(afterRemove).toHaveLength(2); + // Alice still at index 0, Carol now at index 1 — descriptions and + // stake keys must follow the same row, NOT slip out of alignment. + expect(afterRemove[0]!.description).toBe("Alice"); + expect(afterRemove[0]!.stakeKey).toBe("stake1_alice"); + expect(afterRemove[1]!.description).toBe("Carol"); + expect(afterRemove[1]!.stakeKey).toBe("stake1_carol"); + expect(afterRemove[1]!.address).toBe("addr1_carol"); + }); + + test("two consecutive Add Signer clicks produce distinct synthetic ids", () => { + // The exact bug case: with key={signer} (address) two empty rows + // would collide. Synthetic ids must differ regardless of address. + let rows: SignerRow[] = []; + rows = applyAdd(rows, "id-1"); + rows = applyAdd(rows, "id-2"); + + const ids = rows.map((r) => r.id); + expect(new Set(ids).size).toBe(rows.length); + }); + + test( + "addSigner-then-removeSigner: surviving row keeps its synthetic id, " + + "data-shape invariant pinned via inline simulation", + () => { + // Mirrors the array-manipulation contract the hook's `addSigner` + // / `removeSigner` setters apply, inline. `makeSignerId` runs + // unmocked so we exercise real id minting (crypto.randomUUID + // under Node >= 14.17, fallback otherwise). + // + // Scenario: start with one creator-seeded row (the first-user + // effect), append two empty rows, capture the id of the third + // row (the survivor), then splice index 1 — a mid-array empty + // remove. Assert that ids[1] is the survivor's *original* id + // (not a re-issued one) and that every parallel array stays + // index-aligned with ids. + + type Arrays = { + addresses: string[]; + descriptions: string[]; + stakeKeys: string[]; + drepKeys: string[]; + ids: string[]; + }; + + // Seed three rows. Index 0 is the creator (Alice). Index 1 is an + // empty row (Bob, never filled in) — preserves the empty-row + // collision scenario this test was originally written for. Index + // 2 (Carol, the survivor) carries distinct, non-empty values in + // every parallel array. After removing index 1, index 1 must + // hold Carol's distinct values; an off-by-one that mis-spliced + // any single non-id array would leave "" in that array's slot + // and fail the assertion. + let state: Arrays = { + addresses: ["addr1_creator", "", "addr1_carol"], + descriptions: ["Alice", "", "Carol"], + stakeKeys: ["stake1_creator", "", "stake1_carol"], + drepKeys: ["", "", "drep1_carol"], + ids: [makeSignerId(), makeSignerId(), makeSignerId()], + }; + + expect(state.ids).toHaveLength(3); + // Three distinct synthetic ids — the bug case (two empty rows + // sharing key="") cannot reproduce when ids are minted per-row. + expect(new Set(state.ids).size).toBe(3); + + // Survivor: the row currently at index 2. Its id must follow the + // row, not the index, after we remove index 1. + const survivorOriginalId = state.ids[2]!; + + // Splice index 1 out of every parallel array, mirroring + // removeSigner. + const spliceOut = (arr: T[], i: number): T[] => { + const next = arr.slice(); + next.splice(i, 1); + return next; + }; + state = { + addresses: spliceOut(state.addresses, 1), + descriptions: spliceOut(state.descriptions, 1), + stakeKeys: spliceOut(state.stakeKeys, 1), + drepKeys: spliceOut(state.drepKeys, 1), + ids: spliceOut(state.ids, 1), + }; + + expect(state.ids).toHaveLength(2); + // The id now at index 1 must be the survivor's original id — + // proving identity follows the row, not the position. If the hook + // re-minted ids on remove (the broken pattern), this would fail. + expect(state.ids[1]).toBe(survivorOriginalId); + // Every parallel array stays index-aligned with ids AND the + // survivor's distinct values land at index 1. An off-by-one that + // spliced index 2 instead of index 1 would leave "" here in any + // single array — making the splice direction directly testable. + expect(state.addresses[1]).toBe("addr1_carol"); + expect(state.descriptions[1]).toBe("Carol"); + expect(state.stakeKeys[1]).toBe("stake1_carol"); + expect(state.drepKeys[1]).toBe("drep1_carol"); + // Creator row at index 0 untouched. + expect(state.ids[0]).not.toBe(survivorOriginalId); + expect(state.addresses[0]).toBe("addr1_creator"); + expect(state.descriptions[0]).toBe("Alice"); + expect(state.stakeKeys[0]).toBe("stake1_creator"); + }, + ); +}); + +describe("ReviewSignersCard tripwire on source", () => { + // Pin the source-level fix: anyone reverting back to address-as-key + // (or removing the parallel signerIds) will see this test fail. + const SOURCE_PATH = path.resolve( + __dirname, + "../components/pages/homepage/wallets/new-wallet-flow/create/ReviewSignersCard.tsx", + ); + const HOOK_PATH = path.resolve( + __dirname, + "../components/pages/homepage/wallets/new-wallet-flow/shared/useWalletFlowState.tsx", + ); + const MIGRATION_HOOK_PATH = path.resolve( + __dirname, + "../components/pages/wallet/info/migration/useMigrationWalletFlowState.tsx", + ); + + test("ReviewSignersCard never uses the raw address as a React key", () => { + const src = fs.readFileSync(SOURCE_PATH, "utf8"); + + // ---- Negative tripwire ---- + // + // Nothing within `key={ ... }` may start with the bare identifier + // `signer` (the per-iteration address). The boundary class + // `(\s|\}|[^a-zA-Z_0-9])` after `signer` rejects the entire + // address-as-key family: + // - `key={signer}` (the original bug) + // - `key={ signer }` (whitespace variant) + // - `key={signer ?? ""}` (nullish-coalescing fallback) + // - `key={signer.address}` (member-access — different revert) + // - `key={String(signer)}` ('(' is non-alphanumeric) + // + // It deliberately does NOT trip on the synthetic forms + // `key={signerIds[index]}` or `key={signerIds[index] ?? "..."}` + // because the `I` in `Ids` is a-zA-Z and falls outside the + // boundary class — `signer` followed by a word char is fine. + expect(src).not.toMatch(/key=\{\s*signer(\s|\}|[^a-zA-Z_0-9])/); + + // ---- Positive tripwire ---- + // + // Within 200 chars of an opening ` + // without breaking this test (the synthetic-id behavior survives). + // It still fails if anyone reverts to the bare address or to a + // bare index-as-key. + expect(src).toMatch( + //g) ?? []; + const mobileCardBlock = divMobileBlocks.find( + (block) => + /rounded-lg border/.test(block) && + /key=\{[\s\S]{0,80}(signerIds|rowKey\()/.test(block), + ); + expect(mobileCardBlock).toBeDefined(); + }); + + test("useWalletFlowState exposes signerIds parallel to signersAddresses", () => { + const src = fs.readFileSync(HOOK_PATH, "utf8"); + expect(src).toMatch(/signerIds/); + expect(src).toMatch(/setSignerIds/); + }); + + test("useMigrationWalletFlowState exposes signerIds parallel to signersAddresses", () => { + const src = fs.readFileSync(MIGRATION_HOOK_PATH, "utf8"); + expect(src).toMatch(/signerIds/); + expect(src).toMatch(/setSignerIds/); + }); +}); diff --git a/src/components/pages/homepage/wallets/new-wallet-flow/create/ReviewSignersCard.tsx b/src/components/pages/homepage/wallets/new-wallet-flow/create/ReviewSignersCard.tsx index 9232b688..e96c685d 100644 --- a/src/components/pages/homepage/wallets/new-wallet-flow/create/ReviewSignersCard.tsx +++ b/src/components/pages/homepage/wallets/new-wallet-flow/create/ReviewSignersCard.tsx @@ -52,10 +52,20 @@ interface SignerConfig { setSignerStakeKeys: React.Dispatch>; signersDRepKeys: string[]; setSignerDRepKeys: React.Dispatch>; + signerIds?: string[]; addSigner: () => void; removeSigner?: (index: number) => void; } +// Synthetic React-key helper. The on-chain address is unique per signer in the +// happy path, but during edits or paste flows it can briefly collide; using +// the address as a key triggers React reconciliation bugs (lost focus, +// duplicated DOM nodes). `signerIds` is kept index-aligned by the host hook +// (`useWalletFlowState` / `useMigrationWalletFlowState`); this fallback runs +// only when a caller hasn't wired it through yet. +const rowKey = (signerIds: string[] | undefined, index: number): string => + signerIds?.[index] ?? `signer-row-${index}`; + interface ReviewSignersCardProps { signerConfig: SignerConfig; currentUserAddress?: string; @@ -85,6 +95,7 @@ const ReviewSignersCard: React.FC = ({ setSignerStakeKeys, signersDRepKeys = [], setSignerDRepKeys, + signerIds, addSigner, removeSigner, } = signerConfig; @@ -263,7 +274,7 @@ const ReviewSignersCard: React.FC = ({ {signersAddresses.map((signer, index) => ( - + {/* Signer name */} @@ -374,7 +385,7 @@ const ReviewSignersCard: React.FC = ({
{signersAddresses.map((signer, index) => (
{/* Top row: Name and Actions */} diff --git a/src/components/pages/homepage/wallets/new-wallet-flow/create/index.tsx b/src/components/pages/homepage/wallets/new-wallet-flow/create/index.tsx index 68b0e44d..29a73531 100644 --- a/src/components/pages/homepage/wallets/new-wallet-flow/create/index.tsx +++ b/src/components/pages/homepage/wallets/new-wallet-flow/create/index.tsx @@ -37,6 +37,7 @@ export default function PageReviewWallet() { setSignerStakeKeys: walletFlow.setSignerStakeKeys, signersDRepKeys: walletFlow.signersDRepKeys, setSignerDRepKeys: walletFlow.setSignerDRepKeys, + signerIds: walletFlow.signerIds, addSigner: walletFlow.addSigner, removeSigner: walletFlow.removeSigner, }} diff --git a/src/components/pages/homepage/wallets/new-wallet-flow/shared/signerRows.ts b/src/components/pages/homepage/wallets/new-wallet-flow/shared/signerRows.ts new file mode 100644 index 00000000..e908067f --- /dev/null +++ b/src/components/pages/homepage/wallets/new-wallet-flow/shared/signerRows.ts @@ -0,0 +1,23 @@ +/** + * Shared synthetic-id minter for the parallel signer arrays that + * `useWalletFlowState` and `useMigrationWalletFlowState` maintain + * (signersAddresses / signersDescriptions / signersStakeKeys / + * signersDRepKeys / signerIds). + * + * Lives in a standalone module — with no React, next/router, zustand, + * tRPC, or toast imports — so it can be unit-tested without rendering + * either hook. Both hooks import `makeSignerId` directly when + * appending a new row. + */ + +/** + * Generate a stable synthetic id for a signer row. `crypto.randomUUID` + * is available in modern browsers and Node >= 14.17; fall back to a + * timestamp+random string in environments where it isn't. + */ +export function makeSignerId(): string { + if (typeof crypto !== "undefined" && typeof crypto.randomUUID === "function") { + return crypto.randomUUID(); + } + return `signer-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`; +} diff --git a/src/components/pages/homepage/wallets/new-wallet-flow/shared/useWalletFlowState.tsx b/src/components/pages/homepage/wallets/new-wallet-flow/shared/useWalletFlowState.tsx index 3759303d..b52dc5f3 100644 --- a/src/components/pages/homepage/wallets/new-wallet-flow/shared/useWalletFlowState.tsx +++ b/src/components/pages/homepage/wallets/new-wallet-flow/shared/useWalletFlowState.tsx @@ -16,6 +16,7 @@ import { useUserStore } from "@/lib/zustand/user"; import { useSiteStore } from "@/lib/zustand/site"; import { useToast } from "@/hooks/use-toast"; import useUser from "@/hooks/useUser"; +import { makeSignerId } from "./signerRows"; export interface WalletFlowState { // Core wallet data @@ -35,9 +36,23 @@ export interface WalletFlowState { setSignerStakeKeys: React.Dispatch>; signersDRepKeys: string[]; setSignerDRepKeys: React.Dispatch>; - addSigner: () => void; + // Stable synthetic identifiers for each signer row, parallel to + // signersAddresses. Used as React `key` so duplicate / empty addresses + // do not collide and stomp form state on re-render. Never sent to the + // backend — purely local UI identity. + signerIds: string[]; + // Optional positional params let callers (e.g. ReviewSignersCard's + // "Add signer" dialog) push a fully-populated row through the hook so + // signerIds stays index-aligned with signersAddresses. Called with no + // args, it appends an empty row (legacy behavior). + addSigner: ( + address?: string, + stakeKey?: string, + drepKey?: string, + description?: string, + ) => void; removeSigner: (index: number) => void; - + // Signature rules numRequiredSigners: number; setNumRequiredSigners: React.Dispatch>; @@ -99,12 +114,14 @@ export interface WalletFlowState { handleSaveAdvanced: (newStakeKey: string, scriptType: "all" | "any" | "atLeast") => void; } + export function useWalletFlowState(): WalletFlowState { const router = useRouter(); const [signersAddresses, setSignerAddresses] = useState([]); const [signersDescriptions, setSignerDescriptions] = useState([]); const [signersStakeKeys, setSignerStakeKeys] = useState([]); const [signersDRepKeys, setSignerDRepKeys] = useState([]); + const [signerIds, setSignerIds] = useState([]); const [numRequiredSigners, setNumRequiredSigners] = useState(1); const [name, setName] = useState(""); const [description, setDescription] = useState(""); @@ -303,6 +320,7 @@ export function useWalletFlowState(): WalletFlowState { setSignerStakeKeys([stakeKey ? "" : user.stakeAddress]); // Import DRep key from user data setSignerDRepKeys([(user as any).drepKeyHash || ""]); + setSignerIds([makeSignerId()]); } }, [user, stakeKey, pathIsWalletInvite]); @@ -363,6 +381,20 @@ export function useWalletFlowState(): WalletFlowState { : []; setSignerStakeKeys(incomingStakeKeys); setSignerDRepKeys((walletInvite as any).signersDRepKeys ?? []); + // Keep synthetic ids stable across refetches: only mint new ids + // for rows that grow the array, trim if it shrinks. Re-minting on + // every refetch would unmount in-flight description inputs and + // drop focus / IME state. Mirrors handleSaveSigners' pattern. + setSignerIds((prev) => { + const incoming = walletInvite.signersAddresses ?? []; + if (prev.length === incoming.length) return prev; + if (prev.length < incoming.length) { + const next = prev.slice(); + while (next.length < incoming.length) next.push(makeSignerId()); + return next; + } + return prev.slice(0, incoming.length); + }); setNumRequiredSigners(walletInvite.numRequiredSigners!); setStakeKey((walletInvite as any).stakeCredentialHash ?? ""); setNativeScriptType((walletInvite as any).scriptType ?? "atLeast"); @@ -371,30 +403,58 @@ export function useWalletFlowState(): WalletFlowState { // Utility functions - function addSigner() { - setSignerAddresses([...signersAddresses, ""]); - setSignerDescriptions([...signersDescriptions, ""]); - // Always add empty stake key when external stake credential is set, otherwise add empty string - setSignerStakeKeys([...signersStakeKeys, stakeKey ? "" : ""]); - setSignerDRepKeys([...signersDRepKeys, ""]); + function addSigner( + address: string = "", + newStakeKey: string = "", + drepKey: string = "", + description: string = "", + ) { + // Functional updaters so two synchronous addSigner() calls in the + // same render append two rows. With closure-reading non-functional + // setters, both calls would read the same `signersAddresses` and + // collapse into a single appended row, leaving signerIds.length + // out of sync with the other arrays. + const stakeKeyForRow = stakeKey ? "" : newStakeKey; + setSignerAddresses((arr) => [...arr, address]); + setSignerDescriptions((arr) => [...arr, description]); + // Always blank the per-signer stake key when an external stake + // credential is set, regardless of what was passed in. + setSignerStakeKeys((arr) => [...arr, stakeKeyForRow]); + setSignerDRepKeys((arr) => [...arr, drepKey]); + setSignerIds((arr) => [...arr, makeSignerId()]); } function removeSigner(index: number) { - const updatedAddresses = [...signersAddresses]; - updatedAddresses.splice(index, 1); - setSignerAddresses(updatedAddresses); - - const updatedDescriptions = [...signersDescriptions]; - updatedDescriptions.splice(index, 1); - setSignerDescriptions(updatedDescriptions); - - const updatedStakeKeys = [...signersStakeKeys]; - updatedStakeKeys.splice(index, 1); - setSignerStakeKeys(updatedStakeKeys); - - const updatedDRepKeys = [...signersDRepKeys]; - updatedDRepKeys.splice(index, 1); - setSignerDRepKeys(updatedDRepKeys); + // Functional updaters mirror the Pass 3 fix in addSigner: two + // synchronous removeSigner() calls in the same render must each + // see the previous batch's update, otherwise the second call would + // splice from a stale closure-read array and the five parallel + // arrays could drift out of alignment. + setSignerAddresses((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerDescriptions((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerStakeKeys((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerDRepKeys((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerIds((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); } function createNativeScript() { @@ -666,9 +726,10 @@ export function useWalletFlowState(): WalletFlowState { setSignerStakeKeys, signersDRepKeys, setSignerDRepKeys, + signerIds, addSigner, removeSigner, - + // Signature rules numRequiredSigners, setNumRequiredSigners, diff --git a/src/components/pages/wallet/info/migration/NewWalletCreationStep.tsx b/src/components/pages/wallet/info/migration/NewWalletCreationStep.tsx index 6c8c6aae..b9a2aa99 100644 --- a/src/components/pages/wallet/info/migration/NewWalletCreationStep.tsx +++ b/src/components/pages/wallet/info/migration/NewWalletCreationStep.tsx @@ -200,6 +200,7 @@ export default function NewWalletCreationStep({ setSignerStakeKeys: walletFlow.setSignerStakeKeys, signersDRepKeys: walletFlow.signersDRepKeys, setSignerDRepKeys: walletFlow.setSignerDRepKeys, + signerIds: walletFlow.signerIds, addSigner: walletFlow.addSigner, removeSigner: walletFlow.removeSigner, }} diff --git a/src/components/pages/wallet/info/migration/useMigrationWalletFlowState.tsx b/src/components/pages/wallet/info/migration/useMigrationWalletFlowState.tsx index 5a796a00..2683683e 100644 --- a/src/components/pages/wallet/info/migration/useMigrationWalletFlowState.tsx +++ b/src/components/pages/wallet/info/migration/useMigrationWalletFlowState.tsx @@ -15,6 +15,7 @@ import { useUserStore } from "@/lib/zustand/user"; import { useSiteStore } from "@/lib/zustand/site"; import { useToast } from "@/hooks/use-toast"; import type { Wallet } from "@/types/wallet"; +import { makeSignerId } from "@/components/pages/homepage/wallets/new-wallet-flow/shared/signerRows"; const apiClient = api; @@ -34,9 +35,23 @@ export interface MigrationWalletFlowState { setSignerStakeKeys: React.Dispatch>; signersDRepKeys: string[]; setSignerDRepKeys: React.Dispatch>; - addSigner: () => void; + // Stable synthetic identifiers for each signer row, parallel to + // signersAddresses. Used as React `key` so duplicate / empty addresses + // do not collide and stomp form state on re-render. Never sent to the + // backend — purely local UI identity. + signerIds: string[]; + // Optional positional params let callers (e.g. ReviewSignersCard's + // "Add signer" dialog) push a fully-populated row through the hook so + // signerIds stays index-aligned with signersAddresses. Called with no + // args, it appends an empty row (legacy behavior). + addSigner: ( + address?: string, + stakeKey?: string, + drepKey?: string, + description?: string, + ) => void; removeSigner: (index: number) => void; - + // Signature rules numRequiredSigners: number; setNumRequiredSigners: React.Dispatch>; @@ -81,6 +96,7 @@ export function useMigrationWalletFlowState(appWallet: Wallet, migrationId?: str const [signersDescriptions, setSignerDescriptions] = useState([]); const [signersStakeKeys, setSignerStakeKeys] = useState([]); const [signersDRepKeys, setSignerDRepKeys] = useState([]); + const [signerIds, setSignerIds] = useState([]); const [numRequiredSigners, setNumRequiredSigners] = useState(1); const [name, setName] = useState(""); const [description, setDescription] = useState(""); @@ -147,6 +163,18 @@ export function useMigrationWalletFlowState(appWallet: Wallet, migrationId?: str setName(`${walletData.name} - Migration in Progress`); setDescription(walletData.description ?? ""); setSignerAddresses(walletData.signersAddresses ?? []); + // Stable across refetches — only grow / shrink, never re-mint, so + // an in-flight description input does not lose focus / IME state. + setSignerIds((prev) => { + const incoming = walletData.signersAddresses ?? []; + if (prev.length === incoming.length) return prev; + if (prev.length < incoming.length) { + const next = prev.slice(); + while (next.length < incoming.length) next.push(makeSignerId()); + return next; + } + return prev.slice(0, incoming.length); + }); setSignerDescriptions(walletData.signersDescriptions ?? []); setNumRequiredSigners(walletData.numRequiredSigners ?? 1); setNativeScriptType((walletData.type as "atLeast" | "all" | "any") ?? "atLeast"); @@ -183,6 +211,18 @@ export function useMigrationWalletFlowState(appWallet: Wallet, migrationId?: str setName(existingNewWallet.name); setDescription(existingNewWallet.description ?? ""); setSignerAddresses(existingNewWallet.signersAddresses ?? []); + // Stable across refetches — only grow / shrink, never re-mint, so + // an in-flight description input does not lose focus / IME state. + setSignerIds((prev) => { + const incoming = existingNewWallet.signersAddresses ?? []; + if (prev.length === incoming.length) return prev; + if (prev.length < incoming.length) { + const next = prev.slice(); + while (next.length < incoming.length) next.push(makeSignerId()); + return next; + } + return prev.slice(0, incoming.length); + }); setSignerDescriptions(existingNewWallet.signersDescriptions ?? []); setSignerStakeKeys(existingNewWallet.signersStakeKeys ?? []); setSignerDRepKeys( @@ -354,29 +394,55 @@ export function useMigrationWalletFlowState(appWallet: Wallet, migrationId?: str }); // Utility functions - function addSigner() { - setSignerAddresses([...signersAddresses, ""]); - setSignerDescriptions([...signersDescriptions, ""]); - setSignerStakeKeys([...signersStakeKeys, ""]); - setSignerDRepKeys([...signersDRepKeys, ""]); + function addSigner( + address: string = "", + newStakeKey: string = "", + drepKey: string = "", + description: string = "", + ) { + // Functional updaters so two synchronous addSigner() calls in the + // same render append two rows. With closure-reading non-functional + // setters, both calls would read the same `signersAddresses` and + // collapse into a single appended row, leaving signerIds.length + // out of sync with the other arrays. + setSignerAddresses((arr) => [...arr, address]); + setSignerDescriptions((arr) => [...arr, description]); + setSignerStakeKeys((arr) => [...arr, newStakeKey]); + setSignerDRepKeys((arr) => [...arr, drepKey]); + setSignerIds((arr) => [...arr, makeSignerId()]); } function removeSigner(index: number) { - const updatedAddresses = [...signersAddresses]; - updatedAddresses.splice(index, 1); - setSignerAddresses(updatedAddresses); - - const updatedDescriptions = [...signersDescriptions]; - updatedDescriptions.splice(index, 1); - setSignerDescriptions(updatedDescriptions); - - const updatedStakeKeys = [...signersStakeKeys]; - updatedStakeKeys.splice(index, 1); - setSignerStakeKeys(updatedStakeKeys); - - const updatedDRepKeys = [...signersDRepKeys]; - updatedDRepKeys.splice(index, 1); - setSignerDRepKeys(updatedDRepKeys); + // Functional updaters mirror the Pass 3 fix in addSigner: two + // synchronous removeSigner() calls in the same render must each + // see the previous batch's update, otherwise the second call would + // splice from a stale closure-read array and the five parallel + // arrays could drift out of alignment. + setSignerAddresses((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerDescriptions((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerStakeKeys((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerDRepKeys((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); + setSignerIds((arr) => { + const next = arr.slice(); + next.splice(index, 1); + return next; + }); } // Adjust numRequiredSigners if it exceeds the number of signers @@ -629,6 +695,13 @@ export function useMigrationWalletFlowState(appWallet: Wallet, migrationId?: str setSignerDescriptions(paddedDescriptions); setSignerStakeKeys(paddedStakeKeys); setSignerDRepKeys(paddedDRepKeys); + // Keep synthetic ids parallel to the address array. Reuse existing ids + // for the rows that survive, mint fresh ones for any newly-added rows. + setSignerIds((prevIds) => { + const next = prevIds.slice(0, paddedAddresses.length); + while (next.length < paddedAddresses.length) next.push(makeSignerId()); + return next; + }); if (newWalletId) { const updateData = { @@ -778,9 +851,10 @@ export function useMigrationWalletFlowState(appWallet: Wallet, migrationId?: str setSignerStakeKeys, signersDRepKeys, setSignerDRepKeys, + signerIds, addSigner, removeSigner, - + // Signature rules numRequiredSigners, setNumRequiredSigners,