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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/ts/background/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { METAMASK_ERROR_FAILED_TO_PARSE_REQUEST, METAMASK_ERROR_NOT_AUTHORIZED,
import { clearWebsiteConnectionIntent, finalizeWebsiteAccessChange, persistWebsiteAccessChange, sendAccountsChangedToPort, verifyAccess, withSuppressedUnscopedConnectionEventsForSocket } from './accessManagement.js'
import { getActiveAddressEntryForChain, getWalletActiveAddressEntryForChain } from './metadataUtils.js'
import { getActiveAddress } from './backgroundUtils.js'
import { assertNever } from '../utils/typescript.js'
import { assertNever, hasOwnKey } from '../utils/typescript.js'
import { JsonRpcResponseError, reportUnexpectedError, isFailedToFetchError } from '../utils/errors.js'
import { InterceptedRequest, type WebsiteSocket } from '../utils/requests.js'
import { replyToInterceptedRequest } from './messageSending.js'
Expand All @@ -32,7 +32,7 @@ import { getActiveAddressForCurrentSignerState, getConfirmedSignerStateToken, is
import { handleWatchAssetRequest, initializeWatchAssetWindowListeners, processWatchAssetQueue } from './windows/watchAsset.js'
import { getSafeModeRpcPolicyReply } from '../safe/safeRequestPolicy.js'
import { getWatchAssetRpcParseFailureReply } from './watchAssetRpc.js'
import { createMethodHandlerFor, hasOwnKey } from '../utils/methodHandlers.js'
import { createMethodHandlerFor } from '../utils/methodHandlers.js'
import { getWalletCapabilities } from './walletCapabilities.js'
import { getWalletGetCapabilitiesParseFailureReply } from './walletGetCapabilitiesRpc.js'
import { hasAccess as getWebsiteAccessApprovalState, hasAddressAccess as getWebsiteAddressAccessApprovalState } from './websiteAccessPolicy.js'
Expand Down
37 changes: 26 additions & 11 deletions app/ts/background/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Semaphore } from '../utils/semaphore.js'
import type { EthereumAddress } from '../types/wire-types.js'
import type { Website, WebsiteAccessArray } from '../types/websiteAccessTypes.js'
import type { BlockExplorer, RpcNetwork } from '../types/rpc.js'
import { type RichListElement, browserStorageLocalGet, browserStorageLocalSafeParseGet, browserStorageLocalSet } from '../utils/storageUtils.js'
import { type RichListElement, browserStorageLocalGet, browserStorageLocalSafeParse, browserStorageLocalSet } from '../utils/storageUtils.js'
import { getUserAddressBookEntries, updateUserAddressBookEntries } from './storageVariables.js'
import { getUniqueItemsByProperties } from '../utils/typed-arrays.js'
import type { AddressBookEntry } from '../types/addressBookTypes.js'
Expand All @@ -13,6 +13,7 @@ import { DEFAULT_ACTIVE_ADDRESSES, DEFAULT_BLOCK_MANIPULATION, DEFAULT_RPCS } fr
import { silenceChromeUnCaughtPromise } from '../utils/requests.js'
import { mergeStoredWebsiteMetadata, sanitizeWebsiteAccess } from '../utils/websiteIcons.js'
import type { SigningAddressPreference, SigningAddressPreferences } from '../types/signerTypes.js'
import { hasOwnKey } from '../utils/typescript.js'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor cohesion concern (low severity, non-blocking): this import pulls the generic object-own-key guard hasOwnKey from methodHandlers.js, a module whose purpose is RPC-method dispatch. Importing a general-purpose utility from that module into the settings/storage domain extends coupling across module boundaries and is a cohesion smell. This is noted as pre-existing utility misplacement rather than a regression introduced here, but a generic utils module (e.g. utils/typescript.ts) would be a cleaner home for hasOwnKey.

export const defaultActiveAddresses = DEFAULT_ACTIVE_ADDRESSES

Expand Down Expand Up @@ -54,9 +55,10 @@ type StartupStorageDefaults = {
signingAddressPreferences: SigningAddressPreferences
}

async function getParsedStorageValueOrDefault<Key extends keyof StartupStorageDefaults>(key: Key, defaultValue: StartupStorageDefaults[Key]): Promise<StartupStorageDefaults[Key]> {
const rawValue = (await browser.storage.local.get(key))[key]
const parsedValue: Readonly<Partial<StartupStorageDefaults>> | undefined = await browserStorageLocalSafeParseGet(key)
async function getParsedStorageValueOrDefaultFromItems<Key extends keyof StartupStorageDefaults>(storedItems: Readonly<Record<string, unknown>>, key: Key, defaultValue: StartupStorageDefaults[Key]): Promise<StartupStorageDefaults[Key]> {
const rawValue = storedItems[key]
const storedItem = hasOwnKey(storedItems, key) ? { [key]: rawValue } : {}
const parsedValue: Readonly<Partial<StartupStorageDefaults>> | undefined = browserStorageLocalSafeParse(storedItem)
if (parsedValue !== undefined && key in parsedValue) return parsedValue[key] as StartupStorageDefaults[Key]
if (rawValue === undefined) return defaultValue
console.warn(`${ key } was corrupt:`)
Expand All @@ -65,16 +67,29 @@ async function getParsedStorageValueOrDefault<Key extends keyof StartupStorageDe
return defaultValue
}

async function getParsedStorageValueOrDefault<Key extends keyof StartupStorageDefaults>(key: Key, defaultValue: StartupStorageDefaults[Key]): Promise<StartupStorageDefaults[Key]> {
return await getParsedStorageValueOrDefaultFromItems(await browser.storage.local.get(key), key, defaultValue)
}

export async function getSettings() : Promise<Settings> {
if (defaultRpcs[0] === undefined || defaultActiveAddresses[0] === undefined) throw new Error('default rpc or default address was missing')
const defaultPage: Page = { page: 'Home' }
const activeSimulationAddressPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefault('independentActiveSimulationAddress', defaultActiveAddresses[0].address))
const activeSigningSafeAddressPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefault('activeSigningSafeAddress', undefined))
const openedPagePromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefault('openedPageV2', defaultPage))
const useSignersAddressAsActiveAddressPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefault('useSignersAddressAsActiveAddress', false))
const websiteAccessPromise = silenceChromeUnCaughtPromise(getWebsiteAccess())
const simulationModePromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefault('simulationMode', defaultSimulationMode))
const activeRpcNetworkPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefault('activeRpcNetwork', defaultRpcs[0]))
const storedItems = await silenceChromeUnCaughtPromise(browser.storage.local.get([
'independentActiveSimulationAddress',
'activeSigningSafeAddress',
'openedPageV2',
'useSignersAddressAsActiveAddress',
'websiteAccess',
'simulationMode',
'activeRpcNetwork',
]))
const activeSimulationAddressPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefaultFromItems(storedItems, 'independentActiveSimulationAddress', defaultActiveAddresses[0].address))
const activeSigningSafeAddressPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefaultFromItems(storedItems, 'activeSigningSafeAddress', undefined))
const openedPagePromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefaultFromItems(storedItems, 'openedPageV2', defaultPage))
const useSignersAddressAsActiveAddressPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefaultFromItems(storedItems, 'useSignersAddressAsActiveAddress', false))
const websiteAccessPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefaultFromItems(storedItems, 'websiteAccess', []).then(sanitizeWebsiteAccess))
const simulationModePromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefaultFromItems(storedItems, 'simulationMode', defaultSimulationMode))
const activeRpcNetworkPromise = silenceChromeUnCaughtPromise(getParsedStorageValueOrDefaultFromItems(storedItems, 'activeRpcNetwork', defaultRpcs[0]))
const [activeSimulationAddress, activeSigningSafeAddress, openedPage, useSignersAddressAsActiveAddress, websiteAccess, activeRpcNetwork, simulationMode] = await Promise.all([
activeSimulationAddressPromise,
activeSigningSafeAddressPromise,
Expand Down
2 changes: 1 addition & 1 deletion app/ts/types/popupMessageProtocol.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { PopupMessage } from './interceptor-messages.js'
import { hasOwnKey } from '../utils/methodHandlers.js'
import { hasOwnKey } from '../utils/typescript.js'

export type PopupMessageDomain = 'address-book' | 'confirmation' | 'diagnostics' | 'home' | 'navigation' | 'safe' | 'settings' | 'simulation' | 'website-access'

Expand Down
4 changes: 0 additions & 4 deletions app/ts/utils/methodHandlers.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
type MethodValue = { readonly method: string }

export function hasOwnKey<ObjectType extends object>(value: ObjectType, key: PropertyKey): key is keyof ObjectType {
return Object.prototype.hasOwnProperty.call(value, key)
}

function hasMethod<Union extends MethodValue, Method extends Union['method']>(
value: Union,
method: Method,
Expand Down
9 changes: 6 additions & 3 deletions app/ts/utils/storageUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { UnexpectedErrorOccured } from '../types/interceptor-reply-messages.js'
import { InterceptorErrorDiagnostic } from '../types/errorDiagnostics.js'
import { InterceptedRequestForward } from '../types/interceptor-messages.js'
import { ICON_ACCESS_DENIED } from './constants.js'
import { hasOwnKey } from './methodHandlers.js'
import { hasOwnKey } from './typescript.js'

type IdsOfOpenedTabs = funtypes.Static<typeof IdsOfOpenedTabs>
const IdsOfOpenedTabs = funtypes.Intersect(
Expand Down Expand Up @@ -167,11 +167,14 @@ export async function browserStorageLocalSet2(items: LocalStorageItems2) {
export async function browserStorageLocalGet(keys: LocalStorageKey | LocalStorageKey[]): Promise<LocalStorageItems> {
return LocalStorageItems.parse(await browser.storage.local.get(Array.isArray(keys) ? keys : [keys]))
}
export async function browserStorageLocalSafeParseGet(keys: LocalStorageKey | LocalStorageKey[]): Promise<LocalStorageItems | undefined> {
const parsed = LocalStorageItems.safeParse(await browser.storage.local.get(Array.isArray(keys) ? keys : [keys]))
export function browserStorageLocalSafeParse(items: unknown): LocalStorageItems | undefined {
const parsed = LocalStorageItems.safeParse(items)
if (parsed.success) return parsed.value
return undefined
}
export async function browserStorageLocalSafeParseGet(keys: LocalStorageKey | LocalStorageKey[]): Promise<LocalStorageItems | undefined> {
return browserStorageLocalSafeParse(await browser.storage.local.get(Array.isArray(keys) ? keys : [keys]))
}

export async function browserStorageLocalRemove(keys: LocalStorageKey | LocalStorageKey[]) {
return await browser.storage.local.remove(Array.isArray(keys) ? keys : [keys])
Expand Down
4 changes: 4 additions & 0 deletions app/ts/utils/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ const isNumber = (value: unknown): value is number => typeof value === 'number'
export const isBigint = (value: unknown): value is bigint => typeof value === 'bigint'
export const isNumberOrBigint = (value: unknown): value is number | bigint => isNumber(value) || isBigint(value)

export function hasOwnKey<ObjectType extends object>(value: ObjectType, key: PropertyKey): key is keyof ObjectType {
return Object.prototype.hasOwnProperty.call(value, key)
}

function isObject(maybe: unknown): maybe is Object {
return typeof maybe === 'object' && maybe !== null && !Array.isArray(maybe)
}
Expand Down
2 changes: 1 addition & 1 deletion test/tests/backgroundAccountPermissions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,7 @@ describe('background eth_accounts', () => {
configurable: true,
value: async (keys?: string | string[] | Record<string, unknown> | null) => {
const result = await originalStorageGet(keys)
if (delayNextSafeAppsModeRead && keys === 'simulationMode') {
if (delayNextSafeAppsModeRead && (keys === 'simulationMode' || (Array.isArray(keys) && keys.includes('simulationMode')))) {
delayNextSafeAppsModeRead = false
delayedReadStarted.resolve(undefined)
await releaseDelayedRead.promise
Expand Down
8 changes: 1 addition & 7 deletions test/tests/methodHandlers.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as assert from 'assert'
import { test } from 'bun:test'
import { createMethodHandlerFor, hasOwnKey } from '../../app/ts/utils/methodHandlers.js'
import { createMethodHandlerFor } from '../../app/ts/utils/methodHandlers.js'

type TestMessage =
| { readonly method: 'double', readonly value: number }
Expand All @@ -22,9 +22,3 @@ test('method handler tables dispatch narrowed messages and reject mismatched dir
/Handler for double received length/,
)
})

test('handler table key checks reject properties inherited from Object.prototype', () => {
assert.equal(hasOwnKey(handlers, 'double'), true)
assert.equal(hasOwnKey(handlers, 'toString'), false)
assert.equal(hasOwnKey(handlers, 'constructor'), false)
})
76 changes: 76 additions & 0 deletions test/tests/settingsSnapshot.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import * as assert from 'assert'
import { test } from 'bun:test'

const firstSnapshot = {
independentActiveSimulationAddress: '0x1111111111111111111111111111111111111111',
activeSigningSafeAddress: '0x3333333333333333333333333333333333333333',
openedPageV2: { page: 'Home' },
useSignersAddressAsActiveAddress: false,
websiteAccess: [],
simulationMode: true,
activeRpcNetwork: {
name: 'First network',
chainId: '0x1',
httpsRpc: 'https://first.example',
currencyName: 'Ether',
currencyTicker: 'ETH',
primary: true,
minimized: true,
},
}

const secondSnapshot = {
independentActiveSimulationAddress: '0x2222222222222222222222222222222222222222',
activeSigningSafeAddress: '0x4444444444444444444444444444444444444444',
openedPageV2: { page: 'Settings' },
useSignersAddressAsActiveAddress: true,
websiteAccess: [],
simulationMode: false,
activeRpcNetwork: {
name: 'Second network',
chainId: '0xa',
httpsRpc: 'https://second.example',
currencyName: 'Ether',
currencyTicker: 'ETH',
primary: true,
minimized: true,
},
}

let storageReadCount = 0

Object.defineProperty(globalThis, 'browser', {
configurable: true,
writable: true,
value: {
storage: {
local: {
async get(keys: string | readonly string[]) {
const snapshot = storageReadCount++ === 0 ? firstSnapshot : secondSnapshot
const requestedKeys = Array.isArray(keys) ? keys : [keys]
return Object.fromEntries(Object.entries(snapshot).filter(([key]) => requestedKeys.includes(key)))
},
async set() {
throw new Error('Valid settings should not require repair writes')
},
},
},
},
})

const { getSettings } = await import('../../app/ts/background/settings.js')

test('getSettings returns one atomic browser storage snapshot', async () => {
storageReadCount = 0

const settings = await getSettings()

assert.equal(storageReadCount, 1)
assert.equal(settings.activeSimulationAddress, 0x1111111111111111111111111111111111111111n)
assert.equal(settings.activeSigningSafeAddress, 0x3333333333333333333333333333333333333333n)
assert.deepEqual(settings.openedPage, { page: 'Home' })
assert.equal(settings.useSignersAddressAsActiveAddress, false)
assert.equal(settings.simulationMode, true)
assert.equal(settings.activeRpcNetwork.name, 'First network')
assert.equal(settings.activeRpcNetwork.chainId, 1n)
})
9 changes: 8 additions & 1 deletion test/tests/typescriptUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import { describe, test } from 'bun:test'
import * as assert from 'assert'
import { isNumberOrBigint } from '../../app/ts/utils/typescript.js'
import { hasOwnKey, isNumberOrBigint } from '../../app/ts/utils/typescript.js'

describe('typescript utils', () => {
test('isNumberOrBigint accepts numeric primitive values', () => {
assert.equal(isNumberOrBigint(6), true)
assert.equal(isNumberOrBigint(6n), true)
assert.equal(isNumberOrBigint('6'), false)
})

test('hasOwnKey rejects properties inherited from Object.prototype', () => {
const value = { ownProperty: true }
assert.equal(hasOwnKey(value, 'ownProperty'), true)
assert.equal(hasOwnKey(value, 'toString'), false)
assert.equal(hasOwnKey(value, 'constructor'), false)
})
})
Loading