Use 11-bit mask in Bloom.check

[gkaemmer]

Fixes #303.

Page 5-6 of the Ethereum Yellow Paper specifies that the bloom filter take the low-order 11 bits from each of the first 3 pair of bytes of the hash. The proper bitmask for the low 11 bits is 2^11 - 1 = 2047.

For some reason the check function in bloom.js only takes the low-order 9 bits using a mask of 511, so check always returns false.

I think the check method is not used anywhere in this library (I only found this by trying to use bloom.js in my own code), but probably still worth fixing.

[coveralls]

Coverage remained the same at 85.739% when pulling ae5bcf6 on gkaemmer:bloom-check-fix into 4ecae8a on ethereumjs:master.

[axic]

Is there a way to include tests for this?

[holgerd77]

@axic Tests will be covered with #310 (currently running as a Gitcoin bounty), so it would be rather conflicting to introduce separate tests here.

[holgerd77]

@gkaemmer I am also realizing though, that I am not the super-bloom filter expert. From reading up on this, I would says this is correct, but could you provide at least one (here in the comments) working code example where a value is not checked correctly before and checked correctly with the PR?

[holgerd77]

@gkaemmer Would also be good if you could rebase on this occasion.

[gkaemmer]

@holgerd77 Rebased on master. Here's a quick check:

// After `git checkout origin/master`
const Bloom = require("./lib/bloom.js");
const bloom = new Bloom();
bloom.add("test");
bloom.check("test"); // false
bloom.check("test2"); // false
// After `git checkout gkaemmer/bloom-check-fix`
const Bloom = require("./lib/bloom.js");
const bloom = new Bloom();
bloom.add("test");
bloom.check("test"); // true
bloom.check("test2"); // false

[holgerd77]

@gkaemmer Ah, sorry, wasn't aware that this can be tested so easily and initialized without passing any value, this really needs better documentation.

[holgerd77]

Looks good, tested this locally, will merge.

[axic]

I would have merged this after #315, so that unit tests can cover it.

[holgerd77]

This is so obviously wrong and can very easily be tested manually. #315 may very well take 3-4 more weeks.

[axic]

I had the feeling #315 was kind of ready 😉