Extract order validation

api/lib/schema-validator.js

@@ -2,6 +2,7 @@
 var fs = require('fs');
 var path = require('path');
 var JaySchema = require('jayschema');
+var formatJaySchemaErrors = require('jayschema-error-messages');
 
 var baseDir = path.join(__dirname, './schemas');
 
@@ -12,9 +13,12 @@ module.exports = (function() {
   // If schema is valid, return true. Otherwise
   // return array of validation errors
   validator.validate = function() {
-    var result = {err: validate.apply(validator, arguments)};
-    result.isValid = !Boolean(result.err.length);
-    return result;
+    var errors = validate.apply(validator, arguments);
+    return {
+      err: errors,
+      errors: formatJaySchemaErrors(errors),
+      isValid: errors.length === 0
+    };
   };
 
   validator.isValid = function() {

api/lib/schemas/Order.json

@@ -8,22 +8,25 @@
       "description": "The Ripple account address of the order's creator",
       "$ref": "RippleAddress"
     },
-    "buy": {
+    "type": {
       "description": "If set to true the order it indicates that the creator is looking to receive the base_amount in exchange for the counter_amount. If undefined or set to false it indicates that the creator is looking to sell the base_amount to receive the counter_amount",
-      "type": "boolean"
+      "enum": ["buy", "sell"]
     },
-    "base_amount": {
+    "taker_pays": {
       "description": "The amount of currency the seller_account is seeking to buy. If other orders take part of this one, this value will change to represent the amount left in the order. This may be specified along with the counter_amount OR exchange_rate but not both. When the order is parsed from the Ripple Ledger the base currency will be determined according to the Priority Ranking of Currencies (XRP,EUR,GBP,AUD,NZD,USD,CAD,CHF,JPY,CNY) and if neither currency is listed in the ranking the base currency will be the one that is alphabetically first",
       "$ref": "Amount"
     },
-    "counter_amount": {
+    "taker_gets": {
       "description": "The amount of currency being sold. If other orders take part of this one, this value will change to represent the amount left in the order. This may be specified along with the base_amount OR the exchange_rate but not both",
       "$ref": "Amount"
     },
     "exchange_rate": {
       "description": "A string representation of the order price, defined as the cost one unit of the base currency in terms of the counter currency. This may be specified along with the base_amount OR the counter_amount but not both. If it is unspecified it will be computed automatically based on the counter_amount divided by the base_amount",
       "$ref": "FloatString"
     },
+    "passive": {
+      "type": "boolean"
+    },
     "expiration_timestamp": {
       "description": "The ISO combined date and time string representing the point beyond which the order will no longer be considered active or valid",
       "$ref": "Timestamp"
@@ -78,5 +81,6 @@
       "$ref": "Order"
     }
   },
-  "required": ["account"]
-}
+  "required": ["type", "taker_gets", "taker_pays"],
+  "additionalProperties": false
+}

api/lib/submit_transaction_hooks.js

@@ -43,7 +43,11 @@ SubmitTransactionHooks.prototype.initializeTransaction =
  *  @param {Error} error
  */
 SubmitTransactionHooks.prototype.validateParams = function(async_callback) {
-  this.hooks.validateParams(async_callback);
+  if (this.hooks.validateParams) {
+    this.hooks.validateParams(async_callback);
+  } else {
+    async_callback();
+  }
 };
 
 /**

api/lib/utils.js

@@ -4,6 +4,13 @@ var bignum = require('bignumber.js');
 var validator = require('./schema-validator.js');
 var ripple = require('ripple-lib');
 
+function renameCounterpartyToIssuer(amount) {
+  if (amount && amount.counterparty) {
+    amount.issuer = amount.counterparty;
+    delete amount.counterparty;
+  }
+}
+
 function dropsToXrp(drops) {
   return bignum(drops).dividedBy(1000000.0).toString();
 }
@@ -127,6 +134,7 @@ module.exports = {
   parseCurrencyAmount: parseCurrencyAmount,
   parseCurrencyQuery: parseCurrencyQuery,
   txFromRestAmount: txFromRestAmount,
-  compareTransactions: compareTransactions
+  compareTransactions: compareTransactions,
+  renameCounterpartyToIssuer: renameCounterpartyToIssuer
 };
 

api/lib/validate.js

@@ -5,20 +5,20 @@ var validator = require('./schema-validator');
 var ripple = require('ripple-lib');
 var utils = require('./utils');
 
-function isValidBoolString(boolString) {
-  return (boolString === 'true' || boolString === false);
+function isBoolean(value) {
+  return (value === true || value === false);
 }
 
 function error(text) {
   return new InvalidRequestError(text);
 }
 
 function invalid(type, value) {
-  return error('Not a valid ' + type + ': ' + value.toString());
+  return error('Not a valid ' + type + ': ' + JSON.stringify(value));
 }
 
 function missing(name) {
-  return error('Missing parameter: ' + name);
+  return error('Parameter missing: ' + name);
 }
 
 function validateAddress(address) {
@@ -31,17 +31,16 @@ function validateAddress(address) {
 function validateAddressAndSecret(obj) {
   var address = obj.address;
   var secret = obj.secret;
-  if (!address) {
-    throw missing('address');
-  }
+  validateAddress(address);
   if (!secret) {
     throw missing('secret');
   }
-  if (secret[0] !== 's' || !ripple.UInt160.is_valid('r' + secret.slice(1))) {
-    throw invalid('Ripple secret', secret);
-  }
-  if (!ripple.Seed.from_json(secret).get_key(address)) {
-    throw error('Secret does not match specified address', secret);
+  try {
+    if (!ripple.Seed.from_json(secret).get_key(address)) {
+      throw error('Invalid secret', secret);
+    }
+  } catch (exception) {
+    throw error('Invalid secret', secret);
   }
 }
 
@@ -94,16 +93,46 @@ function validateLimit(limit) {
   }
 }
 
-function validateTrustline(trustline) {
-  var errs = validator.validate(trustline, 'Trustline').err;
-  if (errs.length > 0) {
-    throw invalid('trustline', errs[0]);
+function validateSchema(object, schemaName) {
+  var schemaErrors = validator.validate(object, schemaName).errors;
+  if (!_.isEmpty(schemaErrors.fields)) {
+    throw invalid(schemaName, schemaErrors.fields);
+  }
+}
+
+function validateOrder(order) {
+  if (!order) {
+    throw error('Missing parameter: order. '
+      + 'Submission must have order object in JSON form');
+  } else if (!/^buy|sell$/.test(order.type)) {
+    throw error('Parameter must be "buy" or "sell": type');
+  } else if (!_.isUndefined(order.passive) && !_.isBoolean(order.passive)) {
+    throw error('Parameter must be a boolean: passive');
+  } else if (!_.isUndefined(order.immediate_or_cancel)
+      && !_.isBoolean(order.immediate_or_cancel)) {
+    throw error('Parameter must be a boolean: immediate_or_cancel');
+  } else if (!_.isUndefined(order.fill_or_kill)
+      && !_.isBoolean(order.fill_or_kill)) {
+    throw error('Parameter must be a boolean: fill_or_kill');
+  } else if (!order.taker_gets
+      || (!validator.isValid(order.taker_gets, 'Amount'))
+      || (!order.taker_gets.issuer && order.taker_gets.currency !== 'XRP')) {
+    throw error('Parameter must be a valid Amount object: taker_gets');
+  } else if (!order.taker_pays
+      || (!validator.isValid(order.taker_pays, 'Amount'))
+      || (!order.taker_pays.issuer && order.taker_pays.currency !== 'XRP')) {
+    throw error('Parameter must be a valid Amount object: taker_pays');
   }
+  // TODO: validateSchema(order, 'Order');
+}
+
+function validateTrustline(trustline) {
+  validateSchema(trustline, 'Trustline');
 }
 
 function validateValidated(validated) {
-  if (!isValidBoolString(validated)) {
-    throw error('validated must be "true" or "false", not', validated);
+  if (!isBoolean(validated)) {
+    throw error('validated must be boolean, not: ' + validated);
   }
 }
 
@@ -132,6 +161,7 @@ module.exports = createValidators({
   ledger: validateLedger,
   limit: validateLimit,
   paging: validatePaging,
+  order: validateOrder,
   trustline: validateTrustline,
   validated: validateValidated
 });

api/notifications.js

@@ -2,12 +2,12 @@
 'use strict';
 var _ = require('lodash');
 var async = require('async');
-var ripple = require('ripple-lib');
 var transactions = require('./transactions');
 var serverLib = require('./lib/server-lib');
 var NotificationParser = require('./lib/notification_parser.js');
 var errors = require('./lib/errors.js');
 var utils = require('./lib/utils.js');
+var validate = require('./lib/validate.js');
 
 /**
  *  Find the previous and next transaction hashes or
@@ -169,13 +169,7 @@ function attachPreviousAndNextTransactionIdentifiers(api,
  *  @param {Notification} notification
  */
 function getNotificationHelper(api, account, identifier, topCallback) {
-  if (!account) {
-    return topCallback(new errors.InvalidRequestError(
-      'Missing parameter: account. Must be a valid Ripple Address'));
-  } else if (!ripple.UInt160.is_valid(account)) {
-    return topCallback(new errors.InvalidRequestError(
-      'Parameter is not a valid Ripple address: account'));
-  }
+  validate.address(account);
 
   function getTransaction(callback) {
     transactions.getTransaction(api, account, identifier, {}, callback);

api/orders.js

@@ -170,54 +170,13 @@ function placeOrder(account, order, secret, options, callback) {
     validated: options.validated
   };
 
-  function validateParams(_callback) {
-    if (!order) {
-      return _callback(new InvalidRequestError(
-        'Missing parameter: order. '
-        + 'Submission must have order object in JSON form'));
-    }
-
-    if (order.taker_gets && order.taker_gets.currency !== 'XRP') {
-      order.taker_gets.issuer = order.taker_gets.counterparty;
-      delete order.taker_gets.counterparty;
-    }
-
-    if (order.taker_pays && order.taker_pays.currency !== 'XRP') {
-      order.taker_pays.issuer = order.taker_pays.counterparty;
-      delete order.taker_pays.counterparty;
-    }
-
-    if (!ripple.UInt160.is_valid(account)) {
-      return _callback(new errors.InvalidRequestError(
-        'Parameter is not a valid Ripple address: account'));
-    } else if (!/^buy|sell$/.test(order.type)) {
-      return _callback(new InvalidRequestError(
-        'Parameter must be "buy" or "sell": type'));
-    } else if (!_.isUndefined(order.passive) && !_.isBoolean(order.passive)) {
-      return _callback(new InvalidRequestError(
-        'Parameter must be a boolean: passive'));
-    } else if (!_.isUndefined(order.immediate_or_cancel)
-        && !_.isBoolean(order.immediate_or_cancel)) {
-      return _callback(new InvalidRequestError(
-        'Parameter must be a boolean: immediate_or_cancel'));
-    } else if (!_.isUndefined(order.fill_or_kill)
-        && !_.isBoolean(order.fill_or_kill)) {
-      return _callback(new InvalidRequestError(
-        'Parameter must be a boolean: fill_or_kill'));
-    } else if (!order.taker_gets
-        || (!validator.isValid(order.taker_gets, 'Amount'))
-        || (!order.taker_gets.issuer && order.taker_gets.currency !== 'XRP')) {
-      _callback(new InvalidRequestError(
-        'Parameter must be a valid Amount object: taker_gets'));
-    } else if (!order.taker_pays
-        || (!validator.isValid(order.taker_pays, 'Amount'))
-        || (!order.taker_pays.issuer && order.taker_pays.currency !== 'XRP')) {
-      _callback(new InvalidRequestError(
-        'Parameter must be a valid Amount object: taker_pays'));
-    } else {
-      _callback();
-    }
+  if (order) {
+    utils.renameCounterpartyToIssuer(order.taker_gets);
+    utils.renameCounterpartyToIssuer(order.taker_pays);
   }
+  validate.addressAndSecret({address: account, secret: secret});
+  validate.order(order);
+  validate.validated(options.validated, true);
 
   function setTransactionParameters(transaction) {
     var takerPays = order.taker_pays.currency !== 'XRP'
@@ -239,7 +198,6 @@ function placeOrder(account, order, secret, options, callback) {
   }
 
   var hooks = {
-    validateParams: validateParams,
     formatTransactionResponse: TxToRestConverter.parseSubmitOrderFromTx,
     setTransactionParameters: setTransactionParameters
   };

test/_payments-test.js

@@ -2,7 +2,7 @@
 var supertest = require('supertest');
 var _app = require('../server/express_app');
 var app = supertest(_app);
-var assert = require('assert');
+var assert = require('assert-diff');
 var ws = require('ws');
 var route = new (require('events').EventEmitter);
 var fixtures = require('./fixtures')._payments;

test/fixtures/errors.js

@@ -19,6 +19,13 @@ module.exports.RESTMissingSecret = JSON.stringify({
   message: 'Parameter missing: secret'
 });
 
+module.exports.RESTRequestInvalidSecret = JSON.stringify({
+  success: false,
+  error_type: 'invalid_request',
+  error: 'restINVALID_PARAMETER',
+  message: 'Invalid secret'
+});
+
 module.exports.RESTInvalidSecret = JSON.stringify({
   success: false,
   error_type: 'transaction',

test/orders-test.js

@@ -589,9 +589,9 @@ suite('post orders', function() {
     .send(fixtures.order({
       secret: 'foo'
     }))
-    .expect(testutils.checkStatus(500))
+    .expect(testutils.checkStatus(400))
     .expect(testutils.checkHeaders)
-    .expect(testutils.checkBody(errors.RESTInvalidSecret))
+    .expect(testutils.checkBody(errors.RESTRequestInvalidSecret))
     .end(done);
   });
 
@@ -1066,9 +1066,9 @@ suite('post orders', function() {
     .send(fixtures.order({
         secret: 'foo'
     }))
-    .expect(testutils.checkStatus(500))
+    .expect(testutils.checkStatus(400))
     .expect(testutils.checkHeaders)
-    .expect(testutils.checkBody(errors.RESTInvalidSecret))
+    .expect(testutils.checkBody(errors.RESTRequestInvalidSecret))
     .end(done);
   });
 
@@ -1091,7 +1091,7 @@ suite('post orders', function() {
     .expect(testutils.checkBody(errors.RESTErrorResponse({
       type: 'invalid_request',
       error: 'restINVALID_PARAMETER',
-      message: 'Missing parameter: order. Submission must have order object in JSON form'
+      message: 'Parameter missing: order'
     })))
     .end(done);
   });