From 05e5c37230faebe3748e62b66b4ececa5ec0a78c Mon Sep 17 00:00:00 2001 From: William Wolf Date: Mon, 16 Feb 2015 16:36:04 -0800 Subject: [PATCH 1/6] The 'fRelay' param on Version requests is optional. I'v seen `RangeError: index out of range` crashes from nodes that don't send it, when `bitcore-p2p` tries to `readUInt8()` the last bit. This also adds `relay` property to `Peer` objects that is respected when sending `Version` messages. --- lib/messages.js | 11 +++++++++-- lib/peer.js | 5 +++-- test/data/messages.json | 3 +++ test/messages.js | 15 +++++++++++++++ test/peer.js | 23 ++++++++++++++++++++++- 5 files changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/messages.js b/lib/messages.js index 05d6d1e..2fcf9c0 100644 --- a/lib/messages.js +++ b/lib/messages.js @@ -186,12 +186,13 @@ module.exports.Message = Message; * @param{string} subversion - version of the client * @param{Buffer} nonce - a random 8 bytes buffer */ -function Version(subversion, nonce) { +function Version(subversion, nonce, relay) { var packageInfo = require('../package.json'); this.command = 'version'; this.version = PROTOCOL_VERSION; this.subversion = subversion || '/bitcore:' + packageInfo.version + '/'; this.nonce = nonce || CONNECTION_NONCE; + this.relay = relay === false ? false : true; } util.inherits(Version, Message); @@ -257,7 +258,12 @@ Version.prototype.fromBuffer = function(payload) { * @desc Whether the remote peer should announce relayed transactions or not, see BIP 0037 * @type {boolean} */ - this.relay = !!parser.readUInt8(); + // This field is optional, so should not always be read + if(parser.finished()) { + this.relay = true; + } else { + this.relay = !!parser.readUInt8(); + } this._checkFinished(parser); return this; @@ -274,6 +280,7 @@ Version.prototype.getPayload = function() { put.varint(this.subversion.length); put.put(new Buffer(this.subversion, 'ascii')); put.word32le(this.start_height); + put.word8(this.relay); return put.buffer(); }; diff --git a/lib/peer.js b/lib/peer.js index 908f7b4..435fbbb 100644 --- a/lib/peer.js +++ b/lib/peer.js @@ -33,7 +33,7 @@ var MAX_RECEIVE_BUFFER = 10000000; * @returns {Peer} A new instance of Peer. * @constructor */ -function Peer(host, port, network) { +function Peer(host, port, network, relay) { if (!(this instanceof Peer)) { return new Peer(host, port, network); } @@ -48,6 +48,7 @@ function Peer(host, port, network) { this.status = Peer.STATUS.DISCONNECTED; this.network = network || Networks.defaultNetwork; this.port = port || this.network.port; + this.relay = relay === false ? false : true; this.dataBuffer = new Buffers(); @@ -161,7 +162,7 @@ Peer.prototype.sendMessage = function(message) { * Internal function that sends VERSION message to the remote peer. */ Peer.prototype._sendVersion = function() { - var message = new Messages.Version(); + var message = new Messages.Version(null, null, this.relay); this.sendMessage(message); }; diff --git a/test/data/messages.json b/test/data/messages.json index 21728f0..8bbb310 100644 --- a/test/data/messages.json +++ b/test/data/messages.json @@ -3,6 +3,9 @@ "message": "f9beb4d976657273696f6e000000000065000000fc970f17721101000100000000000000ba62885400000000010000000000000000000000000000000000ffffba8886dceab0010000000000000000000000000000000000ffff05095522208de7e1c1ef80a1cea70f2f5361746f7368693a302e392e312fa317050001", "payload": "721101000100000000000000ba62885400000000010000000000000000000000000000000000ffffba8886dceab0010000000000000000000000000000000000ffff05095522208de7e1c1ef80a1cea70f2f5361746f7368693a302e392e312fa317050001" }, + "VERSION_NO_FRELAY": { + "payload": "701101000100000000000000d78be25400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008f7e557f1b892912102f626974636f72653a302e31302e302f00000000" + }, "ALERT": { "message": "", "payload": "73010000003766404f00000000b305434f00000000f2030000f1030000001027000048ee00000064000000004653656520626974636f696e2e6f72672f666562323020696620796f7520686176652074726f75626c6520636f6e6e656374696e67206166746572203230204665627275617279004730450221008389df45f0703f39ec8c1cc42c13810ffcae14995bb648340219e353b63b53eb022009ec65e1c1aaeec1fd334c6b684bde2b3f573060d5b70c3a46723326e4e8a4f1" diff --git a/test/messages.js b/test/messages.js index 204bfaf..ee260c3 100644 --- a/test/messages.js +++ b/test/messages.js @@ -113,7 +113,22 @@ describe('Messages', function() { clazz.forBlock(BufferUtils.reverse(new Buffer(hash, 'hex'))).should.deep.equal(b); clazz.forTransaction(BufferUtils.reverse(new Buffer(hash, 'hex'))).should.deep.equal(t); }); + }); + it('Version#fromBuffer works w/o fRelay arg', function() { + var messageHex = Data.VERSION_NO_FRELAY.payload; + var message = new Messages.Version() + .fromBuffer(new Buffer(messageHex, 'hex')); + }); + + it('Version#relay setting works', function() { + [true,false].forEach(function(relay) { + var message = new Messages.Version(null,null,relay); + message.relay.should.equal(relay); + var messageBuf = message.getPayload(); + var newMessage = new Messages.Version().fromBuffer(messageBuf) + newMessage.relay.should.equal(relay); + }); }); }); diff --git a/test/peer.js b/test/peer.js index d02a3d6..30a2aae 100644 --- a/test/peer.js +++ b/test/peer.js @@ -20,7 +20,7 @@ describe('Peer', function() { describe('Integration test', function() { it('parses this stream of data from a connection', function(callback) { - var peer = new p2p.Peer(''); + var peer = new Peer(''); var stub = sinon.stub(); var dataCallback; var connectCallback; @@ -120,4 +120,25 @@ describe('Peer', function() { peer.should.equal(peer2); }); + + it('Peer.relay setting set properly', function() { + var peer = new Peer('localhost'); + peer.relay.should.equal(true); + var peer2 = new Peer('localhost', null, null, false); + peer2.relay.should.equal(false); + var peer3 = new Peer('localhost', null, null, true); + peer3.relay.should.equal(true); + }); + + it('Peer.relay setting respected', function() { + [true,false].forEach(function(relay) { + var peer = new Peer('localhost', null, null, relay); + var peerSendMessageStub = sinon.stub(Peer.prototype, 'sendMessage', function(message) { + message.relay.should.equal(relay); + }); + peer._sendVersion(); + peerSendMessageStub.restore(); + }); + }); + }); From 63ad4e543828ddbed5872c5ddbca684f709397cf Mon Sep 17 00:00:00 2001 From: William Wolf Date: Wed, 18 Feb 2015 12:02:00 -0800 Subject: [PATCH 2/6] Propogate 'relay' from Pool() to its Peer()s --- lib/pool.js | 27 +++++++++++---------------- test/pool.js | 28 ++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/pool.js b/lib/pool.js index 5dc521b..ad5873b 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -30,6 +30,7 @@ function now() { * ``` * * @param {Network|String} network - The network to connect + * @param {Object} options - Options object * @returns {Pool} * @constructor */ @@ -37,27 +38,21 @@ function Pool(network, options) { var self = this; + options = options || {}; this.network = Networks.get(network) || Networks.defaultNetwork; this.keepalive = false; this._connectedPeers = {}; this._addrs = []; - this.listenAddr = true; - this.dnsSeed = true; + this.listenAddr = options.listenAddr === false ? false : true; + this.dnsSeed = options.dnsSeed === false ? false : true; + this.relay = options.relay === false ? false : true; + this.size = options.size || Pool.MaxConnectedPeers; - // define configuration options - if (options) { - if (options.listenAddr === false) { - this.listenAddr = false; - } - if (options.dnsSeed === false) { - this.dnsSeed = false; - } - if (options.addrs) { - for(var i = 0; i < options.addrs.length; i++) { - this._addAddr(options.addrs[i]); - } + if (options.addrs) { + for(var i = 0; i < options.addrs.length; i++) { + this._addAddr(options.addrs[i]); } } @@ -152,7 +147,7 @@ Pool.prototype.numberConnected = function numberConnected() { Pool.prototype._fillConnections = function _fillConnections() { var length = this._addrs.length; for (var i = 0; i < length; i++) { - if (this.numberConnected() >= Pool.MaxConnectedPeers) { + if (this.numberConnected() >= this.size) { break; } var addr = this._addrs[i]; @@ -186,7 +181,7 @@ Pool.prototype._connectPeer = function _connectPeer(addr) { function addConnectedPeer(addr) { var port = addr.port || self.network.port; var ip = addr.ip.v4 || addr.ip.v6; - var peer = new Peer(ip, port, self.network); + var peer = new Peer(ip, port, self.network, self.relay); peer.on('disconnect', function peerDisconnect() { self.emit('peerdisconnect', peer, addr); }); diff --git a/test/pool.js b/test/pool.js index 097f74b..ff36399 100644 --- a/test/pool.js +++ b/test/pool.js @@ -194,12 +194,8 @@ describe('Pool', function() { this.emit('disconnect', this, {}); }); - var pool = new Pool(); - pool._addAddr({ - ip: { - v4: 'localhost' - } - }); + var pool = new Pool(null, { size: 1 }); + pool._addAddr({ ip: { v4: 'localhost' } }); // Not great, but needed so pool won't catch its on event and fail the test pool.removeAllListeners('peerdisconnect'); @@ -227,4 +223,24 @@ describe('Pool', function() { pool.connect(); }); + it('should propagate Pool.relay property to peers', function(done) { + var count = 0; + var peerConnectStub = sinon.stub(Peer.prototype, 'connect', function() { + this.emit('connect', this, {}); + }); + [true, false].forEach(function(relay) { + var pool = new Pool(null,{ relay: relay, size: 1, dnsSeed: false }); + pool._addAddr({ ip: { v4: 'localhost' } }); + pool.on('peerconnect', function(peer, addr) { + peer.relay.should.equal(relay); + pool.disconnect(); + if(++count == 2) { + done(); + } + }); + pool.connect(); + }); + peerConnectStub.restore(); + }); + }); From 47816940384b8af666f9d0b4099a76c522312338 Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Wed, 18 Feb 2015 19:04:01 -0500 Subject: [PATCH 3/6] Added jsdocs for options and changed size to maxSize --- lib/pool.js | 10 +++++++--- test/pool.js | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/pool.js b/lib/pool.js index ad5873b..6e58342 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -30,7 +30,11 @@ function now() { * ``` * * @param {Network|String} network - The network to connect - * @param {Object} options - Options object + * @param {Object} [options] - Options object + * @param {Boolean} [options.listenAddr=true] - Prevent new peers being added from addr messages + * @param {Boolean} [options.dnsSeed=true] - Prevent seeding with DNS discovered known peers + * @param {Boolean} [options.relay=true] - Prevent inventory announcements until a filter is loaded + * @param {Number} [options.maxSize=Pool.MaxConnectedPeers] - The max number of peers * @returns {Pool} * @constructor */ @@ -48,7 +52,7 @@ function Pool(network, options) { this.listenAddr = options.listenAddr === false ? false : true; this.dnsSeed = options.dnsSeed === false ? false : true; this.relay = options.relay === false ? false : true; - this.size = options.size || Pool.MaxConnectedPeers; + this.maxSize = options.maxSize || Pool.MaxConnectedPeers; if (options.addrs) { for(var i = 0; i < options.addrs.length; i++) { @@ -147,7 +151,7 @@ Pool.prototype.numberConnected = function numberConnected() { Pool.prototype._fillConnections = function _fillConnections() { var length = this._addrs.length; for (var i = 0; i < length; i++) { - if (this.numberConnected() >= this.size) { + if (this.numberConnected() >= this.maxSize) { break; } var addr = this._addrs[i]; diff --git a/test/pool.js b/test/pool.js index ff36399..a5641b4 100644 --- a/test/pool.js +++ b/test/pool.js @@ -194,7 +194,7 @@ describe('Pool', function() { this.emit('disconnect', this, {}); }); - var pool = new Pool(null, { size: 1 }); + var pool = new Pool(null, { dnsSeed: false }); pool._addAddr({ ip: { v4: 'localhost' } }); // Not great, but needed so pool won't catch its on event and fail the test From d751e583b49e98894414deabcd64c39e56f997c2 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Wed, 18 Feb 2015 16:26:12 -0800 Subject: [PATCH 4/6] Remove 'size' from pool Test --- test/pool.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pool.js b/test/pool.js index a5641b4..14e29fb 100644 --- a/test/pool.js +++ b/test/pool.js @@ -229,7 +229,7 @@ describe('Pool', function() { this.emit('connect', this, {}); }); [true, false].forEach(function(relay) { - var pool = new Pool(null,{ relay: relay, size: 1, dnsSeed: false }); + var pool = new Pool(null,{ relay: relay, dnsSeed: false }); pool._addAddr({ ip: { v4: 'localhost' } }); pool.on('peerconnect', function(peer, addr) { peer.relay.should.equal(relay); From e579182e6de494b56929559edc346e4ca433e8a8 Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Wed, 18 Feb 2015 20:21:21 -0500 Subject: [PATCH 5/6] Increased test coverage for Pool --- test/pool.js | 69 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/test/pool.js b/test/pool.js index 14e29fb..b498d9a 100644 --- a/test/pool.js +++ b/test/pool.js @@ -49,10 +49,16 @@ describe('Pool', function() { }); var options = { dnsSeed: false, + maxSize: 1, addrs: [ { ip: { - v4: '10.10.10.1' + v4: 'localhost' + } + }, + { + ip: { + v4: 'localhost2' } } ] @@ -60,7 +66,7 @@ describe('Pool', function() { var pool = new Pool(Networks.livenet, options); pool.connect(); pool.disconnect(); - pool._addrs.length.should.equal(1); + pool._addrs.length.should.equal(2); stub.restore(); }); @@ -70,18 +76,13 @@ describe('Pool', function() { addrs: [ { ip: { - v4: '10.10.10.1' - } - }, - { - ip: { - v4: '10.10.10.245' + v4: 'localhost' } } ] }; var pool = new Pool(Networks.livenet, options); - pool._addrs.length.should.equal(2); + pool._addrs.length.should.equal(1); }); it('should add new addrs as they are announced over the network', function(done) { @@ -193,12 +194,18 @@ describe('Pool', function() { var peerDisconnectStub = sinon.stub(Peer.prototype, 'disconnect', function() { this.emit('disconnect', this, {}); }); + var poolRemoveStub = sinon.stub(Pool.prototype, '_removeConnectedPeer', function() {}); - var pool = new Pool(null, { dnsSeed: false }); - pool._addAddr({ ip: { v4: 'localhost' } }); - - // Not great, but needed so pool won't catch its on event and fail the test - pool.removeAllListeners('peerdisconnect'); + var pool = new Pool(null, { + dnsSeed: false, + addrs: [ + { + ip: { + v4: 'localhost' + } + } + ] + }); var poolDisconnectStub; pool.on('peerconnect', function(peer, addr) { @@ -215,6 +222,7 @@ describe('Pool', function() { peerConnectStub.restore(); peerDisconnectStub.restore(); poolDisconnectStub.restore(); + poolRemoveStub.restore(); // done done(); @@ -243,4 +251,37 @@ describe('Pool', function() { peerConnectStub.restore(); }); + it('should output the console correctly', function() { + var pool = new Pool(); + pool.inspect().should.equal(''); + }); + + it('should emit seederrors with error', function(done) { + var dnsStub = sinon.stub(dns, 'resolve', function(seed, callback) { + callback(new Error('A DNS error')); + }); + var pool = new Pool(Networks.livenet, {maxSize: 1}); + pool.once('seederror', function(error) { + should.exist(error); + pool.disconnect(); + dnsStub.restore(); + done(); + }); + pool.connect(); + }); + + it('should emit seederrors with notfound', function(done) { + var dnsStub = sinon.stub(dns, 'resolve', function(seed, callback) { + callback(null, []); + }); + var pool = new Pool(Networks.livenet, {maxSize: 1}); + pool.once('seederror', function(error) { + should.exist(error); + pool.disconnect(); + dnsStub.restore(); + done(); + }); + pool.connect(); + }); + }); From e1f2fbb7ed6a76dfd933e56ea897edce05a48106 Mon Sep 17 00:00:00 2001 From: William Wolf Date: Wed, 18 Feb 2015 22:49:04 -0800 Subject: [PATCH 6/6] Small Pool.options defaults cleanup --- lib/pool.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pool.js b/lib/pool.js index 6e58342..4f36e44 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -49,9 +49,9 @@ function Pool(network, options) { this._connectedPeers = {}; this._addrs = []; - this.listenAddr = options.listenAddr === false ? false : true; - this.dnsSeed = options.dnsSeed === false ? false : true; - this.relay = options.relay === false ? false : true; + this.listenAddr = options.listenAddr !== false; + this.dnsSeed = options.dnsSeed !== false; + this.relay = options.relay !== false; this.maxSize = options.maxSize || Pool.MaxConnectedPeers; if (options.addrs) {