refactor getLedgerVersionHelper

src/api/ledger/balances.js

@@ -34,25 +34,24 @@ function getTrustlinesAsync(account, options, callback) {
     .catch(callback);
 }
 
-function getLedgerVersion(remote: Remote, optionValue?: number,
+function getLedgerVersionHelper(remote: Remote, optionValue?: number,
   callback: GetLedgerSequenceCallback
 ) {
-  if (typeof optionValue === 'number') {
+  if (optionValue !== undefined && optionValue !== null) {
     callback(null, optionValue);
   } else {
     remote.getLedgerSequence(callback);
   }
 }
 
-
 function getBalancesAsync(account, options, callback) {
   validate.address(account);
   validate.getBalancesOptions(options);
 
   async.parallel({
-    xrp: async.compose(
+    xrp: async.seq(
-      _.partial(utils.getXRPBalance, this.remote, account),
+      _.partial(getLedgerVersionHelper, this.remote, options.ledgerVersion),
-      _.partial(getLedgerVersion, this.remote, options.ledgerVersion)
+      _.partial(utils.getXRPBalance, this.remote, account)
     ),
     trustlines: _.partial(getTrustlinesAsync.bind(this), account, options)
   }, composeAsync(formatBalances, convertErrors(callback)));

src/api/ledger/orders.js

@@ -1,6 +1,7 @@
 /* @flow */
 'use strict';
 const _ = require('lodash');
+const async = require('async');
 const utils = require('./utils');
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
@@ -26,19 +27,6 @@ function getOrdersAsync(account, options, callback) {
   validate.address(account);
   validate.getOrdersOptions(options);
 
-  if (!options.ledgerVersion) {
-    const self = this;
-    this.remote.getLedgerSequence((err, seq) => {
-      if (err) {
-        convertErrors(callback)(err);
-        return;
-      }
-      const newOptions = _.extend(options, {ledgerVersion: seq});
-      getOrdersAsync.call(self, account, newOptions, callback);
-    });
-    return;
-  }
-
   const getter = _.partial(requestAccountOffers, this.remote, account,
                            options.ledgerVersion);
   utils.getRecursive(getter, options.limit,
@@ -47,7 +35,9 @@ function getOrdersAsync(account, options, callback) {
 }
 
 function getOrders(account: string, options = {}) {
-  return utils.promisify(getOrdersAsync).call(this, account, options);
+  return utils.promisify(async.seq(
+    utils.getLedgerOptionsWithLedgerVersion,
+    getOrdersAsync)).call(this, account, options);
 }
 
 module.exports = getOrders;

src/api/ledger/transaction.js

@@ -90,21 +90,19 @@ function getTransactionAsync(identifier: string, options: TransactionOptions,
   }
 
 
-  /* eslint-disable no-unused-vars, handle-callback-err */
+  function maxLedgerGetter(error_?: Error, tx?: Object) {
-  function callbackWrapper2(error_?: Error, tx?: Object) {
     remote.getLedgerSequence(function(err?, seq: number) {
-      const maxLedgerVersion = Math.min(options.maxLedgerVersion || Infinity,
+      _.noop(err);
-        seq);
+      const maxLedgerVersion = options.maxLedgerVersion || seq;
       callbackWrapper(error_, tx, maxLedgerVersion);
     });
   }
-  /* eslint-enable no-unused-vars, handle-callback-err */
 
   async.waterfall([
     _.partial(remote.requestTx.bind(remote),
       {hash: identifier, binary: false}),
     _.partial(attachTransactionDate, remote)
-  ], callbackWrapper2);
+  ], maxLedgerGetter);
 }
 
 function getTransaction(identifier: string,

src/api/ledger/trustlines.js

@@ -1,6 +1,7 @@
 /* @flow */
 'use strict';
 const _ = require('lodash');
+const async = require('async');
 const utils = require('./utils');
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
@@ -42,26 +43,15 @@ function getTrustlinesAsync(account: string, options: {currency: string,
   validate.address(account);
   validate.getTrustlinesOptions(options);
 
-  if (!options.ledgerVersion) {
-    const self = this;
-    this.remote.getLedgerSequence(convertErrors(function(err?, seq: number) {
-      if (err) {
-        callback(err);
-      } else {
-        const newOptions = _.extend(options, {ledgerVersion: seq});
-        getTrustlinesAsync.call(self, account, newOptions, callback);
-      }
-    }));
-    return;
-  }
-
   const getter = _.partial(getAccountLines, this.remote, account,
                            options.ledgerVersion, options);
   utils.getRecursive(getter, options.limit, callback);
 }
 
 function getTrustlines(account: string, options = {}) {
-  return utils.promisify(getTrustlinesAsync).call(this, account, options);
+  return utils.promisify(async.seq(
+    utils.getLedgerOptionsWithLedgerVersion,
+    getTrustlinesAsync)).call(this, account, options);
 }
 
 module.exports = getTrustlines;

src/api/ledger/utils.js

@@ -108,12 +108,28 @@ function hasCompleteLedgerRange(remote: Remote, minLedgerVersion?: number,
 
 function isPendingLedgerVersion(remote: Remote, maxLedgerVersion: ?number
 ): boolean {
-  const currentLedger = remote.getLedgerSequence();
+  const currentLedger = remote.getLedgerSequenceSync();
   return currentLedger < (maxLedgerVersion || 0);
 }
 
+function getLedgerOptionsWithLedgerVersion(account: string, options: Object,
+  callback: (err?: ?Error, account?: string, options: Object) => void
+) {
+  if (Boolean(options) && options.ledgerVersion !== undefined &&
+    options.ledgerVersion !== null
+  ) {
+    callback(null, account, options);
+  } else {
+    this.remote.getLedgerSequence(common.convertErrors((err, sequence) => {
+      callback(err, account, _.assign({}, options, {
+        ledgerVersion: sequence}));
+    }));
+  }
+}
+
 module.exports = {
   getXRPBalance,
+  getLedgerOptionsWithLedgerVersion,
   compareTransactions,
   renameCounterpartyToIssuer,
   renameCounterpartyToIssuerInOrder,

src/api/server/server.js

@@ -63,14 +63,11 @@ function getServerInfoAsync(
 }
 
 function getFee(): ?number {
-  if (!this.remote._servers.length) {
+  if (!this.remote.getConnectedServers().length) {
     throw new common.errors.RippledNetworkError('No servers available.');
   }
   const fee = this.remote.createTransaction()._computeFee();
-  if (typeof fee !== 'string') {
+  return fee === undefined ? undefined : common.dropsToXrp(fee);
-    return undefined;
-  }
-  return common.dropsToXrp(fee);
 }
 
 function getLedgerVersion(): Promise<number> {

src/api/transaction/utils.js

@@ -52,7 +52,7 @@ function prepareTransaction(transaction: any, remote: any, instructions: any,
   const txJSON = transaction.tx_json;
 
 
-  function prepare1(callback_) {
+  function prepareMaxLedgerVersion(callback_) {
     if (instructions.maxLedgerVersion !== undefined) {
       txJSON.LastLedgerSequence = parseInt(instructions.maxLedgerVersion, 10);
       callback_();
@@ -66,7 +66,7 @@ function prepareTransaction(transaction: any, remote: any, instructions: any,
     }
   }
 
-  function prepare2(callback_) {
+  function prepareFee(callback_) {
     if (instructions.fee !== undefined) {
       txJSON.Fee = common.xrpToDrops(instructions.fee);
       callback_();
@@ -82,7 +82,7 @@ function prepareTransaction(transaction: any, remote: any, instructions: any,
     }
   }
 
-  function prepare3(callback_) {
+  function prepareSequence(callback_) {
     if (instructions.sequence !== undefined) {
       txJSON.Sequence = parseInt(instructions.sequence, 10);
       callback_(null, formatPrepareResponse(txJSON));
@@ -95,9 +95,9 @@ function prepareTransaction(transaction: any, remote: any, instructions: any,
   }
 
   async.series([
-    prepare1,
+    prepareMaxLedgerVersion,
-    prepare2,
+    prepareFee,
-    prepare3
+    prepareSequence
   ], common.convertErrors(function(error, results) {
     callback(error, results && results[2]);
   }));

src/core/remote.js

@@ -536,9 +536,8 @@ Remote.prototype.getLedgerSequence = function(callback = function() {}) {
     // the "current" ledger is the one after the most recently closed ledger
     callback(null, this._ledger_current_index - 1);
   } else {
-    const self = this;
+    this.once('ledger_closed', () => {
-    this.once('ledger_closed', function() {
+      callback(null, this._ledger_current_index - 1);
-      callback(null, self._ledger_current_index - 1);
     });
   }
 };

src/core/transaction.js

@@ -581,7 +581,7 @@ Transaction.prototype.setLastLedgerSequence = function(sequence) {
     assert(this.remote, 'Unable to set LastLedgerSequence, missing Remote');
 
     this._setUInt32('LastLedgerSequence',
-                    this.remote.getLedgerSequence() + 1
+                    this.remote.getLedgerSequenceSync() + 1
                     + this.getLastLedgerSequenceOffset());
   }
 

src/core/transactionmanager.js

@@ -681,7 +681,7 @@ TransactionManager.prototype._request = function(tx) {
     }
   }
 
-  tx.submitIndex = this._remote.getLedgerSequence() + 1;
+  tx.submitIndex = this._remote.getLedgerSequenceSync() + 1;
 
   if (tx.attempts === 0) {
     tx.initialSubmitIndex = tx.submitIndex;

test/api-test.js

@@ -643,9 +643,7 @@ describe('RippleAPI', function() {
     this.api.getLedgerVersion().then((ver) => {
       assert.strictEqual(ver, 8819951);
       done();
-    }, (err) => {
+    }, done);
-      done(err);
-    });
   });
 
   it('getLedger', function() {

test/transaction-manager-test.js

@@ -696,7 +696,7 @@ describe('TransactionManager', function() {
       assert.strictEqual(summary.submissionAttempts, 0);
       assert.strictEqual(summary.submitIndex, undefined);
       assert.strictEqual(summary.initialSubmitIndex, undefined);
-      assert.strictEqual(summary.lastLedgerSequence, remote.getLedgerSequence() + 1 + Remote.DEFAULTS.last_ledger_offset);
+      assert.strictEqual(summary.lastLedgerSequence, remote.getLedgerSequenceSync() + 1 + Remote.DEFAULTS.last_ledger_offset);
       assert.strictEqual(summary.state, 'failed');
       assert.strictEqual(summary.finalized, true);
       assert.deepEqual(summary.result, {