feat: hot key, cold key and guardian key by 0xnullifier · Pull Request #227 · 0xMiden/wallet

0xnullifier · 2026-05-01T17:58:37Z

No description provided.

WiktorStarczewski

Approving — direction is right, the secure-hot-key facade is a clean abstraction and the test coverage on the happy path is solid.

Leaving inline recommendations rather than blockers, but flagging three I'd like addressed before this rolls to users:

importGuardianAccountBySeed returns success while producing a broken-recovery account until Phase 8 lands. The docstring acknowledges it; the function surface doesn't. Throwing — or surfacing a clear UI banner — is safer than letting the user think their import worked.
No legacy-Guardian detection. Pre-cutover Guardian records decode fine but fail at sign time. Detecting type === Guardian && hotPublicKey == null at vault load and surfacing a re-creation prompt would catch the failure earlier than the next send.
Mobile crashes on Guardian creation because nativePlugin.generateHotKey() throws. Gate the Guardian flow behind !isMobile() until Phase 4 lands, or have nativePlugin defer to jsFallback as a soft-degradation.

The rest are mechanical cleanups (typo, type-fencing, missing tests for signHotDigest/nativePlugin, signer-commitment ordering invariant). Empty PR body is also worth filling in given this is a phased migration with a hard cutover.

WiktorStarczewski · 2026-05-04T17:37:57Z

  MigrationLevel = 'migration',
  Mnemonic = 'mnemonic',
  AccAuthSecretKey = 'accauthsecretkey',
+  AccColdSecretKey = 'accouldsecretkey',


Typo: 'accouldsecretkey' — should be 'accoldsecretkey'. Not user-visible, but this becomes the on-disk storage prefix and gets baked into every cold-key record forever once accounts start writing. Easier to rename now (no live data yet) than after launch.

WiktorStarczewski · 2026-05-04T17:37:57Z

