Skip to content

Commit

Permalink
Fix serializedobject append for excessively large bytes length
Browse files Browse the repository at this point in the history
  • Loading branch information
wltsmrz committed May 21, 2015
1 parent 6ea0713 commit f487378
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 15 deletions.
17 changes: 11 additions & 6 deletions src/serializedobject.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,18 @@ SerializedObject.check_fields = function(typedef, obj) {
SerializedObject.prototype.append = function(bytes_) {
const bytes = bytes_ instanceof SerializedObject ? bytes_.buffer : bytes_;

// Make sure both buffer and bytes are Array. Either could potentially be a
// Buffer.
// Make sure both buffer and bytes are Array. Either could be a Buffer.
if (Array.isArray(this.buffer) && Array.isArray(bytes)) {
// Array::concat is horribly slow where buffer length is 100 kbytes + One
// transaction with 1100 affected nodes took around 23 seconds to convert
// from json to bytes.
Array.prototype.push.apply(this.buffer, bytes);
// `this.buffer = this.buffer.concat(bytes)` can be unbearably slow for
// large bytes length and acceptable bytes length is limited for
// `Array.prototype.push.apply(this.buffer, bytes)` as every element in the
// bytes array is pushed onto the stack, potentially causing a RangeError
// exception. Both of these solutions are known to be problematic for
// ledger 7501326. KISS instead

for (let i = 0; i < bytes.length; i++) {
this.buffer.push(bytes[i]);
}
} else {
this.buffer = this.buffer.concat(bytes);
}
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/ledger-full-7501326.json

Large diffs are not rendered by default.

22 changes: 13 additions & 9 deletions test/ledger-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/* eslint-disable max-len */
/* eslint-disable max-len, valid-jsdoc */
'use strict';

const assert = require('assert');
const lodash = require('lodash');
const fs = require('fs');

const Ledger = require('ripple-lib').Ledger;
Expand All @@ -10,22 +11,23 @@ const Ledger = require('ripple-lib').Ledger;
* @param ledger_index {Number}
* Expects a corresponding ledger dump in $repo/test/fixtures/ folder
*/

function create_ledger_test(ledger_index) {
describe(String(ledger_index), function() {

const path = __dirname + '/fixtures/ledger-full-' + ledger_index + '.json';

const ledger_raw = fs.readFileSync(path);
const ledger_json = JSON.parse(ledger_raw);
const ledger = Ledger.from_json(ledger_json);

it('has account_hash of ' + ledger_json.account_hash, function() {
assert.equal(ledger_json.account_hash,
ledger.calc_account_hash({
sanity_test: true
}).to_hex());
});
const hasAccounts = Array.isArray(ledger_json.accountState)
&& ledger_json.accountState.length > 0;

if (hasAccounts) {
it('has account_hash of ' + ledger_json.account_hash, function() {
assert.equal(ledger_json.account_hash,
ledger.calc_account_hash({sanity_test: true}).to_hex());
});
}
it('has transaction_hash of ' + ledger_json.transaction_hash, function() {
assert.equal(ledger_json.transaction_hash,
ledger.calc_tx_hash().to_hex());
Expand All @@ -37,6 +39,8 @@ describe('Ledger', function() {
create_ledger_test(38129);
// Because, why not.
create_ledger_test(40000);
// 1311 AffectedNodes, no accounts
create_ledger_test(7501326);

describe('#calcAccountRootEntryHash', function() {
it('will calculate the AccountRoot entry hash for rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh', function() {
Expand Down

0 comments on commit f487378

Please sign in to comment.