Skip to content

Commit

Permalink
[WIP] Only emit model event after ledger Closed
Browse files Browse the repository at this point in the history
  • Loading branch information
@sublimator
sublimator committed Aug 26, 2015
1 parent e3787e0 commit ab0c915
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 11 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -39,7 +39,7 @@
"babel-eslint": "^4.0.5",
"babel-loader": "^5.3.2",
"coveralls": "~2.10.0",
"eslint": "^1.2.0",
"eslint": "^1.2.1",
"eslint-plugin-flowtype": "^1.0.0",
"eventemitter2": "^0.4.14",
"flow-bin": "^0.14",
Expand Down
42 changes: 35 additions & 7 deletions src/core/orderbook.js
View file
Expand Up @@ -38,9 +38,10 @@ function assertValidNumber(number, message) {
* @param {String} orderbook key
*/

function OrderBook(remote,
currencyGets, issuerGets, currencyPays, issuerPays,
key) {
/* eslint-disable max-len*/
// Otherwise it gets confused by the indentation!
function OrderBook(remote, currencyGets, issuerGets, currencyPays, issuerPays, key) {
/* eslint-enable max-len*/
EventEmitter.call(this);

const self = this;
Expand All @@ -61,6 +62,7 @@ function OrderBook(remote,
this._ownerFundsUnadjusted = {};
this._ownerFunds = {};
this._ownerOffersTotal = {};
this._ledgerLastNotified = NaN;

// We consider ourselves synced if we have a current
// copy of the offers, we are online and subscribed to updates
Expand Down Expand Up @@ -123,6 +125,13 @@ function OrderBook(remote,
self.updateFundedAmounts(transaction);
}

function ledgerClosed(message) {
if (message.ledger_index === self._ledgerLastNotified) {
// We had transaction notifications, so notify and recompute
self.notifyDirectOffersChanged();
}
}

this.on('newListener', function(event) {
listenersModified('add', event);
});
Expand All @@ -137,6 +146,7 @@ function OrderBook(remote,
self.resetCache();

self._remote.removeListener('transaction', updateFundedAmountsWrapper);
self._remote.removeListener('ledger_closed', ledgerClosed);
});

this._remote.once('prepare_subscribe', function() {
Expand All @@ -150,6 +160,8 @@ function OrderBook(remote,
});
});

this._remote.on('ledger_closed', ledgerClosed);

return this;
}

Expand Down Expand Up @@ -256,7 +268,7 @@ OrderBook.prototype.unsubscribe = function() {
* @param {Function} callback
*/

OrderBook.prototype.requestOffers = function(callback=function() {}) {
OrderBook.prototype.requestOffers = function(callback = function() {}) {
const self = this;

if (!this._shouldSubscribe) {
Expand Down Expand Up @@ -409,6 +421,7 @@ OrderBook.prototype.resetCache = function() {
this._ownerOffersTotal = {};
this._offerCounts = {};
this._synced = false;
this._ledgerLastNotified = NaN;
};

/**
Expand Down Expand Up @@ -841,11 +854,13 @@ OrderBook.prototype.updateOwnerOffersFundedAmount = function(account) {
/**
* Notify orderbook of a relevant transaction
*
* @param {Object} transaction
* @param {Object} transaction - {"type" : "transaction", ...} notification
* message with {Meta} `mmeta` property
* @api private
*/

OrderBook.prototype.notify = function(transaction) {
OrderBook.prototype.notify = function(transaction, options = {}) {
const bufferNotifications = options.bufferNotifications !== false;
const self = this;

if (!(this._subscribed && this._synced)) {
Expand Down Expand Up @@ -919,7 +934,20 @@ OrderBook.prototype.notify = function(transaction) {
_.each(affectedNodes, handleNode);

this.emit('transaction', transaction);
this.notifyDirectOffersChanged();

if (bufferNotifications) {
// We'll wait until the ledger close message which always comes AFTER all
// the {"type: "transaction", ... } messages in a well specified (if not
// documented) order.
const ledger_index = transaction.ledger_index;
assert(ledger_index !== undefined,
'transaction notification message had no `ledger_index`');
this._ledgerLastNotified = ledger_index;
} else {
this.notifyDirectOffersChanged();
}
//

if (!takerGetsTotal.is_zero()) {
this.emit('trade', takerPaysTotal, takerGetsTotal);
}
Expand Down
8 changes: 7 additions & 1 deletion test/fixtures/orderbook.js
View file
@@ -1,4 +1,4 @@
/*eslint-disable max-len, no-param-reassign*/
/* eslint-disable max-len, no-param-reassign*/

'use strict';

Expand Down Expand Up @@ -843,6 +843,7 @@ module.exports.transactionWithCreatedOffer = function(options) {

return {
mmeta: meta,
ledger_index: 2400000,
transaction: {
TransactionType: 'OfferCreate',
owner_funds: '2010.027702881682'
Expand Down Expand Up @@ -888,6 +889,7 @@ module.exports.transactionWithDeletedOffer = function(options) {

return {
mmeta: meta,
ledger_index: 2400000,
transaction: {
TransactionType: options.transaction_type,
owner_funds: '2010.027702881682'
Expand Down Expand Up @@ -933,6 +935,7 @@ module.exports.transactionWithDeletedOfferR = function(options) {

return {
mmeta: meta,
ledger_index: 2400000,
transaction: {
TransactionType: options.transaction_type,
owner_funds: '2010.027702881682'
Expand All @@ -949,6 +952,7 @@ module.exports.transactionWithModifiedOffer = function() {

return {
mmeta: meta,
ledger_index: 2400000,
transaction: {
TransactionType: 'OfferCreate',
owner_funds: '2010.027702881682'
Expand All @@ -965,6 +969,7 @@ module.exports.transactionWithModifiedOffers = function() {

return {
mmeta: meta,
ledger_index: 2400000,
transaction: {
TransactionType: 'OfferCreate',
owner_funds: '2010.027702881682'
Expand All @@ -981,6 +986,7 @@ module.exports.transactionWithNoNodes = function() {

return {
mmeta: meta,
ledger_index: 2400000,
transaction: {
TransactionType: 'OfferCreate',
owner_funds: '2010.027702881682'
Expand Down
23 changes: 21 additions & 2 deletions test/orderbook-test.js
View file
@@ -1,4 +1,4 @@
/*eslint-disable max-len */
/* eslint-disable max-len */

'use strict';

Expand All @@ -10,6 +10,10 @@ const Meta = require('ripple-lib').Meta;
const addresses = require('./fixtures/addresses');
const fixtures = require('./fixtures/orderbook');

function ledgerClosed(book, ledger_index) {
book._remote.emit('ledger_closed', {ledger_index});
}

describe('OrderBook', function() {
this.timeout(0);

Expand Down Expand Up @@ -1594,6 +1598,7 @@ describe('OrderBook', function() {
book.notify(offer);
book.notify(lowQualityOffer);
book.notify(highQualityOffer);
ledgerClosed(book, offer.ledger_index);

assert.strictEqual(book._offers.length, 3);
assert.strictEqual(book._offers[0].Account, addresses.THIRD_ACCOUNT);
Expand Down Expand Up @@ -1636,9 +1641,12 @@ describe('OrderBook', function() {
book.notify(offer);
book.notify(offer2);
book.notify(offer3);
assert.strictEqual(book._ledgerLastNotified, offer.ledger_index);

ledgerClosed(book, offer.ledger_index);

assert.strictEqual(numTransactionEvents, 3);
assert.strictEqual(numModelEvents, 3);
assert.strictEqual(numModelEvents, 1);
assert.strictEqual(numOfferAddedEvents, 3);
});

Expand All @@ -1659,6 +1667,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithDeletedOffer();

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(book._offers.length, 2);
assert.strictEqual(book.getOwnerOfferTotal(addresses.ACCOUNT).to_text(), '4.9656112525');
Expand All @@ -1682,6 +1691,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithDeletedOffer();

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(book._offers.length, 0);
assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), undefined);
Expand Down Expand Up @@ -1724,6 +1734,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithDeletedOffer();

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(numTransactionEvents, 1);
assert.strictEqual(numModelEvents, 1);
Expand Down Expand Up @@ -1761,6 +1772,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithDeletedOffer();

book.notify(message);
ledgerClosed(book, message.ledger_index);
});

it('Notify - deleted node - offer cancel', function() {
Expand All @@ -1782,6 +1794,7 @@ describe('OrderBook', function() {
});

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(book._offers.length, 2);
assert.strictEqual(book.getOwnerOfferTotal(addresses.ACCOUNT).to_text(), '4.9656112525');
Expand All @@ -1807,6 +1820,7 @@ describe('OrderBook', function() {
});

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(book._offers.length, 0);
assert.strictEqual(book.getOwnerFunds(addresses.ACCOUNT), undefined);
Expand All @@ -1829,6 +1843,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithModifiedOffer();

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(book._offers.length, 3);
assert.strictEqual(book.getOwnerOfferTotal(addresses.ACCOUNT).to_text(), '23.8114145625');
Expand Down Expand Up @@ -1880,6 +1895,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithModifiedOffer();

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(numTransactionEvents, 1);
assert.strictEqual(numModelEvents, 1);
Expand Down Expand Up @@ -1917,6 +1933,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithModifiedOffer();

book.notify(message);
ledgerClosed(book, message.ledger_index);
});

it('Notify - modified nodes - trade', function(done) {
Expand Down Expand Up @@ -1949,6 +1966,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithModifiedOffers();

book.notify(message);
ledgerClosed(book, message.ledger_index);
});

it('Notify - no nodes', function() {
Expand Down Expand Up @@ -1988,6 +2006,7 @@ describe('OrderBook', function() {
const message = fixtures.transactionWithNoNodes();

book.notify(message);
ledgerClosed(book, message.ledger_index);

assert.strictEqual(numTransactionEvents, 0);
assert.strictEqual(numModelEvents, 0);
Expand Down

0 comments on commit ab0c915

Please sign in to comment.