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] ENS Resolves to Wrong Address when DeepLink #5656

Merged
merged 5 commits into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ const ModalConfirmation = ({ route }: ModalConfirmationProps) => {
modalRef.current?.dismissModal(onConfirm);
};

const handleModalDismiss = (hasPendingAction: boolean) =>
!hasPendingAction && onCancel?.();

const renderHeader = () => (
<Text style={styles.headerLabel} variant={TextVariants.sHeadingMD}>
{title}
Expand Down Expand Up @@ -74,7 +77,11 @@ const ModalConfirmation = ({ route }: ModalConfirmationProps) => {
);

return (
<ReusableModal ref={modalRef} style={styles.screen} onDismiss={onCancel}>
<ReusableModal
ref={modalRef}
style={styles.screen}
onDismiss={handleModalDismiss}
>
<View style={styles.modal}>
<View style={styles.bodyContainer}>
{renderHeader()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

exports[`ModalConfirmation should render correctly 1`] = `
<ForwardRef
onDismiss={[Function]}
style={
Object {
"justifyContent": "center",
Expand Down
7 changes: 6 additions & 1 deletion app/components/Nav/App/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ const App = ({ userLoggedIn }) => {
state?.engine?.backgroundState?.PreferencesController?.frequentRpcList,
);

const network = useSelector(
(state) => state.engine.backgroundState.NetworkController.network,
);

const handleDeeplink = useCallback(({ error, params, uri }) => {
if (error) {
trackErrorAsAnalytics(error, 'Branch:');
Expand Down Expand Up @@ -226,6 +230,7 @@ const App = ({ userLoggedIn }) => {
},
},
frequentRpcList,
network,
dispatch,
});
if (!prevNavigator.current) {
Expand All @@ -246,7 +251,7 @@ const App = ({ userLoggedIn }) => {
}
prevNavigator.current = navigator;
}
}, [dispatch, handleDeeplink, frequentRpcList, navigator]);
}, [dispatch, handleDeeplink, frequentRpcList, navigator, network]);

useEffect(() => {
const initAnalytics = async () => {
Expand Down
18 changes: 5 additions & 13 deletions app/components/Views/Send/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {
generateTransferData,
} from '../../../util/transactions';
import Logger from '../../../util/Logger';
import { isENS, isValidHexAddress } from '../../../util/address';
import { getAddress } from '../../../util/address';
import TransactionTypes from '../../../core/TransactionTypes';
import { MAINNET } from '../../../constants/network';
import BigNumber from 'bignumber.js';
Expand All @@ -47,7 +47,6 @@ import AnalyticsV2 from '../../../util/analyticsV2';

import { KEYSTONE_TX_CANCELED } from '../../../constants/error';
import { ThemeContext, mockTheme } from '../../../util/theme';
import { doENSLookup } from '../../../util/ENSUtils';

const REVIEW = 'review';
const EDIT = 'edit';
Expand Down Expand Up @@ -269,17 +268,10 @@ class Send extends PureComponent {
* Handle deeplink txMeta recipient
*/
handleNewTxMetaRecipient = async (recipient) => {
let ensRecipient, to;
if (isENS(recipient)) {
ensRecipient = recipient;
to = await doENSLookup(ensRecipient, this.props.network);
}
if (
recipient &&
isValidHexAddress(recipient, { mixedCaseUseChecksum: true })
) {
to = recipient;
}
let ensRecipient;
blackdevelopa marked this conversation as resolved.
Show resolved Hide resolved

const to = await getAddress(recipient, this.props.network);

if (!to) {
NotificationManager.showSimpleNotification({
status: 'simple_notification_rejected',
Expand Down
29 changes: 24 additions & 5 deletions app/core/DeeplinkManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { NETWORK_ERROR_MISSING_NETWORK_ID } from '../constants/error';
import { strings } from '../../locales/i18n';
import { getNetworkTypeById, handleNetworkSwitch } from '../util/networks';
import { WalletDevice } from '@metamask/transaction-controller';
import NotificationManager from '../core/NotificationManager';
import {
ACTIONS,
ETH_ACTIONS,
Expand All @@ -22,12 +23,14 @@ import { showAlert } from '../actions/alert';
import SDKConnect from '../core/SDKConnect';
import Routes from '../constants/navigation/Routes';
import Minimizer from 'react-native-minimizer';
import { getAddress } from '../util/address';
class DeeplinkManager {
constructor({ navigation, frequentRpcList, dispatch }) {
constructor({ navigation, frequentRpcList, dispatch, network }) {
this.navigation = navigation;
this.pendingDeeplink = null;
this.frequentRpcList = frequentRpcList;
this.dispatch = dispatch;
this.network = network;
}

setDeeplink = (url) => (this.pendingDeeplink = url);
Expand Down Expand Up @@ -60,7 +63,7 @@ class DeeplinkManager {
);
};

_approveTransaction = (ethUrl, origin) => {
_approveTransaction = async (ethUrl, origin) => {
const {
parameters: { address, uint256 },
target_address,
Expand All @@ -83,11 +86,22 @@ class DeeplinkManager {

const value = uint256Number.toString(16);

const spenderAddress = await getAddress(address, this.network);
if (!spenderAddress) {
NotificationManager.showSimpleNotification({
status: 'simple_notification_rejected',
duration: 5000,
title: strings('transaction.invalid_recipient'),
description: strings('transaction.invalid_recipient_description'),
});
this.navigation.navigate('WalletView');
}

const txParams = {
to: target_address.toString(),
from: PreferencesController.state.selectedAddress.toString(),
value: '0x0',
data: generateApproveData({ spender: address, value }),
data: generateApproveData({ spender: spenderAddress, value }),
};

TransactionController.addTransaction(
Expand Down Expand Up @@ -340,8 +354,13 @@ class DeeplinkManager {
let instance = null;

const SharedDeeplinkManager = {
init: ({ navigation, frequentRpcList, dispatch }) => {
instance = new DeeplinkManager({ navigation, frequentRpcList, dispatch });
init: ({ navigation, frequentRpcList, dispatch, network }) => {
instance = new DeeplinkManager({
navigation,
frequentRpcList,
dispatch,
network,
});
},
parse: (url, args) => instance.parse(url, args),
setDeeplink: (url) => instance.setDeeplink(url),
Expand Down
16 changes: 16 additions & 0 deletions app/util/address/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,3 +437,19 @@ export const stripHexPrefix = (str) => {
}
return isHexPrefixed(str) ? str.slice(2) : str;
};

/**
* Method to check if address is ENS and return the address
* @param {String} toAccount - Address or ENS
* @param {String} network - Network id
* @returns {String} - Address or null
*/
export async function getAddress(toAccount, network) {
if (isENS(toAccount)) {
return await doENSLookup(toAccount, network);
}
if (isValidHexAddress(toAccount, { mixedCaseUseChecksum: true })) {
return toAccount;
}
return null;
}
blackdevelopa marked this conversation as resolved.
Show resolved Hide resolved
24 changes: 24 additions & 0 deletions app/util/address/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
isValidHexAddress,
isValidAddressInputViaQRCode,
stripHexPrefix,
getAddress,
} from '.';

describe('isENS', () => {
Expand Down Expand Up @@ -138,3 +139,26 @@ describe('stripHexPrefix', () => {
expect(stripHexPrefix(stripped)).toBe(stripped);
});
});

describe('getAddress', () => {
const validAddress = '0x87187657B35F461D0CEEC338D9B8E944A193AFE2';
const inValidAddress = '0x87187657B35F461D0CEEC338D9B8E944A193AFE';
const validENSAddress = 'test.eth';

it('should resolve ENS if ENS is valid', async () => {
const network = '1';
const doENSLookup = jest.fn();
await doENSLookup(validENSAddress, network);
expect(doENSLookup).toHaveBeenCalledWith(validENSAddress, network);
});

it('should return address if address is valid', async () => {
const response = await getAddress(validAddress, '1');
expect(response).toBe(validAddress);
});

it('should return null if address is invalid', async () => {
const response = await getAddress(inValidAddress, '1');
expect(response).toBe(null);
});
});