Skip to content

Commit

Permalink
Add advanced setting to enable editing nonce on confirmation screens (M…
Browse files Browse the repository at this point in the history
…etaMask#7089)

* Add UseNonce toggle

* Get the toggle actually working and dispatching

* Display nonce field on confirmation page

* Remove console.log

* Add placeholder

* Set customNonceValue

* Add nonce key/value to txParams

* remove customNonceValue from component state

* Use translation file and existing CSS class

* Use existing TextField component

* Remove console.log

* Fix lint nits

* Okay this sorta works?

* Move nonce toggle to advanced tab

* Set min to 0

* Wrap value in Number()

* Add customNonceMap

* Update custom nonce translation

* Update styles

* Reset CustomNonce

* Fix lint

* Get tests passing

* Add customNonceValue to defaults

* Fix test

* Fix comments

* Update tests

* Use camel case

* Ensure custom nonce can only be whole number

* Correct font size for custom nonce input

* UX improvements for custom nonce feature

* Fix advanced-tab-component tests for custom nonce changes

* Update title of nonce toggle in settings

* Remove unused locale message

* Cast custom nonce to string in confirm-transaction-base.component

* Handle string conversion and invalid values for custom nonces in handler

* Don't call getNonceLock in tx controller if there is a custom nonce

* Set nonce details for cases where nonce is customized

* Fix incorrectly use value for deciding whether to getnoncelock in approveTransaction

* Default nonceLock to empty object in approveTransaction

* Reapply use on nonceLock in cases where customNonceValue in approveTransaction.

* Show warning message if custom nonce is higher than MetaMask's next nonce

* Fix e2e test failure caused by custom nonce and 3box toggle conflict

* Update nonce warning message to include the suggested nonce

* Handle nextNonce comparison and update logic in lifecycle

* Default nonce field to suggested nonce

* Clear custom nonce on reject or confirm

* Fix bug where nonces are not shown in tx list on self sent transactions

* Ensure custom nonce is reset after tx is created in background

* Convert customNonceValue to number in approve tranasction controller

* Lint fix

* Call getNextNonce after updating custom nonce
  • Loading branch information
rickycodes authored and danjm committed Sep 27, 2019
1 parent 970e90e commit 5f254f7
Show file tree
Hide file tree
Showing 17 changed files with 319 additions and 18 deletions.
16 changes: 16 additions & 0 deletions app/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,18 @@
"blockiesIdenticon": {
"message": "Use Blockies Identicon"
},
"nonceField": {
"message": "Customize transaction nonce"
},
"nonceFieldPlaceholder": {
"message": "Automatically calculate"
},
"nonceFieldHeading": {
"message": "Custom Nonce"
},
"nonceFieldDescription": {
"message": "Turn this on to change the nonce (transaction number) on confirmation screens. This is an advanced feature, use cautiously."
},
"browserNotSupported": {
"message": "Your Browser is not supported..."
},
Expand Down Expand Up @@ -851,6 +863,10 @@
"next": {
"message": "Next"
},
"nextNonceWarning": {
"message": "Nonce is higher than suggested nonce of $1",
"description": "The next nonce according to MetaMask's internal logic"
},
"noAddressForName": {
"message": "No address has been set for this name."
},
Expand Down
22 changes: 22 additions & 0 deletions app/scripts/controllers/preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class PreferencesController {
* @property {object} store.accountTokens The tokens stored per account and then per network type
* @property {object} store.assetImages Contains assets objects related to assets added
* @property {boolean} store.useBlockie The users preference for blockie identicons within the UI
* @property {boolean} store.useNonceField The users preference for nonce field within the UI
* @property {object} store.featureFlags A key-boolean map, where keys refer to features and booleans to whether the
* user wishes to see that feature.
*
Expand All @@ -35,6 +36,7 @@ class PreferencesController {
tokens: [],
suggestedTokens: {},
useBlockie: false,
useNonceField: false,

// WARNING: Do not use feature flags for security-sensitive things.
// Feature flag toggling is available in the global namespace
Expand Down Expand Up @@ -89,6 +91,16 @@ class PreferencesController {
this.store.updateState({ useBlockie: val })
}

/**
* Setter for the `useNonceField` property
*
* @param {boolean} val Whether or not the user prefers to set nonce
*
*/
setUseNonceField (val) {
this.store.updateState({ useNonceField: val })
}

/**
* Setter for the `participateInMetaMetrics` property
*
Expand Down Expand Up @@ -204,6 +216,16 @@ class PreferencesController {
return this.store.getState().useBlockie
}

/**
* Getter for the `getUseNonceField` property
*
* @returns {boolean} this.store.getUseNonceField
*
*/
getUseNonceField () {
return this.store.getState().useNonceField
}

/**
* Setter for the `currentLocale` property
*
Expand Down
9 changes: 8 additions & 1 deletion app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,21 @@ class TransactionController extends EventEmitter {
const txMeta = this.txStateManager.getTx(txId)
const fromAddress = txMeta.txParams.from
// wait for a nonce
let { customNonceValue = null } = txMeta
customNonceValue = Number(customNonceValue)
nonceLock = await this.nonceTracker.getNonceLock(fromAddress)
// add nonce to txParams
// if txMeta has lastGasPrice then it is a retry at same nonce with higher
// gas price transaction and their for the nonce should not be calculated
const nonce = txMeta.lastGasPrice ? txMeta.txParams.nonce : nonceLock.nextNonce
txMeta.txParams.nonce = ethUtil.addHexPrefix(nonce.toString(16))
const customOrNonce = customNonceValue || nonce

txMeta.txParams.nonce = ethUtil.addHexPrefix(customOrNonce.toString(16))
// add nonce debugging information to txMeta
txMeta.nonceDetails = nonceLock.nonceDetails
if (customNonceValue) {
txMeta.nonceDetails.customNonceValue = customNonceValue
}
this.txStateManager.updateTx(txMeta, 'transactions#approveTransaction')
// sign transaction
const rawTx = await this.signTransaction(txId)
Expand Down
16 changes: 16 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ module.exports = class MetamaskController extends EventEmitter {
getState: (cb) => cb(null, this.getState()),
setCurrentCurrency: this.setCurrentCurrency.bind(this),
setUseBlockie: this.setUseBlockie.bind(this),
setUseNonceField: this.setUseNonceField.bind(this),
setParticipateInMetaMetrics: this.setParticipateInMetaMetrics.bind(this),
setMetaMetricsSendCount: this.setMetaMetricsSendCount.bind(this),
setFirstTimeFlowType: this.setFirstTimeFlowType.bind(this),
Expand Down Expand Up @@ -519,6 +520,7 @@ module.exports = class MetamaskController extends EventEmitter {
getFilteredTxList: nodeify(txController.getFilteredTxList, txController),
isNonceTaken: nodeify(txController.isNonceTaken, txController),
estimateGas: nodeify(this.estimateGas, this),
getPendingNonce: nodeify(this.getPendingNonce, this),

// messageManager
signMessage: nodeify(this.signMessage, this),
Expand Down Expand Up @@ -1727,6 +1729,20 @@ module.exports = class MetamaskController extends EventEmitter {
}
}

/**
* Sets whether or not to use the nonce field.
* @param {boolean} val - True for nonce field, false for not nonce field.
* @param {Function} cb - A callback function called when complete.
*/
setUseNonceField (val, cb) {
try {
this.preferencesController.setUseNonceField(val)
cb(null)
} catch (err) {
cb(err)
}
}

/**
* Sets whether or not the user will have usage data tracked with MetaMetrics
* @param {boolean} bool - True for users that wish to opt-in, false for users that wish to remain out.
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/threebox.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('MetaMask', function () {
await advancedButton.click()

const threeBoxToggle = await findElements(driver, By.css('.toggle-button'))
const threeBoxToggleButton = await threeBoxToggle[3].findElement(By.css('div'))
const threeBoxToggleButton = await threeBoxToggle[4].findElement(By.css('div'))
await threeBoxToggleButton.click()
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,11 @@
.advanced-gas-inputs__gas-edit-rows {
margin-bottom: 16px;
}

.custom-nonce-input {
input {
width: 90px;
font-size: 1rem;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ const mapStateToProps = (state, ownProps) => {
const { showFiatInTestnets } = preferencesSelector(state)
const isMainnet = getIsMainnet(state)
const { transactionGroup: { primaryTransaction } = {} } = ownProps
const { txParams: { gas: gasLimit, gasPrice, data, to } = {} } = primaryTransaction
const { txParams: { gas: gasLimit, gasPrice, data } = {}, transactionCategory } = primaryTransaction
const selectedAddress = getSelectedAddress(state)
const selectedAccountBalance = accounts[selectedAddress].balance
const isDeposit = selectedAddress === to
const isDeposit = transactionCategory === 'incoming'
const selectRpcInfo = frequentRpcListDetail.find(rpcInfo => rpcInfo.rpcUrl === provider.rpcTarget)
const { rpcPrefs } = selectRpcInfo || {}

Expand Down
14 changes: 13 additions & 1 deletion ui/app/ducks/metamask/metamask.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ function reduceMetamask (state, action) {
tokenExchangeRates: {},
tokens: [],
pendingTokens: {},
customNonceValue: '',
send: {
gasLimit: null,
gasPrice: null,
Expand Down Expand Up @@ -57,6 +58,7 @@ function reduceMetamask (state, action) {
knownMethodData: {},
participateInMetaMetrics: null,
metaMetricsSendCount: 0,
nextNonce: null,
}, state.metamask)

switch (action.type) {
Expand Down Expand Up @@ -188,7 +190,10 @@ function reduceMetamask (state, action) {
gasLimit: action.value,
},
})

case actions.UPDATE_CUSTOM_NONCE:
return extend(metamaskState, {
customNonceValue: action.value,
})
case actions.UPDATE_GAS_PRICE:
return extend(metamaskState, {
send: {
Expand Down Expand Up @@ -412,6 +417,13 @@ function reduceMetamask (state, action) {
})
}

case actions.SET_NEXT_NONCE: {
console.log('action.value', action.value)
return extend(metamaskState, {
nextNonce: action.value,
})
}

default:
return metamaskState

Expand Down
Loading

0 comments on commit 5f254f7

Please sign in to comment.