Sort transactions in getAccountTransactions based on real order in le…

…dger...

...instead of lexicographical order. Also, remove custom compareTransaction
function for api/transactions

api/lib/utils.js

@@ -96,9 +96,11 @@ function signum(num) {
  */
 function compareTransactions(first, second) {
   if (first.ledger_index === second.ledger_index) {
-    return signum(first.meta.TransactionIndex - second.meta.TransactionIndex);
+    return signum(
+      Number(first.meta.TransactionIndex) -
+      Number(second.meta.TransactionIndex));
   }
-  return first.ledger_index < second.ledger_index ? -1 : 1;
+  return Number(first.ledger_index) < Number(second.ledger_index) ? -1 : 1;
 }
 
 function isValidLedgerSequence(ledger) {

api/transactions.js

@@ -518,43 +518,6 @@ function transactionFilter(transactions, options) {
   return filtered_transactions;
 }
 
-/**
- * Order two transactions based on their ledger_index.
- * If two transactions took place in the same ledger, sort
- * them based on a lexicographical comparison of their hashes
- * to ensure the ordering is deterministic.
- *
- * @param {transaction in JSON format} first
- * @param {transaction in JSON format} second
- * @param {Boolean} [false] earliestFirst
- * @returns {Number} comparison Returns -1 or 1
- */
-function compareTransactions(first, second, earliestFirst) {
-  var firstIndex = first.ledger || first.ledger_index;
-  var secondIndex = second.ledger || second.ledger_index;
-  var firstLessThanSecond = true;
-  if (firstIndex === secondIndex) {
-    if (first.hash <= second.hash) {
-      firstLessThanSecond = true;
-    } else {
-      firstLessThanSecond = false;
-    }
-  } else if (firstIndex < secondIndex) {
-    firstLessThanSecond = true;
-  } else {
-    firstLessThanSecond = false;
-  }
-  if (earliestFirst) {
-    if (firstLessThanSecond) {
-      return -1;
-    }
-    return 1;
-  } else if (firstLessThanSecond) {
-    return 1;
-  }
-  return -1;
-}
-
 /**
  * Recursively get transactions for the specified account from
  * the Remote and local database. If options.min is set, this will
@@ -606,9 +569,9 @@ function getAccountTransactions(api, options, callback) {
   }
 
   function sortTransactions(transactions, async_callback) {
-    transactions.sort(function(first, second) {
-      return compareTransactions(first, second, options.earliestFirst);
-    });
+    var compare = options.earliestFirst ? utils.compareTransactions :
+      _.rearg(utils.compareTransactions, 1, 0);
+    transactions.sort(compare);
     async_callback(null, transactions);
   }
 

test/unit/utils-test.js

@@ -4,6 +4,7 @@ var assert = require('assert');
 var utils = require('../../api/lib/utils.js');
 var convertAmount = require('../../api/transaction/utils').convertAmount;
 var addresses = require('./../fixtures/addresses.js');
+var _ = require('lodash');
 
 suite('unit - utils.parseLedger()', function() {
   var DEFAULT_LEDGER = 'validated';
@@ -234,6 +235,18 @@ suite('unit - convertAmount()', function() {
 });
 
 suite('unit - utils.compareTransactions()', function() {
+
+  function toStringValues(tx) {
+    return _.mapValues(tx, function(val) {
+      if (typeof val === 'number') {
+        return val.toString();
+      } else if (typeof val === 'object') {
+        return toStringValues(val);
+      }
+      return val;
+    });
+  }
+
   test('compareTransactions() -- different ledgers', function() {
     var tx1 = {
       ledger_index: 1
@@ -244,6 +257,9 @@ suite('unit - utils.compareTransactions()', function() {
     };
 
     assert.strictEqual(utils.compareTransactions(tx1, tx2), -1);
+    assert.strictEqual(
+      utils.compareTransactions(
+        toStringValues(tx1), toStringValues(tx2)), -1);
   });
 
   test('compareTransactions() -- same ledger', function() {
@@ -262,6 +278,9 @@ suite('unit - utils.compareTransactions()', function() {
     };
 
     assert.strictEqual(utils.compareTransactions(tx1, tx2), 1);
+    assert.strictEqual(
+      utils.compareTransactions(
+        toStringValues(tx1), toStringValues(tx2)), 1);
   });
 
   test('compareTransactions() -- same transaction', function() {
@@ -273,6 +292,9 @@ suite('unit - utils.compareTransactions()', function() {
     };
 
     assert.strictEqual(utils.compareTransactions(tx1, tx1), 0);
+    assert.strictEqual(
+      utils.compareTransactions(
+        toStringValues(tx1), toStringValues(tx1)), 0);
   });
 
 });