1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-23 09:52:26 +01:00

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
This commit is contained in:
Mark Stacey 2020-01-28 22:49:32 -04:00
parent acf1f704ea
commit 3bd5d714f7
5 changed files with 40 additions and 34 deletions

View File

@ -4,6 +4,7 @@ import Modal from '../../modal'
import Identicon from '../../../ui/identicon' import Identicon from '../../../ui/identicon'
import TextField from '../../../ui/text-field' import TextField from '../../../ui/text-field'
import classnames from 'classnames' import classnames from 'classnames'
import BigNumber from 'bignumber.js'
export default class EditApprovalPermission extends PureComponent { export default class EditApprovalPermission extends PureComponent {
static propTypes = { static propTypes = {
@ -61,7 +62,7 @@ export default class EditApprovalPermission extends PureComponent {
<div>{ t('balance') }</div> <div>{ t('balance') }</div>
</div> </div>
<div className="edit-approval-permission__account-info__balance"> <div className="edit-approval-permission__account-info__balance">
{`${tokenBalance} ${tokenSymbol}`} {`${Number(tokenBalance).toPrecision(9)} ${tokenSymbol}`}
</div> </div>
</div> </div>
<div className="edit-approval-permission__edit-section"> <div className="edit-approval-permission__edit-section">
@ -89,7 +90,7 @@ export default class EditApprovalPermission extends PureComponent {
'edit-approval-permission__edit-section__option-label--selected': selectedOptionIsUnlimited, 'edit-approval-permission__edit-section__option-label--selected': selectedOptionIsUnlimited,
})}> })}>
{ {
tokenAmount < tokenBalance (new BigNumber(tokenAmount)).lessThan(new BigNumber(tokenBalance))
? t('proposedApprovalLimit') ? t('proposedApprovalLimit')
: t('unlimited') : t('unlimited')
} }
@ -98,7 +99,7 @@ export default class EditApprovalPermission extends PureComponent {
{ t('spendLimitRequestedBy', [origin]) } { t('spendLimitRequestedBy', [origin]) }
</div> </div>
<div className="edit-approval-permission__edit-section__option-value" > <div className="edit-approval-permission__edit-section__option-value" >
{`${tokenAmount} ${tokenSymbol}`} {`${Number(tokenAmount)} ${tokenSymbol}`}
</div> </div>
</div> </div>
</div> </div>
@ -128,7 +129,7 @@ export default class EditApprovalPermission extends PureComponent {
<TextField <TextField
type="number" type="number"
min="0" min="0"
placeholder={ `${customTokenAmount || tokenAmount} ${tokenSymbol}` } placeholder={ `${Number(customTokenAmount || tokenAmount)} ${tokenSymbol}` }
onChange={(event) => { onChange={(event) => {
this.setState({ customSpendLimit: event.target.value }) this.setState({ customSpendLimit: event.target.value })
if (selectedOptionIsUnlimited) { if (selectedOptionIsUnlimited) {

View File

@ -100,7 +100,7 @@ export default class ConfirmApproveContent extends Component {
<div className="confirm-approve-content__small-text">{ t('accessAndSpendNotice', [origin]) }</div> <div className="confirm-approve-content__small-text">{ t('accessAndSpendNotice', [origin]) }</div>
<div className="flex-row"> <div className="flex-row">
<div className="confirm-approve-content__label">{ t('amountWithColon') }</div> <div className="confirm-approve-content__label">{ t('amountWithColon') }</div>
<div className="confirm-approve-content__medium-text">{ `${customTokenAmount || tokenAmount} ${tokenSymbol}` }</div> <div className="confirm-approve-content__medium-text">{ `${Number(customTokenAmount || tokenAmount)} ${tokenSymbol}` }</div>
</div> </div>
<div className="flex-row"> <div className="flex-row">
<div className="confirm-approve-content__label">{ t('toWithColon') }</div> <div className="confirm-approve-content__label">{ t('toWithColon') }</div>

View File

@ -15,7 +15,7 @@ export default class ConfirmApprove extends Component {
static propTypes = { static propTypes = {
tokenAddress: PropTypes.string, tokenAddress: PropTypes.string,
toAddress: PropTypes.string, toAddress: PropTypes.string,
tokenAmount: PropTypes.number, tokenAmount: PropTypes.string,
tokenSymbol: PropTypes.string, tokenSymbol: PropTypes.string,
fiatTransactionTotal: PropTypes.string, fiatTransactionTotal: PropTypes.string,
ethTransactionTotal: PropTypes.string, ethTransactionTotal: PropTypes.string,
@ -33,7 +33,7 @@ export default class ConfirmApprove extends Component {
} }
static defaultProps = { static defaultProps = {
tokenAmount: 0, tokenAmount: '0',
} }
state = { state = {
@ -69,14 +69,14 @@ export default class ConfirmApprove extends Component {
} = this.props } = this.props
const { customPermissionAmount } = this.state const { customPermissionAmount } = this.state
const tokensText = `${tokenAmount} ${tokenSymbol}` const tokensText = `${Number(tokenAmount)} ${tokenSymbol}`
const tokenBalance = tokenTrackerBalance const tokenBalance = tokenTrackerBalance
? Number(calcTokenAmount(tokenTrackerBalance, decimals)).toPrecision(9) ? calcTokenAmount(tokenTrackerBalance, decimals).toString(10)
: '' : ''
const customData = customPermissionAmount const customData = customPermissionAmount
? getCustomTxParamsData(data, { customPermissionAmount, tokenAmount, decimals }) ? getCustomTxParamsData(data, { customPermissionAmount, decimals })
: null : null
return ( return (
@ -92,7 +92,7 @@ export default class ConfirmApprove extends Component {
this.setState({ customPermissionAmount: newAmount }) this.setState({ customPermissionAmount: newAmount })
}} }}
customTokenAmount={String(customPermissionAmount)} customTokenAmount={String(customPermissionAmount)}
tokenAmount={String(tokenAmount)} tokenAmount={tokenAmount}
origin={origin} origin={origin}
tokenSymbol={tokenSymbol} tokenSymbol={tokenSymbol}
tokenBalance={tokenBalance} tokenBalance={tokenBalance}

View File

@ -43,7 +43,7 @@ const mapStateToProps = (state, ownProps) => {
const tokenData = getTokenData(data) const tokenData = getTokenData(data)
const tokenValue = tokenData && getTokenValue(tokenData.params) const tokenValue = tokenData && getTokenValue(tokenData.params)
const toAddress = tokenData && getTokenToAddress(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 contractExchangeRate = contractExchangeRateSelector(state)
const { origin } = transaction const { origin } = transaction

View File

@ -1,28 +1,33 @@
import { decimalToHex } from '../../helpers/utils/conversions.util' import { decimalToHex } from '../../helpers/utils/conversions.util'
import { calcTokenValue } from '../../helpers/utils/token-util.js' import { calcTokenValue } from '../../helpers/utils/token-util.js'
import { getTokenData } from '../../helpers/utils/transactions.util'
export function getCustomTxParamsData (data, { customPermissionAmount, tokenAmount, decimals }) { export function getCustomTxParamsData (data, { customPermissionAmount, decimals }) {
if (customPermissionAmount) { const tokenData = getTokenData(data)
const tokenValue = decimalToHex(calcTokenValue(tokenAmount, decimals))
const re = new RegExp('(^.+)' + tokenValue + '$') if (!tokenData) {
const matches = re.exec(data) throw new Error('Invalid data')
} else if (tokenData.name !== 'approve') {
if (!matches || !matches[1]) { throw new Error(`Invalid data; should be 'approve' method, but instead is '${tokenData.name}'`)
return data
} }
let dataWithoutCurrentAmount = matches[1] let spender = tokenData.params[0].value
const customPermissionValue = decimalToHex(calcTokenValue(Number(customPermissionAmount), decimals)) if (spender.startsWith('0x')) {
spender = spender.substring(2)
}
const [signature, tokenValue] = data.split(spender)
const differenceInLengths = customPermissionValue.length - tokenValue.length if (!signature || !tokenValue) {
const zeroModifier = dataWithoutCurrentAmount.length - differenceInLengths throw new Error('Invalid data')
if (differenceInLengths > 0) { } else if (tokenValue.length !== 64) {
dataWithoutCurrentAmount = dataWithoutCurrentAmount.slice(0, zeroModifier) throw new Error('Invalid token value; should be exactly 64 hex digits long (u256)')
} else if (differenceInLengths < 0) {
dataWithoutCurrentAmount = dataWithoutCurrentAmount.padEnd(zeroModifier, 0)
} }
const customTxParamsData = dataWithoutCurrentAmount + customPermissionValue 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 return customTxParamsData
} }
}