From 21fbaed97c84e75968cff1810999d0ec1aa5ef26 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 27 Mar 2018 23:55:18 -0700 Subject: tx controller - explode on non-hex txParams + dont add chainId to txParams + sign with chainId as number --- app/scripts/controllers/transactions.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 3e3909361..7e2cc15da 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -250,7 +250,7 @@ module.exports = class TransactionController extends EventEmitter { // wait for a nonce nonceLock = await this.nonceTracker.getNonceLock(fromAddress) // add nonce to txParams - // if txMeta has lastGasPrice then it is a retry at same nonce with higher + // 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)) @@ -273,12 +273,14 @@ module.exports = class TransactionController extends EventEmitter { async signTransaction (txId) { const txMeta = this.txStateManager.getTx(txId) - const txParams = txMeta.txParams - const fromAddress = txParams.from // add network/chain id - txParams.chainId = ethUtil.addHexPrefix(this.getChainId().toString(16)) + const chainId = this.getChainId() + const txParams = Object.assign({}, txMeta.txParams, { chainId }) + // sign tx + const fromAddress = txParams.from const ethTx = new Transaction(txParams) await this.signEthTx(ethTx, fromAddress) + // set state to signed this.txStateManager.setTxStatusSigned(txMeta.id) const rawTx = ethUtil.bufferToHex(ethTx.serialize()) return rawTx -- cgit v1.2.3 From 03b123a85d2ce693980b83eef7adb8939737bff8 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 30 Mar 2018 15:25:13 -0700 Subject: transactions - put the origing on the txMeta to help with debugging --- app/scripts/controllers/transactions.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 7e2cc15da..458bf912b 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -161,9 +161,11 @@ module.exports = class TransactionController extends EventEmitter { this.emit(`${txMeta.id}:unapproved`, txMeta) } - async newUnapprovedTransaction (txParams) { + async newUnapprovedTransaction (txParams, opts = {origin: 'metamask'}) { log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`) const initialTxMeta = await this.addUnapprovedTransaction(txParams) + initialTxMeta.origin = opts.origin + this.txStateManager.updateTx(initialTxMeta, '#newUnapprovedTransaction - adding the origin') // listen for tx completion (success, fail) return new Promise((resolve, reject) => { this.txStateManager.once(`${initialTxMeta.id}:finished`, (finishedTxMeta) => { -- cgit v1.2.3 From 3def45004a0024c4f48b06618f73f19ffa4ffd6b Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 30 Mar 2018 16:00:11 -0700 Subject: transactions#newUnapprovedTransaction - dont default origin to metamask --- app/scripts/controllers/transactions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 458bf912b..a18a2d2e2 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -161,7 +161,7 @@ module.exports = class TransactionController extends EventEmitter { this.emit(`${txMeta.id}:unapproved`, txMeta) } - async newUnapprovedTransaction (txParams, opts = {origin: 'metamask'}) { + async newUnapprovedTransaction (txParams, opts = {}) { log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`) const initialTxMeta = await this.addUnapprovedTransaction(txParams) initialTxMeta.origin = opts.origin -- cgit v1.2.3 From ab126b8c7894a0cfb8e728eeed48689200ed7a6c Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 2 Apr 2018 15:43:32 -0700 Subject: transactions gasLimit - use the block gasLimit if getCode fails --- app/scripts/controllers/transactions.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index a18a2d2e2..31e53554d 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -187,12 +187,12 @@ module.exports = class TransactionController extends EventEmitter { // validate await this.txGasUtil.validateTxParams(txParams) // construct txMeta - const txMeta = this.txStateManager.generateTxMeta({txParams}) + let txMeta = this.txStateManager.generateTxMeta({txParams}) this.addTx(txMeta) this.emit('newUnapprovedTx', txMeta) // add default tx params try { - await this.addTxDefaults(txMeta) + txMeta = await this.addTxDefaults(txMeta) } catch (error) { console.log(error) this.txStateManager.setTxStatusFailed(txMeta.id, error) @@ -215,6 +215,7 @@ module.exports = class TransactionController extends EventEmitter { } txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16)) txParams.value = txParams.value || '0x0' + if (txParams.to === null) delete txParams.to // set gasLimit return await this.txGasUtil.analyzeGasUsage(txMeta) } -- cgit v1.2.3 From 457a47bf62272deb257e3935a62e0ed265a49d78 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 4 Apr 2018 12:25:51 -0700 Subject: transactions - normalize txParams --- app/scripts/controllers/transactions.js | 54 +++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 31e53554d..9568fcbb9 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -185,7 +185,8 @@ module.exports = class TransactionController extends EventEmitter { async addUnapprovedTransaction (txParams) { // validate - await this.txGasUtil.validateTxParams(txParams) + await this._validateTxParams(txParams) + this._normalizeTxParams(txParams) // construct txMeta let txMeta = this.txStateManager.generateTxMeta({txParams}) this.addTx(txMeta) @@ -215,7 +216,6 @@ module.exports = class TransactionController extends EventEmitter { } txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16)) txParams.value = txParams.value || '0x0' - if (txParams.to === null) delete txParams.to // set gasLimit return await this.txGasUtil.analyzeGasUsage(txMeta) } @@ -314,6 +314,56 @@ module.exports = class TransactionController extends EventEmitter { // PRIVATE METHODS // + _normalizeTxParams (txParams) { + delete txParams.chainId + + if ( !txParams.to ) delete txParams.to + else txParams.to = ethUtil.addHexPrefix(txParams.to) + + txParams.from = ethUtil.addHexPrefix(txParams.from).toLowerCase() + + if (!txParams.data) delete txParams.data + else txParams.data = ethUtil.addHexPrefix(txParams.data) + + if (txParams.value) txParams.value = ethUtil.addHexPrefix(txParams.value) + + if (txParams.gas) txParams.gas = ethUtil.addHexPrefix(txParams.gas) + if (txParams.gasPrice) txParams.gas = ethUtil.addHexPrefix(txParams.gas) + } + + async _validateTxParams (txParams) { + this._validateFrom(txParams) + this._validateRecipient(txParams) + if ('value' in txParams) { + const value = txParams.value.toString() + if (value.includes('-')) { + throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) + } + + if (value.includes('.')) { + throw new Error(`Invalid transaction value of ${txParams.value} number must be in wei`) + } + } + } + + _validateFrom (txParams) { + if ( !(typeof txParams.from === 'string') ) throw new Error(`Invalid from address ${txParams.from} not a string`) + if (!ethUtil.isValidAddress(txParams.from)) throw new Error('Invalid from address') + } + + _validateRecipient (txParams) { + if (txParams.to === '0x' || txParams.to === null ) { + if (txParams.data) { + delete txParams.to + } else { + throw new Error('Invalid recipient address') + } + } else if ( txParams.to !== undefined && !ethUtil.isValidAddress(txParams.to) ) { + throw new Error('Invalid recipient address') + } + return txParams + } + _markNonceDuplicatesDropped (txId) { this.txStateManager.setTxStatusConfirmed(txId) // get the confirmed transactions nonce and from address -- cgit v1.2.3 From 245c01bc0fed585c4ac8ed05edf7ebe1a65de80b Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 4 Apr 2018 14:56:30 -0700 Subject: transactions - make #_validateTxParams not async and "linting" wink wink nudge nudge --- app/scripts/controllers/transactions.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 9568fcbb9..a73a8b36d 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -185,7 +185,7 @@ module.exports = class TransactionController extends EventEmitter { async addUnapprovedTransaction (txParams) { // validate - await this._validateTxParams(txParams) + this._validateTxParams(txParams) this._normalizeTxParams(txParams) // construct txMeta let txMeta = this.txStateManager.generateTxMeta({txParams}) @@ -317,13 +317,18 @@ module.exports = class TransactionController extends EventEmitter { _normalizeTxParams (txParams) { delete txParams.chainId - if ( !txParams.to ) delete txParams.to - else txParams.to = ethUtil.addHexPrefix(txParams.to) - + if ( !txParams.to ) { + delete txParams.to + } else { + txParams.to = ethUtil.addHexPrefix(txParams.to) + } txParams.from = ethUtil.addHexPrefix(txParams.from).toLowerCase() - if (!txParams.data) delete txParams.data - else txParams.data = ethUtil.addHexPrefix(txParams.data) + if (!txParams.data) { + delete txParams.data + } else { + txParams.data = ethUtil.addHexPrefix(txParams.data) + } if (txParams.value) txParams.value = ethUtil.addHexPrefix(txParams.value) @@ -331,7 +336,7 @@ module.exports = class TransactionController extends EventEmitter { if (txParams.gasPrice) txParams.gas = ethUtil.addHexPrefix(txParams.gas) } - async _validateTxParams (txParams) { + _validateTxParams (txParams) { this._validateFrom(txParams) this._validateRecipient(txParams) if ('value' in txParams) { -- cgit v1.2.3 From 343f0e9e80af804f256a5fa1a55b136c8241c368 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 4 Apr 2018 20:18:44 -0700 Subject: transactions - remove unnecessary keys on txParams --- app/scripts/controllers/transactions.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index a73a8b36d..0b78d62f1 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -315,6 +315,18 @@ module.exports = class TransactionController extends EventEmitter { // _normalizeTxParams (txParams) { + const acceptableKeys = [ + 'from', + 'to', + 'nonce', + 'value', + 'data', + 'gas', + 'gasPrice', + ] + Object.keys(txParams).forEach((key) => { + if (!acceptableKeys.includes(key)) delete txParams[key] + }) delete txParams.chainId if ( !txParams.to ) { -- cgit v1.2.3 From c02da0f27ca4a4239ebae4cfd3348a656e258b86 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Thu, 5 Apr 2018 12:12:02 -0700 Subject: transactions - _normalizeTxParams will now return a new object for txParams --- app/scripts/controllers/transactions.js | 49 ++++++++++++--------------------- 1 file changed, 18 insertions(+), 31 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 0b78d62f1..336b0d8f7 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -185,10 +185,10 @@ module.exports = class TransactionController extends EventEmitter { async addUnapprovedTransaction (txParams) { // validate - this._validateTxParams(txParams) - this._normalizeTxParams(txParams) + const normalizedTxParams = this._normalizeTxParams(txParams) + this._validateTxParams(normalizedTxParams) // construct txMeta - let txMeta = this.txStateManager.generateTxMeta({txParams}) + let txMeta = this.txStateManager.generateTxMeta({ txParams: normalizedTxParams }) this.addTx(txMeta) this.emit('newUnapprovedTx', txMeta) // add default tx params @@ -315,37 +315,24 @@ module.exports = class TransactionController extends EventEmitter { // _normalizeTxParams (txParams) { - const acceptableKeys = [ - 'from', - 'to', - 'nonce', - 'value', - 'data', - 'gas', - 'gasPrice', - ] - Object.keys(txParams).forEach((key) => { - if (!acceptableKeys.includes(key)) delete txParams[key] - }) - delete txParams.chainId - - if ( !txParams.to ) { - delete txParams.to - } else { - txParams.to = ethUtil.addHexPrefix(txParams.to) + // functions that handle normalizing of that key in txParams + const whiteList = { + from: from => ethUtil.addHexPrefix(from).toLowerCase(), + to: to => ethUtil.addHexPrefix(txParams.to).toLowerCase(), + nonce: nonce => ethUtil.addHexPrefix(nonce), + value: value => ethUtil.addHexPrefix(value), + data: data => ethUtil.addHexPrefix(data), + gas: gas => ethUtil.addHexPrefix(gas), + gasPrice: gasPrice => ethUtil.addHexPrefix(gasPrice), } - txParams.from = ethUtil.addHexPrefix(txParams.from).toLowerCase() - if (!txParams.data) { - delete txParams.data - } else { - txParams.data = ethUtil.addHexPrefix(txParams.data) - } - - if (txParams.value) txParams.value = ethUtil.addHexPrefix(txParams.value) + // apply only keys in the whiteList + const normalizedTxParams = {} + Object.keys(whiteList).forEach((key) => { + if (txParams[key]) normalizedTxParams[key] = whiteList[key](txParams[key]) + }) - if (txParams.gas) txParams.gas = ethUtil.addHexPrefix(txParams.gas) - if (txParams.gasPrice) txParams.gas = ethUtil.addHexPrefix(txParams.gas) + return normalizedTxParams } _validateTxParams (txParams) { -- cgit v1.2.3