From 8fd310cdb3f3c0e3708ea77c73d0b7bce19bd26d Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:44:04 +0100 Subject: [PATCH] fix: token decimals fetched from the chain (#7540) Co-authored-by: Cal Leung --- .../__snapshots__/index.test.tsx.snap | 3 +- app/components/UI/AddCustomToken/index.js | 90 ++++++--- app/constants/error.ts | 1 + app/core/RPCMethods/RPCMethodMiddleware.ts | 35 +--- app/core/RPCMethods/index.js | 2 + app/core/RPCMethods/wallet_watchAsset.test.ts | 189 ++++++++++++++++++ app/core/RPCMethods/wallet_watchAsset.ts | 100 +++++++++ 7 files changed, 359 insertions(+), 61 deletions(-) create mode 100644 app/core/RPCMethods/wallet_watchAsset.test.ts create mode 100644 app/core/RPCMethods/wallet_watchAsset.ts diff --git a/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap b/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap index d744811ca5e..da48b5e2861 100644 --- a/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap +++ b/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap @@ -76,7 +76,6 @@ exports[`AddCustomToken should render correctly 1`] = ` ...fontStyles.normal, color: colors.text.default, }, + textInputDisabled: { + color: colors.text.muted, + fontWeight: 'bold', + }, inputLabel: { ...fontStyles.normal, color: colors.text.default, @@ -82,6 +86,7 @@ export default class AddCustomToken extends PureComponent { warningAddress: '', warningSymbol: '', warningDecimals: '', + isSymbolAndDecimalEditable: true, }; static propTypes = { @@ -155,8 +160,42 @@ export default class AddCustomToken extends PureComponent { this.props.navigation.goBack(); }; - onAddressChange = (address) => { + onAddressChange = async (address) => { this.setState({ address }); + if (address.length === 42) { + try { + this.setState({ isSymbolAndDecimalEditable: false }); + const validated = await this.validateCustomTokenAddress(address); + if (validated) { + const { AssetsContractController } = Engine.context; + const [decimals, symbol, name] = await Promise.all([ + AssetsContractController.getERC20TokenDecimals(address), + AssetsContractController.getERC721AssetSymbol(address), + AssetsContractController.getERC20TokenName(address), + ]); + + this.setState({ + decimals: String(decimals), + symbol, + name, + }); + } else { + this.setState({ isSymbolAndDecimalEditable: true }); + } + } catch (e) { + this.setState({ isSymbolAndDecimalEditable: true }); + } + } else { + // We are cleaning other fields when changing the token address + this.setState({ + decimals: '', + symbol: '', + name: '', + warningAddress: '', + warningSymbol: '', + warningDecimals: '', + }); + } }; onSymbolChange = (symbol) => { @@ -167,36 +206,25 @@ export default class AddCustomToken extends PureComponent { this.setState({ decimals }); }; - onAddressBlur = async () => { - const validated = await this.validateCustomTokenAddress(); - if (validated) { - const address = this.state.address; - const { AssetsContractController } = Engine.context; - const decimals = await AssetsContractController.getERC20TokenDecimals( - address, - ); - const symbol = await AssetsContractController.getERC721AssetSymbol( - address, - ); - const name = await AssetsContractController.getERC20TokenName(address); - - this.setState({ decimals: String(decimals), symbol, name }); - } - }; - - validateCustomTokenAddress = async () => { + validateCustomTokenAddress = async (address) => { let validated = true; - const address = this.state.address; const isValidTokenAddress = isValidAddress(address); + const { chainId } = this.props; const toSmartContract = isValidTokenAddress && (await isSmartContractAddress(address, chainId)); + const addressWithoutSpaces = address.replace(regex.addressWithSpaces, ''); + if (addressWithoutSpaces.length === 0) { - this.setState({ warningAddress: strings('token.address_cant_be_empty') }); + this.setState({ + warningAddress: strings('token.address_cant_be_empty'), + }); validated = false; } else if (!isValidTokenAddress) { - this.setState({ warningAddress: strings('token.address_must_be_valid') }); + this.setState({ + warningAddress: strings('token.address_must_be_valid'), + }); validated = false; } else if (!toSmartContract) { this.setState({ @@ -238,7 +266,9 @@ export default class AddCustomToken extends PureComponent { }; validateCustomToken = async () => { - const validatedAddress = await this.validateCustomTokenAddress(); + const validatedAddress = await this.validateCustomTokenAddress( + this.state.address, + ); const validatedSymbol = this.validateCustomTokenSymbol(); const validatedDecimals = this.validateCustomTokenDecimals(); return validatedAddress && validatedSymbol && validatedDecimals; @@ -340,11 +370,16 @@ export default class AddCustomToken extends PureComponent { : this.renderInfoBanner(); render = () => { - const { address, symbol, decimals } = this.state; + const { address, symbol, decimals, isSymbolAndDecimalEditable } = + this.state; const colors = this.context.colors || mockTheme.colors; const themeAppearance = this.context.themeAppearance || 'light'; const styles = createStyles(colors); + const textInputSymbolAndDecimalsStyle = !isSymbolAndDecimalEditable + ? { ...styles.textInput, ...styles.textInputDisabled } + : styles.textInput; + return ( {this.state.warningSymbol} @@ -409,7 +444,7 @@ export default class AddCustomToken extends PureComponent { {strings('token.token_decimal')} { - const { - params: { - options: { address, decimals, image, symbol }, - type, - }, - } = req; - const { TokensController } = Engine.context; - const chainId = selectChainId(store.getState()); - - checkTabActive(); - - // Check if token exists on wallet's active network. - const isTokenOnNetwork = await isSmartContractAddress(address, chainId); - if (!isTokenOnNetwork) { - throw new Error(TOKEN_NOT_SUPPORTED_FOR_NETWORK); - } - const permittedAccounts = await getPermittedAccounts(hostname); - // This should return the current active account on the Dapp. - const selectedAddress = - Engine.context.PreferencesController.state.selectedAddress; - // Fallback to wallet address if there is no connected account to Dapp. - const interactingAddress = permittedAccounts?.[0] || selectedAddress; - await TokensController.watchAsset( - { address, symbol, decimals, image }, - type, - safeToChecksumAddress(interactingAddress), - ); - res.result = true; - }, + wallet_watchAsset: async () => + RPCMethods.wallet_watchAsset({ req, res, hostname, checkTabActive }), metamask_removeFavorite: async () => { checkTabActive(); diff --git a/app/core/RPCMethods/index.js b/app/core/RPCMethods/index.js index 3c074804744..69c30a97474 100644 --- a/app/core/RPCMethods/index.js +++ b/app/core/RPCMethods/index.js @@ -1,11 +1,13 @@ import eth_sendTransaction from './eth_sendTransaction'; import wallet_addEthereumChain from './wallet_addEthereumChain.js'; import wallet_switchEthereumChain from './wallet_switchEthereumChain.js'; +import wallet_watchAsset from './wallet_watchAsset.ts'; const RPCMethods = { eth_sendTransaction, wallet_addEthereumChain, wallet_switchEthereumChain, + wallet_watchAsset, }; export default RPCMethods; diff --git a/app/core/RPCMethods/wallet_watchAsset.test.ts b/app/core/RPCMethods/wallet_watchAsset.test.ts new file mode 100644 index 00000000000..4b606edda1f --- /dev/null +++ b/app/core/RPCMethods/wallet_watchAsset.test.ts @@ -0,0 +1,189 @@ +import Engine from '../Engine'; +import wallet_watchAsset from './wallet_watchAsset'; +// eslint-disable-next-line import/no-namespace +import * as transactionsUtils from '../../util/transactions'; +import { + TOKEN_NOT_SUPPORTED_FOR_NETWORK, + TOKEN_NOT_VALID, +} from '../../constants/error'; + +const mockEngine = Engine; +jest.mock('../Engine', () => ({ + init: () => mockEngine.init({}), + context: { + NetworkController: { + state: { + networkConfigurations: {}, + providerConfig: { + chainId: '1', + }, + }, + }, + TokensController: { + watchAsset: jest.fn(), + }, + TokenListController: { + state: { + tokenList: { + '0x1': [], + }, + }, + }, + PermissionController: { + requestPermissions: jest.fn(), + getPermissions: jest.fn(), + }, + PreferencesController: { + state: { + selectedAddress: '0x123', + }, + }, + AssetsContractController: { + getERC721AssetSymbol: jest.fn().mockResolvedValue('WBTC'), + }, + }, +})); + +jest.mock('../Permissions', () => ({ + getPermittedAccounts: jest.fn(), +})); + +jest.mock('../../store', () => ({ + store: { + getState: jest.fn(() => ({ + engine: { + backgroundState: { + NetworkController: { + networkConfigurations: {}, + providerConfig: { + chainId: '1', + }, + }, + }, + }, + })), + }, +})); + +describe('wallet_watchAsset', () => { + const ERC20 = 'ERC20'; + const correctWBTC = { + address: '0x2260fac5e5542a773aa44fbcfedf7c193bc2c599', + symbol: 'WBTC', + decimals: '8', + image: 'https://metamask.github.io/test-dapp/metamask-fox.svg', + }; + it('should throw an error if the token address is not valid', async () => { + await expect( + wallet_watchAsset({ + req: { + params: { + options: { + address: '0x2260fac5e5542a773aa44fbcfedf7c193bc2c59', + symbol: '', + decimals: '', + image: 'https://metamask.github.io/test-dapp/metamask-fox.svg', + }, + type: '', + }, + jsonrpc: '2.0', + method: '', + id: '', + }, + res: {} as any, + checkTabActive: () => null as any, + hostname: '', + }), + ).rejects.toThrow(TOKEN_NOT_VALID); + }); + it('should throw an error if the token address is not a smart contract address', async () => { + jest + .spyOn(transactionsUtils, 'isSmartContractAddress') + .mockResolvedValue(false); + await expect( + wallet_watchAsset({ + req: { + params: { + options: { + address: '0x2260fac5e5542a773aa44fbcfedf7c193bc2c599', + symbol: '', + decimals: '', + image: 'https://metamask.github.io/test-dapp/metamask-fox.svg', + }, + type: '', + }, + jsonrpc: '2.0', + method: '', + id: '', + }, + res: {} as any, + checkTabActive: () => null as any, + hostname: '', + }), + ).rejects.toThrow(TOKEN_NOT_SUPPORTED_FOR_NETWORK); + }); + + it('should call watchAsset with legit WBTC decimals and symbol', async () => { + jest + .spyOn(transactionsUtils, 'isSmartContractAddress') + .mockResolvedValue(true); + Engine.context.AssetsContractController = { + getERC20TokenDecimals: jest.fn().mockResolvedValue(correctWBTC.decimals), + getERC721AssetSymbol: jest.fn().mockResolvedValue(correctWBTC.symbol), + }; + const spyOnWatchAsset = jest.spyOn( + Engine.context.TokensController, + 'watchAsset', + ); + await wallet_watchAsset({ + req: { + params: { + options: correctWBTC, + type: ERC20, + }, + jsonrpc: '2.0', + method: '', + id: '', + }, + res: {} as any, + checkTabActive: () => null as any, + hostname: '', + }); + expect(spyOnWatchAsset).toHaveBeenCalledWith(correctWBTC, ERC20, '0x123'); + }); + it('should call watchAsset with fake WBTC decimals and symbol', async () => { + const fakeWBTC = { + address: '0x2260fac5e5542a773aa44fbcfedf7c193bc2c599', + symbol: 'WBTCFake', + decimals: '16', + image: 'https://metamask.github.io/test-dapp/metamask-fox.svg', + }; + + jest + .spyOn(transactionsUtils, 'isSmartContractAddress') + .mockResolvedValue(true); + Engine.context.AssetsContractController = { + getERC20TokenDecimals: jest.fn().mockResolvedValue(correctWBTC.decimals), + getERC721AssetSymbol: jest.fn().mockResolvedValue(correctWBTC.symbol), + }; + const spyOnWatchAsset = jest.spyOn( + Engine.context.TokensController, + 'watchAsset', + ); + await wallet_watchAsset({ + req: { + params: { + options: fakeWBTC, + type: ERC20, + }, + jsonrpc: '2.0', + method: '', + id: '', + }, + res: {} as any, + checkTabActive: () => null as any, + hostname: '', + }); + expect(spyOnWatchAsset).toHaveBeenCalledWith(correctWBTC, ERC20, '0x123'); + }); +}); diff --git a/app/core/RPCMethods/wallet_watchAsset.ts b/app/core/RPCMethods/wallet_watchAsset.ts new file mode 100644 index 00000000000..0c351ab1825 --- /dev/null +++ b/app/core/RPCMethods/wallet_watchAsset.ts @@ -0,0 +1,100 @@ +import Engine from '../Engine'; + +import { safeToChecksumAddress } from '../../util/address'; +import { store } from '../../store'; + +import { getPermittedAccounts } from '../Permissions'; +import { isSmartContractAddress } from '../../util/transactions'; +import { + TOKEN_NOT_SUPPORTED_FOR_NETWORK, + TOKEN_NOT_VALID, +} from '../../constants/error'; +import { selectChainId } from '../../selectors/networkController'; +import { isValidAddress } from 'ethereumjs-util'; +import { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine'; + +const wallet_watchAsset = async ({ + req, + res, + hostname, + checkTabActive, +}: { + req: JsonRpcRequest<{ + options: { + address: string; + decimals: string; + symbol: string; + image: string; + }; + type: string; + }>; + res: PendingJsonRpcResponse; + hostname: string; + checkTabActive: () => true | undefined; +}) => { + const { AssetsContractController } = Engine.context; + if (!req.params) { + throw new Error('wallet_watchAsset params is undefined'); + } + const { + params: { + options: { address, decimals, image, symbol }, + type, + }, + } = req; + + const { TokensController } = Engine.context; + const chainId = selectChainId(store.getState()); + + checkTabActive(); + + const isValidTokenAddress = isValidAddress(address); + + if (!isValidTokenAddress) { + throw new Error(TOKEN_NOT_VALID); + } + + // Check if token exists on wallet's active network. + const isTokenOnNetwork = await isSmartContractAddress(address, chainId); + if (!isTokenOnNetwork) { + throw new Error(TOKEN_NOT_SUPPORTED_FOR_NETWORK); + } + + const permittedAccounts = await getPermittedAccounts(hostname); + // This should return the current active account on the Dapp. + const selectedAddress = + Engine.context.PreferencesController.state.selectedAddress; + + // Fallback to wallet address if there is no connected account to Dapp. + const interactingAddress = permittedAccounts?.[0] || selectedAddress; + // This variables are to override the value of decimals and symbol from the dapp + // if they are wrong accordingly to the token address + // *This is an hotfix this logic should live on whatchAsset method on TokensController* + let fetchedDecimals, fetchedSymbol; + try { + [fetchedDecimals, fetchedSymbol] = await Promise.all([ + AssetsContractController.getERC20TokenDecimals(address), + AssetsContractController.getERC721AssetSymbol(address), + ]); + //The catch it's only to prevent the fetch from the chain to fail + // eslint-disable-next-line no-empty + } catch (e) {} + + const finalTokenSymbol = fetchedSymbol ?? symbol; + const finalTokenDecimals = fetchedDecimals ?? decimals; + + await TokensController.watchAsset( + { + address, + symbol: finalTokenSymbol, + decimals: finalTokenDecimals, + image, + }, + type, + safeToChecksumAddress(interactingAddress), + ); + + res.result = true; +}; + +export default wallet_watchAsset;