diff --git a/AGENTS.md b/AGENTS.md index 327543246e..9f278ba527 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ Note: This package does not include compiled JS and TS output in the repository. ### Code quality: - Code usually runs continuously for long periods of time (especially in browser extension environments), so memory leaks, listeners, and orphaned async processes can accumulate and cause performance, stability, or reliability issues over time. -- Always ensure subscriptions, event listeners, timers and other side effects are properly cleaned up +- Always ensure subscriptions, event listeners, timers and other side effects are properly cleaned up. Even if it's a simple `setTimeout` used to reject a promise, it should be cleared. - NEVER delete existing comments when updating a code block; update inaccurate comments instead. Delete a comment only if the logic it describes is completely removed or the new logic is entirely self-explanatory - NEVER swallow errors; log them and handle appropriately. - NEVER modify git config or run destructive git operations @@ -38,6 +38,7 @@ Note: This package does not include compiled JS and TS output in the repository. - Avoid TypeScript casts when possible. Prefer narrowing with if statements, discriminated unions, assertion functions, and type guards. Cast only at trusted boundaries, and keep it local - Avoid regex for parsing strings or business logic. Prefer explicit parsing, small helper functions, existing parsers or available library functions. - All warnings, errors, and other user-facing strings (controller `errors`/banners, e.g. `signAccountOp.ts`, `swapAndBridge.ts`; humanized strings in `src/libs/humanizer`, `src/libs/errorHumanizer`) must be phrased in plain language a non-technical user can understand. NEVER assume Web3/blockchain knowledge — avoid unexplained jargon (e.g. "RPC", "nonce", "gas limit", "Paymaster", "Bundler", "EOA", "Smart Account", "delegatecall", "calldata", "simulation failed") and instead describe the real-world action, cause, or risk in plain terms (e.g. "a pending transaction" instead of "nonce too low", "insufficient funds to cover the fee" instead of "low gas") +- Comments of reusable functions, types and constants should be public to allow reading them on hover (but don't edit existing comments that are outside of the scope of the task). ## Tests: - ALWAYS write test cases that cover positive, negative, edge cases and security implications of the code you change or add. diff --git a/package-lock.json b/package-lock.json index d7132b6482..c2bfa2e689 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ambire-common", - "version": "2.102.3", + "version": "2.107.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ambire-common", - "version": "2.102.3", + "version": "2.107.1", "dependencies": { "@ambire/signature-validator": "^1.5.0", "@corpus-core/colibri-stateless": "^1.1.30", diff --git a/package.json b/package.json index 276e61704a..364c8d148e 100644 --- a/package.json +++ b/package.json @@ -1,5 +1,5 @@ { - "version": "2.106.5", + "version": "2.107.4", "name": "ambire-common", "description": "Common ground for the Ambire apps", "scripts": { diff --git a/src/classes/session.ts b/src/classes/session.ts index 678cedec04..d605b6fbb8 100644 --- a/src/classes/session.ts +++ b/src/classes/session.ts @@ -116,6 +116,13 @@ export class Session { constructor({ tabId, windowId, url, wcTopic, frameId, topFrameUrl }: SessionInitProps = {}) { if (url) { + // SECURITY: `origin` must stay exactly what the browser/WebView reports, trailing dot and + // all - it is later compared byte-for-byte against the page's own read-only `location.origin` + // before a response or broadcast is delivered to it (see the mobile WebView's origin gate). + // A live, unmodified page cannot be "canonicalized" - only our copy of its origin could be - + // so normalizing it here would make that comparison fail for a legitimate page and silently + // drop data meant for it. `id` (below) is the canonicalized identity that every phishing and + // permission check resolves against; do not canonicalize `origin` to "fix" that too. this.origin = new URL(url).origin } else { this.origin = 'internal' diff --git a/src/consts/featureFlags.ts b/src/consts/featureFlags.ts index 72c8304d42..c178ffa1b4 100644 --- a/src/consts/featureFlags.ts +++ b/src/consts/featureFlags.ts @@ -12,6 +12,11 @@ export interface FeatureFlags { * will need an EOA account just like using a Safe) */ erc4337: boolean + /** + * Allow the user to opt out of upgrading EOA accounts through ERC-7702. + * Existing onchain delegations are not revoked when this is disabled. + */ + eip7702: boolean /** * Off by default for privacy: passively bulk-resolving ENS/Namoshi for all * accounts links them together. When enabled, the wallet keeps every account's @@ -32,6 +37,7 @@ export const defaultFeatureFlags: FeatureFlags = { tokenAndDefiAutoDiscovery: true, apiForFunctionSelectors: true, erc4337: true, + eip7702: true, keepEnsProfilesUpToDate: false, // @TODO: Introduce a setting and flip to false namoshiDomains: true, diff --git a/src/consts/hardwareWallets.ts b/src/consts/hardwareWallets.ts index 77363a39aa..114d0ba7da 100644 --- a/src/consts/hardwareWallets.ts +++ b/src/consts/hardwareWallets.ts @@ -4,5 +4,6 @@ export const HARDWARE_WALLET_DEVICE_NAMES: { [key in ExternalKey['type']]: strin ledger: 'Ledger', trezor: 'Trezor', lattice: 'GridPlus', - qr: 'QR-based' + qr: 'QR-based', + nfc: 'NFC-based' } diff --git a/src/consts/intervals.ts b/src/consts/intervals.ts index 2c434b622e..47216533c9 100644 --- a/src/consts/intervals.ts +++ b/src/consts/intervals.ts @@ -12,6 +12,9 @@ export const BLACKLIST_UPDATE_INTERVAL = 8 * 60 * 60 * 1000 // 8 hrs export const PHISHING_INACTIVE_UPDATE_INTERVAL = 6 * 60 * 60 * 1000 // 6 hrs export const PHISHING_ACTIVE_UPDATE_INTERVAL = 15 * 60 * 1000 // 15 minutes export const PHISHING_FAILED_TO_GET_UPDATE_INTERVAL = 600000 // 10 minutes +export const TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL = 10 * 60 * 1000 // 10 minutes +export const TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL = 4 * 60 * 60 * 1000 // 4 hours +export const TRENDING_TOKENS_FAILED_UPDATE_INTERVAL = 60 * 1000 // 1 minute export const ESTIMATE_UPDATE_INTERVAL = 30000 export const GAS_PRICE_UPDATE_INTERVAL = 12000 export const FETCH_SAFE_TXNS = 3 * 60 * 1000 // 3 minutes diff --git a/src/consts/safe.ts b/src/consts/safe.ts index 680bd66f89..99761fafc3 100644 --- a/src/consts/safe.ts +++ b/src/consts/safe.ts @@ -10,6 +10,9 @@ export const SAFE_NETWORKS = [ export const SAFE_API_TIMEOUT_MS = 15000 +// Keep Safe Transaction Service requests below its bulk request limits. +export const SAFE_API_BATCH_SIZE = 4 + /** * SimulateTxAccessor addresses by Safe version. */ diff --git a/src/consts/signAccountOp/errorHandling.ts b/src/consts/signAccountOp/errorHandling.ts index 742dcdd0f8..62c1bccb29 100644 --- a/src/consts/signAccountOp/errorHandling.ts +++ b/src/consts/signAccountOp/errorHandling.ts @@ -7,7 +7,6 @@ const ERRORS = { const WARNINGS: Record< | 'significantBalanceDecrease' | 'unknownToken' - | 'possibleBalanceDecrease' | 'feeTokenPriceUnavailable' | 'v1Acc' | 'safeDelegateCall', @@ -15,9 +14,9 @@ const WARNINGS: Record< > = { significantBalanceDecrease: { id: 'significantBalanceDecrease', - title: 'Significant Account Balance Decrease', - text: 'The transaction you are about to sign will significantly decrease your account balance. Please review the transaction details carefully.', - promptBefore: ['sign'] + title: 'Significant balance decrease detected', + text: 'Our checks indicate this transaction may significantly reduce your account balance.', + secondaryText: 'May be inaccurate when moving funds to another network or providing liquidity.' }, unknownToken: { id: 'unknownToken', @@ -25,12 +24,6 @@ const WARNINGS: Record< text: 'The transaction you are about to sign contains an unknown token. Please review carefully, as this token may be misleading.', promptBefore: ['sign'] }, - possibleBalanceDecrease: { - id: 'possibleBalanceDecrease', - title: 'Significant Account Balance Decrease (Possibly Inaccurate)', - text: 'The transaction you are about to sign may significantly decrease your account balance. However, due to temporary issues in discovering new portfolio tokens, this information might not be fully accurate. Please review the transaction details carefully.', - promptBefore: ['sign'] - }, feeTokenPriceUnavailable: { id: 'feeTokenPriceUnavailable', title: 'Unable to estimate the transaction fee in USD.' diff --git a/src/controllers/accountPicker/accountPicker.test.ts b/src/controllers/accountPicker/accountPicker.test.ts index 503b690adb..3a08370faa 100644 --- a/src/controllers/accountPicker/accountPicker.test.ts +++ b/src/controllers/accountPicker/accountPicker.test.ts @@ -1,8 +1,8 @@ import { Wallet } from 'ethers' -import { describe, expect, test } from '@jest/globals' +import { describe, expect, jest, test } from '@jest/globals' -import { suppressConsoleBeforeEach } from '../../../test/helpers/console' +import { suppressConsole, suppressConsoleBeforeEach } from '../../../test/helpers/console' import { makeMainController } from '../../../test/helpers/mainController' import { DEFAULT_ACCOUNT_LABEL } from '../../consts/account' import { @@ -149,6 +149,136 @@ describe('AccountPicker', () => { expect(controller.accountsOnPage.filter((a) => !isSmartAccount(a.account))).toHaveLength(5) }) + test('should update basic account usage while smart accounts are still loading', async () => { + const { controller } = await prepareTest() + const pageSize = 5 + const keyIterator = new KeyIterator(process.env.SEED) + const retrieve = keyIterator.retrieve.bind(keyIterator) + let resolveSmartAccountKeys: (keys: string[]) => void = () => {} + const smartAccountKeysPromise = new Promise((resolve) => { + resolveSmartAccountKeys = resolve + }) + + jest.spyOn(keyIterator, 'retrieve').mockImplementation((indices, hdPathTemplate) => { + if ((indices[0]?.from || 0) >= SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET) { + return smartAccountKeysPromise + } + + return retrieve(indices, hdPathTemplate) + }) + + controller.setInitParams({ + keyIterator, + pageSize, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + shouldGetAccountsUsedOnNetworks: false, + shouldSearchForLinkedAccounts: false, + shouldAddNextAccountAutomatically: false + }) + await controller.init() + + const setPagePromise = controller.setPage({ page: 1 }) + + while (!controller.smartAccountsLoading) await wait(0) + await wait(0) + + expect(controller.accountsLoading).toBe(false) + expect(controller.accountsOnPage).toHaveLength(pageSize) + expect(controller.accountsOnPage.every((a) => !isSmartAccount(a.account))).toBe(true) + expect(controller.accountsOnPage.every((a) => a.account.usedOnNetworks === null)).toBe(true) + + controller.selectAccount(controller.accountsOnPage[0]!.account) + expect(controller.selectedAccounts).toHaveLength(1) + + resolveSmartAccountKeys( + key1to11BasicAccUsedForSmartAccKeysOnlyPublicAddresses.slice(0, pageSize) + ) + await setPagePromise + + expect(controller.smartAccountsLoading).toBe(false) + expect(controller.accountsOnPage).toHaveLength(pageSize + 1) + expect(controller.selectedAccounts).toHaveLength(1) + }) + + test('should keep basic accounts available when smart account retrieval fails', async () => { + const { controller } = await prepareTest() + const pageSize = 5 + const keyIterator = new KeyIterator(process.env.SEED) + const retrieve = keyIterator.retrieve.bind(keyIterator) + + jest.spyOn(keyIterator, 'retrieve').mockImplementation((indices, hdPathTemplate) => { + if ((indices[0]?.from || 0) >= SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET) { + return Promise.reject(new Error('Smart account key retrieval failed')) + } + + return retrieve(indices, hdPathTemplate) + }) + + controller.setInitParams({ + keyIterator, + pageSize, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + shouldGetAccountsUsedOnNetworks: false, + shouldSearchForLinkedAccounts: false, + shouldAddNextAccountAutomatically: false + }) + await controller.init() + const { restore } = suppressConsole() + await controller.setPage({ page: 1 }) + restore() + + expect(controller.accountsLoading).toBe(false) + expect(controller.smartAccountsLoading).toBe(false) + expect(controller.accountsOnPage).toHaveLength(pageSize) + expect(controller.pageError).toBeNull() + expect(controller.emittedErrors.at(-1)?.level).toBe('minor') + expect(controller.emittedErrors.at(-1)?.message).toBe( + 'We could not finish loading smart accounts. You can still import the accounts already shown.' + ) + + controller.selectAccount(controller.accountsOnPage[0]!.account) + expect(controller.selectedAccounts).toHaveLength(1) + }) + + test('should ignore smart accounts retrieved after the account picker is reset', async () => { + const { controller } = await prepareTest() + const keyIterator = new KeyIterator(process.env.SEED) + const retrieve = keyIterator.retrieve.bind(keyIterator) + let resolveSmartAccountKeys: (keys: string[]) => void = () => {} + const smartAccountKeysPromise = new Promise((resolve) => { + resolveSmartAccountKeys = resolve + }) + + jest.spyOn(keyIterator, 'retrieve').mockImplementation((indices, hdPathTemplate) => { + if ((indices[0]?.from || 0) >= SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET) { + return smartAccountKeysPromise + } + + return retrieve(indices, hdPathTemplate) + }) + + controller.setInitParams({ + keyIterator, + pageSize: 5, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + shouldGetAccountsUsedOnNetworks: false, + shouldSearchForLinkedAccounts: false, + shouldAddNextAccountAutomatically: false + }) + await controller.init() + + const setPagePromise = controller.setPage({ page: 1 }) + while (!controller.smartAccountsLoading) await wait(0) + + await controller.reset() + resolveSmartAccountKeys(key1to11BasicAccUsedForSmartAccKeysOnlyPublicAddresses.slice(0, 5)) + await setPagePromise + + expect(controller.isInitialized).toBe(false) + expect(controller.smartAccountsLoading).toBe(false) + expect(controller.accountsOnPage).toHaveLength(0) + }) + test('should find linked accounts', async () => { const { controller } = await prepareTest() const keyIterator = new KeyIterator(process.env.SEED) diff --git a/src/controllers/accountPicker/accountPicker.ts b/src/controllers/accountPicker/accountPicker.ts index 5d72fc9395..d210d142f0 100644 --- a/src/controllers/accountPicker/accountPicker.ts +++ b/src/controllers/accountPicker/accountPicker.ts @@ -129,6 +129,8 @@ export class AccountPickerController extends EventEmitter implements IAccountPic accountsLoading: boolean = false + smartAccountsLoading: boolean = false + linkedAccountsLoading: boolean = false linkedAccountsError: string = '' @@ -161,6 +163,8 @@ export class AccountPickerController extends EventEmitter implements IAccountPic */ #findAndSetLinkedAccountsAbortController?: AbortController + #setPageRequestId = 0 + #shouldDebounceFlags: { [key: string]: boolean } = {} #addAccountsOnKeystoreReady: { @@ -471,6 +475,7 @@ export class AccountPickerController extends EventEmitter implements IAccountPic } async reset(resetInitParams: boolean = true) { + this.#setPageRequestId++ await this.addAccountsPromise // Abort any ongoing findAndSetLinkedAccounts operation if (this.#findAndSetLinkedAccountsAbortController) { @@ -487,6 +492,8 @@ export class AccountPickerController extends EventEmitter implements IAccountPic this.shouldGetAccountsUsedOnNetworks = DEFAULT_SHOULD_GET_ACCOUNTS_USED_ON_NETWORKS this.pageError = null + this.accountsLoading = false + this.smartAccountsLoading = false this.linkedAccountsLoading = false this.linkedAccountsError = '' this.addAccountsStatus = 'INITIAL' @@ -707,11 +714,13 @@ export class AccountPickerController extends EventEmitter implements IAccountPic this.page = page } else if (page === this.page && this.#derivedAccounts.length) return + const requestId = ++this.#setPageRequestId this.page = page this.pageError = null this.#derivedAccounts = [] this.#linkedAccounts = [] this.accountsLoading = true + this.smartAccountsLoading = false this.networksWithAccountStateError = [] this.linkedAccountsLoading = false this.emitUpdate() @@ -719,39 +728,99 @@ export class AccountPickerController extends EventEmitter implements IAccountPic if (page <= 0) { this.pageError = `Unexpected page was requested (page ${page}). Please try again or contact support for help.` this.page = DEFAULT_PAGE // fallback to the default (initial) page + this.accountsLoading = false this.emitUpdate() return } try { - const derivedAccounts = await this.#deriveAccounts() + const derivedAccounts = await this.#deriveAccounts({ + shouldRetrieveSmartAccountIndices: false + }) - if (this.page !== page) return + if (this.#isSetPageRequestCancelled(requestId, page)) return this.#derivedAccounts = derivedAccounts - // The used on information is not critical. Allow the user to proceed after - // 1 second. It will get popuplated in the background. - const minWaitTimeout = setTimeout(() => { - if (this.page !== page) return - + // Since v4.31.0, do not retrieve smart accounts for the private key + // type. That's because we can't use the common derivation offset + // (SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET), and deriving smart + // accounts out of the private key (with another approach - salt and + // extra entropy) was creating confusion. + // + // + no smart accounts for QR wallets. Reasons: + // - some hws sign only if the signer is imported + // - we are generally moving in another direction + const shouldRetrieveSmartAccountIndices = + this.keyIterator.subType !== 'private-key' && this.type !== 'qr' + + if (shouldRetrieveSmartAccountIndices) { + // Basic accounts are ready to use while smart accounts are retrieved + // from their dedicated derivation indices in a second request. this.accountsLoading = false + this.smartAccountsLoading = true this.emitUpdate() - }, 1000) - const derivedAccountsWithUsedOn = await this.#getAccountsUsedOnNetworks({ - accounts: this.#derivedAccounts, - page - }) + const basicAccountsUsedOnNetworksPromise = this.#getAndSetAccountsUsedOnNetworks({ + accounts: derivedAccounts, + requestId, + page + }) + const smartAccountsPromise = this.#deriveAccounts({ + shouldRetrieveSmartAccountIndices: true + }) + .then(async (smartAccounts) => { + if (this.#isSetPageRequestCancelled(requestId, page)) return + + this.#derivedAccounts = [...this.#derivedAccounts, ...smartAccounts] + this.smartAccountsLoading = false + this.emitUpdate() + + await this.#getAndSetAccountsUsedOnNetworks({ + accounts: smartAccounts, + requestId, + page + }) + }) + .catch((error: unknown) => { + if (this.#isSetPageRequestCancelled(requestId, page)) return + + const message = + 'We could not finish loading smart accounts. You can still import the accounts already shown.' + this.smartAccountsLoading = false + this.emitError({ + error: error instanceof Error ? error : new Error(message), + message, + level: 'minor', + sendCrashReport: !(error instanceof ExternalSignerError) + }) + this.emitUpdate() + }) - if (this.page !== page) return + await Promise.all([basicAccountsUsedOnNetworksPromise, smartAccountsPromise]) + } else { + // The used on information is not critical. Allow the user to proceed after + // 1 second. It will get populated in the background. + const minWaitTimeout = setTimeout(() => { + if (this.#isSetPageRequestCancelled(requestId, page)) return + + this.accountsLoading = false + this.emitUpdate() + }, 1000) + + await this.#getAndSetAccountsUsedOnNetworks({ + accounts: derivedAccounts, + requestId, + page + }) - this.#derivedAccounts = derivedAccountsWithUsedOn + clearTimeout(minWaitTimeout) - if (minWaitTimeout) clearTimeout(minWaitTimeout) + if (this.#isSetPageRequestCancelled(requestId, page)) return - this.accountsLoading = false - this.emitUpdate() + this.accountsLoading = false + this.emitUpdate() + } if (this.keyIterator?.type === 'internal' && this.keyIterator?.subType === 'private-key') { const accountsOnPageWithoutTheLinked = this.accountsOnPage.filter((acc) => !acc.isLinked) @@ -767,14 +836,17 @@ export class AccountPickerController extends EventEmitter implements IAccountPic } } } catch (e: any) { - if (this.page !== page) return + if (this.#isSetPageRequestCancelled(requestId, page)) return const fallbackMessage = `Failed to retrieve accounts on page ${this.page}. Please try again or contact support for assistance. Error details: ${e?.message}.` + this.#setPageRequestId++ this.accountsLoading = false + this.smartAccountsLoading = false this.pageError = e instanceof ExternalSignerError ? e.message : fallbackMessage this.emitUpdate() + return } - if (this.page !== page) return + if (this.#isSetPageRequestCancelled(requestId, page)) return await this.findAndSetLinkedAccounts() } @@ -871,18 +943,22 @@ export class AccountPickerController extends EventEmitter implements IAccountPic ledger: this.#externalSignerControllers.ledger?.deviceId || '', trezor: this.#externalSignerControllers.trezor?.deviceId || '', lattice: this.#externalSignerControllers?.lattice?.deviceId || '', - qr: this.#externalSignerControllers.qr?.deviceId || '' + qr: this.#externalSignerControllers.qr?.deviceId || '', + nfc: this.#externalSignerControllers.nfc?.deviceId || '' } const deviceModels: { [key in ExternalKey['type']]: string } = { ledger: this.#externalSignerControllers.ledger?.deviceModel || '', trezor: this.#externalSignerControllers.trezor?.deviceModel || '', lattice: this.#externalSignerControllers.lattice?.deviceModel || '', - qr: this.#externalSignerControllers.qr?.deviceModel || '' + qr: this.#externalSignerControllers.qr?.deviceModel || '', + nfc: this.#externalSignerControllers.nfc?.deviceModel || '' } const masterFingerprint = this.#externalSignerControllers.qr?.masterFingerprint || '' + const nfcWalletType = this.#externalSignerControllers.nfc?.nfcWalletType + const hdPathTemplate = this.hdPathTemplate as HD_PATH_TEMPLATE_TYPE const readyToAddExternalKeys = this.selectedAccountsFromCurrentSession.flatMap( @@ -908,6 +984,11 @@ export class AccountPickerController extends EventEmitter implements IAccountPic masterFingerprint } : {}), + ...(keyType === 'nfc' + ? { + nfcWalletType + } + : {}), index, createdAt: new Date().getTime() } @@ -1054,7 +1135,15 @@ export class AccountPickerController extends EventEmitter implements IAccountPic this.emitUpdate() } - async #deriveAccounts(): Promise { + #isSetPageRequestCancelled(requestId: number, page: number) { + return requestId !== this.#setPageRequestId || page !== this.page || !this.isInitialized + } + + async #deriveAccounts({ + shouldRetrieveSmartAccountIndices + }: { + shouldRetrieveSmartAccountIndices: boolean + }): Promise { // Should never happen, because before the #deriveAccounts method gets // called - there is a check if the keyIterator exists. if (!this.keyIterator) { @@ -1064,52 +1153,32 @@ export class AccountPickerController extends EventEmitter implements IAccountPic return [] } - const accounts: DerivedAccountWithoutNetworkMeta[] = [] - const startIdx = (this.page - 1) * this.pageSize const endIdx = (this.page - 1) * this.pageSize + (this.pageSize - 1) + const indexOffset = shouldRetrieveSmartAccountIndices + ? SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET + : 0 + const retrievedKeys = await this.keyIterator.retrieve( + [{ from: startIdx + indexOffset, to: endIdx + indexOffset }], + this.hdPathTemplate + ) - const indicesToRetrieve = [ - { from: startIdx, to: endIdx } // Indices for the basic (EOA) accounts - ] - // Since v4.31.0, do not retrieve smart accounts for the private key - // type. That's because we can't use the common derivation offset - // (SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET), and deriving smart - // accounts out of the private key (with another approach - salt and - // extra entropy) was creating confusion. - // - // + no smart accounts for QR wallets. Reasons: - // - some hws sign only if the signer is imported - // - we are generally moving in another direction - const shouldRetrieveSmartAccountIndices = - this.keyIterator.subType !== 'private-key' && this.type !== 'qr' - if (shouldRetrieveSmartAccountIndices) { - // Indices for the smart accounts. - indicesToRetrieve.push({ - from: startIdx + SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET, - to: endIdx + SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET + if (!shouldRetrieveSmartAccountIndices) { + return retrievedKeys.map((basicAccKey, index) => { + const slot = startIdx + index + 1 + const account = getBasicAccount(basicAccKey, this.#alreadyImportedAccounts) + + return { account, isLinked: false, slot, index: slot - 1 } }) } - // Combine the requests for all accounts in one call to the keyIterator. - // That's optimization primarily focused on hardware wallets, to reduce the - // number of calls to the hardware device. This is important, especially - // for Trezor, because it fires a confirmation popup for each call. - const combinedBasicAndSmartAccKeys = await this.keyIterator.retrieve( - indicesToRetrieve, - this.hdPathTemplate - ) - const basicAccKeys = combinedBasicAndSmartAccKeys.slice(0, this.pageSize) - const smartAccKeys = combinedBasicAndSmartAccKeys.slice( - this.pageSize, - combinedBasicAndSmartAccKeys.length - ) + const accounts: DerivedAccountWithoutNetworkMeta[] = [] const smartAccountsPromises: Promise[] = [] // Replace the parallel getKeys with foreach to prevent issues with Ledger, // which can only handle one request at a time. - for (const [index, smartAccKey] of smartAccKeys.entries()) { + for (const [index, smartAccKey] of retrievedKeys.entries()) { const slot = startIdx + (index + 1) const indexWithOffset = slot - 1 + SMART_ACCOUNT_SIGNER_KEY_DERIVATION_OFFSET @@ -1150,14 +1219,6 @@ export class AccountPickerController extends EventEmitter implements IAccountPic accounts.push(...smartAccounts) - for (const [index, basicAccKey] of basicAccKeys.entries()) { - const slot = startIdx + (index + 1) - // The EOA (basic) account on this slot - const account = getBasicAccount(basicAccKey, this.#alreadyImportedAccounts) - const result = { account, isLinked: false, slot, index: slot - 1 } - accounts.push(result) - } - return accounts } @@ -1250,6 +1311,28 @@ export class AccountPickerController extends EventEmitter implements IAccountPic return sortedAccountsWithNetworksArray } + async #getAndSetAccountsUsedOnNetworks({ + accounts, + requestId, + page + }: { + accounts: DerivedAccountWithoutNetworkMeta[] + requestId: number + page: number + }) { + const accountsWithUsedOn = await this.#getAccountsUsedOnNetworks({ accounts, page }) + + if (this.#isSetPageRequestCancelled(requestId, page)) return + + const accountsWithUsedOnByAddress = new Map( + accountsWithUsedOn.map((account) => [account.account.addr, account]) + ) + this.#derivedAccounts = this.#derivedAccounts.map( + (account) => accountsWithUsedOnByAddress.get(account.account.addr) ?? account + ) + this.emitUpdate() + } + /** * Guard to ensure we only proceed with data that matches the current page and * that the operation hasn't been cancelled via reset(). diff --git a/src/controllers/accounts/accounts.ts b/src/controllers/accounts/accounts.ts index a63bda47f4..193c37c8c9 100644 --- a/src/controllers/accounts/accounts.ts +++ b/src/controllers/accounts/accounts.ts @@ -199,6 +199,8 @@ export class AccountsController extends EventEmitter implements IAccountsControl this.emitUpdate() + let readyNetworks = 0 + await Promise.all( networksToUpdate.map(async (network) => { try { @@ -260,9 +262,14 @@ export class AccountsController extends EventEmitter implements IAccountsControl }) this.#updateProviderIsWorking(network.chainId, false) } finally { + readyNetworks++ this.accountStatesLoadingState[network.chainId.toString()] = undefined } - this.emitUpdate() + + const areAllReady = readyNetworks === networksToUpdate.length + // Prevent spamming updates as users may have dozens of networks and updating + // every tick causes a lot of rerenders in the UI + this.emitUpdate({ throttleMs: areAllReady ? 0 : 200 }) }) ) @@ -302,7 +309,9 @@ export class AccountsController extends EventEmitter implements IAccountsControl this.accounts = getUniqueAccountsArray(nextAccounts) await this.#storage.set('accounts', this.accounts) - this.#onAddAccounts(accounts) + // we add newAccountsNotAddedYet first so the extension selects + // a newly imported account first + this.#onAddAccounts([...newAccountsNotAddedYet, ...newAccountsAlreadyAdded]) // update the state of new accounts. Otherwise, the user needs to restart his extension // eslint-disable-next-line @typescript-eslint/no-floating-promises diff --git a/src/controllers/continuousUpdates/continuousUpdates.test.ts b/src/controllers/continuousUpdates/continuousUpdates.test.ts index e59b24dd05..265b15d313 100644 --- a/src/controllers/continuousUpdates/continuousUpdates.test.ts +++ b/src/controllers/continuousUpdates/continuousUpdates.test.ts @@ -3,12 +3,18 @@ import { Account } from '@/interfaces/account' import { suppressConsole } from '../../../test/helpers/console' import { makeMainController } from '../../../test/helpers/mainController' import { waitForFnToBeCalledAndExecuted } from '../../../test/recurringTimeout' -import { ACTIVITY_REFRESH_INTERVAL } from '../../consts/intervals' +import { + ACTIVITY_REFRESH_INTERVAL, + TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL, + TRENDING_TOKENS_FAILED_UPDATE_INTERVAL, + TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL +} from '../../consts/intervals' import { SubmittedAccountOp } from '../../libs/accountOp/submittedAccountOp' import { SwapProviderParallelExecutor } from '../../services/swapIntegrators/swapProviderParallelExecutor' import wait from '../../utils/wait' import EventEmitter from '../eventEmitter/eventEmitter' import { MainController } from '../main/main' +import { MAX_TRENDING_TOKENS_FAILED_RETRIES } from './continuousUpdates' const accounts: Account[] = [ { @@ -106,6 +112,7 @@ const prepareTest = async () => { await wait(500) }) mainCtrl.updateAccountsOpsStatuses = jest.fn().mockResolvedValue({ newestOpTimestamp: 0 }) + mainCtrl.dapps.updateTrendingTokens = jest.fn().mockResolvedValue(undefined) return { mainCtrl } } @@ -359,4 +366,95 @@ describe('ContinuousUpdatesController intervals', () => { initialFnExecutionsCount + 2 ) }) + + test('backs off the trending interval on failure and recovers on success', async () => { + const { mainCtrl } = await prepareTest() + await waitForContinuousUpdatesCtrlReady(mainCtrl) + + const interval = mainCtrl.continuousUpdates!.updateTrendingTokensInterval + // No view is open, so the interval runs at the inactive cadence. + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL) + + // Next fetch fails → back off to the 1-minute failed-retry cadence. + ;(mainCtrl.dapps.updateTrendingTokens as jest.Mock).mockRejectedValueOnce(new Error('boom')) + await waitForFnToBeCalledAndExecuted(interval) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_FAILED_UPDATE_INTERVAL) + + // The following fetch succeeds → recover the inactive cadence (still no view open). + await waitForFnToBeCalledAndExecuted(interval) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL) + }) + + test('gives up on the fast trending retry cadence after too many consecutive failures', async () => { + const { restore } = suppressConsole() + const { mainCtrl } = await prepareTest() + await waitForContinuousUpdatesCtrlReady(mainCtrl) + + const interval = mainCtrl.continuousUpdates!.updateTrendingTokensInterval + const updateSpy = mainCtrl.dapps.updateTrendingTokens as jest.Mock + updateSpy.mockRejectedValue(new Error('boom')) + updateSpy.mockClear() + + // Failures below the max keep the 1-minute failed-retry cadence. + for (let i = 0; i < MAX_TRENDING_TOKENS_FAILED_RETRIES - 1; i++) { + await waitForFnToBeCalledAndExecuted(interval) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_FAILED_UPDATE_INTERVAL) + } + + // The last allowed retry fails too → stop hammering the API and fall back to the normal + // (inactive, as no view is open) cadence. + await waitForFnToBeCalledAndExecuted(interval) + expect(updateSpy).toHaveBeenCalledTimes(MAX_TRENDING_TOKENS_FAILED_RETRIES) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL) + + // A later success recovers the same normal cadence and resets the retry counter, so the fast + // cadence is used again on the next failure. + updateSpy.mockResolvedValueOnce(undefined) + await waitForFnToBeCalledAndExecuted(interval) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL) + await waitForFnToBeCalledAndExecuted(interval) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_FAILED_UPDATE_INTERVAL) + + restore() + }) + + test('switches the trending interval to the active cadence while a view is open', async () => { + const { mainCtrl } = await prepareTest() + await waitForContinuousUpdatesCtrlReady(mainCtrl) + + const interval = mainCtrl.continuousUpdates!.updateTrendingTokensInterval + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL) + + jest.spyOn(interval, 'restart') + mainCtrl.ui.addView({ id: '1', type: 'popup', currentRoute: 'dashboard', isReady: true }) + await jest.advanceTimersByTimeAsync(0) + expect(interval.restart).toHaveBeenCalledWith({ + timeout: TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL, + runImmediately: true + }) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL) + + mainCtrl.ui.removeView('1') + await jest.advanceTimersByTimeAsync(0) + expect(interval.currentTimeout).toBe(TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL) + }) + + test('skips the trending fetch when the last update is still fresh', async () => { + const { mainCtrl } = await prepareTest() + await waitForContinuousUpdatesCtrlReady(mainCtrl) + + const interval = mainCtrl.continuousUpdates!.updateTrendingTokensInterval + const updateSpy = mainCtrl.dapps.updateTrendingTokens as jest.Mock + // Pretend trending was just refreshed. Resolved on every read, as the fake timers advance the + // clock by the whole interval while waiting for the scheduled run below. + jest + .spyOn(mainCtrl.dapps, 'trendingTokensUpdatedAt', 'get') + .mockImplementation(() => Date.now()) + updateSpy.mockClear() + + // Becoming active triggers an immediate refresh, but the freshness guard skips the fetch. + mainCtrl.ui.addView({ id: '1', type: 'popup', currentRoute: 'dashboard', isReady: true }) + await waitForFnToBeCalledAndExecuted(interval) + expect(updateSpy).not.toHaveBeenCalled() + }) }) diff --git a/src/controllers/continuousUpdates/continuousUpdates.ts b/src/controllers/continuousUpdates/continuousUpdates.ts index c3a84d941a..081645c23e 100644 --- a/src/controllers/continuousUpdates/continuousUpdates.ts +++ b/src/controllers/continuousUpdates/continuousUpdates.ts @@ -6,7 +6,10 @@ import { ACCOUNT_STATE_STAND_BY_INTERVAL, ACTIVE_EXTENSION_PORTFOLIO_UPDATE_INTERVAL, ACTIVITY_REFRESH_INTERVAL, - INACTIVE_EXTENSION_PORTFOLIO_UPDATE_INTERVAL + INACTIVE_EXTENSION_PORTFOLIO_UPDATE_INTERVAL, + TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL, + TRENDING_TOKENS_FAILED_UPDATE_INTERVAL, + TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL } from '../../consts/intervals' import { IEventEmitterRegistryController } from '../../interfaces/eventEmitter' import { Hex } from '../../interfaces/hex' @@ -21,6 +24,10 @@ import EventEmitter from '../eventEmitter/eventEmitter' /* eslint-disable @typescript-eslint/no-floating-promises */ +/** How many consecutive failed trending tokens fetches are retried at the fast failed-retry +cadence before falling back to the normal one, so a long API outage isn't retried every minute. */ +export const MAX_TRENDING_TOKENS_FAILED_RETRIES = 5 + export class ContinuousUpdatesController extends EventEmitter { #main: IMainController @@ -67,6 +74,14 @@ export class ContinuousUpdatesController extends EventEmitter { #safeGlobalMessageInterval: IRecurringTimeout + #updateTrendingTokensInterval: IRecurringTimeout + + get updateTrendingTokensInterval() { + return this.#updateTrendingTokensInterval + } + + #trendingTokensFailedRetries = 0 + // Holds the initial load promise, so that one can wait until it completes initialLoadPromise?: Promise | undefined @@ -158,6 +173,35 @@ export class ContinuousUpdatesController extends EventEmitter { 'resolveConfirmedSafeMessages' ) + // Trending tokens poll frequently only while the extension is active and back off to a long + // cadence otherwise. On becoming active we refresh immediately, but the freshness guard in + // #updateTrendingTokens skips the fetch when the last update is still recent. + this.#updateTrendingTokensInterval = new RecurringTimeout( + this.#updateTrendingTokens.bind(this), + TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL, + this.emitError.bind(this), + 'updateTrendingTokensInterval' + ) + + this.#main.ui.uiEvent.on('addView', () => { + const isAlreadyActive = + this.#updateTrendingTokensInterval.currentTimeout === TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL + + if (this.#main.ui.views.length === 1 && !isAlreadyActive) { + this.#updateTrendingTokensInterval.restart({ + timeout: TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL, + runImmediately: true + }) + } + }) + this.#main.ui.uiEvent.on('removeView', () => { + if (!this.#main.ui.views.length) { + this.#updateTrendingTokensInterval.restart({ + timeout: TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL + }) + } + }) + this.#main.swapAndBridge.onUpdate(() => { if (this.#main.swapAndBridge.signAccountOpController?.broadcastStatus === 'SUCCESS') { this.#accountStateLatestInterval.restart() @@ -218,6 +262,7 @@ export class ContinuousUpdatesController extends EventEmitter { this.#accountStateLatestInterval.start() this.#safeGlobalTxnInterval.start() this.#safeGlobalMessageInterval.start() + this.#updateTrendingTokensInterval.start({ runImmediately: true }) } async #updatePortfolio() { @@ -229,6 +274,58 @@ export class ContinuousUpdatesController extends EventEmitter { }) } + async #updateTrendingTokens() { + await this.initialLoadPromise + await this.#main.dapps.initialLoadPromise + + // Skip if the last successful update is still fresh — prevents redundant requests when the + // background reloads multiple times within a short period (e.g. service worker wake-ups) and + // makes "refresh on becoming active" a no-op unless the data is older than the current cadence. + const updatedAt = this.#main.dapps.trendingTokensUpdatedAt + const timeSinceLastUpdate = updatedAt ? Date.now() - updatedAt : null + if ( + updatedAt && + timeSinceLastUpdate !== null && + timeSinceLastUpdate < this.#updateTrendingTokensInterval.currentTimeout + ) { + return + } + + try { + await this.#main.dapps.updateTrendingTokens() + this.#trendingTokensFailedRetries = 0 + + // Recover the normal cadence after a previously failed fetch bumped it down. + if ( + this.#updateTrendingTokensInterval.currentTimeout === TRENDING_TOKENS_FAILED_UPDATE_INTERVAL + ) { + this.#updateTrendingTokensInterval.updateTimeout({ + timeout: this.#getTrendingTokensNormalInterval() + }) + } + } catch (err) { + this.#trendingTokensFailedRetries += 1 + const hasExhaustedRetries = + this.#trendingTokensFailedRetries >= MAX_TRENDING_TOKENS_FAILED_RETRIES + + // Back off to the fast failed-retry cadence, but give up on it once the retries are + // exhausted (the API is likely down for a while), then rethrow so RecurringTimeout's + // onError handler reports it (with level 'silent', i.e. no user-facing toast). + this.#updateTrendingTokensInterval.updateTimeout({ + timeout: hasExhaustedRetries + ? this.#getTrendingTokensNormalInterval() + : TRENDING_TOKENS_FAILED_UPDATE_INTERVAL + }) + throw err + } + } + + #getTrendingTokensNormalInterval() { + return this.#main.ui.views.length + ? TRENDING_TOKENS_ACTIVE_UPDATE_INTERVAL + : TRENDING_TOKENS_INACTIVE_UPDATE_INTERVAL + } + async #updateAccountsOpsStatuses() { try { await this.initialLoadPromise diff --git a/src/controllers/dapps/dapps.test.ts b/src/controllers/dapps/dapps.test.ts index a85387f354..8687f569a8 100644 --- a/src/controllers/dapps/dapps.test.ts +++ b/src/controllers/dapps/dapps.test.ts @@ -5,7 +5,7 @@ import wait from '@/utils/wait' import { expect } from '@jest/globals' import { suppressConsole } from '../../../test/helpers/console' -import { makeDapp } from '../../../test/helpers/dapps' +import { blacklistedDapp, makeDapp } from '../../../test/helpers/dapps' import { makeMainController } from '../../../test/helpers/mainController' import { Session } from '../../classes/session' import { predefinedDapps } from '../../consts/dapps/dapps' @@ -16,6 +16,60 @@ import { IStorageController } from '../../interfaces/storage' import { DappConnectRequest } from '../../interfaces/userRequest' import { PhishingController } from '../phishing/phishing' +const TRENDING_TOKENS_URL = 'https://cena.ambire.com/api/v3/trending/' + +// Two valid entries plus one invalid (no price) to exercise normalization + filtering. +// Mirrors the trimmed endpoint shape: a { tokens: [...] } wrapper of minimal coin objects +// with flat USD fields, a top-level homepage and pre-deduped exchange ids. +const mockTrending = { + tokens: [ + { + id: 'bitcoin', + name: 'Bitcoin', + symbol: 'BTC', + market_cap_rank: 1, + image: { + thumb: 'https://example.com/btc-thumb.png', + small: 'https://example.com/btc-small.png', + large: 'https://example.com/btc-large.png' + }, + asset_platform_id: 'ethereum', + contract_address: '0xbtc', + platforms: { ethereum: '0xbtc' }, + decimals: { ethereum: 8 }, + homepage: ['https://bitcoin.org'], + exchanges: ['binance', 'coinbase'], + usd: 65000.5, + usd_24h_change: 1.23, + usd_market_cap: 1200000000, + usd_24h_vol: 45000000, + usd_fully_diluted_valuation: 1300000000, + total_supply: 21000000, + description: { en: 'The first cryptocurrency.' } + }, + { + id: 'ethereum', + name: 'Ethereum', + symbol: 'ETH', + market_cap_rank: 2, + // No `large` → the normalizer falls back to `small`. + image: { small: 'https://example.com/eth-small.png' }, + usd: 3200, + usd_24h_change: -2.5, + usd_market_cap: 400000000, + usd_24h_vol: 20000000, + description: null + }, + // Invalid: missing price → must be filtered out by normalizeTrendingTokens. + { + id: 'no-price-coin', + name: 'No Price Coin', + symbol: 'NPC', + market_cap_rank: 999 + } + ] +} + const prepareTest = async ( storageInit?: (storageController: IStorageController) => Promise, getMockFetchImplementation?: (url: string, ...args: any) => Promise @@ -42,6 +96,14 @@ const prepareTest = async ( } } + if (url === TRENDING_TOKENS_URL) { + return { + ok: true, + status: 200, + json: async () => mockTrending + } + } + return fetch(url, ...args) }) } @@ -323,6 +385,7 @@ describe('DappsController', () => { expect(controller.getDappVerificationBanner([aave.url])).toEqual({ id: DAPP_VERIFICATION_BANNER_IDS.LOADING, type: 'warning', + title: 'Safety check in progress', text: "We're still verifying the app. Please wait, or make sure you trust it before signing requests: AAVE" }) } finally { @@ -349,6 +412,7 @@ describe('DappsController', () => { expect(controller.getDappVerificationBanner([aave.url])).toEqual({ id: DAPP_VERIFICATION_BANNER_IDS.LOADING, type: 'warning', + title: 'Safety check in progress', text: "We're still verifying the app. Please wait, or make sure you trust it before signing requests: AAVE" }) @@ -357,6 +421,7 @@ describe('DappsController', () => { expect(controller.getDappVerificationBanner([aave.url])).toEqual({ id: DAPP_VERIFICATION_BANNER_IDS.FAILED_TO_GET_OR_UNKNOWN, type: 'warning', + title: "App couldn't be verified", text: "We couldn't verify the app. Make sure you trust it before signing requests: AAVE" }) } finally { @@ -379,6 +444,7 @@ describe('DappsController', () => { expect(controller.getDappVerificationBanner([aave.url])).toEqual({ id: DAPP_VERIFICATION_BANNER_IDS.FAILED_TO_GET_OR_UNKNOWN, type: 'warning', + title: "App couldn't be verified", text: "We couldn't verify the app. Make sure you trust it before signing requests: AAVE" }) } finally { @@ -401,6 +467,7 @@ describe('DappsController', () => { expect(controller.getDappVerificationBanner([aave.url])).toEqual({ id: DAPP_VERIFICATION_BANNER_IDS.BLACKLISTED, type: 'error', + title: 'Potentially harmful app', text: "This app didn't pass our safety check. Proceed at your own risk: AAVE" }) } finally { @@ -430,6 +497,7 @@ describe('DappsController', () => { expect(controller.getDappVerificationBanner([verifiedCustomDapp.url])).toEqual({ id: DAPP_VERIFICATION_BANNER_IDS.NOT_IN_CATALOG, type: 'warning', + title: "App not in Ambire's catalog", text: 'App is not on the default Ambire App Catalog. Make sure you trust it before signing requests: Custom Dapp' }) } finally { @@ -939,6 +1007,144 @@ describe('DappsController', () => { }) }) + // A fully-qualified hostname ("my-dapp.vercel.app.") loads the identical site as its + // dotted-free form, so it must resolve to the same dApp identity everywhere - otherwise + // appending one dot turns a flagged dApp into an unknown one. + describe('fully-qualified (trailing dot) dApp urls', () => { + test('a suspicious hosting dApp visited with a trailing dot still shows the SUSPICIOUS_HOSTING banner', async () => { + const vercelDapp = makeDapp({ + id: 'my-dapp.vercel.app', + name: 'Fake Uniswap on Vercel', + url: 'https://my-dapp.vercel.app', + blacklisted: 'LOADING', + isCustom: true + }) + + const { controller } = await prepareTest(async (storageCtrl) => { + await storageCtrl.set('dappsV2', [...predefinedDapps, vercelDapp]) + await storageCtrl.set('lastDappsUpdateVersion', '1.0.0') + }) + await controller.fetchAndUpdatePromise + + const banner = controller.getDappVerificationBanner(['https://my-dapp.vercel.app./claim']) + expect(banner?.id).toBe(DAPP_VERIFICATION_BANNER_IDS.SUSPICIOUS_HOSTING) + expect(banner?.type).toBe('warning') + }) + + test('a BLACKLISTED dApp visited with a trailing dot still shows the BLACKLISTED banner', async () => { + const { controller } = await prepareTest(async (storageCtrl) => { + await storageCtrl.set('dappsV2', [...predefinedDapps, blacklistedDapp]) + await storageCtrl.set('lastDappsUpdateVersion', '1.0.0') + }) + await controller.fetchAndUpdatePromise + + expect(controller.getDappVerificationBanner(['https://blacklisted-dapp.com./'])?.id).toBe( + DAPP_VERIFICATION_BANNER_IDS.BLACKLISTED + ) + }) + + test('a suspicious hosting top frame written with a trailing dot still poisons the frame context', async () => { + const { controller } = await prepareTest(async (storageCtrl) => { + await storageCtrl.set('dappsV2', predefinedDapps) + await storageCtrl.set('lastDappsUpdateVersion', 'test-version') + }) + await controller.fetchAndUpdatePromise + + const aave = controller.dapps.find((d) => d.name === 'AAVE')! + expect(aave.blacklisted).toBe('VERIFIED') + + const aaveSession = new Session({ + tabId: 90, + windowId: 1, + url: aave.url, + frameId: 3, + topFrameUrl: 'https://sites.google.com./view/fake-aave' + }) + controller.dappSessions[aaveSession.sessionId] = aaveSession + + const banner = controller.getDappVerificationBanner([aave.url], { + sessionId: aaveSession.sessionId + }) + expect(banner?.id).toBe(DAPP_VERIFICATION_BANNER_IDS.SUSPICIOUS_HOSTING) + }) + + test('getOrCreateDappSession reuses the session of the dotted-free url', async () => { + const { controller } = await prepareTest() + + const session = await controller.getOrCreateDappSession({ + tabId: 91, + windowId: 1, + url: 'https://app.aave.com', + frameId: 0, + topFrameUrl: 'https://app.aave.com' + }) + const dottedSession = await controller.getOrCreateDappSession({ + tabId: 91, + windowId: 1, + url: 'https://app.aave.com./', + frameId: 0, + topFrameUrl: 'https://app.aave.com./' + }) + + expect(dottedSession).toBe(session) + expect(dottedSession.id).toBe('app.aave.com') + }) + + test('a session created from a dotted url keeps the origin the browser reported', async () => { + const { controller } = await prepareTest() + + const session = await controller.getOrCreateDappSession({ + tabId: 92, + windowId: 1, + url: 'https://app.aave.com./', + frameId: 0, + topFrameUrl: 'https://app.aave.com./' + }) + + // The identity is canonical, while the origin stays byte-identical to the page's own + // `location.origin` - platform messengers compare against it before delivering data. + expect(session.id).toBe('app.aave.com') + expect(session.origin).toBe('https://app.aave.com.') + }) + + test('canonicalizes stored dApp ids on load, dropping a trailing-dot duplicate', async () => { + const canonicalDapp = makeDapp({ + id: 'my-dapp.vercel.app', + name: 'Canonical', + url: 'https://my-dapp.vercel.app', + blacklisted: 'SUSPICIOUS_HOSTING' + }) + const dottedDuplicate = makeDapp({ + id: 'my-dapp.vercel.app.', + name: 'Trailing dot duplicate', + url: 'https://my-dapp.vercel.app./', + blacklisted: 'VERIFIED', + isConnected: true, + connectedSources: ['injected'] + }) + const dottedOnly = makeDapp({ + id: 'other-dapp.vercel.app.', + name: 'Trailing dot only', + url: 'https://other-dapp.vercel.app./', + blacklisted: 'VERIFIED' + }) + + const { controller } = await prepareTest(async (storageCtrl) => { + await storageCtrl.set('dappsV2', [dottedDuplicate, canonicalDapp, dottedOnly]) + await storageCtrl.set('lastDappsUpdateVersion', '1.0.0') + }) + + // The canonical record wins over the duplicate, together with its reviewed permissions. + expect(controller.getDapp('my-dapp.vercel.app')!.name).toBe('Canonical') + expect(controller.getDapp('my-dapp.vercel.app')!.isConnected).toBe(false) + expect(controller.getDapp('my-dapp.vercel.app.')).toBeUndefined() + + // A record that only exists in dotted form is renamed, so it stays reachable. + expect(controller.getDapp('other-dapp.vercel.app')!.name).toBe('Trailing dot only') + expect(controller.getDapp('other-dapp.vercel.app.')).toBeUndefined() + }) + }) + describe('per-dapp account scoping', () => { const ADDR_1 = '0x16c81367c30c71d6B712355255A07FCe8fd3b5bB' const ADDR_2 = '0xa07D75aacEFd11b425AF7181958F0F85c312f143' @@ -1680,6 +1886,84 @@ describe('DappsController', () => { }) }) + describe('disconnectWcSessionByTopic', () => { + const wcDapp = (): Dapp => + makeDapp({ + id: 'aave.com', + name: 'Aave', + url: 'https://aave.com', + isCustom: false, + isConnected: true, + chainId: 1, + blacklisted: 'VERIFIED' + }) + + const prepareConnectedWcDapp = async () => { + const { controller } = await prepareTest(async (storageCtrl) => { + await storageCtrl.set('dappsV2', predefinedDapps) + await storageCtrl.set('lastDappsUpdateVersion', '1.0.0') + }) + await controller.addDapp(wcDapp(), 'wc') + + return controller + } + + test('revokes the wc connection when the dapp terminates its only session', async () => { + const controller = await prepareConnectedWcDapp() + await controller.getOrCreateDappSession({ + tabId: 1000001, + url: 'https://aave.com', + wcTopic: 'topic-a' + }) + + controller.disconnectWcSessionByTopic('topic-a') + + expect(controller.getDappSessionByWcTopic('topic-a')).toBeUndefined() + const stored = controller.getDapp('aave.com')! + expect(stored.connectedSources).toEqual([]) + expect(stored.isConnected).toBe(false) + // A later pairing must ask the user for approval again instead of auto-connecting. + expect(controller.hasPermission('aave.com', 'wc')).toBe(false) + }) + + test('keeps the wc connection while another session of the same dapp remains', async () => { + const controller = await prepareConnectedWcDapp() + await controller.getOrCreateDappSession({ + tabId: 1000001, + url: 'https://aave.com', + wcTopic: 'topic-a' + }) + await controller.getOrCreateDappSession({ + tabId: 1000002, + url: 'https://aave.com', + wcTopic: 'topic-b' + }) + + controller.disconnectWcSessionByTopic('topic-a') + + expect(controller.getDappSessionByWcTopic('topic-b')).toBeDefined() + expect(controller.getDapp('aave.com')!.connectedSources).toEqual(['wc']) + expect(controller.hasPermission('aave.com', 'wc')).toBe(true) + }) + + test('leaves the injected connection intact', async () => { + const controller = await prepareConnectedWcDapp() + await controller.addDapp(wcDapp(), 'injected') + await controller.getOrCreateDappSession({ + tabId: 1000001, + url: 'https://aave.com', + wcTopic: 'topic-a' + }) + + controller.disconnectWcSessionByTopic('topic-a') + + const stored = controller.getDapp('aave.com')! + expect(stored.connectedSources).toEqual(['injected']) + expect(stored.isConnected).toBe(true) + }) + + }) + describe('disconnectAllDapps', () => { const connectedNonCustomDapp = (id: string): Dapp => makeDapp({ @@ -1880,4 +2164,133 @@ describe('DappsController', () => { expect(stored.chainId).toBe(1) }) }) + + describe('trending tokens', () => { + const seedStorage = async (storageCtrl: IStorageController) => { + await storageCtrl.set('dappsV2', predefinedDapps) + await storageCtrl.set('lastDappsUpdateVersion', '1.0.0') + } + + test('fetches, normalizes and filters invalid entries on load', async () => { + const { controller } = await prepareTest(seedStorage) + await controller.updateTrendingTokens() + + // The third fixture entry has no price and must be dropped. + expect(controller.trendingTokens).toHaveLength(2) + + const btc = controller.trendingTokens.find((tt) => tt.symbol === 'BTC')! + expect(btc.id).toBe('bitcoin') + expect(btc.priceUSD).toBe(65000.5) + expect(btc.priceChange24hUSD).toBe(1.23) + expect(btc.marketCapRank).toBe(1) + expect(btc.icon).toBe('https://example.com/btc-large.png') // prefers `large` + expect(btc.marketCapUSD).toBe(1200000000) + expect(btc.totalVolumeUSD).toBe(45000000) + expect(btc.fullyDilutedValuationUSD).toBe(1300000000) + expect(btc.totalSupply).toBe(21000000) + expect(btc.description).toBe('The first cryptocurrency.') + expect(btc.address).toBe('0xbtc') + expect(btc.platformId).toBe('ethereum') + expect(btc.decimals).toBe(8) + expect(btc.website).toBe('https://bitcoin.org') + // Exchange ids come pre-deduped from the server and pass through as-is. + expect(btc.exchangeIds).toEqual(['binance', 'coinbase']) + + const eth = controller.trendingTokens.find((tt) => tt.symbol === 'ETH')! + expect(eth.priceChange24hUSD).toBe(-2.5) + expect(eth.description).toBeNull() // description was null + expect(eth.address).toBeNull() // no contract/platform provided + expect(eth.exchangeIds).toEqual([]) + }) + + test('persists fetched trending tokens to storage', async () => { + const { controller, mainCtrl } = await prepareTest(seedStorage) + await controller.updateTrendingTokens() + + const stored = await mainCtrl.storage.get('trending', { updatedAt: 0, tokens: [] }) + expect(stored.tokens).toHaveLength(2) + expect(typeof stored.updatedAt).toBe('number') + expect(stored.updatedAt).toBeGreaterThan(0) + }) + + test('restores trending tokens from storage on init', async () => { + const seeded = { + id: 'solana', + name: 'Solana', + symbol: 'SOL', + icon: 'https://example.com/sol.png', + priceUSD: 150, + priceChange24hUSD: 5, + marketCapRank: 5, + description: 'A fast L1.', + address: null, + platformId: null, + decimals: null, + marketCapUSD: 70000000, + totalVolumeUSD: 3000000, + fullyDilutedValuationUSD: null, + totalSupply: null, + website: null, + exchangeIds: [] + } + const { controller } = await prepareTest(async (storageCtrl) => { + await seedStorage(storageCtrl) + // A fresh updatedAt keeps the skip-if-fresh guard from refetching over the seed. + await storageCtrl.set('trending', { updatedAt: Date.now(), tokens: [seeded] }) + }) + + expect(controller.trendingTokens).toEqual([seeded]) + }) + + test('keeps trending empty and throws when the fetch fails', async () => { + const { restore } = suppressConsole() + const { controller } = await prepareTest(seedStorage, async (url: string, ...args: any) => { + if (url === 'https://api.llama.fi/protocols') + return { ok: true, status: 200, json: async () => mockDapps } + if (url === 'https://api.llama.fi/v2/chains') + return { ok: true, status: 200, json: async () => mockChains } + if (url === TRENDING_TOKENS_URL) return { ok: false, status: 500, json: async () => ({}) } + return fetch(url, ...args) + }) + + // Throws so the ContinuousUpdatesController scheduler can back off its retry cadence. + await expect(controller.updateTrendingTokens()).rejects.toThrow() + expect(controller.trendingTokens).toEqual([]) + restore() + }) + + // Integration test - the trending endpoint is NOT mocked here (unlike in the tests above), so + // a change in the response structure that the normalizer and the UI can't handle fails here + // instead of silently reaching users as an empty or broken trending list. + test('normalizes the response of the real trending tokens endpoint', async () => { + const { controller } = await prepareTest(seedStorage, async (url: string, ...args: any) => { + if (url === 'https://api.llama.fi/protocols') + return { ok: true, status: 200, json: async () => mockDapps } + if (url === 'https://api.llama.fi/v2/chains') + return { ok: true, status: 200, json: async () => mockChains } + return fetch(url, ...args) + }) + + await controller.updateTrendingTokens() + + expect(controller.trendingTokens.length).toBeGreaterThan(0) + + controller.trendingTokens.forEach((token) => { + expect(token.id.length).toBeGreaterThan(0) + expect(token.name.length).toBeGreaterThan(0) + expect(token.symbol.length).toBeGreaterThan(0) + expect(Number.isFinite(token.priceUSD)).toBe(true) + }) + + // The market data and the icon the trending list and the token-details screen render must + // arrive for the first (most trending) token at the very least. + const [topToken] = controller.trendingTokens + expect(topToken!.icon.startsWith('http')).toBe(true) + expect(topToken!.priceUSD).toBeGreaterThan(0) + expect(topToken!.priceChange24hUSD).not.toBeNull() + expect(topToken!.marketCapUSD).not.toBeNull() + expect(topToken!.totalVolumeUSD).not.toBeNull() + expect(topToken!.marketCapRank).not.toBeNull() + }, 40000) + }) }) diff --git a/src/controllers/dapps/dapps.ts b/src/controllers/dapps/dapps.ts index 3549d99dd6..fadfe20171 100644 --- a/src/controllers/dapps/dapps.ts +++ b/src/controllers/dapps/dapps.ts @@ -27,7 +27,9 @@ import { GetCurrentDappRes, HasUnverifiedDappsRes, IDappsController, - RecentDappEntry + RawTrendingToken, + RecentDappEntry, + TrendingToken } from '../../interfaces/dapp' import { IEventEmitterRegistryController } from '../../interfaces/eventEmitter' import { Fetch } from '../../interfaces/fetch' @@ -43,8 +45,11 @@ import { getDappIdFromUrl, getDappNameFromId, getDomainFromUrl, + getNormalizedHostnameFromUrl, modifyDappPropsIfNeeded, normalizeDappConnection, + normalizeHostname, + normalizeTrendingTokens, sortDapps, unifyDefiLlamaDappUrl } from '../../libs/dapps/helpers' @@ -52,6 +57,8 @@ import { networkChainIdToHex } from '../../libs/networks/networks' import { fetchWithTimeout } from '../../utils/fetch' import EventEmitter from '../eventEmitter/eventEmitter' +const TRENDING_TOKENS_URL = 'https://cena.ambire.com/api/v3/trending/' + const mergeSource = ( existing: ConnectionSource[] | undefined, source: ConnectionSource @@ -102,6 +109,20 @@ export class DappsController extends EventEmitter implements IDappsController { #selectedAccount: ISelectedAccountController + #trendingTokens: TrendingToken[] = [] + + #trendingTokensUpdatedAt: number | null = null + + get trendingTokens(): TrendingToken[] { + return this.#trendingTokens + } + + // Timestamp of the last successful trending-tokens fetch. Read by the + // ContinuousUpdatesController which owns the trending update interval. + get trendingTokensUpdatedAt(): number | null { + return this.#trendingTokensUpdatedAt + } + get shouldRetryFetchAndUpdate() { return this.#shouldRetryFetchAndUpdate } @@ -247,14 +268,28 @@ export class DappsController extends EventEmitter implements IDappsController { await this.#networks.initialLoadPromise await this.#selectedAccount.initialLoadPromise - const [storedDapps, storedRecentDapps] = await Promise.all([ + const [storedDapps, storedRecentDapps, storedTrending] = await Promise.all([ this.#storage.get('dappsV2', predefinedDapps), - this.#storage.get('recentDapps', [] as RecentDappEntry[]) + this.#storage.get('recentDapps', [] as RecentDappEntry[]), + this.#storage.get('trending', { updatedAt: 0, tokens: [] as TrendingToken[] }) ]) // Normalize on read so a drifted record (e.g. isConnected: true but connectedSources: []) // can't show a dapp as connected in the UI while permission checks force a reconnect. - this.#dapps = new Map(storedDapps.map((d) => [d.id, normalizeDappConnection(d)])) + // Ids are canonicalized as well: a record stored before trailing-dot normalization + // ("my-dapp.vercel.app.") is unreachable by any lookup, so it would linger as an orphan + // entry in the UI while its permissions can never be resolved again. + this.#dapps = new Map() + storedDapps.forEach((dapp) => { + const id = normalizeHostname(dapp.id) + // The canonical record wins over its trailing-dot duplicate - it is the one every lookup + // resolves to, and its permissions are the ones the user reviewed for it. + if (id !== dapp.id && this.#dapps.has(id)) return + + this.#dapps.set(id, normalizeDappConnection({ ...dapp, id })) + }) this.#recentDapps = storedRecentDapps + this.#trendingTokens = storedTrending.tokens + this.#trendingTokensUpdatedAt = storedTrending.updatedAt || null void this.fetchAndUpdateDapps() } @@ -511,6 +546,34 @@ export class DappsController extends EventEmitter implements IDappsController { void this.#storage.set('dappsV2', Array.from(this.#dapps.values())) } + /** + * Fetches, normalizes and persists the trending tokens. Throws on a failed fetch or a + * malformed response so the caller can react (e.g. back off its retry cadence). The update + * interval and its lifecycle are owned by the ContinuousUpdatesController. + */ + async updateTrendingTokens() { + await this.initialLoadPromise + + const res = await fetchWithTimeout(this.#fetch, TRENDING_TOKENS_URL, {}, 30000) + + if (!res.ok || res.status !== 200) { + throw new Error(`Failed to update trending tokens (status: ${res.status}, url: ${res.url})`) + } + + const json = await res.json() + const raw: RawTrendingToken[] = json?.tokens + if (!Array.isArray(raw)) { + throw new Error('Trending tokens response does not contain a tokens array') + } + + this.#trendingTokens = normalizeTrendingTokens(raw) + const updatedAt = Date.now() + this.#trendingTokensUpdatedAt = updatedAt + this.emitUpdate() + + await this.#storage.set('trending', { updatedAt, tokens: this.#trendingTokens }) + } + async #createDappSession(initProps: SessionInitProps) { await this.initialLoadPromise const dappSession = new Session(initProps) @@ -615,6 +678,31 @@ export class DappsController extends EventEmitter implements IDappsController { } } + /** + * Removes a WalletConnect session terminated by the dApp and, once none of its WC sessions + * remain, revokes the `'wc'` connection so the next pairing asks for approval again. + */ + disconnectWcSessionByTopic = (wcTopic: string) => { + const session = this.getDappSessionByWcTopic(wcTopic) + if (!session) return + + const dappId = session.id + delete this.dappSessions[session.sessionId] + this.emitUpdate() + + const hasOtherWcSession = Object.values(this.dappSessions).some( + (s) => s.id === dappId && !!s.wcTopic + ) + if (hasOtherWcSession) return + + const dapp = this.#dapps.get(dappId) + if (!dapp?.connectedSources?.includes('wc')) return + + this.updateDapp(dappId, { + connectedSources: dapp.connectedSources.filter((source) => source !== 'wc') + }) + } + broadcastDappSessionEvent = async ( ev: any, data?: any, @@ -1277,7 +1365,10 @@ export class DappsController extends EventEmitter implements IDappsController { : this.initialLoadPromise ? 'LOADING' : (contextStatus ?? intrinsic), - name: dapp?.name || new URL(url).hostname + // The canonical hostname, so the banner names the site the user believes they are on + // instead of the fully-qualified spelling a phishing page may navigate to. Falls back to + // the raw url for inputs the URL parser rejects, which must not throw here. + name: dapp?.name || getNormalizedHostnameFromUrl(url) || url } }) @@ -1314,6 +1405,7 @@ export class DappsController extends EventEmitter implements IDappsController { return { id: DAPP_VERIFICATION_BANNER_IDS.BLACKLISTED, type: 'error', + title: 'Potentially harmful app', text: withOptionalDappNames( "This app didn't pass our safety check. Proceed at your own risk.", blacklistedDappNames @@ -1329,6 +1421,7 @@ export class DappsController extends EventEmitter implements IDappsController { return { id: DAPP_VERIFICATION_BANNER_IDS.SUSPICIOUS_HOSTING, type: 'warning', + title: 'Suspicious app hosting', text: withOptionalDappNames( 'This app is hosted on a shared platform commonly used for phishing. Be careful - do not sign unless you are certain you trust it.', '' // We explicitly don't append the dApp name, because here what matters is the suspicious hosting URL, but showing the name could confuse the user, so we simply don't @@ -1342,6 +1435,7 @@ export class DappsController extends EventEmitter implements IDappsController { return { id: DAPP_VERIFICATION_BANNER_IDS.LOADING, type: 'warning', + title: 'Safety check in progress', text: withOptionalDappNames( "We're still verifying the app. Please wait, or make sure you trust it before signing requests.", loadingDappNames @@ -1357,6 +1451,7 @@ export class DappsController extends EventEmitter implements IDappsController { return { id: DAPP_VERIFICATION_BANNER_IDS.FAILED_TO_GET_OR_UNKNOWN, type: 'warning', + title: "App couldn't be verified", text: withOptionalDappNames( "We couldn't verify the app. Make sure you trust it before signing requests.", failedToVerifyDappNames @@ -1372,6 +1467,7 @@ export class DappsController extends EventEmitter implements IDappsController { return { id: DAPP_VERIFICATION_BANNER_IDS.NOT_IN_CATALOG, type: 'warning', + title: "App not in Ambire's catalog", text: withOptionalDappNames( 'App is not on the default Ambire App Catalog. Make sure you trust it before signing requests.', notInCatalogDappNames @@ -1390,6 +1486,7 @@ export class DappsController extends EventEmitter implements IDappsController { recentDapps: this.recentDapps, categories: this.categories, isReady: this.isReady, + trendingTokens: this.trendingTokens, shouldRetryFetchAndUpdate: this.shouldRetryFetchAndUpdate, retryFetchAndUpdateInterval: this.retryFetchAndUpdateInterval, retryFetchAndUpdateAttempts: this.retryFetchAndUpdateAttempts diff --git a/src/controllers/domains/domains.test.ts b/src/controllers/domains/domains.test.ts index 6012e948f0..1aa4d95f15 100644 --- a/src/controllers/domains/domains.test.ts +++ b/src/controllers/domains/domains.test.ts @@ -4,11 +4,11 @@ import { expect, jest } from '@jest/globals' import { suppressConsole } from '../../../test/helpers/console' import { networks } from '../../consts/networks' +import { Network } from '../../interfaces/network' // Must match the direct-file import used in domains.ts (not the barrel) — jest.spyOn // can't intercept calls through a different module instance, and tslib 2's `export *` // getter-only bindings make the barrel un-spyable anyway. import * as ensDomainsModule from '../../services/ensDomains/ensDomains' -import { Network } from '../../interfaces/network' import { NameResolver, NameServiceId } from '../../services/nameResolvers' import { getRpcProvider } from '../../services/provider' import { @@ -289,6 +289,101 @@ describe('Domains', () => { resolveENSDomainSpy.mockRestore() } }) + it('stores the resolved name normalized while keying coordination state by the raw input', async () => { + const controller = new DomainsController({ + providers: { ['1']: {} as any }, + featureFlags: makeFeatureFlags(true), + getNetwork: allNetworksEnabled + }) + const resolvedAddress = getAddress('0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045') + const resolveENSDomainSpy = jest + .spyOn(ensDomainsModule, 'resolveENSDomain') + .mockResolvedValue({ address: resolvedAddress, avatar: null, expiry: null }) + + try { + await controller.resolveDomain({ domain: 'VITALIK.ETH' }) + + // The stored name is the resolver-normalized (ENSIP-15) form. + expect(controller.domains[resolvedAddress]!.names.ens).toBe('vitalik.eth') + // The resolver is queried with the normalized name. + expect(resolveENSDomainSpy).toHaveBeenCalledWith( + expect.objectContaining({ domain: 'vitalik.eth' }) + ) + // Coordination state stays keyed by the raw input the UI dispatched (no UI-side normalizer). + expect(controller.domainToAddresses['VITALIK.ETH']?.address).toBe(resolvedAddress) + expect(controller.domainToAddresses['VITALIK.ETH']?.type).toBe('ens') + } finally { + resolveENSDomainSpy.mockRestore() + } + }) + it('routes an uppercase Namoshi TLD (.BTC) to the Namoshi service (case-insensitive matching)', async () => { + const controller = new DomainsController({ + providers: { ['1']: {} as any, ['4114']: {} as any }, + featureFlags: makeFeatureFlags(true), + getNetwork: allNetworksEnabled + }) + const resolvedAddress = getAddress('0x4f0b5579136f88135572010276c2a4a884729e7b') + const resolveENSDomainSpy = jest + .spyOn(ensDomainsModule, 'resolveENSDomain') + .mockResolvedValue({ address: resolvedAddress, avatar: null, expiry: null }) + + try { + // Without case-insensitive matching 'SATOSHI.BTC'.endsWith('.btc') is false and it would fall back to ENS. + await controller.resolveDomain({ domain: 'SATOSHI.BTC' }) + + expect(controller.domainToAddresses['SATOSHI.BTC']?.type).toBe('namoshi') + expect(controller.domainToAddresses['SATOSHI.BTC']?.address).toBe(resolvedAddress) + // Stored name is normalized for the owning service. + expect(controller.domains[resolvedAddress]!.names.namoshi).toBe('satoshi.btc') + } finally { + resolveENSDomainSpy.mockRestore() + } + }) + it('marks an un-normalizable domain as failed without querying any resolver', async () => { + const controller = new DomainsController({ + providers: { ['1']: {} as any }, + featureFlags: makeFeatureFlags(true), + getNetwork: allNetworksEnabled + }) + const resolveENSDomainSpy = jest.spyOn(ensDomainsModule, 'resolveENSDomain') + + try { + await controller.resolveDomain({ domain: 'has space.eth' }) + + expect(resolveENSDomainSpy).not.toHaveBeenCalled() + expect(controller.domainToAddresses['has space.eth']).toBeUndefined() + } finally { + resolveENSDomainSpy.mockRestore() + } + }) + it('keeps case-variant inputs as separate raw cache keys, each stored normalized', async () => { + const controller = new DomainsController({ + providers: { ['1']: {} as any }, + featureFlags: makeFeatureFlags(true), + getNetwork: allNetworksEnabled + }) + const resolvedAddress = getAddress('0xf9D6794F16CDbdC5b4873AEdeF4dC69d8D5edcaD') + const resolveENSDomainSpy = jest + .spyOn(ensDomainsModule, 'resolveENSDomain') + .mockResolvedValue({ address: resolvedAddress, avatar: null, expiry: null }) + + try { + // 'Vitalik.eth' and 'vitalik.eth' are distinct raw keys, so each resolves independently. This is + // the accepted trade-off of keeping coordination keys raw (no UI-side normalizer to dedup them). + await Promise.all([ + controller.resolveDomain({ domain: 'Vitalik.eth' }), + controller.resolveDomain({ domain: 'vitalik.eth' }) + ]) + + expect(resolveENSDomainSpy).toHaveBeenCalledTimes(2) + expect(controller.domainToAddresses['Vitalik.eth']?.address).toBe(resolvedAddress) + expect(controller.domainToAddresses['vitalik.eth']?.address).toBe(resolvedAddress) + // Both variants store the same normalized name. + expect(controller.domains[resolvedAddress]!.names.ens).toBe('vitalik.eth') + } finally { + resolveENSDomainSpy.mockRestore() + } + }) it(`reverse lookup should expire after ${ PERSIST_DOMAIN_FOR_IN_MS / 1000 / 60 } min, if the lookup succeeds (the happy case)`, async () => { @@ -886,6 +981,7 @@ describe('Domains', () => { label: id, capabilities: { reverse: true, avatar: false, expiry: false }, matches: () => false, + normalize: (domain: string) => domain, resolve: async () => null, reverse, getAvatar: async () => null, diff --git a/src/controllers/domains/domains.ts b/src/controllers/domains/domains.ts index 562fe718de..9633071fac 100644 --- a/src/controllers/domains/domains.ts +++ b/src/controllers/domains/domains.ts @@ -15,11 +15,11 @@ import { IFeatureFlagsController } from '../../interfaces/featureFlags' import { Network } from '../../interfaces/network' import { RPCProviders } from '../../interfaces/provider' import { IStorageController } from '../../interfaces/storage' +import { IVerificationController } from '../../interfaces/verification' // Import directly from ensDomains.ts, not the barrel (./index.ts). With tslib 2, // `export *` re-exports become getter-only bindings that jest.spyOn cannot override, // so domains.test.ts must spy on this same direct-file module instance. import { NameExpiry, ReverseLookupResult } from '../../services/ensDomains/ensDomains' -import { IVerificationController } from '../../interfaces/verification' import { DEFAULT_RESOLVERS, getPrimaryName, @@ -359,6 +359,12 @@ export class DomainsController extends EventEmitter implements IDomainsControlle return } + const name = resolver.normalize(domain) + if (!name) { + await this.#setResolveDomainFailure(domain, new Error(`Invalid domain name: ${domain}`)) + return + } + if ( this.resolveDomainsStatus[domain] === 'LOADING' || this.resolveDomainsStatus[domain] === 'RESOLVED' @@ -388,11 +394,11 @@ export class DomainsController extends EventEmitter implements IDomainsControlle } await resolver - .resolve(domain, this.#context()) + .resolve(name, this.#context()) .then(async (result) => { if (result?.address) { // Verify before caching, so a mismatch throws into the catch and nothing bad is persisted. - const isVerified = await this.#verifyResolvedAddress(resolver, domain, result.address) + const isVerified = await this.#verifyResolvedAddress(resolver, name, result.address) if (isVerified) this.verifiedDomainsStatus[domain] = 'VERIFIED' this.domainToAddresses[domain] = { @@ -403,7 +409,7 @@ export class DomainsController extends EventEmitter implements IDomainsControlle address: result.address, avatar: result.avatar, expiry: result.expiry, - domain, + domain: name, type: resolver.id }) } diff --git a/src/controllers/estimation/estimation.ts b/src/controllers/estimation/estimation.ts index 63afb7fc23..462aa883e0 100644 --- a/src/controllers/estimation/estimation.ts +++ b/src/controllers/estimation/estimation.ts @@ -121,7 +121,8 @@ export class EstimationController extends EventEmitter { account, accountState, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) // Take the fee tokens from two places: the user's tokens and his gasTank diff --git a/src/controllers/eventEmitter/eventEmitter.test.ts b/src/controllers/eventEmitter/eventEmitter.test.ts index 10bfabf339..c4a51acd71 100644 --- a/src/controllers/eventEmitter/eventEmitter.test.ts +++ b/src/controllers/eventEmitter/eventEmitter.test.ts @@ -104,6 +104,108 @@ describe('EventEmitter', () => { expect(mockErrorCallback).not.toHaveBeenCalled() restore() }) + describe('throttled emitUpdate', () => { + const THROTTLE_MS = 100 + + beforeEach(() => { + jest.useFakeTimers() + }) + + afterEach(() => { + jest.clearAllTimers() + jest.useRealTimers() + }) + + const emit = (options?: { throttleMs?: number }) => + // Accessing the protected method for testing + (eventEmitter as any).emitUpdate(options) + + it('should emit immediately on the first emit', () => { + const cb = jest.fn() + eventEmitter.onUpdate(cb) + + emit({ throttleMs: THROTTLE_MS }) + + expect(cb).toHaveBeenCalledTimes(1) + }) + + it('should coalesce multiple throttled emits within the window into one trailing emit', () => { + const cb = jest.fn() + eventEmitter.onUpdate(cb) + + emit({ throttleMs: THROTTLE_MS }) // leading -> 1 + emit({ throttleMs: THROTTLE_MS }) + emit({ throttleMs: THROTTLE_MS }) + expect(cb).toHaveBeenCalledTimes(1) + + jest.advanceTimersByTime(THROTTLE_MS) // trailing -> 2 + expect(cb).toHaveBeenCalledTimes(2) + + // Window is reopened after a trailing emit; with nothing pending it must + // not fire again and must release the timer. + jest.advanceTimersByTime(THROTTLE_MS) + expect(cb).toHaveBeenCalledTimes(2) + }) + + it('should keep throttling a continuous stream to one emit per window', () => { + const cb = jest.fn() + eventEmitter.onUpdate(cb) + + emit({ throttleMs: THROTTLE_MS }) // leading -> 1 + emit({ throttleMs: THROTTLE_MS }) + jest.advanceTimersByTime(THROTTLE_MS) // trailing -> 2 + + emit({ throttleMs: THROTTLE_MS }) + jest.advanceTimersByTime(THROTTLE_MS) // trailing -> 3 + + expect(cb).toHaveBeenCalledTimes(3) + }) + + it('should flush a pending throttled emit immediately on a plain emitUpdate', () => { + const cb = jest.fn() + eventEmitter.onUpdate(cb) + + emit({ throttleMs: THROTTLE_MS }) // leading -> 1 + emit({ throttleMs: THROTTLE_MS }) // pending trailing + + emit() // plain emit supersedes the pending trailing -> 2 + expect(cb).toHaveBeenCalledTimes(2) + + // The superseded trailing emit must not fire afterwards + jest.advanceTimersByTime(THROTTLE_MS) + expect(cb).toHaveBeenCalledTimes(2) + }) + + it('should cancel a pending throttled emit on forceEmitUpdate', async () => { + const cb = jest.fn() + eventEmitter.onUpdate(cb) + + emit({ throttleMs: THROTTLE_MS }) // leading -> 1 + emit({ throttleMs: THROTTLE_MS }) // pending trailing + + const forced = eventEmitter.forceEmitUpdate() + await jest.advanceTimersByTimeAsync(1) // forceEmitUpdate awaits wait(1) + await forced + expect(cb).toHaveBeenCalledTimes(2) + + jest.advanceTimersByTime(THROTTLE_MS) + expect(cb).toHaveBeenCalledTimes(2) + }) + + it('should not fire a pending throttled emit after destroy', () => { + const cb = jest.fn() + eventEmitter.onUpdate(cb) + + emit({ throttleMs: THROTTLE_MS }) // leading -> 1 + emit({ throttleMs: THROTTLE_MS }) // pending trailing + + eventEmitter.destroy() + + jest.advanceTimersByTime(THROTTLE_MS) + expect(cb).toHaveBeenCalledTimes(1) + }) + }) + describe('EventEmitter memory leak with nested controllers', () => { suppressConsoleBeforeEach() const externalClosure = {} diff --git a/src/controllers/eventEmitter/eventEmitter.ts b/src/controllers/eventEmitter/eventEmitter.ts index 4e53139a9d..737c8537ba 100644 --- a/src/controllers/eventEmitter/eventEmitter.ts +++ b/src/controllers/eventEmitter/eventEmitter.ts @@ -32,6 +32,11 @@ export default class EventEmitter { #errors: ErrorRef[] = [] + // Trailing throttle used by `emitUpdate({ throttleMs })` + #throttleTimeout: ReturnType | null = null + + #hasTrailingUpdate = false + statuses: Statuses = {} /** @@ -87,20 +92,73 @@ export default class EventEmitter { * normal batching may skip intermediate states and only emit the first and last ones. */ async forceEmitUpdate() { + // An immediate emit supersedes any pending throttled update + this.#clearThrottle() + // Bypassing background batching on the same tick await wait(1) // Passing `true` to the cb will bypass React batching + this.#doEmit(true) + } - for (const i of this.#callbacksWithId) i.cb(true) + #doEmit(forceEmit?: boolean) { + for (const i of this.#callbacksWithId) i.cb(forceEmit) - for (const cb of this.#callbacks) cb(true) + for (const cb of this.#callbacks) cb(forceEmit) } - protected emitUpdate() { - for (const i of this.#callbacksWithId) i.cb() + #clearThrottle() { + if (this.#throttleTimeout !== null) { + clearTimeout(this.#throttleTimeout) + this.#throttleTimeout = null + } + this.#hasTrailingUpdate = false + } - for (const cb of this.#callbacks) cb() + #openThrottleWindow(throttleMs: number) { + this.#throttleTimeout = setTimeout(() => { + // Keep throttling as long as updates keep arriving; stop once a window + // passes with nothing pending so an idle controller holds no timer. + if (this.#hasTrailingUpdate) { + this.#hasTrailingUpdate = false + this.#doEmit() + this.#openThrottleWindow(throttleMs) + } else { + this.#throttleTimeout = null + } + }, throttleMs) + } + + /** + * Emits an update to all subscribers. + * + * Pass `throttleMs` for high-frequency background updates (e.g. portfolio + * ticks) that don't need to reach the UI on every single change. The first + * emit fires immediately, while further throttled emits within the + * window are coalesced into a single trailing emit that carries the latest + * state. A plain `emitUpdate()` or `forceEmitUpdate()` in the meantime flushes + * the pending update instantly, so user interactions are never delayed. + */ + protected emitUpdate(options?: { throttleMs?: number }) { + const throttleMs = options?.throttleMs ?? 0 + + if (throttleMs <= 0) { + // An immediate emit supersedes any pending throttled update + this.#clearThrottle() + this.#doEmit() + return + } + + // Leading edge: emit now and open the throttle window + if (this.#throttleTimeout === null) { + this.#doEmit() + this.#openThrottleWindow(throttleMs) + return + } + + // Within the window: defer to a single trailing emit + this.#hasTrailingUpdate = true } /** @@ -130,9 +188,9 @@ export default class EventEmitter { * and the controller updates its own state), use `emitUpdate()` or `forceEmitUpdate()`. */ protected propagateUpdate(forceEmit?: boolean) { - for (const i of this.#callbacksWithId) i.cb(forceEmit) - - for (const cb of this.#callbacks) cb(forceEmit) + // An immediate emit supersedes any pending throttled update + this.#clearThrottle() + this.#doEmit(forceEmit) } /** True when this controller's debug logging is toggled on. */ @@ -285,6 +343,7 @@ export default class EventEmitter { * clearing all callbacks and errors. */ destroy() { + this.#clearThrottle() this.unregisterFromRegistry() this.#callbacks = [] this.#callbacksWithId = [] diff --git a/src/controllers/keystore/keystore.test.ts b/src/controllers/keystore/keystore.test.ts index 7792d603f3..99acdc0c03 100644 --- a/src/controllers/keystore/keystore.test.ts +++ b/src/controllers/keystore/keystore.test.ts @@ -431,6 +431,66 @@ describe('KeystoreController', () => { }) }) +describe('KeystoreController recovery phrase backup state', () => { + let keystoreCtrl: IKeystoreController + + beforeEach(async () => { + const storageCtrl = new StorageController(produceMemoryStore()) + const uiCtrl = new UiController({ uiManager }) + keystoreCtrl = new KeystoreController('default', storageCtrl, keystoreSigners, uiCtrl) + await keystoreCtrl.addSecret('password', pass, '', false) + await keystoreCtrl.unlockWithSecret('password', pass) + }) + + test('a generated phrase is flagged as not backed up', async () => { + const tempSeed = await keystoreCtrl.generateTempSeed({}) + expect(tempSeed.notBackedUp).toBe(true) + + await keystoreCtrl.persistTempSeed() + + expect(keystoreCtrl.seeds.length).toBe(1) + expect(keystoreCtrl.seeds[0]!.notBackedUp).toBe(true) + }) + + test('an imported phrase is not flagged, as the user has already seen it', async () => { + await keystoreCtrl.addTempSeed({ + seed: process.env.SEED, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE + }) + await keystoreCtrl.persistTempSeed() + + expect(keystoreCtrl.seeds[0]!.notBackedUp).toBeFalsy() + }) + + test('markSeedAsBackedUp clears the flag and does not touch other seeds', async () => { + await keystoreCtrl.generateTempSeed({}) + await keystoreCtrl.persistTempSeed() + await keystoreCtrl.addTempSeed({ + seed: process.env.SEED, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + notBackedUp: true + }) + await keystoreCtrl.persistTempSeed() + + expect(keystoreCtrl.seeds.length).toBe(2) + const [firstSeed, secondSeed] = keystoreCtrl.seeds + + await keystoreCtrl.markSeedAsBackedUp(secondSeed!.id) + + expect(keystoreCtrl.seeds.find((s) => s.id === secondSeed!.id)?.notBackedUp).toBe(false) + expect(keystoreCtrl.seeds.find((s) => s.id === firstSeed!.id)?.notBackedUp).toBe(true) + }) + + test('markSeedAsBackedUp is a no-op for an unknown phrase id', async () => { + await keystoreCtrl.generateTempSeed({}) + await keystoreCtrl.persistTempSeed() + + await keystoreCtrl.markSeedAsBackedUp('does-not-exist') + + expect(keystoreCtrl.seeds[0]!.notBackedUp).toBe(true) + }) +}) + describe('import/export with pub key test', () => { const wallet = ethers.Wallet.createRandom() let keystore2: IKeystoreController diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 76c33a8ab2..1f3564d62d 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -662,15 +662,18 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl } get seeds() { - return this.#keystoreSeeds.map(({ id, label, hdPathTemplate, seedPassphrase }) => ({ - id, - label: label || 'Unnamed Recovery Seed', - hdPathTemplate, - withPassphrase: !!seedPassphrase - })) + return this.#keystoreSeeds.map( + ({ id, label, hdPathTemplate, seedPassphrase, notBackedUp }) => ({ + id, + label: label || 'Unnamed Recovery Seed', + hdPathTemplate, + withPassphrase: !!seedPassphrase, + notBackedUp + }) + ) } - async addTempSeed({ seed, seedPassphrase, hdPathTemplate }: KeystoreTempSeed) { + async addTempSeed({ seed, seedPassphrase, hdPathTemplate, notBackedUp }: KeystoreTempSeed) { const validHdPath = DERIVATION_OPTIONS.some((o) => o.value === hdPathTemplate) if (!validHdPath) throw new EmittableError({ @@ -680,18 +683,30 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl error: new Error('keystore: hd path to temp seed incorrect') }) - this.#tempSeed = { seed, seedPassphrase, hdPathTemplate } + this.#tempSeed = { seed, seedPassphrase, hdPathTemplate, notBackedUp } this.emitUpdate() } - async generateTempSeed({ extraEntropy }: { extraEntropy?: string }) { + /** + * Generates a brand new phrase without revealing it to the user. It is marked as + * not backed up, so the app can prompt for a backup once the account holds funds. + * Returns the generated temp seed, so the background can derive accounts from it + * without the phrase ever reaching the UI. + */ + async generateTempSeed({ extraEntropy }: { extraEntropy?: string }): Promise { const entropyGenerator = new EntropyGenerator() const seed = entropyGenerator.generateRandomMnemonic(12, extraEntropy || '').phrase - this.#tempSeed = { seed, hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE } + this.#tempSeed = { + seed, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + notBackedUp: true + } this.emitUpdate() + + return this.#tempSeed } deleteTempSeed(shouldUpdate = true) { @@ -707,7 +722,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl this.emitUpdate() } - async #addSeed({ seed, seedPassphrase, hdPathTemplate }: KeystoreTempSeed) { + async #addSeed({ seed, seedPassphrase, hdPathTemplate, notBackedUp }: KeystoreTempSeed) { await this.initialLoadPromise if (this.#mainKey === null) @@ -740,7 +755,8 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl seedPassphrase: seedPassphrase ? await encryptWithKey(this.#mainKey, new TextEncoder().encode(seedPassphrase)) : null, - hdPathTemplate + hdPathTemplate, + notBackedUp } this.#keystoreSeeds.push(newEntry) @@ -757,13 +773,15 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl async #updateSeed({ id, label, - hdPathTemplate + hdPathTemplate, + notBackedUp }: { id: KeystoreSeed['id'] label?: KeystoreSeed['label'] hdPathTemplate?: KeystoreSeed['hdPathTemplate'] + notBackedUp?: KeystoreSeed['notBackedUp'] }) { - if (!label && !hdPathTemplate) return + if (!label && !hdPathTemplate && notBackedUp === undefined) return const keystoreSeed = this.#keystoreSeeds.find((s) => s.id === id) if (!keystoreSeed) return @@ -772,6 +790,8 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl if (hdPathTemplate) keystoreSeed.hdPathTemplate = hdPathTemplate + if (notBackedUp !== undefined) keystoreSeed.notBackedUp = notBackedUp + const updatedKeystoreSeeds = this.#keystoreSeeds.map((s) => s.id === keystoreSeed.id ? keystoreSeed : s ) @@ -785,13 +805,27 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl async updateSeed({ id, label, - hdPathTemplate + hdPathTemplate, + notBackedUp }: { id: KeystoreSeed['id'] label?: KeystoreSeed['label'] hdPathTemplate?: KeystoreSeed['hdPathTemplate'] + notBackedUp?: KeystoreSeed['notBackedUp'] }) { - await this.withStatus('updateSeed', () => this.#updateSeed({ id, label, hdPathTemplate }), true) + await this.withStatus( + 'updateSeed', + () => this.#updateSeed({ id, label, hdPathTemplate, notBackedUp }), + true + ) + } + + /** + * Called once the user has gone through the reveal + confirm-words backup flow, + * so the app stops prompting them to back this phrase up. + */ + async markSeedAsBackedUp(id: KeystoreSeed['id']) { + await this.updateSeed({ id, notBackedUp: false }) } async deleteSeed(id: KeystoreSeed['id']) { diff --git a/src/controllers/main/main.test.ts b/src/controllers/main/main.test.ts index 50b0c8b4c3..2ef05d7605 100644 --- a/src/controllers/main/main.test.ts +++ b/src/controllers/main/main.test.ts @@ -184,6 +184,88 @@ describe('Main Controller ', () => { ) }) + describe('updateAccounts', () => { + const getAccount = (addr: string) => ({ + addr, + associatedKeys: [], + initialPrivileges: [], + creation: accounts[0]!.creation, + preferences: { label: DEFAULT_ACCOUNT_LABEL, pfp: addr } + }) + + test('adds selected accounts and removes deselected imported accounts', async () => { + const accountToKeep = getAccount('0x1111111111111111111111111111111111111111') + const accountToRemove = getAccount('0x2222222222222222222222222222222222222222') + const accountToAdd = getAccount('0x3333333333333333333333333333333333333333') + const { mainCtrl } = await makeMainController(async (storageCtrl) => { + await storageCtrl.set('accounts', [accountToKeep, accountToRemove]) + }) + await mainCtrl.initialLoadPromise + + await mainCtrl.updateAccounts({ + accountsToAdd: [accountToAdd], + accountAddressesToRemove: [accountToRemove.addr] + }) + + expect(mainCtrl.accounts.accounts.map((account) => account.addr)).toEqual([ + accountToKeep.addr, + accountToAdd.addr + ]) + }) + + test('ignores removal requests for accounts that are not imported', async () => { + const importedAccount = getAccount('0x1111111111111111111111111111111111111111') + const { mainCtrl } = await makeMainController(async (storageCtrl) => { + await storageCtrl.set('accounts', [importedAccount]) + }) + await mainCtrl.initialLoadPromise + + await mainCtrl.updateAccounts({ + accountsToAdd: [], + accountAddressesToRemove: ['0x2222222222222222222222222222222222222222'] + }) + + expect(mainCtrl.accounts.accounts.map((account) => account.addr)).toEqual([ + importedAccount.addr + ]) + }) + + test('does not remove an account that is also being added', async () => { + const importedAccount = getAccount('0x1111111111111111111111111111111111111111') + const { mainCtrl } = await makeMainController(async (storageCtrl) => { + await storageCtrl.set('accounts', [importedAccount]) + }) + await mainCtrl.initialLoadPromise + + await mainCtrl.updateAccounts({ + accountsToAdd: [importedAccount], + accountAddressesToRemove: [importedAccount.addr] + }) + + expect(mainCtrl.accounts.accounts.map((account) => account.addr)).toEqual([ + importedAccount.addr + ]) + }) + + test('selects the first newly imported account when imported accounts come first', async () => { + const importedAccount = getAccount('0x1111111111111111111111111111111111111111') + const firstNewAccount = getAccount('0x2222222222222222222222222222222222222222') + const secondNewAccount = getAccount('0x3333333333333333333333333333333333333333') + const { mainCtrl } = await makeMainController(async (storageCtrl) => { + await storageCtrl.set('accounts', [importedAccount]) + await storageCtrl.set('selectedAccount', importedAccount.addr) + }) + await mainCtrl.initialLoadPromise + + await mainCtrl.updateAccounts({ + accountsToAdd: [importedAccount, firstNewAccount, secondNewAccount], + accountAddressesToRemove: [] + }) + + expect(mainCtrl.selectedAccount.account?.addr).toBe(firstNewAccount.addr) + }) + }) + test('should check if network features get displayed correctly for ethereum', async () => { const eth = controller.networks.networks.find((n) => n.chainId === 1n)! expect(eth?.features.length).toBe(3) diff --git a/src/controllers/main/main.ts b/src/controllers/main/main.ts index edf6a87ebd..996430b6fe 100644 --- a/src/controllers/main/main.ts +++ b/src/controllers/main/main.ts @@ -111,8 +111,14 @@ import { SquidAPI } from '@/services/squid/api' import { SwapProviderParallelExecutor } from '@/services/swapIntegrators/swapProviderParallelExecutor' import { UniswapAPI } from '@/services/uniswap/api' import { getHdPathFromTemplate } from '@/utils/hdPath' +import { generateUuid } from '@/utils/uuid' import wait from '@/utils/wait' +type AccountsUpdate = { + accountsToAdd: Account[] + accountAddressesToRemove: Account['addr'][] +} + export class MainController extends EventEmitter implements IMainController { #storageAPI: Storage @@ -919,14 +925,12 @@ export class MainController extends EventEmitter implements IMainController { await this.keystore.addKeys(this.accountPicker.readyToAddKeys.internal) await this.keystore.addKeysExternallyStored(this.accountPicker.readyToAddKeys.external) - if (this.accountPicker.readyToRemoveAccounts) { - for (const acc of this.accountPicker.readyToRemoveAccounts) { - await this.#removeAccount(acc.addr) - } - } - - // Add accounts as a final step, because some of the next steps check if accounts have keys. - await this.accounts.addAccounts(this.accountPicker.readyToAddAccounts) + await this.#updateAccounts({ + accountsToAdd: this.accountPicker.readyToAddAccounts, + accountAddressesToRemove: this.accountPicker.readyToRemoveAccounts.map( + (account) => account.addr + ) + }) } async commonHandlerForBroadcastSuccess({ @@ -1413,6 +1417,49 @@ export class MainController extends EventEmitter implements IMainController { ) } + async #handleAccountPickerInitNfc( + NfcKeyIterator: any, // TODO: KeyIterator type mismatch + payload: { extendedPublicKey: string; hdPath: string } + ) { + try { + const nfcCtrl = this.#externalSignerControllers.nfc + + if (!nfcCtrl) { + const message = + 'Could not initialize connection with your card. Please try again later or contact Ambire support.' + throw new EmittableError({ message, level: 'major', error: new Error(message) }) + } + + const keyIterator = new NfcKeyIterator({ controller: nfcCtrl }) + // Initialize the iterator from the extended public key exported by the card + // before the AccountPicker init, so it can derive addresses on its own + // (the card is tapped only once, not per address). + keyIterator.initFromExportedKey(payload) + + // v1 accounts have never supported NFC cards, so there is nothing to look + // for on the relayer (same reasoning as the QR flow). + this.accountPicker.setInitParams({ + keyIterator, + hdPathTemplate: keyIterator.hdPathTemplate, + pageSize: 5, + shouldAddNextAccountAutomatically: false, + shouldSearchForLinkedAccounts: false + }) + } catch (error: any) { + const message = error?.message || 'Could not import the card account. Please try again.' + throw new EmittableError({ message, level: 'major', error }) + } + } + + async handleAccountPickerInitNfc( + NfcKeyIterator: any, // TODO: KeyIterator type mismatch + payload: { extendedPublicKey: string; hdPath: string } + ) { + await this.withStatus('handleAccountPickerInitNfc', async () => + this.#handleAccountPickerInitNfc(NfcKeyIterator, payload) + ) + } + async updateAccountsOpsStatuses() { await this.initialLoadPromise @@ -1586,6 +1633,30 @@ export class MainController extends EventEmitter implements IMainController { await this.withStatus('removeAccount', async () => this.#removeAccount(address)) } + async #updateAccounts({ accountsToAdd, accountAddressesToRemove }: AccountsUpdate) { + const addressesToAdd = new Set(accountsToAdd.map((account) => account.addr.toLowerCase())) + const importedAddresses = new Set( + this.accounts.accounts.map((account) => account.addr.toLowerCase()) + ) + const uniqueAddressesToRemove = Array.from( + new Map(accountAddressesToRemove.map((address) => [address.toLowerCase(), address])).values() + ) + + for (const address of uniqueAddressesToRemove) { + const normalizedAddress = address.toLowerCase() + if (!importedAddresses.has(normalizedAddress) || addressesToAdd.has(normalizedAddress)) + continue + await this.#removeAccount(address) + } + + // Add accounts as a final step, because some of the next steps check if accounts have keys. + await this.accounts.addAccounts(accountsToAdd) + } + + async updateAccounts(accountsUpdate: AccountsUpdate) { + await this.withStatus('updateAccounts', async () => this.#updateAccounts(accountsUpdate)) + } + async reloadSelectedAccount(options?: { chainIds?: bigint[] maxDataAgeMs?: number @@ -1865,7 +1936,7 @@ export class MainController extends EventEmitter implements IMainController { if (openBenzin) { const benzinUserRequest: BenzinUserRequest = { - id: new Date().getTime(), + id: generateUuid(), kind: 'benzin', meta, dappPromises: [] @@ -1963,6 +2034,25 @@ export class MainController extends EventEmitter implements IMainController { await this.accountPicker.setInitParams({ keyIterator, hdPathTemplate }) } + /** + * Creates a brand new recovery phrase and prepares the account picker with it, all + * in the background. The phrase is never sent to the UI - the user gets prompted to + * write it down later, once the account holds funds. + */ + async accountPickerSetInitParamsFromNewSeed({ extraEntropy }: { extraEntropy?: string }) { + await this.withStatus( + 'accountPickerSetInitParamsFromNewSeed', + async () => { + const tempSeed = await this.keystore.generateTempSeed({ extraEntropy }) + + await this.accountPickerSetInitParamsFromPrivateKeyOrSeedPhrase({ + privKeyOrSeed: tempSeed.seed + }) + }, + true + ) + } + // includes the getters in the stringified instance toJSON() { return { diff --git a/src/controllers/phishing/phishing.test.ts b/src/controllers/phishing/phishing.test.ts index 8e355a7c33..75d5743c2f 100644 --- a/src/controllers/phishing/phishing.test.ts +++ b/src/controllers/phishing/phishing.test.ts @@ -128,6 +128,51 @@ describe('PhishingController', () => { expect(controller.getDomainBlacklistedStatus('https://sites.google.com')).toBe('BLACKLISTED') }) + test('getDomainBlacklistedStatus returns SUSPICIOUS_HOSTING for a fully-qualified host with a trailing dot', async () => { + const { controller } = await prepareTest(['some-other-phishing-site.com']) + + // "my-dapp.vercel.app." loads the identical site as "my-dapp.vercel.app" - DNS, TLS and the + // browser treat the trailing root-label dot as the same host - so it must not slip through. + expect(controller.getDomainBlacklistedStatus('https://my-dapp.vercel.app./')).toBe( + 'SUSPICIOUS_HOSTING' + ) + expect(controller.getDomainBlacklistedStatus('https://example.web.app./claim')).toBe( + 'SUSPICIOUS_HOSTING' + ) + expect(controller.getDomainBlacklistedStatus('https://sites.google.com./view/fake')).toBe( + 'SUSPICIOUS_HOSTING' + ) + }) + + test('getDomainBlacklistedStatus flags a trailing-dot host regardless of casing, www. or repeated dots', async () => { + const { controller } = await prepareTest(['some-other-phishing-site.com']) + + expect(controller.getDomainBlacklistedStatus('https://My-Dapp.Vercel.App./')).toBe( + 'SUSPICIOUS_HOSTING' + ) + expect(controller.getDomainBlacklistedStatus('https://www.my-dapp.vercel.app./')).toBe( + 'SUSPICIOUS_HOSTING' + ) + expect(controller.getDomainBlacklistedStatus('https://my-dapp.vercel.app../')).toBe( + 'SUSPICIOUS_HOSTING' + ) + // The URL parser maps the ideographic full stop to a regular dot, trailing one included. + expect(controller.getDomainBlacklistedStatus('https://my-dapp。vercel。app。/')).toBe( + 'SUSPICIOUS_HOSTING' + ) + }) + + test('getDomainBlacklistedStatus keeps not flagging parent domains written with a trailing dot', async () => { + const { controller } = await prepareTest(['some-other-phishing-site.com']) + + expect(controller.getDomainBlacklistedStatus('https://google.com./')).not.toBe( + 'SUSPICIOUS_HOSTING' + ) + expect(controller.getDomainBlacklistedStatus('https://vercel.com./')).not.toBe( + 'SUSPICIOUS_HOSTING' + ) + }) + test('updateDomainsBlacklistedStatus callback receives SUSPICIOUS_HOSTING for all suspicious hosting domains', async () => { const { controller } = await prepareTest() const results: Record = {} @@ -142,4 +187,56 @@ describe('PhishingController', () => { } }) }) + + describe('fully-qualified (trailing dot) hostnames', () => { + test('getDomainBlacklistedStatus returns BLACKLISTED for a host-level phishing DB entry visited with a trailing dot', async () => { + const { controller } = await prepareTest(['example.web.app']) + + expect(controller.getDomainBlacklistedStatus('https://example.web.app')).toBe('BLACKLISTED') + expect(controller.getDomainBlacklistedStatus('https://example.web.app./')).toBe( + 'BLACKLISTED' + ) + expect(controller.getDomainBlacklistedStatus('https://example.web.app./claim?ref=1')).toBe( + 'BLACKLISTED' + ) + }) + + test('getDomainBlacklistedStatus returns BLACKLISTED for an apex phishing DB entry and its subdomains visited with a trailing dot', async () => { + const { controller } = await prepareTest(['foourmemez.com']) + + expect(controller.getDomainBlacklistedStatus('https://foourmemez.com./')).toBe('BLACKLISTED') + expect(controller.getDomainBlacklistedStatus('https://claim.foourmemez.com./')).toBe( + 'BLACKLISTED' + ) + }) + + test('getDomainBlacklistedStatus matches an internationalized phishing DB entry written in unicode with a trailing dot', async () => { + // The DB stores punycode, which is also what the URL parser produces for a unicode host. + const { controller } = await prepareTest(['xn--e1afmkfd.xn--90ae']) + + expect(controller.getDomainBlacklistedStatus('https://пример.бг./')).toBe('BLACKLISTED') + expect(controller.getDomainBlacklistedStatus('https://xn--e1afmkfd.xn--90ae./')).toBe( + 'BLACKLISTED' + ) + }) + + test('getDomainBlacklistedStatus returns VERIFIED for an unrelated host with a trailing dot', async () => { + const { controller } = await prepareTest(['example.web.app']) + + expect(controller.getDomainBlacklistedStatus('https://rewards.ambire.com./')).toBe('VERIFIED') + }) + + test('updateDomainsBlacklistedStatus keys the callback by the canonical dApp id', async () => { + const { controller } = await prepareTest(['some-other-phishing-site.com']) + const results: Record = {} + + await controller.updateDomainsBlacklistedStatus( + ['https://example.web.app./claim'], + (statuses) => Object.assign(results, statuses) + ) + + expect(results['example.web.app']).toBe('SUSPICIOUS_HOSTING') + expect(results['example.web.app.']).toBeUndefined() + }) + }) }) diff --git a/src/controllers/phishing/phishing.ts b/src/controllers/phishing/phishing.ts index 405edf9e39..b286f023fb 100644 --- a/src/controllers/phishing/phishing.ts +++ b/src/controllers/phishing/phishing.ts @@ -14,7 +14,7 @@ import { Fetch } from '../../interfaces/fetch' import { BlacklistedStatus, IPhishingController } from '../../interfaces/phishing' import { IStorageController } from '../../interfaces/storage' import { IUiController } from '../../interfaces/ui' -import { getDappIdFromUrl } from '../../libs/dapps/helpers' +import { getDappIdFromUrl, getNormalizedHostnameFromUrl } from '../../libs/dapps/helpers' import { fetchWithTimeout } from '../../utils/fetch' import EventEmitter from '../eventEmitter/eventEmitter' @@ -38,6 +38,9 @@ const PHISHING_ACTIVE_VIEW_TYPES = new Set(['request-window', 'popup', 'tab']) * * 1. Intrinsic status — the dApp's own domain, resolved by getDomainBlacklistedStatus(). * Priority: BLACKLISTED (phishing DB) > SUSPICIOUS_HOSTING (this list) > VERIFIED. + * Both lookups are string comparisons, so they run on the canonical hostname produced by + * getNormalizedHostnameFromUrl()/getDappIdFromUrl() — never on a raw URL hostname, which keeps + * the trailing dot of a fully-qualified host and would miss every entry in both lists. * * 2. Frame context — if a dApp is loaded as an iframe inside a tab whose top-level document is * on a SUSPICIOUS_HOSTING or BLACKLISTED domain, #getFrameContextStatus() returns @@ -156,12 +159,12 @@ export const SUSPICIOUS_HOSTING_DOMAINS = [ ] function isSuspiciousHostingDomain(url: string): boolean { - try { - const { hostname } = new URL(url) - return SUSPICIOUS_HOSTING_DOMAINS.some((d) => hostname === d || hostname.endsWith(`.${d}`)) - } catch { - return false - } + // The canonical hostname, so a fully-qualified host ("my-dapp.vercel.app.") is matched against + // the list just like the form the user believes they are on. + const hostname = getNormalizedHostnameFromUrl(url) + if (hostname === null) return false + + return SUSPICIOUS_HOSTING_DOMAINS.some((d) => hostname === d || hostname.endsWith(`.${d}`)) } export class PhishingController extends EventEmitter implements IPhishingController { diff --git a/src/controllers/portfolio/portfolio.ts b/src/controllers/portfolio/portfolio.ts index 320d0584ec..d6e875a02b 100644 --- a/src/controllers/portfolio/portfolio.ts +++ b/src/controllers/portfolio/portfolio.ts @@ -1759,7 +1759,8 @@ export class PortfolioController acc, networkState, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) return baseAcc.getNonceId() } @@ -1917,7 +1918,8 @@ export class PortfolioController selectedAccount, state, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) : null diff --git a/src/controllers/requests/requests.test.ts b/src/controllers/requests/requests.test.ts index f68f6c692e..f6b8988f14 100644 --- a/src/controllers/requests/requests.test.ts +++ b/src/controllers/requests/requests.test.ts @@ -585,6 +585,9 @@ describe('RequestsController ', () => { await controller.addUserRequests([SIGN_ACCOUNT_OP_REQUEST]) expect(controller.banners).toHaveLength(2) + controller.banners.forEach((banner) => { + expect(banner.meta?.accountAddr).toEqual('0x77777777789A8BBEE6C64381e5E89E501fb0e4c8') + }) }) test('should update visible requests on account change', async () => { const { controller, selectedAccountCtrl, getCallsRequest } = await prepareTest() diff --git a/src/controllers/requests/requests.ts b/src/controllers/requests/requests.ts index f87c5e9303..9e69ddd27d 100644 --- a/src/controllers/requests/requests.ts +++ b/src/controllers/requests/requests.ts @@ -1146,7 +1146,8 @@ export class RequestsController extends EventEmitter implements IRequestsControl this.#selectedAccount.account, accountState, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) const accountAddr = getAddress(request.params[0].from) @@ -1215,7 +1216,7 @@ export class RequestsController extends EventEmitter implements IRequestsControl } userRequest = { - id: new Date().getTime(), + id: generateUuid(), kind: 'message', meta: { params: { message: msg[0] }, accountAddr: msgAddress, chainId: network.chainId }, dappPromises: [ @@ -1366,7 +1367,7 @@ export class RequestsController extends EventEmitter implements IRequestsControl } userRequest = { - id: new Date().getTime(), + id: generateUuid(), kind: 'typedMessage', meta: { params: { @@ -1382,7 +1383,7 @@ export class RequestsController extends EventEmitter implements IRequestsControl } as TypedMessageUserRequest } else { userRequest = { - id: new Date().getTime(), + id: generateUuid(), kind, meta: { params: request.params }, dappPromises: [{ ...dappPromise, session: request.session, meta: {} }] @@ -1470,7 +1471,8 @@ export class RequestsController extends EventEmitter implements IRequestsControl this.#selectedAccount.account, accountState, this.#networks.networks.find((net) => net.chainId === selectedToken.chainId)!, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) const requestParams = getIntentRequestParams({ @@ -1620,7 +1622,8 @@ export class RequestsController extends EventEmitter implements IRequestsControl this.#selectedAccount.account, accountState, this.#networks.networks.find((net) => net.chainId === selectedToken.chainId)!, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) const callsRequestParams = getTransferRequestParams({ @@ -1705,7 +1708,8 @@ export class RequestsController extends EventEmitter implements IRequestsControl this.#selectedAccount.account, accountState, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) const swapAndBridgeRequestParams = await getSwapAndBridgeRequestParams( transaction, diff --git a/src/controllers/safe/safe.test.ts b/src/controllers/safe/safe.test.ts new file mode 100644 index 0000000000..d8dad3829e --- /dev/null +++ b/src/controllers/safe/safe.test.ts @@ -0,0 +1,272 @@ +import { getAddress } from 'ethers' + +import { beforeEach, describe, expect, it, jest } from '@jest/globals' + +import { Hex } from '../../interfaces/hex' +import { SafeAccountByOwner } from '../../interfaces/safe' +import { getApiKit, getSafeAccountByOwner } from '../../libs/safe/safe' +import { SafeController } from './safe' + +jest.mock('../../libs/safe/safe', () => ({ + ...jest.requireActual('../../libs/safe/safe'), + getApiKit: jest.fn(), + getSafeAccountByOwner: jest.fn() +})) + +const OWNER = '0xD8293ad21678c6F09Da139b4B62D38e514a03B78' +const OTHER_OWNER = '0x94b0080a00579c1307b0ef2c499ad98a8ce58e58' +const SAFE_A = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48' +const SAFE_B = '0x833589fcd6edb6e08f4c7c32d4f71b54bda02913' +const SAFE_C = '0x4200000000000000000000000000000000000006' +const SAFE_D = '0x0000000000000000000000000000000000000001' +const SAFE_E = '0x0000000000000000000000000000000000000002' +const SAFE_F = '0x0000000000000000000000000000000000000003' + +const createApi = ({ safes = [] }: { safes?: string[] }) => ({ + getSafesByOwner: jest.fn(async () => ({ safes })) +}) + +const createController = (chainIds: bigint[]) => + new SafeController({ + accounts: { + accounts: [], + accountStates: {}, + initialLoadPromise: Promise.resolve() + } as any, + networks: { + initialLoadPromise: Promise.resolve(), + networks: chainIds.map((chainId) => ({ chainId, name: `Network ${chainId.toString()}` })) + } as any, + providers: { providers: {} } as any, + storage: { get: jest.fn(async (_key: string, fallback: unknown) => fallback) } as any + }) + +const getOwnerSearch = (controller: SafeController, owner = OWNER) => + controller.safeOwnerSearches[getAddress(owner) as Hex] + +describe('SafeController findSafesByOwner', () => { + beforeEach(() => { + jest.mocked(getApiKit).mockReset() + jest.mocked(getSafeAccountByOwner).mockReset() + jest.mocked(getSafeAccountByOwner).mockImplementation(async (safeAddr, _owner, deployedOn) => ({ + account: { + addr: getAddress(safeAddr), + deployedOn + } as SafeAccountByOwner, + failed: false + })) + }) + + it('searches supported networks in batches and emits each completed batch', async () => { + const chainIds = [1n, 10n, 56n, 100n, 137n, 8453n] + const apis = new Map>() + chainIds.forEach((chainId) => apis.set(chainId, createApi({ safes: [] }))) + apis.set(1n, createApi({ safes: [SAFE_A] })) + apis.set(10n, createApi({ safes: [SAFE_A] })) + + let resolveLastBatch!: (value: { safes: string[] }) => void + const lastBatchPromise = new Promise<{ safes: string[] }>((resolve) => { + resolveLastBatch = resolve + }) + const lastApi = createApi({ safes: [] }) + lastApi.getSafesByOwner.mockImplementation(() => lastBatchPromise) + apis.set(8453n, lastApi) + jest.mocked(getApiKit).mockImplementation((chainId) => apis.get(chainId) as any) + + const controller = createController(chainIds) + let resolveFirstBatch!: () => void + const firstBatchEmitted = new Promise((resolve) => { + resolveFirstBatch = resolve + }) + controller.onUpdate(() => { + if (getOwnerSearch(controller)?.searchedNetworks.length === 4) resolveFirstBatch() + }) + + const searchPromise = controller.findSafesByOwner(OWNER) + await firstBatchEmitted + + expect(getOwnerSearch(controller)?.accounts).toHaveLength(1) + expect(getOwnerSearch(controller)?.accounts[0]?.deployedOn).toEqual([1n, 10n]) + + resolveLastBatch({ safes: [SAFE_B] }) + await searchPromise + + expect(getOwnerSearch(controller)?.accounts.map((account) => account.addr)).toEqual([ + getAddress(SAFE_A), + getAddress(SAFE_B) + ]) + expect(getOwnerSearch(controller)?.searchedNetworks).toEqual(chainIds) + expect(getOwnerSearch(controller)?.updatedAt).toBeGreaterThan(0) + }) + + it('emits each completed Safe account batch before completing its network batch', async () => { + const safes = [SAFE_A, SAFE_B, SAFE_C, SAFE_D, SAFE_E, SAFE_F] + jest.mocked(getApiKit).mockReturnValue(createApi({ safes }) as any) + + let resolveLastSafeBatch!: () => void + const lastSafeBatchPromise = new Promise((resolve) => { + resolveLastSafeBatch = resolve + }) + jest.mocked(getSafeAccountByOwner).mockImplementation(async (safeAddr, _owner, deployedOn) => { + if (safeAddr === SAFE_E) await lastSafeBatchPromise + + return { + account: { + addr: getAddress(safeAddr), + deployedOn + } as SafeAccountByOwner, + failed: false + } + }) + + const controller = createController([1n]) + let resolveFirstSafeBatch!: () => void + const firstSafeBatchEmitted = new Promise((resolve) => { + resolveFirstSafeBatch = resolve + }) + controller.onUpdate(() => { + if (getOwnerSearch(controller)?.accounts.length === 4) resolveFirstSafeBatch() + }) + + const searchPromise = controller.findSafesByOwner(OWNER) + await firstSafeBatchEmitted + + expect(getOwnerSearch(controller)?.accounts.map((account) => account.addr)).toEqual( + safes.slice(0, 4).map((safe) => getAddress(safe)) + ) + expect(getOwnerSearch(controller)?.searchedNetworks).toEqual([]) + expect(getOwnerSearch(controller)?.updatedAt).toBeGreaterThan(0) + expect(getOwnerSearch(controller)?.status).toBe('LOADING') + + resolveLastSafeBatch() + await searchPromise + + expect(getOwnerSearch(controller)?.accounts.map((account) => account.addr)).toEqual( + safes.map((safe) => getAddress(safe)) + ) + expect(getOwnerSearch(controller)?.searchedNetworks).toEqual([1n]) + expect(getOwnerSearch(controller)?.updatedAt).toBeGreaterThan(0) + }) + + it('keeps results from successful networks and reports failed networks', async () => { + const mainnetApi = createApi({ safes: [SAFE_A] }) + const optimismApi = createApi({ safes: [] }) + optimismApi.getSafesByOwner.mockRejectedValue(new Error('Service unavailable')) + jest + .mocked(getApiKit) + .mockImplementation((chainId) => (chainId === 1n ? mainnetApi : optimismApi) as any) + const controller = createController([1n, 10n]) + + await controller.findSafesByOwner(OWNER) + + expect(getOwnerSearch(controller)?.accounts).toHaveLength(1) + expect(getOwnerSearch(controller)?.failedNetworks).toEqual([10n]) + expect(getOwnerSearch(controller)?.searchedNetworks).toEqual([1n, 10n]) + expect(getOwnerSearch(controller)?.updatedAt).toBeGreaterThan(0) + }) + + it('keeps search results separate for each owner', async () => { + const api = createApi({ safes: [] }) + api.getSafesByOwner + .mockResolvedValueOnce({ safes: [SAFE_A] }) + .mockResolvedValueOnce({ safes: [SAFE_B] }) + jest.mocked(getApiKit).mockReturnValue(api as any) + const controller = createController([1n]) + + await controller.findSafesByOwner(OWNER) + await controller.findSafesByOwner(OTHER_OWNER) + + expect(getOwnerSearch(controller)?.accounts.map((account) => account.addr)).toEqual([ + getAddress(SAFE_A) + ]) + expect(getOwnerSearch(controller, OTHER_OWNER)?.owner).toBe(getAddress(OTHER_OWNER)) + expect( + getOwnerSearch(controller, OTHER_OWNER)?.accounts.map((account) => account.addr) + ).toEqual([getAddress(SAFE_B)]) + }) + + it('replaces only the matching stale owner cache when searching again', async () => { + const api = createApi({ safes: [] }) + api.getSafesByOwner + .mockResolvedValueOnce({ safes: [SAFE_A] }) + .mockResolvedValueOnce({ safes: [SAFE_B] }) + .mockResolvedValueOnce({ safes: [SAFE_C] }) + jest.mocked(getApiKit).mockReturnValue(api as any) + const controller = createController([1n]) + + await controller.findSafesByOwner(OWNER) + await controller.findSafesByOwner(OTHER_OWNER) + getOwnerSearch(controller)!.updatedAt = 0 + await controller.findSafesByOwner(OWNER.toLowerCase()) + + expect(Object.keys(controller.safeOwnerSearches)).toHaveLength(2) + expect(getOwnerSearch(controller)?.accounts.map((account) => account.addr)).toEqual([ + getAddress(SAFE_C) + ]) + expect( + getOwnerSearch(controller, OTHER_OWNER)?.accounts.map((account) => account.addr) + ).toEqual([getAddress(SAFE_B)]) + }) + + it('does not cancel an active owner search when another owner search fails', async () => { + let resolveSearch!: (value: { safes: string[] }) => void + let markSearchAsStarted!: () => void + const searchStarted = new Promise((resolve) => { + markSearchAsStarted = resolve + }) + const searchResponse = new Promise<{ safes: string[] }>((resolve) => { + resolveSearch = resolve + }) + const api = createApi({ safes: [] }) + api.getSafesByOwner + .mockImplementationOnce(() => { + markSearchAsStarted() + return searchResponse + }) + .mockRejectedValueOnce(new Error('Service unavailable')) + jest.mocked(getApiKit).mockReturnValue(api as any) + const controller = createController([1n]) + + const ownerSearchPromise = controller.findSafesByOwner(OWNER) + await searchStarted + await controller.findSafesByOwner(OTHER_OWNER) + resolveSearch({ safes: [SAFE_A] }) + await ownerSearchPromise + + expect(getOwnerSearch(controller)?.accounts.map((account) => account.addr)).toEqual([ + getAddress(SAFE_A) + ]) + expect(getOwnerSearch(controller, OTHER_OWNER)?.failedNetworks).toEqual([1n]) + expect(getOwnerSearch(controller, OTHER_OWNER)?.status).toBe('DONE') + }) + + it('debounces a repeated search for the same owner while loading', async () => { + let resolveSearch!: (value: { safes: string[] }) => void + let markSearchAsStarted!: () => void + const searchStarted = new Promise((resolve) => { + markSearchAsStarted = resolve + }) + const searchResponse = new Promise<{ safes: string[] }>((resolve) => { + resolveSearch = resolve + }) + const api = createApi({ safes: [] }) + api.getSafesByOwner.mockImplementation(() => { + markSearchAsStarted() + return searchResponse + }) + jest.mocked(getApiKit).mockReturnValue(api as any) + const controller = createController([1n]) + + const ownerSearchPromise = controller.findSafesByOwner(OWNER) + await searchStarted + const repeatedSearchPromise = controller.findSafesByOwner(OWNER.toLowerCase()) + resolveSearch({ safes: [SAFE_A] }) + await Promise.all([ownerSearchPromise, repeatedSearchPromise]) + + expect(api.getSafesByOwner).toHaveBeenCalledTimes(1) + expect(getOwnerSearch(controller)?.accounts.map((account) => account.addr)).toEqual([ + getAddress(SAFE_A) + ]) + expect(getOwnerSearch(controller)?.status).toBe('DONE') + }) +}) diff --git a/src/controllers/safe/safe.ts b/src/controllers/safe/safe.ts index dfba0bf3e6..07ac1ac39f 100644 --- a/src/controllers/safe/safe.ts +++ b/src/controllers/safe/safe.ts @@ -1,13 +1,18 @@ -import { toBeHex } from 'ethers' +import { getAddress, toBeHex } from 'ethers' import { FETCH_SAFE_TXNS } from '../../consts/intervals' -import { SAFE_NETWORKS, safeNullOwner } from '../../consts/safe' +import { + SAFE_API_BATCH_SIZE, + SAFE_API_TIMEOUT_MS, + SAFE_NETWORKS, + safeNullOwner +} from '../../consts/safe' import { IAccountsController, SafeAccountCreation } from '../../interfaces/account' import { IEventEmitterRegistryController, Statuses } from '../../interfaces/eventEmitter' import { Hex } from '../../interfaces/hex' import { INetworksController } from '../../interfaces/network' import { IProvidersController } from '../../interfaces/provider' -import { ISafeController } from '../../interfaces/safe' +import { ISafeController, SafeAccountByOwner } from '../../interfaces/safe' import { IStorageController } from '../../interfaces/storage' import { ExtendedSafeMessage, @@ -15,17 +20,31 @@ import { fetchExecutedTransactions, getApiKit, getMessage, + getSafeAccountByOwner, SafeResults } from '../../libs/safe/safe' +import { withTimeout } from '../../utils/with-timeout' import EventEmitter from '../eventEmitter/eventEmitter' import type { SafeCreationInfoResponse, SafeInfoResponse, SafeMessage } from '@safe-global/api-kit' import type { SafeMultisigConfirmationResponse } from '@safe-global/types-kit' +const SAFE_OWNER_SEARCH_TTL = 5 * 60 * 1000 +const SAFE_OWNER_SEARCH_DEBOUNCE = 3 * 1000 + export const STATUS_WRAPPED_METHODS = { findSafe: 'INITIAL' } as const +type SafeOwnerSearch = { + owner: Hex + accounts: SafeAccountByOwner[] + searchedNetworks: bigint[] + failedNetworks: bigint[] + updatedAt: number + status: 'LOADING' | 'DONE' +} + export class SafeController extends EventEmitter implements ISafeController { #storage: IStorageController @@ -62,6 +81,9 @@ export class SafeController extends EventEmitter implements ISafeController { requiresModules: boolean } + ownerCurrentlyDisplayingFor?: Hex + safeOwnerSearches: Record = {} + constructor({ eventEmitterRegistry, networks, @@ -116,7 +138,7 @@ export class SafeController extends EventEmitter implements ISafeController { safeNetworks.map((n) => this.#providers.providers[n.chainId.toString()]!.getCode(safeAddr) .then((code) => ({ chainId: n.chainId, code })) - .catch((e) => ({ chainId: n.chainId, code: '0x' })) + .catch(() => ({ chainId: n.chainId, code: '0x' })) ) ) const deployedOn = codes.find((c) => c.code && c.code !== '0x') @@ -169,6 +191,147 @@ export class SafeController extends EventEmitter implements ISafeController { this.importError = undefined } + resetSearchByOwner() { + this.ownerCurrentlyDisplayingFor = undefined + this.emitUpdate() + } + async findSafesByOwner(ownerAddress: string) { + const owner = getAddress(ownerAddress) as Hex + await this.#networks.initialLoadPromise + + const safeNetworks = this.#networks.networks.filter((network) => + SAFE_NETWORKS.includes(Number(network.chainId)) + ) + + this.ownerCurrentlyDisplayingFor = owner + const dataForCurrent = this.safeOwnerSearches[owner] + if ( + dataForCurrent && + dataForCurrent.status === 'DONE' && + dataForCurrent.updatedAt > Date.now() - SAFE_OWNER_SEARCH_TTL && + !dataForCurrent.failedNetworks.length && + safeNetworks.every(({ chainId }) => dataForCurrent.searchedNetworks.includes(chainId)) + ) { + this.emitUpdate() + return + } + if ( + dataForCurrent?.status === 'LOADING' && + dataForCurrent.updatedAt > Date.now() - SAFE_OWNER_SEARCH_DEBOUNCE + ) + return + + const accountsByAddress = new Map() + this.safeOwnerSearches[owner] = { + owner, + accounts: [], + searchedNetworks: [], + failedNetworks: [], + updatedAt: Date.now(), + status: 'LOADING' + } + this.emitUpdate() + + for (let i = 0; i < safeNetworks.length; i += SAFE_API_BATCH_SIZE) { + const networkBatch = safeNetworks.slice(i, i + SAFE_API_BATCH_SIZE) + const batchResults = await Promise.allSettled( + networkBatch.map(async (network) => { + const response = await withTimeout( + () => getApiKit(network.chainId).getSafesByOwner(owner), + { + timeoutMs: SAFE_API_TIMEOUT_MS, + message: `Safe API: owner search timed out after ${SAFE_API_TIMEOUT_MS}ms` + } + ) + return { chainId: network.chainId, safes: response.safes } + }) + ) + + const failedNetworks: bigint[] = [] + const deployedOnByAddress = new Map() + + batchResults.forEach((result, index) => { + const network = networkBatch[index]! + if (result.status === 'rejected') { + failedNetworks.push(network.chainId) + console.error(`Failed to search Safe accounts on network ${network.name}`, result.reason) + return + } + + result.value.safes.forEach((safeAddr) => { + const normalizedAddress = safeAddr.toLowerCase() + const existing = deployedOnByAddress.get(normalizedAddress) + if (existing) { + existing.chainIds.push(result.value.chainId) + return + } + deployedOnByAddress.set(normalizedAddress, { + address: safeAddr, + chainIds: [result.value.chainId] + }) + }) + }) + + const newSafeEntries = Array.from(deployedOnByAddress.entries()).filter( + ([address]) => !accountsByAddress.has(address) + ) + for (let safeIndex = 0; safeIndex < newSafeEntries.length; safeIndex += SAFE_API_BATCH_SIZE) { + const safeBatch = newSafeEntries.slice(safeIndex, safeIndex + SAFE_API_BATCH_SIZE) + const safeAccounts = await Promise.all( + safeBatch.map(([, safeData]) => + getSafeAccountByOwner(safeData.address, owner, safeData.chainIds) + ) + ) + safeAccounts.forEach(({ account, failed }, index) => { + if (account) { + accountsByAddress.set(account.addr.toLowerCase(), account) + return + } + if (failed) failedNetworks.push(...safeBatch[index]![1].chainIds) + }) + + // we use this to show results immediately to the user + this.safeOwnerSearches[owner] = { + owner, + accounts: Array.from(accountsByAddress.values()), + searchedNetworks: this.safeOwnerSearches[owner]?.searchedNetworks || [], + failedNetworks: this.safeOwnerSearches[owner]?.failedNetworks || [], + updatedAt: Date.now(), + status: 'LOADING' + } + this.emitUpdate() + } + + deployedOnByAddress.forEach(({ chainIds }, address) => { + const account = accountsByAddress.get(address) + if (!account) return + account.deployedOn = Array.from(new Set([...account.deployedOn, ...chainIds])) + }) + + this.safeOwnerSearches[owner] = { + owner, + accounts: Array.from(accountsByAddress.values()), + searchedNetworks: [ + ...(this.safeOwnerSearches[owner]?.searchedNetworks || []), + ...networkBatch.map((network) => network.chainId) + ], + failedNetworks: Array.from( + new Set([...(this.safeOwnerSearches[owner]?.failedNetworks || []), ...failedNetworks]) + ), + updatedAt: Date.now(), + status: 'LOADING' + } + this.emitUpdate() + } + + this.safeOwnerSearches[owner] = { + ...this.safeOwnerSearches[owner]!, + updatedAt: Date.now(), + status: 'DONE' + } + this.emitUpdate() + } + getMessageId(msg: SafeMessage): string { return `${msg.messageHash}` } diff --git a/src/controllers/selectedAccount/selectedAccount.ts b/src/controllers/selectedAccount/selectedAccount.ts index 0822707c19..250e3ad83d 100644 --- a/src/controllers/selectedAccount/selectedAccount.ts +++ b/src/controllers/selectedAccount/selectedAccount.ts @@ -36,6 +36,10 @@ import { import { getProjectedRewardsStatsAndToken } from '../../utils/rewards' import EventEmitter from '../eventEmitter/eventEmitter' +// Portfolio recalculations fire back-to-back as per-network results stream in. +// Throttle their UI emit so the state isn't serialized on every partial tick. +const PORTFOLIO_UPDATE_THROTTLE_MS = 100 + export class SelectedAccountController extends EventEmitter implements ISelectedAccountController { #storage: IStorageController @@ -327,8 +331,11 @@ export class SelectedAccountController extends EventEmitter implements ISelected }) } + let justLoaded = false + // Reset the loading timestamp if the portfolio is ready if (this.#portfolioLoadingTimeout && newSelectedAccountPortfolio.isAllReady) { + justLoaded = true clearTimeout(this.#portfolioLoadingTimeout) this.#portfolioLoadingTimeout = null } @@ -365,7 +372,7 @@ export class SelectedAccountController extends EventEmitter implements ISelected this.#updatePortfolioErrors(true) if (!skipUpdate) { - this.emitUpdate() + this.emitUpdate({ throttleMs: justLoaded ? 0 : PORTFOLIO_UPDATE_THROTTLE_MS }) } } diff --git a/src/controllers/signAccountOp/helper.test.ts b/src/controllers/signAccountOp/helper.test.ts index ea6b9fef45..58e9fe2507 100644 --- a/src/controllers/signAccountOp/helper.test.ts +++ b/src/controllers/signAccountOp/helper.test.ts @@ -82,5 +82,4 @@ describe('getSafeDelegateCallWarning', () => { test('does not warn when accountOp.safeTx is not set', () => { expect(getSafeDelegateCallWarning(accountOp)).toBeNull() }) - }) diff --git a/src/controllers/signAccountOp/helper.ts b/src/controllers/signAccountOp/helper.ts index 22983ebff9..b8b702940e 100644 --- a/src/controllers/signAccountOp/helper.ts +++ b/src/controllers/signAccountOp/helper.ts @@ -29,11 +29,16 @@ function getTokenUsdAmount(token: TokenResult, gasAmount: bigint): string { function getSignificantBalanceDecreaseWarning( portfolioState: AccountState, chainId: bigint, - traceCallDiscoveryStatus: TraceCallDiscoveryStatus + discoveryStatus: TraceCallDiscoveryStatus ): Warning | null { const portfolioNetworkState = portfolioState?.[chainId.toString()] - if (portfolioNetworkState && portfolioNetworkState.result && !portfolioNetworkState.isLoading) { + // calculate this only after traceCall has ended + const isDiscoveryOver = + discoveryStatus === TraceCallDiscoveryStatus.Failed || + discoveryStatus === TraceCallDiscoveryStatus.Done + + if (portfolioNetworkState && portfolioNetworkState.result && isDiscoveryOver) { const totalInUSD = getAccountPortfolioTotal( portfolioState, ['rewards', 'gasTank', 'projectedRewards'], @@ -73,22 +78,7 @@ function getSignificantBalanceDecreaseWarning( if (!hasSignificantBalanceDecrease) return null - // We wait for the discovery process (main.traceCall) to complete before showing WARNINGS.significantBalanceDecrease. - // This is important because, in the case of a SWAP to a new token, the new token is not yet part of the portfolio, - // which could incorrectly trigger a significant balance drop warning. - // To prevent this, we ensure the discovery process is completed first. - if (traceCallDiscoveryStatus === TraceCallDiscoveryStatus.Done) { - return WARNINGS.significantBalanceDecrease - } - - // If the discovery process takes too long (more than 2 seconds) or fails, - // we still show a warning, but we indicate that our balance decrease assumption may be incorrect. - if ( - traceCallDiscoveryStatus === TraceCallDiscoveryStatus.Failed || - traceCallDiscoveryStatus === TraceCallDiscoveryStatus.SlowPendingResponse - ) { - return WARNINGS.possibleBalanceDecrease - } + return WARNINGS.significantBalanceDecrease } return null diff --git a/src/controllers/signAccountOp/signAccountOp.test.ts b/src/controllers/signAccountOp/signAccountOp.test.ts index 4a9e67f22b..2c13dec9da 100644 --- a/src/controllers/signAccountOp/signAccountOp.test.ts +++ b/src/controllers/signAccountOp/signAccountOp.test.ts @@ -11,6 +11,7 @@ import { } from 'ethers' import fetch from 'node-fetch' +import { WARNINGS } from '@/consts/signAccountOp/errorHandling' import { describe, expect, jest, test } from '@jest/globals' import { recoverTypedSignature, SignTypedDataVersion } from '@metamask/eth-sig-util' @@ -36,6 +37,7 @@ import { networks } from '../../consts/networks' import { Account } from '../../interfaces/account' import { Dapp, DAPP_VERIFICATION_BANNER_IDS, IDappsController } from '../../interfaces/dapp' import { Hex } from '../../interfaces/hex' +import { ExternalSignerController, ExternalSignerControllers } from '../../interfaces/keystore' import { IProvidersController } from '../../interfaces/provider' import { TraceCallDiscoveryStatus } from '../../interfaces/signAccountOp' import { Storage } from '../../interfaces/storage' @@ -48,6 +50,8 @@ import { FullEstimationSummary } from '../../libs/estimate/interfaces' import { clearErc7730RegistryCache } from '../../libs/humanizer' import { KeystoreSigner } from '../../libs/keystoreSigner/keystoreSigner' import { TokenResult } from '../../libs/portfolio' +import { AccountState } from '../../libs/portfolio/interfaces' +import { PORTFOLIO_STATE } from '../../libs/portfolio/testData' import { BindedRelayerCall, relayerCall, RelayerError } from '../../libs/relayerCall/relayerCall' import { adaptTypedMessageForMetaMaskSigUtil, @@ -382,6 +386,41 @@ const nativeFeeToken: TokenResult = { } } +const buildPortfolioState = ({ + amountBeforeSimulation, + amountPostSimulation, + isLoading +}: { + amountBeforeSimulation: bigint + amountPostSimulation: bigint + isLoading: boolean +}): AccountState => { + const networkState = PORTFOLIO_STATE['1'] + const token = networkState?.result?.tokens[0] + + if (!networkState?.result || !token) throw new Error('Invalid portfolio test fixture') + + return { + '1': { + ...networkState, + isLoading, + result: { + ...networkState.result, + total: { usd: Number(amountBeforeSimulation) }, + tokens: [ + { + ...token, + amount: amountBeforeSimulation, + amountPostSimulation, + decimals: 0, + priceIn: [{ baseCurrency: 'usd', price: 1 }] + } + ] + } + } + } +} + const gasTankToken: TokenResult = { address: '0x0000000000000000000000000000000000000000', symbol: 'ETH', @@ -419,6 +458,7 @@ const init = async ( type?: SignAccountOpType initialSetStorage?: (storageCtrl: StorageController) => Promise onUpdateAfterTraceCallSuccess?: () => Promise + externalSignerControllers?: ExternalSignerControllers } ) => { const storage: Storage = produceMemoryStore() @@ -598,6 +638,7 @@ const init = async ( account, accountsCtrl.accountStates[account.addr]![network.chainId.toString()]!, network, + true, true ) @@ -682,7 +723,7 @@ const init = async ( portfolio, featureFlags: featureFlagsCtrl, signAccountOpPreference, - externalSignerControllers: {}, + externalSignerControllers: options?.externalSignerControllers || {}, account, network, activity, @@ -702,7 +743,7 @@ const init = async ( gasPrices: gasPricesOrMock }) - return { controller, storageCtrl, signAccountOpPreference, accountsCtrl } + return { controller, storageCtrl, signAccountOpPreference, accountsCtrl, portfolio } } const initDappVerificationBannerTest = async ( @@ -1088,23 +1129,59 @@ describe('SignAccountOp Controller ', () => { test('uses the saved fee speed as the default for a new signing request', async () => { const { controller } = await initDefaultFeeSelection(undefined, { initialSetStorage: async (storageCtrl) => { - await storageCtrl.set('signAccountOpFeeSpeedPreference', FeeSpeed.Medium) + await storageCtrl.set('signAccountOpFeeSpeedPreference', { '1': FeeSpeed.Medium }) } }) expect(controller.selectedFeeSpeed).toBe(FeeSpeed.Medium) }) - test('persists only explicitly selected fee speeds', async () => { + test('ignores a saved fee speed belonging to another chain', async () => { + const { controller } = await initDefaultFeeSelection(undefined, { + initialSetStorage: async (storageCtrl) => { + await storageCtrl.set('signAccountOpFeeSpeedPreference', { '137': FeeSpeed.Slow }) + } + }) + + expect(controller.selectedFeeSpeed).toBe(FeeSpeed.Fast) + }) + + test('persists a user selected fee speed right away, for the current chain only', async () => { + const { controller, storageCtrl } = await initDefaultFeeSelection() + + controller.update({ speed: FeeSpeed.Slow, shouldPersistSpeed: true }) + await wait(1) + + expect(controller.selectedFeeSpeed).toBe(FeeSpeed.Slow) + expect(await storageCtrl.get('signAccountOpFeeSpeedPreference')).toEqual({ + '1': FeeSpeed.Slow + }) + }) + + test('does not persist a fee speed that was not selected by the user', async () => { const { controller, storageCtrl } = await initDefaultFeeSelection() controller.update({ speed: FeeSpeed.Slow }) await wait(1) + + expect(controller.selectedFeeSpeed).toBe(FeeSpeed.Slow) expect(await storageCtrl.get('signAccountOpFeeSpeedPreference')).toBeUndefined() + }) + + test('saving a fee speed keeps the ones saved for the other chains', async () => { + const { controller, storageCtrl } = await initDefaultFeeSelection(undefined, { + initialSetStorage: async (storage) => { + await storage.set('signAccountOpFeeSpeedPreference', { '137': FeeSpeed.Ape }) + } + }) controller.update({ speed: FeeSpeed.Medium, shouldPersistSpeed: true }) await wait(1) - expect(await storageCtrl.get('signAccountOpFeeSpeedPreference')).toBe(FeeSpeed.Medium) + + expect(await storageCtrl.get('signAccountOpFeeSpeedPreference')).toEqual({ + '1': FeeSpeed.Medium, + '137': FeeSpeed.Ape + }) }) test('uses a saved ERC-20 default only for the matching chain', async () => { @@ -3061,6 +3138,80 @@ describe('ERC-7730 humanization', () => { }) }) +describe('significant balance decrease banners', () => { + test('keeps the previous banner while refreshing and recalculates when the result changes', async () => { + const { controller, portfolio } = await initDappVerificationBannerTest(verifiedDapp) + const portfolioState = portfolio.getAccountPortfolioState(eoaAccount.addr) + + controller.setDiscoveryStatus(TraceCallDiscoveryStatus.Done) + portfolioState['1'] = { isReady: false, isLoading: true, errors: [] } + expect( + controller.banners.find(({ id }) => id === WARNINGS.significantBalanceDecrease.id) + ).toBeUndefined() + + portfolioState['1'] = buildPortfolioState({ + amountBeforeSimulation: 5000n, + amountPostSimulation: 3000n, + isLoading: false + })['1'] + const significantBalanceDecreaseBanner = controller.banners.find( + ({ id }) => id === WARNINGS.significantBalanceDecrease.id + ) + expect(significantBalanceDecreaseBanner).toEqual({ + id: WARNINGS.significantBalanceDecrease.id, + type: 'warning', + title: 'Significant balance decrease detected', + text: 'Our checks indicate this transaction may significantly reduce your account balance.', + secondaryText: + 'May be inaccurate when moving funds to another network or providing liquidity.' + }) + + portfolioState['1']!.isLoading = true + expect( + controller.banners.find(({ id }) => id === WARNINGS.significantBalanceDecrease.id) + ).toEqual(significantBalanceDecreaseBanner) + + portfolioState['1'] = buildPortfolioState({ + amountBeforeSimulation: 5000n, + amountPostSimulation: 5000n, + isLoading: false + })['1'] + expect( + controller.banners.find(({ id }) => id === WARNINGS.significantBalanceDecrease.id) + ).toBeUndefined() + expect( + controller.warnings.find(({ id }) => id === WARNINGS.significantBalanceDecrease.id) + ).toBeUndefined() + }) + + test('waits for token discovery to finish while the portfolio is refreshing', async () => { + const { controller, portfolio } = await initDappVerificationBannerTest(verifiedDapp) + const portfolioState = portfolio.getAccountPortfolioState(eoaAccount.addr) + portfolioState['1'] = buildPortfolioState({ + amountBeforeSimulation: 5000n, + amountPostSimulation: 3000n, + isLoading: true + })['1'] + + controller.setDiscoveryStatus(TraceCallDiscoveryStatus.InProgress) + expect( + controller.banners.find(({ id }) => id === WARNINGS.significantBalanceDecrease.id) + ).toBeUndefined() + + controller.setDiscoveryStatus(TraceCallDiscoveryStatus.Failed) + expect( + controller.banners.find(({ id }) => id === WARNINGS.significantBalanceDecrease.id) + ).toEqual({ + id: WARNINGS.significantBalanceDecrease.id, + type: 'warning', + title: 'Significant balance decrease detected', + text: 'Our checks indicate this transaction may significantly reduce your account balance.', + secondaryText: + 'May be inaccurate when moving funds to another network or providing liquidity.' + }) + }) +}) + describe('dapp verification banners', () => { test('should return loading banners', async () => { const { controller } = await initDappVerificationBannerTest(loadingDapp) @@ -3069,6 +3220,7 @@ describe('dapp verification banners', () => { { id: DAPP_VERIFICATION_BANNER_IDS.LOADING, type: 'warning', + title: 'Safety check in progress', text: "We're still verifying the app. Please wait, or make sure you trust it before signing requests: Loading Dapp" } ]) @@ -3081,6 +3233,7 @@ describe('dapp verification banners', () => { { id: DAPP_VERIFICATION_BANNER_IDS.FAILED_TO_GET_OR_UNKNOWN, type: 'warning', + title: "App couldn't be verified", text: "We couldn't verify the app. Make sure you trust it before signing requests: Failed Dapp" } ]) @@ -3093,6 +3246,7 @@ describe('dapp verification banners', () => { { id: DAPP_VERIFICATION_BANNER_IDS.BLACKLISTED, type: 'error', + title: 'Potentially harmful app', text: "This app didn't pass our safety check. Proceed at your own risk: Blacklisted Dapp" } ]) @@ -3111,6 +3265,7 @@ describe('dapp verification banners', () => { { id: DAPP_VERIFICATION_BANNER_IDS.NOT_IN_CATALOG, type: 'warning', + title: "App not in Ambire's catalog", text: 'App is not on the default Ambire App Catalog. Make sure you trust it before signing requests: Custom Dapp' } ]) @@ -3125,6 +3280,7 @@ describe('dapp verification banners', () => { { id: DAPP_VERIFICATION_BANNER_IDS.SUSPICIOUS_HOSTING, type: 'warning', + title: 'Suspicious app hosting', text: 'This app is hosted on a shared platform commonly used for phishing. Be careful - do not sign unless you are certain you trust it.' } ]) @@ -3266,10 +3422,6 @@ describe('traceCall asset discovery', () => { await (controller as any).traceCall() expect(createAccessListCallSpy).toHaveBeenCalledTimes(1) - // After 2s without a response the status reflects the slow pending state. - jest.advanceTimersByTime(2000) - expect(controller.traceCallDiscoveryStatus).toBe(TraceCallDiscoveryStatus.SlowPendingResponse) - // Resolving discovery learns the assets, fires the success callback and // settles on Done. createAccessListDeferred.resolve(discovered) @@ -3453,3 +3605,79 @@ describe('traceCall asset discovery', () => { }) }) }) + +describe('external signer PIN sessions', () => { + suppressConsoleBeforeEach(true) + + const pinSessionGasPrices = { + slow: { maxFeePerGas: toBeHex(200n) as Hex, maxPriorityFeePerGas: toBeHex(100n) as Hex }, + medium: { maxFeePerGas: toBeHex(400n) as Hex, maxPriorityFeePerGas: toBeHex(200n) as Hex }, + fast: { maxFeePerGas: toBeHex(600n) as Hex, maxPriorityFeePerGas: toBeHex(300n) as Hex }, + ape: { maxFeePerGas: toBeHex(800n) as Hex, maxPriorityFeePerGas: toBeHex(400n) as Hex } + } + + const initPinSession = async () => { + const nfc = { + type: 'nfc', + deviceModel: '', + deviceId: '', + beginPinSession: jest.fn(async () => {}), + endPinSession: jest.fn(async () => {}) + } as unknown as ExternalSignerController & { + beginPinSession: jest.Mock + endPinSession: jest.Mock + } + const feePaymentOptions = [ + { + paidBy: eoaAccount.addr, + availableAmount: 1000000000000000000n, + gasUsed: 0n, + addedNative: 5000n, + token: nativeFeeToken + } + ] + const { controller } = await init( + eoaAccount, + createEOAAccountOp(eoaAccount), + eoaSigner, + { + providerEstimation: { gasUsed: 10000n, feePaymentOptions }, + flags: {}, + updatedAt: Date.now() + } as any, + pinSessionGasPrices, + false, + { externalSignerControllers: { nfc } as any } + ) + + return { controller, nfc } + } + + test('opens the PIN session before signing and closes it once the whole flow is over', async () => { + const { controller, nfc } = await initPinSession() + const callOrder: string[] = [] + + nfc.beginPinSession.mockImplementation(async () => { + callOrder.push('begin') + }) + nfc.endPinSession.mockImplementation(async () => { + callOrder.push('end') + }) + + await controller.signAndBroadcast().catch(() => {}) + + // One session for the whole account op, no matter how many signatures it takes - + // that is what lets a single PIN entry cover all of them. + expect(callOrder).toEqual(['begin', 'end']) + }) + + test('opens a new PIN session for the next account op, so the PIN is asked for again', async () => { + const { controller, nfc } = await initPinSession() + + await controller.signAndBroadcast().catch(() => {}) + await controller.signAndBroadcast().catch(() => {}) + + expect(nfc.beginPinSession).toHaveBeenCalledTimes(2) + expect(nfc.endPinSession).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/controllers/signAccountOp/signAccountOp.ts b/src/controllers/signAccountOp/signAccountOp.ts index 9aa494905d..50b26e9c94 100644 --- a/src/controllers/signAccountOp/signAccountOp.ts +++ b/src/controllers/signAccountOp/signAccountOp.ts @@ -440,7 +440,8 @@ export class SignAccountOpController this.#featureFlags = featureFlags this.#signAccountOpPreference = signAccountOpPreference this.feeTokenPreference = this.#signAccountOpPreference.feeTokenPreference - this.selectedFeeSpeed = this.#signAccountOpPreference.feeSpeedPreference + this.selectedFeeSpeed = + this.#signAccountOpPreference.feeSpeedPreference[network.chainId.toString()] || FeeSpeed.Fast this.#externalSignerControllers = externalSignerControllers this.account = account const accountState = accounts.accountStates[account.addr]![network.chainId.toString()]! // ! is safe as otherwise, nothing will work @@ -448,7 +449,8 @@ export class SignAccountOpController account, accountState, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) this.#network = network this.#activity = activity @@ -564,7 +566,8 @@ export class SignAccountOpController this.account, accountState, this.#network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) this.gasPrice?.setBaseAccount(this.baseAccount) } @@ -1442,13 +1445,6 @@ export class SignAccountOpController const warnings: Warning[] = [] const state = this.#portfolio.getAccountPortfolioState(this.accountOp.accountAddr) - - const significantBalanceDecreaseWarning = getSignificantBalanceDecreaseWarning( - state, - this.accountOp.chainId, - this.traceCallDiscoveryStatus - ) - const unknownTokenWarnings = getUnknownTokenWarning(state, this.accountOp.chainId) if (this.selectedOption) { @@ -1464,7 +1460,6 @@ export class SignAccountOpController warnings.push(feeTokenPriceUnavailableWarning) } - if (significantBalanceDecreaseWarning) warnings.push(significantBalanceDecreaseWarning) if (unknownTokenWarnings) warnings.push(unknownTokenWarnings) const accountState = @@ -1808,8 +1803,13 @@ export class SignAccountOpController if (speed && this.isInitialized && !isSpeedUpTransaction) { this.selectedFeeSpeed = speed + // Only an explicitly picked speed becomes the default for the network. + // Speeds set while switching the fee token are a fallback, not a choice if (shouldPersistSpeed) { - void this.#signAccountOpPreference.setFeeSpeedPreference(speed) + void this.#signAccountOpPreference.setFeeSpeedPreference({ + ...this.#signAccountOpPreference.feeSpeedPreference, + [this.accountOp.chainId.toString()]: speed + }) } } @@ -2218,9 +2218,7 @@ export class SignAccountOpController return } - // `traceCall` should not be invoked too frequently. However, if there is a pending timeout, - // it should be cleared to prevent the previous interval from changing the status - // to `SlowPendingResponse` for the newer `traceCall` invocation. + // clear the timeout on each new invoke if (this.traceCallTimeoutId) clearTimeout(this.traceCallTimeoutId) // Here, we also check the status because, in the case of re-estimation, @@ -2229,7 +2227,6 @@ export class SignAccountOpController if (this.traceCallDiscoveryStatus === TraceCallDiscoveryStatus.NotStarted) this.setDiscoveryStatus(TraceCallDiscoveryStatus.InProgress) - // Flag the discovery logic as `SlowPendingResponse` if the call does not resolve within 2 seconds. const timeoutId = setTimeout(() => { // Prevent race conditions between multiple `traceCall` invocations if ( @@ -2237,9 +2234,6 @@ export class SignAccountOpController this.traceCallTimeoutId !== timeoutId ) return - - this.setDiscoveryStatus(TraceCallDiscoveryStatus.SlowPendingResponse) - this.calculateWarnings() }, 2000) this.traceCallTimeoutId = timeoutId @@ -2284,7 +2278,6 @@ export class SignAccountOpController }) } - this.calculateWarnings() this.traceCallTimeoutId = null clearTimeout(timeoutId) } @@ -2354,8 +2347,12 @@ export class SignAccountOpController const speeds = this.feeSpeeds[identifier] if (!speeds) return - const preferredSpeed = this.#signAccountOpPreference.feeSpeedPreference - if (speeds.find(({ type, disabled }) => type === preferredSpeed && !disabled)) { + const preferredSpeed = + this.#signAccountOpPreference.feeSpeedPreference[this.accountOp.chainId.toString()] + if ( + preferredSpeed && + speeds.find(({ type, disabled }) => type === preferredSpeed && !disabled) + ) { this.selectedFeeSpeed = preferredSpeed return } @@ -3793,6 +3790,8 @@ export class SignAccountOpController this.gasFeeChangedConfirmationRequired = false this.previousFee = null + this.#beginPinSessions() + this.signAndBroadcastPromise = (async () => { this.signPromise = this.sign().finally(() => { this.signPromise = undefined @@ -3826,6 +3825,7 @@ export class SignAccountOpController } })().finally(() => { this.signAndBroadcastPromise = undefined + this.#endPinSessions() }) await this.signAndBroadcastPromise @@ -3845,6 +3845,19 @@ export class SignAccountOpController }) } + /** + * One account op can take several signatures from the same key, and a device that + * unlocks with a PIN asks for it before each. Marking where the signing starts and + * ends lets it keep the PIN for that long. Only the boundaries reach it, never the PIN. + */ + #beginPinSessions() { + Object.values(this.#externalSignerControllers).forEach((c) => c?.beginPinSession?.()) + } + + #endPinSessions() { + Object.values(this.#externalSignerControllers).forEach((c) => c?.endPinSession?.()) + } + get isSignInProgress() { return !!this.signPromise } @@ -3987,6 +4000,11 @@ export class SignAccountOpController setDiscoveryStatus(status: TraceCallDiscoveryStatus) { this.traceCallDiscoveryStatus = status + + // emit an update on done/failed to sync&show the final banners + if (status === TraceCallDiscoveryStatus.Done || status === TraceCallDiscoveryStatus.Failed) { + this.emitUpdate() + } } /** @@ -4050,6 +4068,7 @@ export class SignAccountOpController banners.push({ id: 'blacklisted-addresses-error-banner', type: 'error', + title: 'Potentially harmful transaction', text: getScamDetectedText(blacklistedItems) }) } else { @@ -4061,6 +4080,7 @@ export class SignAccountOpController banners.push({ id: 'blacklisted-addresses-warning-banner', type: 'warning', + title: 'Safety check unavailable', text: "We couldn't check the addresses or tokens in this transaction for malicious activity. Proceed with caution." }) } @@ -4069,11 +4089,27 @@ export class SignAccountOpController const dappVerificationBanner = this.#getDappVerificationBanner() if (dappVerificationBanner) banners.push(dappVerificationBanner) + const significantBalanceDecreaseWarning = getSignificantBalanceDecreaseWarning( + this.#portfolio.getAccountPortfolioState(this.accountOp.accountAddr), + this.accountOp.chainId, + this.traceCallDiscoveryStatus + ) + if (significantBalanceDecreaseWarning) { + banners.push({ + id: significantBalanceDecreaseWarning.id, + type: 'warning', + title: significantBalanceDecreaseWarning.title, + text: significantBalanceDecreaseWarning.text || significantBalanceDecreaseWarning.title, + secondaryText: significantBalanceDecreaseWarning.secondaryText + }) + } + const safeDelegateCallWarning = getSafeDelegateCallWarning(this.accountOp) if (safeDelegateCallWarning) { banners.push({ id: safeDelegateCallWarning.id, type: 'warning', + title: safeDelegateCallWarning.title, text: safeDelegateCallWarning.text || safeDelegateCallWarning.title }) } diff --git a/src/controllers/signAccountOp/signAccountOpPreference.test.ts b/src/controllers/signAccountOp/signAccountOpPreference.test.ts index 9edddedc05..e7ee557645 100644 --- a/src/controllers/signAccountOp/signAccountOpPreference.test.ts +++ b/src/controllers/signAccountOp/signAccountOpPreference.test.ts @@ -9,6 +9,7 @@ import { const initializePreference = async (storedFeeSpeed?: unknown) => { const storage = new StorageController(produceMemoryStore()) if (storedFeeSpeed !== undefined) { + // @ts-expect-error intentionally exercising legacy/corrupt stored shapes await storage.set(FEE_SPEED_PREFERENCE_STORAGE_KEY, storedFeeSpeed) } @@ -19,31 +20,68 @@ const initializePreference = async (storedFeeSpeed?: unknown) => { } describe('SignAccountOpPreferenceController fee speed preference', () => { - test('defaults to fast when no preference is stored', async () => { + test('defaults to no saved speeds when nothing is stored', async () => { const { preference } = await initializePreference() - expect(preference.feeSpeedPreference).toBe(FeeSpeed.Fast) + expect(preference.feeSpeedPreference).toEqual({}) }) - test('loads a saved fee speed preference', async () => { + test('loads saved per-network fee speeds', async () => { + const { preference } = await initializePreference({ + '1': FeeSpeed.Medium, + '10': FeeSpeed.Ape + }) + + expect(preference.feeSpeedPreference).toEqual({ + '1': FeeSpeed.Medium, + '10': FeeSpeed.Ape + }) + }) + + test('drops only the invalid entries of a stored map', async () => { + const { preference } = await initializePreference({ + '1': FeeSpeed.Slow, + '10': 'invalid-speed' + }) + + expect(preference.feeSpeedPreference).toEqual({ '1': FeeSpeed.Slow }) + }) + + test('discards a legacy global fee speed stored as a plain string', async () => { const { preference } = await initializePreference(FeeSpeed.Medium) - expect(preference.feeSpeedPreference).toBe(FeeSpeed.Medium) + expect(preference.feeSpeedPreference).toEqual({}) }) - test('ignores an invalid stored fee speed', async () => { - const { preference } = await initializePreference('invalid-speed') + test('discards a stored array', async () => { + const { preference } = await initializePreference([FeeSpeed.Medium]) - expect(preference.feeSpeedPreference).toBe(FeeSpeed.Fast) + expect(preference.feeSpeedPreference).toEqual({}) }) - test('updates immediately and persists the selected fee speed', async () => { + test('updates immediately and persists the selected fee speeds', async () => { const { preference, storage } = await initializePreference() - await preference.setFeeSpeedPreference(FeeSpeed.Slow) + await preference.setFeeSpeedPreference({ '1': FeeSpeed.Slow }) + + expect(preference.feeSpeedPreference).toEqual({ '1': FeeSpeed.Slow }) + await expect(storage.get(FEE_SPEED_PREFERENCE_STORAGE_KEY)).resolves.toEqual({ + '1': FeeSpeed.Slow + }) + }) + + test('saving a speed for one network leaves the others intact', async () => { + const { preference, storage } = await initializePreference({ '1': FeeSpeed.Medium }) + + await preference.setFeeSpeedPreference({ + ...preference.feeSpeedPreference, + '10': FeeSpeed.Ape + }) - expect(preference.feeSpeedPreference).toBe(FeeSpeed.Slow) - await expect(storage.get(FEE_SPEED_PREFERENCE_STORAGE_KEY)).resolves.toBe(FeeSpeed.Slow) + await expect(storage.get(FEE_SPEED_PREFERENCE_STORAGE_KEY)).resolves.toEqual({ + '1': FeeSpeed.Medium, + '10': FeeSpeed.Ape + }) }) test('keeps the in-memory preference and emits an error when storage fails', async () => { @@ -53,9 +91,9 @@ describe('SignAccountOpPreferenceController fee speed preference', () => { const onError = jest.fn() preference.onError(onError) - await expect(preference.setFeeSpeedPreference(FeeSpeed.Ape)).resolves.toBeUndefined() + await expect(preference.setFeeSpeedPreference({ '1': FeeSpeed.Ape })).resolves.toBeUndefined() - expect(preference.feeSpeedPreference).toBe(FeeSpeed.Ape) + expect(preference.feeSpeedPreference).toEqual({ '1': FeeSpeed.Ape }) expect(onError).toHaveBeenCalledWith({ message: 'Error saving SignAccountOp fee speed preference', error: storageError, diff --git a/src/controllers/signAccountOp/signAccountOpPreference.ts b/src/controllers/signAccountOp/signAccountOpPreference.ts index 62a108384b..ba5108e77d 100644 --- a/src/controllers/signAccountOp/signAccountOpPreference.ts +++ b/src/controllers/signAccountOp/signAccountOpPreference.ts @@ -5,9 +5,27 @@ import EventEmitter from '../eventEmitter/eventEmitter' export type SignAccountOpFeeTokenPreference = StorageProps['signAccountOpFeeTokenPreference'] +export type SignAccountOpFeeSpeedPreference = StorageProps['signAccountOpFeeSpeedPreference'] + export const FEE_TOKEN_PREFERENCE_STORAGE_KEY = 'signAccountOpFeeTokenPreference' export const FEE_SPEED_PREFERENCE_STORAGE_KEY = 'signAccountOpFeeSpeedPreference' +/** + * Keeps only valid per-chain fee speeds. Values stored before the preference + * became per-network were a single FeeSpeed string and are dropped here, so + * those users fall back to the default speed instead of a corrupt map. + */ +const sanitizeFeeSpeedPreference = (stored: unknown): SignAccountOpFeeSpeedPreference => { + if (typeof stored !== 'object' || stored === null || Array.isArray(stored)) return {} + + const sanitized: SignAccountOpFeeSpeedPreference = {} + Object.entries(stored).forEach(([chainId, speed]) => { + if (Object.values(FeeSpeed).includes(speed as FeeSpeed)) sanitized[chainId] = speed as FeeSpeed + }) + + return sanitized +} + export class SignAccountOpPreferenceController extends EventEmitter { #storage: IStorageController @@ -15,7 +33,7 @@ export class SignAccountOpPreferenceController extends EventEmitter { feeTokenPreference: SignAccountOpFeeTokenPreference = {} - feeSpeedPreference: FeeSpeed = FeeSpeed.Fast + feeSpeedPreference: SignAccountOpFeeSpeedPreference = {} initialLoadPromise?: Promise @@ -37,13 +55,9 @@ export class SignAccountOpPreferenceController extends EventEmitter { async #load() { try { this.feeTokenPreference = await this.#storage.get(FEE_TOKEN_PREFERENCE_STORAGE_KEY, {}) - const feeSpeedPreference = await this.#storage.get( - FEE_SPEED_PREFERENCE_STORAGE_KEY, - FeeSpeed.Fast + this.feeSpeedPreference = sanitizeFeeSpeedPreference( + await this.#storage.get(FEE_SPEED_PREFERENCE_STORAGE_KEY, {}) ) - this.feeSpeedPreference = Object.values(FeeSpeed).includes(feeSpeedPreference) - ? feeSpeedPreference - : FeeSpeed.Fast this.emitUpdate() } catch (error) { this.emitError({ @@ -74,7 +88,7 @@ export class SignAccountOpPreferenceController extends EventEmitter { await update } - async setFeeSpeedPreference(feeSpeedPreference: FeeSpeed) { + async setFeeSpeedPreference(feeSpeedPreference: SignAccountOpFeeSpeedPreference) { this.feeSpeedPreference = feeSpeedPreference this.emitUpdate() diff --git a/src/controllers/signMessage/signMessage.test.ts b/src/controllers/signMessage/signMessage.test.ts index 5b21418fd0..774fff02a5 100644 --- a/src/controllers/signMessage/signMessage.test.ts +++ b/src/controllers/signMessage/signMessage.test.ts @@ -740,6 +740,7 @@ describe('SignMessageController', () => { { id: DAPP_VERIFICATION_BANNER_IDS.LOADING, type: 'warning', + title: 'Safety check in progress', text: "We're still verifying the app. Please wait, or make sure you trust it before signing requests." } ]) @@ -752,6 +753,7 @@ describe('SignMessageController', () => { { id: DAPP_VERIFICATION_BANNER_IDS.FAILED_TO_GET_OR_UNKNOWN, type: 'warning', + title: "App couldn't be verified", text: "We couldn't verify the app. Make sure you trust it before signing requests." } ]) @@ -764,6 +766,7 @@ describe('SignMessageController', () => { { id: DAPP_VERIFICATION_BANNER_IDS.BLACKLISTED, type: 'error', + title: 'Potentially harmful app', text: "This app didn't pass our safety check. Proceed at your own risk." } ]) @@ -790,6 +793,7 @@ describe('SignMessageController', () => { { id: DAPP_VERIFICATION_BANNER_IDS.SUSPICIOUS_HOSTING, type: 'warning', + title: 'Suspicious app hosting', text: 'This app is hosted on a shared platform commonly used for phishing. Be careful - do not sign unless you are certain you trust it.' } ]) @@ -851,6 +855,7 @@ describe('SignMessageController', () => { { id: DAPP_VERIFICATION_BANNER_IDS.LOADING, type: 'warning', + title: 'Safety check in progress', text: "We're still verifying the app. Please wait, or make sure you trust it before signing requests." } ]) diff --git a/src/controllers/swapAndBridge/socketApiMock.ts b/src/controllers/swapAndBridge/socketApiMock.ts index bc56f82fd2..09457895f7 100644 --- a/src/controllers/swapAndBridge/socketApiMock.ts +++ b/src/controllers/swapAndBridge/socketApiMock.ts @@ -83,6 +83,24 @@ export class SocketAPIMock { ] } + async getToken({ + address, + chainId + }: { + address: string + chainId: number + }): Promise { + return { + name: 'Token Not In The List', + address, + icon: '', + decimals: 18, + symbol: 'NOTLISTED', + chainId, + logoURI: '' + } + } + async quote({ fromChainId, fromTokenAddress, diff --git a/src/controllers/swapAndBridge/swapAndBridge.test.ts b/src/controllers/swapAndBridge/swapAndBridge.test.ts index 2294da038d..0a2b7c2614 100644 --- a/src/controllers/swapAndBridge/swapAndBridge.test.ts +++ b/src/controllers/swapAndBridge/swapAndBridge.test.ts @@ -451,6 +451,85 @@ describe('SwapAndBridge Controller', () => { swapAndBridgeController.reset() await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS) }) + test('should select a preselected to token that the service provider returns in another case', async () => { + // Trending tokens carry lowercased CoinGecko addresses, while the service providers return + // checksummed ones. + const toTokenAddr = '0xcbB7C0000aB88B473b1f5aFd9ef808440eed33Bf' + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS, { + preselectedToToken: { address: toTokenAddr.toLowerCase(), chainId: 8453n } + }) + + expect(swapAndBridgeController.toChainId).toEqual(8453) + expect(swapAndBridgeController.toSelectedToken?.address).toEqual(toTokenAddr) + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS) + }) + test('should select a preselected to token that is missing from the service provider list', async () => { + const toTokenAddr = '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984' + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS, { + preselectedToToken: { address: toTokenAddr, chainId: 8453n } + }) + + expect(swapAndBridgeController.toSelectedToken?.address).toEqual(toTokenAddr) + expect(swapAndBridgeController.toTokenShortList).toContainEqual( + expect.objectContaining({ address: toTokenAddr }) + ) + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS) + }) + test('should keep a preselected to token when the from token network changes', async () => { + const toTokenAddr = '0x94b008aA00579c1307B0EF2c499aD98a8ce58e58' // USDT + const fromToken = PORTFOLIO_TOKENS[2]! // ETH on Optimism + const fromTokenOnAnotherChain = PORTFOLIO_TOKENS[1]! // cbBTC on Base + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS, { + preselectedToken: { address: fromToken.address, chainId: fromToken.chainId }, + preselectedToToken: { address: toTokenAddr, chainId: 8453n } + }) + expect(swapAndBridgeController.fromChainId).toEqual(10) + expect(swapAndBridgeController.toSelectedToken?.address).toEqual(toTokenAddr) + + await swapAndBridgeController.updateForm({ fromSelectedToken: fromTokenOnAnotherChain }) + + expect(swapAndBridgeController.fromChainId).toEqual(8453) + expect(swapAndBridgeController.toChainId).toEqual(8453) + expect(swapAndBridgeController.toSelectedToken?.address).toEqual(toTokenAddr) + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS) + }) + test('should stop keeping a preselected to token after the user selects another one', async () => { + const toTokenAddr = '0x94b008aA00579c1307B0EF2c499aD98a8ce58e58' // USDT + const userSelectedToTokenAddr = '0x2Ae3F1Ec7F1F5012CFEab0185bfc7aa3cf0DEc22' // cbETH + const fromToken = PORTFOLIO_TOKENS[2]! // ETH on Optimism + const fromTokenOnAnotherChain = PORTFOLIO_TOKENS[1]! // cbBTC on Base + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS, { + preselectedToken: { address: fromToken.address, chainId: fromToken.chainId }, + preselectedToToken: { address: toTokenAddr, chainId: 8453n } + }) + await swapAndBridgeController.updateForm( + { toSelectedTokenAddr: userSelectedToTokenAddr }, + { isToSelectionByUser: true } + ) + expect(swapAndBridgeController.toSelectedToken?.address).toEqual(userSelectedToTokenAddr) + + await swapAndBridgeController.updateForm({ fromSelectedToken: fromTokenOnAnotherChain }) + + // The default behavior applies again - the to token gets reset on a from network change + expect(swapAndBridgeController.toSelectedToken).toBeNull() + + swapAndBridgeController.reset() + await swapAndBridgeController.updatePortfolioTokenList(PORTFOLIO_TOKENS) + }) test('should update toChainId', (done) => { let emitCounter = 0 const unsubscribe = swapAndBridgeController.onUpdate(async () => { @@ -731,6 +810,9 @@ describe('SwapAndBridge Controller', () => { expect(swapAndBridgeController.activeRoutes[0]!.routeStatus).toEqual('in-progress') expect(swapAndBridgeController.banners).toHaveLength(1) expect(swapAndBridgeController.banners[0]!.actions).toHaveLength(1) + expect(swapAndBridgeController.banners[0]!.meta?.accountAddr).toEqual( + '0x77777777789A8BBEE6C64381e5E89E501fb0e4c8' + ) }) describe('continuous active-route updates', () => { beforeEach(() => { diff --git a/src/controllers/swapAndBridge/swapAndBridge.ts b/src/controllers/swapAndBridge/swapAndBridge.ts index f96b79aba3..bf82e57daa 100644 --- a/src/controllers/swapAndBridge/swapAndBridge.ts +++ b/src/controllers/swapAndBridge/swapAndBridge.ts @@ -336,6 +336,13 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri */ #cachedSupportedChains: CachedSupportedChains = { lastFetched: 0, data: [] } + /** + * A "to" token preselected from outside Swap & Bridge (e.g. from the trending tokens list). + * Kept so it can be re-selected after the automatic "to" token list reset (which happens on + * every "from" network change), until the user changes the "to" token or network themselves. + */ + #preselectedToToken: { address: string; chainId: number } | null = null + routePriority: 'output' | 'time' = 'output' // Holds the initial load promise, so that one can wait until it completes @@ -985,6 +992,11 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri emitUpdate?: boolean updateQuote?: boolean shouldIncrementFromAmountUpdateCounter?: boolean + /** + * Set it when the user changes the "to" token or network from the UI, so a "to" token + * preselected from outside Swap & Bridge stops being restored on "from" network changes. + */ + isToSelectionByUser?: boolean } ) { const { @@ -1007,9 +1019,12 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri const { emitUpdate = true, updateQuote = true, - shouldIncrementFromAmountUpdateCounter = false + shouldIncrementFromAmountUpdateCounter = false, + isToSelectionByUser = false } = updateProps || {} + if (isToSelectionByUser) this.#preselectedToToken = null + const chainId = toChainId ?? this.toChainId let toSelectedTokenAddr: string | undefined @@ -1088,14 +1103,28 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri } } + // Only valid while the "to" network is still the one the token was preselected on + const preselectedToTokenAddrToRestore = + this.#preselectedToToken?.chainId === this.toChainId + ? this.#preselectedToToken.address + : undefined + const toTokensKey = this.#toTokenListKey const toTokenList = toTokensKey ? this.#toTokenList[toTokensKey] : undefined + // Addresses passed from outside Swap & Bridge (e.g. a trending token, which comes lowercased + // from CoinGecko) may be in a different case than the service provider's, so compare lowercased. const nextToToken = toTokenList - ? toTokenList.tokens.find((t) => t.address === toSelectedTokenAddr) + ? toTokenList.tokens.find( + (t) => t.address.toLowerCase() === toSelectedTokenAddr?.toLowerCase() + ) : null if (nextToToken) this.toSelectedToken = { ...nextToToken } + // The requested token isn't in the (possibly not yet fetched) list. Let updateToTokenList + // select it once the list is there, instead of dropping the request silently. + const shouldSelectToTokenFromList = !nextToToken && !!toSelectedTokenAddr + if (routePriority) { this.routePriority = routePriority if (this.quote) { @@ -1111,9 +1140,12 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri if (emitUpdate) this.#emitUpdateIfNeeded() await Promise.all([ - shouldUpdateToTokenList + shouldUpdateToTokenList || shouldSelectToTokenFromList ? // we put toSelectedTokenAddr so that "retry" btn functionality works - this.updateToTokenList(true, nextToToken?.address || toSelectedTokenAddr) + this.updateToTokenList( + shouldUpdateToTokenList, + nextToToken?.address || toSelectedTokenAddr || preselectedToTokenAddrToRestore + ) : undefined, updateQuote ? this.updateQuote({ @@ -1131,6 +1163,7 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri this.#setFromAmountAndNotifyUI('') this.#setFromAmountInFiatAndNotifyUI('') this.toSelectedToken = null + this.#preselectedToToken = null this.quote = null this.updateQuoteStatus = 'INITIAL' this.quoteRoutesStatuses = {} @@ -1175,6 +1208,13 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri const isSelectedTokenFalsyBeforeListUpdate = !this.fromSelectedToken && !!this.toSelectedToken const { preselectedToken, preselectedToToken, fromAmount } = params || {} + if (preselectedToToken) { + this.#preselectedToToken = { + address: preselectedToToken.address, + chainId: Number(preselectedToToken.chainId) + } + } + // When the price endpoint is down, tokens come back without a USD price. We must // not exclude them as "priceless" in that case, otherwise switching to an account // with such tokens would wrongly hide them. Skip the price requirement for chains @@ -1349,13 +1389,20 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri throw new SwapAndBridgeError(NETWORK_MISMATCH_MESSAGE) } - if (toTokenListKeyAtStart === this.#toTokenListKey && !this.toSelectedToken) { - if (addressToSelect) { - const token = toTokenList.tokens.find((t) => t.address === addressToSelect) - if (token) { - await this.updateForm({ toSelectedTokenAddr: token.address }, { emitUpdate: false }) - this.#emitUpdateIfNeeded() - } + if ( + toTokenListKeyAtStart === this.#toTokenListKey && + !this.toSelectedToken && + addressToSelect + ) { + // Compare lowercased, as the address may come from outside Swap & Bridge (e.g. a trending + // token) in a different case than the service provider's. + const token = + toTokenList.tokens.find((t) => t.address.toLowerCase() === addressToSelect.toLowerCase()) || + (await this.#fetchAndCacheToTokenToSelect(addressToSelect)) + + if (token) { + await this.updateForm({ toSelectedTokenAddr: token.address }, { emitUpdate: false }) + this.#emitUpdateIfNeeded() } } @@ -1427,6 +1474,38 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri return toTokenList.status } + /** + * Fetches a single token that the service provider left out of its "to" token list and caches it, + * so a token preselected from outside Swap & Bridge (e.g. from the trending tokens list) can + * still be selected. Fails silently, because the selection is not user-initiated. + */ + async #fetchAndCacheToTokenToSelect(address: string) { + if (!this.toChainId || !isAddress(address)) return null + + const toTokenListKey = this.#toTokenListKey + const tokenList = toTokenListKey ? this.#toTokenList[toTokenListKey] : undefined + + if (!tokenList) return null + + try { + const token = await this.#serviceProviderAPI.getToken({ address, chainId: this.toChainId }) + + if (!token) return null + + // Cache it the same way tokens added by address are cached + tokenList.apiTokens.push(token) + tokenList.tokens.push(token) + + return token + } catch (error: any) { + const { message } = getHumanReadableSwapAndBridgeError(error) + + this.emitError({ error, level: 'silent', message }) + + return null + } + } + async #addToTokenByAddress(address: string) { if (!this.toChainId) return // should never happen if (!isAddress(address)) return // no need to attempt with invalid addresses @@ -1600,6 +1679,8 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri async switchFromAndToTokens() { this.switchTokensStatus = 'LOADING' + // The user takes over the "to" token by switching the sides + this.#preselectedToToken = null this.#emitUpdateIfNeeded() const prevFromSelectedToken = this.fromSelectedToken ? { ...this.fromSelectedToken } : null @@ -2651,7 +2732,8 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri this.#selectedAccount.account, accountState, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) const swapSponsorship = getSwapSponsorship({ isErc4337Enabled: this.#featureFlags.isFeatureEnabled('erc4337'), @@ -2850,7 +2932,11 @@ export class SwapAndBridgeController extends EventEmitter implements ISwapAndBri // Swap banners aren't generated because swaps are completed instantly, // thus the activity banner on broadcast is sufficient - return getBridgeBanners(activeRoutesForSelectedAccount, callsUserRequests) + return getBridgeBanners( + activeRoutesForSelectedAccount, + callsUserRequests, + this.#selectedAccount.account.addr + ) } get #shouldAutoUpdateQuote() { diff --git a/src/controllers/transfer/transfer.ts b/src/controllers/transfer/transfer.ts index 06d2ef3376..6ea399ef0a 100644 --- a/src/controllers/transfer/transfer.ts +++ b/src/controllers/transfer/transfer.ts @@ -1125,7 +1125,8 @@ export class TransferController extends EventEmitter implements ITransferControl this.#selectedAccount.account, accountState, network, - this.#featureFlags.isFeatureEnabled('erc4337') + this.#featureFlags.isFeatureEnabled('erc4337'), + this.#featureFlags.isFeatureEnabled('eip7702') ) const accountOp = { id: generateUuid(), diff --git a/src/interfaces/dapp.ts b/src/interfaces/dapp.ts index 83b79d1e51..4dac62f3c6 100644 --- a/src/interfaces/dapp.ts +++ b/src/interfaces/dapp.ts @@ -64,6 +64,71 @@ export interface RecentDappEntry { openedAt: number } +/** + * Raw shape of a single item returned by the cena trending tokens endpoint + * (https://cena.ambire.com/api/v3/trending/). Only the fields the wallet consumes are typed; + * the endpoint returns more (sparkline, btc-denominated values, etc.) that we ignore. + */ +export interface RawTrendingToken { + id: string + name: string + symbol: string + market_cap_rank: number | null + image?: { thumb?: string; small?: string; large?: string } + /** + * Primary CoinGecko asset platform (e.g. 'ethereum') plus the per-platform contract addresses + * and decimals. Used to reuse the portfolio token-details components and match held balances. + * Null/absent for native coins without an on-chain contract (e.g. BNB). + */ + asset_platform_id?: string | null + contract_address?: string + platforms?: { [platform: string]: string } + decimals?: { [platform: string]: number } + homepage?: string[] + /** CoinGecko exchange identifiers the token is traded on, deduped server-side. */ + exchanges?: string[] + /** USD market data (flat). */ + usd?: number + usd_24h_change?: number + usd_market_cap?: number + usd_24h_vol?: number + usd_fully_diluted_valuation?: number + total_supply?: number + description?: { en?: string } | null +} + +/** Normalized trending token kept in the DappsController state and rendered by the UI. */ +export interface TrendingToken { + /** CoinGecko id (e.g. 'zignaly'); stable, used as the list key and details-screen lookup id. */ + id: string + name: string + symbol: string + icon: string + priceUSD: number + priceChange24hUSD: number | null + marketCapRank: number | null + description: string | null + /** + * Contract of the token on its primary CoinGecko asset platform, and that platform's CoinGecko + * id (e.g. 'ethereum'). Used to derive the chainId, resolve the token icon and match a held + * balance in the account portfolio. Null when the token has no on-chain contract (e.g. BTC). + */ + address: string | null + platformId: string | null + decimals: number | null + /** USD market data, mapped into the same numeric fields the portfolio "About" section reads. */ + marketCapUSD: number | null + totalVolumeUSD: number | null + fullyDilutedValuationUSD: number | null + totalSupply: number | null + website: string | null + /** + * CoinGecko exchange ids the token is traded on; resolved against the PortfolioController's + * exchange registry when rendering the supported-exchanges row. + */ + exchangeIds: string[] +} + export interface DefiLlamaProtocol { id: string name: string @@ -116,7 +181,9 @@ export interface HasUnverifiedDappsRes { export type DappVerificationBanner = { id: string type: 'error' | 'warning' + title?: string text: string + secondaryText?: string } export const DAPP_VERIFICATION_BANNER_IDS = { diff --git a/src/interfaces/domains.ts b/src/interfaces/domains.ts index d57a07cc2e..8dbeee92c1 100644 --- a/src/interfaces/domains.ts +++ b/src/interfaces/domains.ts @@ -48,6 +48,9 @@ type ReverseLookupOptions = { } type AddressState = { + /** + * fieldValue can contain a domain name. Keep in mind that it IS NOT normalized + */ fieldValue: string resolvedAddress: string resolvedAddressType: NameServiceId | null diff --git a/src/interfaces/keystore.ts b/src/interfaces/keystore.ts index 561678682a..6670dcd5e6 100644 --- a/src/interfaces/keystore.ts +++ b/src/interfaces/keystore.ts @@ -58,6 +58,12 @@ export interface ExternalSignerController { moveToResponseScan?: () => void //Qr based specific submitSignatureResponse?: (payload: string | Uint8Array) => void //Qr based specific parseAndSetAccountFromQR?: (payload: string | Uint8Array) => Promise //Qr based specific + nfcWalletType?: NfcWalletType // NFC (tap-to-sign card) specific + // Mark the start and the end of one account op's signing, so a device that unlocks + // with a PIN can keep it for that long and ask for it once instead of once per + // signature. NFC (tap-to-sign card) specific. + beginPinSession?: () => Promise + endPinSession?: () => Promise } export type ExternalSignerControllers = Partial<{ [key in Key['type']]: ExternalSignerController }> @@ -159,9 +165,11 @@ export type InternalKey = { export type QrWalletType = 'keystone' | 'imtoken' | 'keycard' // We can add more supported QR wallets here in the future, and they will be handled by the QrProtocolAdapter implementations, which are specific to each wallet type export type QrProtocolType = 'ur' | 'airgap' +export type NfcWalletType = 'keycard' // We can add more supported NFC (tap-to-sign) cards here in the future + export type ExternalKey = { addr: Account['addr'] - type: 'trezor' | 'ledger' | 'lattice' | 'qr' + type: 'trezor' | 'ledger' | 'lattice' | 'qr' | 'nfc' label: string dedicatedToOneSA: boolean meta: { @@ -174,6 +182,7 @@ export type ExternalKey = { qrWalletType?: QrWalletType qrProtocol?: QrProtocolType masterFingerprint?: string // BIP32 root fingerprint used to identify/verify the originating hardware wallet account set in QR flows + nfcWalletType?: NfcWalletType [key: string]: any } } @@ -188,6 +197,7 @@ export type KeystoreSeed = { seed: string seedPassphrase?: string | null hdPathTemplate: HD_PATH_TEMPLATE_TYPE + notBackedUp?: boolean } export type StoredKeystoreSeed = Omit & { @@ -202,6 +212,7 @@ export type KeystoreTempSeed = { seed: string seedPassphrase?: string | null hdPathTemplate: HD_PATH_TEMPLATE_TYPE + notBackedUp?: boolean } export type KeystoreSignerType = { diff --git a/src/interfaces/main.ts b/src/interfaces/main.ts index 61a8730c5c..5e35cdd895 100644 --- a/src/interfaces/main.ts +++ b/src/interfaces/main.ts @@ -6,10 +6,13 @@ export type IMainController = ControllerInterface< export const STATUS_WRAPPED_METHODS = { removeAccount: 'INITIAL', + updateAccounts: 'INITIAL', handleAccountPickerInitLedger: 'INITIAL', handleAccountPickerInitTrezor: 'INITIAL', handleAccountPickerInitLattice: 'INITIAL', handleAccountPickerInitQr: 'INITIAL', + handleAccountPickerInitNfc: 'INITIAL', importSmartAccountFromDefaultSeed: 'INITIAL', - selectAccount: 'INITIAL' + selectAccount: 'INITIAL', + accountPickerSetInitParamsFromNewSeed: 'INITIAL' } as const diff --git a/src/interfaces/safe.ts b/src/interfaces/safe.ts index 659e54ed1e..e944b15893 100644 --- a/src/interfaces/safe.ts +++ b/src/interfaces/safe.ts @@ -1,3 +1,4 @@ +import { Account } from './account' import { ControllerInterface } from './controller' import { Hex } from './hex' @@ -5,6 +6,10 @@ export type ISafeController = ControllerInterface< InstanceType > +export interface SafeAccountByOwner extends Account { + deployedOn: bigint[] +} + export interface SafeTx { to: Hex value: Hex diff --git a/src/interfaces/signAccountOp.ts b/src/interfaces/signAccountOp.ts index 2ebd02fd3e..d6db603904 100644 --- a/src/interfaces/signAccountOp.ts +++ b/src/interfaces/signAccountOp.ts @@ -55,6 +55,7 @@ type Warning = { id: string title: string text?: string + secondaryText?: string promptBefore?: ('sign' | 'one-click-sign')[] type?: Type } @@ -71,7 +72,9 @@ type SignAccountOpError = { type SignAccountOpBanner = { id: string type: 'error' | 'warning' + title?: string text: string + secondaryText?: string } type HardwareWalletSigningRequest = { @@ -82,10 +85,9 @@ type HardwareWalletSigningRequest = { enum TraceCallDiscoveryStatus { NotStarted = 'not-started', InProgress = 'in-progress', - SlowPendingResponse = 'slow-pending-response', Done = 'done', Failed = 'failed' } export { TraceCallDiscoveryStatus } -export type { Warning, SignAccountOpError, SignAccountOpBanner, HardwareWalletSigningRequest } +export type { HardwareWalletSigningRequest, SignAccountOpBanner, SignAccountOpError, Warning } diff --git a/src/interfaces/storage.ts b/src/interfaces/storage.ts index 8ec8f62584..4254a35944 100644 --- a/src/interfaces/storage.ts +++ b/src/interfaces/storage.ts @@ -17,7 +17,7 @@ import { Account, AccountId, AccountPreferences } from './account' import { AutoLoginPoliciesByAccount, AutoLoginSettings } from './autoLogin' import { Selectors } from './contractInfo' import { ControllerInterface } from './controller' -import { Dapp, RecentDappEntry } from './dapp' +import { Dapp, RecentDappEntry, TrendingToken } from './dapp' import { Domains } from './domains' import { Key, MainKeyEncryptedWithSecret, StoredKey, StoredKeystoreSeed } from './keystore' import { Network } from './network' @@ -57,6 +57,10 @@ export type StorageProps = { dappsV2: Dapp[] dapps: Dapp[] recentDapps: RecentDappEntry[] + trending: { + updatedAt: number + tokens: TrendingToken[] + } // Selected account dismissedBanners: (string | number)[] selectedAccount: string | null @@ -90,7 +94,9 @@ export type StorageProps = { signAccountOpFeeTokenPreference: { [chainId: string]: string | 'gasTank' } - signAccountOpFeeSpeedPreference: FeeSpeed + signAccountOpFeeSpeedPreference: { + [chainId: string]: FeeSpeed + } networks: { [key: string]: Network } accounts: Account[] networkPreferences: { [key: string]: Partial } diff --git a/src/libs/7702/7702.ts b/src/libs/7702/7702.ts index d79305749f..2731fb3f22 100644 --- a/src/libs/7702/7702.ts +++ b/src/libs/7702/7702.ts @@ -4,7 +4,7 @@ import { Network } from '../../interfaces/network' export function getContractImplementation( chainId: bigint, - accountKeys: { type: 'internal' | 'lattice' | 'trezor' | 'ledger' | 'qr' }[] + accountKeys: { type: 'internal' | 'lattice' | 'trezor' | 'ledger' | 'qr' | 'nfc' }[] ): Hex { if (accountKeys.find((key) => key.type === 'lattice')) { return EIP_7702_GRID_PLUS diff --git a/src/libs/account/BaseAccount.ts b/src/libs/account/BaseAccount.ts index ae34e36f92..f6981f0062 100644 --- a/src/libs/account/BaseAccount.ts +++ b/src/libs/account/BaseAccount.ts @@ -23,6 +23,13 @@ export abstract class BaseAccount { protected isErc4337Enabled: boolean + // when doing the 7702 activator/revoke, we should add the additional gas required + // for the authorization list: + // PER_EMPTY_ACCOUNT_COST: 25000 + // access list storage key: 1900 + // access list address: 2400 + ACTIVATOR_GAS_USED = 29300n + constructor( account: Account, network: Network, diff --git a/src/libs/account/EOA.ts b/src/libs/account/EOA.ts index d39c4207b0..6e45fb6807 100644 --- a/src/libs/account/EOA.ts +++ b/src/libs/account/EOA.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ import { ZeroAddress } from 'ethers' import { Hex } from '../../interfaces/hex' @@ -72,11 +71,15 @@ export class EOA extends BaseAccount { const isError = estimation instanceof Error if (isError || !estimation.providerEstimation || !options.op) return 0n + // add extra gas if the user wants to revoke + const isDelegating = options.op.meta && options.op.meta.setDelegation !== undefined + const revokeGas = isDelegating ? this.ACTIVATOR_GAS_USED : 0n + const calls = options.op.calls if (calls.length === 1) { const call = calls[0]! // ! as we check calls.length === 1 one line above // a normal transfer is 21k, so just return the providerEstimation - if (call.data === '0x') return estimation.providerEstimation.gasUsed + if (call.data === '0x') return estimation.providerEstimation.gasUsed + revokeGas } const ambireGasUsed = estimation.ambireEstimation ? estimation.ambireEstimation.gasUsed : 0n @@ -85,7 +88,7 @@ export class EOA extends BaseAccount { ? estimation.providerEstimation.gasUsed : ambireGasUsed // add a 10% overhead to prevent OOG - return gasUsed + gasUsed / 10n + return gasUsed + gasUsed / 10n + revokeGas } getBroadcastOption( @@ -94,6 +97,9 @@ export class EOA extends BaseAccount { op: AccountOp } ): string { + if (options.op.meta && options.op.meta.setDelegation !== undefined) + return BROADCAST_OPTIONS.delegation + return BROADCAST_OPTIONS.bySelf } diff --git a/src/libs/account/EOA7702.ts b/src/libs/account/EOA7702.ts index abcc9946e4..dc0f8f9776 100644 --- a/src/libs/account/EOA7702.ts +++ b/src/libs/account/EOA7702.ts @@ -21,13 +21,6 @@ import { isTransferredTokenFeeOption } from './feeOptions' // this class describes an EOA that CAN transition to 7702 // even if it is YET to transition to 7702 export class EOA7702 extends BaseAccount { - // when doing the 7702 activator, we should add the additional gas required - // for the authorization list: - // PER_EMPTY_ACCOUNT_COST: 25000 - // access list storage key: 1900 - // access list address: 2400 - ACTIVATOR_GAS_USED = 29300n - /** * Introduce a public variable we can use to make a simple check on the FE * whether this account type is 7702. diff --git a/src/libs/account/account.test.ts b/src/libs/account/account.test.ts index 93892d8d54..6a25243137 100644 --- a/src/libs/account/account.test.ts +++ b/src/libs/account/account.test.ts @@ -5,11 +5,19 @@ import { describe, expect, test } from '@jest/globals' import { DEFAULT_ACCOUNT_LABEL } from '../../consts/account' import { AMBIRE_ACCOUNT_FACTORY } from '../../consts/deploy' import { BIP44_STANDARD_DERIVATION_TEMPLATE } from '../../consts/derivation' -import { Account, AccountCreation, AccountOnPage, ImportStatus } from '../../interfaces/account' +import { + Account, + AccountCreation, + AccountOnchainState, + AccountOnPage, + AccountStates, + ImportStatus +} from '../../interfaces/account' import { dedicatedToOneSAPriv, Key } from '../../interfaces/keystore' import { getBytecode } from '../proxyDeploy/bytecode' import { getAmbireAccountAddress } from '../proxyDeploy/getAmbireAddressTwo' import { + canOrHasBecomeSmarter, getAccountDeployParams, getAccountImportStatus, getBasicAccount, @@ -434,4 +442,59 @@ describe('Account', () => { }) ).toBe(ImportStatus.NotImported) }) + + test('Should detect an EOA that can become smarter', () => { + const internalKey = { + addr: basicAccount.addr, + type: 'internal', + label: 'Account key', + dedicatedToOneSA: false, + isExternallyStored: false, + meta: { createdAt: null } + } as Key + + expect(canOrHasBecomeSmarter(basicAccount, {}, [internalKey])).toBe(true) + }) + + test('Should detect an EOA that has already become smarter', () => { + const accountStates = { + [basicAccount.addr]: { + '1': { isSmarterEoa: true } as AccountOnchainState + } + } as AccountStates + + expect(canOrHasBecomeSmarter(basicAccount, accountStates, [])).toBe(true) + }) + + test('Should not classify an unsupported EOA as smarter', () => { + const ledgerKey = { + addr: basicAccount.addr, + type: 'ledger', + label: 'Account key', + dedicatedToOneSA: false, + isExternallyStored: false, + meta: { + createdAt: null, + deviceId: 'device-id', + deviceModel: 'device-model', + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + index: 0 + } + } as Key + + expect(canOrHasBecomeSmarter(basicAccount, {}, [ledgerKey])).toBe(false) + }) + + test('Should not classify a V2 smart account as an EIP-7702 account', () => { + const smartAccount = { + ...basicAccount, + creation: { + factoryAddr: AMBIRE_ACCOUNT_FACTORY, + bytecode: '0x', + salt: '0x' + } + } + + expect(canOrHasBecomeSmarter(smartAccount, {}, [])).toBe(false) + }) }) diff --git a/src/libs/account/account.ts b/src/libs/account/account.ts index cf9848fcfd..bd2c6ea18f 100644 --- a/src/libs/account/account.ts +++ b/src/libs/account/account.ts @@ -352,6 +352,10 @@ export function hasBecomeSmarter(account: Account, state: AccountStates) { return false } +export function canOrHasBecomeSmarter(account: Account, state: AccountStates, keys: Key[]) { + return canBecomeSmarter(account, keys) || hasBecomeSmarter(account, state) +} + export function shouldUseStateOverrideForEOA(account: Account, state: AccountOnchainState) { return isBasicAccount(account, state) } diff --git a/src/libs/account/getBaseAccount.test.ts b/src/libs/account/getBaseAccount.test.ts new file mode 100644 index 0000000000..d15d0b42f8 --- /dev/null +++ b/src/libs/account/getBaseAccount.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test } from '@jest/globals' + +import { Account, AccountOnchainState } from '../../interfaces/account' +import { Network } from '../../interfaces/network' +import { EOA } from './EOA' +import { EOA7702 } from './EOA7702' +import { getBaseAccount } from './getBaseAccount' + +const accountAddr = '0x1111111111111111111111111111111111111111' + +const account = { + addr: accountAddr, + associatedKeys: [accountAddr], + initialPrivileges: [], + creation: null, + preferences: { label: 'Account', pfp: accountAddr } +} as Account + +const network = { + chainId: 1n, + has7702: true +} as Network + +const accountState = { + isEOA: true, + isSmarterEoa: false, + importedAccountKeys: [ + { + addr: accountAddr, + type: 'internal', + label: 'Account key', + dedicatedToOneSA: false, + isExternallyStored: false, + meta: { createdAt: null } + } + ] +} as AccountOnchainState + +describe('getBaseAccount', () => { + test('returns an EOA7702 when the account is eligible and ERC-7702 is enabled', () => { + expect(getBaseAccount(account, accountState, network, true, true)).toBeInstanceOf(EOA7702) + }) + + test('keeps ERC-7702 enabled when the setting has not been provided yet', () => { + expect(getBaseAccount(account, accountState, network, true, true)).toBeInstanceOf(EOA7702) + }) + + test('returns an EOA when the account is eligible but ERC-7702 is disabled', () => { + expect(getBaseAccount(account, accountState, network, true, false)).toBeInstanceOf(EOA) + }) + + test('returns an EOA for an existing onchain delegation when ERC-7702 is disabled', () => { + const delegatedAccountState = { ...accountState, isSmarterEoa: true } + + expect(getBaseAccount(account, delegatedAccountState, network, true, false)).toBeInstanceOf(EOA) + }) + + test('returns an EOA when the network does not support ERC-7702', () => { + const networkWithout7702 = { ...network, has7702: false } + + expect(getBaseAccount(account, accountState, networkWithout7702, true, true)).toBeInstanceOf( + EOA + ) + }) +}) diff --git a/src/libs/account/getBaseAccount.ts b/src/libs/account/getBaseAccount.ts index 5034cdaf1a..44a7a66b70 100644 --- a/src/libs/account/getBaseAccount.ts +++ b/src/libs/account/getBaseAccount.ts @@ -12,11 +12,15 @@ export function getBaseAccount( account: Account, accountState: AccountOnchainState, network: Network, - isErc4337Enabled: boolean + isErc4337Enabled: boolean, + isErc7702Enabled: boolean ): BaseAccount { if (account.safeCreation) return new Safe(account, network, accountState, isErc4337Enabled) if (accountState.isEOA) { - if (accountState.isSmarterEoa || canBecomeSmarterOnChain(network, account, accountState)) { + if ( + isErc7702Enabled && + (accountState.isSmarterEoa || canBecomeSmarterOnChain(network, account, accountState)) + ) { return new EOA7702(account, network, accountState, isErc4337Enabled) } diff --git a/src/libs/accountOp/balanceChanges.test.ts b/src/libs/accountOp/balanceChanges.test.ts index 439b7ecff3..97f76f379d 100644 --- a/src/libs/accountOp/balanceChanges.test.ts +++ b/src/libs/accountOp/balanceChanges.test.ts @@ -5,6 +5,8 @@ import { describe, expect, test } from '@jest/globals' import { TokenError, TokenResult } from '../portfolio/interfaces' import { getAccountOpBalanceChanges, getBalanceChangeTokenAddresses } from './balanceChanges' +const NATIVE_TOKEN_TRANSFER_LOG_ADDRESS = '0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE' + const buildToken = (overrides: Partial): TokenResult => ({ symbol: 'TOKEN', name: 'Token', @@ -66,6 +68,19 @@ describe('balanceChanges', () => { ]) }) + test('filters the native transfer log address on every chain', () => { + const usdc = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' + + expect(getBalanceChangeTokenAddresses([NATIVE_TOKEN_TRANSFER_LOG_ADDRESS, usdc], 1n)).toEqual([ + ZeroAddress, + usdc + ]) + expect(getBalanceChangeTokenAddresses([NATIVE_TOKEN_TRANSFER_LOG_ADDRESS, usdc])).toEqual([ + ZeroAddress, + usdc + ]) + }) + test('keeps native ETH snapshots while skipping Abstract native token alias', async () => { const accountAddr = '0xB674F3fd5F43464dB0448a57529eAF37F04cceA5' const abstractNativeToken = '0x000000000000000000000000000000000000800A' @@ -118,6 +133,49 @@ describe('balanceChanges', () => { ]) }) + test('keeps native snapshots while skipping the native transfer log address', async () => { + const accountAddr = '0xB674F3fd5F43464dB0448a57529eAF37F04cceA5' + const getTokenBalancesOnBlock = jest + .fn() + .mockImplementation(async (_accountId, _chainId, _tokenAddrs, blockTag) => [ + ok( + buildToken({ + symbol: 'USDC', + name: 'USD Coin', + address: ZeroAddress, + chainId: 5042002n, + amount: blockTag === 101 ? 9n : 10n + }) + ) + ]) + + const balanceChanges = await getAccountOpBalanceChanges({ + accountAddr, + chainId: 5042002n, + tokenAddrs: [ZeroAddress, NATIVE_TOKEN_TRANSFER_LOG_ADDRESS], + receiptBlockNumber: 101, + getTokenBalancesOnBlock + }) + + expect(getTokenBalancesOnBlock).toHaveBeenCalledTimes(2) + expect(getTokenBalancesOnBlock).toHaveBeenCalledWith( + accountAddr, + 5042002n, + [ZeroAddress], + expect.any(Number), + accountAddr + ) + expect(balanceChanges).toEqual([ + expect.objectContaining({ + address: ZeroAddress, + symbol: 'USDC', + amountBefore: 10n, + amountAfter: 9n, + balanceChange: -1n + }) + ]) + }) + test('computes expected balance changes on ethereum', async () => { const accountAddr = '0xB674F3fd5F43464dB0448a57529eAF37F04cceA5' const tokenAddrs = [ZeroAddress, '0xA0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'] @@ -532,6 +590,12 @@ describe('balanceChanges', () => { from: sender, to: accountAddr, value: 2500000n + }), + buildTransferLog({ + address: NATIVE_TOKEN_TRANSFER_LOG_ADDRESS, + from: accountAddr, + to: recipient, + value: 1000n }) ] const getTokenBalancesOnBlock = jest @@ -594,7 +658,7 @@ describe('balanceChanges', () => { expect(getTokenBalancesOnBlock).toHaveBeenCalledWith( accountAddr, 999n, - [ZeroAddress, usdcAddr], + [ZeroAddress, usdcAddr, NATIVE_TOKEN_TRANSFER_LOG_ADDRESS], 'latest', accountAddr ) diff --git a/src/libs/accountOp/balanceChanges.ts b/src/libs/accountOp/balanceChanges.ts index 47c7084192..4a9bc11bd0 100644 --- a/src/libs/accountOp/balanceChanges.ts +++ b/src/libs/accountOp/balanceChanges.ts @@ -9,33 +9,37 @@ export type { BalanceChangesReceipt, BalanceChangeTransferLog } from './hyperEvm const ABSTRACT_CHAIN_ID = 2741n const ABSTRACT_NATIVE_TOKEN_ADDRESS = '0x000000000000000000000000000000000000800A' +const NATIVE_TOKEN_TRANSFER_LOG_ADDRESS = '0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE' /** - * The ETH token on abstract is represented on an address - * that isn't a standard ERC-20 but it emits such a transfer log, - * causing our balance changes to break. We're fixing that here by - * omiting it + * Some native tokens emit ERC-20-shaped transfer logs from addresses that + * aren't standard ERC-20 contracts. Exclude the universal native transfer + * log address and Abstract's native token alias from ERC-20 balance checks. */ -const filterAbstractNativeTokenAlias = (tokenAddrs: string[], chainId?: bigint) => { - if (chainId !== ABSTRACT_CHAIN_ID) return tokenAddrs +const filterNativeTokenAliases = (tokenAddrs: string[], chainId?: bigint) => + tokenAddrs.filter((tokenAddr) => { + const normalizedTokenAddr = tokenAddr.toLowerCase() - return tokenAddrs.filter( - (tokenAddr) => tokenAddr.toLowerCase() !== ABSTRACT_NATIVE_TOKEN_ADDRESS.toLowerCase() - ) -} + if (normalizedTokenAddr === NATIVE_TOKEN_TRANSFER_LOG_ADDRESS.toLowerCase()) return false + + return ( + chainId !== ABSTRACT_CHAIN_ID || + normalizedTokenAddr !== ABSTRACT_NATIVE_TOKEN_ADDRESS.toLowerCase() + ) + }) export const getBalanceChangeTokenAddresses = ( tokenAddrs: string[], chainId?: bigint ): string[] => { - const tokenAddrsToNormalize = filterAbstractNativeTokenAlias(tokenAddrs, chainId) + const tokenAddrsToNormalize = filterNativeTokenAliases(tokenAddrs, chainId) return Array.from( new Set( [ZeroAddress, ...tokenAddrsToNormalize].map((tokenAddr) => { try { return getAddress(tokenAddr) - } catch (e) { + } catch { return null } }) @@ -149,7 +153,7 @@ export const getAccountOpBalanceChanges = async ({ debugTraceTransaction }) } - const balanceChangeTokenAddrs = filterAbstractNativeTokenAlias(tokenAddrs, chainId) + const balanceChangeTokenAddrs = filterNativeTokenAliases(tokenAddrs, chainId) const previousBlockNumber = prevBlockNumber ? prevBlockNumber : receiptBlockNumber > 0 diff --git a/src/libs/banners/banners.test.ts b/src/libs/banners/banners.test.ts new file mode 100644 index 0000000000..64073a8c3d --- /dev/null +++ b/src/libs/banners/banners.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, test } from '@jest/globals' + +import { Account, SafeAccountCreation } from '../../interfaces/account' +import { Banner } from '../../interfaces/banner' +import { SwapAndBridgeActiveRoute } from '../../interfaces/swapAndBridge' +import { DappConnectRequest, PlainTextMessageUserRequest } from '../../interfaces/userRequest' +import { + getBridgeBanners, + getCurrentAccountBanners, + getDappUserRequestsBanners, + getSafeMessageRequestBanners +} from './banners' + +const ACCOUNT_ADDR = '0x77777777789A8BBEE6C64381e5E89E501fb0e4c8' +const OTHER_ACCOUNT_ADDR = '0x8888888888888888888888888888888888888a' + +const account: Account = { + addr: ACCOUNT_ADDR, + associatedKeys: [], + initialPrivileges: [], + creation: null, + preferences: { label: '', pfp: '' } +} + +describe('getCurrentAccountBanners', () => { + test('keeps only banners matching the selected account', () => { + const banners: Banner[] = [ + { id: 1, type: 'info', title: '', actions: [], meta: { accountAddr: ACCOUNT_ADDR } }, + { id: 2, type: 'info', title: '', actions: [], meta: { accountAddr: OTHER_ACCOUNT_ADDR } } + ] + + expect(getCurrentAccountBanners(banners, ACCOUNT_ADDR)).toEqual([banners[0]]) + }) + + test('keeps banners without meta.accountAddr regardless of the selected account', () => { + const globalBanner: Banner = { id: 'global', type: 'info', title: '', actions: [] } + + expect(getCurrentAccountBanners([globalBanner], ACCOUNT_ADDR)).toEqual([globalBanner]) + expect(getCurrentAccountBanners([globalBanner], undefined)).toEqual([globalBanner]) + }) +}) + +describe('getDappUserRequestsBanners', () => { + test('tags the banner with the account it was built for', () => { + const dappConnectRequest: DappConnectRequest = { + id: 1, + kind: 'dappConnect', + meta: {}, + dappPromises: [ + { + id: 'testID', + resolve: () => {}, + reject: () => {}, + session: {} as DappConnectRequest['dappPromises'][0]['session'], + meta: {} + } + ] + } + + const banners = getDappUserRequestsBanners(account, [dappConnectRequest]) + + expect(banners).toHaveLength(1) + expect(banners[0]!.meta?.accountAddr).toEqual(ACCOUNT_ADDR) + }) +}) + +describe('getSafeMessageRequestBanners', () => { + test('tags the banner with the Safe account it was built for', () => { + const safeCreation: SafeAccountCreation = { + factoryAddr: '0x0', + singleton: '0x0', + saltNonce: '0x0', + setupData: '0x0', + version: '1.4.1' + } + const safeAccount: Account = { ...account, safeCreation } + const messageRequest: PlainTextMessageUserRequest = { + id: 1, + kind: 'message', + meta: { + params: { message: '0x0' }, + accountAddr: ACCOUNT_ADDR, + chainId: 1n + }, + dappPromises: [] + } + + const banners = getSafeMessageRequestBanners(safeAccount, [messageRequest]) + + expect(banners).toHaveLength(1) + expect(banners[0]!.meta?.accountAddr).toEqual(ACCOUNT_ADDR) + }) +}) + +describe('getBridgeBanners', () => { + test('tags the banner with the account the active routes belong to', () => { + const activeRoutes: SwapAndBridgeActiveRoute[] = [ + { + serviceProviderId: 'squid', + fromAssetAddress: '0x0', + toAssetAddress: '0x0', + steps: [], + sender: ACCOUNT_ADDR, + activeRouteId: 'route-1', + userTxIndex: 0, + userTxHash: null, + identifiedBy: null, + // Only the fields getIsBridgeRoute/getBridgeBanners actually read are filled in. + route: { + routeStatus: 'in-progress', + fromChainId: 1, + toChainId: 10, + currentUserTxIndex: 0, + transactionData: null, + userAddress: ACCOUNT_ADDR + } as unknown as SwapAndBridgeActiveRoute['route'], + routeStatus: 'in-progress' + } + ] + + const banners = getBridgeBanners(activeRoutes, [], ACCOUNT_ADDR) + + expect(banners).toHaveLength(1) + expect(banners[0]!.meta?.accountAddr).toEqual(ACCOUNT_ADDR) + }) +}) diff --git a/src/libs/banners/banners.ts b/src/libs/banners/banners.ts index d236e551c2..89d78b2e6f 100644 --- a/src/libs/banners/banners.ts +++ b/src/libs/banners/banners.ts @@ -19,7 +19,8 @@ export const getCurrentAccountBanners = (banners: Banner[], selectedAccount?: Ac export const getBridgeBanners = ( activeRoutes: SwapAndBridgeActiveRoute[], - callsUserRequests: CallsUserRequest[] + callsUserRequests: CallsUserRequest[], + accountAddr: AccountId ): Banner[] => { const isRouteTurnedIntoAccountOp = (route: SwapAndBridgeActiveRoute) => { return callsUserRequests.some((req) => { @@ -99,6 +100,9 @@ export const getBridgeBanners = ( category: 'bridge-in-progress', title, text, + meta: { + accountAddr + }, actions: [ { actionName: 'view-bridge', @@ -134,6 +138,9 @@ export const getSafeMessageRequestBanners = ( type: 'info', title: `You have ${requests.length} pending signature request${requests.length > 1 ? 's' : ''}`, text: '', + meta: { + accountAddr: account.addr + }, actions: [ { actionName: 'open-pending-dapp-requests', @@ -161,6 +168,9 @@ export const getDappUserRequestsBanners = ( type: 'info', title: `You have ${requests.length} pending app request${requests.length > 1 ? 's' : ''}`, text: '', + meta: { + accountAddr: account.addr + }, actions: [ { actionName: 'open-pending-dapp-requests', @@ -189,8 +199,9 @@ const getSafeBanner = ({ id: `${selectedAccount.addr}-${network.chainId.toString()}`, type: 'info', category: 'pending-to-be-signed-acc-op', - title: `${requestCount === 1 ? 'Pending transaction' : `${requestCount} Pending transactions`} on`, - meta: { chainId: network.chainId }, + // the network is rendered by the UI on a second row, below the title + title: requestCount === 1 ? 'Pending transaction' : `${requestCount} Pending transactions`, + meta: { chainId: network.chainId, accountAddr: selectedAccount.addr }, actions: [ { actionName: 'open-accountOp', @@ -250,11 +261,10 @@ export const getAccountOpBanners = ({ id: `${selectedAccount.addr}-${netId}`, type: 'info', category: 'pending-to-be-signed-acc-op', - title: `${ - callCount === 1 ? 'Pending transaction' : `${callCount} Pending transactions` - } on`, + // the network is rendered by the UI on a second row, below the title + title: callCount === 1 ? 'Pending transaction' : `${callCount} Pending transactions`, text: '', - meta: { chainId: network.chainId }, + meta: { chainId: network.chainId, accountAddr: selectedAccount.addr }, actions: [ { actionName: 'open-accountOp', diff --git a/src/libs/dapps/helpers.test.ts b/src/libs/dapps/helpers.test.ts new file mode 100644 index 0000000000..063443d268 --- /dev/null +++ b/src/libs/dapps/helpers.test.ts @@ -0,0 +1,92 @@ +import { expect } from '@jest/globals' + +import { predefinedDapps } from '../../consts/dapps/dapps' +import { getDappIdFromUrl, getNormalizedHostnameFromUrl, normalizeHostname } from './helpers' + +describe('dapps helpers', () => { + describe('normalizeHostname', () => { + it('strips the trailing dot of a fully-qualified hostname', () => { + expect(normalizeHostname('example.web.app.')).toBe('example.web.app') + expect(normalizeHostname('app.aave.com.')).toBe('app.aave.com') + }) + + it('strips repeated trailing dots', () => { + expect(normalizeHostname('example.web.app..')).toBe('example.web.app') + expect(normalizeHostname('example.web.app...')).toBe('example.web.app') + }) + + it('leaves an already canonical hostname untouched', () => { + expect(normalizeHostname('app.aave.com')).toBe('app.aave.com') + expect(normalizeHostname('localhost')).toBe('localhost') + expect(normalizeHostname('127.0.0.1')).toBe('127.0.0.1') + expect(normalizeHostname('')).toBe('') + }) + + it('does not touch dots that are not trailing', () => { + expect(normalizeHostname('sites.google.com')).toBe('sites.google.com') + }) + }) + + describe('getNormalizedHostnameFromUrl', () => { + it('returns the canonical hostname of a fully-qualified host', () => { + expect(getNormalizedHostnameFromUrl('https://example.web.app./')).toBe('example.web.app') + expect(getNormalizedHostnameFromUrl('https://example.web.app./claim?ref=1')).toBe( + 'example.web.app' + ) + expect(getNormalizedHostnameFromUrl('https://example.web.app.:8443/claim')).toBe( + 'example.web.app' + ) + }) + + it('normalizes casing and the ideographic full stop the URL parser maps to a dot', () => { + expect(getNormalizedHostnameFromUrl('https://ExAmple.WEB.App./')).toBe('example.web.app') + expect(getNormalizedHostnameFromUrl('https://example。web。app。/')).toBe('example.web.app') + }) + + it('keeps internationalized hostnames in punycode, as the phishing lists store them', () => { + expect(getNormalizedHostnameFromUrl('https://пример.бг./')).toBe('xn--e1afmkfd.xn--90ae') + expect(getNormalizedHostnameFromUrl('https://xn--e1afmkfd.xn--90ae./')).toBe( + 'xn--e1afmkfd.xn--90ae' + ) + }) + + it('ignores the userinfo part instead of reading it as the host', () => { + expect(getNormalizedHostnameFromUrl('https://app.uniswap.org@evil.com/')).toBe('evil.com') + }) + + it('returns null for urls the parser rejects', () => { + expect(getNormalizedHostnameFromUrl('not a url')).toBe(null) + expect(getNormalizedHostnameFromUrl('')).toBe(null) + }) + }) + + describe('getDappIdFromUrl', () => { + it('resolves a fully-qualified host to the same id as its canonical form', () => { + expect(getDappIdFromUrl('https://example.web.app./')).toBe('example.web.app') + expect(getDappIdFromUrl('https://example.web.app./')).toBe( + getDappIdFromUrl('https://example.web.app/') + ) + expect(getDappIdFromUrl('https://app.aave.com.')).toBe('app.aave.com') + }) + + it('strips www. from a fully-qualified host as well', () => { + expect(getDappIdFromUrl('https://www.example.web.app./')).toBe('example.web.app') + }) + + it('keeps resolving the existing cases', () => { + expect(getDappIdFromUrl('https://app.uniswap.org/swap')).toBe('app.uniswap.org') + expect(getDappIdFromUrl('https://www.aave.com')).toBe('aave.com') + expect(getDappIdFromUrl('internal')).toBe('internal') + expect(getDappIdFromUrl('')).toBe('internal') + }) + + it('still returns predefined ids by url', () => { + const predefinedDapp = predefinedDapps[0] + expect(getDappIdFromUrl(predefinedDapp.url)).toBe(predefinedDapp.id) + }) + + it('falls back to the raw input when it is not a url', () => { + expect(getDappIdFromUrl('not a url')).toBe('not a url') + }) + }) +}) diff --git a/src/libs/dapps/helpers.ts b/src/libs/dapps/helpers.ts index b5b5468812..b4bf65aa87 100644 --- a/src/libs/dapps/helpers.ts +++ b/src/libs/dapps/helpers.ts @@ -1,7 +1,45 @@ import { getDomain } from 'tldts' import { predefinedDapps } from '../../consts/dapps/dapps' -import { ConnectionSource, Dapp, DefiLlamaProtocol } from '../../interfaces/dapp' +import { + ConnectionSource, + Dapp, + DefiLlamaProtocol, + RawTrendingToken, + TrendingToken +} from '../../interfaces/dapp' + +/** + * Strips the trailing dot(s) a hostname may carry when written in fully-qualified form + * ("app.example.com."). DNS, TLS and the browser resolve such a host to the exact same site as the + * dotted-free form, but the WHATWG URL parser keeps the dot, so the resulting string matches none + * of the canonical forms the wallet compares against - the phishing blacklist, the suspicious + * hosting list and the stored dApp records. Left unnormalized, appending a single dot is enough to + * turn a known-malicious dApp into an unknown one. + */ +const normalizeHostname = (hostname: string): string => { + let end = hostname.length + while (end > 0 && hostname[end - 1] === '.') end -= 1 + + return hostname.slice(0, end) +} + +/** + * The dApp's canonical hostname, or `null` when the url is not parsable. + * + * Normalization deliberately starts from `new URL().hostname` - the parser already lowercases the + * host and converts internationalized ones to punycode, which is the form the phishing blacklist + * and the stored dApp records use. `tldts.getHostname()` also drops the trailing dot, but returns + * the unicode form of internationalized hostnames, so using it here would silently change the + * identity of every non-ASCII dApp. + */ +const getNormalizedHostnameFromUrl = (url: string): string | null => { + try { + return normalizeHostname(new URL(url).hostname) + } catch { + return null + } +} const getDappIdFromUrl = (url: string): string => { if (!url || url === 'internal') return 'internal' @@ -9,12 +47,10 @@ const getDappIdFromUrl = (url: string): string => { const predefinedDapp = predefinedDapps.find((d) => d.url === url) if (predefinedDapp) return predefinedDapp.id - try { - const { hostname } = new URL(url) - return hostname.startsWith('www.') ? hostname.slice(4) : hostname - } catch { - return url - } + const hostname = getNormalizedHostnameFromUrl(url) + if (hostname === null) return url + + return hostname.startsWith('www.') ? hostname.slice(4) : hostname } // Safe messages co-signed from another device carry only the dapp name and url @@ -154,7 +190,42 @@ function normalizeDappConnection(dapp: Dapp): Dapp { return { ...dapp, connectedSources, isConnected: connectedSources.length > 0 } } +// Maps the raw cena trending response to the UI-ready shape kept in state. +// Items without a usable id or price are dropped — they can't be keyed or displayed meaningfully. +function normalizeTrendingTokens(raw: RawTrendingToken[]): TrendingToken[] { + return raw + .filter((token) => !!token?.id && typeof token.usd === 'number') + .map((token) => { + const platformId = token.asset_platform_id ?? null + const address = + token.contract_address ?? (platformId ? token.platforms?.[platformId] : undefined) ?? null + const decimals = (platformId ? token.decimals?.[platformId] : undefined) ?? null + + return { + id: token.id, + name: token.name, + symbol: token.symbol, + icon: token.image?.large || token.image?.small || token.image?.thumb || '', + priceUSD: token.usd ?? 0, + priceChange24hUSD: typeof token.usd_24h_change === 'number' ? token.usd_24h_change : null, + marketCapRank: token.market_cap_rank ?? null, + description: token.description?.en ?? null, + address, + platformId, + decimals, + marketCapUSD: token.usd_market_cap ?? null, + totalVolumeUSD: token.usd_24h_vol ?? null, + fullyDilutedValuationUSD: token.usd_fully_diluted_valuation ?? null, + totalSupply: token.total_supply ?? null, + website: token.homepage?.find((url) => !!url) ?? null, + exchangeIds: (token.exchanges ?? []).filter((id): id is string => !!id) + } + }) +} + export { + normalizeHostname, + getNormalizedHostnameFromUrl, getDappIdFromUrl, getDappIconFromUrl, getDomainFromUrl, @@ -163,5 +234,6 @@ export { modifyDappPropsIfNeeded, getDappNameFromId, unifyDefiLlamaDappUrl, - normalizeDappConnection + normalizeDappConnection, + normalizeTrendingTokens } diff --git a/src/libs/erc7677/erc7677.ts b/src/libs/erc7677/erc7677.ts index e4274ba913..b57d7e8f63 100644 --- a/src/libs/erc7677/erc7677.ts +++ b/src/libs/erc7677/erc7677.ts @@ -1,5 +1,7 @@ import { toBeHex, toQuantity } from 'ethers' +import { generateUuid } from '@/utils/uuid' + import { ERC_4337_ENTRYPOINT } from '../../consts/deploy' import { Network } from '../../interfaces/network' import { getRpcProvider } from '../../services/provider' @@ -15,6 +17,11 @@ import { export const AMBIRE_SWAP_POLICY = 'ambireSwapSponsorship' +/** + * Currently, only the Gnosis chain is sponsored unconditionally + */ +export const AMBIRE_NETWORK_WIDE_SPONSORSHIP_POLICY = 'ambireGnosisSponsorship' + export function getPaymasterService( chainId: bigint, capabilities?: { paymasterService?: PaymasterCapabilities | PaymasterService } @@ -24,7 +31,7 @@ export function getPaymasterService( // this means it's v2 if ('url' in capabilities.paymasterService) { const paymasterService = capabilities.paymasterService - paymasterService.id = new Date().getTime() + paymasterService.id = generateUuid() return paymasterService } @@ -38,7 +45,7 @@ export function getPaymasterService( if (!foundChainId) return undefined const paymasterService = capabilities.paymasterService[foundChainId] - paymasterService.id = new Date().getTime() + paymasterService.id = generateUuid() return paymasterService } @@ -54,7 +61,10 @@ export function getAmbirePaymasterService( return { url: getAmbireSponsorshipUrl(relayerUrl), - id: new Date().getTime() + id: generateUuid(), + context: { + policyId: AMBIRE_NETWORK_WIDE_SPONSORSHIP_POLICY + } } } diff --git a/src/libs/erc7677/types.ts b/src/libs/erc7677/types.ts index 41f454e791..606ae794c7 100644 --- a/src/libs/erc7677/types.ts +++ b/src/libs/erc7677/types.ts @@ -9,7 +9,7 @@ export interface PaymasterService { decimals: number } } - id: number + id: string failed?: boolean } diff --git a/src/libs/estimate/estimate.test.ts b/src/libs/estimate/estimate.test.ts index f519a25a28..dfe739bb04 100644 --- a/src/libs/estimate/estimate.test.ts +++ b/src/libs/estimate/estimate.test.ts @@ -373,7 +373,7 @@ describe('estimate', () => { const accountStates = await getAccountsInfo([EOAAccount]) const accountState = accountStates[EOAAccount.addr]![ethereum.chainId.toString()]! - const baseAcc = getBaseAccount(EOAAccount, accountState, ethereum, true) + const baseAcc = getBaseAccount(EOAAccount, accountState, ethereum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -428,12 +428,13 @@ describe('estimate', () => { chainId: 137n, nonce: null, signature: null, - calls: [call] + calls: [call], + id: 'id' } const accountStates = await getAccountsInfo([EOAAccount]) const accountState = accountStates[EOAAccount.addr]![polygon.chainId.toString()]! - const baseAcc = getBaseAccount(EOAAccount, accountState, polygon, true) + const baseAcc = getBaseAccount(EOAAccount, accountState, polygon, true, true) const response = await getEstimation( baseAcc, accountState, @@ -503,12 +504,13 @@ describe('estimate', () => { chainId: 137n, nonce: null, signature: null, - calls: [call] + calls: [call], + id: 'id' } const accountStates = await getAccountsInfo([EOAAccount]) const accountState = accountStates[EOAAccount.addr]![polygon.chainId.toString()]! - const baseAcc = getBaseAccount(EOAAccount, accountState, polygon, true) + const baseAcc = getBaseAccount(EOAAccount, accountState, polygon, true, true) const response = await getEstimation( baseAcc, accountState, @@ -560,12 +562,13 @@ describe('estimate', () => { chainId: 137n, nonce: null, signature: null, - calls: [call] + calls: [call], + id: 'id' } const accountStates = await getAccountsInfo([EOAAccount]) const accountState = accountStates[EOAAccount.addr]![polygon.chainId.toString()]! - const baseAcc = getBaseAccount(EOAAccount, accountState, polygon, true) + const baseAcc = getBaseAccount(EOAAccount, accountState, polygon, true, true) const response = await getEstimation( baseAcc, accountState, @@ -598,7 +601,8 @@ describe('estimate', () => { chainId: 1n, nonce: await (v1AccAbi as any).nonce(), signature: spoofSig, - calls: [{ to: eoaAddr, value: BigInt(1), data: '0x' }] + calls: [{ to: eoaAddr, value: BigInt(1), data: '0x' }], + id: 'id' } const portfolioResponse = await portfolio.get('0xa07D75aacEFd11b425AF7181958F0F85c312f143') @@ -608,7 +612,7 @@ describe('estimate', () => { const accountStates = await getAccountsInfo([v1Acc]) const accountState = accountStates[v1Acc.addr]![ethereum.chainId.toString()]! - const baseAcc = getBaseAccount(v1Acc, accountState, ethereum, true) + const baseAcc = getBaseAccount(v1Acc, accountState, ethereum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -658,12 +662,13 @@ describe('estimate', () => { chainId: 1n, nonce: await (v1AccAbi as any).nonce(), signature: spoofSig, - calls: [{ to: eoaAddr, value: 1n, data: '0x' }] + calls: [{ to: eoaAddr, value: 1n, data: '0x' }], + id: 'id' } const accountStates = await getAccountsInfo([v1Acc]) const accountState = accountStates[v1Acc.addr]![ethereum.chainId.toString()]! - const baseAcc = getBaseAccount(v1Acc, accountState, ethereum, true) + const baseAcc = getBaseAccount(v1Acc, accountState, ethereum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -697,12 +702,13 @@ describe('estimate', () => { chainId: 1n, nonce: 1n, signature: spoofSig, - calls: [{ to: eoaAddr, value: BigInt(1), data: '0x' }] + calls: [{ to: eoaAddr, value: BigInt(1), data: '0x' }], + id: 'id' } const accountStates = await getAccountsInfo([viewOnlyAcc]) const accountState = accountStates[viewOnlyAcc.addr]![ethereum.chainId.toString()]! - const baseAcc = getBaseAccount(viewOnlyAcc, accountState, ethereum, true) + const baseAcc = getBaseAccount(viewOnlyAcc, accountState, ethereum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -765,7 +771,7 @@ describe('estimate', () => { const accountStates = await getAccountsInfo([accountOptimismv1]) const accountState = accountStates[accountOptimismv1.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(accountOptimismv1, accountState, optimism, true) + const baseAcc = getBaseAccount(accountOptimismv1, accountState, optimism, true, true) const response = await getEstimation( baseAcc, accountState, @@ -813,7 +819,7 @@ describe('estimate', () => { const accountStates = await getAccountsInfo([deprycatedV2]) const accountState = accountStates[deprycatedV2.addr]![arbitrum.chainId.toString()]! - const baseAcc = getBaseAccount(deprycatedV2, accountState, arbitrum, true) + const baseAcc = getBaseAccount(deprycatedV2, accountState, arbitrum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -852,7 +858,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([smartAccDeployed]) const accountState = accountStates[smartAccDeployed.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true, true) const response = await getEstimation( baseAcc, accountState, @@ -901,7 +907,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([smartAcc]) const accountState = accountStates[smartAcc.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true, true) const response = await getEstimation( baseAcc, accountState, @@ -955,7 +961,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([smartAcc]) const accountState = accountStates[smartAcc.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1005,7 +1011,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([smartAcc]) const accountState = accountStates[smartAcc.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1040,7 +1046,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([smartAccDeployed]) const accountState = accountStates[smartAccDeployed.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1090,7 +1096,7 @@ describe('estimate', () => { // corrupt the nonce to be lower accountState.erc4337Nonce = 6n - const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1138,7 +1144,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([deprycatedV2]) const accountState = accountStates[deprycatedV2.addr]![polygon.chainId.toString()]! - const baseAcc = getBaseAccount(deprycatedV2, accountState, polygon, true) + const baseAcc = getBaseAccount(deprycatedV2, accountState, polygon, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1173,7 +1179,9 @@ describe('estimate', () => { const baseAcc = getBaseAccount( { ...deprycatedV2, associatedKeys: [trezorSlot6v2NotDeployed.associatedKeys[0]!] }, accountState, - polygon + polygon, + true, + true ) const response = await getEstimation( baseAcc, @@ -1207,7 +1215,7 @@ describe('estimate', () => { const accountStates = await getAccountsInfo([v1Acc]) const accountState = accountStates[v1Acc.addr]![ethereum.chainId.toString()]! - const baseAcc = getBaseAccount(v1Acc, accountState, ethereum, true) + const baseAcc = getBaseAccount(v1Acc, accountState, ethereum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1246,7 +1254,7 @@ describe('estimate', () => { const gasGuardInterface = new Interface(gasGuardAbi) const accountStates = await getAccountsInfo([devconSmart]) const accountState = accountStates[devconSmart.addr]![bsc.chainId.toString()]! - const baseAcc = getBaseAccount(devconSmart, accountState, bsc, true) + const baseAcc = getBaseAccount(devconSmart, accountState, bsc, true, true) const bscProvider = getRpcProvider(bsc.rpcUrls, bsc.chainId) const switcher = new BundlerSwitcher(bsc, areUpdatesForbidden) @@ -1483,7 +1491,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([devconSmart]) const accountState = accountStates[devconSmart.addr]![ethereum.chainId.toString()]! - const baseAcc = getBaseAccount(devconSmart, accountState, ethereum, true) + const baseAcc = getBaseAccount(devconSmart, accountState, ethereum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1540,7 +1548,7 @@ describe('estimate', () => { } const accountStates = await getAccountsInfo([devconSmart]) const accountState = accountStates[devconSmart.addr]![ethereum.chainId.toString()]! - const baseAcc = getBaseAccount(devconSmart, accountState, ethereum, true) + const baseAcc = getBaseAccount(devconSmart, accountState, ethereum, true, true) const response = await getEstimation( baseAcc, accountState, @@ -1578,7 +1586,7 @@ describe('estimate', () => { const accountStates = await getAccountsInfo([localRelayer]) const accountState = accountStates[localRelayer.addr]!['25']! const cronos = networks.find((n) => n.chainId === 25n)! - const baseAcc = getBaseAccount(localRelayer, accountState, cronos, true) + const baseAcc = getBaseAccount(localRelayer, accountState, cronos, true, true) const response = await getEstimation( baseAcc, accountState, diff --git a/src/libs/estimate/estimateBundler.test.ts b/src/libs/estimate/estimateBundler.test.ts index 02b5050b4b..e174e8415c 100644 --- a/src/libs/estimate/estimateBundler.test.ts +++ b/src/libs/estimate/estimateBundler.test.ts @@ -116,7 +116,7 @@ describe('Bundler estimation tests', () => { ] const switcher = new BundlerSwitcher(optimism, areUpdatesForbidden) const accountState = accountStates[smartAcc.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAcc, accountState, optimism, true, true) const gasPrices = await fetchBundlerGasPrice(baseAcc, optimism, switcher) expect(gasPrices instanceof Error).toBe(false) const result = await bundlerEstimate( @@ -181,7 +181,7 @@ describe('Bundler estimation tests', () => { ] const switcher = new BundlerSwitcher(optimism, areUpdatesForbidden) const accountState = accountStates[smartAccDeployed.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true, true) const gasPrices = await fetchBundlerGasPrice(baseAcc, optimism, switcher) expect(gasPrices instanceof Error).toBe(false) const result = await bundlerEstimate( @@ -243,7 +243,7 @@ describe('Bundler estimation tests', () => { ] const switcher = new BundlerSwitcher(optimism, areUpdatesForbidden) const accountState = accountStates[smartAccDeployed.addr]![optimism.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, optimism, true, true) const gasPrices = await fetchBundlerGasPrice(baseAcc, optimism, switcher) expect(gasPrices instanceof Error).toBe(false) const result = await bundlerEstimate( @@ -324,7 +324,7 @@ describe('Bundler fallback tests', () => { ] const switcher = new ExtendedBundlerSwitcher(base, areUpdatesForbidden, [PIMLICO]) const accountState = accountStates[smartAccDeployed.addr]![base.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true, true) const gasPrices = await fetchBundlerGasPrice(baseAcc, base, switcher) expect(gasPrices instanceof Error).toBe(false) const result = await bundlerEstimate( diff --git a/src/libs/humanizer/erc7730/humanize.ts b/src/libs/humanizer/erc7730/humanize.ts index afb3b3161c..f63a15bc4e 100644 --- a/src/libs/humanizer/erc7730/humanize.ts +++ b/src/libs/humanizer/erc7730/humanize.ts @@ -32,6 +32,7 @@ import { genericErc20Humanizer } from '../modules/Tokens' import { eToNative, flattenHumanizerVisualizations, + getAction, getAddressVisualization, getChain, getErc7730Visualization, @@ -297,15 +298,6 @@ const valueToText = (value: unknown): string => { } } -const interpolatedValueToText = (path: string, value: unknown): string => { - const amount = toBigIntOrNull(value) - if ((path === '@.value' || path === '#.@.value') && amount !== null) { - return formatUnits(amount, 18) - } - - return valueToText(value) -} - const normalizeDecodedParam = (value: unknown, param: ParamType): unknown => { if (param.baseType === 'array' && param.arrayChildren) { return Array.from(value as ArrayLike).map((item) => @@ -969,38 +961,117 @@ const fieldsToRows = ( }, []) } -const interpolateIntent = ( +// Per the ERC-7730 spec's interpolation algorithm, each `{path}` placeholder +// must be resolved against the matching entry in the format's `fields` array +// (by path, after resolving `$ref`) so its declared `format`/`params` control +// how the value is rendered - not just the raw value. Only leaf fields are +// searched: interpolated intents may only reference always-visible paths, and +// those never live under a grouped/array `fields` sub-list. +const findInterpolationField = ( + fields: Erc7730Field[] | undefined, + path: string, + context: FormatContext +): Erc7730Field | null => { + if (!fields) return null + + for (const field of fields) { + const resolvedField = resolveFieldReference(field, context) + if (resolvedField.path === path) return resolvedField + } + + return null +} + +// Builds the ERC-7730 `interpolatedIntent` title as structured parts. Every +// placeholder uses the same field formatter as its corresponding detail row. +// Interpolation is all-or-nothing: malformed templates, unresolved paths, +// fields that are not always visible, or missing formatters return null so the +// UI can fall back to the static `intent` and its rows. +const interpolateIntentParts = ( template: string, + fields: Erc7730Field[] | undefined, context: FormatContext, base: unknown -): string | null => { - let interpolated = '' +): HumanizerVisualization[] | null => { + const parts: HumanizerVisualization[] = [] let currentIndex = 0 + // The leading word(s) of an interpolated intent are the verb ("Swap ", + // "Stake ", ...), so render them as an `action` part - same styling as the + // rest of the app's action verbs (e.g. getAction('Swap') in the Uniswap/ + // CowSwap/etc. modules) - instead of plain text. + const pushText = (text: string) => { + if (!text) return + parts.push(parts.length === 0 ? getAction(text) : getText(text)) + } + while (currentIndex < template.length) { const openingBraceIndex = template.indexOf('{', currentIndex) - if (openingBraceIndex === -1) { - interpolated += template.slice(currentIndex) + const closingBraceIndex = template.indexOf('}', currentIndex) + const nextBraceIndex = + openingBraceIndex === -1 + ? closingBraceIndex + : closingBraceIndex === -1 + ? openingBraceIndex + : Math.min(openingBraceIndex, closingBraceIndex) + + if (nextBraceIndex === -1) { + pushText(template.slice(currentIndex)) break } - const closingBraceIndex = template.indexOf('}', openingBraceIndex + 1) - if (closingBraceIndex === -1) { - interpolated += template.slice(currentIndex) - break + pushText(template.slice(currentIndex, nextBraceIndex)) + + const brace = template.charAt(nextBraceIndex) + const isEscapedBrace = template.charAt(nextBraceIndex + 1) === brace + if (isEscapedBrace) { + pushText(brace) + currentIndex = nextBraceIndex + 2 + continue } - interpolated += template.slice(currentIndex, openingBraceIndex) + if (brace === '}') return null + + const placeholderEndIndex = template.indexOf('}', nextBraceIndex + 1) + if (placeholderEndIndex === -1) return null + + const path = template.slice(nextBraceIndex + 1, placeholderEndIndex).trim() + if (!path || path.includes('{')) return null + + const matchingField = findInterpolationField(fields, path, context) + if ( + !matchingField || + (matchingField.visible !== undefined && matchingField.visible !== 'always') || + matchingField.fields?.length || + matchingField.format === 'calldata' + ) { + return null + } - const path = template.slice(openingBraceIndex + 1, closingBraceIndex).trim() const value = resolvePath(path, context, base) if (value === undefined) return null - interpolated += interpolatedValueToText(path, value) - currentIndex = closingBraceIndex + 1 + const formattedValue = formatFieldValue(matchingField, value, context, base) + if (!formattedValue.length) return null + if ( + (matchingField.format === 'amount' || matchingField.format === 'tokenAmount') && + !formattedValue.some((item) => item.type === 'token') + ) { + return null + } + if ( + (matchingField.format === 'addressName' || + matchingField.format === 'interoperableAddressName') && + !formattedValue.some((item) => item.type === 'address') + ) { + return null + } + parts.push(...formattedValue) + + currentIndex = placeholderEndIndex + 1 } - return interpolated + return parts.length ? parts : null } const formatToVisualizations = ( @@ -1008,14 +1079,20 @@ const formatToVisualizations = ( context: FormatContext, dapp?: Call['dapp'] ): HumanizerVisualization[] | null => { - const intent = - (format.interpolatedIntent && - interpolateIntent(format.interpolatedIntent, context, context.root)) || - format.intent + const titleParts = format.interpolatedIntent + ? interpolateIntentParts(format.interpolatedIntent, format.fields, context, context.root) + : null + // `format.intent` is the spec's plain, non-interpolated short title (e.g. + // "Swap") and is always used as `title` - it needs no token/decimals lookup, + // so it can never fail the way interpolation can. Consumers that need a rich, + // fully-interpolated title (e.g. "Swap 0.5 ETH for at least 120 USDC") must + // render `titleParts` instead; `title` is only the safe fallback text for + // non-rendering consumers (label comparisons, non-rich surfaces) and for + // when `titleParts` itself is null (interpolation couldn't be resolved). const rows = fieldsToRows(format.fields || [], context, context.root) if (!rows) return null - return [getErc7730Visualization(intent, rows, dapp)] + return [getErc7730Visualization(format.intent, rows, dapp, titleParts ?? undefined)] } const isOneInchFillOrderFormat = (formatKey: string, descriptorPath?: string) => diff --git a/src/libs/humanizer/index.test.ts b/src/libs/humanizer/index.test.ts index 1cfde01d9e..c0ac123ee3 100644 --- a/src/libs/humanizer/index.test.ts +++ b/src/libs/humanizer/index.test.ts @@ -611,6 +611,116 @@ describe('ERC-7730 descriptors', () => { ]) }) + // Real world tx (mainnet) calling LI.FI Diamond's swapTokensSingleV3NativeToERC20 + // (selector 0xaf7060fd - confirmed via 4byte.directory). Descriptor fields below + // are copied verbatim from the LI.FI entry in the ERC-7730 registry: + // https://github.com/ethereum/clear-signing-erc7730-registry/blob/master/registry/lifi/calldata-LIFIDiamond.json + // The descriptor's "interpolatedIntent" - "Swap {@.value} for at least + // {_minAmountOut} to {_receiver}" - exercises interpolateIntentParts()'s + // per-spec field lookup: {_minAmountOut} resolves through the "Minimum to + // Receive" field's tokenAmount format/tokenPath, so it renders as a `type: + // 'token'` titleParts item, matching the "Minimum to Receive" row below. + // `title` itself stays the plain, non-interpolated "Swap" intent. + test('humanizes a LI.FI swapTokensSingleV3NativeToERC20 call with its ERC-7730 registry descriptor', () => { + accountOp.calls = [ + { + to: '0x1231DEB6f5749EF6cE6943a275A1D3E7486F4EaE', + value: 5837776470906329n, + data: '0xaf7060fdedbb23ef4269219df4d4d0183bea7af79cc46298a7df01a4e949e02b9384f19b00000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000001000000000000000000000000006969174fd72466430a46e18234d0b530c9fd5f490000000000000000000000000000000000000000000000000014bd6d40d395d900000000000000000000000000000000000000000000000000000000000001600000000000000000000000000000000000000000000000000000000000000015616d626972652d657874656e73696f6e2d70726f640000000000000000000000000000000000000000000000000000000000000000000000000000000000002a307830303030303030303030303030303030303030303030303030303030303030303030303030303030000000000000000000000000000000000000000000000000000000000000000000005c57cf61e473ae865e733a3a23fbb7618b4621f60000000000000000000000005c57cf61e473ae865e733a3a23fbb7618b4621f60000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20000000000000000000000000000000000000000000000000014bd6d40d395d900000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000004d0e30db000000000000000000000000000000000000000000000000000000000' + } + ] + + const irCalls = humanizeAccountOp(accountOp, { + erc7730Descriptors: { + 0: { + path: 'registry/lifi/calldata-LIFIDiamond.json', + descriptor: { + display: { + formats: { + 'swapTokensSingleV3NativeToERC20(bytes32 _transactionId, string _integrator, string _referrer, address _receiver, uint256 _minAmountOut, (address callTo, address approveTo, address sendingAssetId, address receivingAssetId, uint256 fromAmount, bytes callData, bool requiresDeposit) _swapData)': + { + intent: 'Swap', + interpolatedIntent: + 'Swap {@.value} for at least {_minAmountOut} to {_receiver}', + fields: [ + { path: '@.value', label: 'Amount to send', format: 'amount' }, + { + path: '_minAmountOut', + label: 'Minimum to Receive', + format: 'tokenAmount', + params: { tokenPath: '_swapData.receivingAssetId' }, + visible: 'always' + }, + { + path: '_receiver', + label: 'Recipient', + format: 'addressName', + params: { types: ['eoa', 'contract'], sources: ['local', 'ens'] }, + visible: 'always' + }, + { path: '_transactionId', label: 'Transaction Id', visible: 'never' }, + { path: '_integrator', label: 'Integrator', visible: 'never' }, + { path: '_referrer', label: 'Referrer', visible: 'never' }, + { + path: '_swapData.callData', + label: 'Swap Data Call Data', + visible: 'never' + }, + { path: '_swapData.callTo', label: 'Swap Data Call To', visible: 'never' }, + { + path: '_swapData.approveTo', + label: 'Swap Data Approve To', + visible: 'never' + }, + { + path: '_swapData.requiresDeposit', + label: 'Swap Data Requires Deposit', + visible: 'never' + } + ] + } + } + } + } + } + } + }) + + compareHumanizerVisualizations(irCalls, [ + [ + getErc7730Visualization( + 'Swap', + [ + { + label: 'Amount to send', + value: [getToken(ZeroAddress, 5837776470906329n, 1n)] + }, + { + label: 'Minimum to Receive', + value: [getToken('0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', 5837776470906329n, 1n)] + }, + { + label: 'Recipient', + value: [getAddressVisualization('0x6969174fd72466430a46e18234d0b530c9fd5f49')] + } + ], + undefined, + // Structured title parts, so the UI can render the two amounts as + // `type: 'token'` items (live decimals/symbol lookup via TokenOrNft) + // instead of relying on a static, possibly incomplete token registry. + [ + getAction('Swap '), + getToken(ZeroAddress, 5837776470906329n, 1n), + getText(' for at least '), + getToken('0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', 5837776470906329n, 1n), + getText(' to '), + getAddressVisualization('0x6969174fd72466430a46e18234d0b530c9fd5f49') + ] + ) + ] + ]) + }) + test('adds the native transaction value when it is not already displayed', async () => { const call = { ...transactions.erc20[1]!, @@ -1168,17 +1278,125 @@ describe('ERC-7730 descriptors', () => { compareHumanizerVisualizations(irCalls, [ [ - getErc7730Visualization('Stake 0.001 ETH', [ - { - label: 'Amount', - value: [getToken(ZeroAddress, 1000000000000000n, 1n)] - } - ]) + getErc7730Visualization( + 'Stake ETH', + [ + { + label: 'Amount', + value: [getToken(ZeroAddress, 1000000000000000n, 1n)] + } + ], + undefined, + [getAction('Stake '), getToken(ZeroAddress, 1000000000000000n, 1n), getText(' ETH')] + ) ] ]) expect(irCalls[0]!.warnings).toEqual([]) }) + test.each([ + { + name: 'malformed braces', + interpolatedIntent: 'Stake {@.value ETH', + fields: [{ label: 'Amount', format: 'amount', path: '@.value' }] + }, + { + name: 'a missing field formatter', + interpolatedIntent: 'Stake {_referral}', + fields: [{ label: 'Amount', format: 'amount', path: '@.value' }] + }, + { + name: 'a field that is not always visible', + interpolatedIntent: 'Stake {@.value} ETH', + fields: [{ label: 'Amount', format: 'amount', path: '@.value', visible: 'optional' as const }] + }, + { + name: 'a token amount with an invalid token reference', + interpolatedIntent: 'Stake {_referral}', + fields: [ + { + label: 'Amount', + format: 'tokenAmount', + path: '_referral', + params: { token: 'not-an-address' } + } + ] + } + ])( + 'falls back to the static intent when interpolation has $name', + ({ interpolatedIntent, fields }) => { + accountOp.calls = [ + { + to: '0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84', + value: ethers.parseEther('0.001'), + data: '0xa1903eab00000000000000000000000011d00000000000000000000000000000000011d0' + } + ] + + const irCalls = humanizeAccountOp(accountOp, { + erc7730Descriptors: { + 0: { + descriptor: { + display: { + formats: { + 'submit(address _referral)': { + intent: 'Stake ETH', + interpolatedIntent, + fields + } + } + } + } + } + } + }) + const visualization = irCalls[0]!.fullVisualization?.find((item) => item.type === 'erc7730') + + expect(visualization).toMatchObject({ type: 'erc7730', title: 'Stake ETH' }) + if (visualization?.type !== 'erc7730') throw new Error('Expected ERC-7730 visualization') + expect(visualization.titleParts).toBeUndefined() + } + ) + + test('supports escaped braces in an interpolated intent', () => { + accountOp.calls = [ + { + to: '0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84', + value: ethers.parseEther('0.001'), + data: '0xa1903eab00000000000000000000000011d00000000000000000000000000000000011d0' + } + ] + + const irCalls = humanizeAccountOp(accountOp, { + erc7730Descriptors: { + 0: { + descriptor: { + display: { + formats: { + 'submit(address _referral)': { + intent: 'Stake ETH', + interpolatedIntent: 'Stake {{ETH}} {@.value}', + fields: [{ label: 'Amount', format: 'amount', path: '@.value' }] + } + } + } + } + } + } + }) + const visualization = irCalls[0]!.fullVisualization?.find((item) => item.type === 'erc7730') + + if (visualization?.type !== 'erc7730') throw new Error('Expected ERC-7730 visualization') + expect(visualization.titleParts?.map((item) => item.content || item.type)).toEqual([ + 'Stake ', + '{', + 'ETH', + '}', + ' ', + 'token' + ]) + }) + test('does not warn when an ERC-7730 descriptor displays the native transaction value', () => { accountOp.calls = [ { @@ -4196,17 +4414,19 @@ describe('ERC-7730 descriptors', () => { ]) }) - // Real Base mainnet SafeTx multisend (4 calls: Safe self-setup x2, an ERC-20 approval and an - // unrecognized settlement call) captured to catch a regression where calls that no humanizer - // module could recognize were silently dropped instead of falling back to an address+selector row. + // Real Base mainnet SafeTx multisend (4 calls: Safe self-setup x2 and an ERC-20 approval), with + // the last call swapped for a call to an unknown contract with an unknown selector, to catch a + // regression where calls that no humanizer module could recognize were silently dropped instead + // of falling back to an address+selector row. test('keeps all 4 calls of a real SafeTx multisend after humanization', async () => { const safeAddress = '0x2c5d356f2244b942c72ddfccbfa2e61529dc9c8d' const multiSend = '0x9641d764fc13c8b624c04430c7356c1c7c8102e2' const usdc = '0x833589fcd6edb6e08f4c7c32d4f71b54bda02913' const spender = '0xc92e8bdf79f0507f65a392b0ab4667716bfe0110' const settlementContract = '0xfdafc9d1902f4e0b84f65f49f244b32b31013b74' + const undecodableContract = '0xa1b2c3d4e5f60718293a4b5c6d7e8f9012345678' const multiSendData = - '0x8d80ff0a00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000464002c5d356f2244b942c72ddfccbfa2e61529dc9c8d00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000024f08a03230000000000000000000000002f55e8b20d0b9fefa187aa7d00b6cbe563605bf5002c5d356f2244b942c72ddfccbfa2e61529dc9c8d000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000443365582cd72ffa789b6fae41254d0b5a13e6e1e92ed947ec6a251edf1cf0b6c02c257b4b000000000000000000000000fdafc9d1902f4e0b84f65f49f244b32b31013b7400833589fcd6edb6e08f4c7c32d4f71b54bda0291300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044095ea7b3000000000000000000000000c92e8bdf79f0507f65a392b0ab4667716bfe0110ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff00fdafc9d1902f4e0b84f65f49f244b32b31013b74000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002640d0d9800000000000000000000000000000000000000000000000000000000000000008000000000000000000000000052ed56da04309aca4c3fecc595298d80c2f16bac000000000000000000000000000000000000000000000000000000000000024000000000000000000000000000000000000000000000000000000000000000010000000000000000000000006cf1e9ca41f7611def408122793c358a3d11e5a50000000000000000000000000000000000000000000000000000019fa3b9fe0100000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000140000000000000000000000000833589fcd6edb6e08f4c7c32d4f71b54bda02913000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee0000000000000000000000002c5d356f2244b942c72ddfccbfa2e61529dc9c8d00000000000000000000000000000000000000000000000000000000000f55c80000000000000000000000000000000000000000000000000001a02678851ac10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000007080000000000000000000000000000000000000000000000000000000000000000d1735c8b769e3b06acd45b0c09c76b4961b8215a15d6eaeefa05593ab382156500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' + '0x8d80ff0a00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000244002c5d356f2244b942c72ddfccbfa2e61529dc9c8d00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000024f08a03230000000000000000000000002f55e8b20d0b9fefa187aa7d00b6cbe563605bf5002c5d356f2244b942c72ddfccbfa2e61529dc9c8d000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000443365582cd72ffa789b6fae41254d0b5a13e6e1e92ed947ec6a251edf1cf0b6c02c257b4b000000000000000000000000fdafc9d1902f4e0b84f65f49f244b32b31013b7400833589fcd6edb6e08f4c7c32d4f71b54bda0291300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044095ea7b3000000000000000000000000c92e8bdf79f0507f65a392b0ab4667716bfe0110ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff00a1b2c3d4e5f60718293a4b5c6d7e8f901234567800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044b0f3f1c2000000000000000000000000000000000000000000000000000000000000002a0000000000000000000000006cf1e9ca41f7611def408122793c358a3d11e5a500000000000000000000000000000000000000000000000000000000' const safeTxMessage = { fromRequestId: 1, @@ -4312,24 +4532,11 @@ describe('ERC-7730 descriptors', () => { value: [getAddressVisualization(spender)] } ]), - getErc7730Visualization('Create CoW TWAP order', [ - { - label: 'Create CoW TWAP order', - value: [getToken(usdc, 2010000n, 8453n)] - }, - { - label: 'For at least', - value: [ - getToken( - '0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee', - 915124135802242n, - 8453n - ) - ] - }, + // The call no humanizer module can decode falls back to a plain "Interacting with" row + getErc7730Visualization('Interacting', [ { - label: 'And send it to', - value: [getAddressVisualization(safeAddress)] + label: 'With', + value: [getAddressVisualization(undecodableContract)] } ]) ] diff --git a/src/libs/humanizer/interfaces.ts b/src/libs/humanizer/interfaces.ts index b9de8f6450..71c999f409 100644 --- a/src/libs/humanizer/interfaces.ts +++ b/src/libs/humanizer/interfaces.ts @@ -11,7 +11,21 @@ export interface HumanizerErc7730Row { export interface HumanizerErc7730Visualization { type: 'erc7730' + // The format's plain, non-interpolated short title (e.g. "Swap") per the + // ERC-7730 spec's `intent`. It never needs a token/decimals lookup, so it's + // always safe to use as-is - this is the string other logic reads (label + // comparisons, heuristics, non-rich surfaces) and the fallback text when + // `titleParts` is absent. Prefer `titleParts` for display when present. title?: string + // The interpolated title (per the format's `interpolatedIntent`), split into + // renderable parts (text/token/address/...) so the UI can render a token + // amount with the same live decimals/symbol/price lookup used for row values + // (e.g. via a `type: 'token'` item), instead of requiring decimals to be + // statically known at humanization time. Present only when the format used + // `interpolatedIntent` AND every placeholder resolved successfully - per the + // spec, a failed interpolation falls back entirely to `title` above rather + // than leaking a raw/unformatted value into the UI. + titleParts?: HumanizerVisualization[] dapp?: Call['dapp'] rows: HumanizerErc7730Row[] } diff --git a/src/libs/humanizer/testHelpers.ts b/src/libs/humanizer/testHelpers.ts index 2b7b72688d..74955ce940 100644 --- a/src/libs/humanizer/testHelpers.ts +++ b/src/libs/humanizer/testHelpers.ts @@ -7,6 +7,7 @@ const stripVisualizationIds = (visualization: HumanizerVisualization): Humanizer return { ...strippedVisualization, + titleParts: strippedVisualization.titleParts?.map(stripVisualizationIds), rows: strippedVisualization.rows.map((row) => ({ ...row, value: row.value.map(stripVisualizationIds) diff --git a/src/libs/humanizer/utils.ts b/src/libs/humanizer/utils.ts index 92724527fc..2332802728 100644 --- a/src/libs/humanizer/utils.ts +++ b/src/libs/humanizer/utils.ts @@ -82,9 +82,10 @@ export function getText(text: string, mlMi?: boolean): HumanizerVisualization { export function getErc7730Visualization( title: string | undefined, rows: HumanizerErc7730Row[], - dapp?: IrCall['dapp'] + dapp?: IrCall['dapp'], + titleParts?: HumanizerVisualization[] ): HumanizerVisualization { - return { type: 'erc7730', title, dapp, rows, id: randomId() } + return { type: 'erc7730', title, titleParts, dapp, rows, id: randomId() } } export function flattenHumanizerVisualizations( diff --git a/src/libs/paymaster/paymaster.ts b/src/libs/paymaster/paymaster.ts index 844afee476..d4779a311c 100644 --- a/src/libs/paymaster/paymaster.ts +++ b/src/libs/paymaster/paymaster.ts @@ -1,5 +1,7 @@ import { AbiCoder, Contract, Interface, toBeHex, ZeroAddress } from 'ethers' +import { generateUuid } from '@/utils/uuid' + import AmbireFactory from '../../../contracts/compiled/AmbireFactory.json' import entryPointAbi from '../../../contracts/compiled/EntryPoint.json' import { FEE_COLLECTOR } from '../../consts/addresses' @@ -16,6 +18,7 @@ import { AccountOp } from '../accountOp/accountOp' import { Call } from '../accountOp/types' import { getFeeCall } from '../calls/calls' import { + AMBIRE_NETWORK_WIDE_SPONSORSHIP_POLICY, AMBIRE_SWAP_POLICY, getAmbireSponsorshipUrl, getPaymasterData, @@ -79,6 +82,8 @@ export class Paymaster extends AbstractPaymaster { op: AccountOp | null = null + #account: Account | null = null + paymasterService: PaymasterService | null = null network: Network | null = null @@ -100,6 +105,44 @@ export class Paymaster extends AbstractPaymaster { this.#relayerUrl = relayerUrl } + async #tryToSetErc7677(userOp: UserOperation) { + if (!this.paymasterService || !this.#account || !this.network) return + + try { + // when requesting stub data with an empty account, send over + // the deploy data as per EIP-7677 standard + const localOp = { ...userOp } + if (BigInt(localOp.nonce) === 0n && this.#account.creation) { + const factoryInterface = new Interface(AmbireFactory.abi) + localOp.factory = this.#account.creation.factoryAddr + localOp.factoryData = factoryInterface.encodeFunctionData('deploy', [ + this.#account.creation.bytecode, + this.#account.creation.salt + ]) + } + + let timeout: ReturnType | undefined + try { + const response = await Promise.race([ + getPaymasterStubData(this.paymasterService, localOp, this.network), + new Promise((_resolve, reject) => { + timeout = setTimeout( + () => reject(new Error('Sponsorship error, request too slow')), + 5000 + ) + }) + ]) + this.sponsorDataEstimation = response as PaymasterEstimationData + this.type = 'ERC7677' + return + } finally { + if (timeout !== undefined) clearTimeout(timeout) + } + } catch (e) { + console.log('ERC7677 sponsorship declined', e) + } + } + async init( op: AccountOp, userOp: UserOperation, @@ -110,37 +153,14 @@ export class Paymaster extends AbstractPaymaster { this.op = op this.network = network this.provider = provider + this.#account = account this.ambirePaymasterUrl = `/v2/paymaster/${this.network.chainId}/request` + if (op.meta?.paymasterService) this.paymasterService = op.meta.paymasterService + // try to init ERC-7677 if a paymasterService has been provided and it hasn't failed if (op.meta?.paymasterService && !op.meta?.paymasterService.failed) { - try { - this.paymasterService = op.meta.paymasterService - - // when requesting stub data with an empty account, send over - // the deploy data as per EIP-7677 standard - const localOp = { ...userOp } - if (BigInt(localOp.nonce) === 0n && account.creation) { - const factoryInterface = new Interface(AmbireFactory.abi) - localOp.factory = account.creation.factoryAddr - localOp.factoryData = factoryInterface.encodeFunctionData('deploy', [ - account.creation.bytecode, - account.creation.salt - ]) - } - - const response = await Promise.race([ - getPaymasterStubData(op.meta.paymasterService, localOp, network), - new Promise((_resolve, reject) => { - setTimeout(() => reject(new Error('Sponsorship error, request too slow')), 5000) - }) - ]) - this.sponsorDataEstimation = response as PaymasterEstimationData - this.type = 'ERC7677' - return - } catch (e) { - // TODO: error handling - console.log(e) - } + await this.#tryToSetErc7677(userOp) + if (this.type === 'ERC7677') return } // has the paymaster dried up @@ -382,21 +402,12 @@ export class Paymaster extends AbstractPaymaster { ) } - /** - * We use the upgrade method when we initially need to start with another - * paymaster type, e.g. Ambire, but then we understand we can use another - * one because special conditions apply. - * One such case is the swap&bridge where we first need to know the estimation - * from the bundler so we could calculate the txn fee. If the swap fee is - * bigger than the txn fee, we upgrade the paymaster to SwapSponsorship. - */ - async upgrade( + async #tryToSetSwapSponsorship( bundlerEstimateResult: BundlerEstimateResult, gasPrices: GasSpeeds, userOp: UserOperation - ): Promise { - // ERC7677 is already sponsoring the userOperation so we don't upgrade over it - if (!this.op?.meta?.swapSponsorship || this.type === 'ERC7677' || !this.network) return + ) { + if (!this.op?.meta?.swapSponsorship || !this.network) return const gas = BigInt(bundlerEstimateResult.callGasLimit) + BigInt(bundlerEstimateResult.preVerificationGas) @@ -425,7 +436,7 @@ export class Paymaster extends AbstractPaymaster { getPaymasterStubData( { url: getAmbireSponsorshipUrl(this.#relayerUrl), - id: new Date().getTime(), + id: generateUuid(), context: { policyId: AMBIRE_SWAP_POLICY, swapSponsorship: { @@ -451,4 +462,46 @@ export class Paymaster extends AbstractPaymaster { if (sponsorshipTimeout !== undefined) clearTimeout(sponsorshipTimeout) } } + + /** + * We use the upgrade method when we initially need to start with another + * paymaster type, e.g. Ambire, but then we understand we can use another + * one because special conditions apply. + * One such case is the swap&bridge where we first need to know the estimation + * from the bundler so we could calculate the txn fee. If the swap fee is + * bigger than the txn fee, we upgrade the paymaster to SwapSponsorship. + */ + async upgrade( + bundlerEstimateResult: BundlerEstimateResult, + gasPrices: GasSpeeds, + userOp: UserOperation + ): Promise { + // ERC7677 is already sponsoring the userOperation so we don't upgrade over it + if (this.type === 'ERC7677' || !this.network) return + + // do not upgrade over the gnosis paymaster sponsorship + if ( + this.type === 'Ambire' && + this.paymasterService?.context?.policyId === AMBIRE_NETWORK_WIDE_SPONSORSHIP_POLICY + ) + return + + // apply estimation and gas changes to the userOp so a more realistic, + // final userOp could be sent over for paymaster stub data + // + // do not use structuredClone here as we rely on mutating nested objects + const localOp = { ...userOp } + localOp.preVerificationGas = bundlerEstimateResult.preVerificationGas + localOp.verificationGasLimit = bundlerEstimateResult.verificationGasLimit + localOp.callGasLimit = bundlerEstimateResult.callGasLimit + localOp.maxFeePerGas = gasPrices.medium.maxFeePerGas + localOp.maxPriorityFeePerGas = gasPrices.medium.maxPriorityFeePerGas + + await this.#tryToSetSwapSponsorship(bundlerEstimateResult, gasPrices, localOp) + if (!!this.op?.meta?.swapSponsorship) return + + // try to init ERC-7677 if a paymasterService has been provided and it hasn't failed + if (this.op?.meta?.paymasterService && !this.op.meta.paymasterService.failed) + await this.#tryToSetErc7677(localOp) + } } diff --git a/src/libs/portfolio/getOnchainBalances.ts b/src/libs/portfolio/getOnchainBalances.ts index a26e385bb7..a4d9ae8e88 100644 --- a/src/libs/portfolio/getOnchainBalances.ts +++ b/src/libs/portfolio/getOnchainBalances.ts @@ -218,19 +218,27 @@ export async function getNFTs( // simulation was performed if the nonce is changed const hasSimulation = afterNonce !== beforeNonce - const simulationTokens: (CollectionResult & { addr: any })[] | null = hasSimulation - ? after.collections.map((simulationToken: any, tokenIndex: number) => ({ - ...mapNft(simulationToken, deltaAddressesMapping[tokenIndex]), - addr: deltaAddressesMapping[tokenIndex] - })) - : null + // Index all to prevent nested loops + const simulationTokensByAddr = new Map() + + if (hasSimulation) { + after.collections.forEach((simulationToken: any, tokenIndex: number) => { + const addr = deltaAddressesMapping[tokenIndex] + + if (addr === undefined) return + + const key = addr.toLowerCase() + + if (simulationTokensByAddr.has(key)) return + + simulationTokensByAddr.set(key, { ...mapNft(simulationToken, addr), addr }) + }) + } return [ before.collections.map((beforeToken: any, i: number) => { - const simulationToken = simulationTokens - ? simulationTokens.find( - (token: any) => token.addr.toLowerCase() === tokenAddrs[i]![0].toLowerCase() - ) + const simulationToken = hasSimulation + ? simulationTokensByAddr.get(tokenAddrs[i]![0].toLowerCase()) : null const token = mapNft(beforeToken, tokenAddrs[i]![0]) @@ -350,18 +358,22 @@ export async function getTokens( // simulation was performed if the nonce is changed const hasSimulation = afterNonce !== beforeNonce - const simulationTokens = hasSimulation - ? after.balances.map((simulationToken: any, tokenIndex: number) => ({ - ...simulationToken, - amount: simulationToken.amount, - addr: deltaAddressesMapping[tokenIndex] - })) - : null + // Index all to prevent nested loops + const simulationTokensByAddr = new Map() + + if (hasSimulation) { + after.balances.forEach((simulationToken: any, tokenIndex: number) => { + const addr = deltaAddressesMapping[tokenIndex] + + if (addr === undefined || simulationTokensByAddr.has(addr)) return + + simulationTokensByAddr.set(addr, { ...simulationToken, addr }) + }) + } + return [ before.balances.map((token: any, i: number) => { - const simulation = simulationTokens - ? simulationTokens.find((simulationToken: any) => simulationToken.addr === tokenAddrs[i]) - : null + const simulation = hasSimulation ? (simulationTokensByAddr.get(tokenAddrs[i]!) ?? null) : null const simulationAmount = simulation ? simulation.amount - token.amount : undefined const amountPostSimulation = simulation ? simulation.amount : token.amount diff --git a/src/libs/portfolio/helpers.ts b/src/libs/portfolio/helpers.ts index 4893cfa4b0..36d40397e0 100644 --- a/src/libs/portfolio/helpers.ts +++ b/src/libs/portfolio/helpers.ts @@ -521,12 +521,23 @@ export const erc721CollectionToLearnedAssetKeys = (collection: [string, bigint[] */ export const learnedErc721sToHints = (keys: string[]): ERC721s => { const hints: ERC721s = {} + // Split once and collect the enumerable collections up front. Checking for an + // enumerable key while building the hints would mean scanning every key for + // every key, and an account with many collections brings thousands of them. + const parsedKeys: [string, string | undefined][] = [] + const enumerableCollections = new Set() keys.forEach((key) => { const [collectionAddress, tokenId] = key.split(':') if (!collectionAddress) return + parsedKeys.push([collectionAddress, tokenId]) + + if (tokenId === 'enumerable') enumerableCollections.add(collectionAddress) + }) + + parsedKeys.forEach(([collectionAddress, tokenId]) => { if (tokenId === 'enumerable') { hints[collectionAddress] = [] @@ -535,7 +546,7 @@ export const learnedErc721sToHints = (keys: string[]): ERC721s => { // The key already exists as an enumerable hint. Example: // collectionA:enumerable exists and collectionB:id is attempted to be added // (it shouldn't be) - if (keys.includes(`${collectionAddress}:enumerable`)) { + if (enumerableCollections.has(collectionAddress)) { return } diff --git a/src/libs/portfolio/portfolio.test.ts b/src/libs/portfolio/portfolio.test.ts index 5e785eb249..35ac79df8a 100644 --- a/src/libs/portfolio/portfolio.test.ts +++ b/src/libs/portfolio/portfolio.test.ts @@ -181,6 +181,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -252,6 +253,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]![accountOp.chainId.toString()]! @@ -342,6 +344,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -390,6 +393,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -441,6 +445,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -463,6 +468,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -518,6 +524,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -567,6 +574,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -621,6 +629,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! @@ -676,6 +685,7 @@ describe('Portfolio', () => { account, accountStates[accountOp.accountAddr]!['1']!, ethereum, + true, true ), state: accountStates[accountOp.accountAddr]!['1']! diff --git a/src/libs/portfolio/portfolio.ts b/src/libs/portfolio/portfolio.ts index 1a9ce3504a..55b2169f00 100644 --- a/src/libs/portfolio/portfolio.ts +++ b/src/libs/portfolio/portfolio.ts @@ -1,6 +1,8 @@ import { ZeroAddress } from 'ethers' import { getAddress } from 'viem' +import { getFeeToken } from '@/libs/portfolio/tokenProcessing' + import BalanceGetter from '../../../contracts/compiled/BalanceGetter.json' import NFTGetter from '../../../contracts/compiled/NFTGetter.json' import gasTankFeeTokens from '../../consts/gasTankFeeTokens' @@ -245,17 +247,26 @@ export class Portfolio { ...Object.values(specialErc721Hints || {}) ]) - const checksummedErc20Hints = hints.erc20s - .map((address) => { - try { - // getAddress may throw an error. This will break the portfolio - // if the error isn't caught - return getAddress(address) - } catch { - return null - } - }) - .filter(Boolean) as string[] + // Deduped before checksumming for performance + const seenErc20Hints = new Set() + const checksummedErc20Hints: string[] = [] + + hints.erc20s.forEach((address) => { + try { + const lowercasedAddress = address.toLowerCase() + + if (seenErc20Hints.has(lowercasedAddress)) return + + // getAddress may throw an error. This will break the portfolio + // if the error isn't caught + const checksummedAddress = getAddress(address) + + seenErc20Hints.add(lowercasedAddress) + checksummedErc20Hints.push(checksummedAddress) + } catch { + // Not an address, so it can't be a token + } + }) // Merge static and dynamic blacklisted addresses for this chain const chainIdStr = this.network.chainId.toString() @@ -575,11 +586,7 @@ export class Portfolio { // return the native token if (t.address === ZeroAddress && t.chainId === this.network.chainId) return true - return gasTankFeeTokens.find( - (gasTankT) => - gasTankT.address.toLowerCase() === t.address.toLowerCase() && - gasTankT.chainId === t.chainId - ) + return getFeeToken(t.address, t.chainId) }), beforeNonce, afterNonce, diff --git a/src/libs/portfolio/simulation.test.ts b/src/libs/portfolio/simulation.test.ts index f551c38d4e..2e730257f0 100644 --- a/src/libs/portfolio/simulation.test.ts +++ b/src/libs/portfolio/simulation.test.ts @@ -160,7 +160,7 @@ describe('Portfolio simulation', () => { fetchPinned: false, simulation: { accountOps: [accountOp], - baseAccount: getBaseAccount(account, accountState, ethereum, true), + baseAccount: getBaseAccount(account, accountState, ethereum, true, true), state: accountState } }) diff --git a/src/libs/portfolio/tokenProcessing.test.ts b/src/libs/portfolio/tokenProcessing.test.ts new file mode 100644 index 0000000000..dd101f6687 --- /dev/null +++ b/src/libs/portfolio/tokenProcessing.test.ts @@ -0,0 +1,92 @@ +import { ZeroAddress } from 'ethers' + +import { describe, expect, it } from '@jest/globals' + +import gasTankFeeTokens from '../../consts/gasTankFeeTokens' +import { getFeeToken, getFlags } from './tokenProcessing' + +const USDT_ETHEREUM = '0xdAC17F958D2ee523a2206206994597C13D831ec7' +const WETH_OPTIMISM = '0x4200000000000000000000000000000000000006' +const DUPLICATED_ON_AVALANCHE = '0xB97EF9Ef8734C71904D8002F8b6Bc66Dd9c48a6E' +const NOT_A_FEE_TOKEN = '0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' + +describe('getFeeToken', () => { + it('returns the first of two entries sharing an address and a chain', () => { + const duplicates = gasTankFeeTokens.filter( + (t) => + t.address.toLowerCase() === DUPLICATED_ON_AVALANCHE.toLowerCase() && t.chainId === 43114n + ) + + expect(duplicates.length).toBeGreaterThan(1) + expect(getFeeToken(DUPLICATED_ON_AVALANCHE, 43114n)).toBe(duplicates[0]) + }) + + it('is case-insensitive on the given address', () => { + const usdt = gasTankFeeTokens.find( + (t) => t.address.toLowerCase() === USDT_ETHEREUM.toLowerCase() && t.chainId === 1n + ) + + expect(usdt).toBeDefined() + expect(getFeeToken(USDT_ETHEREUM.toLowerCase(), 1n)).toBe(usdt) + expect(getFeeToken(USDT_ETHEREUM.toUpperCase(), 1n)).toBe(usdt) + }) + + it('returns undefined for an address that is not a fee token', () => { + expect(getFeeToken(NOT_A_FEE_TOKEN, 1n)).toBeUndefined() + expect(getFeeToken(NOT_A_FEE_TOKEN, 1n)).toBeUndefined() + }) + + it('returns undefined when the address is a fee token but on another chain', () => { + const wethOnOptimism = gasTankFeeTokens.find( + (t) => t.address.toLowerCase() === WETH_OPTIMISM.toLowerCase() && t.chainId === 10n + ) + + expect(wethOnOptimism).toBeDefined() + expect(getFeeToken(WETH_OPTIMISM, 1n)).toBeUndefined() + expect(getFeeToken(WETH_OPTIMISM, 1n)).toBeUndefined() + }) + + it('reuses the index across calls instead of rebuilding it', () => { + expect(getFeeToken(USDT_ETHEREUM, 1n)).toBe(getFeeToken(USDT_ETHEREUM, 1n)) + }) +}) + +describe('getFlags fee token flags', () => { + it('marks a gas tank fee token as topped up and usable as a fee', () => { + const usdt = gasTankFeeTokens.find( + (t) => t.address.toLowerCase() === USDT_ETHEREUM.toLowerCase() && t.chainId === 1n + )! + + expect(usdt.disableGasTankDeposit).toBeFalsy() + expect(usdt.disableAsFeeToken).toBeFalsy() + + const flags = getFlags({}, '1', 1n, USDT_ETHEREUM, 'Tether USD', 'USDT') + + expect(flags.canTopUpGasTank).toBe(true) + expect(flags.isFeeToken).toBe(true) + expect(flags.onGasTank).toBe(false) + }) + + it('does not mark an unknown token as a fee token', () => { + const flags = getFlags({}, '1', 1n, NOT_A_FEE_TOKEN, 'Random', 'RND') + + expect(flags.canTopUpGasTank).toBe(false) + expect(flags.isFeeToken).toBeFalsy() + }) + + it('treats the native token as a fee token even without a gas tank entry', () => { + const flags = getFlags({}, '31337', 31337n, ZeroAddress, 'Ether', 'ETH') + + expect(getFeeToken(ZeroAddress, 31337n)).toBeUndefined() + expect(flags.isFeeToken).toBe(true) + expect(flags.canTopUpGasTank).toBe(false) + }) + + it('resolves fee tokens on the gasTank pseudo chain by the token chain id', () => { + const flags = getFlags({}, 'gasTank', 1n, USDT_ETHEREUM, 'Tether USD', 'USDT') + + expect(flags.onGasTank).toBe(true) + expect(flags.canTopUpGasTank).toBe(true) + expect(flags.isFeeToken).toBe(true) + }) +}) diff --git a/src/libs/portfolio/tokenProcessing.ts b/src/libs/portfolio/tokenProcessing.ts index 2fc2bc8860..5f8e48e16c 100644 --- a/src/libs/portfolio/tokenProcessing.ts +++ b/src/libs/portfolio/tokenProcessing.ts @@ -83,6 +83,34 @@ export const isSuspectedToken = ( return null } +let feeTokenIndex: Map | null = null + +const feeTokenKey = (address: string, chainId: string) => `${address.toLowerCase()}|${chainId}` + +const getFeeTokenIndex = () => { + if (feeTokenIndex) return feeTokenIndex + + feeTokenIndex = new Map() + + gasTankFeeTokens.forEach((feeToken) => { + const key = feeTokenKey(feeToken.address, feeToken.chainId.toString()) + + if (!feeTokenIndex!.has(key)) feeTokenIndex!.set(key, feeToken) + }) + + return feeTokenIndex +} + +/** + * Look up a gas-tank fee token by address and chain in O(1) + */ +export function getFeeToken( + address: string, + chainid: bigint +): (typeof gasTankFeeTokens)[number] | undefined { + return getFeeTokenIndex().get(feeTokenKey(address, chainid.toString())) +} + export function getFlags( networkData: any, chainId: string, @@ -101,11 +129,7 @@ export function getFlags( if (networkData?.walletClaimableBalance?.address.toLowerCase() === address.toLowerCase()) rewardsType = 'wallet-vesting' - const foundFeeToken = gasTankFeeTokens.find( - (t) => - t.address.toLowerCase() === address.toLowerCase() && - (isRewardsOrGasTank ? t.chainId === tokenChainId : t.chainId.toString() === chainId) - ) + const foundFeeToken = getFeeToken(address, tokenChainId) const canTopUpGasTank = !!foundFeeToken && !foundFeeToken?.disableGasTankDeposit && !rewardsType const isFeeToken = diff --git a/src/libs/requests/requests.ts b/src/libs/requests/requests.ts index 7abacbbc95..f5de88c830 100644 --- a/src/libs/requests/requests.ts +++ b/src/libs/requests/requests.ts @@ -1,3 +1,5 @@ +import { generateUuid } from '@/utils/uuid' + import { DappProviderRequest } from '../../interfaces/dapp' import { CallsUserRequest, @@ -87,7 +89,7 @@ export const buildSwitchAccountUserRequest = ({ dappPromises: UserRequest['dappPromises'] }): SwitchAccountRequest => { return { - id: new Date().getTime(), + id: generateUuid(), kind: 'switchAccount', meta: { accountAddr: selectedAccountAddr, diff --git a/src/libs/safe/safe.test.ts b/src/libs/safe/safe.test.ts index 9f3347e0c2..44793d28f9 100644 --- a/src/libs/safe/safe.test.ts +++ b/src/libs/safe/safe.test.ts @@ -1,9 +1,89 @@ -import { describe, expect, test } from '@jest/globals' +import { describe, expect, jest, test } from '@jest/globals' +import { getAddress } from 'ethers' + +import { Hex } from '../../interfaces/hex' +import { buildSafeMessageOrigin, parseSafeMessageOrigin } from './helpers' +import { getSafeAccountByOwner, normalizeSafeGlobalMessage } from './safe' import type { EIP712TypedData } from '@safe-global/types-kit' -import { buildSafeMessageOrigin, parseSafeMessageOrigin } from './helpers' -import { normalizeSafeGlobalMessage } from './safe' +const OWNER = '0xD8293ad21678c6F09Da139b4B62D38e514a03B78' as Hex +const OTHER_OWNER = '0x94b0080A00579C1307B0eF2C499AD98A8ce58e58' +const SAFE_ADDRESS = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48' + +const getSafeInfo = (owners = [OWNER]) => ({ + address: SAFE_ADDRESS, + fallbackHandler: '0x0000000000000000000000000000000000000000', + guard: '0x0000000000000000000000000000000000000000', + masterCopy: '0x0000000000000000000000000000000000000000', + modules: [], + nonce: 0, + owners, + threshold: 1, + version: '1.4.1' +}) + +const getSafeCreationInfo = () => ({ + created: '2025-01-01T00:00:00Z', + creator: OWNER, + factoryAddress: '0x1234567890123456789012345678901234567890', + saltNonce: '1', + setupData: '0x1234', + singleton: '0x2345678901234567890123456789012345678901', + transactionHash: `0x${'1'.repeat(64)}` +}) + +const createApi = (owners = [OWNER]) => ({ + getSafeCreationInfo: jest.fn(async () => getSafeCreationInfo()), + getSafeInfo: jest.fn(async () => getSafeInfo(owners)) +}) + +describe('getSafeAccountByOwner', () => { + test('falls back to the next deployed network when fetching Safe details fails', async () => { + const mainnetApi = createApi() + mainnetApi.getSafeInfo.mockRejectedValue(new Error('Service unavailable')) + const optimismApi = createApi() + const apiKitFactory = jest.fn((chainId: bigint) => (chainId === 1n ? mainnetApi : optimismApi)) + + const result = await getSafeAccountByOwner(SAFE_ADDRESS, OWNER, [1n, 10n], apiKitFactory) + + expect(mainnetApi.getSafeInfo).toHaveBeenCalledWith(SAFE_ADDRESS) + expect(mainnetApi.getSafeCreationInfo).not.toHaveBeenCalled() + expect(optimismApi.getSafeInfo).toHaveBeenCalledWith(SAFE_ADDRESS) + expect(optimismApi.getSafeCreationInfo).toHaveBeenCalledWith(SAFE_ADDRESS) + expect(result.account).toMatchObject({ + addr: getAddress(SAFE_ADDRESS), + associatedKeys: [OWNER], + deployedOn: [1n, 10n] + }) + expect(result.failed).toBe(false) + }) + + test('falls back to the next deployed network when the first one does not include the owner', async () => { + const mainnetApi = createApi([OTHER_OWNER]) + const optimismApi = createApi() + const apiKitFactory = jest.fn((chainId: bigint) => (chainId === 1n ? mainnetApi : optimismApi)) + + const result = await getSafeAccountByOwner(SAFE_ADDRESS, OWNER, [1n, 10n], apiKitFactory) + + expect(mainnetApi.getSafeInfo).toHaveBeenCalledWith(SAFE_ADDRESS) + expect(mainnetApi.getSafeCreationInfo).not.toHaveBeenCalled() + expect(optimismApi.getSafeInfo).toHaveBeenCalledWith(SAFE_ADDRESS) + expect(optimismApi.getSafeCreationInfo).toHaveBeenCalledWith(SAFE_ADDRESS) + expect(result.account?.addr).toBe(getAddress(SAFE_ADDRESS)) + expect(result.failed).toBe(false) + }) + + test('does not return an account when no deployed network includes the owner', async () => { + const api = createApi([OTHER_OWNER]) + + const result = await getSafeAccountByOwner(SAFE_ADDRESS, OWNER, [1n, 10n], () => api) + + expect(api.getSafeInfo).toHaveBeenCalledTimes(2) + expect(api.getSafeCreationInfo).not.toHaveBeenCalled() + expect(result).toEqual({ account: null, failed: false }) + }) +}) describe('normalizeSafeGlobalMessage', () => { test('converts a typed message domain chainId bigint to a decimal string', () => { @@ -74,9 +154,10 @@ describe('buildSafeMessageOrigin', () => { describe('parseSafeMessageOrigin', () => { test('parses name and url out of the JSON origin', () => { - expect( - parseSafeMessageOrigin('{"name":"Uniswap","url":"https://app.uniswap.org"}') - ).toEqual({ name: 'Uniswap', url: 'https://app.uniswap.org' }) + expect(parseSafeMessageOrigin('{"name":"Uniswap","url":"https://app.uniswap.org"}')).toEqual({ + name: 'Uniswap', + url: 'https://app.uniswap.org' + }) }) test('round-trips with buildSafeMessageOrigin', () => { diff --git a/src/libs/safe/safe.ts b/src/libs/safe/safe.ts index f95e81284a..ae74306d6b 100644 --- a/src/libs/safe/safe.ts +++ b/src/libs/safe/safe.ts @@ -17,10 +17,12 @@ import { SignTypedDataVersion, TypedDataUtils } from '@metamask/eth-sig-util' import SafeApiKit from '@safe-global/api-kit' import SafeAbi from '../../../contracts/compiled/Safe.json' +import { SAFE_API_TIMEOUT_MS } from '../../consts/safe' import { Hex } from '../../interfaces/hex' import { RPCProvider } from '../../interfaces/provider' -import { SafeTx } from '../../interfaces/safe' +import { SafeAccountByOwner, SafeTx } from '../../interfaces/safe' import { CallsUserRequest, TypedMessageUserRequest } from '../../interfaces/userRequest' +import { withTimeout } from '../../utils/with-timeout' import wait from '../../utils/wait' import { adaptTypedMessageForMetaMaskSigUtil } from '../signMessage/signMessage' import { decodeMultiSend, multiCallAbi, parseSafeMessageOrigin } from './helpers' @@ -62,6 +64,77 @@ export function getApiKit(chainId: bigint) { }) } +type SafeAccountApiKitFactory = ( + chainId: bigint +) => Pick, 'getSafeInfo' | 'getSafeCreationInfo'> + +export async function getSafeAccountByOwner( + safeAddr: string, + owner: Hex, + deployedOn: bigint[], + apiKitFactory: SafeAccountApiKitFactory = getApiKit +): Promise<{ account: SafeAccountByOwner | null; failed: boolean }> { + const getAccountFromChain = async ( + [chainId, ...remainingChainIds]: bigint[], + hasRequestFailed = false + ): Promise<{ + account: SafeAccountByOwner | null + failed: boolean + }> => { + if (chainId === undefined) return { account: null, failed: hasRequestFailed } + + const apiKit = apiKitFactory(chainId) + try { + const safeInfo = await withTimeout(() => apiKit.getSafeInfo(safeAddr), { + timeoutMs: SAFE_API_TIMEOUT_MS, + message: `Safe API: get Safe info timed out after ${SAFE_API_TIMEOUT_MS}ms` + }) + const address = getAddress(safeAddr.toLowerCase()) + const owners = safeInfo.owners.map((safeOwner: string) => getAddress(safeOwner.toLowerCase())) + if (!owners.some((safeOwner) => safeOwner === owner)) { + return getAccountFromChain(remainingChainIds, hasRequestFailed) + } + + const safeCreationInfo = await withTimeout(() => apiKit.getSafeCreationInfo(safeAddr), { + timeoutMs: SAFE_API_TIMEOUT_MS, + message: `Safe API: get Safe creation info timed out after ${SAFE_API_TIMEOUT_MS}ms` + }) + + return { + account: { + addr: address, + associatedKeys: owners, + initialPrivileges: owners.map((safeOwner) => [safeOwner, '0x01']), + creation: null, + safeCreation: { + factoryAddr: safeCreationInfo.factoryAddress as Hex, + singleton: safeCreationInfo.singleton as Hex, + setupData: safeCreationInfo.setupData as Hex, + saltNonce: safeCreationInfo.saltNonce + ? (toBeHex(BigInt(safeCreationInfo.saltNonce), 32) as Hex) + : (toBeHex(0, 32) as Hex), + version: safeInfo.version + }, + preferences: { + label: 'Safe', + pfp: address + }, + deployedOn + }, + failed: false + } + } catch (error) { + console.error( + `Failed to retrieve Safe account ${safeAddr} on network ${chainId.toString()}`, + error + ) + return getAccountFromChain(remainingChainIds, true) + } + } + + return getAccountFromChain(deployedOn) +} + export async function getCalculatedSafeAddress( creation: SafeCreationInfoResponse, provider: RPCProvider @@ -76,7 +149,8 @@ export async function getCalculatedSafeAddress( proxyCreationCode = await (factory as any).proxyCreationCode() } catch (e) { console.error( - `failed to call proxyCreationCode on Safe factory with addr: ${creation.factoryAddress}` + `failed to call proxyCreationCode on Safe factory with addr: ${creation.factoryAddress}`, + e ) return null } @@ -102,7 +176,7 @@ export function decodeSetupData(setupData: Hex): Hex[] { try { decoded = setupMethodInterface.decodeFunctionData('setup', setupData) } catch (e) { - console.error('failed to decode the Safe setup data') + console.error('failed to decode the Safe setup data', e) return [] } @@ -205,7 +279,7 @@ export async function getMessage({ messageHash: Hex }): Promise { const apiKit = getApiKit(chainId) - const msg = await apiKit.getMessage(messageHash).catch((e) => null) + const msg = await apiKit.getMessage(messageHash).catch(() => null) if (!msg) return null return { ...msg, diff --git a/src/libs/tracer/debugTraceCall.test.ts b/src/libs/tracer/debugTraceCall.test.ts index b7befcc629..ceaa559252 100644 --- a/src/libs/tracer/debugTraceCall.test.ts +++ b/src/libs/tracer/debugTraceCall.test.ts @@ -225,7 +225,7 @@ describe('Debug tracecall detection for transactions', () => { } } - const baseAccount = getBaseAccount(account, state, network, true) + const baseAccount = getBaseAccount(account, state, network, true, true) const res = await debugTraceCall(baseAccount, accountOp, network, state, overrideData) expect(res.nfts.length).toBe(1) diff --git a/src/services/bundlers/bundlerSwitcher.test.ts b/src/services/bundlers/bundlerSwitcher.test.ts index 5d195b1056..93a809ad9a 100644 --- a/src/services/bundlers/bundlerSwitcher.test.ts +++ b/src/services/bundlers/bundlerSwitcher.test.ts @@ -57,7 +57,7 @@ describe('bundler switcher: switch cases', () => { it('should switch when sign account op is in a ready to sign state and there are extra bundlers to switch to', async () => { const accountStates = await getAccountsInfo([smartAccDeployed]) const accountState = accountStates[smartAccDeployed.addr]![base.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true, true) const switcher = new BundlerSwitcher(base, () => { return false }) @@ -70,7 +70,7 @@ describe('bundler switcher: no switch cases', () => { it('should not switch when sign account op is in a signing state', async () => { const accountStates = await getAccountsInfo([smartAccDeployed]) const accountState = accountStates[smartAccDeployed.addr]![base.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true, true) const switcher = new BundlerSwitcher(base, () => { return true }) @@ -80,7 +80,7 @@ describe('bundler switcher: no switch cases', () => { it('should not switch when there is no extra bundler to switch to', async () => { const accountStates = await getAccountsInfo([smartAccDeployed]) const accountState = accountStates[smartAccDeployed.addr]![base.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true, true) const switcher = new BundlerSwitcher(avalanche, () => { return false }) @@ -90,7 +90,7 @@ describe('bundler switcher: no switch cases', () => { it('should not switch when there is no available bundler to switch to', async () => { const accountStates = await getAccountsInfo([smartAccDeployed]) const accountState = accountStates[smartAccDeployed.addr]![base.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true, true) const switcher = new DevBundlerSwitcher( base, () => { @@ -104,7 +104,7 @@ describe('bundler switcher: no switch cases', () => { it('should switch on an estimation error if there is a bundler available', async () => { const accountStates = await getAccountsInfo([smartAccDeployed]) const accountState = accountStates[smartAccDeployed.addr]![base.chainId.toString()]! - const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true) + const baseAcc = getBaseAccount(smartAccDeployed, accountState, base, true, true) const switcher = new DevBundlerSwitcher(base, () => { return false }) diff --git a/src/services/nameResolvers/nameResolvers.test.ts b/src/services/nameResolvers/nameResolvers.test.ts index 7985555f53..b997a40241 100644 --- a/src/services/nameResolvers/nameResolvers.test.ts +++ b/src/services/nameResolvers/nameResolvers.test.ts @@ -33,6 +33,30 @@ describe('matchNameResolver / getNameService', () => { // Fallback removed and no specific match -> nothing owns it. expect(matchNameResolver([namoshiResolver, gnsResolver], 'vitalik.eth')).toBeUndefined() }) + + it('routes case-insensitively, so an uppercase TLD reaches its service before normalization', () => { + expect(getNameService('SATOSHI.BTC')?.id).toBe('namoshi') + expect(getNameService('Nemo.Citrea')?.id).toBe('namoshi') + expect(getNameService('DONNOH.GWEI')?.id).toBe('gns') + expect(getNameService(' satoshi.btc ')?.id).toBe('namoshi') + }) +}) + +describe('resolver.normalize (per-service, ENSIP-15 for the ENS family)', () => { + it('lowercases and trims to the canonical form', () => { + expect(ensResolver.normalize('VITALIK.ETH')).toBe('vitalik.eth') + expect(namoshiResolver.normalize(' Satoshi.BTC ')).toBe('satoshi.btc') + expect(gnsResolver.normalize('Donnoh.GWEI')).toBe('donnoh.gwei') + }) + + it('applies UTS-46 mapping, not a plain toLowerCase (uppercase unicode folds to one form)', () => { + expect(ensResolver.normalize('BÜCHER.eth')).toBe(ensResolver.normalize('bücher.eth')) + }) + + it('returns null for an invalid name (disallowed character) instead of throwing', () => { + expect(ensResolver.normalize('has space.eth')).toBeNull() + expect(ensResolver.normalize('')).toBeNull() + }) }) describe('getPrimaryName', () => { diff --git a/src/services/nameResolvers/resolvers/EnsCompatibleResolver.ts b/src/services/nameResolvers/resolvers/EnsCompatibleResolver.ts index fc661e2240..736769f17e 100644 --- a/src/services/nameResolvers/resolvers/EnsCompatibleResolver.ts +++ b/src/services/nameResolvers/resolvers/EnsCompatibleResolver.ts @@ -1,3 +1,5 @@ +import { normalize as ensNormalize } from 'viem/ens' + import { FeatureFlags } from '@/consts/featureFlags' import { getEnsAvatar, @@ -64,6 +66,17 @@ export abstract class EnsCompatibleResolver implements NameResolver { abstract matches(domain: string): boolean + /** + * ENS-compatible services share ENSIP-15 (UTS-46) normalization. + */ + normalize(domain: string): string | null { + try { + return ensNormalize(domain.trim()) || null + } catch { + return null + } + } + requiredChainId(networkMode: NetworkMode): string | undefined { return this.chainId[networkMode] } diff --git a/src/services/nameResolvers/resolvers/GnsResolver.ts b/src/services/nameResolvers/resolvers/GnsResolver.ts index 49b2e50406..871a3c73f7 100644 --- a/src/services/nameResolvers/resolvers/GnsResolver.ts +++ b/src/services/nameResolvers/resolvers/GnsResolver.ts @@ -18,6 +18,6 @@ export class GnsResolver extends EnsCompatibleResolver { } matches(domain: string): boolean { - return domain.endsWith('.gwei') + return domain.trim().toLowerCase().endsWith('.gwei') } } diff --git a/src/services/nameResolvers/resolvers/NamoshiResolver.ts b/src/services/nameResolvers/resolvers/NamoshiResolver.ts index 94b4977fba..4eb6baf8b6 100644 --- a/src/services/nameResolvers/resolvers/NamoshiResolver.ts +++ b/src/services/nameResolvers/resolvers/NamoshiResolver.ts @@ -19,6 +19,7 @@ export class NamoshiResolver extends EnsCompatibleResolver { } matches(domain: string): boolean { - return domain.endsWith('.btc') || domain.endsWith('.citrea') + const tld = domain.trim().toLowerCase() + return tld.endsWith('.btc') || tld.endsWith('.citrea') } } diff --git a/src/services/nameResolvers/types.ts b/src/services/nameResolvers/types.ts index fd556e3d50..af769056be 100644 --- a/src/services/nameResolvers/types.ts +++ b/src/services/nameResolvers/types.ts @@ -44,6 +44,12 @@ export interface NameResolver { readonly isFallback?: boolean readonly capabilities: { reverse: boolean; avatar: boolean; expiry: boolean } matches(domain: string): boolean + /** + * Normalizes a name to this service's own canonical form. Returns null when the input isn't a valid + * name for the service. Each service owns its own normalization rules (ENS-compatible services use + * ENSIP-15); this keeps the controller and UI provider-agnostic rather than assuming ENS everywhere. + */ + normalize(domain: string): string | null /** * The chain this service resolves on for the given network mode, or undefined for a chain-agnostic * service. Lets the controller (which owns the enabled-networks list) detect a disabled network diff --git a/src/services/paymaster/FailedPaymasters.ts b/src/services/paymaster/FailedPaymasters.ts index 13c7df519b..bfbf84eea1 100644 --- a/src/services/paymaster/FailedPaymasters.ts +++ b/src/services/paymaster/FailedPaymasters.ts @@ -10,7 +10,7 @@ import { RPCProvider } from '../../interfaces/provider' // so the app can fallback to a standard Paymaster if a sponsorship fails export class FailedPaymasters { - failedSponsorshipIds: number[] = [] + failedSponsorshipIds: string[] = [] insufficientFundsNetworks: { [chainId: number]: { @@ -18,11 +18,11 @@ export class FailedPaymasters { } } = {} - addFailedSponsorship(id: number) { + addFailedSponsorship(id: string) { this.failedSponsorshipIds.push(id) } - hasFailedSponsorship(id: number): boolean { + hasFailedSponsorship(id: string): boolean { return this.failedSponsorshipIds.includes(id) } diff --git a/src/utils/domains.test.ts b/src/utils/domains.test.ts new file mode 100644 index 0000000000..137eee082c --- /dev/null +++ b/src/utils/domains.test.ts @@ -0,0 +1,75 @@ +import { getAddress } from 'ethers' + +import { expect } from '@jest/globals' + +import { Domains } from '../interfaces/domains' +import { + getAddressFromAddressState, + getDomainFromAddressState, + getResolvedDomainName +} from './domains' + +const ADDRESS = getAddress('0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045') + +describe('utils/domains', () => { + describe('getAddressFromAddressState', () => { + it('prefers the resolved address over the raw field value', () => { + expect( + getAddressFromAddressState({ resolvedAddress: ADDRESS, fieldValue: 'vitalik.eth' }) + ).toBe(ADDRESS) + }) + + it('falls back to the trimmed field value when nothing resolved', () => { + expect(getAddressFromAddressState({ resolvedAddress: '', fieldValue: ' 0xabc ' })).toBe( + '0xabc' + ) + }) + }) + + describe('getDomainFromAddressState', () => { + it('returns the field value when a domain resolved', () => { + expect( + getDomainFromAddressState({ resolvedAddressType: 'ens', fieldValue: 'vitalik.eth' }) + ).toBe('vitalik.eth') + }) + + it('returns undefined when nothing resolved (no resolvedAddressType)', () => { + expect( + getDomainFromAddressState({ resolvedAddressType: null, fieldValue: 'invalid.ethc' }) + ).toBeUndefined() + }) + }) + + describe('getResolvedDomainName', () => { + const domains: Domains = { + [ADDRESS]: { names: { ens: 'vitalik.eth', namoshi: null }, createdAt: 1, updatedAt: 1 } + } + + it('reads the resolver-normalized name stored for the resolved address and service', () => { + expect( + getResolvedDomainName(domains, { resolvedAddress: ADDRESS, resolvedAddressType: 'ens' }) + ).toBe('vitalik.eth') + }) + + it('checksums the address before lookup, so a lowercased resolvedAddress still matches', () => { + expect( + getResolvedDomainName(domains, { + resolvedAddress: ADDRESS.toLowerCase(), + resolvedAddressType: 'ens' + }) + ).toBe('vitalik.eth') + }) + + it('returns undefined when nothing resolved', () => { + expect( + getResolvedDomainName(domains, { resolvedAddress: '', resolvedAddressType: null }) + ).toBeUndefined() + }) + + it('returns undefined when the address has no name for that service', () => { + expect( + getResolvedDomainName(domains, { resolvedAddress: ADDRESS, resolvedAddressType: 'namoshi' }) + ).toBeUndefined() + }) + }) +}) diff --git a/src/utils/domains.ts b/src/utils/domains.ts index 00773eea3e..3ab5392a8b 100644 --- a/src/utils/domains.ts +++ b/src/utils/domains.ts @@ -1,4 +1,5 @@ -import { AddressState } from '../interfaces/domains' +import { AddressState, Domains } from '../interfaces/domains' +import { getAddressCaught } from './getAddressCaught' const getAddressFromAddressState = ( addressState: Pick @@ -16,4 +17,17 @@ const getDomainFromAddressState = ( return !!normalized ? normalized : undefined } -export { getAddressFromAddressState, getDomainFromAddressState } +/** + * Finds the normalized domain name matching the address state's resolved address and service type, if any. + */ +const getResolvedDomainName = ( + domains: Domains, + addressState: Pick +): string | undefined => { + const { resolvedAddress, resolvedAddressType } = addressState + if (!resolvedAddress || !resolvedAddressType) return undefined + + return domains[getAddressCaught(resolvedAddress)]?.names?.[resolvedAddressType] ?? undefined +} + +export { getAddressFromAddressState, getDomainFromAddressState, getResolvedDomainName } diff --git a/test/keystore.ts b/test/keystore.ts index 76256c4853..4fe79a7544 100644 --- a/test/keystore.ts +++ b/test/keystore.ts @@ -59,7 +59,6 @@ class InternalSigner { class LedgerSigner { key - // eslint-disable-next-line @typescript-eslint/no-empty-function constructor(_key: Key) { this.key = _key } diff --git a/test/recurringTimeout.ts b/test/recurringTimeout.ts index d5b4f98b92..708f70ede1 100644 --- a/test/recurringTimeout.ts +++ b/test/recurringTimeout.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-await-in-loop */ import { IRecurringTimeout } from '../src/classes/recurringTimeout/recurringTimeout' export const waitForFnToBeCalledAndExecuted = async (