Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 133 additions & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# Key Rotation: Expand & Collapse Architecture

## Problem

The current `addAndRevokeKeys` Cadence transaction is atomic: it adds the new
key **and** revokes the old key in a single transaction. This means if anything
goes wrong with the new key after the transaction succeeds, the user has no
fallback.

The fundamental risk of an atomic rotation remains: **the old key is destroyed
before the new key is proven to work.**

## Proposed Architecture: 3-Phase Rotation

Split the atomic rotation into three distinct phases:

```
Phase 1: EXPAND Phase 2: VERIFY Phase 3: COLLAPSE
┌──────────────┐ ┌──────────────┐ ┌──────────────┐
│ Add new key │──────>│ Sign test │──────>│ Revoke old │
│ (keep old) │ │ payload with │ │ key(s) │
│ │ │ new key │ │ │
└──────────────┘ └──────────────┘ └──────────────┘
Account has Proves new key Old key removed
2 active keys actually works only after proof
```

### Phase 1: Expand (On-chain)

A new Cadence transaction (`addKey`) that **only adds** the new public key to
the account. The old Blocto key remains active. The account temporarily has two
valid signing keys.

**Why this is safe:** If the app crashes here, the user still has the old key.
Nothing has been destroyed.

### Phase 2: Verify (Off-chain)

The app constructs a test payload and signs it using the **new** private key.
This proves:

1. The key material was correctly stored
2. The signing algorithms are compatible
3. The key can produce valid signatures

**Why this is critical:** An atomic architecture skips verification entirely.
The new key is assumed to work because it was generated correctly. But storage
corruption, algorithm mismatches, or platform bridge bugs could silently produce
an unusable key.

### Phase 3: Collapse (On-chain)

Only after successful verification, a second Cadence transaction (`revokeKeys`)
removes the old Blocto key(s).

**Why this is safe:** By this point, we have mathematical proof that the new key
works. The old key can be safely retired.

## Required Changes

### New Cadence Transactions

The existing `addAndRevokeKeys` transaction must be decomposed into two separate
transactions:

```cadence
// addKey.cdc - Phase 1
transaction(publicKeys: [String]) {
prepare(signer: auth(Keys) &Account) {
for publicKey in publicKeys {
let key = PublicKey(
publicKey: publicKey.decodeHex(),
signatureAlgorithm: SignatureAlgorithm.ECDSA_secp256k1
)
signer.keys.add(
publicKey: key,
hashAlgorithm: HashAlgorithm.SHA2_256,
weight: 1000.0
)
}
}
}
```

```cadence
// revokeKeys.cdc - Phase 3
transaction(revokeKeyIndexs: [Int]) {
prepare(signer: auth(Keys) &Account) {
for revokeKeyIndex in revokeKeyIndexs {
signer.keys.revoke(keyIndex: revokeKeyIndex)
}
}
}
```

### Service Layer Changes

`KeyRotationService.rotateKey` would be refactored to orchestrate the 3 phases:

```typescript
async rotateKey(address: string, newKeyInfo: NewKeyInfo) {
// Phase 1: Add key (old key still works)
const addTxId = await this.workflow.addKeyOnChain(newKeyInfo.flowKey.publicKey);
await waitForExecuted(addTxId);

// Phase 2: Verify new key works
const testPayload = crypto.randomBytes(32).toString('hex');
const signature = await signWithNewKey(newKeyInfo, testPayload);
const verified = await verifySignature(newKeyInfo.flowKey.publicKey, testPayload, signature);

if (!verified) {
throw new Error('New key verification failed. Old key is still active. No assets at risk.');
}

// Phase 3: Revoke old keys (new key is proven)
const revokeTxId = await this.workflow.revokeKeysOnChain(bloctoKeyIndexes);
await waitForExecuted(revokeTxId);
}
```

## Failure Safety Comparison

| Failure Point | Current (Atomic) | Expand & Collapse |
| ------------------------------ | ----------------------- | ------------------------------ |
| Crash after add, before revoke | N/A (atomic) | ✅ Both keys work, retry later |
| New key is corrupted | 🔴 **Locked out** | ✅ Verification catches it |
| Storage write fails silently | 🔴 **Locked out** | ✅ Verification catches it |
| Network dies mid-rotation | 🔴 **Possible lockout** | ✅ Old key still active |

## Status

