Add fuzzing harness for bloom filter classes (CBloomFilter + CRollingBloomFilter).
Test this PR using:
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/bloom_filter
…
$ src/test/fuzz/rolling_bloom_filter
…
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
55 | + case 4: { 56 | + const Optional<COutPoint> out_point = ConsumeDeserializable<COutPoint>(fuzzed_data_provider); 57 | + if (!out_point) { 58 | + break; 59 | + } 60 | + (void)bloom_filter.contains(*out_point);
Why does this need to be a different fuzzer-instructed path? I'd guess it would be simpler and more efficient to call this as the first step in case 1
Good point. Fixed!
63 | + case 5: { 64 | + const Optional<uint256> u256 = ConsumeDeserializable<uint256>(fuzzed_data_provider); 65 | + if (!u256) { 66 | + break; 67 | + } 68 | + (void)bloom_filter.contains(*u256);
Same
47 | + assert(present); 48 | + break; 49 | + } 50 | + case 3: { 51 | + const std::vector<unsigned char>& b = ConsumeRandomLengthByteVector(fuzzed_data_provider); 52 | + (void)bloom_filter.contains(b);
Same
15 | + 16 | +void test_one_input(const std::vector<uint8_t>& buffer) 17 | +{ 18 | + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); 19 | + 20 | + CBloomFilter bloom_filter{fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(1, 10000000), 1.0 / fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(1, std::numeric_limits<unsigned int>::max()), fuzzed_data_provider.ConsumeIntegral<unsigned int>(), static_cast<unsigned char>(fuzzed_data_provider.PickValueInArray({BLOOM_UPDATE_NONE, BLOOM_UPDATE_ALL, BLOOM_UPDATE_P2PUBKEY_ONLY, BLOOM_UPDATE_MASK}))};
I know that our clang-format allows infinite length lines. But in editors that don't line-wrap this is really hard to read. Maybe add a newline after each ,?
73 | + break; 74 | + case 7: 75 | + bloom_filter.reset(fuzzed_data_provider.ConsumeIntegral<unsigned int>()); 76 | + break; 77 | + case 8: 78 | + (void)bloom_filter.IsWithinSizeConstraints();
Why does this need to be fuzzer-instructed? More coverage can be achieved by calling it unconditionally.
9 | +#include <attributes.h> 10 | +#include <optional.h> 11 | +#include <serialize.h> 12 | +#include <streams.h> 13 | +#include <test/fuzz/FuzzedDataProvider.h> 14 | +#include <test/fuzz/fuzz.h>
Why include itself?
4 | @@ -5,10 +5,39 @@ 5 | #ifndef BITCOIN_TEST_FUZZ_FUZZ_H 6 | #define BITCOIN_TEST_FUZZ_FUZZ_H 7 | 8 | -#include <stdint.h> 9 | +#include <attributes.h> 10 | +#include <optional.h> 11 | +#include <serialize.h>
This header takes a long time to parse, but not every fuzz target is using it. It might reduce compile time to put the fuzzedDataProvider helpers in a new module (header) and keep this header the raw and minimal "main fuzz header"
Good point. Now fixed.
44 | + case 3: { 45 | + const Optional<uint256> u256 = ConsumeDeserializable<uint256>(fuzzed_data_provider); 46 | + if (!u256) { 47 | + break; 48 | + } 49 | + (void)rolling_bloom_filter.contains(*u256);
Why does this need to be a different fuzzer-instructed path? I'd guess it would be simpler and more efficient to call this as the first step in case 1
36 | + assert(present); 37 | + break; 38 | + } 39 | + case 2: { 40 | + const std::vector<unsigned char>& b = ConsumeRandomLengthByteVector(fuzzed_data_provider); 41 | + (void)rolling_bloom_filter.contains(b);
Same
ACK
@MarcoFalke Thanks for reviewing. Good feedback! All points addressed - please re-review :)
ACK eabbbe409f397e97b1e6fad7385d9d1813ae2880 🤞
<details><summary>Show signature and timestamp</summary>
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK eabbbe409f397e97b1e6fad7385d9d1813ae2880 🤞
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUjMAAwAon5Oa4NRYASpbv3ahHtb4uOLNZJZq/Grjzc8/XyogORj0+Lsz7+y7p1n
zFxGKrCeLJfpJ5IyetU8ZCprs55CfgDyIT4jy+8GfPgQ/bbpa2Rxg7m87q2osAgi
Xa5C/jnQvILDTvwAE+RA7zdc2GFBCJilmG2WCB+XRm4OZzd4bOmPKyCfjj7+DVrQ
zLdlxaKrSJHxp50DAntkH8g57k2LSdIn+0bw4HQ1Wx/l36viiMlzcq+LBWc+5ERr
A/jWaxEYYee7CcpjtW+ICANYY//C3sK1yvxtW/szKmApmvg5oEQELDTUaqUBZuC5
BlmfI1r3O43K2a8VYSZUgKOzQiHAvanYq7a8Bg42dMmf7/lJaBt+j2OHktdpwYeB
OR/MN2NQER7j12goLn+QNUl/AN/SlZJJX6gF2t6qvDzxIGQ27zYkSgUr3DfSjaMq
sWLHbtioYvwJAi8eHosNmQIyoOpbs8/VmUFevvjSZpjVnT4f7QfBYwOdc4S9kkjQ
cbSFuGSL
=QsZQ
-----END PGP SIGNATURE-----
Timestamp of file with hash 6fe8d1c8528304d7c8bc08ad9e3af6e5c9f2a116bb4b9ad3ff899c36e90eb082 -
</details>