Skip to content

Sam/optimize initialize account #5858

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
+567 −308
Open

Sam/optimize initialize account #5858

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
40469e9
WIP: use separate settings files for performance testing
samholmes Nov 28, 2025
a2f01c1
Fix lint errors
samholmes Nov 28, 2025
ef8907b
fix: remove disabled scamWarningModal calls from login flow
samholmes Nov 26, 2025
099640c
perf: avoid writing defaults in readSyncedSettings
samholmes Nov 26, 2025
f86ab08
perf: avoid writing defaults in readLocalAccountSettings
samholmes Nov 26, 2025
1f94725
perf: load biometric state in background during login
samholmes Nov 26, 2025
4af7d28
perf: add one-time migration to clean up denomination settings
samholmes Nov 26, 2025
e1c6722
perf: simplify denomination settings merge in initializeAccount
samholmes Nov 26, 2025
e664176
perf: remove denomination defaults from LOGIN reducer
samholmes Nov 26, 2025
5d92708
perf: cache local settings read to prevent duplicate disk I/O
samholmes Nov 28, 2025
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
8 changes: 3 additions & 5 deletions eslint.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,12 @@ export default [
'src/actions/FioAddressActions.ts',
'src/actions/FirstOpenActions.tsx',
'src/actions/LoanWelcomeActions.tsx',
'src/actions/LocalSettingsActions.ts',

'src/actions/LoginActions.tsx',
'src/actions/NotificationActions.ts',
'src/actions/PaymentProtoActions.tsx',
'src/actions/ReceiveDropdown.tsx',
'src/actions/RecoveryReminderActions.tsx',
'src/actions/RequestReviewActions.tsx',

'src/actions/ScamWarningActions.tsx',
'src/actions/ScanActions.tsx',

Expand Down Expand Up @@ -290,7 +288,7 @@ export default [
'src/components/scenes/OtpSettingsScene.tsx',
'src/components/scenes/PasswordRecoveryScene.tsx',
'src/components/scenes/PromotionSettingsScene.tsx',
'src/components/scenes/ReviewTriggerTestScene.tsx',

'src/components/scenes/SecurityAlertsScene.tsx',

'src/components/scenes/SettingsScene.tsx',
Expand All @@ -311,7 +309,7 @@ export default [
'src/components/scenes/TransactionListScene.tsx',
'src/components/scenes/TransactionsExportScene.tsx',
'src/components/scenes/UpgradeUsernameScreen.tsx',
'src/components/scenes/WalletListScene.tsx',

'src/components/scenes/WalletRestoreScene.tsx',
'src/components/scenes/WcConnectionsScene.tsx',
'src/components/scenes/WcConnectScene.tsx',
Expand Down
2 changes: 1 addition & 1 deletion src/actions/LocalSettingsActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const getLocalAccountSettings = async (
return settings
}

export function useAccountSettings() {
export function useAccountSettings(): LocalAccountSettings {
const [accountSettings, setAccountSettings] =
React.useState(localAccountSettings)
React.useEffect(() => watchAccountSettings(setAccountSettings), [])
Expand Down
24 changes: 12 additions & 12 deletions src/actions/LoginActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,14 @@ export function initializeAccount(
'createWalletSelectCrypto' | 'createWalletSelectCryptoNewAccount'
>['navigation'],
items: WalletCreateItem[]
) => {
): Promise<void> => {
navigation.replace('edgeTabs', { screen: 'home' })
const createWalletsPromise = createCustomWallets(
account,
fiatCurrencyCode,
items,
dispatch
).catch(error => {
).catch((error: unknown) => {
showError(error)
})

Expand Down Expand Up @@ -222,12 +222,12 @@ export function initializeAccount(
const { userSettings = {} } = currencyConfig
currencyConfig
.changeUserSettings(userSettings)
.catch((error: unknown) => {
showError(error)
.catch((err: unknown) => {
Copy link
Contributor

@swansontec swansontec Dec 3, 2025

Choose a reason for hiding this comment

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

Why rename this? If we had to standardize on one name, I would prefer "error" over "err" or "e".

showError(err)
})
}
})
.catch(err => {
.catch((err: unknown) => {
showError(err)
})
}
Expand All @@ -245,7 +245,7 @@ export function initializeAccount(
const { context } = state.core

// Sign up for push notifications:
dispatch(registerNotificationsV2()).catch(e => {
dispatch(registerNotificationsV2()).catch((e: unknown) => {
console.error(e)
})

Expand Down Expand Up @@ -292,13 +292,13 @@ export function initializeAccount(
const mergedDenominationSettings = {}

for (const plugin of Object.keys(defaultDenominationSettings)) {
// @ts-expect-error
// @ts-expect-error - Dynamic object property assignment for denomination merging
Copy link
Contributor

@swansontec swansontec Dec 3, 2025

Choose a reason for hiding this comment

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

These comments are the wrong fix. The mergedDenominationSettings should be typed as const mergedDenominationSettings: DenominationSettings = {}. Then the error itself goes away.

mergedDenominationSettings[plugin] = {}
// @ts-expect-error
// @ts-expect-error - Dynamic object property access for denomination merging
Copy link
Contributor

@swansontec swansontec Dec 3, 2025

Choose a reason for hiding this comment

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

The solution to this error is a simple ?? []

for (const code of Object.keys(defaultDenominationSettings[plugin])) {
// @ts-expect-error
// @ts-expect-error - Dynamic object property assignment for denomination merging
mergedDenominationSettings[plugin][code] = {
// @ts-expect-error
// @ts-expect-error - Dynamic object property access for denomination merging
Copy link
Contributor

@swansontec swansontec Dec 3, 2025

Choose a reason for hiding this comment

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

The solution to these errors is:

          mergedDenominationSettings[plugin][code] = {
            ...defaultDenominationSettings[plugin]?.[code],
            ...syncedDenominationSettings[plugin]?.[code],
            name: '',
            multiplier: '',
            symbol: ''
          }

I know we are just going to delete this code, but I'd rather not get in the habit of doubling down on @ts-expect-error comments when they are trivial to fix.

...defaultDenominationSettings[plugin][code],
...(syncedDenominationSettings?.[plugin]?.[code] ?? {})
}
Expand All @@ -324,7 +324,7 @@ export function initializeAccount(
},
onNotificationPermit(info) {
dispatch(updateNotificationSettings(info.notificationOptIns)).catch(
error => {
(error: unknown) => {
trackError(error, 'LoginScene:onLogin:setDeviceSettings')
console.error(error)
}
Expand Down Expand Up @@ -430,7 +430,7 @@ async function createCustomWallets(
account.createCurrencyWallets(options),
timeoutMs,
new Error(lstrings.error_creating_wallets)
).catch(error => {
).catch((error: unknown) => {
dispatch(logEvent('Signup_Wallets_Created_Failed', { error }))
throw error
})
Expand Down
6 changes: 4 additions & 2 deletions src/actions/RequestReviewActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,14 @@ export const readReviewTriggerData = async (
// Initialize new data structure with old swap count data
const migratedData: ReviewTriggerData = {
...initReviewTriggerData(),
swapCount: parseInt(swapCountData.swapCount) || 0
swapCount: Number.isNaN(parseInt(swapCountData.swapCount))
? 0
: parseInt(swapCountData.swapCount)
Copy link
Contributor

@swansontec swansontec Dec 3, 2025

Choose a reason for hiding this comment

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

I don't like the double-parse. Why not:

const swapCount = parseInt(swapCountData.swapCount)
const migratedData: ReviewTriggerData = {
        ...initReviewTriggerData(),
        swapCount: Number.isNaN(swapCount) ? 0 : swapCount

}

// If user was already asked for review in the old system,
// set nextTriggerDate to 1 year in the future
if (swapCountData.hasReviewBeenRequested) {
if (swapCountData.hasReviewBeenRequested === true) {
const nextYear = new Date()
nextYear.setFullYear(nextYear.getFullYear() + 1)
migratedData.nextTriggerDate = nextYear
Expand Down
2 changes: 1 addition & 1 deletion src/components/scenes/ReviewTriggerTestScene.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { EdgeText } from '../themed/EdgeText'

interface Props extends EdgeSceneProps<'reviewTriggerTest'> {}

export const ReviewTriggerTestScene = (props: Props) => {
export const ReviewTriggerTestScene = (props: Props): React.JSX.Element => {
Copy link
Contributor

@swansontec swansontec Dec 3, 2025

Choose a reason for hiding this comment

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

This should be export const ReviewTriggerTestScene: React.FC<Props> = props => {

const dispatch = useDispatch()
const theme = useTheme()
const styles = getStyles(theme)
Expand Down
4 changes: 2 additions & 2 deletions src/components/scenes/WalletListScene.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { WalletListSwipeable } from '../themed/WalletListSwipeable'

interface Props extends WalletsTabSceneProps<'walletList'> {}

export function WalletListScene(props: Props) {
export function WalletListScene(props: Props): React.JSX.Element {
Copy link
Contributor

@swansontec swansontec Dec 3, 2025

Choose a reason for hiding this comment

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

This should be export const WalletListScene: React.FC<Props> = props => {

const { navigation } = props
const theme = useTheme()
const styles = getStyles(theme)
Expand Down Expand Up @@ -77,7 +77,7 @@ export function WalletListScene(props: Props) {
setSorting(true)
}
})
.catch(error => {
.catch((error: unknown) => {
showError(error)
})
})
Expand Down