From 1f40f9cecd8997ec3c28d07034611b5b46a98a3a Mon Sep 17 00:00:00 2001 From: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com> Date: Thu, 3 Oct 2024 14:30:07 +0530 Subject: [PATCH 1/6] define SkipError --- src/content/keyAutoAdd/lib/keyAutoAdd.ts | 39 +++++++++++++----------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/content/keyAutoAdd/lib/keyAutoAdd.ts b/src/content/keyAutoAdd/lib/keyAutoAdd.ts index 24d84401..978955d6 100644 --- a/src/content/keyAutoAdd/lib/keyAutoAdd.ts +++ b/src/content/keyAutoAdd/lib/keyAutoAdd.ts @@ -23,8 +23,6 @@ export type { StepRun } from './types'; export const LOGIN_WAIT_TIMEOUT = 10 * 60 * 1000; -const SYMBOL_SKIP = Symbol.for('skip'); - export class KeyAutoAdd { private port: Runtime.Port; private ui: HTMLIFrameElement; @@ -135,10 +133,9 @@ export class KeyAutoAdd { keyId, keyAddUrl, skip: (details) => { - throw { - type: SYMBOL_SKIP, - details: typeof details === 'string' ? new Error(details) : details, - }; + throw new SkipError( + typeof details === 'string' ? { message: details } : details, + ); }, setNotificationSize: (size: 'notification' | 'fullscreen') => { this.setNotificationSize(size); @@ -165,14 +162,12 @@ export class KeyAutoAdd { this.setStatus(stepIdx, 'success', {}); prevStepId = step.name; } catch (error) { - if (this.isSkip(error)) { - const details = this.errorToDetails( - error.details.error || error.details, - ); + if (error instanceof SkipError) { + const details = error.toJSON(); this.setStatus(stepIdx, 'skipped', { details }); continue; } - const details = this.errorToDetails(error); + const details = errorToDetails(error); this.setStatus(stepIdx, 'error', { details: details }); this.postMessage('ERROR', { details, stepName: step.name, stepIdx }); this.port.disconnect(); @@ -205,15 +200,23 @@ export class KeyAutoAdd { }; this.postMessage('PROGRESS', { steps: this.steps }); } +} - private isSkip(err: unknown): err is { type: symbol; details: Details } { - if (!err || typeof err !== 'object') return false; - return 'type' in err && err.type === SYMBOL_SKIP; +class SkipError extends Error { + public readonly error?: ErrorWithKeyLike; + constructor(err: ErrorWithKeyLike | { message: string }) { + const { message, error } = errorToDetails(err); + super(message); + this.error = error; } - private errorToDetails(err: { message: string } | ErrorWithKeyLike) { - return isErrorWithKey(err) - ? { error: errorWithKeyToJSON(err), message: err.key } - : { message: err.message as string }; + toJSON(): Details { + return { message: this.message, error: this.error }; } } + +function errorToDetails(err: { message: string } | ErrorWithKeyLike) { + return isErrorWithKey(err) + ? { error: errorWithKeyToJSON(err), message: err.key } + : { message: err.message as string }; +} From c4ec1b4b3650e05422489f99c4f558a671a53839 Mon Sep 17 00:00:00 2001 From: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com> Date: Thu, 3 Oct 2024 14:43:01 +0530 Subject: [PATCH 2/6] step runner: allow getting output of any step; not just previous --- src/content/keyAutoAdd/lib/keyAutoAdd.ts | 35 +++++++++------------ src/content/keyAutoAdd/lib/types.ts | 17 +++++------ src/content/keyAutoAdd/testWallet.ts | 39 ++++++++++-------------- 3 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/content/keyAutoAdd/lib/keyAutoAdd.ts b/src/content/keyAutoAdd/lib/keyAutoAdd.ts index 978955d6..63bf2b45 100644 --- a/src/content/keyAutoAdd/lib/keyAutoAdd.ts +++ b/src/content/keyAutoAdd/lib/keyAutoAdd.ts @@ -15,7 +15,8 @@ import type { KeyAutoAddToBackgroundMessage, KeyAutoAddToBackgroundMessagesMap, Step, - StepRunParams, + StepRun, + StepRunHelpers, StepWithStatus, } from './types'; @@ -29,6 +30,7 @@ export class KeyAutoAdd { private stepsInput: Map; private steps: StepWithStatus[]; + private outputs = new Map(); constructor(steps: Step[]) { this.stepsInput = new Map(steps.map((step) => [step.name, step])); @@ -119,19 +121,15 @@ export class KeyAutoAdd { return promise; } - private async run({ - walletAddressUrl, - publicKey, - nickName, - keyId, - keyAddUrl, - }: BeginPayload) { - const params: StepRunParams = { - walletAddressUrl, - publicKey, - nickName, - keyId, - keyAddUrl, + private async run(payload: BeginPayload) { + const helpers: StepRunHelpers = { + output: (fn: T) => { + if (!this.outputs.has(fn)) { + // Was never run? Was skipped? + throw new Error('Given step has no output'); + } + return this.outputs.get(fn) as Awaited>; + }, skip: (details) => { throw new SkipError( typeof details === 'string' ? { message: details } : details, @@ -145,8 +143,6 @@ export class KeyAutoAdd { await this.addNotification(); this.postMessage('PROGRESS', { steps: this.steps }); - let prevStepId = ''; - let prevStepResult: unknown = undefined; for (let stepIdx = 0; stepIdx < this.steps.length; stepIdx++) { const step = this.steps[stepIdx]; const stepInfo = this.stepsInput.get(step.name)!; @@ -156,11 +152,10 @@ export class KeyAutoAdd { : undefined, }); try { - prevStepResult = await this.stepsInput - .get(step.name)! - .run(params, prevStepId ? prevStepResult : null); + const run = this.stepsInput.get(step.name)!.run; + const res = await run(payload, helpers); + this.outputs.set(run, res); this.setStatus(stepIdx, 'success', {}); - prevStepId = step.name; } catch (error) { if (error instanceof SkipError) { const details = error.toJSON(); diff --git a/src/content/keyAutoAdd/lib/types.ts b/src/content/keyAutoAdd/lib/types.ts index abb28a1e..d1b0cae1 100644 --- a/src/content/keyAutoAdd/lib/types.ts +++ b/src/content/keyAutoAdd/lib/types.ts @@ -1,21 +1,20 @@ import type { ErrorWithKeyLike } from '@/shared/helpers'; import type { ErrorResponse } from '@/shared/messages'; -export interface StepRunParams extends BeginPayload { +export interface StepRunHelpers { skip: (message: string | Error | ErrorWithKeyLike) => never; setNotificationSize: (size: 'notification' | 'fullscreen') => void; + output: (fn: T) => Awaited>; } -export type StepRun = ( - params: StepRunParams, - prevStepResult: T extends (...args: any[]) => PromiseLike - ? Exclude>, void | { type: symbol }> - : T, -) => Promise; +export type StepRun = ( + payload: BeginPayload, + helpers: StepRunHelpers, +) => Promise; -export interface Step { +export interface Step { name: string; - run: StepRun; + run: StepRun; maxDuration?: number; } diff --git a/src/content/keyAutoAdd/testWallet.ts b/src/content/keyAutoAdd/testWallet.ts index afcf6ace..da315ae0 100644 --- a/src/content/keyAutoAdd/testWallet.ts +++ b/src/content/keyAutoAdd/testWallet.ts @@ -3,7 +3,7 @@ import { errorWithKey, ErrorWithKey, sleep } from '@/shared/helpers'; import { KeyAutoAdd, LOGIN_WAIT_TIMEOUT, - type StepRun as Step, + type StepRun as Run, } from './lib/keyAutoAdd'; import { isTimedOut, waitForURL } from './lib/helpers'; import { toWalletAddressUrl } from '@/popup/lib/utils'; @@ -26,11 +26,10 @@ type AccountDetails = { }; }; -const waitForLogin: Step = async ({ - skip, - setNotificationSize, - keyAddUrl, -}) => { +const waitForLogin: Run = async ( + { keyAddUrl }, + { skip, setNotificationSize }, +) => { let alreadyLoggedIn = window.location.href.startsWith(keyAddUrl); if (!alreadyLoggedIn) setNotificationSize('notification'); try { @@ -51,9 +50,7 @@ const waitForLogin: Step = async ({ } }; -const getAccountDetails: Step = async ({ - setNotificationSize, -}) => { +const getAccounts: Run = async (_, { setNotificationSize }) => { setNotificationSize('fullscreen'); await sleep(1000); @@ -88,16 +85,13 @@ const getAccountDetails: Step = async ({ * with a different account (user disconnected and changed account), revoke from * there first. */ -const revokeExistingKey: Step = async ( - { keyId, skip }, - accounts, -) => { +const revokeExistingKey: Run = async ({ keyId }, { skip, output }) => { + const accounts = output(getAccounts); for (const account of accounts) { for (const wallet of account.walletAddresses) { for (const key of wallet.keys) { if (key.id === keyId) { await revokeKey(account.id, wallet.id, key.id); - return accounts; } } } @@ -106,10 +100,11 @@ const revokeExistingKey: Step = async ( skip('No existing keys that need to be revoked'); }; -const findWallet: Step< - typeof revokeExistingKey, - { accountId: string; walletId: string } -> = async ({ walletAddressUrl }, accounts) => { +const findWallet: Run<{ accountId: string; walletId: string }> = async ( + { walletAddressUrl }, + { output }, +) => { + const accounts = output(getAccounts); for (const account of accounts) { for (const wallet of account.walletAddresses) { if (toWalletAddressUrl(wallet.url) === walletAddressUrl) { @@ -120,10 +115,8 @@ const findWallet: Step< throw new ErrorWithKey('connectWalletKeyService_error_accountNotFound'); }; -const addKey: Step = async ( - { publicKey, nickName }, - { accountId, walletId }, -) => { +const addKey: Run = async ({ publicKey, nickName }, { output }) => { + const { accountId, walletId } = output(findWallet); const url = `https://api.rafiki.money/accounts/${accountId}/wallet-addresses/${walletId}/upload-key`; const res = await fetch(url, { method: 'POST', @@ -169,7 +162,7 @@ new KeyAutoAdd([ run: waitForLogin, maxDuration: LOGIN_WAIT_TIMEOUT, }, - { name: 'Getting account details', run: getAccountDetails }, + { name: 'Getting account details', run: getAccounts }, { name: 'Revoking existing key', run: revokeExistingKey }, { name: 'Finding wallet', run: findWallet }, { name: 'Adding key', run: addKey }, From 94f1fcb0a3d7a39a9dde7a93980f37d0759f4eb2 Mon Sep 17 00:00:00 2001 From: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:02:42 +0530 Subject: [PATCH 3/6] nit: types --- src/content/keyAutoAdd/lib/keyAutoAdd.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/content/keyAutoAdd/lib/keyAutoAdd.ts b/src/content/keyAutoAdd/lib/keyAutoAdd.ts index 63bf2b45..ea61534e 100644 --- a/src/content/keyAutoAdd/lib/keyAutoAdd.ts +++ b/src/content/keyAutoAdd/lib/keyAutoAdd.ts @@ -210,7 +210,7 @@ class SkipError extends Error { } } -function errorToDetails(err: { message: string } | ErrorWithKeyLike) { +function errorToDetails(err: { message: string } | ErrorWithKeyLike): Details { return isErrorWithKey(err) ? { error: errorWithKeyToJSON(err), message: err.key } : { message: err.message as string }; From f614cae224fad5a4e0f72bd6120640c892189198 Mon Sep 17 00:00:00 2001 From: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com> Date: Thu, 3 Oct 2024 17:07:19 +0530 Subject: [PATCH 4/6] T -> TOutput --- src/content/keyAutoAdd/lib/types.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/content/keyAutoAdd/lib/types.ts b/src/content/keyAutoAdd/lib/types.ts index d1b0cae1..76906df7 100644 --- a/src/content/keyAutoAdd/lib/types.ts +++ b/src/content/keyAutoAdd/lib/types.ts @@ -7,14 +7,14 @@ export interface StepRunHelpers { output: (fn: T) => Awaited>; } -export type StepRun = ( +export type StepRun = ( payload: BeginPayload, helpers: StepRunHelpers, -) => Promise; +) => Promise; -export interface Step { +export interface Step { name: string; - run: StepRun; + run: StepRun; maxDuration?: number; } From 318abdbc2dccb73ab78ecf7116fbe94d23c31732 Mon Sep 17 00:00:00 2001 From: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com> Date: Thu, 3 Oct 2024 17:45:34 +0530 Subject: [PATCH 5/6] add comment to explain why test wallet revokeKey --- src/content/keyAutoAdd/testWallet.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/content/keyAutoAdd/testWallet.ts b/src/content/keyAutoAdd/testWallet.ts index da315ae0..ae5698a5 100644 --- a/src/content/keyAutoAdd/testWallet.ts +++ b/src/content/keyAutoAdd/testWallet.ts @@ -84,6 +84,14 @@ const getAccounts: Run = async (_, { setNotificationSize }) => { * The test wallet associates key with an account. If the same key is associated * with a different account (user disconnected and changed account), revoke from * there first. + * + * Why? Say, user connected once to `USD -> Addr#1`. Then disconnected. The key + * is still there in wallet added to `USD -> Addr#1` account. If now user wants + * to connect `EUR -> Addr#2` account, the extension still has the same key. So + * adding it again will throw an `internal server error`. But we'll continue + * getting `invalid_client` if we try to connect without the key added to new + * address. That's why we first revoke existing key (by ID) if any (from any + * existing account/address). It's a test-wallet specific thing. */ const revokeExistingKey: Run = async ({ keyId }, { skip, output }) => { const accounts = output(getAccounts); From 4f01b7cb3dfb3c878c37395705309d2f6263a9bc Mon Sep 17 00:00:00 2001 From: Sid Vishnoi <8426945+sidvishnoi@users.noreply.github.com> Date: Thu, 3 Oct 2024 17:47:10 +0530 Subject: [PATCH 6/6] run -> runAll in runner --- src/content/keyAutoAdd/lib/keyAutoAdd.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/content/keyAutoAdd/lib/keyAutoAdd.ts b/src/content/keyAutoAdd/lib/keyAutoAdd.ts index ea61534e..94cb2ea4 100644 --- a/src/content/keyAutoAdd/lib/keyAutoAdd.ts +++ b/src/content/keyAutoAdd/lib/keyAutoAdd.ts @@ -43,7 +43,7 @@ export class KeyAutoAdd { this.port.onMessage.addListener( (message: BackgroundToKeyAutoAddMessage) => { if (message.action === 'BEGIN') { - this.run(message.payload); + this.runAll(message.payload); } }, ); @@ -121,7 +121,7 @@ export class KeyAutoAdd { return promise; } - private async run(payload: BeginPayload) { + private async runAll(payload: BeginPayload) { const helpers: StepRunHelpers = { output: (fn: T) => { if (!this.outputs.has(fn)) {