Implement filterload, filteradd, filterclear, merkleblock messages

[eordano]

Original discussion here: bitpay/bitcore#115

• filterload,
• filteradd,
• filterclear,
• merkleblock

[braydonf]

And "merkleblock" associated messages, and full BIP37 support as documented: https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki

[braydonf]

The bloom-filter that is needed to implement is located here: https://github.com/bitpay/bloom-filter We can extend it to include fromBuffer and toBuffer so that it can easily be instantiated from each of the filter messages. Also, isRelevantAndUpdate was started to be implemented in bloom-filter, however we can implement that here, so that the filter will update itself based upon the flags set in the filter.

[throughnothing]

I've attempted some work here, not sure that I'm doing things the best way, so didn't want to PR it (and no filteradd or merkleblocks yet), but would love some feedback: https://github.com/throughnothing/bitcore-p2p/compare/filter-messages

[throughnothing]

Maybe not the best interface, but the current way is used easiest via:

var BloomFilter = require('bloom-filter'),
    Messages = require('bitcore-p2p').Messages;

var bf = BloomFilter.create(20, 0.01);
// Do your bloom filter inserts here
var msg = new Messages.FilterLoad().fromBloomFilter(bf);

Short of using fromBloomFilter(), the user would have to create the data Buffer() themselves, which is less than ideal. I see the external interface requiring Buffers elsewhere, but it also seems thats a TODO to get rid of?

[braydonf]

Awesome!

Back in an earlier commit of bloom-filter: bitpay/bloom-filter@cde5fc6#diff-77a92250315fdf44b4ecf5b3fcf5fd3cL51 there are toBuffer and fromBuffer method that can be implemented here where BufferWriter/Reader are available, this way it is more portable (e.g. other situations where those methods may be needed, such as saving).

var BloomFilter = require('bloom-filter');
BloomFilter.prototype.fromBuffer() = function fromBuffer() {
...
}

Also many constants are available on Filter at https://github.com/bitpay/bloom-filter/blob/master/lib/filter.js#L127

Having a convenience method fromBloomFilter() sounds good, also the filter could be created before and then converted to a buffer:

var msg = Messages.FilterLoad(bloomfilter.toBuffer());

I think the buffer dependency issue may be on the fact that it's in package.json when it's native to node. Buffers are not going anywhere.

[throughnothing]

Thanks for the help. I didn't mean the Buffer dependency should be removed, just that it should be abstracted from end-users of bitcore-p2p. Obviously buffers are there to stay in the internal implementation :)

[throughnothing]

I don't know if this interface makes sense:

var msg = Messages.FilterLOad(bloomfilter.toBuffer());

Would we want the constructor signature to just take 1 param (a buffer for the whole Filter) or should it require the individual params, like nHashFuncs, nFlags etc.?

function FilterLoad (data) { ... }
// Vs.
function FilterLoad(data, nHashFuncs, nTweak, nFlags) { ... }

If data were a buffer in both, it'd be confusing to determine what the caller wanted.

I'm not a fan of interfaces that can take different params, or try to take different numbers of args and coerce that into other things, etc., unless its really necessary.

[braydonf]

FilterLoad should accept a 'filter' (which is an instance of BloomFilter), and fromBloomFilter isn't necessary. This is a common pattern for other messages.

[braydonf]

Merkle block is the only left, I think we need a MerkleBlock constructor and a method to verify that a transaction exist in a block: bitpay/bitcore#1082

[throughnothing]

@braydonf thanks for the help on this. Do you have any suggestions on an interface for the 'verify that a transaction exist in a block' interface piece?

[braydonf]

Commented on the associated PR, I think that sounds great.

[braydonf]

Awesome work! Thanks @throughnothing