Skip to content

Commit

Permalink
fix: notification preferences, post-switch registration + sequential …
Browse files Browse the repository at this point in the history
…signatures (#2551)

* fix: notification preferences + post-switch registration

* fix: sequentially sign for hardware wallets

* fix: default value

* fix: catch chain assertion rejections
  • Loading branch information
iamacook authored Sep 27, 2023
1 parent ebd9221 commit b420813
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement }
const allPreferences = getAllPreferences()
const safesToRegister = getSafesToRegister(addedSafes, allPreferences)

await assertWalletChain(onboard, safe.chainId)
try {
await assertWalletChain(onboard, safe.chainId)
} catch {
return
}

await registerNotifications(safesToRegister)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ describe('useNotificationRegistrations', () => {
describe('registerNotifications', () => {
beforeEach(() => {
const mockProvider = new Web3Provider(jest.fn())
jest.spyOn(web3, 'useWeb3').mockImplementation(() => mockProvider)
jest.spyOn(web3, 'createWeb3').mockImplementation(() => mockProvider)
jest.spyOn(wallet, 'default').mockImplementation(
() =>
({
label: 'MetaMask',
} as unknown as ConnectedWallet),
} as ConnectedWallet),
)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
} from '@/services/push-notifications/preferences'
import { logError } from '@/services/exceptions'
import ErrorCodes from '@/services/exceptions/ErrorCodes'
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import type { PushNotificationPreferences, PushNotificationPrefsKey } from '@/services/push-notifications/preferences'
import type { NotifiableSafes } from '../logic'

Expand All @@ -26,7 +25,7 @@ export const DEFAULT_NOTIFICATION_PREFERENCES: PushNotificationPreferences[PushN
[WebhookType.INCOMING_ETHER]: true,
[WebhookType.INCOMING_TOKEN]: true,
[WebhookType.MODULE_TRANSACTION]: true,
[WebhookType.CONFIRMATION_REQUEST]: false, // Requires signature
[WebhookType.CONFIRMATION_REQUEST]: true, // Requires signature
[WebhookType.SAFE_CREATED]: false, // We do not preemptively subscribe to Safes before they are created
// Disabled on the Transaction Service but kept here for completeness
[WebhookType._PENDING_MULTISIG_TRANSACTION]: true,
Expand Down Expand Up @@ -59,7 +58,6 @@ export const useNotificationPreferences = (): {
// State
const uuid = useUuid()
const preferences = usePreferences()
const isOwner = useIsSafeOwner()

// Getters
const getPreferences = (chainId: string, safeAddress: string) => {
Expand Down Expand Up @@ -152,10 +150,7 @@ export const useNotificationPreferences = (): {
const defaultPreferences: PushNotificationPreferences[PushNotificationPrefsKey] = {
chainId,
safeAddress,
preferences: {
...DEFAULT_NOTIFICATION_PREFERENCES,
[WebhookType.CONFIRMATION_REQUEST]: isOwner,
},
preferences: DEFAULT_NOTIFICATION_PREFERENCES,
}

return [key, defaultPreferences]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { registerDevice, unregisterDevice, unregisterSafe } from '@safe-global/safe-gateway-typescript-sdk'

import { useWeb3 } from '@/hooks/wallets/web3'
import { useAppDispatch } from '@/store'
import { showNotification } from '@/store/notificationsSlice'
import { useNotificationPreferences } from './useNotificationPreferences'
Expand All @@ -10,7 +9,6 @@ import { getRegisterDevicePayload } from '../logic'
import { logError } from '@/services/exceptions'
import ErrorCodes from '@/services/exceptions/ErrorCodes'
import useWallet from '@/hooks/wallets/useWallet'
import { isLedger } from '@/utils/wallets'
import type { NotifiableSafes } from '../logic'

const registrationFlow = async (registrationFn: Promise<unknown>, callback: () => void): Promise<boolean> => {
Expand Down Expand Up @@ -39,22 +37,20 @@ export const useNotificationRegistrations = (): {
unregisterDeviceNotifications: (chainId: string) => Promise<boolean | undefined>
} => {
const dispatch = useAppDispatch()
const web3 = useWeb3()
const wallet = useWallet()

const { uuid, _createPreferences, _deletePreferences, _deleteAllPreferences } = useNotificationPreferences()

const registerNotifications = async (safesToRegister: NotifiableSafes) => {
if (!uuid || !web3 || !wallet) {
if (!uuid || !wallet) {
return
}

const register = async () => {
const payload = await getRegisterDevicePayload({
uuid,
safesToRegister,
web3,
isLedger: isLedger(wallet),
wallet,
})

return registerDevice(payload)
Expand Down
6 changes: 5 additions & 1 deletion src/components/settings/PushNotifications/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ export const PushNotifications = (): ReactElement => {

setIsRegistering(true)

await assertWalletChain(onboard, safe.chainId)
try {
await assertWalletChain(onboard, safe.chainId)
} catch {
return
}

if (!preferences) {
await registerNotifications({ [safe.chainId]: [safe.address.value] })
Expand Down
14 changes: 10 additions & 4 deletions src/components/settings/PushNotifications/logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { Web3Provider } from '@ethersproject/providers'
import type { JsonRpcSigner } from '@ethersproject/providers'

import * as logic from './logic'
import * as web3 from '@/hooks/wallets/web3'
import packageJson from '../../../../package.json'
import type { ConnectedWallet } from '@/services/onboard'

jest.mock('firebase/messaging')

Expand Down Expand Up @@ -128,6 +130,7 @@ describe('Notifications', () => {
signMessage: jest.fn().mockResolvedValueOnce(MM_SIGNATURE),
} as unknown as JsonRpcSigner),
)
jest.spyOn(web3, 'createWeb3').mockImplementation(() => mockProvider)

const uuid = crypto.randomUUID()

Expand All @@ -137,8 +140,9 @@ describe('Notifications', () => {
['2']: [hexZeroPad('0x1', 20)],
},
uuid,
web3: mockProvider,
isLedger: false,
wallet: {
label: 'MetaMask',
} as ConnectedWallet,
})

expect(payload).toStrictEqual({
Expand Down Expand Up @@ -176,6 +180,7 @@ describe('Notifications', () => {
signMessage: jest.fn().mockResolvedValueOnce(LEDGER_SIGNATURE),
} as unknown as JsonRpcSigner),
)
jest.spyOn(web3, 'createWeb3').mockImplementation(() => mockProvider)

const uuid = crypto.randomUUID()

Expand All @@ -185,8 +190,9 @@ describe('Notifications', () => {
['2']: [hexZeroPad('0x1', 20)],
},
uuid,
web3: mockProvider,
isLedger: true,
wallet: {
label: 'Ledger',
} as ConnectedWallet,
})

expect(payload).toStrictEqual({
Expand Down
54 changes: 30 additions & 24 deletions src/components/settings/PushNotifications/logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import packageJson from '../../../../package.json'
import { logError } from '@/services/exceptions'
import ErrorCodes from '@/services/exceptions/ErrorCodes'
import { checksumAddress } from '@/utils/addresses'
import { isLedger } from '@/utils/wallets'
import { createWeb3 } from '@/hooks/wallets/web3'
import type { ConnectedWallet } from '@/services/onboard'

type WithRequired<T, K extends keyof T> = T & { [P in K]-?: T[P] }

Expand Down Expand Up @@ -85,13 +88,11 @@ export type NotifiableSafes = { [chainId: string]: Array<string> }
export const getRegisterDevicePayload = async ({
safesToRegister,
uuid,
web3,
isLedger,
wallet,
}: {
safesToRegister: NotifiableSafes
uuid: string
web3: Web3Provider
isLedger: boolean
wallet: ConnectedWallet
}): Promise<RegisterNotificationsRequest> => {
const BUILD_NUMBER = '0' // Required value, but does not exist on web
const BUNDLE = 'safe'
Expand All @@ -107,6 +108,9 @@ export const getRegisterDevicePayload = async ({
serviceWorkerRegistration,
})

const web3 = createWeb3(wallet.provider)
const isLedgerWallet = isLedger(wallet)

// If uuid is not provided a new device will be created.
// If a uuid for an existing Safe is provided the FirebaseDevice will be updated with all the new data provided.
// Safes provided on the request are always added and never removed/replaced
Expand All @@ -115,26 +119,28 @@ export const getRegisterDevicePayload = async ({

const timestamp = Math.floor(new Date().getTime() / 1000).toString()

const safeRegistrations = await Promise.all(
Object.entries(safesToRegister).map(async ([chainId, safeAddresses]) => {
const checksummedSafeAddresses = safeAddresses.map((address) => checksumAddress(address))
// We require a signature for confirmation request notifications
const signature = await getSafeRegistrationSignature({
safeAddresses: checksummedSafeAddresses,
web3,
uuid,
timestamp,
token,
isLedger,
})

return {
chainId,
safes: checksummedSafeAddresses,
signatures: [signature],
}
}),
)
let safeRegistrations: RegisterNotificationsRequest['safeRegistrations'] = []

// We cannot `Promise.all` here as Ledger/Trezor return a "busy" error when signing multiple messages at once
for await (const [chainId, safeAddresses] of Object.entries(safesToRegister)) {
const checksummedSafeAddresses = safeAddresses.map((address) => checksumAddress(address))

// We require a signature for confirmation request notifications
const signature = await getSafeRegistrationSignature({
safeAddresses: checksummedSafeAddresses,
web3,
uuid,
timestamp,
token,
isLedger: isLedgerWallet,
})

safeRegistrations.push({
chainId,
safes: checksummedSafeAddresses,
signatures: [signature],
})
}

return {
uuid,
Expand Down

0 comments on commit b420813

Please sign in to comment.