-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extensible Messages #48
Changes from 40 commits
9a813ba
0b20724
e8f0725
11bee8b
32a28b1
42288e8
7e29af4
d63c117
1dfe709
608e41d
6ca2f69
3c2afed
feda6e9
e460bf9
2eb7712
091893b
faf2bb1
7f61fc6
6461748
641443f
4ecb34e
a8b8c59
8c9babc
3b53593
97f39db
6007dc6
e31f28e
cfbee2d
a57f864
a15f11c
6db209b
1545abb
e62ddd9
ede5f0b
f17bbf5
42c829e
50d7d37
4c4d53a
42985a7
f1aa4d3
39d1ae9
8f2d008
664ceb2
16aa989
f6e9c43
c0e3bdb
7cfe6d1
34c3846
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,34 +4,29 @@ var bitcore = require('bitcore'); | |
var BloomFilter = require('bloom-filter'); | ||
var BufferReader = bitcore.encoding.BufferReader; | ||
var BufferWriter = bitcore.encoding.BufferWriter; | ||
var $ = bitcore.util.preconditions; | ||
|
||
|
||
/** | ||
* A constructor for Bloom Filters | ||
* @see https://github.com/bitpay/bloom-filter | ||
* @param {Buffer} - payload | ||
*/ | ||
BloomFilter.fromBuffer = function fromBuffer(payload) { | ||
var obj = {}; | ||
var parser = new BufferReader(payload); | ||
var data = parser.readVarLengthBuffer(); | ||
$.checkState(data.length <= BloomFilter.MAX_BLOOM_FILTER_SIZE, | ||
'Filter data must be <= MAX_BLOOM_FILTER_SIZE bytes'); | ||
var nHashFuncs = parser.readUInt32LE(); | ||
$.checkState(nHashFuncs <= BloomFilter.MAX_HASH_FUNCS, | ||
'Filter nHashFuncs must be <= MAX_HASH_FUNCS'); | ||
var nTweak = parser.readUInt32LE(); | ||
var nFlags = parser.readUInt8(); | ||
|
||
var vData = []; | ||
var dataParser = new BufferReader(data); | ||
for(var i = 0; i < data.length; i++) { | ||
vData.push(dataParser.readUInt8()); | ||
var length = parser.readUInt8(); | ||
obj.vData = []; | ||
for(var i = 0; i < length; i++) { | ||
obj.vData.push(parser.readUInt8()); | ||
} | ||
obj.nHashFuncs = parser.readUInt32LE(); | ||
obj.nTweak = parser.readUInt32LE(); | ||
obj.nFlags = parser.readUInt8(); | ||
return new BloomFilter(obj); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is dropping the checks the way to go here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe there are checks already in BloomFilter (e.g. https://github.com/bitpay/bloom-filter/blob/master/lib/filter.js#L69). Though these may need to be moved to apply in the main constructor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I was going to suggest to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. merged! |
||
}; | ||
|
||
return new BloomFilter({ | ||
vData: vData, | ||
nHashFuncs: nHashFuncs, | ||
nTweak: nTweak, | ||
nFlags: nFlags | ||
}); | ||
} | ||
|
||
/** | ||
* @returns {Buffer} | ||
*/ | ||
BloomFilter.prototype.toBuffer = function toBuffer() { | ||
var bw = new BufferWriter(); | ||
bw.writeVarintNum(this.vData.length); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
'use strict'; | ||
|
||
var Buffers = require('buffers'); | ||
|
||
Buffers.prototype.skip = function(i) { | ||
if (i === 0) { | ||
return; | ||
} | ||
|
||
if (i === this.length) { | ||
this.buffers = []; | ||
this.length = 0; | ||
return; | ||
} | ||
|
||
var pos = this.pos(i); | ||
this.buffers = this.buffers.slice(pos.buf); | ||
this.buffers[0] = new Buffer(this.buffers[0].slice(pos.offset)); | ||
this.length -= i; | ||
}; | ||
|
||
module.exports = Buffers; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a pretty useful function. Should we submit it to the global buffers module https://github.com/substack/node-buffers? I'd be happy to give it a go if that's the right thing to do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly, we may not need this dependency either though, since we may only be using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, I wanted to take out this dependency as well at some point, don't remember what changed my mind. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spawned an issue: #52 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you unit-test the skip function please? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added test in: braydonf@7cfe6d1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
/** | ||
* @namespace P2P | ||
*/ | ||
|
||
module.exports = { | ||
Inventory: require('./inventory'), | ||
BloomFilter: require('./bloomfilter'), | ||
Messages: require('./messages'), | ||
Peer: require('./peer'), | ||
Pool: require('./pool'), | ||
BloomFilter: require('./bloomfilter') | ||
Pool: require('./pool') | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in: 664ceb2