Update transaction params validation (#9992)

* Update transaction params validation

* fixup! Update transaction params validation

* Update to/data error message

* fixup! Update to/data error message

app/scripts/controllers/transactions/index.js

@@ -827,9 +827,9 @@ export default class TransactionController extends EventEmitter {
     ].find((methodName) => methodName === name && name.toLowerCase())
 
     let result
-    if (txParams.data && tokenMethodName) {
+    if (data && tokenMethodName) {
       result = tokenMethodName
-    } else if (txParams.data && !to) {
+    } else if (data && !to) {
       result = TRANSACTION_CATEGORIES.DEPLOY_CONTRACT
     }
 

app/scripts/controllers/transactions/lib/util.js

@@ -1,4 +1,5 @@
 import { isValidAddress } from 'ethereumjs-util'
+import { ethErrors } from 'eth-json-rpc-errors'
 import { addHexPrefix } from '../../../lib/util'
 import { TRANSACTION_STATUSES } from '../../../../../shared/constants/transaction'
 
@@ -37,19 +38,30 @@ export function normalizeTxParams(txParams, lowerCase = true) {
  * @throws {Error} if the tx params contains invalid fields
  */
 export function validateTxParams(txParams) {
+  if (!txParams || typeof txParams !== 'object' || Array.isArray(txParams)) {
+    throw ethErrors.rpc.invalidParams(
+      'Invalid transaction params: must be an object.',
+    )
+  }
+  if (!txParams.to && !txParams.data) {
+    throw ethErrors.rpc.invalidParams(
+      'Invalid transaction params: must specify "data" for contract deployments, or "to" (and optionally "data") for all other types of transactions.',
+    )
+  }
+
   validateFrom(txParams)
   validateRecipient(txParams)
   if ('value' in txParams) {
     const value = txParams.value.toString()
     if (value.includes('-')) {
-      throw new Error(
+      throw ethErrors.rpc.invalidParams(
-        `Invalid transaction value of ${txParams.value} not a positive number.`,
+        `Invalid transaction value "${txParams.value}": not a positive number.`,
       )
     }
 
     if (value.includes('.')) {
-      throw new Error(
+      throw ethErrors.rpc.invalidParams(
-        `Invalid transaction value of ${txParams.value} number must be in wei`,
+        `Invalid transaction value of "${txParams.value}": number must be in wei.`,
       )
     }
   }
@@ -62,10 +74,12 @@ export function validateTxParams(txParams) {
  */
 export function validateFrom(txParams) {
   if (!(typeof txParams.from === 'string')) {
-    throw new Error(`Invalid from address ${txParams.from} not a string`)
+    throw ethErrors.rpc.invalidParams(
+      `Invalid "from" address "${txParams.from}": not a string.`,
+    )
   }
   if (!isValidAddress(txParams.from)) {
-    throw new Error('Invalid from address')
+    throw ethErrors.rpc.invalidParams('Invalid "from" address.')
   }
 }
 
@@ -80,10 +94,10 @@ export function validateRecipient(txParams) {
     if (txParams.data) {
       delete txParams.to
     } else {
-      throw new Error('Invalid recipient address')
+      throw ethErrors.rpc.invalidParams('Invalid "to" address.')
     }
   } else if (txParams.to !== undefined && !isValidAddress(txParams.to)) {
-    throw new Error('Invalid recipient address')
+    throw ethErrors.rpc.invalidParams('Invalid "to" address.')
   }
   return txParams
 }

test/unit/app/controllers/transactions/tx-controller-test.js

@@ -276,6 +276,7 @@ describe('Transaction Controller', function () {
 
   describe('#addUnapprovedTransaction', function () {
     const selectedAddress = '0x1678a085c290ebd122dc42cba69373b5953b831d'
+    const recipientAddress = '0xc42edfcc21ed14dda456aa0756c153f7985d8813'
 
     let getSelectedAddress, getPermittedAccounts
     beforeEach(function () {
@@ -295,6 +296,7 @@ describe('Transaction Controller', function () {
     it('should add an unapproved transaction and return a valid txMeta', async function () {
       const txMeta = await txController.addUnapprovedTransaction({
         from: selectedAddress,
+        to: recipientAddress,
       })
       assert.ok('id' in txMeta, 'should have a id')
       assert.ok('time' in txMeta, 'should have a time stamp')
@@ -321,7 +323,10 @@ describe('Transaction Controller', function () {
         done()
       })
       txController
-        .addUnapprovedTransaction({ from: selectedAddress })
+        .addUnapprovedTransaction({
+          from: selectedAddress,
+          to: recipientAddress,
+        })
         .catch(done)
     })
 

test/unit/app/controllers/transactions/tx-utils-test.js

@@ -6,21 +6,47 @@ describe('txUtils', function () {
     it('does not throw for positive values', function () {
       const sample = {
         from: '0x1678a085c290ebd122dc42cba69373b5953b831d',
+        to: '0xc42edfcc21ed14dda456aa0756c153f7985d8813',
         value: '0x01',
       }
       txUtils.validateTxParams(sample)
     })
 
-    it('returns error for negative values', function () {
+    it('throws for invalid params value', function () {
+      assert.throws(() => txUtils.validateTxParams(), {
+        message: 'Invalid transaction params: must be an object.',
+      })
+      assert.throws(() => txUtils.validateTxParams(null), {
+        message: 'Invalid transaction params: must be an object.',
+      })
+      assert.throws(() => txUtils.validateTxParams(true), {
+        message: 'Invalid transaction params: must be an object.',
+      })
+      assert.throws(() => txUtils.validateTxParams([]), {
+        message: 'Invalid transaction params: must be an object.',
+      })
+    })
+
+    it('throws for missing "to" and "data"', function () {
       const sample = {
         from: '0x1678a085c290ebd122dc42cba69373b5953b831d',
+        value: '0x01',
+      }
+      assert.throws(() => txUtils.validateTxParams(sample), {
+        message:
+          'Invalid transaction params: must specify "data" for contract deployments, or "to" (and optionally "data") for all other types of transactions.',
+      })
+    })
+
+    it('throws for negative values', function () {
+      const sample = {
+        from: '0x1678a085c290ebd122dc42cba69373b5953b831d',
+        to: '0xc42edfcc21ed14dda456aa0756c153f7985d8813',
         value: '-0x01',
       }
-      try {
+      assert.throws(() => txUtils.validateTxParams(sample), {
-        txUtils.validateTxParams(sample)
+        message: 'Invalid transaction value "-0x01": not a positive number.',
-      } catch (err) {
+      })
-        assert.ok(err, 'error')
-      }
     })
   })