Skip to content

Commit 3bd584c

Browse files
kwvgkwvg
committed
merge bitcoin#24832: Verify the block filter hash when reading the filter from disk
1 parent e507a51 commit 3bd584c

File tree

5 files changed

+81
-28
lines changed

5 files changed

+81
-28
lines changed

‎src/bench/gcs_filter.cpp‎

Lines changed: 63 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,38 +5,84 @@
55
#include <bench/bench.h>
66
#include <blockfilter.h>
77

8-
static void ConstructGCSFilter(benchmark::Bench& bench)
8+
static const GCSFilter::ElementSet GenerateGCSTestElements()
99
{
1010
GCSFilter::ElementSet elements;
11-
for (int i = 0; i < 10000; ++i) {
11+
12+
// Testing the benchmarks with different number of elements show that a filter
13+
// with at least 100,000 elements results in benchmarks that have the same
14+
// ns/op. This makes it easy to reason about how long (in nanoseconds) a single
15+
// filter element takes to process.
16+
for (int i = 0; i < 100000; ++i) {
1217
GCSFilter::Element element(32);
1318
element[0] = static_cast<unsigned char>(i);
1419
element[1] = static_cast<unsigned char>(i >> 8);
1520
elements.insert(std::move(element));
1621
}
1722

23+
return elements;
24+
}
25+
26+
static void GCSBlockFilterGetHash(benchmark::Bench& bench)
27+
{
28+
auto elements = GenerateGCSTestElements();
29+
30+
GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
31+
BlockFilter block_filter(BlockFilterType::BASIC_FILTER, {}, filter.GetEncoded(), /*skip_decode_check=*/false);
32+
33+
bench.run([&] {
34+
block_filter.GetHash();
35+
});
36+
}
37+
38+
static void GCSFilterConstruct(benchmark::Bench& bench)
39+
{
40+
auto elements = GenerateGCSTestElements();
41+
1842
uint64_t siphash_k0 = 0;
19-
bench.batch(elements.size()).unit("elem").run([&] {
20-
GCSFilter filter({siphash_k0, 0, 20, 1 << 20}, elements);
43+
bench.run([&]{
44+
GCSFilter filter({siphash_k0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
45+
2146
siphash_k0++;
2247
});
2348
}
2449

25-
static void MatchGCSFilter(benchmark::Bench& bench)
50+
static void GCSFilterDecode(benchmark::Bench& bench)
2651
{
27-
GCSFilter::ElementSet elements;
28-
for (int i = 0; i < 10000; ++i) {
29-
GCSFilter::Element element(32);
30-
element[0] = static_cast<unsigned char>(i);
31-
element[1] = static_cast<unsigned char>(i >> 8);
32-
elements.insert(std::move(element));
33-
}
34-
GCSFilter filter({0, 0, 20, 1 << 20}, elements);
52+
auto elements = GenerateGCSTestElements();
3553

36-
bench.unit("elem").run([&] {
37-
filter.Match(GCSFilter::Element());
54+
GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
55+
auto encoded = filter.GetEncoded();
56+
57+
bench.run([&] {
58+
GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, encoded, /*skip_decode_check=*/false);
59+
});
60+
}
61+
62+
static void GCSFilterDecodeSkipCheck(benchmark::Bench& bench)
63+
{
64+
auto elements = GenerateGCSTestElements();
65+
66+
GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
67+
auto encoded = filter.GetEncoded();
68+
69+
bench.run([&] {
70+
GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, encoded, /*skip_decode_check=*/true);
3871
});
3972
}
4073

41-
BENCHMARK(ConstructGCSFilter);
42-
BENCHMARK(MatchGCSFilter);
74+
static void GCSFilterMatch(benchmark::Bench& bench)
75+
{
76+
auto elements = GenerateGCSTestElements();
77+
78+
GCSFilter filter({0, 0, BASIC_FILTER_P, BASIC_FILTER_M}, elements);
79+
80+
bench.run([&] {
81+
filter.Match(GCSFilter::Element());
82+
});
83+
}
84+
BENCHMARK(GCSBlockFilterGetHash);
85+
BENCHMARK(GCSFilterConstruct);
86+
BENCHMARK(GCSFilterDecode);
87+
BENCHMARK(GCSFilterDecodeSkipCheck);
88+
BENCHMARK(GCSFilterMatch);

‎src/blockfilter.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ GCSFilter::GCSFilter(const Params& params)
4747
: m_params(params), m_N(0), m_F(0), m_encoded{0}
4848
{}
4949

50-
GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter)
50+
GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check)
5151
: m_params(params), m_encoded(std::move(encoded_filter))
5252
{
5353
SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0};
@@ -59,6 +59,8 @@ GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_fi
5959
}
6060
m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);
6161

62+
if (skip_decode_check) return;
63+
6264
// Verify that the encoded filter contains exactly N elements. If it has too much or too little
6365
// data, a std::ios_base::failure exception will be raised.
6466
BitStreamReader<SpanReader> bitreader{stream};
@@ -219,14 +221,14 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block,
219221
}
220222

