Make sanity check in GCSFilter constructor optional

pstratem · kcalvinalvin · commit b0a53d50d914 · 2022-05-02T16:04:00.000+09:00
BlockFilterIndex will perform the cheaper check of verifying the filter
hash when reading the filter from disk.
diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp
@@ -47,7 +47,7 @@ GCSFilter::GCSFilter(const Params& params)
     : m_params(params), m_N(0), m_F(0), m_encoded{0}
 {}
 
-GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter)
+GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check)
     : m_params(params), m_encoded(std::move(encoded_filter))
 {
     SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded};
@@ -59,6 +59,8 @@ GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_fi
     }
     m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);
 
+    if (skip_decode_check) return;
+
     // Verify that the encoded filter contains exactly N elements. If it has too much or too little
     // data, a std::ios_base::failure exception will be raised.
     BitStreamReader<SpanReader> bitreader{stream};
@@ -219,14 +221,14 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block,
 }
 
 BlockFilter::BlockFilter(BlockFilterType filter_type, const uint256& block_hash,
-                         std::vector<unsigned char> filter)
+                         std::vector<unsigned char> filter, bool skip_decode_check)
     : m_filter_type(filter_type), m_block_hash(block_hash)
 {
     GCSFilter::Params params;
     if (!BuildParams(params)) {
         throw std::invalid_argument("unknown filter_type");
     }
-    m_filter = GCSFilter(params, std::move(filter));
+    m_filter = GCSFilter(params, std::move(filter), skip_decode_check);
 }
 
 BlockFilter::BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo)
diff --git a/src/blockfilter.h b/src/blockfilter.h
@@ -59,7 +59,7 @@ class GCSFilter
     explicit GCSFilter(const Params& params = Params());
 
     /** Reconstructs an already-created filter from an encoding. */
-    GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter);
+    GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter, bool skip_decode_check);
 
     /** Builds a new filter from the params and set of elements. */
     GCSFilter(const Params& params, const ElementSet& elements);
@@ -122,7 +122,7 @@ class BlockFilter
 
     //! Reconstruct a BlockFilter from parts.
     BlockFilter(BlockFilterType filter_type, const uint256& block_hash,
-                std::vector<unsigned char> filter);
+                std::vector<unsigned char> filter, bool skip_decode_check);
 
     //! Construct a new BlockFilter of the specified type from a block.
     BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo);
@@ -164,7 +164,7 @@ class BlockFilter
         if (!BuildParams(params)) {
             throw std::ios_base::failure("unknown filter_type");
         }
-        m_filter = GCSFilter(params, std::move(encoded_filter));
+        m_filter = GCSFilter(params, std::move(encoded_filter), /*skip_decode_check=*/false);
     }
 };
 
diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp
@@ -5,6 +5,7 @@
 #include <map>
 
 #include <dbwrapper.h>
+#include <hash.h>
 #include <index/blockfilterindex.h>
 #include <node/blockstorage.h>
 #include <util/system.h>
@@ -143,18 +144,22 @@ bool BlockFilterIndex::CommitInternal(CDBBatch& batch)
     return BaseIndex::CommitInternal(batch);
 }
 
-bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const
+bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const
 {
     CAutoFile filein(m_filter_fileseq->Open(pos, true), SER_DISK, CLIENT_VERSION);
     if (filein.IsNull()) {
         return false;
     }
 
+    // Check that the hash of the encoded_filter matches the one stored in the db.
     uint256 block_hash;
     std::vector<uint8_t> encoded_filter;
     try {
         filein >> block_hash >> encoded_filter;
-        filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter));
+        uint256 result;
+        CHash256().Write(encoded_filter).Finalize(result);
+        if (result != hash) return error("Checksum mismatch in filter decode.");
+        filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter), /*skip_decode_check=*/true);
     }
     catch (const std::exception& e) {
         return error("%s: Failed to deserialize block filter from disk: %s", __func__, e.what());
@@ -381,7 +386,7 @@ bool BlockFilterIndex::LookupFilter(const CBlockIndex* block_index, BlockFilter&
         return false;
     }
 
-    return ReadFilterFromDisk(entry.pos, filter_out);
+    return ReadFilterFromDisk(entry.pos, entry.hash, filter_out);
 }
 
 bool BlockFilterIndex::LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out)
@@ -425,7 +430,7 @@ bool BlockFilterIndex::LookupFilterRange(int start_height, const CBlockIndex* st
     filters_out.resize(entries.size());
     auto filter_pos_it = filters_out.begin();
     for (const auto& entry : entries) {
-        if (!ReadFilterFromDisk(entry.pos, *filter_pos_it)) {
+        if (!ReadFilterFromDisk(entry.pos, entry.hash, *filter_pos_it)) {
             return false;
         }
         ++filter_pos_it;
diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h
@@ -31,7 +31,7 @@ class BlockFilterIndex final : public BaseIndex
     FlatFilePos m_next_filter_pos;
     std::unique_ptr<FlatFileSeq> m_filter_fileseq;
 
-    bool ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const;
+    bool ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const;
     size_t WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& filter);
 
     Mutex m_cs_headers_cache;

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ GCSFilter::GCSFilter(const Params& params)`
`47`	`47`	`: m_params(params), m_N(0), m_F(0), m_encoded{0}`
`48`	`48`	`{}`
`49`	`49`
`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)`
`51`	`51`	`: m_params(params), m_encoded(std::move(encoded_filter))`
`52`	`52`	`{`
`53`	`53`	`SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded};`
`@@ -59,6 +59,8 @@ GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_fi`
`59`	`59`	`}`
`60`	`60`	`m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);`
`61`	`61`
	`62`	`+ if (skip_decode_check) return;`
	`63`	`+`
`62`	`64`	`// Verify that the encoded filter contains exactly N elements. If it has too much or too little`
`63`	`65`	`// data, a std::ios_base::failure exception will be raised.`
`64`	`66`	`BitStreamReader<SpanReader> bitreader{stream};`
`@@ -219,14 +221,14 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block,`
`219`	`221`	`}`
`220`	`222`
`221`	`223`	`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)`
`223`	`225`	`: m_filter_type(filter_type), m_block_hash(block_hash)`
`224`	`226`	`{`
`225`	`227`	`GCSFilter::Params params;`
`226`	`228`	`if (!BuildParams(params)) {`
`227`	`229`	`throw std::invalid_argument("unknown filter_type");`
`228`	`230`	`}`
`229`		`- m_filter = GCSFilter(params, std::move(filter));`
	`231`	`+ m_filter = GCSFilter(params, std::move(filter), skip_decode_check);`
`230`	`232`	`}`
`231`	`233`
`232`	`234`	`BlockFilter::BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo)`