From 11bee8b900a2fa1881559f865b23bc4e6da849f8 Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Wed, 11 Mar 2015 23:34:50 -0400 Subject: [PATCH] Improved API: - Renamed "Commands" to "builder" - "Messages.parseMessage" to "Messages.parseBuffer" - Changed to use private methods for "discardUntilNextMessage" and "buildFromBuffer" - Cleaned up tests --- lib/messages/{commands.js => builder.js} | 20 +++++++++++--- lib/messages/index.js | 33 +++++++++-------------- lib/peer.js | 2 +- test/bloomfilter.js | 8 +++--- test/inventory.js | 18 ++++++------- test/messages/{commands.js => builder.js} | 28 +++++++++---------- test/messages/{messages.js => index.js} | 17 +++++------- test/messages/message.js | 6 ++--- test/peer.js | 16 +++++------ test/pool.js | 24 ++++++++--------- 10 files changed, 88 insertions(+), 84 deletions(-) rename lib/messages/{commands.js => builder.js} (98%) rename test/messages/{commands.js => builder.js} (71%) rename test/messages/{messages.js => index.js} (83%) diff --git a/lib/messages/commands.js b/lib/messages/builder.js similarity index 98% rename from lib/messages/commands.js rename to lib/messages/builder.js index 1e34f35..860dad8 100644 --- a/lib/messages/commands.js +++ b/lib/messages/builder.js @@ -13,7 +13,7 @@ var Put = require('bufferput'); //todo remove var $ = bitcore.util.preconditions; var _ = bitcore.deps._; -function Commands(options) { +function builder(options) { /* jshint maxstatements: 150 */ /* jshint maxcomplexity: 10 */ @@ -33,6 +33,20 @@ function Commands(options) { var commands = {}; + var exported = { + constructors: { + Block: Block, + BlockHeader: BlockHeader, + Transaction: Transaction, + MerkleBlock: MerkleBlock + }, + defaults: { + protocolVersion: protocolVersion, + magicNumber: magicNumber + }, + commands: commands + }; + /* shared */ function checkFinished(parser) { @@ -890,8 +904,8 @@ function Commands(options) { return BufferUtil.EMPTY_BUFFER; }; - return commands; + return exported; } -module.exports = Commands; +module.exports = builder; diff --git a/lib/messages/index.js b/lib/messages/index.js index a833e73..8c68772 100644 --- a/lib/messages/index.js +++ b/lib/messages/index.js @@ -5,17 +5,10 @@ var BufferUtil = bitcore.util.buffer; var Hash = bitcore.crypto.Hash; function Messages(options) { - this.commands = new Messages.Commands(options); - + this.builder = Messages.builder(options); if (!options) { options = {}; } - - // map command messages - for (var key in this.commands) { - this[key] = this.commands[key]; - } - var defaultMagicNumber = bitcore.Networks.defaultNetwork.networkMagic.readUInt32LE(0); this.magicNumber = options.magicNumber || defaultMagicNumber; } @@ -23,16 +16,20 @@ function Messages(options) { Messages.MINIMUM_LENGTH = 20; Messages.PAYLOAD_START = 16; Messages.Message = require('./message'); -Messages.Commands = require('./commands'); +Messages.builder = require('./builder'); -Messages.prototype.parseMessage = function(dataBuffer) { +Messages.prototype.build = Messages.prototype.buildFromObject = function(command, object) { + return this.builder.commands[command].fromObject(object); +}; + +Messages.prototype.parseBuffer = function(dataBuffer) { /* jshint maxstatements: 18 */ if (dataBuffer.length < Messages.MINIMUM_LENGTH) { return; } // Search the next magic number - if (!this.discardUntilNextMessage(dataBuffer)) return; + if (!this._discardUntilNextMessage(dataBuffer)) return; var payloadLen = (dataBuffer.get(Messages.PAYLOAD_START)) + (dataBuffer.get(Messages.PAYLOAD_START + 1) << 8) + @@ -56,10 +53,10 @@ Messages.prototype.parseMessage = function(dataBuffer) { dataBuffer.skip(messageLength); - return this.buildFromBuffer(command, payload); + return this._buildFromBuffer(command, payload); }; -Messages.prototype.discardUntilNextMessage = function(dataBuffer) { +Messages.prototype._discardUntilNextMessage = function(dataBuffer) { var i = 0; for (;;) { // check if it's the beginning of a new message @@ -79,15 +76,11 @@ Messages.prototype.discardUntilNextMessage = function(dataBuffer) { } }; -Messages.prototype.buildFromBuffer = function(command, payload) { - if (!this.commands[command]) { +Messages.prototype._buildFromBuffer = function(command, payload) { + if (!this.builder.commands[command]) { throw new Error('Unsupported message command: ' + command); } - return this.commands[command].fromBuffer(payload); -}; - -Messages.prototype.build = Messages.prototype.buildFromObject = function(command, object) { - return this.commands[command].fromObject(object); + return this.builder.commands[command].fromBuffer(payload); }; module.exports = Messages; diff --git a/lib/peer.js b/lib/peer.js index 583e0c6..70e0050 100644 --- a/lib/peer.js +++ b/lib/peer.js @@ -183,7 +183,7 @@ Peer.prototype._sendPong = function(nonce) { * Internal function that tries to read a message from the data buffer */ Peer.prototype._readMessage = function() { - var message = this.messages.parseMessage(this.dataBuffer); + var message = this.messages.parseBuffer(this.dataBuffer); if (message) { this.emit(message.command, message); this._readMessage(); diff --git a/test/bloomfilter.js b/test/bloomfilter.js index 45c7ac5..aa9c410 100644 --- a/test/bloomfilter.js +++ b/test/bloomfilter.js @@ -26,7 +26,7 @@ function ParseHex(str) { describe('BloomFilter', function() { - it('BloomFilter#fromBuffer and toBuffer methods work', function() { + it('#fromBuffer and #toBuffer round trip', function() { var testPayloadBuffer = getPayloadBuffer(Data.filterload.message); var filter = new BloomFilter.fromBuffer(testPayloadBuffer); filter.toBuffer().should.deep.equal(testPayloadBuffer); @@ -34,7 +34,7 @@ describe('BloomFilter', function() { // test data from: https://github.com/bitcoin/bitcoin/blob/master/src/test/bloom_tests.cpp - it('correctly serialize filter with public keys added', function() { + it('serialize filter with public keys added', function() { var privateKey = bitcore.PrivateKey.fromWIF('5Kg1gnAjaLfKiwhhPpGS3QfRg2m6awQvaj98JCZBZQ5SuS2F15C'); var publicKey = privateKey.toPublicKey(); @@ -49,7 +49,7 @@ describe('BloomFilter', function() { }); - it('correctly serialize to a buffer', function() { + it('serialize to a buffer', function() { var filter = BloomFilter.create(3, 0.01, 0, BloomFilter.BLOOM_UPDATE_ALL); @@ -68,7 +68,7 @@ describe('BloomFilter', function() { actual.should.deep.equal(expected); }); - it('correctly deserialize a buffer', function() { + it('deserialize a buffer', function() { var buffer = new Buffer('03614e9b050000000000000001', 'hex'); var filter = BloomFilter.fromBuffer(buffer); diff --git a/test/inventory.js b/test/inventory.js index fde8969..ef9add8 100644 --- a/test/inventory.js +++ b/test/inventory.js @@ -20,12 +20,12 @@ describe('Inventory', function() { ); describe('@constructor', function() { - it('should create inventory', function() { + it('create inventory', function() { var inventory = new Inventory({type: Inventory.TYPE.TX, hash: hash}); should.exist(inventory); }); - it('should error with string hash', function() { + it('error with string hash', function() { (function() { var inventory = new Inventory({type: Inventory.TYPE.TX, hash: hashedStr}); should.not.exist(inventory); @@ -35,7 +35,7 @@ describe('Inventory', function() { }); describe('#forItem', function() { - it('should handle a string hash (reversed)', function() { + it('handle a string hash (reversed)', function() { var inventory = Inventory.forItem(Inventory.TYPE.TX, hashedStr); should.exist(inventory); inventory.hash.should.deep.equal(new Buffer(hash, 'hex')); @@ -44,7 +44,7 @@ describe('Inventory', function() { }); describe('#forBlock', function() { - it('should use correct block type', function() { + it('use correct block type', function() { var inventory = Inventory.forBlock(hash); should.exist(inventory); inventory.type.should.equal(Inventory.TYPE.BLOCK); @@ -52,7 +52,7 @@ describe('Inventory', function() { }); describe('#forFilteredBlock', function() { - it('should use correct filtered block type', function() { + it('use correct filtered block type', function() { var inventory = Inventory.forFilteredBlock(hash); should.exist(inventory); inventory.type.should.equal(Inventory.TYPE.FILTERED_BLOCK); @@ -60,7 +60,7 @@ describe('Inventory', function() { }); describe('#forTransaction', function() { - it('should use correct filtered tx type', function() { + it('use correct filtered tx type', function() { var inventory = Inventory.forTransaction(hash); should.exist(inventory); inventory.type.should.equal(Inventory.TYPE.TX); @@ -68,7 +68,7 @@ describe('Inventory', function() { }); describe('#toBuffer', function() { - it('should serialize correctly', function() { + it('serialize correctly', function() { var inventory = Inventory.forTransaction(hash); var buffer = inventory.toBuffer(); buffer.should.deep.equal(inventoryBuffer); @@ -76,7 +76,7 @@ describe('Inventory', function() { }); describe('#toBufferWriter', function() { - it('should write to a buffer writer', function() { + it('write to a buffer writer', function() { var bw = new BufferWriter(); var inventory = Inventory.forTransaction(hash); inventory.toBufferWriter(bw); @@ -85,7 +85,7 @@ describe('Inventory', function() { }); describe('#fromBuffer', function() { - it('should deserialize a buffer', function() { + it('deserialize a buffer', function() { var inventory = Inventory.fromBuffer(inventoryBuffer); should.exist(inventory); inventory.type.should.equal(Inventory.TYPE.TX); diff --git a/test/messages/commands.js b/test/messages/builder.js similarity index 71% rename from test/messages/commands.js rename to test/messages/builder.js index 2bb09ee..1a67c92 100644 --- a/test/messages/commands.js +++ b/test/messages/builder.js @@ -2,7 +2,7 @@ var should = require('chai').should(); var P2P = require('../../'); -var Commands = P2P.Messages.Commands; +var builder = P2P.Messages.builder; var commandData = require('../data/messages.json'); var Data = require('../data/messages');//todo merge with commandData var bitcore = require('bitcore'); @@ -11,39 +11,39 @@ function getPayloadBuffer(messageBuffer) { return new Buffer(messageBuffer.slice(48), 'hex'); } -describe('P2P Command Builder', function() { +describe('Messages Builder', function() { describe('@constructor', function() { it('should return commands based on default', function() { // instantiate - var commands = new Commands(); - should.exist(commands); + var b = builder(); + should.exist(b); }); it('should return commands with customizations', function() { // instantiate - var commands = new Commands({ + var b = builder({ magicNumber: 0xd9b4bef9, Block: bitcore.Block, Transaction: bitcore.Transaction }); - should.exist(commands); + should.exist(b); }); }); describe('Commands', function() { - var commands = new Commands(); + var b = builder(); describe('#fromBuffer/#toBuffer round trip for all commands', function() { - Object.keys(commands).forEach(function(command) { + Object.keys(b.commands).forEach(function(command) { - it('should round trip buffers for command: ' + command, function(done) { + it(command, function(done) { var payloadBuffer = getPayloadBuffer(commandData[command].message); - should.exist(commands[command]); - var message = commands[command].fromBuffer(payloadBuffer); + should.exist(b.commands[command]); + var message = b.commands[command].fromBuffer(payloadBuffer); var outputBuffer = message.getPayload(); outputBuffer.toString('hex').should.equal(payloadBuffer.toString('hex')); outputBuffer.should.deep.equal(payloadBuffer); @@ -58,16 +58,16 @@ describe('P2P Command Builder', function() { describe('version', function() { it('#fromBuffer works w/o fRelay arg', function() { var payloadBuffer = getPayloadBuffer(Data.version.messagenofrelay); - var message = commands.version.fromBuffer(payloadBuffer); + var message = b.commands.version.fromBuffer(payloadBuffer); message.relay.should.equal(true); }); it('#relay setting works', function() { [true,false].forEach(function(relay) { - var message = commands.version.fromObject({relay: relay}); + var message = b.commands.version.fromObject({relay: relay}); message.relay.should.equal(relay); var messageBuf = message.getPayload(); - var newMessage = commands.version.fromBuffer(messageBuf); + var newMessage = b.commands.version.fromBuffer(messageBuf); newMessage.relay.should.equal(relay); }); }); diff --git a/test/messages/messages.js b/test/messages/index.js similarity index 83% rename from test/messages/messages.js rename to test/messages/index.js index c01aa2c..44a6337 100644 --- a/test/messages/messages.js +++ b/test/messages/index.js @@ -25,24 +25,21 @@ describe('Messages', function() { Block: bitcore.Block, Transaction: bitcore.Transaction }); - should.exist(messages.commands); + should.exist(messages.builder.commands); + should.exist(messages.builder.constructors); + messages.builder.constructors.Block.should.equal(bitcore.Block); + messages.builder.constructors.Transaction.should.equal(bitcore.Transaction); messages.magicNumber.should.equal(magicNumber); - - // check that commands are mapped as messages - for(var key in messages.commands) { - messages[key].should.equal(messages.commands[key]); - } - }); }); - describe('#parseMessage', function() { + describe('#parseBuffer', function() { it('fails with invalid command', function() { var invalidCommand = 'f9beb4d96d616c6963696f757300000025000000bd5e830c' + '0102000000ec3995c1bf7269ff728818a65e53af00cbbee6b6eca8ac9ce7bc79d87' + '7041ed8'; var fails = function() { - messages.parseMessage(buildMessage(invalidCommand)); + messages.parseBuffer(buildMessage(invalidCommand)); }; fails.should.throw('Unsupported message command: malicious'); }); @@ -59,7 +56,7 @@ describe('Messages', function() { '00000069ebcbc34a4f9890da9aea0f773beba883a9afb1ab9ad7647dd4a1cd346c3' + '728'; [malformed1, malformed2, malformed3].forEach(function(malformed) { - var ret = messages.parseMessage(buildMessage(malformed)); + var ret = messages.parseBuffer(buildMessage(malformed)); should.not.exist(ret); }); }); diff --git a/test/messages/message.js b/test/messages/message.js index b5e7e9e..f93b0fd 100644 --- a/test/messages/message.js +++ b/test/messages/message.js @@ -4,10 +4,10 @@ var should = require('chai').should(); var P2P = require('../../'); var Message = P2P.Messages.Message; -describe('P2P Message', function() { +describe('Message', function() { describe('@constructor', function() { - it('should construct with magic number and command', function() { + it('construct with magic number and command', function() { var message = new Message({magicNumber: 0xd9b4bef9, command: 'command'}); message.command.should.equal('command'); message.magicNumber.should.equal(0xd9b4bef9); @@ -15,7 +15,7 @@ describe('P2P Message', function() { }); describe('#toBuffer', function() { - it('should serialize to a buffer', function() { + it('serialize to a buffer', function() { var message = new Message({magicNumber: 0xd9b4bef9, command: 'command'}); message.getPayload = function() { return new Buffer(0); diff --git a/test/peer.js b/test/peer.js index e575b64..98d0de4 100644 --- a/test/peer.js +++ b/test/peer.js @@ -69,42 +69,42 @@ describe('Peer', function() { }); - it('should be able to create instance', function() { + it('create instance', function() { var peer = new Peer('localhost'); peer.host.should.equal('localhost'); peer.network.should.equal(Networks.livenet); peer.port.should.equal(Networks.livenet.port); }); - it('should be able to create instance setting a port', function() { + it('create instance setting a port', function() { var peer = new Peer({host: 'localhost', port: 8111}); peer.host.should.equal('localhost'); peer.network.should.equal(Networks.livenet); peer.port.should.equal(8111); }); - it('should be able to create instance setting a network', function() { + it('create instance setting a network', function() { var peer = new Peer({host: 'localhost', network: Networks.testnet}); peer.host.should.equal('localhost'); peer.network.should.equal(Networks.testnet); peer.port.should.equal(Networks.testnet.port); }); - it('should be able to create instance setting port and network', function() { + it('create instance setting port and network', function() { var peer = new Peer({host: 'localhost', port: 8111, network: Networks.testnet}); peer.host.should.equal('localhost'); peer.network.should.equal(Networks.testnet); peer.port.should.equal(8111); }); - it('should support creating instance without new', function() { + it('create instance without new', function() { var peer = Peer({host: 'localhost', port: 8111, network: Networks.testnet}); peer.host.should.equal('localhost'); peer.network.should.equal(Networks.testnet); peer.port.should.equal(8111); }); - it('should be able to set a proxy', function() { + it('set a proxy', function() { var peer, peer2, socket; peer = new Peer('localhost'); @@ -121,7 +121,7 @@ describe('Peer', function() { peer.should.equal(peer2); }); - it('Peer.relay setting set properly', function() { + it('relay set properly', function() { var peer = new Peer({host: 'localhost'}); peer.relay.should.equal(true); var peer2 = new Peer({host: 'localhost', relay: false}); @@ -130,7 +130,7 @@ describe('Peer', function() { peer3.relay.should.equal(true); }); - it('Peer.relay setting respected', function() { + it('relay setting respected', function() { [true,false].forEach(function(relay) { var peer = new Peer({host: 'localhost', relay: relay}); var peerSendMessageStub = sinon.stub(Peer.prototype, 'sendMessage', function(message) { diff --git a/test/pool.js b/test/pool.js index 3daba0c..e575d13 100644 --- a/test/pool.js +++ b/test/pool.js @@ -24,7 +24,7 @@ function getPayloadBuffer(messageBuffer) { describe('Pool', function() { - it('should be able to create instance', function() { + it('create instance', function() { var pool = new Pool(); should.exist(pool.network); expect(pool.network).to.satisfy(function(network) { @@ -32,12 +32,12 @@ describe('Pool', function() { }); }); - it('should be able to create instance setting the network', function() { + it('create instance setting the network', function() { var pool = new Pool({network: Networks.testnet}); pool.network.should.equal(Networks.testnet); }); - it('should discover peers via dns', function() { + it('discover peers via dns', function() { var stub = sinon.stub(dns, 'resolve', function(seed, callback) { callback(null, ['10.10.10.1', '10.10.10.2', '10.10.10.3']); }); @@ -48,7 +48,7 @@ describe('Pool', function() { stub.restore(); }); - it('can optionally connect without dns seeds', function() { + it('optionally connect without dns seeds', function() { var stub = sinon.stub(dns, 'resolve', function(seed, callback) { throw new Error('DNS should not be called'); }); @@ -92,7 +92,7 @@ describe('Pool', function() { pool._addrs.length.should.equal(1); }); - it('should add new addrs as they are announced over the network', function(done) { + it('add new addrs as they are announced over the network', function(done) { // only emit an event, no need to connect var peerConnectStub = sinon.stub(Peer.prototype, 'connect', function() { @@ -103,7 +103,7 @@ describe('Pool', function() { // mock a addr peer event var peerMessageStub = sinon.stub(Peer.prototype, '_readMessage', function() { var payloadBuffer = getPayloadBuffer(MessagesData.addr.message); - var message = messages.buildFromBuffer('addr', payloadBuffer); + var message = messages._buildFromBuffer('addr', payloadBuffer); this.emit(message.command, message); }); @@ -154,7 +154,7 @@ describe('Pool', function() { // mock a addr peer event var peerMessageStub = sinon.stub(Peer.prototype, '_readMessage', function() { var payloadBuffer = getPayloadBuffer(MessagesData.addr.message); - var message = messages.buildFromBuffer('addr', payloadBuffer); + var message = messages._buildFromBuffer('addr', payloadBuffer); this.emit(message.command, message); }); @@ -195,7 +195,7 @@ describe('Pool', function() { }); - it('should propagate connect, ready, and disconnect peer events', function(done) { + it('propagate connect, ready, and disconnect peer events', function(done) { var peerConnectStub = sinon.stub(Peer.prototype, 'connect', function() { this.emit('connect', this, {}); this.emit('ready'); @@ -240,7 +240,7 @@ describe('Pool', function() { pool.connect(); }); - it('should propagate Pool.relay property to peers', function(done) { + it('propagate relay property to peers', function(done) { var count = 0; var peerConnectStub = sinon.stub(Peer.prototype, 'connect', function() { this.emit('connect', this, {}); @@ -260,12 +260,12 @@ describe('Pool', function() { peerConnectStub.restore(); }); - it('should output the console correctly', function() { + it('output the console correctly', function() { var pool = new Pool(); pool.inspect().should.equal(''); }); - it('should emit seederrors with error', function(done) { + it('emit seederrors with error', function(done) { var dnsStub = sinon.stub(dns, 'resolve', function(seed, callback) { callback(new Error('A DNS error')); }); @@ -279,7 +279,7 @@ describe('Pool', function() { pool.connect(); }); - it('should emit seederrors with notfound', function(done) { + it('emit seederrors with notfound', function(done) { var dnsStub = sinon.stub(dns, 'resolve', function(seed, callback) { callback(null, []); });