Skip to content

Commit

Permalink
fix: reset wallet manager to ensure correct generation of new accounts (
Browse files Browse the repository at this point in the history
#1188)

Previously, `Wallet Manager` was failing to clear properly when
`db.clear();` is called.

This led to wrong account addresses generation, as updates to the
`IndexedDB` didn't reflect in the Wallet Manager's internal state,
particularly the `#vaults` property.

To resolve this issue, I implemented a manual call to `removeVault`
during logout.

This ensures that each new wallet generated starts from scratch, free
from interference by any previous mnemonic vault.
  • Loading branch information
helciofranco authored Mar 28, 2024
1 parent 7fac95e commit d51591e
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 29 deletions.
11 changes: 11 additions & 0 deletions .changeset/thick-feet-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"fuels-wallet": patch
---

Previously, `Wallet Manager` was failing to clear properly when `db.clear();` is called.

This led to wrong account addresses generation, as updates to the `IndexedDB` didn't reflect in the Wallet Manager's internal state, particularly the `#vaults` property.

To resolve this issue, I implemented a manual call to `removeVault` during logout.

This ensures that each new wallet generated starts from scratch, free from interference by any previous mnemonic vault.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export type AccountListProps = {
};

export function AccountList({
accounts,
accounts = [],
canHideAccounts,
hasHiddenAccounts,
isLoading,
Expand All @@ -36,15 +36,14 @@ export function AccountList({
<Box.Stack gap="$4">
{isLoading && (
<CardList>
{[...Array(3)].map((_, i) => {
// biome-ignore lint/suspicious/noArrayIndexKey: <explanation>
{[...Array(3).keys()].map((i) => {
return <AccountItem.Loader key={i} />;
})}
</CardList>
)}
{!isLoading && (
<CardList isClickable>
{(accounts ?? []).map((account) => (
{accounts.map((account) => (
<AccountItem
onPress={() => onPress?.(account)}
onUpdate={onUpdate}
Expand Down
21 changes: 13 additions & 8 deletions packages/app/src/systems/Account/machines/addAccountMachine.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,27 +95,32 @@ export const addAccountMachine = createMachine(
throw new Error('Account name already exists');
}

// Get the first vault since users can have multiple vaults added via private keys,
// and the first vault is typically the initial one (generated or imported via mnemonic).
const [vault] = await VaultService.getVaults();

// Add account to vault
const accountVault = await VaultService.addAccount({
// TODO: remove this when we have multiple vaults
// https://github.com/FuelLabs/fuels-wallet/issues/562
vaultId: 0,
vaultId: vault.vaultId,
});

// Add account to the database
let account = await AccountService.addAccount({
const account = await AccountService.addAccount({
data: {
name,
...accountVault,
address: accountVault.address,
publicKey: accountVault.publicKey,
vaultId: accountVault.vaultId,
isHidden: false,
},
});

// set as active account
account = await AccountService.setCurrentAccount({
address: account.address.toString(),
const currentAccount = await AccountService.setCurrentAccount({
address: account.address,
});

return account;
return currentAccount;
},
}),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export const importAccountMachine = createMachine(
const account = await AccountService.addAccount({
data: {
name: input.name,
address: accountVault.address.toString(),
address: accountVault.address,
publicKey: accountVault.publicKey,
isHidden: false,
vaultId: accountVault.vaultId,
Expand All @@ -121,7 +121,7 @@ export const importAccountMachine = createMachine(

// set as active account
const activeAccount = await AccountService.setCurrentAccount({
address: account.address.toString(),
address: account.address,
});

return activeAccount;
Expand Down
2 changes: 2 additions & 0 deletions packages/app/src/systems/Core/services/core.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { VaultService } from '~/systems/Vault';
import { db } from '../utils/database';
import { Storage } from '../utils/storage';

// biome-ignore lint/complexity/noStaticOnlyClass: <explanation>
export class CoreService {
static async clear() {
await VaultService.clear();
await db.clear();
await Storage.clear();
}
Expand Down
6 changes: 5 additions & 1 deletion packages/app/src/systems/Settings/hooks/useExportVault.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useInterpret, useSelector } from '@xstate/react';

import { VaultService } from '~/systems/Vault';
import type { ExportVaultMachineState } from '../machines';
import { exportVaultMachine } from '../machines';

Expand All @@ -18,11 +19,14 @@ export function useExportVault() {
const words = useSelector(service, selectors.words);
const isUnlockOpened = useSelector(service, selectors.isUnlockOpened);

function exportVault(password: string) {
async function exportVault(password: string) {
const [vault] = await VaultService.getVaults();

service.send({
type: 'EXPORT_VAULT',
input: {
password,
vaultId: vault.vaultId,
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,19 @@ describe('exportAccountMachine', () => {

it('should be able to export account private key', async () => {
state = await expectStateMatch(service, 'waitingPassword');
service.send('EXPORT_VAULT', { input: { password: pass } });
service.send('EXPORT_VAULT', { input: { password: pass, vaultId: 0 } });
state = await expectStateMatch(service, 'done');
expect(state.context.words.join(' ')).toBe(phrase);
});

it('should fail with incorrect password and be able to try again', async () => {
state = await expectStateMatch(service, 'waitingPassword');
service.send('EXPORT_VAULT', { input: { password: `${pass}1` } });
service.send('EXPORT_VAULT', {
input: { password: `${pass}1`, vaultId: 0 },
});
state = await expectStateMatch(service, 'waitingPassword');
expect(state.context.error).toBe('Invalid password');
service.send('EXPORT_VAULT', { input: { password: pass } });
service.send('EXPORT_VAULT', { input: { password: pass, vaultId: 0 } });
state = await expectStateMatch(service, 'done');
expect(state.context.words.join(' ')).toStrictEqual(phrase);
});
Expand Down
15 changes: 10 additions & 5 deletions packages/app/src/systems/Settings/machines/exportVaultMachine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type MachineContext = {

type MachineEvents = {
type: 'EXPORT_VAULT';
input: Omit<VaultInputs['exportVault'], 'vaultId'>;
input: VaultInputs['exportVault'];
};

export const exportVaultMachine = createMachine(
Expand Down Expand Up @@ -54,6 +54,7 @@ export const exportVaultMachine = createMachine(
ev: Extract<MachineEvents, { type: 'EXPORT_VAULT' }>
) => ({
password: ev.input.password,
vaultId: ev.input.vaultId,
}),
},
onDone: [
Expand Down Expand Up @@ -94,12 +95,16 @@ export const exportVaultMachine = createMachine(
if (!input?.password) {
throw new Error('Password is required to export Vault!');
}

if (input?.vaultId === undefined) {
throw new Error('Vault ID is required to export Vault!');
}

const secret = await VaultService.exportVault({
...input,
// TODO change once we add multiple vault management
// https://github.com/FuelLabs/fuels-wallet/issues/562
vaultId: 0,
password: input.password,
vaultId: input.vaultId,
});

return secret.split(' ');
},
}),
Expand Down
5 changes: 3 additions & 2 deletions packages/app/src/systems/SignUp/services/signup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class SignUpService {

// Ensure database is open
await db.open();
// Clear databse on create
// Clear database on create
await db.clear();

// Add networks
Expand All @@ -38,9 +38,10 @@ export class SignUpService {
});

// Register the first account retuned from the vault
const name = await AccountService.generateAccountName();
return AccountService.addAccount({
data: {
name: 'Account 1',
name,
address: account.address.toString(),
publicKey: account.publicKey,
isHidden: false,
Expand Down
26 changes: 22 additions & 4 deletions packages/app/src/systems/Vault/services/VaultServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class VaultServer extends EventEmitter {
'isLocked',
'unlock',
'createVault',
'getVaults',
'getAccounts',
'addAccount',
'signMessage',
Expand All @@ -59,6 +60,7 @@ export class VaultServer extends EventEmitter {
'exportVault',
'exportPrivateKey',
'lock',
'clear',
];

constructor() {
Expand Down Expand Up @@ -88,17 +90,26 @@ export class VaultServer extends EventEmitter {
type,
secret,
});
const accounts = await this.manager.getAccounts();
const vaults = await this.manager.getVaults();
const vaultId = vaults.length - 1;

const [vaults, accounts] = await Promise.all([
this.manager.getVaults(),
this.manager.getAccounts(),
]);

const [vault] = vaults.slice(-1);
const [account] = accounts.slice(-1);

return {
address: account.address.toString(),
publicKey: account.publicKey,
vaultId,
vaultId: vault.vaultId,
};
}

async getVaults() {
return await this.manager.getVaults();
}

async isLocked(): Promise<boolean> {
return this.manager.isLocked;
}
Expand Down Expand Up @@ -176,6 +187,13 @@ export class VaultServer extends EventEmitter {
await this.manager.unlock(password);
return this.manager.exportPrivateKey(Address.fromString(address));
}

async clear(): Promise<void> {
const vaults = await this.manager.getVaults();
for (const vault of vaults) {
await this.manager.removeVault(vault.vaultId);
}
}
}

export type VaultMethods = {
Expand Down

0 comments on commit d51591e

Please sign in to comment.