From 3c6cdef0747542215b928d9781d3ed1ff3e43d2a Mon Sep 17 00:00:00 2001 From: ryanml Date: Tue, 2 Mar 2021 17:28:12 -0700 Subject: [PATCH] Adding a warning when sending a token to its own contract address (#10546) Fixes MetaMask/metamask-extension#9437 --- app/_locales/en/messages.json | 3 ++ ui/app/helpers/utils/util.js | 7 ++++ ui/app/helpers/utils/util.test.js | 32 +++++++++++++++++ .../add-recipient/add-recipient.js | 6 +++- .../tests/add-recipient-utils.test.js | 13 +++++++ .../send-content/send-content.component.js | 15 +++++++- ui/app/pages/send/send.component.js | 34 ++++++++++++++++--- ui/app/pages/send/send.constants.js | 2 ++ ui/app/pages/send/send.container.js | 2 ++ 9 files changed, 108 insertions(+), 6 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 99789f491..369d72469 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -405,6 +405,9 @@ "continueToWyre": { "message": "Continue to Wyre" }, + "contractAddressError": { + "message": "You are sending tokens to the token's contract address. This may result in the loss of these tokens." + }, "contractDeployment": { "message": "Contract Deployment" }, diff --git a/ui/app/helpers/utils/util.js b/ui/app/helpers/utils/util.js index a7c06c052..1480781c0 100644 --- a/ui/app/helpers/utils/util.js +++ b/ui/app/helpers/utils/util.js @@ -108,6 +108,13 @@ export function isValidDomainName(address) { return match !== null; } +export function isOriginContractAddress(to, sendTokenAddress) { + if (!to || !sendTokenAddress) { + return false; + } + return to.toLowerCase() === sendTokenAddress.toLowerCase(); +} + export function isAllOneCase(address) { if (!address) { return true; diff --git a/ui/app/helpers/utils/util.test.js b/ui/app/helpers/utils/util.test.js index 9f9c7ba54..fc79ef3b0 100644 --- a/ui/app/helpers/utils/util.test.js +++ b/ui/app/helpers/utils/util.test.js @@ -152,6 +152,38 @@ describe('util', function () { }); }); + describe('isOriginContractAddress', function () { + it('should return true when the send address is the same as the selected tokens contract address', function () { + assert.equal( + util.isOriginContractAddress( + '0x8d6b81208414189a58339873ab429b6c47ab92d3', + '0x8d6b81208414189a58339873ab429b6c47ab92d3', + ), + true, + ); + }); + + it('should return true when the send address is the same as the selected tokens contract address, capitalized input', function () { + assert.equal( + util.isOriginContractAddress( + '0x8d6b81208414189a58339873ab429b6c47ab92d3', + '0X8D6B81208414189A58339873AB429B6C47AB92D3', + ), + true, + ); + }); + + it('should return false when the recipient address differs', function () { + assert.equal( + util.isOriginContractAddress( + '0x8d6b81208414189a58339873ab429b6c47ab92d3', + '0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B', + ), + false, + ); + }); + }); + describe('#numericBalance', function () { it('should return a BN 0 if given nothing', function () { const result = util.numericBalance(); diff --git a/ui/app/pages/send/send-content/add-recipient/add-recipient.js b/ui/app/pages/send/send-content/add-recipient/add-recipient.js index 317d02b88..c735957cf 100644 --- a/ui/app/pages/send/send-content/add-recipient/add-recipient.js +++ b/ui/app/pages/send/send-content/add-recipient/add-recipient.js @@ -7,6 +7,7 @@ import { KNOWN_RECIPIENT_ADDRESS_ERROR, INVALID_RECIPIENT_ADDRESS_NOT_ETH_NETWORK_ERROR, CONFUSING_ENS_ERROR, + CONTRACT_ADDRESS_ERROR, } from '../../send.constants'; import { @@ -14,9 +15,10 @@ import { isEthNetwork, checkExistingAddresses, isValidDomainName, + isOriginContractAddress, } from '../../../../helpers/utils/util'; -export function getToErrorObject(to, network) { +export function getToErrorObject(to, sendTokenAddress, network) { let toError = null; if (!to) { toError = REQUIRED_ERROR; @@ -24,6 +26,8 @@ export function getToErrorObject(to, network) { toError = isEthNetwork(network) ? INVALID_RECIPIENT_ADDRESS_ERROR : INVALID_RECIPIENT_ADDRESS_NOT_ETH_NETWORK_ERROR; + } else if (isOriginContractAddress(to, sendTokenAddress)) { + toError = CONTRACT_ADDRESS_ERROR; } return { to: toError }; diff --git a/ui/app/pages/send/send-content/add-recipient/tests/add-recipient-utils.test.js b/ui/app/pages/send/send-content/add-recipient/tests/add-recipient-utils.test.js index d93cd3fe8..3ffdc96af 100644 --- a/ui/app/pages/send/send-content/add-recipient/tests/add-recipient-utils.test.js +++ b/ui/app/pages/send/send-content/add-recipient/tests/add-recipient-utils.test.js @@ -7,6 +7,7 @@ import { INVALID_RECIPIENT_ADDRESS_ERROR, KNOWN_RECIPIENT_ADDRESS_ERROR, CONFUSING_ENS_ERROR, + CONTRACT_ADDRESS_ERROR, } from '../../../send.constants'; const stubs = { @@ -41,6 +42,18 @@ describe('add-recipient utils', function () { to: null, }); }); + + it('should return a contract address error if the recipient is the same as the tokens contract address', function () { + assert.deepStrictEqual(getToErrorObject('0xabc123', '0xabc123'), { + to: CONTRACT_ADDRESS_ERROR, + }); + }); + + it('should return null if the recipient address is not the token contract address', function () { + assert.deepStrictEqual(getToErrorObject('0xabc123', '0xabc456'), { + to: null, + }); + }); }); describe('getToWarningObject()', function () { diff --git a/ui/app/pages/send/send-content/send-content.component.js b/ui/app/pages/send/send-content/send-content.component.js index 2dee77e65..7b38d14ae 100644 --- a/ui/app/pages/send/send-content/send-content.component.js +++ b/ui/app/pages/send/send-content/send-content.component.js @@ -19,15 +19,17 @@ export default class SendContent extends Component { contact: PropTypes.object, isOwnedAccount: PropTypes.bool, warning: PropTypes.string, + error: PropTypes.string, }; updateGas = (updateData) => this.props.updateGas(updateData); render() { - const { warning } = this.props; + const { warning, error } = this.props; return (
+ {error && this.renderError()} {warning && this.renderWarning()} {this.maybeRenderAddContact()} @@ -74,4 +76,15 @@ export default class SendContent extends Component { ); } + + renderError() { + const { t } = this.context; + const { error } = this.props; + + return ( + + {t(error)} + + ); + } } diff --git a/ui/app/pages/send/send.component.js b/ui/app/pages/send/send.component.js index ffd305287..9a366d3da 100644 --- a/ui/app/pages/send/send.component.js +++ b/ui/app/pages/send/send.component.js @@ -17,7 +17,11 @@ import AddRecipient from './send-content/add-recipient'; import SendContent from './send-content'; import SendFooter from './send-footer'; import EnsInput from './send-content/add-recipient/ens-input'; -import { INVALID_RECIPIENT_ADDRESS_ERROR } from './send.constants'; +import { + INVALID_RECIPIENT_ADDRESS_ERROR, + KNOWN_RECIPIENT_ADDRESS_ERROR, + CONTRACT_ADDRESS_ERROR, +} from './send.constants'; export default class SendTransactionScreen extends Component { static propTypes = { @@ -53,6 +57,7 @@ export default class SendTransactionScreen extends Component { scanQrCode: PropTypes.func.isRequired, qrCodeDetected: PropTypes.func.isRequired, qrCodeData: PropTypes.object, + sendTokenAddress: PropTypes.string, }; static contextTypes = { @@ -93,6 +98,7 @@ export default class SendTransactionScreen extends Component { qrCodeData, qrCodeDetected, } = this.props; + const { toError, toWarning } = this.state; let updateGas = false; const { @@ -187,6 +193,25 @@ export default class SendTransactionScreen extends Component { this.updateGas(); } } + + // If selecting ETH after selecting a token, clear token related messages. + if (prevSendToken && !sendToken) { + let error = toError; + let warning = toWarning; + + if (toError === CONTRACT_ADDRESS_ERROR) { + error = null; + } + + if (toWarning === KNOWN_RECIPIENT_ADDRESS_ERROR) { + warning = null; + } + + this.setState({ + toError: error, + toWarning: warning, + }); + } } componentDidMount() { @@ -233,7 +258,7 @@ export default class SendTransactionScreen extends Component { } validate(query) { - const { tokens, sendToken, network } = this.props; + const { tokens, sendToken, network, sendTokenAddress } = this.props; const { internalSearch } = this.state; @@ -242,7 +267,7 @@ export default class SendTransactionScreen extends Component { return; } - const toErrorObject = getToErrorObject(query, network); + const toErrorObject = getToErrorObject(query, sendTokenAddress, network); const toWarningObject = getToWarningObject(query, tokens, sendToken); this.setState({ @@ -358,7 +383,7 @@ export default class SendTransactionScreen extends Component { renderSendContent() { const { history, showHexData } = this.props; - const { toWarning } = this.state; + const { toWarning, toError } = this.state; return [ , , ]; diff --git a/ui/app/pages/send/send.constants.js b/ui/app/pages/send/send.constants.js index 5584a6c99..93b620d14 100644 --- a/ui/app/pages/send/send.constants.js +++ b/ui/app/pages/send/send.constants.js @@ -35,6 +35,7 @@ const INVALID_RECIPIENT_ADDRESS_NOT_ETH_NETWORK_ERROR = 'invalidAddressRecipientNotEthNetwork'; const REQUIRED_ERROR = 'required'; const KNOWN_RECIPIENT_ADDRESS_ERROR = 'knownAddressRecipient'; +const CONTRACT_ADDRESS_ERROR = 'contractAddressError'; const CONFUSING_ENS_ERROR = 'confusingEnsDomain'; const SIMPLE_GAS_COST = '0x5208'; // Hex for 21000, cost of a simple send. @@ -45,6 +46,7 @@ export { INSUFFICIENT_TOKENS_ERROR, INVALID_RECIPIENT_ADDRESS_ERROR, KNOWN_RECIPIENT_ADDRESS_ERROR, + CONTRACT_ADDRESS_ERROR, INVALID_RECIPIENT_ADDRESS_NOT_ETH_NETWORK_ERROR, MIN_GAS_LIMIT_DEC, MIN_GAS_LIMIT_HEX, diff --git a/ui/app/pages/send/send.container.js b/ui/app/pages/send/send.container.js index c460b4526..3f0ddb0a0 100644 --- a/ui/app/pages/send/send.container.js +++ b/ui/app/pages/send/send.container.js @@ -22,6 +22,7 @@ import { getQrCodeData, getSelectedAddress, getAddressBook, + getSendTokenAddress, } from '../../selectors'; import { @@ -65,6 +66,7 @@ function mapStateToProps(state) { tokens: getTokens(state), tokenBalance: getTokenBalance(state), tokenContract: getSendTokenContract(state), + sendTokenAddress: getSendTokenAddress(state), }; }