[FIX] fix handling of quality in order book

src/js/ripple/orderbook.js

@@ -141,7 +141,6 @@ OrderBook.offerRewrite = function(offer) {
     switch (key) {
       case 'PreviousTxnID':
       case 'PreviousTxnLgrSeq':
-      case 'quality':
         break;
       default:
         result[key] = offer[key];
@@ -355,12 +354,23 @@ OrderBook.prototype.setOwnerFunds = function(account, fundedAmount) {
  * Get owner's cached, transfer rate adjusted, funds
  *
  * @param {String} account - owner's account address
- * @return {String}
+ * @return {Amount}
  */
 
 OrderBook.prototype.getOwnerFunds = function(account) {
   assert(UInt160.is_valid(account), 'Account is invalid');
-  return this._ownerFunds[account];
+  
+  var amount;
+
+  if (this.hasOwnerFunds(account)) {
+    if (this._currencyGets.is_native()) {
+      amount = Amount.from_json(this._ownerFunds[account]);
+    } else {
+      amount = Amount.from_json(this._ownerFunds[account] + OrderBook.IOU_SUFFIX);
+    }
+  }
+
+  return amount;
 };
 
 /**
@@ -443,9 +453,9 @@ OrderBook.prototype.decrementOwnerOfferCount = function(account) {
 OrderBook.prototype.addOwnerOfferTotal = function(account, amount) {
   assert(UInt160.is_valid(account), 'Account is invalid');
   var previousAmount = this.getOwnerOfferTotal(account);
-  var newAmount = previousAmount.add(Amount.from_json(amount));
+  this._ownerOffersTotal[account] = previousAmount.add(Amount.from_json(amount));
-  this._ownerOffersTotal[account] = newAmount;
+
-  return newAmount;
+  return this._ownerOffersTotal[account];
 };
 
 /**
@@ -496,6 +506,7 @@ OrderBook.prototype.getOwnerOfferTotal = function(account) {
  * @param {String} account - owner's account address
  * @return {Amount}
  */
+
 OrderBook.prototype.resetOwnerOfferTotal = function(account) {
   var amount;
 
@@ -510,11 +521,24 @@ OrderBook.prototype.resetOwnerOfferTotal = function(account) {
   return amount;
 };
 
+/** 
+ * Casts and returns offer's taker gets funded amount as a default IOU amount
+ *
+ * @param {Object} offer
+ * @return {Amount}
+ */
+
+OrderBook.prototype.getOfferTakerGetsFunded = function(offer) {
+  assert(!_.isNull(offer.taker_gets_funded) && !_.isNaN(offer.taker_gets_funded));
+
+  return Amount.from_json(offer.taker_gets_funded + OrderBook.IOU_SUFFIX);
+};
+
 /**
  * Compute adjusted balance that would be left after issuer's transfer fee is deducted
  *
  * @param {String} balance
- * @return {Amount}
+ * @return {String}
  */
 
 OrderBook.prototype.applyTransferRate = function(balance) {
@@ -565,7 +589,6 @@ OrderBook.prototype.requestTransferRate = function(callback) {
   });
 };
 
-
 /**
  * Set funded amount on offer with its owner's cached funds
  * 
@@ -576,17 +599,13 @@ OrderBook.prototype.requestTransferRate = function(callback) {
  * @param {Object} offer
  * @return offer
  */
+
 OrderBook.prototype.setOfferFundedAmount = function(offer) {
   assert.strictEqual(typeof offer, 'object', 'Offer is invalid');
-  assert(!isNaN(this.getOwnerFunds(offer.Account)), 'Funds is invalid');
 
-  var fundedAmount = Amount.from_json(
+  var fundedAmount = this.getOwnerFunds(offer.Account);
-    this._currencyGets.is_native()
-    ? this.getOwnerFunds(offer.Account)
-    : this.getOwnerFunds(offer.Account) + OrderBook.IOU_SUFFIX
-  );
   var previousOfferSum = this.getOwnerOfferTotal(offer.Account);
-  var currentOfferSum = Amount.from_json(offer.TakerGets).add(previousOfferSum);
+  var currentOfferSum = previousOfferSum.add(Amount.from_json(offer.TakerGets));
 
   offer.owner_funds = this.getUnadjustedOwnerFunds(offer.Account);
 
@@ -596,22 +615,10 @@ OrderBook.prototype.setOfferFundedAmount = function(offer) {
     offer.taker_gets_funded = Amount.from_json(offer.TakerGets).to_text();
     offer.taker_pays_funded = Amount.from_json(offer.TakerPays).to_text();
   } else if (previousOfferSum.compareTo(fundedAmount) < 0) {
-    var takerGetsFunded = fundedAmount.subtract(previousOfferSum);
+    offer.taker_gets_funded = fundedAmount.subtract(previousOfferSum).to_text();
     
-    var takerPaysValue = this._currencyPays.is_native()
+    var takerPaysFunded = this.getOfferQuality(offer).multiply(this.getOfferTakerGetsFunded(offer));
-    ? offer.TakerPays
-    : offer.TakerPays.value;
-    var takerPays = Amount.from_json(takerPaysValue + OrderBook.IOU_SUFFIX);
     
-    var takerGetsValue = this._currencyGets.is_native()
-    ? offer.TakerGets
-    : offer.TakerGets.value;
-    var takerGets = Amount.from_json(takerGetsValue + OrderBook.IOU_SUFFIX);
-
-    var quality = takerPays.divide(takerGets);
-    var takerPaysFunded = Amount.from_json(takerGetsFunded.to_text() + OrderBook.IOU_SUFFIX).multiply(quality);
-
-    offer.taker_gets_funded = takerGetsFunded.to_text();
     offer.taker_pays_funded = this._currencyPays.is_native() ? String(parseInt(takerPaysFunded.to_json().value, 10)) : takerPaysFunded.to_json().value;
   } else {
     offer.taker_gets_funded = '0';
@@ -627,6 +634,7 @@ OrderBook.prototype.setOfferFundedAmount = function(offer) {
  * @param {Object} node - RippleState or AccountRoot meta node
  * @return {Object}
  */
+
 OrderBook.prototype.parseAccountBalanceFromNode = function(node) {
   var result = {
     account: void(0),
@@ -695,7 +703,7 @@ OrderBook.prototype.isBalanceChangeNode = function(node) {
 };
 
 /**
- * Updates funded amounts/balances using ModifiedNode
+ * Updates funded amounts/balances using modified balance nodes
  *
  * Update owner funds using modified AccountRoot and RippleState nodes.
  * Update funded amounts for offers in the orderbook using owner funds.
@@ -749,7 +757,7 @@ OrderBook.prototype.updateOwnerOffersFundedAmount = function(account) {
   var self = this;
 
   if (this._remote.trace) {
-    log.info('updating offer funds', this._key, account, this.getOwnerFunds(account));
+    log.info('updating offer funds', this._key, account, this.getOwnerFunds(account).to_text());
   }
 
   this.resetOwnerOfferTotal(account);
@@ -761,29 +769,21 @@ OrderBook.prototype.updateOwnerOffersFundedAmount = function(account) {
 
     // Save a copy of the old offer so we can show how the offer has changed
     var previousOffer = extend({}, offer);
-    var previousFundedGetsValue = offer.taker_gets_funded;
     var previousFundedGets;
 
-    if (_.isString(previousFundedGetsValue)) {
+    if (_.isString(offer.taker_gets_funded)) {
-      previousFundedGets = Amount.from_json(
+      // Offer is not new, so we should consider it for offer_changed and offer_funds_changed events
-        offer.taker_gets_funded + OrderBook.IOU_SUFFIX
+      previousFundedGets = self.getOfferTakerGetsFunded(offer);
-      );
     }
 
     self.setOfferFundedAmount(offer);
     self.addOwnerOfferTotal(offer.Account, offer.TakerGets);
 
-    var areFundsChanged = previousFundedGets && !previousFundedGets.equals(
+    var areFundsChanged = previousFundedGets && !self.getOfferTakerGetsFunded(offer).equals(previousFundedGets);
-      Amount.from_json(offer.taker_gets_funded + OrderBook.IOU_SUFFIX)
-    );
 
     if (areFundsChanged) {
       self.emit('offer_changed', previousOffer, offer);
-      self.emit('offer_funds_changed',
+      self.emit('offer_funds_changed', offer, previousOffer.taker_gets_funded, offer.taker_gets_funded);
-        offer,
-        previousOffer.taker_gets_funded,
-        offer.taker_gets_funded
-      );
     }
   });
 };
@@ -818,13 +818,13 @@ OrderBook.prototype.notify = function(transaction) {
   var takerGetsTotal = Amount.from_json(
     '0' + ((Currency.from_json(this._currencyGets).is_native())
            ? ''
-           : ('/' + this._currencyGets + '/' + this._issuerGets))
+           : ('/' + this._currencyGets.to_human() + '/' + this._issuerGets))
   );
 
   var takerPaysTotal = Amount.from_json(
     '0' + ((Currency.from_json(this._currencyPays).is_native())
            ? ''
-           : ('/' + this._currencyPays + '/' + this._issuerPays))
+           : ('/' + this._currencyPays.to_human() + '/' + this._issuerPays))
   );
 
   function handleNode(node) {
@@ -879,29 +879,22 @@ OrderBook.prototype.insertOffer = function(node) {
   }
 
   var offer = OrderBook.offerRewrite(node.fields);
+  var takerGets = this.normalizeAmount(this._currencyGets, offer.TakerGets);
+  var takerPays = this.normalizeAmount(this._currencyPays, offer.TakerPays);
 
   offer.LedgerEntryType = node.entryType;
   offer.index = node.ledgerIndex;
 
-  var DATE_REF = {
+  // We're safe to calculate quality for newly created offers
-    reference_date: new Date()
+  offer.quality = takerPays.divide(takerGets).to_text();
-  };
-
-  // XXX Should use Amount#from_quality
-  var price = Amount.from_json(
-    offer.TakerPays
-  ).ratio_human(offer.TakerGets, DATE_REF);
 
+  var quality = this.getOfferQuality(offer);
   var originalLength = this._offers.length;
 
   for (var i=0; i<originalLength; i++) {
-    var currentOffer = this._offers[i];
+    var existingOfferQuality = this.getOfferQuality(this._offers[i]);
 
-    var priceItem = Amount.from_json(
+    if (quality.compareTo(existingOfferQuality) <= 0) {
-      currentOffer.TakerPays
-    ).ratio_human(currentOffer.TakerGets, DATE_REF);
-
-    if (price.compareTo(priceItem) <= 0) {
       this._offers.splice(i, 0, offer);
 
       break;
@@ -919,6 +912,47 @@ OrderBook.prototype.insertOffer = function(node) {
   this.emit('offer_added', offer);
 };
 
+/**
+ * Retrieve offer quality
+ *
+ * @param {Object} offer
+ */
+
+OrderBook.prototype.getOfferQuality = function(offer) {
+  var amount;
+
+  if (this._currencyGets.has_interest()) {
+    // XXX Should use Amount#from_quality
+    amount = Amount.from_json(
+      offer.TakerPays
+    ).ratio_human(offer.TakerGets, {
+      reference_date: new Date()
+    });
+  } else {
+    amount = Amount.from_json(offer.quality + OrderBook.IOU_SUFFIX);
+  }
+
+  return amount;
+};
+
+/**
+ * Convert any amount into default IOU
+ *
+ * NOTE: This is necessary in some places because Amount.js arithmetic
+ * does not deal with native and non-native amounts well.
+ *
+ * @param {Currency} currency
+ * @param {Object} amountObj
+ */
+
+OrderBook.prototype.normalizeAmount = function(currency, amountObj) {
+  var value = currency.is_native()
+  ? amountObj
+  : amountObj.value
+
+  return Amount.from_json(value + OrderBook.IOU_SUFFIX);
+};
+
 /**
  * Modify an existing offer in the orderbook
  *
@@ -1000,14 +1034,14 @@ OrderBook.prototype.setOffers = function(offers) {
     var offer = OrderBook.offerRewrite(rawOffer);
 
     if (offer.hasOwnProperty('owner_funds')) {
-      // The first offer from book_offers contains owner balance of offer's output
+      // The first offer of each owner from book_offers contains owner balance of offer's output
       self.setOwnerFunds(offer.Account, offer.owner_funds);
     }
 
     self.incrementOwnerOfferCount(offer.Account);
       
     self.setOfferFundedAmount(offer);
-    self.addOwnerOfferTotal(rawOffer.Account, rawOffer.TakerGets);
+    self.addOwnerOfferTotal(offer.Account, offer.TakerGets);
 
     return offer;
   });

test/fixtures/orderbook.js

@@ -3,7 +3,7 @@ var addresses = require('./addresses');
 var Meta      = require('ripple-lib').Meta;
 
 module.exports.FIAT_BALANCE = '10';
-module.exports.NATIVE_BALANCE = '25';
+module.exports.NATIVE_BALANCE = '55';
 module.exports.NATIVE_BALANCE_PREVIOUS = '100';
 
 module.exports.TAKER_GETS = '19.84580331';
@@ -187,7 +187,7 @@ module.exports.REQUEST_OFFERS = [
     },
     index: 'B6BC3B0F87976370EE11F5575593FE63AA5DC1D602830DC96F04B2D597F044BF',
     owner_funds: '0.1129267125000245',
-    quality: '496.5'
+    quality: '496.4999999999999'
   },
   {
     Account: addresses.OTHER_ACCOUNT,
@@ -262,7 +262,7 @@ module.exports.REQUEST_OFFERS = [
     },
     index: 'A437D85DF80D250F79308F2B613CF5391C7CF8EE9099BC4E553942651CD9FA86',
     owner_funds: '0.950363009783092',
-    quality: '498.6116758238228'
+    quality: '199.4446703295291'
   }
 ];
 
@@ -282,7 +282,8 @@ module.exports.REQUEST_OFFERS_NATIVE = [
       value: '56.06639660617357'
     },
     index: 'B6BC3B0F87976370EE11F5575593FE63AA5DC1D602830DC96F04B2D597F044BF',
-    owner_funds: '600'
+    owner_funds: '600',
+    quality: '.0560663966061735'
   },
   {
     Account: addresses.OTHER_ACCOUNT,
@@ -301,6 +302,7 @@ module.exports.REQUEST_OFFERS_NATIVE = [
     },
     index: 'A437D85DF80D250F79308F2B613CF5391C7CF8EE9099BC4E553942651CD9FA86',
     owner_funds: '4000',
+    quality: '0.049861167582382'
   },
   {
     Account: addresses.THIRD_ACCOUNT,
@@ -319,6 +321,7 @@ module.exports.REQUEST_OFFERS_NATIVE = [
     },
     index: 'A437D85DF80D250F79308F2B613CF5391C7CF8EE9099BC4E553942651CD9FA86',
     owner_funds: '3900',
+    quality: '0.049861167582382'
   },
   {
     Account: addresses.THIRD_ACCOUNT,
@@ -338,7 +341,30 @@ module.exports.REQUEST_OFFERS_NATIVE = [
       value: '99.72233516476456'
     },
     index: 'A437D85DF80D250F79308F2B613CF5391C7CF8EE9099BC4E553942651CD9FA86',
-    quality: '498.6116758238228'
+    quality: '0.049861167582382'
+  }
+];
+
+module.exports.QUALITY_OFFERS = [
+  {
+    Account: addresses.ACCOUNT,
+    BookDirectory: '4627DFFCFF8B5A265EDBD8AE8C14A52325DBFEDAF4F5C32E5C1AFE1EE71A605F',
+    BookNode: '0000000000000000',
+    Flags: 0,
+    LedgerEntryType: 'Offer',
+    OwnerNode: '0000000000000009',
+    PreviousTxnID: 'BCA728C17DBA10F100C41D4EF8B37318F33BC6156E94DB16703D2A1EE43DCCE6',
+    PreviousTxnLgrSeq: 11929146,
+    Sequence: 668643,
+    TakerGets: {
+      currency: 'USD',
+      issuer: 'rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59B',
+      value: '301.3426005599325'
+    },
+    TakerPays: '22895281765',
+    index: '79B34D7DF703580B86099EFD6B2E419AA39A585A50C82A3F9B446721B7C1490C',
+    owner_funds: '5910.437716613066',
+    quality: '75977580.74216543'
   }
 ];
 

test/orderbook-test.js

@@ -149,7 +149,7 @@ describe('OrderBook', function() {
     book._issuerTransferRate = 1000000000;
     book.setOwnerFunds(addresses.ACCOUNT, '1');
 
-    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), '1');
+    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT).to_text(), '1');
   });
 
   it('Set owner funds - unadjusted funds', function() {
@@ -276,7 +276,7 @@ describe('OrderBook', function() {
 
     assert.strictEqual(book.decrementOwnerOfferCount(addresses.ACCOUNT), 0);
     assert.strictEqual(book.getOwnerOfferCount(addresses.ACCOUNT), 0);
-    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT, undefined));
+    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), undefined);
   });
 
   it('Decrement owner offer count - invalid address', function() {
@@ -577,7 +577,8 @@ describe('OrderBook', function() {
         currency: 'BTC',
         issuer: addresses.ISSUER
       },
-      TakerPays: '123456'
+      TakerPays: '123456',
+      quality: '1234.56'
     };
 
     book.setOwnerFunds(addresses.ACCOUNT, '99');
@@ -590,7 +591,8 @@ describe('OrderBook', function() {
       is_fully_funded: false,
       taker_gets_funded: '99',
       taker_pays_funded: '122221',
-      owner_funds: '99'
+      owner_funds: '99',
+      quality: '1234.56'
     };
 
     assert.deepEqual(offer, expected);
@@ -649,7 +651,8 @@ describe('OrderBook', function() {
         value: '123.456',
         currency: 'BTC',
         issuer: addresses.ISSUER
-      }
+      },
+      quality: '1.23456'
     };
 
     book.setOwnerFunds(addresses.ACCOUNT, '99');
@@ -662,7 +665,8 @@ describe('OrderBook', function() {
       is_fully_funded: false,
       taker_gets_funded: '99',
       taker_pays_funded: '122.22144',
-      owner_funds: '99'
+      owner_funds: '99',
+      quality: '1.23456'
     };
 
     assert.deepEqual(offer, expected);
@@ -1138,7 +1142,7 @@ describe('OrderBook', function() {
     book.updateFundedAmounts(message);
 
     setImmediate(function() {
-      assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), fixtures.FIAT_BALANCE);
+      assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT).to_text(), fixtures.FIAT_BALANCE);
       assert.strictEqual(receivedChangedEvents, 2);
       assert.strictEqual(receivedFundsChangedEvents, 2);
       done();
@@ -1237,7 +1241,7 @@ describe('OrderBook', function() {
     book.updateFundedAmounts(message);
 
     setImmediate(function() {
-      assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), '9.980039920159681');
+      assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT).to_text(), '9.980039920159681');
 
       done();
     });
@@ -1270,10 +1274,10 @@ describe('OrderBook', function() {
       assert.notStrictEqual(previousFunds, newFunds);
       switch (++receivedFundsChangedEvents) {
         case 1:
-          assert(!offer.is_fully_funded);
+          assert(offer.is_fully_funded);
           break;
         case 2:
-          assert(offer.is_fully_funded);
+          assert(!offer.is_fully_funded);
           break;
       }
     });
@@ -1413,10 +1417,10 @@ describe('OrderBook', function() {
     assert.strictEqual(book.getOwnerOfferCount(addresses.THIRD_ACCOUNT), 1);
     assert.strictEqual(book.getOwnerOfferCount(addresses.FOURTH_ACCOUNT), 1);
 
-    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), '2006.015671538605');
+    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT).to_text(), '2006.015671538605');
-    assert.strictEqual(book.getOwnerFunds(addresses.OTHER_ACCOUNT), '24.01284027983332');
+    assert.strictEqual(book.getOwnerFunds(addresses.OTHER_ACCOUNT).to_text(), '24.01284027983332');
-    assert.strictEqual(book.getOwnerFunds(addresses.THIRD_ACCOUNT), '9053.294314019701');
+    assert.strictEqual(book.getOwnerFunds(addresses.THIRD_ACCOUNT).to_text(), '9053.294314019701');
-    assert.strictEqual(book.getOwnerFunds(addresses.FOURTH_ACCOUNT), '7229.594289344439');
+    assert.strictEqual(book.getOwnerFunds(addresses.FOURTH_ACCOUNT).to_text(), '7229.594289344439');
   });
 
   it('Set offers - issuer transfer rate set - iou/xrp - funded amounts', function() {
@@ -1510,44 +1514,20 @@ describe('OrderBook', function() {
 
     assert.strictEqual(book._offers.length, 5);
 
-    var accountOfferTotal = Amount.from_json({
+    assert.strictEqual(book.getOwnerOfferTotal(addresses.ACCOUNT).to_text(), '275.85192574');
-      value: 275.85192574,
+    assert.strictEqual(book.getOwnerOfferTotal(addresses.OTHER_ACCOUNT).to_text(), '24.060765960393');
-      currency: 'USD',
+    assert.strictEqual(book.getOwnerOfferTotal(addresses.THIRD_ACCOUNT).to_text(), '712.60995');
-      issuer: addresses.ISSUER
+    assert.strictEqual(book.getOwnerOfferTotal(addresses.FOURTH_ACCOUNT).to_text(), '288.08');
-    });
-
-    var otherAccountOfferTotal = Amount.from_json({
-      value: 24.060765960393,
-      currency: 'USD',
-      issuer: addresses.ISSUER
-    });
-
-    var thirdAccountOfferTotal = Amount.from_json({
-      value: 712.60995,
-      currency: 'USD',
-      issuer: addresses.ISSUER
-    });
-
-    var fourthAccountOfferTotal = Amount.from_json({
-      value: 288.08,
-      currency: 'USD',
-      issuer: addresses.ISSUER
-    });
-
-    assert(book.getOwnerOfferTotal(addresses.ACCOUNT).equals(accountOfferTotal));
-    assert(book.getOwnerOfferTotal(addresses.OTHER_ACCOUNT).equals(otherAccountOfferTotal));
-    assert(book.getOwnerOfferTotal(addresses.THIRD_ACCOUNT).equals(thirdAccountOfferTotal));
-    assert(book.getOwnerOfferTotal(addresses.FOURTH_ACCOUNT).equals(fourthAccountOfferTotal));
 
     assert.strictEqual(book.getOwnerOfferCount(addresses.ACCOUNT), 2);
     assert.strictEqual(book.getOwnerOfferCount(addresses.OTHER_ACCOUNT), 1);
     assert.strictEqual(book.getOwnerOfferCount(addresses.THIRD_ACCOUNT), 1);
     assert.strictEqual(book.getOwnerOfferCount(addresses.FOURTH_ACCOUNT), 1);
 
-    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), '2006.015671538605');
+    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT).to_text(), '2006.015671538605');
-    assert.strictEqual(book.getOwnerFunds(addresses.OTHER_ACCOUNT), '24.01284027983332');
+    assert.strictEqual(book.getOwnerFunds(addresses.OTHER_ACCOUNT).to_text(), '24.01284027983332');
-    assert.strictEqual(book.getOwnerFunds(addresses.THIRD_ACCOUNT), '9053.294314019701');
+    assert.strictEqual(book.getOwnerFunds(addresses.THIRD_ACCOUNT).to_text(), '9053.294314019701');
-    assert.strictEqual(book.getOwnerFunds(addresses.FOURTH_ACCOUNT), '7229.594289344439');
+    assert.strictEqual(book.getOwnerFunds(addresses.FOURTH_ACCOUNT).to_text(), '7229.594289344439');
   });
 
   it.skip('Notify - created node', function() {
@@ -1573,16 +1553,10 @@ describe('OrderBook', function() {
 
     book.notify(message);
 
-    var accountOfferTotal = Amount.from_json({
-      value: 1.9951,
-      currency: 'USD',
-      issuer: addresses.ISSUER
-    });
-
     assert.strictEqual(book._offers.length, 1);
-    assert(book.getOwnerOfferTotal(addresses.ACCOUNT).equals(accountOfferTotal));
+    assert.strictEqual(book.getOwnerOfferTotal(addresses.ACCOUNT).to_text(), '1.9951');
     assert.strictEqual(book.getOwnerOfferCount(addresses.ACCOUNT), 1);
-    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), '2006.015671538605');
+    assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT).to_text(), '2006.015671538605');
   });
 
   it('Notify - created nodes - correct sorting', function() {
@@ -1771,7 +1745,7 @@ describe('OrderBook', function() {
       currency_pays: 'XRP'
     });
 
-    book.on('trade', function(tradeGets, tradePays) {
+    book.on('trade', function(tradePays, tradeGets) {
       var expectedTradePays = Amount.from_json(fixtures.TAKER_PAYS);
       var expectedTradeGets = Amount.from_json({
         value: fixtures.TAKER_GETS,
@@ -2104,6 +2078,30 @@ describe('OrderBook', function() {
     assert.strictEqual(book._offers[0].taker_pays_funded, '881086106');
   });
 
+  it('Insert offer - best quality', function() {
+    var remote = new Remote();
+    var book = remote.createOrderBook({
+      currency_gets: 'USD',
+      issuer_gets: addresses.ISSUER,
+      currency_pays: 'XRP'
+    });
+
+    book._subscribed = true;
+    book._issuerTransferRate = 1000000000;
+
+    book.setOffers(fixtures.QUALITY_OFFERS);
+
+    book.insertOffer(fixtures.transactionWithCreatedOffer({
+      amount: '51.04587961502088'
+    }).mmeta.getNodes()[0]);
+    
+    assert.strictEqual(book._offers.length, 2);
+
+    assert.strictEqual(book._offers[0].taker_gets_funded, '51.04587961502088');
+    assert.strictEqual(book._offers[0].taker_pays_funded, fixtures.TAKER_PAYS);
+    assert.strictEqual(book._offers[0].quality, '75977580.74206542');
+  });
+
   it('Insert offer - best quality - insufficient funds for all offers', function() {
     var remote = new Remote();
     var book = remote.createOrderBook({
@@ -2275,7 +2273,8 @@ describe('OrderBook', function() {
         owner_funds: '0.1129267125000245',
         taker_gets_funded: '0.112701309880264',
         taker_pays_funded: '55.95620035555106',
-        is_fully_funded: false
+        is_fully_funded: false,
+        quality: '496.4999999999999'
       },
       {
         Account: addresses.OTHER_ACCOUNT,
@@ -2300,7 +2299,8 @@ describe('OrderBook', function() {
         owner_funds: '0.950363009783092',
         is_fully_funded: true,
         taker_gets_funded: '0.2',
-        taker_pays_funded: '99.72233516476456'
+        taker_pays_funded: '99.72233516476456',
+        quality: '498.6116758238228'
       },
       { 
         Account: addresses.THIRD_ACCOUNT,
@@ -2324,7 +2324,8 @@ describe('OrderBook', function() {
         owner_funds: '0.950363009783092',
         is_fully_funded: true,
         taker_gets_funded: '0.5',
-        taker_pays_funded: '99.72233516476456' 
+        taker_pays_funded: '99.72233516476456',
+        quality: '498.6116758238228'
       },
       { 
         Account: addresses.THIRD_ACCOUNT,
@@ -2349,7 +2350,8 @@ describe('OrderBook', function() {
         owner_funds: '0.950363009783092',
         is_fully_funded: false,
         taker_gets_funded: '0.4484660776278363',
-        taker_pays_funded: '89.44416900646082'
+        taker_pays_funded: '89.44416900646082',
+        quality: '199.4446703295291'
       }
     ];
 
@@ -2386,7 +2388,8 @@ describe('OrderBook', function() {
         owner_funds: '600',
         is_fully_funded: false,
         taker_gets_funded: '600',
-        taker_pays_funded: '33.63983796370414'
+        taker_pays_funded: '33.6398379637041',
+        quality: '.0560663966061735'
       },
       {
         Account: addresses.OTHER_ACCOUNT,
@@ -2407,7 +2410,8 @@ describe('OrderBook', function() {
         owner_funds: '4000',
         is_fully_funded: true,
         taker_gets_funded: '2000',
-        taker_pays_funded: '99.72233516476456'
+        taker_pays_funded: '99.72233516476456',
+        quality: '0.049861167582382'
       },
       {
         Account: addresses.THIRD_ACCOUNT,
@@ -2428,7 +2432,8 @@ describe('OrderBook', function() {
         owner_funds: '3900',
         is_fully_funded: true,
         taker_gets_funded: '2000',
-        taker_pays_funded: '99.72233516476456'
+        taker_pays_funded: '99.72233516476456',
+        quality: '0.049861167582382'
       },
       {
         Account: addresses.THIRD_ACCOUNT,
@@ -2448,8 +2453,9 @@ describe('OrderBook', function() {
         index: 'A437D85DF80D250F79308F2B613CF5391C7CF8EE9099BC4E553942651CD9FA86',
         is_fully_funded: false,
         taker_gets_funded: '1900',
-        taker_pays_funded: '94.73621840652633',
+        taker_pays_funded: '94.7362184065258',
-        owner_funds: '3900'
+        owner_funds: '3900',
+        quality: '0.049861167582382'
       }
     ];