Skip to content
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.

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

fix: re-designs signatures, continue to use old designs when signing with hardware wallets #12976

Merged
merged 15 commits into from
Jan 15, 2025
Merged
11 changes: 11 additions & 0 deletions app/components/Views/confirmations/Confirm/Confirm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ import Confirm from './index';

jest.mock('../../../../core/Engine', () => ({
getTotalFiatAccountBalance: () => ({ tokenFiat: 10 }),
context: {
KeyringController: {
state: {
keyrings: [],
},
getOrAddQRKeyring: jest.fn(),
},
},
controllerMessenger: {
subscribe: jest.fn(),
},
}));

jest.mock('../../../../util/address', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jest.mock('../../../../../../core/Engine', () => ({
getQRKeyringState: jest.fn(() =>
Promise.resolve({ subscribe: jest.fn(), unsubscribe: jest.fn() }),
),
getOrAddQRKeyring: jest.fn(),
state: {
keyrings: [],
},
Expand All @@ -33,6 +34,9 @@ jest.mock('../../../../../../core/Engine', () => ({
},
},
},
controllerMessenger: {
subscribe: jest.fn(),
},
}));

jest.mock('@react-navigation/native', () => ({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// eslint-disable-next-line import/no-namespace
import * as AddressUtils from '../../../../util/address';
import { renderHookWithProvider } from '../../../../util/test/renderWithProvider';
import { personalSignatureConfirmationState } from '../../../../util/test/confirm-data-helpers';

// eslint-disable-next-line import/no-namespace
import * as QRHardwareAwareness from './useQRHardwareAwareness';
import useConfirmationRedesignEnabled from './useConfirmationRedesignEnabled';

jest.mock('../../../../core/Engine', () => ({
getTotalFiatAccountBalance: () => ({ tokenFiat: 10 }),
context: {
KeyringController: {
state: {
keyrings: [],
},
getOrAddQRKeyring: jest.fn(),
},
},
controllerMessenger: {
subscribe: jest.fn(),
},
}));

describe('useConfirmationRedesignEnabled', () => {
it('return true for personal sign request', async () => {
const { result } = renderHookWithProvider(
() => useConfirmationRedesignEnabled(),
{
state: personalSignatureConfirmationState,
},
);
expect(result?.current.isRedesignedEnabled).toBeTruthy();
});

it('return false for external accounts', async () => {
jest.spyOn(AddressUtils, 'isExternalHardwareAccount').mockReturnValue(true);
const { result } = renderHookWithProvider(
() => useConfirmationRedesignEnabled(),
{
state: personalSignatureConfirmationState,
},
);
expect(result?.current.isRedesignedEnabled).toBeFalsy();
});

it('return false if QR hardware is syncing', async () => {
jest
.spyOn(QRHardwareAwareness, 'default')
.mockReturnValue({ isSigningQRObject: true, isSyncingQRHardware: false });
const { result } = renderHookWithProvider(
() => useConfirmationRedesignEnabled(),
{
state: personalSignatureConfirmationState,
},
);
expect(result?.current.isRedesignedEnabled).toBeFalsy();
});

it('return false if QR hardware has synced successfully', async () => {
jest
.spyOn(QRHardwareAwareness, 'default')
.mockReturnValue({ isSigningQRObject: false, isSyncingQRHardware: true });
const { result } = renderHookWithProvider(
() => useConfirmationRedesignEnabled(),
{
state: personalSignatureConfirmationState,
},
);
expect(result?.current.isRedesignedEnabled).toBeFalsy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,43 @@ import { useMemo } from 'react';
import { useSelector } from 'react-redux';

import { ApprovalTypes } from '../../../../core/RPCMethods/RPCMethodMiddleware';
import { isExternalHardwareAccount } from '../../../../util/address';
import { selectRemoteFeatureFlags } from '../../../../selectors/featureFlagController';
import useApprovalRequest from './useApprovalRequest';
import useQRHardwareAwareness from './useQRHardwareAwareness';

const useConfirmationRedesignEnabled = () => {
const { approvalRequest } = useApprovalRequest();
const { isSigningQRObject, isSyncingQRHardware } = useQRHardwareAwareness();
const { confirmation_redesign } = useSelector(selectRemoteFeatureFlags);

const { type: approvalRequestType } = approvalRequest ?? {
const {
type: approvalRequestType,
requestData: { from: fromAddress },
} = approvalRequest ?? {
requestData: {},
};

const isRedesignedEnabled = useMemo(
() =>
(confirmation_redesign as Record<string, string>)?.signatures &&
process.env.REDESIGNED_SIGNATURE_REQUEST === 'true' &&
// following condition will ensure that user is redirected to old designs is using QR scan aware hardware
!isSyncingQRHardware &&
jpuri marked this conversation as resolved.
Show resolved Hide resolved
jpuri marked this conversation as resolved.
Show resolved Hide resolved
!isSigningQRObject &&
// following condition will ensure that user is redirected to old designs for hardware wallets
!isExternalHardwareAccount(fromAddress) &&
approvalRequestType &&
[ApprovalTypes.PERSONAL_SIGN, ApprovalTypes.ETH_SIGN_TYPED_DATA].includes(
approvalRequestType as ApprovalTypes,
),
[approvalRequestType, confirmation_redesign],
[
approvalRequestType,
confirmation_redesign,
fromAddress,
isSigningQRObject,
isSyncingQRHardware,
],
);

return { isRedesignedEnabled };
Expand Down
jpuri marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { useState, useEffect } from 'react';
jpuri marked this conversation as resolved.
Show resolved Hide resolved

import Engine from '../../../../core/Engine';
import { IQRState } from '../../../UI/QRHardware/types';

const useQRHardwareAwareness = () => {
const [qrState, setQRState] = useState<IQRState>({
sync: {
reading: false,
},
sign: {},
});

const subscribe = (value: IQRState) => {
setQRState(value);
};

useEffect(() => {
Engine.context.KeyringController.getOrAddQRKeyring();
Engine.controllerMessenger.subscribe(
'KeyringController:qrKeyringStateChange',
subscribe,
);
return () => {
Engine.controllerMessenger.unsubscribe(
'KeyringController:qrKeyringStateChange',
subscribe,
);
};
}, []);

return {
isSigningQRObject: !!qrState.sign?.request,
isSyncingQRHardware: qrState.sync.reading,
};
};

export default useQRHardwareAwareness;
Loading