From 3bd5d714f71668f539ce98e31b08f0f57c20a969 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 28 Jan 2020 22:49:32 -0400 Subject: [PATCH] Allow editing max spend limit (#7919) In the case where the initial spend limit for the `approve` function was set to the maximum amount, editing this value would result in the new limit being silently ignored. The transaction would be submitted with the original max spend limit. This occurred because function to generate the new custom data would look for the expected spend limit in the existing data, then bail if it was not found (and in these cases, it was never found). The reason the value was not found is that it was erroneously being converted to a `Number`. A JavaScript `Number` is not precise enough to represent larger spend limits, so it would give the wrong hex value (after rounding had taken place in the conversion to a floating-point number). The data string is now updated without relying upon the original token value; the new value is inserted after the `spender` argument instead, as the remainder of the `data` string is guaranteed to be the original limit. Additionally, the conversion to a `Number` is now omitted so that the custom spend limit is encoded correctly. Fixes #7915 --- .../edit-approval-permission.component.js | 9 ++-- .../confirm-approve-content.component.js | 2 +- .../confirm-approve.component.js | 12 ++--- .../confirm-approve.container.js | 2 +- .../confirm-approve/confirm-approve.util.js | 49 ++++++++++--------- 5 files changed, 40 insertions(+), 34 deletions(-) diff --git a/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js b/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js index 53ff473e4..28f9fb1f9 100644 --- a/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js +++ b/ui/app/components/app/modals/edit-approval-permission/edit-approval-permission.component.js @@ -4,6 +4,7 @@ import Modal from '../../modal' import Identicon from '../../../ui/identicon' import TextField from '../../../ui/text-field' import classnames from 'classnames' +import BigNumber from 'bignumber.js' export default class EditApprovalPermission extends PureComponent { static propTypes = { @@ -61,7 +62,7 @@ export default class EditApprovalPermission extends PureComponent {
{ t('balance') }
- {`${tokenBalance} ${tokenSymbol}`} + {`${Number(tokenBalance).toPrecision(9)} ${tokenSymbol}`}
@@ -89,7 +90,7 @@ export default class EditApprovalPermission extends PureComponent { 'edit-approval-permission__edit-section__option-label--selected': selectedOptionIsUnlimited, })}> { - tokenAmount < tokenBalance + (new BigNumber(tokenAmount)).lessThan(new BigNumber(tokenBalance)) ? t('proposedApprovalLimit') : t('unlimited') } @@ -98,7 +99,7 @@ export default class EditApprovalPermission extends PureComponent { { t('spendLimitRequestedBy', [origin]) }
- {`${tokenAmount} ${tokenSymbol}`} + {`${Number(tokenAmount)} ${tokenSymbol}`}
@@ -128,7 +129,7 @@ export default class EditApprovalPermission extends PureComponent { { this.setState({ customSpendLimit: event.target.value }) if (selectedOptionIsUnlimited) { diff --git a/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js b/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js index 38644541d..d1acc6681 100644 --- a/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js +++ b/ui/app/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js @@ -100,7 +100,7 @@ export default class ConfirmApproveContent extends Component {
{ t('accessAndSpendNotice', [origin]) }
{ t('amountWithColon') }
-
{ `${customTokenAmount || tokenAmount} ${tokenSymbol}` }
+
{ `${Number(customTokenAmount || tokenAmount)} ${tokenSymbol}` }
{ t('toWithColon') }
diff --git a/ui/app/pages/confirm-approve/confirm-approve.component.js b/ui/app/pages/confirm-approve/confirm-approve.component.js index 3408a6ab8..2e02f6ba3 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.component.js +++ b/ui/app/pages/confirm-approve/confirm-approve.component.js @@ -15,7 +15,7 @@ export default class ConfirmApprove extends Component { static propTypes = { tokenAddress: PropTypes.string, toAddress: PropTypes.string, - tokenAmount: PropTypes.number, + tokenAmount: PropTypes.string, tokenSymbol: PropTypes.string, fiatTransactionTotal: PropTypes.string, ethTransactionTotal: PropTypes.string, @@ -33,7 +33,7 @@ export default class ConfirmApprove extends Component { } static defaultProps = { - tokenAmount: 0, + tokenAmount: '0', } state = { @@ -69,14 +69,14 @@ export default class ConfirmApprove extends Component { } = this.props const { customPermissionAmount } = this.state - const tokensText = `${tokenAmount} ${tokenSymbol}` + const tokensText = `${Number(tokenAmount)} ${tokenSymbol}` const tokenBalance = tokenTrackerBalance - ? Number(calcTokenAmount(tokenTrackerBalance, decimals)).toPrecision(9) + ? calcTokenAmount(tokenTrackerBalance, decimals).toString(10) : '' const customData = customPermissionAmount - ? getCustomTxParamsData(data, { customPermissionAmount, tokenAmount, decimals }) + ? getCustomTxParamsData(data, { customPermissionAmount, decimals }) : null return ( @@ -92,7 +92,7 @@ export default class ConfirmApprove extends Component { this.setState({ customPermissionAmount: newAmount }) }} customTokenAmount={String(customPermissionAmount)} - tokenAmount={String(tokenAmount)} + tokenAmount={tokenAmount} origin={origin} tokenSymbol={tokenSymbol} tokenBalance={tokenBalance} diff --git a/ui/app/pages/confirm-approve/confirm-approve.container.js b/ui/app/pages/confirm-approve/confirm-approve.container.js index 43f5aab90..5a3223564 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.container.js +++ b/ui/app/pages/confirm-approve/confirm-approve.container.js @@ -43,7 +43,7 @@ const mapStateToProps = (state, ownProps) => { const tokenData = getTokenData(data) const tokenValue = tokenData && getTokenValue(tokenData.params) const toAddress = tokenData && getTokenToAddress(tokenData.params) - const tokenAmount = tokenData && calcTokenAmount(tokenValue, decimals).toNumber() + const tokenAmount = tokenData && calcTokenAmount(tokenValue, decimals).toString(10) const contractExchangeRate = contractExchangeRateSelector(state) const { origin } = transaction diff --git a/ui/app/pages/confirm-approve/confirm-approve.util.js b/ui/app/pages/confirm-approve/confirm-approve.util.js index be77c65f9..0318c6bed 100644 --- a/ui/app/pages/confirm-approve/confirm-approve.util.js +++ b/ui/app/pages/confirm-approve/confirm-approve.util.js @@ -1,28 +1,33 @@ import { decimalToHex } from '../../helpers/utils/conversions.util' import { calcTokenValue } from '../../helpers/utils/token-util.js' +import { getTokenData } from '../../helpers/utils/transactions.util' -export function getCustomTxParamsData (data, { customPermissionAmount, tokenAmount, decimals }) { - if (customPermissionAmount) { - const tokenValue = decimalToHex(calcTokenValue(tokenAmount, decimals)) +export function getCustomTxParamsData (data, { customPermissionAmount, decimals }) { + const tokenData = getTokenData(data) - const re = new RegExp('(^.+)' + tokenValue + '$') - const matches = re.exec(data) - - if (!matches || !matches[1]) { - return data - } - let dataWithoutCurrentAmount = matches[1] - const customPermissionValue = decimalToHex(calcTokenValue(Number(customPermissionAmount), decimals)) - - const differenceInLengths = customPermissionValue.length - tokenValue.length - const zeroModifier = dataWithoutCurrentAmount.length - differenceInLengths - if (differenceInLengths > 0) { - dataWithoutCurrentAmount = dataWithoutCurrentAmount.slice(0, zeroModifier) - } else if (differenceInLengths < 0) { - dataWithoutCurrentAmount = dataWithoutCurrentAmount.padEnd(zeroModifier, 0) - } - - const customTxParamsData = dataWithoutCurrentAmount + customPermissionValue - return customTxParamsData + if (!tokenData) { + throw new Error('Invalid data') + } else if (tokenData.name !== 'approve') { + throw new Error(`Invalid data; should be 'approve' method, but instead is '${tokenData.name}'`) } + let spender = tokenData.params[0].value + if (spender.startsWith('0x')) { + spender = spender.substring(2) + } + const [signature, tokenValue] = data.split(spender) + + if (!signature || !tokenValue) { + throw new Error('Invalid data') + } else if (tokenValue.length !== 64) { + throw new Error('Invalid token value; should be exactly 64 hex digits long (u256)') + } + + let customPermissionValue = decimalToHex(calcTokenValue(customPermissionAmount, decimals)) + if (customPermissionValue.length > 64) { + throw new Error('Custom value is larger than u256') + } + + customPermissionValue = customPermissionValue.padStart(tokenValue.length, '0') + const customTxParamsData = `${signature}${spender}${customPermissionValue}` + return customTxParamsData }