diff --git a/src/logic/collectibles/utils/__tests__/getTransferMethodByContractAddress.test.ts b/src/logic/collectibles/utils/__tests__/getTransferMethodByContractAddress.test.ts new file mode 100644 index 00000000..7a55a818 --- /dev/null +++ b/src/logic/collectibles/utils/__tests__/getTransferMethodByContractAddress.test.ts @@ -0,0 +1,68 @@ +import { getTransferMethodByContractAddress } from 'src/logic/collectibles/utils' + +jest.mock('src/config', () => { + // Require the original module to not be mocked... + const originalModule = jest.requireActual('src/config') + + return { + __esModule: true, // Use it when dealing with esModules + ...originalModule, + getNetworkId: jest.fn().mockReturnValue(4), + } +}) + +describe('getTransferMethodByContractAddress', () => { + const config = require('src/config') + + afterAll(() => { + jest.unmock('src/config') + }) + + it(`should return "transfer" method, if CK address is provided for MAINNET`, () => { + // Given + config.getNetworkId.mockReturnValue(1) + const contractAddress = '0x06012c8cf97bead5deae237070f9587f8e7a266d' + + // When + const selectedMethod = getTransferMethodByContractAddress(contractAddress) + + // Then + expect(selectedMethod).toBe('transfer') + }) + + it(`should return "transfer" method, if CK address is provided for RINKEBY`, () => { + // Given + config.getNetworkId.mockReturnValue(4) + const contractAddress = '0x16baf0de678e52367adc69fd067e5edd1d33e3bf' + + // When + const selectedMethod = getTransferMethodByContractAddress(contractAddress) + + // Then + expect(selectedMethod).toBe('transfer') + }) + + it(`should return "0x42842e0e" method, if CK address is provided any other network`, () => { + // Given + config.getNetworkId.mockReturnValue(100) + const contractAddress = '0x06012c8cf97bead5deae237070f9587f8e7a266d' + + // When + const selectedMethod = getTransferMethodByContractAddress(contractAddress) + + // Then + expect(selectedMethod).toBe('0x42842e0e') + }) + + it(`should return "0x42842e0e" method, if non-CK address is provided`, () => { + // Given + config.getNetworkId.mockReturnValue(4) + const contractAddress = '0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85' + + // When + const selectedMethod = getTransferMethodByContractAddress(contractAddress) + + // Then + expect(selectedMethod).toBe('0x42842e0e') + }) +}) diff --git a/src/logic/collectibles/utils/index.ts b/src/logic/collectibles/utils/index.ts new file mode 100644 index 00000000..c117aead --- /dev/null +++ b/src/logic/collectibles/utils/index.ts @@ -0,0 +1,131 @@ +import { getNetworkId } from 'src/config' +import { ETHEREUM_NETWORK } from 'src/config/networks/network.d' +import { nftAssetsListAddressesSelector } from 'src/logic/collectibles/store/selectors' +import { TxServiceModel } from 'src/logic/safe/store/actions/transactions/fetchTransactions/loadOutgoingTransactions' +import { TOKEN_TRANSFER_METHODS_NAMES } from 'src/logic/safe/store/models/types/transactions.d' +import { getERC721TokenContract, getStandardTokenContract } from 'src/logic/tokens/store/actions/fetchTokens' +import { sameAddress } from 'src/logic/wallets/ethAddresses' +import { CollectibleTx } from 'src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible' +import { store } from 'src/store' +import { sameString } from 'src/utils/strings' + +// CryptoKitties Contract Addresses by network +// This is an exception made for a popular NFT that's not ERC721 standard-compatible, +// so we can allow the user to transfer the assets by using `transferFrom` instead of +// the standard `safeTransferFrom` method. +export const CK_ADDRESS = { + [ETHEREUM_NETWORK.MAINNET]: '0x06012c8cf97bead5deae237070f9587f8e7a266d', + [ETHEREUM_NETWORK.RINKEBY]: '0x16baf0de678e52367adc69fd067e5edd1d33e3bf', +} + +// safeTransferFrom(address,address,uint256) +export const SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH = '42842e0e' + +/** + * Verifies that a tx received by the transaction service is an ERC721 token-related transaction + * @param {TxServiceModel} tx + * @returns boolean + */ +export const isSendERC721Transaction = (tx: TxServiceModel): boolean => { + let hasERC721Transfer = false + + if (tx.dataDecoded && sameString(tx.dataDecoded.method, TOKEN_TRANSFER_METHODS_NAMES.SAFE_TRANSFER_FROM)) { + hasERC721Transfer = tx.dataDecoded.parameters.findIndex((param) => sameString(param.name, 'tokenId')) !== -1 + } + + // Note: this is only valid with our current case (client rendering), if we move to server side rendering we need to refactor this + const state = store.getState() + const knownAssets = nftAssetsListAddressesSelector(state) + return knownAssets.includes(tx.to) || hasERC721Transfer +} + +/** + * Returns the symbol of the provided ERC721 contract + * @param {string} contractAddress + * @returns Promise + */ +export const getERC721Symbol = async (contractAddress: string): Promise => { + let tokenSymbol = 'UNKNOWN' + + try { + const ERC721token = await getERC721TokenContract() + const tokenInstance = await ERC721token.at(contractAddress) + tokenSymbol = tokenInstance.symbol() + } catch (err) { + // If the contract address is an ENS token contract, we know that the ERC721 standard is not proper implemented + // The method symbol() is missing + const ENS_TOKEN_CONTRACT = '0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85' + if (sameAddress(contractAddress, ENS_TOKEN_CONTRACT)) { + return 'ENS' + } + console.error(`Failed to retrieve token symbol for ERC721 token ${contractAddress}`) + } + + return tokenSymbol +} + +/** + * Verifies if the provided contract is a valid ERC721 + * @param {string} contractAddress + * @returns boolean + */ +export const isERC721Contract = async (contractAddress: string): Promise => { + const ERC721Token = await getStandardTokenContract() + let isERC721 = false + + try { + await ERC721Token.at(contractAddress) + isERC721 = true + } catch (error) { + console.warn('Asset not found') + } + + return isERC721 +} + +/** + * Returns a method identifier based on the asset specified and the current network + * @param {string} contractAddress + * @returns string + */ +export const getTransferMethodByContractAddress = (contractAddress: string): string => { + if (sameAddress(contractAddress, CK_ADDRESS[getNetworkId()])) { + // on mainnet `transferFrom` seems to work fine but we can assure that `transfer` will work on both networks + // so that's the reason why we're falling back to `transfer` for CryptoKitties + return 'transfer' + } + + return `0x${SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH}` +} + +/** + * Builds the encodedABI data for the transfer of an NFT token + * @param {CollectibleTx} tx + * @param {string} safeAddress + * @returns Promise + */ +export const generateERC721TransferTxData = async ( + tx: CollectibleTx, + safeAddress: string | undefined, +): Promise => { + if (!safeAddress) { + throw new Error('Failed to build NFT transfer tx data. SafeAddress is not defined.') + } + + const methodToCall = getTransferMethodByContractAddress(tx.assetAddress) + let transferParams = [tx.recipientAddress, tx.nftTokenId] + let NFTTokenContract + + if (methodToCall.includes(SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH)) { + // we add the `from` param for the `safeTransferFrom` method call + transferParams = [safeAddress, ...transferParams] + NFTTokenContract = await getERC721TokenContract() + } else { + // we fallback to an ERC20 Token contract whose ABI implements the `transfer` method + NFTTokenContract = await getStandardTokenContract() + } + + const tokenInstance = await NFTTokenContract.at(tx.assetAddress) + + return tokenInstance.contract.methods[methodToCall](...transferParams).encodeABI() +} diff --git a/src/logic/safe/store/actions/__tests__/transactionHelpers.test.ts b/src/logic/safe/store/actions/__tests__/transactionHelpers.test.ts index 43a76e6a..c4e80e91 100644 --- a/src/logic/safe/store/actions/__tests__/transactionHelpers.test.ts +++ b/src/logic/safe/store/actions/__tests__/transactionHelpers.test.ts @@ -273,9 +273,11 @@ describe('isOutgoingTransaction', () => { }) }) +jest.mock('src/logic/collectibles/utils') jest.mock('src/logic/tokens/utils/tokenHelpers') describe('isCustomTransaction', () => { afterAll(() => { + jest.unmock('src/logic/collectibles/utils') jest.unmock('src/logic/tokens/utils/tokenHelpers') }) it('It should return true if Is outgoing transaction, is not an erc20 transaction, not an upgrade transaction and not and erc721 transaction', async () => { @@ -292,10 +294,11 @@ describe('isCustomTransaction', () => { }) knownTokens.set('0x00Df91984582e6e96288307E9c2f20b38C8FeCE9', token) + const collectiblesHelpers = require('src/logic/collectibles/utils') const txHelpers = require('src/logic/tokens/utils/tokenHelpers') txHelpers.isSendERC20Transaction.mockImplementationOnce(() => false) - txHelpers.isSendERC721Transaction.mockImplementationOnce(() => false) + collectiblesHelpers.isSendERC721Transaction.mockImplementationOnce(() => false) // when const result = await isCustomTransaction(transaction, safeAddress) @@ -303,7 +306,7 @@ describe('isCustomTransaction', () => { // then expect(result).toBe(true) expect(txHelpers.isSendERC20Transaction).toHaveBeenCalled() - expect(txHelpers.isSendERC721Transaction).toHaveBeenCalled() + expect(collectiblesHelpers.isSendERC721Transaction).toHaveBeenCalled() }) it('It should return true if is outgoing transaction, is not SendERC20Transaction, is not isUpgradeTransaction and not isSendERC721Transaction', async () => { // given @@ -319,10 +322,11 @@ describe('isCustomTransaction', () => { }) knownTokens.set('0x00Df91984582e6e96288307E9c2f20b38C8FeCE9', token) + const collectiblesHelpers = require('src/logic/collectibles/utils') const txHelpers = require('src/logic/tokens/utils/tokenHelpers') txHelpers.isSendERC20Transaction.mockImplementationOnce(() => false) - txHelpers.isSendERC721Transaction.mockImplementationOnce(() => false) + collectiblesHelpers.isSendERC721Transaction.mockImplementationOnce(() => false) // when const result = await isCustomTransaction(transaction, safeAddress) @@ -330,7 +334,7 @@ describe('isCustomTransaction', () => { // then expect(result).toBe(true) expect(txHelpers.isSendERC20Transaction).toHaveBeenCalled() - expect(txHelpers.isSendERC721Transaction).toHaveBeenCalled() + expect(collectiblesHelpers.isSendERC721Transaction).toHaveBeenCalled() }) it('It should return false if is outgoing transaction, not SendERC20Transaction, isUpgradeTransaction and not isSendERC721Transaction', async () => { // given @@ -347,10 +351,11 @@ describe('isCustomTransaction', () => { }) knownTokens.set('0x00Df91984582e6e96288307E9c2f20b38C8FeCE9', token) + const collectiblesHelpers = require('src/logic/collectibles/utils') const txHelpers = require('src/logic/tokens/utils/tokenHelpers') txHelpers.isSendERC20Transaction.mockImplementationOnce(() => true) - txHelpers.isSendERC721Transaction.mockImplementationOnce(() => false) + collectiblesHelpers.isSendERC721Transaction.mockImplementationOnce(() => false) // when const result = await isCustomTransaction(transaction, safeAddress) @@ -373,10 +378,11 @@ describe('isCustomTransaction', () => { }) knownTokens.set('0x00Df91984582e6e96288307E9c2f20b38C8FeCE9', token) + const collectiblesHelpers = require('src/logic/collectibles/utils') const txHelpers = require('src/logic/tokens/utils/tokenHelpers') txHelpers.isSendERC20Transaction.mockImplementationOnce(() => false) - txHelpers.isSendERC721Transaction.mockImplementationOnce(() => true) + collectiblesHelpers.isSendERC721Transaction.mockImplementationOnce(() => true) // when const result = await isCustomTransaction(transaction, safeAddress) @@ -384,7 +390,7 @@ describe('isCustomTransaction', () => { // then expect(result).toBe(false) expect(txHelpers.isSendERC20Transaction).toHaveBeenCalled() - expect(txHelpers.isSendERC721Transaction).toHaveBeenCalled() + expect(collectiblesHelpers.isSendERC721Transaction).toHaveBeenCalled() }) }) diff --git a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts index 5df56fc5..b776b71c 100644 --- a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts +++ b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts @@ -1,11 +1,7 @@ import { List } from 'immutable' import { getNetworkInfo } from 'src/config' -import { - getERC20DecimalsAndSymbol, - getERC721Symbol, - isSendERC20Transaction, - isSendERC721Transaction, -} from 'src/logic/tokens/utils/tokenHelpers' +import { getERC20DecimalsAndSymbol, isSendERC20Transaction } from 'src/logic/tokens/utils/tokenHelpers' +import { getERC721Symbol, isSendERC721Transaction } from 'src/logic/collectibles/utils' import { sameAddress, ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses' import { EMPTY_DATA } from 'src/logic/wallets/ethTransactions' import { makeConfirmation } from 'src/logic/safe/store/models/confirmation' diff --git a/src/logic/tokens/utils/__tests__/tokenHelpers.test.ts b/src/logic/tokens/utils/__tests__/tokenHelpers.test.ts index 52ae9253..c211e8b4 100644 --- a/src/logic/tokens/utils/__tests__/tokenHelpers.test.ts +++ b/src/logic/tokens/utils/__tests__/tokenHelpers.test.ts @@ -1,5 +1,6 @@ import { makeToken } from 'src/logic/tokens/store/model/token' -import { getERC20DecimalsAndSymbol, isERC721Contract, isTokenTransfer } from 'src/logic/tokens/utils/tokenHelpers' +import { getERC20DecimalsAndSymbol, isTokenTransfer } from 'src/logic/tokens/utils/tokenHelpers' +import { isERC721Contract } from 'src/logic/collectibles/utils' import { getMockedTxServiceModel } from 'src/test/utils/safeHelper' import { fromTokenUnit, toTokenUnit } from 'src/logic/tokens/utils/humanReadableValue' diff --git a/src/logic/tokens/utils/tokenHelpers.ts b/src/logic/tokens/utils/tokenHelpers.ts index ef2f7852..fbf6dfe1 100644 --- a/src/logic/tokens/utils/tokenHelpers.ts +++ b/src/logic/tokens/utils/tokenHelpers.ts @@ -2,22 +2,13 @@ import { AbiItem } from 'web3-utils' import { getNetworkInfo } from 'src/config' import generateBatchRequests from 'src/logic/contracts/generateBatchRequests' -import { - getStandardTokenContract, - getTokenInfos, - getERC721TokenContract, -} from 'src/logic/tokens/store/actions/fetchTokens' +import { getTokenInfos } from 'src/logic/tokens/store/actions/fetchTokens' +import { isSendERC721Transaction } from 'src/logic/collectibles/utils' import { makeToken, Token } from 'src/logic/tokens/store/model/token' import { ALTERNATIVE_TOKEN_ABI } from 'src/logic/tokens/utils/alternativeAbi' import { web3ReadOnly as web3 } from 'src/logic/wallets/getWeb3' import { isEmptyData } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers' import { TxServiceModel } from 'src/logic/safe/store/actions/transactions/fetchTransactions/loadOutgoingTransactions' -import { sameString } from 'src/utils/strings' -import { TOKEN_TRANSFER_METHODS_NAMES } from 'src/logic/safe/store/models/types/transactions.d' -import { store } from 'src/store' -import { nftAssetsListAddressesSelector } from 'src/logic/collectibles/store/selectors' - -export const SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH = '42842e0e' export const getEthAsToken = (balance: string | number): Token => { const { nativeCoin } = getNetworkInfo() @@ -45,37 +36,6 @@ export const isTokenTransfer = (tx: TxServiceModel): boolean => { return !isEmptyData(tx.data) && tx.data?.substring(0, 10) === '0xa9059cbb' && Number(tx.value) === 0 } -export const isSendERC721Transaction = (tx: TxServiceModel): boolean => { - let hasERC721Transfer = false - - if (tx.dataDecoded && sameString(tx.dataDecoded.method, TOKEN_TRANSFER_METHODS_NAMES.SAFE_TRANSFER_FROM)) { - hasERC721Transfer = tx.dataDecoded.parameters.findIndex((param) => sameString(param.name, 'tokenId')) !== -1 - } - - // Note: this is only valid with our current case (client rendering), if we move to server side rendering we need to refactor this - const state = store.getState() - const knownAssets = nftAssetsListAddressesSelector(state) - return knownAssets.includes(tx.to) || hasERC721Transfer -} - -export const getERC721Symbol = async (contractAddress: string): Promise => { - let tokenSymbol = 'UNKNOWN' - try { - const ERC721token = await getERC721TokenContract() - const tokenInstance = await ERC721token.at(contractAddress) - tokenSymbol = tokenInstance.symbol() - } catch (err) { - // If the contract address is an ENS token contract, we know that the ERC721 standard is not proper implemented - // The method symbol() is missing - const ENS_TOKEN_CONTRACT = '0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85' - if (sameString(contractAddress, ENS_TOKEN_CONTRACT)) { - return 'ENS' - } - console.error(`Failed to retrieve token symbol for ERC721 token ${contractAddress}`) - } - return tokenSymbol -} - export const getERC20DecimalsAndSymbol = async ( tokenAddress: string, ): Promise<{ decimals: number; symbol: string }> => { @@ -115,17 +75,3 @@ export const isSendERC20Transaction = async (tx: TxServiceModel): Promise => { - const ERC721Token = await getStandardTokenContract() - let isERC721 = false - - try { - await ERC721Token.at(contractAddress) - isERC721 = true - } catch (error) { - console.warn('Asset not found') - } - - return isERC721 -} diff --git a/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx index 26ed380a..c1d5cd18 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx @@ -20,13 +20,12 @@ import createTransaction from 'src/logic/safe/store/actions/createTransaction' import { safeSelector } from 'src/logic/safe/store/selectors' import { TX_NOTIFICATION_TYPES } from 'src/logic/safe/transactions' import { estimateTxGasCosts } from 'src/logic/safe/transactions/gas' -import { getERC721TokenContract } from 'src/logic/tokens/store/actions/fetchTokens' import { formatAmount } from 'src/logic/tokens/utils/formatAmount' -import { SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH } from 'src/logic/tokens/utils/tokenHelpers' import SafeInfo from 'src/routes/safe/components/Balances/SendModal/SafeInfo' import { setImageToPlaceholder } from 'src/routes/safe/components/Balances/utils' import { sm } from 'src/theme/variables' import { textShortener } from 'src/utils/strings' +import { generateERC721TransferTxData } from 'src/logic/collectibles/utils' import ArrowDown from '../assets/arrow-down.svg' @@ -67,14 +66,8 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement = const estimateGas = async () => { try { - const methodToCall = `0x${SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH}` - const transferParams = [tx.recipientAddress, tx.nftTokenId] - const params = [safeAddress, ...transferParams] - const ERC721Token = await getERC721TokenContract() - const tokenInstance = await ERC721Token.at(tx.assetAddress) - const txData = tokenInstance.contract.methods[methodToCall](...params).encodeABI() - - const estimatedGasCosts = await estimateTxGasCosts(safeAddress as string, tx.recipientAddress, txData) + const txData = await generateERC721TransferTxData(tx, safeAddress) + const estimatedGasCosts = await estimateTxGasCosts(safeAddress ?? '', tx.recipientAddress, txData) const gasCosts = fromTokenUnit(estimatedGasCosts, nativeCoin.decimals) const formattedGasCosts = formatAmount(gasCosts) @@ -92,7 +85,7 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement = return () => { isCurrent = false } - }, [safeAddress, tx.assetAddress, tx.nftTokenId, tx.recipientAddress]) + }, [safeAddress, tx]) const submitTx = async () => { try { diff --git a/src/routes/safe/components/Balances/Tokens/screens/AddCustomAsset/validators.ts b/src/routes/safe/components/Balances/Tokens/screens/AddCustomAsset/validators.ts index 4bd84f6a..95dcbcb4 100644 --- a/src/routes/safe/components/Balances/Tokens/screens/AddCustomAsset/validators.ts +++ b/src/routes/safe/components/Balances/Tokens/screens/AddCustomAsset/validators.ts @@ -1,5 +1,5 @@ import memoize from 'lodash.memoize' -import { isERC721Contract } from 'src/logic/tokens/utils/tokenHelpers' +import { isERC721Contract } from 'src/logic/collectibles/utils' import { sameAddress } from 'src/logic/wallets/ethAddresses' // eslint-disable-next-line