This document is an architectural proposal. The Expand & Collapse pattern is the
recommended long-term architecture for key rotation safety.
70 changes: 52 additions & 18 deletions apps/extension/src/background/controller/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1325,21 +1325,55 @@ export class WalletController extends BaseController {
*/
signRotationRequest = async (
address: string,
signatureData: string
signatureData: string,
pendingNewKeyInfo?: any
): Promise<AccountKeySignature> => {
// Check if keyring is unlocked
if (!keyringService.isUnlocked()) {
throw new Error('Keyring must be unlocked to sign rotation request');
}

// Get private key and public key from keyring service (runs in background)
const privateKey = await keyringService.getCurrentPrivateKey();
const signAlgo = keyringService.getCurrentSignAlgo() || 2; // Default to secp256k1
const oldPublicKey = keyringService.getCurrentPublicKey();

// Import signing utilities
const { signWithKey } = await import('@/core/utils/modules/publicPrivateKey');
const { HASH_ALGO_NUM_SHA3_256 } = await import('@/shared/constant');
const {
HASH_ALGO_NUM_SHA3_256,
HASH_ALGO_NUM_SHA2_256,
SIGN_ALGO_NUM_ECDSA_secp256k1,
SIGN_ALGO_NUM_ECDSA_P256,
} = await import('@/shared/constant');

let privateKey: string;
let signAlgo: number;
let hashAlgo: number;
let publicKey: string;
let weight = 1000;

const isVerifyPayload = signatureData.startsWith('key-rotation-verify:');
if (isVerifyPayload && pendingNewKeyInfo && pendingNewKeyInfo.seedphrase) {
// Phase 2 Verify of the 3-phase flow: sign with the NEW key.
const { seedWithPathAndPhrase2PublicPrivateKey } = await import(
'@/core/utils/modules/publicPrivateKey'
);
const keyTuple = await seedWithPathAndPhrase2PublicPrivateKey(pendingNewKeyInfo.seedphrase);

signAlgo = pendingNewKeyInfo.flowKey?.signAlgo || SIGN_ALGO_NUM_ECDSA_P256;
hashAlgo = pendingNewKeyInfo.flowKey?.hashAlgo || HASH_ALGO_NUM_SHA3_256;
weight = pendingNewKeyInfo.flowKey?.weight || 1000;

if (signAlgo === SIGN_ALGO_NUM_ECDSA_secp256k1) {
privateKey = keyTuple.SECP256K1.pk;
publicKey = keyTuple.SECP256K1.pubK;
} else {
privateKey = keyTuple.P256.pk;
publicKey = keyTuple.P256.pubK;
}
} else {
// Check if keyring is unlocked
if (!keyringService.isUnlocked()) {
throw new Error('Keyring must be unlocked to sign rotation request');
}

// Default to current key from keyring (old behavior, e.g. for fallback/other flows)
privateKey = await keyringService.getCurrentPrivateKey();
signAlgo = keyringService.getCurrentSignAlgo() || 2; // Default to secp256k1
hashAlgo = HASH_ALGO_NUM_SHA3_256; // Legacy used hardcoded SHA3_256
publicKey = keyringService.getCurrentPublicKey();
}

// The backend verification uses Flow's verify with domain separation tag "FLOW-V0.0-user"
// We need to prepend this tag to the message before hashing, just like login does
Expand All @@ -1349,24 +1383,24 @@ export class WalletController extends BaseController {
const USER_DOMAIN_TAG = rightPaddedHexBuffer(Buffer.from('FLOW-V0.0-user').toString('hex'), 32);
const message = USER_DOMAIN_TAG + Buffer.from(signatureData, 'utf8').toString('hex');

// Sign the message (with domain tag prepended) with SHA3_256
// signWithKey will hash the message with SHA3_256 internally
// Sign the message (with domain tag prepended)
const signatureString = await signWithKey(
message,
signAlgo,
HASH_ALGO_NUM_SHA3_256,
hashAlgo,
privateKey,
false,
false // isPrehashed=false - let signWithKey hash it with SHA3_256
false // isPrehashed=false - let signWithKey hash it
);

// Return AccountKeySignature object
return {
public_key: oldPublicKey, // Use the OLD key's public key (the one that signed)
hash_algo: HASH_ALGO_NUM_SHA3_256,
public_key: publicKey,
hash_algo: hashAlgo,
sign_algo: signAlgo,
signature: signatureString,
sign_message: signatureData,
weight: weight,
};
};

Expand Down
34 changes: 26 additions & 8 deletions apps/extension/src/bridge/PlatformImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -866,16 +866,19 @@ class ExtensionPlatformImpl implements PlatformSpec {

// Temporary storage for password during key rotation
private keyRotationPassword: string | null = null;
// Temporary storage for the new key during the 3-phase key rotation
private keyRotationPendingKey: NewKeyInfo | null = null;

setKeyRotationPassword(password: string | null): void {
this.keyRotationPassword = password;
}

async saveNewKey(key: NewKeyInfo): Promise<void> {
// The new key info is already stored in component state (via handleTipContinue)
// and will be used directly in handlePasswordSubmit to login with the new mnemonic
// No need to store it here - just a no-op
this.log('debug', 'saveNewKey: New key info will be used in UI component');
// and will be used directly in handlePasswordSubmit to login with the new mnemonic.
// However, KeyRotationService (Phase 2) needs it to verify the new key, so we store it temporarily here.
this.keyRotationPendingKey = key;
this.log('debug', 'saveNewKey: New key info saved temporarily for Phase 2 verification');
return Promise.resolve();
}

Expand All @@ -899,11 +902,26 @@ class ExtensionPlatformImpl implements PlatformSpec {
throw new Error('Wallet controller not available - cannot sign rotation request');
}

// Route signing to wallet controller which runs in background context
// where keyring service is properly booted and unlocked
// The wallet controller will get the public key internally from the keyring service
// We pass an empty string for publicKey since the wallet controller will get it from keyring
return await this.walletController.signRotationRequest(address, signatureData);
// Only Phase 2 verification payloads should use the pending NEW key.
// API submit signing must continue to use the currently active key.
const isVerifyPayload = signatureData.startsWith('key-rotation-verify:');
const pendingKey = isVerifyPayload ? this.keyRotationPendingKey : null;

try {
const signature = await this.walletController.signRotationRequest(
address,
signatureData,
pendingKey
);

return signature;
} finally {
// Pending key is a one-shot bridge value for Phase 2 verification.
// Always clear it after a verify attempt (success or failure).
if (isVerifyPayload) {
this.keyRotationPendingKey = null;
}
}
}

getKeyRotationDependencies(): KeyRotationDependencies {
Expand Down
12 changes: 12 additions & 0 deletions apps/react-native/src/bridge/NativeFRWBridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ interface AccountKeySignature {
weight?: number;
}

interface PendingRotationState {
address: string;
publicKey: string;
seedphrase: string;
timestamp: number;
txId?: string;
phase: 'pre-tx' | 'key-added' | 'key-verified' | 'api-registered' | 'tx-confirmed';
}

interface NewKeyInfo {
seedphrase: string;
flowKey: AccountKey;
Expand Down Expand Up @@ -194,6 +203,9 @@ export interface Spec extends TurboModule {
createSeedKey(strength: number): Promise<NewKeyInfo>;
saveNewKey(key: NewKeyInfo): Promise<void>;
removeOldKey(address: string, publicKey: string): Promise<void>;
savePendingRotation(state: PendingRotationState): Promise<void>;
getPendingRotation(address: string): Promise<PendingRotationState | null>;
clearPendingRotation(address: string): Promise<void>;
signRotationRequest(address: string, signatureData: string): Promise<AccountKeySignature>;

// Onboarding methods
Expand Down
14 changes: 13 additions & 1 deletion apps/react-native/src/bridge/PlatformImpl.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Luciq from '@luciq/react-native';
import { type forms_DeviceInfo } from '@onflow/frw-api';
import { type Cache, type Navigation, type PlatformSpec, type Storage } from '@onflow/frw-context';
import type { AccountKeySignature, NewKeyInfo } from '@onflow/frw-types';
import type { AccountKeySignature, NewKeyInfo, PendingRotationState } from '@onflow/frw-types';
import type {
CreateAccountResponse,
Currency,
Expand Down Expand Up @@ -310,6 +310,18 @@ class PlatformImpl implements PlatformSpec {
return NativeFRWBridge.removeOldKey(address, publicKey);
}

savePendingRotation(state: PendingRotationState): Promise<void> {
return NativeFRWBridge.savePendingRotation(state);
}

getPendingRotation(address: string): Promise<PendingRotationState | null> {
return NativeFRWBridge.getPendingRotation(address);
}

clearPendingRotation(address: string): Promise<void> {
return NativeFRWBridge.clearPendingRotation(address);
}

signRotationRequest(address: string, signatureData: string): Promise<AccountKeySignature> {
return NativeFRWBridge.signRotationRequest(address, signatureData);
}
Expand Down
Loading
Loading