+    [
+      [accAuthPubKeyStrgKey(keys.hotPublicKey), keys.hotPublicKey],
+      [accAuthSecretKeyStrgKey(keys.hotPublicKey), keys.hotCiphertext],
+      [accColdSecretKeyStrgKey(keys.coldPublicKey), keys.coldSecretKeyHex]


Asymmetric with the hot path: hot writes both accAuthPubKeyStrgKey(hotPublicKey) and the secret-key entry, but cold only writes accColdSecretKeyStrgKey(coldPublicKey) — no accAuthPubKeyStrgKey(coldPublicKey). If anything in the codebase enumerates "all known auth pubkeys" via the accAuthPubKey namespace, cold is invisible to it. Probably intentional (cold is for rotation, not daily signing) but worth a one-line comment so the next reader doesn't think it's a bug.

WiktorStarczewski · 2026-05-04T17:37:57Z

+      // createGuardianAccount so the vault can persist them. Callers must use
+      // createGuardianMidenWallet directly — this branch protects against the
+      // older string-returning API being relied on.
+      throw new Error('Guardian wallets must be created via createGuardianMidenWallet');


Runtime throw rather than type-fence. The signature still accepts WalletType.Guardian, so accidental calls only blow up at runtime. Better:

async createMidenWallet( walletType: Exclude<WalletType, WalletType.Guardian>, seed?: Uint8Array, ): Promise<string>

so tsc catches the misuse. Same applies to importAccountBySeed below.

WiktorStarczewski · 2026-05-04T17:37:57Z

-        signerCommitments: [signerCommitment.toHex()],
-        guardianCommitment: commitment,
-        guardianPublicKey: pubkey,
+        signerCommitments: [hot.commitmentHex, coldCommitmentHex],


Comment above says "order is load-bearing for downstream role routing" — but only the test asserts it. A future contributor sorting the array (for deterministic hashing, dedup, etc.) would silently break role routing. Recommend codifying the invariant:

const SIGNER_ROLES = ['hot', 'cold'] as const; // ... signerCommitments: [hot.commitmentHex, coldCommitmentHex] satisfies readonly string[]

or a runtime assertion at signer-key persistence time that maps commitment → role and traps mismatches.

WiktorStarczewski · 2026-05-04T17:37:57Z

  private signWordFn: (wordHex: string) => Promise<string>;

-  constructor(publicKey: string, commitment: string, signWordFn: SignWordFunction) {
+  constructor(publicKey: string, commitment: string, signWordFn: SignWordFunction, scheme: SignatureScheme = 'ecdsa') {


Default flipped from hardcoded 'falcon' to parameter default 'ecdsa'. Intentional in the cutover, but every existing 3-arg new WalletSigner(pk, commit, signFn) call site now silently becomes ECDSA. Worth grepping the base branch (utk-psm-integration) for all WalletSigner constructions to confirm the cutover is what's wanted at each one — silent default flips are exactly where unintended behavior changes hide.

WiktorStarczewski · 2026-05-04T17:37:57Z

+  return { ciphertext, publicKeyHex, commitmentHex };
+}
+
+export async function signHotDigest(ciphertext: string, wordHex: string): Promise<string> {


signHotDigest and deleteHotKey have zero test coverage in this PR. They're not invoked yet (Phase 3 wires them up), but the ciphertext format is the on-disk blob shape — high-stakes to keep stable across phases. A round-trip test like:

const hot = await generateHotKey(); const sig = await signHotDigest(hot.ciphertext, '0x' + '00'.repeat(32)); expect(sig).toMatch(/^0x[0-9a-f]+$/);

would catch a future refactor that changes the ciphertext encoding and silently breaks loading of existing hot keys.

WiktorStarczewski · 2026-05-04T17:37:58Z

+  // Set on Guardian accounts created with the 3-key model (hot + cold + guardian).
+  // Absent on non-Guardian accounts and on legacy single-signer Guardian records
+  // produced before the migration; consumers should treat absence as "not 3-key".
+  hotPublicKey?: string;


Optional fields are non-breaking for non-Guardian accounts (good) but they're also non-breaking for legacy Guardian accounts created on the pre-cutover Falcon build — those records decode fine, then fail at sign time because the on-chain account uses Falcon and the wallet now produces ECDSA sigs.

Recommend a load-time check: type === Guardian && hotPublicKey == null ⇒ flag the account as "requires re-creation" and surface a banner. Catching the breakage at vault load is much better UX than the user discovering it when their next send fails.

…vice dev tooling

…JSCore

….14.5

…leanup

…var in generateGuardianTransaction

…ee rotated state

… accounts

…izeGuardianSwitch

WiktorStarczewski approved these changes May 4, 2026

View reviewed changes

0xnullifier marked this pull request as draft May 5, 2026 12:24

0xnullifier added 10 commits May 10, 2026 14:46

feat(guardian): switch default auth scheme to ECDSA

f052d41

feat(guardian): 3-key (hot+cold+guardian) account creation

ec0a4cd

feat(guardian): split signWord into cold/hot paths keyed by hotPublicKey

18cf412

feat(ios): SE-backed HotKey plugin with recoverable secp256k1 + on-de…

7ad8dbc

…vice dev tooling

fix(mobile): drop inlineDynamicImports to avoid TLA wrapper crash in …

ab9fee5

…JSCore

feat(android): StrongBox-backed HotKey plugin with recoverable secp256k1

973bd74

feat(guardian): replace-hot-key flow + cold co-signs switch_guardian

375be34

fix(guardian): refresh sync state and cache after hot-key rotation

49ca879

chore(deps): bump @openzeppelin/{guardian,miden-multisig}-client to 0…

d9048a7

….14.5

feat(guardian): switch-guardian cold-signing confirmation + Phase 7 c…

cee363e

…leanup

0xnullifier force-pushed the utk-3keys branch from 44aaded to cee363e Compare May 11, 2026 10:07

0xnullifier added 6 commits May 12, 2026 15:42

feat(guardian): defer post-recovery hot-key activation to a banner

731dc33

refactor(guardian): collapse multisigService/signingService into one …

42d6094

…var in generateGuardianTransaction

fix(guardian): fire accountsUpdated on SW-side swapHotKey so popups s…

c650267

…ee rotated state

feat(guardian): reveal hot key + reshape reveal private key for 3-key…

f4b2f2f

… accounts

fix(guardian): pass 'ecdsa' scheme to nextGuardian.getPubkey in final…

2129763

…izeGuardianSwitch

feat: bump up package

44a369d

0xnullifier mentioned this pull request May 31, 2026

bridge integration #262

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: hot key, cold key and guardian key#227

feat: hot key, cold key and guardian key#227
0xnullifier wants to merge 16 commits into
utk-psm-integrationfrom
utk-3keys

0xnullifier commented May 1, 2026

Uh oh!

WiktorStarczewski left a comment

Uh oh!

Uh oh!

WiktorStarczewski May 4, 2026

Uh oh!

WiktorStarczewski May 4, 2026

Uh oh!

WiktorStarczewski May 4, 2026

Uh oh!

Uh oh!

WiktorStarczewski May 4, 2026

Uh oh!

WiktorStarczewski May 4, 2026

Uh oh!

WiktorStarczewski May 4, 2026

Uh oh!

Uh oh!

WiktorStarczewski May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

0xnullifier commented May 1, 2026

Uh oh!

WiktorStarczewski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WiktorStarczewski May 4, 2026

Choose a reason for hiding this comment

Uh oh!

WiktorStarczewski May 4, 2026

Choose a reason for hiding this comment

Uh oh!

WiktorStarczewski May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WiktorStarczewski May 4, 2026

Choose a reason for hiding this comment

Uh oh!

WiktorStarczewski May 4, 2026

Choose a reason for hiding this comment

Uh oh!

WiktorStarczewski May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WiktorStarczewski May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants