From 4f0115fcdc0f32921e07ec0a095e95972fe9814d Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Mon, 11 Jul 2022 18:32:55 -0500 Subject: [PATCH] Add setApprovalForAll confirmation view (#15010) * enhance setApprovalForAll confirmation flow * cleanup * address feedback --- app/_locales/en/messages.json | 26 ++++++++ shared/constants/transaction.js | 3 + shared/modules/transaction.utils.js | 5 +- ...onfirm-page-container-summary.component.js | 3 +- ...transaction-list-item-details.component.js | 5 +- ui/helpers/constants/routes.js | 3 + ui/helpers/constants/transactions.js | 1 + ui/helpers/utils/token-util.js | 4 ++ ui/helpers/utils/transactions.util.js | 4 ++ ui/hooks/useTransactionDisplayData.js | 6 ++ .../confirm-approve-content.component.js | 64 +++++++++++++++---- ui/pages/confirm-approve/confirm-approve.js | 15 ++++- .../confirm-transaction-base.component.js | 1 + .../confirm-transaction-switch.component.js | 5 ++ .../confirm-token-transaction-switch.js | 25 ++++++++ 15 files changed, 154 insertions(+), 16 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index e7fe44c60..78bb505b8 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -230,6 +230,10 @@ "alerts": { "message": "Alerts" }, + "allOfYour": { + "message": "All of your $1", + "description": "$1 is the symbol or name of the token that the user is approving spending" + }, "allowExternalExtensionTo": { "message": "Allow this external extension to:" }, @@ -266,6 +270,10 @@ "approve": { "message": "Approve spend limit" }, + "approveAllTokensTitle": { + "message": "Give permission to access all of your $1?", + "description": "$1 is the symbol of the token for which the user is granting approval" + }, "approveAndInstall": { "message": "Approve & Install" }, @@ -1287,6 +1295,9 @@ "functionApprove": { "message": "Function: Approve" }, + "functionSetApprovalForAll": { + "message": "Function: SetApprovalForAll" + }, "functionType": { "message": "Function Type" }, @@ -2696,6 +2707,14 @@ "revealTheSeedPhrase": { "message": "Reveal seed phrase" }, + "revokeAllTokensTitle": { + "message": "Revoke permission to access all of your $1?", + "description": "$1 is the symbol of the token for which the user is revoking approval" + }, + "revokeApproveForAllDescription": { + "message": "By revoking permission, the following $1 will no longer be able to access your $2", + "description": "$1 is either key 'account' or 'contract', and $2 is either a string or link of a given token symbol or name" + }, "rinkeby": { "message": "Rinkeby Test Network" }, @@ -2878,6 +2897,13 @@ "setAdvancedPrivacySettingsDetails": { "message": "MetaMask uses these trusted third-party services to enhance product usability and safety." }, + "setApprovalForAll": { + "message": "Set Approval for All" + }, + "setApprovalForAllTitle": { + "message": "Approve $1 with no spend limit", + "description": "The token symbol that is being approved" + }, "settings": { "message": "Settings" }, diff --git a/shared/constants/transaction.js b/shared/constants/transaction.js index 8cb365f79..d2a54ae35 100644 --- a/shared/constants/transaction.js +++ b/shared/constants/transaction.js @@ -15,6 +15,8 @@ import { MESSAGE_TYPE } from './app'; * to ensure that the receiver is an address capable of handling with the token being sent. * @property {'approve'} TOKEN_METHOD_APPROVE - A token transaction requesting an * allowance of the token to spend on behalf of the user + * @property {'setapprovalforall'} TOKEN_METHOD_SET_APPROVAL_FOR_ALL - A token transaction requesting an + * allowance of all of a user's token to spend on behalf of the user * @property {'incoming'} INCOMING - An incoming (deposit) transaction * @property {'simpleSend'} SIMPLE_SEND - A transaction sending a network's native asset to a recipient * @property {'contractInteraction'} CONTRACT_INTERACTION - A transaction that is @@ -66,6 +68,7 @@ export const TRANSACTION_TYPES = { TOKEN_METHOD_SAFE_TRANSFER_FROM: 'safetransferfrom', TOKEN_METHOD_TRANSFER: 'transfer', TOKEN_METHOD_TRANSFER_FROM: 'transferfrom', + TOKEN_METHOD_SET_APPROVAL_FOR_ALL: 'setapprovalforall', }; /** diff --git a/shared/modules/transaction.utils.js b/shared/modules/transaction.utils.js index 08adb46e3..1910ef2f3 100644 --- a/shared/modules/transaction.utils.js +++ b/shared/modules/transaction.utils.js @@ -8,7 +8,7 @@ import { readAddressAsContract } from './contract-utils'; import { isEqualCaseInsensitive } from './string-utils'; /** - * @typedef { 'transfer' | 'approve' | 'transferfrom' | 'contractInteraction'| 'simpleSend' } InferrableTransactionTypes + * @typedef { 'transfer' | 'approve' | 'setapprovalforall' | 'transferfrom' | 'contractInteraction'| 'simpleSend' } InferrableTransactionTypes */ /** @@ -150,6 +150,7 @@ export async function determineTransactionType(txParams, query) { const tokenMethodName = [ TRANSACTION_TYPES.TOKEN_METHOD_APPROVE, + TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL, TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER, TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM, TRANSACTION_TYPES.TOKEN_METHOD_SAFE_TRANSFER_FROM, @@ -181,6 +182,7 @@ export async function determineTransactionType(txParams, query) { const INFERRABLE_TRANSACTION_TYPES = [ TRANSACTION_TYPES.TOKEN_METHOD_APPROVE, + TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL, TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER, TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM, TRANSACTION_TYPES.CONTRACT_INTERACTION, @@ -220,6 +222,7 @@ export async function determineTransactionAssetType( // method to get the asset type. const isTokenMethod = [ TRANSACTION_TYPES.TOKEN_METHOD_APPROVE, + TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL, TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER, TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM, ].find((methodName) => methodName === inferrableType); diff --git a/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-summary/confirm-page-container-summary.component.js b/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-summary/confirm-page-container-summary.component.js index 2d5fb8193..e31e81bce 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-summary/confirm-page-container-summary.component.js +++ b/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-summary/confirm-page-container-summary.component.js @@ -53,7 +53,8 @@ const ConfirmPageContainerSummary = (props) => { contractAddress = transactionType === TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER || transactionType === TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM || - transactionType === TRANSACTION_TYPES.TOKEN_METHOD_SAFE_TRANSFER_FROM + transactionType === TRANSACTION_TYPES.TOKEN_METHOD_SAFE_TRANSFER_FROM || + transactionType === TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL ? tokenAddress : toAddress; } diff --git a/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js b/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js index 331885364..f65c82f63 100644 --- a/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js +++ b/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js @@ -251,7 +251,10 @@ export default class TransactionListItemDetails extends PureComponent {
param.name === '_value'); return valueData && valueData.value; diff --git a/ui/helpers/utils/transactions.util.js b/ui/helpers/utils/transactions.util.js index d1380d8c0..07d37341d 100644 --- a/ui/helpers/utils/transactions.util.js +++ b/ui/helpers/utils/transactions.util.js @@ -116,6 +116,7 @@ export function isTokenMethodAction(type) { return [ TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER, TRANSACTION_TYPES.TOKEN_METHOD_APPROVE, + TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL, TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM, TRANSACTION_TYPES.TOKEN_METHOD_SAFE_TRANSFER_FROM, ].includes(type); @@ -217,6 +218,9 @@ export function getTransactionTypeTitle(t, type, nativeCurrency = 'ETH') { case TRANSACTION_TYPES.TOKEN_METHOD_APPROVE: { return t('approve'); } + case TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL: { + return t('setApprovalForAll'); + } case TRANSACTION_TYPES.SIMPLE_SEND: { return t('sendingNativeAsset', [nativeCurrency]); } diff --git a/ui/hooks/useTransactionDisplayData.js b/ui/hooks/useTransactionDisplayData.js index 184374496..9c6a13768 100644 --- a/ui/hooks/useTransactionDisplayData.js +++ b/ui/hooks/useTransactionDisplayData.js @@ -222,6 +222,12 @@ export function useTransactionDisplayData(transactionGroup) { title = t('approveSpendLimit', [token?.symbol || t('token')]); subtitle = origin; subtitleContainsOrigin = true; + } else if (type === TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL) { + category = TRANSACTION_GROUP_CATEGORIES.APPROVAL; + prefix = ''; + title = t('setApprovalForAllTitle', [token?.symbol || t('token')]); + subtitle = origin; + subtitleContainsOrigin = true; } else if (type === TRANSACTION_TYPES.CONTRACT_INTERACTION) { category = TRANSACTION_GROUP_CATEGORIES.INTERACTION; const transactionTypeTitle = getTransactionTypeTitle(t, type); diff --git a/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js b/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js index 7a6d32cce..cb831bbd3 100644 --- a/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js +++ b/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js @@ -71,6 +71,8 @@ export default class ConfirmApproveContent extends Component { assetName: PropTypes.string, tokenId: PropTypes.string, assetStandard: PropTypes.string, + isSetApproveForAll: PropTypes.bool, + setApproveForAllArg: PropTypes.bool, }; state = { @@ -184,7 +186,7 @@ export default class ConfirmApproveContent extends Component { renderERC721OrERC1155PermissionContent() { const { t } = this.context; - const { origin, toAddress, isContract } = this.props; + const { origin, toAddress, isContract, isSetApproveForAll } = this.props; const titleTokenDescription = this.getTitleTokenDescription(); @@ -201,7 +203,9 @@ export default class ConfirmApproveContent extends Component { {t('approvedAsset')}:
- {titleTokenDescription} + {isSetApproveForAll + ? `${t('allOfYour', [titleTokenDescription])} ` + : titleTokenDescription}
@@ -299,12 +303,19 @@ export default class ConfirmApproveContent extends Component { renderDataContent() { const { t } = this.context; - const { data } = this.props; + const { data, isSetApproveForAll, setApproveForAllArg } = this.props; return (
- {t('functionApprove')} + {isSetApproveForAll + ? t('functionSetApprovalForAll') + : t('functionApprove')}
+ {isSetApproveForAll && setApproveForAllArg !== undefined ? ( +
+ {t('parameters')}: {setApproveForAllArg} +
+ ) : null}
{data}
@@ -509,6 +520,41 @@ export default class ConfirmApproveContent extends Component { return titleTokenDescription; } + renderTitle() { + const { t } = this.context; + const { isSetApproveForAll, setApproveForAllArg } = this.props; + const titleTokenDescription = this.getTitleTokenDescription(); + + let title; + + if (isSetApproveForAll) { + title = t('approveAllTokensTitle', [titleTokenDescription]); + if (setApproveForAllArg === false) { + title = t('revokeAllTokensTitle', [titleTokenDescription]); + } + } + return title || t('allowSpendToken', [titleTokenDescription]); + } + + renderDescription() { + const { t } = this.context; + const { isContract, isSetApproveForAll, setApproveForAllArg } = this.props; + const grantee = isContract + ? t('contract').toLowerCase() + : t('account').toLowerCase(); + + let description = t('trustSiteApprovePermission', [grantee]); + + if (isSetApproveForAll && setApproveForAllArg === false) { + description = t('revokeApproveForAllDescription', [ + grantee, + this.getTitleTokenDescription(), + ]); + } + + return description; + } + render() { const { t } = this.context; const { @@ -534,8 +580,6 @@ export default class ConfirmApproveContent extends Component { } = this.props; const { showFullTxDetails } = this.state; - const titleTokenDescription = this.getTitleTokenDescription(); - return (
- {t('allowSpendToken', [titleTokenDescription])} + {this.renderTitle()}
- {t('trustSiteApprovePermission', [ - isContract - ? t('contract').toLowerCase() - : t('account').toLowerCase(), - ])} + {this.renderDescription()}
diff --git a/ui/pages/confirm-approve/confirm-approve.js b/ui/pages/confirm-approve/confirm-approve.js index 6dca97d1b..c1b35c24d 100644 --- a/ui/pages/confirm-approve/confirm-approve.js +++ b/ui/pages/confirm-approve/confirm-approve.js @@ -8,7 +8,10 @@ import { updateCustomNonce, getNextNonce, } from '../../store/actions'; -import { calcTokenAmount } from '../../helpers/utils/token-util'; +import { + calcTokenAmount, + getTokenApprovedParam, +} from '../../helpers/utils/token-util'; import { readAddressAsContract } from '../../../shared/modules/contract-utils'; import { GasFeeContextProvider } from '../../contexts/gasFee'; import { TransactionModalContextProvider } from '../../contexts/transaction-modal'; @@ -34,6 +37,7 @@ import EditGasFeePopover from '../../components/app/edit-gas-fee-popover'; import EditGasPopover from '../../components/app/edit-gas-popover/edit-gas-popover.component'; import Loading from '../../components/ui/loading-screen'; import { ERC20, ERC1155, ERC721 } from '../../helpers/constants/common'; +import { parseStandardTokenTransactionData } from '../../../shared/modules/transaction.utils'; import { getCustomTxParamsData } from './confirm-approve.util'; import ConfirmApproveContent from './confirm-approve-content'; @@ -57,6 +61,7 @@ export default function ConfirmApprove({ ethTransactionTotal, fiatTransactionTotal, hexTransactionTotal, + isSetApproveForAll, }) { const dispatch = useDispatch(); const { txParams: { data: transactionData } = {} } = transaction; @@ -150,6 +155,11 @@ export default function ConfirmApprove({ }) : null; + const parsedTransactionData = parseStandardTokenTransactionData( + transactionData, + ); + const setApproveForAllArg = getTokenApprovedParam(parsedTransactionData); + return tokenSymbol === undefined && assetName === undefined ? ( ) : ( @@ -162,6 +172,8 @@ export default function ConfirmApprove({ contentComponent={ ; } + case TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL: { + const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_SET_APPROVAL_FOR_ALL_PATH}`; + return ; + } case TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM: { const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_TRANSFER_FROM_PATH}`; return ; diff --git a/ui/pages/confirm-transaction/confirm-token-transaction-switch.js b/ui/pages/confirm-transaction/confirm-token-transaction-switch.js index 036f2b149..43b554595 100644 --- a/ui/pages/confirm-transaction/confirm-token-transaction-switch.js +++ b/ui/pages/confirm-transaction/confirm-token-transaction-switch.js @@ -6,6 +6,7 @@ import { CONFIRM_APPROVE_PATH, CONFIRM_SAFE_TRANSFER_FROM_PATH, CONFIRM_SEND_TOKEN_PATH, + CONFIRM_SET_APPROVAL_FOR_ALL_PATH, CONFIRM_TRANSACTION_ROUTE, CONFIRM_TRANSFER_FROM_PATH, } from '../../helpers/constants/routes'; @@ -66,6 +67,30 @@ export default function ConfirmTokenTransactionSwitch({ transaction }) { /> )} /> + ( + + )} + />