m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-22 17:33:23 +01:00
Code

Improve tokenId parsing and clean up useAssetDetails hook (#15304)

Browse Source
This commit is contained in:
Alex Donesky 2022-07-23 09:37:31 -05:00 committed by GitHub
parent 1f943a7d69
commit 8536c86ed5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 139 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
app/scripts/metamask-controller.js
View File

@ -91,7 +91,10 @@ import {
import { EVENT, EVENT_NAMES } from '../../shared/constants/metametrics';
import { hexToDecimal } from '../../ui/helpers/utils/conversions.util';
import { getTokenValueParam } from '../../ui/helpers/utils/token-util';
import {
getTokenIdParam,
getTokenValueParam,
} from '../../ui/helpers/utils/token-util';
import { isEqualCaseInsensitive } from '../../shared/modules/string-utils';
import { parseStandardTokenTransactionData } from '../../shared/modules/transaction.utils';
import {
@ -866,7 +869,12 @@ export default class MetamaskController extends EventEmitter {
} = txMeta.txParams;
const { chainId } = txMeta;
const transactionData = parseStandardTokenTransactionData(data);
const tokenAmountOrTokenId = getTokenValueParam(transactionData);
// Sometimes the tokenId value is parsed as "_value" param. Not seeing this often any more, but still occasionally:
// i.e. call approve() on BAYC contract - https://etherscan.io/token/0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d#writeContract, and tokenId shows up as _value,
// not sure why since it doesn't match the ERC721 ABI spec we use to parse these transactions - https://github.com/MetaMask/metamask-eth-abis/blob/d0474308a288f9252597b7c93a3a8deaad19e1b2/src/abis/abiERC721.ts#L62.
const transactionDataTokenId =
getTokenIdParam(transactionData) ??
getTokenValueParam(transactionData);
const { allCollectibles } = this.collectiblesController.state;
// check if its a known collectible
@ -875,7 +883,7 @@ export default class MetamaskController extends EventEmitter {
].find(
({ address, tokenId }) =>
isEqualCaseInsensitive(address, contractAddress) &&
tokenId === tokenAmountOrTokenId,
tokenId === transactionDataTokenId,
);
// if it is we check and update ownership status.

7
ui/ducks/send/send.js
View File

@ -70,6 +70,7 @@ import {
getTokenAddressParam,
getTokenValueParam,
getTokenMetadata,
getTokenIdParam,
} from '../../helpers/utils/token-util';
import {
checkExistingAddresses,
@ -1749,7 +1750,11 @@ export function editExistingTransaction(assetType, transactionId) {
details: {
address: transaction.txParams.to,
...(assetType === ASSET_TYPES.COLLECTIBLE
? { tokenId: getTokenValueParam(tokenData) }
? {
tokenId:
getTokenIdParam(tokenData) ??
getTokenValueParam(tokenData),
}
: {}),
},
},

93
ui/helpers/utils/token-util.js
View File

@ -7,7 +7,7 @@ import {
import { getTokenStandardAndDetails } from '../../store/actions';
import { isEqualCaseInsensitive } from '../../../shared/modules/string-utils';
import { parseStandardTokenTransactionData } from '../../../shared/modules/transaction.utils';
import { ERC1155, ERC721 } from '../../../shared/constants/transaction';
import { ERC20 } from '../../../shared/constants/transaction';
import * as util from './util';
import { formatCurrency } from './confirm-tx.util';
@ -151,13 +151,29 @@ export function getTokenValueParam(tokenData = {}) {
return tokenData?.args?._value?.toString();
}
export function getTokenApprovedParam(tokenData = {}) {
return tokenData?.args?._approved;
/**
* Gets either the '_tokenId' parameter or the 'id' param of the passed token transaction data.,
* These are the parsed tokenId values returned by `parseStandardTokenTransactionData` as defined
* in the ERC721 and ERC1155 ABIs from metamask-eth-abis (https://github.com/MetaMask/metamask-eth-abis/tree/main/src/abis)
*
* @param {Object} tokenData - ethers Interface token data.
* @returns {string | undefined} A decimal string value.
*/
export function getTokenIdParam(tokenData = {}) {
return (
tokenData?.args?._tokenId?.toString() ?? tokenData?.args?.id?.toString()
);
}
export function getTokenValue(tokenParams = []) {
const valueData = tokenParams.find((param) => param.name === '_value');
return valueData && valueData.value;
/**
* Gets the '_approved' parameter of the given token transaction data
* (i.e function call) per the Human Standard Token ABI, if present.
*
* @param {Object} tokenData - ethers Interface token data.
* @returns {boolean | undefined} A boolean indicating whether the function is being called to approve or revoke access.
*/
export function getTokenApprovedParam(tokenData = {}) {
return tokenData?.args?._approved;
}
/**
@ -232,8 +248,32 @@ export async function getAssetDetails(
throw new Error('Unable to detect valid token data');
}
const tokenId = getTokenValueParam(tokenData);
// Sometimes the tokenId value is parsed as "_value" param. Not seeing this often any more, but still occasionally:
// i.e. call approve() on BAYC contract - https://etherscan.io/token/0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d#writeContract, and tokenId shows up as _value,
// not sure why since it doesn't match the ERC721 ABI spec we use to parse these transactions - https://github.com/MetaMask/metamask-eth-abis/blob/d0474308a288f9252597b7c93a3a8deaad19e1b2/src/abis/abiERC721.ts#L62.
let tokenId =
getTokenIdParam(tokenData)?.toString() ?? getTokenValueParam(tokenData);
const toAddress = getTokenAddressParam(tokenData);
let tokenDetails;
// if a tokenId is present check if there is a collectible in state matching the address/tokenId
// and avoid unnecessary network requests to query token details we already have
if (existingCollectibles?.length && tokenId) {
const existingCollectible = existingCollectibles.find(
({ address, tokenId: _tokenId }) =>
isEqualCaseInsensitive(tokenAddress, address) && _tokenId === tokenId,
);
if (existingCollectible) {
return {
toAddress,
...existingCollectible,
};
}
}
try {
tokenDetails = await getTokenStandardAndDetails(
tokenAddress,
@ -242,26 +282,31 @@ export async function getAssetDetails(
);
} catch (error) {
log.warn(error);
return {};
// if we can't determine any token standard or details return the data we can extract purely from the parsed transaction data
return { toAddress, tokenId };
}
if (tokenDetails?.standard) {
const { standard } = tokenDetails;
if (standard === ERC721 || standard === ERC1155) {
const existingCollectible = existingCollectibles.find(({ address }) =>
isEqualCaseInsensitive(tokenAddress, address),
);
const tokenAmount =
tokenData &&
tokenDetails?.decimals &&
calcTokenAmount(
getTokenValueParam(tokenData),
tokenDetails?.decimals,
).toString(10);
if (existingCollectible) {
return {
...existingCollectible,
standard,
};
}
}
// else if not a collectible already in state or standard === ERC20 just return tokenDetails as it contains all required data
return tokenDetails;
const decimals =
tokenDetails?.decimals && Number(tokenDetails.decimals?.toString(10));
if (tokenDetails?.standard === ERC20) {
tokenId = undefined;
}
return {};
// else if not a collectible already in state or standard === ERC20 return tokenDetails and tokenId
return {
tokenAmount,
toAddress,
decimals,
tokenId,
...tokenDetails,
};
}

70
ui/hooks/useAssetDetails.js
View File

@ -1,14 +1,7 @@
import { useState, useEffect } from 'react';
import { useSelector, useDispatch } from 'react-redux';
import { ERC1155, ERC20, ERC721 } from '../../shared/constants/transaction';
import { parseStandardTokenTransactionData } from '../../shared/modules/transaction.utils';
import { getCollectibles } from '../ducks/metamask/metamask';
import {
calcTokenAmount,
getAssetDetails,
getTokenAddressParam,
getTokenValueParam,
} from '../helpers/utils/token-util';
import { getAssetDetails } from '../helpers/utils/token-util';
import { hideLoadingIndication, showLoadingIndication } from '../store/actions';
import { usePrevious } from './usePrevious';
@ -55,18 +48,6 @@ export function useAssetDetails(tokenAddress, userAddress, transactionData) {
collectibles,
]);
let assetStandard,
assetName,
assetAddress,
tokenSymbol,
decimals,
tokenImage,
userBalance,
tokenValue,
toAddress,
tokenAmount,
tokenId;
if (currentAsset) {
const {
standard,
@ -74,40 +55,25 @@ export function useAssetDetails(tokenAddress, userAddress, transactionData) {
image,
name,
balance,
decimals: currentAssetDecimals,
tokenId,
toAddress,
tokenAmount,
decimals,
} = currentAsset;
const tokenData = parseStandardTokenTransactionData(transactionData);
assetStandard = standard;
assetAddress = tokenAddress;
tokenSymbol = symbol ?? '';
tokenImage = image;
toAddress = getTokenAddressParam(tokenData);
if (assetStandard === ERC721 || assetStandard === ERC1155) {
assetName = name;
tokenId = getTokenValueParam(tokenData);
}
if (assetStandard === ERC20) {
userBalance = balance;
decimals = Number(currentAssetDecimals?.toString(10));
tokenAmount =
tokenData &&
calcTokenAmount(getTokenValueParam(tokenData), decimals).toString(10);
}
return {
toAddress,
tokenId,
decimals,
tokenAmount,
assetAddress: tokenAddress,
assetStandard: standard,
tokenSymbol: symbol ?? '',
tokenImage: image,
userBalance: balance,
assetName: name,
};
}
return {
assetStandard,
assetName,
assetAddress,
userBalance,
tokenSymbol,
decimals,
tokenImage,
tokenValue,
toAddress,
tokenAmount,
tokenId,
};
return {};
}

60
ui/hooks/useAssetDetails.test.js
View File

@ -3,7 +3,7 @@ import { Provider } from 'react-redux';
import { renderHook } from '@testing-library/react-hooks';
import configureStore from '../store/store';
import * as tokenUtils from '../helpers/utils/token-util';
import * as Actions from '../store/actions';
import { ERC1155, ERC20, ERC721 } from '../../shared/constants/transaction';
import { useAssetDetails } from './useAssetDetails';
@ -33,13 +33,15 @@ const renderUseAssetDetails = ({
};
describe('useAssetDetails', () => {
let getAssetDetailsStub;
let getTokenStandardAndDetailsStub;
beforeEach(() => {
getAssetDetailsStub = jest
.spyOn(tokenUtils, 'getAssetDetails')
getTokenStandardAndDetailsStub = jest
.spyOn(Actions, 'getTokenStandardAndDetails')
.mockImplementation(() => Promise.resolve({}));
});
it('should return object with tokenSymbol set to and empty string, when getAssetDetails returns and empty object', async () => {
it('should return object with tokenSymbol set to an empty string, when getAssetDetails returns and empty object', async () => {
const toAddress = '000000000000000000000000000000000000dead';
const tokenAddress = '0x1';
@ -53,19 +55,12 @@ describe('useAssetDetails', () => {
await waitForNextUpdate();
expect(result.current).toStrictEqual({
assetAddress: tokenAddress,
assetName: undefined,
assetStandard: undefined,
decimals: undefined,
toAddress: `0x${toAddress}`,
tokenAmount: undefined,
tokenId: undefined,
tokenImage: undefined,
tokenSymbol: '',
tokenValue: undefined,
userBalance: undefined,
});
expect(result.current).toStrictEqual(
expect.objectContaining({
assetAddress: tokenAddress,
tokenSymbol: '',
}),
);
});
it('should return object with correct tokenValues for an ERC20 token', async () => {
@ -79,11 +74,11 @@ describe('useAssetDetails', () => {
const balance = '1';
const decimals = 18;
getAssetDetailsStub.mockImplementation(() =>
getTokenStandardAndDetailsStub.mockImplementation(() =>
Promise.resolve({
standard,
symbol,
balance,
symbol,
decimals,
}),
);
@ -106,7 +101,6 @@ describe('useAssetDetails', () => {
tokenId: undefined,
tokenImage: undefined,
tokenSymbol: symbol,
tokenValue: undefined,
userBalance: balance,
});
});
@ -114,21 +108,22 @@ describe('useAssetDetails', () => {
it('should return object with correct tokenValues for an ERC721 token', async () => {
const tokenAddress = '0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D';
const toAddress = '000000000000000000000000000000000000dead';
const transactionData = `0x23b872dd000000000000000000000000a544eebe103733f22ef62af556023bc918b73d36000000000000000000000000${toAddress}000000000000000000000000000000000000000000000000000000000000000c`;
const tokenId = '12';
const transactionData = `0x23b872dd000000000000000000000000a544eebe103733f22ef62af556023bc918b73d36000000000000000000000000${toAddress}000000000000000000000000000000000000000000000000000000000000000${Number(
tokenId,
).toString(16)}`;
const symbol = 'BAYC';
const tokenId = '12';
const name = 'BoredApeYachtClub';
const image =
'https://bafybeihw3gvmthmvrenfmcvagtais5tv7r4nmiezgsv7nyknjubxw4lite.ipfs.dweb.link';
const standard = ERC721;
getAssetDetailsStub.mockImplementation(() =>
getTokenStandardAndDetailsStub.mockImplementation(() =>
Promise.resolve({
standard,
symbol,
name,
tokenId,
image,
}),
);
@ -149,7 +144,6 @@ describe('useAssetDetails', () => {
tokenId,
tokenImage: image,
tokenSymbol: symbol,
tokenValue: undefined,
userBalance: undefined,
tokenAmount: undefined,
});
@ -158,17 +152,20 @@ describe('useAssetDetails', () => {
it('should return object with correct tokenValues for an ERC1155 token', async () => {
const tokenAddress = '0x76BE3b62873462d2142405439777e971754E8E77';
const toAddress = '000000000000000000000000000000000000dead';
const transactionData = `0xf242432a000000000000000000000000a544eebe103733f22ef62af556023bc918b73d36000000000000000000000000000000000000000000000000000000000000dead0000000000000000000000000000000000000000000000000000000000000322000000000000000000000000000000000000000000000000000000000000009c00000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000`;
const tokenId = '802';
const transactionData = `0xf242432a000000000000000000000000a544eebe103733f22ef62af556023bc918b73d36000000000000000000000000000000000000000000000000000000000000dead0000000000000000000000000000000000000000000000000000000000000${Number(
tokenId,
).toString(
16,
)}000000000000000000000000000000000000000000000000000000000000009c00000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000`;
const tokenId = '121';
const image =
'https://bafybeihw3gvmthmvrenfmcvagtais5tv7r4nmiezgsv7nyknjubxw4lite.ipfs.dweb.link';
const standard = ERC1155;
getAssetDetailsStub.mockImplementation(() =>
getTokenStandardAndDetailsStub.mockImplementation(() =>
Promise.resolve({
standard,
tokenId,
image,
}),
);
@ -186,10 +183,9 @@ describe('useAssetDetails', () => {
assetStandard: standard,
decimals: undefined,
toAddress: `0x${toAddress}`,
tokenId: undefined,
tokenId,
tokenImage: image,
tokenSymbol: '',
tokenValue: undefined,
userBalance: undefined,
tokenAmount: undefined,
});

11
ui/hooks/useTransactionDisplayData.js
View File

@ -8,6 +8,7 @@ import { camelCaseToCapitalize } from '../helpers/utils/common.util';
import { PRIMARY, SECONDARY } from '../helpers/constants/common';
import {
getTokenAddressParam,
getTokenIdParam,
getTokenValueParam,
} from '../helpers/utils/token-util';
import {
@ -138,16 +139,18 @@ export function useTransactionDisplayData(transactionGroup) {
isTokenCategory,
);
// If this is an ERC20 token transaction this value is equal to the amount sent
// If it is an ERC721 token transaction it is the tokenId being sent
const tokenAmountOrTokenId = getTokenValueParam(tokenData);
// Sometimes the tokenId value is parsed as "_value" param. Not seeing this often any more, but still occasionally:
// i.e. call approve() on BAYC contract - https://etherscan.io/token/0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d#writeContract, and tokenId shows up as _value,
// not sure why since it doesn't match the ERC721 ABI spec we use to parse these transactions - https://github.com/MetaMask/metamask-eth-abis/blob/d0474308a288f9252597b7c93a3a8deaad19e1b2/src/abis/abiERC721.ts#L62.
const transactionDataTokenId =
getTokenIdParam(tokenData) ?? getTokenValueParam(tokenData);
const collectible =
isTokenCategory &&
knownCollectibles.find(
({ address, tokenId }) =>
isEqualCaseInsensitive(address, recipientAddress) &&
tokenId === tokenAmountOrTokenId,
tokenId === transactionDataTokenId,
);
const tokenDisplayValue = useTokenDisplayValue(