From deb0ad8501cc098d05401295a08a60d9f702259b Mon Sep 17 00:00:00 2001 From: Mati Dastugue Date: Mon, 31 Aug 2020 18:07:12 -0300 Subject: [PATCH 1/5] Bump new onboard.js version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bcb633ca..8ed9be47 100644 --- a/package.json +++ b/package.json @@ -176,7 +176,7 @@ "async-sema": "^3.1.0", "axios": "0.19.2", "bignumber.js": "9.0.0", - "bnc-onboard": "1.11.1", + "bnc-onboard": "1.12.0", "classnames": "^2.2.6", "concurrently": "^5.2.0", "connected-react-router": "6.8.0", From 878c32c4a38effd98296eb269d749bf452d5d4fc Mon Sep 17 00:00:00 2001 From: Mati Dastugue Date: Wed, 9 Dec 2020 11:33:22 -0300 Subject: [PATCH 2/5] Collectibles utils --- ...getTransferMethodByContractAddress.test.ts | 68 +++++++++ src/logic/collectibles/utils/index.ts | 143 ++++++++++++++++++ 2 files changed, 211 insertions(+) create mode 100644 src/logic/collectibles/utils/__tests__/getTransferMethodByContractAddress.test.ts create mode 100644 src/logic/collectibles/utils/index.ts 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..8f7b346b --- /dev/null +++ b/src/logic/collectibles/utils/index.ts @@ -0,0 +1,143 @@ +import { getNetworkId, getNetworkInfo } 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', +} + +// Note: xDAI ENS is missing, once we have it we need to add it here +const ENS_CONTRACT_ADDRESS = { + [ETHEREUM_NETWORK.MAINNET]: '0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85', + [ETHEREUM_NETWORK.RINKEBY]: '0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85', + [ETHEREUM_NETWORK.ENERGY_WEB_CHAIN]: '0x0A6d64413c07E10E890220BBE1c49170080C6Ca0', + [ETHEREUM_NETWORK.VOLTA]: '0xd7CeF70Ba7efc2035256d828d5287e2D285CD1ac', +} + +// 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 = await 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 + if (isENSContract(contractAddress)) { + return 'ENS' + } + console.error(`Failed to retrieve token symbol for ERC721 token ${contractAddress}`) + } + + return tokenSymbol +} + +export const isENSContract = (contractAddress: string): boolean => { + const { id } = getNetworkInfo() + return sameAddress(contractAddress, ENS_CONTRACT_ADDRESS[id]) +} + +/** + * 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() +} From 71375f9d6494e04edfd7eeb8217973a3d4fb96d2 Mon Sep 17 00:00:00 2001 From: Mati Dastugue Date: Mon, 14 Dec 2020 14:53:34 -0300 Subject: [PATCH 3/5] Fix wrong status on txs --- .../store/actions/transactions/utils/transactionHelpers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts index b776b71c..fcc32275 100644 --- a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts +++ b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts @@ -173,12 +173,12 @@ export const calculateTransactionStatus = ( if (tx.isExecuted && tx.isSuccessful) { txStatus = TransactionStatus.SUCCESS + } else if (tx.creationTx) { + txStatus = TransactionStatus.SUCCESS } else if (tx.cancelled || nonce > tx.nonce) { txStatus = TransactionStatus.CANCELLED } else if (tx.confirmations.size === threshold) { txStatus = TransactionStatus.AWAITING_EXECUTION - } else if (tx.creationTx) { - txStatus = TransactionStatus.SUCCESS } else if (!!tx.isPending) { txStatus = TransactionStatus.PENDING } else { From 62232e6dc2444dae2fb8f2fca1377987adc8d119 Mon Sep 17 00:00:00 2001 From: Mati Dastugue Date: Fri, 18 Dec 2020 10:34:57 -0300 Subject: [PATCH 4/5] Prevent modal from closing on its own --- .../TxDescription/TransferDescription.tsx | 1 - .../Transactions/TxsTable/ExpandedTx/index.tsx | 12 +++++------- .../safe/components/Transactions/TxsTable/index.tsx | 13 +++++++------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/TransferDescription.tsx b/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/TransferDescription.tsx index 91a324ca..58a7c2a6 100644 --- a/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/TransferDescription.tsx +++ b/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/TransferDescription.tsx @@ -26,7 +26,6 @@ const TransferDescription = ({ }: TransferDescriptionProps): ReactElement | null => { const recipientName = useSelector((state) => getNameFromAddressBookSelector(state, recipient)) const [sendModalOpen, setSendModalOpen] = React.useState(false) - const sendModalOpenHandler = () => { setSendModalOpen(true) } diff --git a/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/index.tsx b/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/index.tsx index 22e8de7e..123f8559 100644 --- a/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/index.tsx +++ b/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/index.tsx @@ -73,13 +73,11 @@ const ExpandedModuleTx = ({ tx }: { tx: SafeModuleTransaction }): ReactElement = - {recipient && ( - - )} + diff --git a/src/routes/safe/components/Transactions/TxsTable/index.tsx b/src/routes/safe/components/Transactions/TxsTable/index.tsx index 9050674b..411b8b9a 100644 --- a/src/routes/safe/components/Transactions/TxsTable/index.tsx +++ b/src/routes/safe/components/Transactions/TxsTable/index.tsx @@ -35,6 +35,10 @@ const TxsTable = ({ classes }) => { trackEvent({ category: SAFE_NAVIGATION_EVENT, action: 'Transactions' }) }, [trackEvent]) + useEffect(() => { + console.log('La coupeeeee') + }, []) + const handleTxExpand = (rowId) => { setExpandedTx((prevRowId) => (prevRowId === rowId ? null : rowId)) } @@ -121,12 +125,9 @@ const TxsTable = ({ classes }) => { colSpan={6} style={{ paddingBottom: 0, paddingTop: 0 }} > - } - in={expandedTx === rowId} - timeout="auto" - unmountOnExit - /> + + + From 2af0fb0cb11cd8b84d99f453408b73dcfaf850e3 Mon Sep 17 00:00:00 2001 From: Mati Dastugue Date: Fri, 18 Dec 2020 10:44:04 -0300 Subject: [PATCH 5/5] Update index.tsx --- src/routes/safe/components/Transactions/TxsTable/index.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/routes/safe/components/Transactions/TxsTable/index.tsx b/src/routes/safe/components/Transactions/TxsTable/index.tsx index f1a66386..0e5b2687 100644 --- a/src/routes/safe/components/Transactions/TxsTable/index.tsx +++ b/src/routes/safe/components/Transactions/TxsTable/index.tsx @@ -38,10 +38,6 @@ const TxsTable = (): React.ReactElement => { trackEvent({ category: SAFE_NAVIGATION_EVENT, action: 'Transactions' }) }, [trackEvent]) - useEffect(() => { - console.log('La coupeeeee') - }, []) - const handleTxExpand = (rowId) => { setExpandedTx((prevRowId) => (prevRowId === rowId ? null : rowId)) }