221223
BlockFilter::BlockFilter(BlockFilterType filter_type, const uint256& block_hash,
222-
std::vector<unsigned char> filter)
224+
std::vector<unsigned char> filter, bool skip_decode_check)
223225
: m_filter_type(filter_type), m_block_hash(block_hash)
224226
{
225227
GCSFilter::Params params;
226228
if (!BuildParams(params)) {
227229
throw std::invalid_argument("unknown filter_type");
228230
}
229-
m_filter = GCSFilter(params, std::move(filter));
231+
m_filter = GCSFilter(params, std::move(filter), skip_decode_check);
230232
}
231233

232234
BlockFilter::BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo)

‎src/blockfilter.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class GCSFilter
6060
explicit GCSFilter(const Params& params = Params());
6161

6262
/** Reconstructs an already-created filter from an encoding. */
63-
GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter);
63+
GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check);
6464

6565
/** Builds a new filter from the params and set of elements. */
6666
GCSFilter(const Params& params, const ElementSet& elements);
@@ -123,7 +123,7 @@ class BlockFilter
123123

124124
//! Reconstruct a BlockFilter from parts.
125125
BlockFilter(BlockFilterType filter_type, const uint256& block_hash,
126-
std::vector<unsigned char> filter);
126+
std::vector<unsigned char> filter, bool skip_decode_check);
127127

128128
//! Construct a new BlockFilter of the specified type from a block.
129129
BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo);
@@ -165,7 +165,7 @@ class BlockFilter
165165
if (!BuildParams(params)) {
166166
throw std::ios_base::failure("unknown filter_type");
167167
}
168-
m_filter = GCSFilter(params, std::move(encoded_filter));
168+
m_filter = GCSFilter(params, std::move(encoded_filter), /*skip_decode_check=*/false);
169169
}
170170
};
171171

‎src/index/blockfilterindex.cpp‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <map>
66

77
#include <dbwrapper.h>
8+
#include <hash.h>
89
#include <index/blockfilterindex.h>
910
#include <node/blockstorage.h>
1011
#include <serialize.h>
@@ -146,18 +147,22 @@ bool BlockFilterIndex::CommitInternal(CDBBatch& batch)
146147
return BaseIndex::CommitInternal(batch);
147148
}
148149

149-
bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const
150+
bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const
150151
{
151152
CAutoFile filein(m_filter_fileseq->Open(pos, true), SER_DISK, CLIENT_VERSION);
152153
if (filein.IsNull()) {
153154
return false;
154155
}
155156

157+
// Check that the hash of the encoded_filter matches the one stored in the db.
156158
uint256 block_hash;
157159
std::vector<uint8_t> encoded_filter;
158160
try {
159161
filein >> block_hash >> encoded_filter;
160-
filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter));
162+
uint256 result;
163+
CHash256().Write(encoded_filter).Finalize(result);
164+
if (result != hash) return error("Checksum mismatch in filter decode.");
165+
filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter), /*skip_decode_check=*/true);
161166
}
162167
catch (const std::exception& e) {
163168
return error("%s: Failed to deserialize block filter from disk: %s", __func__, e.what());
@@ -384,7 +389,7 @@ bool BlockFilterIndex::LookupFilter(const CBlockIndex* block_index, BlockFilter&
384389
return false;
385390
}
386391

387-
return ReadFilterFromDisk(entry.pos, filter_out);
392+
return ReadFilterFromDisk(entry.pos, entry.hash, filter_out);
388393
}
389394

390395
bool BlockFilterIndex::LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out)
@@ -428,7 +433,7 @@ bool BlockFilterIndex::LookupFilterRange(int start_height, const CBlockIndex* st
428433
filters_out.resize(entries.size());
429434
auto filter_pos_it = filters_out.begin();
430435
for (const auto& entry : entries) {
431-
if (!ReadFilterFromDisk(entry.pos, *filter_pos_it)) {
436+
if (!ReadFilterFromDisk(entry.pos, entry.hash, *filter_pos_it)) {
432437
return false;
433438
}
434439
++filter_pos_it;

‎src/index/blockfilterindex.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class BlockFilterIndex final : public BaseIndex
3232
FlatFilePos m_next_filter_pos;
3333
std::unique_ptr<FlatFileSeq> m_filter_fileseq;
3434

35-
bool ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const;
35+
bool ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const;
3636
size_t WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& filter);
3737

3838
Mutex m_cs_headers_cache;

0 commit comments

Comments
 (0)