fuzz: a new target for the coins database #28216

darosior · 2023-08-04T12:31:16Z

Similarly to #28209, this introduces a fuzz target for CCoinsViewDb by using an in-memory LevelDB. We reuse the body of the existing fuzz target for coins_view.

DrahtBot · 2023-08-04T12:31:20Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/28216.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	brunoerg, l0rinc
Stale ACK	TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31703 (Use number of dirty cache entries in flush warnings/logs by sipa)

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.

…in the `coins_view` target e417c98 fuzz: coins_view: remove an incorrect assertion (Antoine Poinsot) c5f6b1d fuzz: coins_view: correct an incorrect assertion (Antoine Poinsot) Pull request description: The `coins_view` fuzz target would assert in two places that the cache is consistent with the backend. But it's never the case (that's the whole point of using a cache). The only reason this didn't result in a crash was that we would never actually hit these assertions. I ran into this while introducing a new target with an in-memory `CCoinsViewDB` as the backend view (see bitcoin/bitcoin#28216) which made the code paths with those assertions actually reachable. ACKs for top commit: dergoegge: Code review ACK e417c98 Tree-SHA512: 5847bb2744a2f2831dace62d32b79cc491bf54e2af4ce425411d245d566622d9aff816d9be5ec8e830d10851c13f2500bf4f0c004d88b4d7cca1d483ef8960a6

fanquake · 2023-08-15T10:06:06Z

Want rebase this now that #28215 is merged (also update PR description).

darosior · 2023-08-15T10:34:07Z

Rebased.

…coins_view` target e417c98 fuzz: coins_view: remove an incorrect assertion (Antoine Poinsot) c5f6b1d fuzz: coins_view: correct an incorrect assertion (Antoine Poinsot) Pull request description: The `coins_view` fuzz target would assert in two places that the cache is consistent with the backend. But it's never the case (that's the whole point of using a cache). The only reason this didn't result in a crash was that we would never actually hit these assertions. I ran into this while introducing a new target with an in-memory `CCoinsViewDB` as the backend view (see bitcoin#28216) which made the code paths with those assertions actually reachable. ACKs for top commit: dergoegge: Code review ACK e417c98 Tree-SHA512: 5847bb2744a2f2831dace62d32b79cc491bf54e2af4ce425411d245d566622d9aff816d9be5ec8e830d10851c13f2500bf4f0c004d88b4d7cca1d483ef8960a6

darosior · 2023-08-17T09:13:31Z

Rebased on master to fix the macOS CI.

darosior · 2023-12-31T16:45:39Z

While rebasing this i found another crash in the coins_view target. Catching the exception on AddCoin() and continuing will cause cachedCoinsUsage to underflow:

bitcoin/src/test/fuzz/coins_view.cpp

Lines 67 to 75 in 4b1196a

    
           try { 
        
               coins_view_cache.AddCoin(random_out_point, std::move(coin), possible_overwrite); 
        
               expected_code_path = true; 
        
           } catch (const std::logic_error& e) { 
        
               if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) { 
        
                   assert(!possible_overwrite); 
        
                   expected_code_path = true; 
        
               } 
        
           }

bitcoin/src/coins.cpp

Lines 76 to 100 in 4b1196a

    
           if (!inserted) { 
        
               cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); 
        
           } 
        
           if (!possible_overwrite) { 
        
               if (!it->second.coin.IsSpent()) { 
        
                   throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)"); 
        
               } 
        
               // If the coin exists in this cache as a spent coin and is DIRTY, then 
        
               // its spentness hasn't been flushed to the parent cache. We're 
        
               // re-adding the coin to this cache now but we can't mark it as FRESH. 
        
               // If we mark it FRESH and then spend it before the cache is flushed 
        
               // we would remove it from this cache and would never flush spentness 
        
               // to the parent cache. 
        
               // 
        
               // Re-adding a spent coin can happen in the case of a re-org (the coin 
        
               // is 'spent' when the block adding it is disconnected and then 
        
               // re-added when it is also added in a newly connected block). 
        
               // 
        
               // If the coin doesn't exist in the current cache, or is spent but not 
        
               // DIRTY, then it can be marked FRESH. 
        
               fresh = !(it->second.flags & CCoinsCacheEntry::DIRTY); 
        
           } 
        
           it->second.coin = std::move(coin); 
        
           it->second.flags |= CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0); 
        
           cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();

darosior · 2024-01-02T10:54:58Z

Rebased on top of #29164 to fix the above.

darosior · 2024-06-27T16:12:51Z

Alright, i can't reproduce the non-determinism he observed.

darosior · 2024-07-28T12:25:30Z

@dergoegge could you have a look here when you have a sec?

dergoegge · 2024-07-30T13:44:44Z

Alright, i can't reproduce the non-determinism he observed.

How did you try to reproduce it? Because I'm still seeing only ~70% stability when running with afl++.

@marcofleon this could also be a target to test your "stability debugging" script on.

…coins_view` target e417c98 fuzz: coins_view: remove an incorrect assertion (Antoine Poinsot) c5f6b1d fuzz: coins_view: correct an incorrect assertion (Antoine Poinsot) Pull request description: The `coins_view` fuzz target would assert in two places that the cache is consistent with the backend. But it's never the case (that's the whole point of using a cache). The only reason this didn't result in a crash was that we would never actually hit these assertions. I ran into this while introducing a new target with an in-memory `CCoinsViewDB` as the backend view (see bitcoin#28216) which made the code paths with those assertions actually reachable. ACKs for top commit: dergoegge: Code review ACK e417c98 Tree-SHA512: 5847bb2744a2f2831dace62d32b79cc491bf54e2af4ce425411d245d566622d9aff816d9be5ec8e830d10851c13f2500bf4f0c004d88b4d7cca1d483ef8960a6

maflcko · 2024-11-08T11:39:47Z

I think fixing stability would be nice, but not a blocker

maflcko · 2025-01-22T16:14:06Z

Also, if you rebase, you get a free check from the newly added "fuzz stability checkers"

We'll reuse it for a target where the coins view is a DB.

DrahtBot · 2025-02-18T19:15:40Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/37422309131}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

It reuses the logic from the `coins_view` target, except it uses an in-memory CCoinsViewDB as the backend. Note `CCoinsViewDB` will assert the best block hash is never null, so we slightly modify the coins_view fuzz logic to take care of this.

davidgumberg · 2025-02-19T04:59:32Z

src/test/fuzz/coins_view.cpp

@@ -207,7 +213,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend

    {
        std::unique_ptr<CCoinsViewCursor> coins_view_cursor = backend_coins_view.Cursor();
-        assert(!coins_view_cursor);
+        assert(is_db ? !!coins_view_cursor : !coins_view_cursor);


Why explicit conversion with !!? Doesn't assert cause contextual conversion to bool of the expression, or is this implementation specific?

E.g. https://godbolt.org/z/5s9688Thn

Wouldn't it be shorter and clearer to write assert(!!coins_view_cursor == is_db);?

I thought I commented on this already, but seems I forgot to submit:
👍 for

Suggested change

assert(is_db ? !!coins_view_cursor : !coins_view_cursor);

assert(is_db == !!coins_view_cursor);

or maybe even

Suggested change

assert(is_db ? !!coins_view_cursor : !coins_view_cursor);

assert(is_db != !coins_view_cursor);

davidgumberg · 2025-02-19T06:52:53Z

Reviewed the added fuzz test and it looks good.

I let the fuzz binary run with FUZZ=coins_db for about an hour and nothing crashed.

And, I generated some corpora (https://github.com/davidgumberg/qa-assets/tree/coins_db_corpora) for ~20 minutes by doing

$ ./build_fuzz/test/fuzz/test_runner.py -g qa-assets/fuzz_corpora/ coins_db

and used the deterministic fuzz coverage tool from #31836 but it found instability:

$ RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/build_fuzz $PWD/qa-assets/fuzz_corpora/ coins_db

deterministic-fuzz-coverage output

--- /bitcoin/build_det/fuzz_det_cov.show.0.txt      2025-02-18 21:44:37.658467363 -0800
+++ /bitcoin/build_det/fuzz_det_cov.show.1.txt      2025-02-18 21:44:38.217482311 -0800
@@ -58994,32 +58994,32 @@
    45|      0|  return "leveldb.InternalKeyComparator";
    46|      0|}
    47|       |
-   48|  81.7k|int InternalKeyComparator::Compare(const Slice& akey, const Slice& bkey) const {
+   48|  81.8k|int InternalKeyComparator::Compare(const Slice& akey, const Slice& bkey) const {
    49|       |  // Order by:
    50|       |  //    increasing user key (according to user-supplied comparator)
    51|       |  //    decreasing sequence number
    52|       |  //    decreasing type (though sequence# should be enough to disambiguate)
-   53|  81.7k|  int r = user_comparator_->Compare(ExtractUserKey(akey), ExtractUserKey(bkey));
-   54|  81.7k|  if (r == 0) {
+   53|  81.8k|  int r = user_comparator_->Compare(ExtractUserKey(akey), ExtractUserKey(bkey));
+   54|  81.8k|  if (r == 0) {
   ------------------
-  |  Branch (54:7): [True: 49.6k, False: 32.0k]
+  |  Branch (54:7): [True: 49.8k, False: 32.0k]
   ------------------
-   55|  49.6k|    const uint64_t anum = DecodeFixed64(akey.data() + akey.size() - 8);
-   56|  49.6k|    const uint64_t bnum = DecodeFixed64(bkey.data() + bkey.size() - 8);
-   57|  49.6k|    if (anum > bnum) {
+   55|  49.8k|    const uint64_t anum = DecodeFixed64(akey.data() + akey.size() - 8);
+   56|  49.8k|    const uint64_t bnum = DecodeFixed64(bkey.data() + bkey.size() - 8);
+   57|  49.8k|    if (anum > bnum) {
   ------------------
-  |  Branch (57:9): [True: 4.60k, False: 45.0k]
+  |  Branch (57:9): [True: 4.60k, False: 45.2k]
   ------------------
    58|  4.60k|      r = -1;
-   59|  45.0k|    } else if (anum < bnum) {
+   59|  45.2k|    } else if (anum < bnum) {
   ------------------
-  |  Branch (59:16): [True: 43.6k, False: 1.46k]
+  |  Branch (59:16): [True: 43.6k, False: 1.50k]
   ------------------
    60|  43.6k|      r = +1;
    61|  43.6k|    }
-   62|  49.6k|  }
-   63|  81.7k|  return r;
-   64|  81.7k|}
+   62|  49.8k|  }
+   63|  81.8k|  return r;
+   64|  81.8k|}
    65|       |
    66|       |void InternalKeyComparator::FindShortestSeparator(std::string* start,
    67|     37|                                                  const Slice& limit) const {
@@ -60131,17 +60131,17 @@
    52|       |
    53|      3|  ~MemTableIterator() override = default;
    54|       |
-   55|  6.60k|  bool Valid() const override { return iter_.Valid(); }
+   55|  6.59k|  bool Valid() const override { return iter_.Valid(); }
    56|      2|  void Seek(const Slice& k) override { iter_.Seek(EncodeKey(&tmp_, k)); }
    57|      1|  void SeekToFirst() override { iter_.SeekToFirst(); }
    58|      0|  void SeekToLast() override { iter_.SeekToLast(); }
    59|  6.62k|  void Next() override { iter_.Next(); }
    60|      0|  void Prev() override { iter_.Prev(); }
    61|  6.59k|  Slice key() const override { return GetLengthPrefixedSlice(iter_.key()); }
-   62|  6.58k|  Slice value() const override {
-   63|  6.58k|    Slice key_slice = GetLengthPrefixedSlice(iter_.key());
-   64|  6.58k|    return GetLengthPrefixedSlice(key_slice.data() + key_slice.size());
-   65|  6.58k|  }
+   62|  6.59k|  Slice value() const override {
+   63|  6.59k|    Slice key_slice = GetLengthPrefixedSlice(iter_.key());
+   64|  6.59k|    return GetLengthPrefixedSlice(key_slice.data() + key_slice.size());
+   65|  6.59k|  }
    66|       |
    67|      1|  Status status() const override { return Status::OK(); }
    68|       |
@@ -60475,12 +60475,12 @@
   150|       |
   151|       |  // Accessors/mutators for links.  Wrapped in methods so we can
   152|       |  // add the appropriate barriers as necessary.
-  153|  78.3k|  Node* Next(int n) {
-  154|  78.3k|    assert(n >= 0);
+  153|  78.4k|  Node* Next(int n) {
+  154|  78.4k|    assert(n >= 0);
   155|       |    // Use an 'acquire load' so that we observe a fully initialized
   156|       |    // version of the returned Node.
-  157|  78.3k|    return next_[n].load(std::memory_order_acquire);
-  158|  78.3k|  }
+  157|  78.4k|    return next_[n].load(std::memory_order_acquire);
+  158|  78.4k|  }
   159|  6.13k|  void SetNext(int n, Node* x) {
   160|  6.13k|    assert(n >= 0);
   161|       |    // Use a 'release store' so that anybody who reads through this
@@ -60518,9 +60518,9 @@
   193|  1.17k|}
   194|       |
   195|       |template <typename Key, class Comparator>
-  196|  28.1k|inline bool SkipList<Key, Comparator>::Iterator::Valid() const {
-  197|  28.1k|  return node_ != nullptr;
-  198|  28.1k|}
+  196|  28.2k|inline bool SkipList<Key, Comparator>::Iterator::Valid() const {
+  197|  28.2k|  return node_ != nullptr;
+  198|  28.2k|}
   199|       |
   200|       |template <typename Key, class Comparator>
   201|  14.1k|inline const Key& SkipList<Key, Comparator>::Iterator::key() const {
@@ -60531,8 +60531,8 @@
   206|       |template <typename Key, class Comparator>
   207|  6.62k|inline void SkipList<Key, Comparator>::Iterator::Next() {
   208|  6.62k|  assert(Valid());
-  209|  6.62k|  node_ = node_->Next(0);
-  210|  6.62k|}
+  209|  6.63k|  node_ = node_->Next(0);
+  210|  6.63k|}
   211|       |
   212|       |template <typename Key, class Comparator>
   213|      0|inline void SkipList<Key, Comparator>::Iterator::Prev() {
@@ -65829,10 +65829,10 @@
    31|  11.3k|  Slice() : data_(""), size_(0) {}
    32|       |
    33|       |  // Create a slice that refers to d[0,n-1].
-   34|   364k|  Slice(const char* d, size_t n) : data_(d), size_(n) {}
+   34|   365k|  Slice(const char* d, size_t n) : data_(d), size_(n) {}
    35|       |
    36|       |  // Create a slice that refers to the contents of "s"
-   37|  16.6k|  Slice(const std::string& s) : data_(s.data()), size_(s.size()) {}
+   37|  16.5k|  Slice(const std::string& s) : data_(s.data()), size_(s.size()) {}
    38|       |
    39|       |  // Create a slice that refers to s[0,strlen(s)-1]
    40|     26|  Slice(const char* s) : data_(s), size_(strlen(s)) {}
@@ -65842,10 +65842,10 @@
    44|       |  Slice& operator=(const Slice&) = default;
    45|       |
    46|       |  // Return a pointer to the beginning of the referenced data
-   47|   345k|  const char* data() const { return data_; }
+   47|   347k|  const char* data() const { return data_; }
    48|       |
    49|       |  // Return the length (in bytes) of the referenced data
-   50|   576k|  size_t size() const { return size_; }
+   50|   579k|  size_t size() const { return size_; }
    51|       |
    52|       |  // Return true iff the length of the referenced data is zero
    53|  5.76k|  bool empty() const { return size_ == 0; }
@@ -65910,30 +65910,30 @@
   101|       |
   102|      0|inline bool operator!=(const Slice& x, const Slice& y) { return !(x == y); }
   103|       |
-  104|  83.5k|inline int Slice::compare(const Slice& b) const {
-  105|  83.5k|  const size_t min_len = (size_ < b.size_) ? size_ : b.size_;
-                                                           ^201    ^83.3k
+  104|  83.7k|inline int Slice::compare(const Slice& b) const {
+  105|  83.7k|  const size_t min_len = (size_ < b.size_) ? size_ : b.size_;
+                                                           ^201    ^83.5k
   ------------------
-  |  Branch (105:26): [True: 201, False: 83.3k]
+  |  Branch (105:26): [True: 201, False: 83.5k]
   ------------------
-  106|  83.5k|  int r = memcmp(data_, b.data_, min_len);
-  107|  83.5k|  if (r == 0) {
+  106|  83.7k|  int r = memcmp(data_, b.data_, min_len);
+  107|  83.7k|  if (r == 0) {
   ------------------
-  |  Branch (107:7): [True: 51.7k, False: 31.8k]
+  |  Branch (107:7): [True: 51.8k, False: 31.9k]
   ------------------
-  108|  51.7k|    if (size_ < b.size_)
+  108|  51.8k|    if (size_ < b.size_)
   ------------------
-  |  Branch (108:9): [True: 0, False: 51.7k]
+  |  Branch (108:9): [True: 0, False: 51.8k]
   ------------------
   109|      0|      r = -1;
-  110|  51.7k|    else if (size_ > b.size_)
+  110|  51.8k|    else if (size_ > b.size_)
   ------------------
-  |  Branch (110:14): [True: 0, False: 51.7k]
+  |  Branch (110:14): [True: 0, False: 51.8k]
   ------------------
   111|      0|      r = +1;
-  112|  51.7k|  }
-  113|  83.5k|  return r;
-  114|  83.5k|}
+  112|  51.8k|  }
+  113|  83.7k|  return r;
+  114|  83.7k|}
   115|       |
   116|       |}  // namespace leveldb
   117|       |
@@ -69664,11 +69664,11 @@
    46|  30.3k|  return reinterpret_cast<char*>(ptr);
    47|  30.3k|}
    48|       |
-   49|  20.0k|void PutVarint32(std::string* dst, uint32_t v) {
-   50|  20.0k|  char buf[5];
-   51|  20.0k|  char* ptr = EncodeVarint32(buf, v);
-   52|  20.0k|  dst->append(buf, ptr - buf);
-   53|  20.0k|}
+   49|  20.1k|void PutVarint32(std::string* dst, uint32_t v) {
+   50|  20.1k|  char buf[5];
+   51|  20.1k|  char* ptr = EncodeVarint32(buf, v);
+   52|  20.1k|  dst->append(buf, ptr - buf);
+   53|  20.1k|}
    54|       |
    55|     89|char* EncodeVarint64(char* dst, uint64_t v) {
    56|     89|  static const int B = 128;
@@ -69979,9 +69979,9 @@
    24|       |
    25|      3|  const char* Name() const override { return "leveldb.BytewiseComparator"; }
    26|       |
-   27|  83.5k|  int Compare(const Slice& a, const Slice& b) const override {
-   28|  83.5k|    return a.compare(b);
-   29|  83.5k|  }
+   27|  83.7k|  int Compare(const Slice& a, const Slice& b) const override {
+   28|  83.7k|    return a.compare(b);
+   29|  83.7k|  }
    30|       |
    31|       |  void FindShortestSeparator(std::string* start,
    32|     37|                             const Slice& limit) const override {

The coverage was not determinstic between runs.
The fuzz target input was /bitcoin/qa-assets/fuzz_corpora/coins_db/0d249c3962392259953e8ce2148addd7ed92cda5.

l0rinc

I still can't run any fuzzing on my Mac (for the past ~5 months), so I only added code review comments based on what I found.

Concept ACK

l0rinc · 2025-02-19T13:55:27Z

src/test/fuzz/coins_view.cpp

@@ -207,7 +213,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend

    {
        std::unique_ptr<CCoinsViewCursor> coins_view_cursor = backend_coins_view.Cursor();
-        assert(!coins_view_cursor);
+        assert(is_db ? !!coins_view_cursor : !coins_view_cursor);


I thought I commented on this already, but seems I forgot to submit:
👍 for

Suggested change

assert(is_db ? !!coins_view_cursor : !coins_view_cursor);

assert(is_db == !!coins_view_cursor);

or maybe even

Suggested change

assert(is_db ? !!coins_view_cursor : !coins_view_cursor);

assert(is_db != !coins_view_cursor);

l0rinc · 2025-02-19T14:04:43Z

src/test/fuzz/coins_view.cpp

+        .memory_only = true,
+    };
+    CCoinsViewDB coins_db{std::move(db_params), CoinsViewOptions{}};
+    TestCoinsView(fuzzed_data_provider, coins_db, /* is_db = */ true);


The boolean parameter basically decides if the second parameter is a database view or not.
We could reduce this duplication by deducing it inside the TestCoinsView method instead:

void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend_coins_view) { const bool is_db{!!dynamic_cast<CCoinsViewDB*>(&backend_coins_view)};

or if you prefer:

void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend_coins_view) { const bool is_db{typeid(backend_coins_view) == typeid(CCoinsViewDB)};

l0rinc · 2025-02-19T14:08:37Z

src/test/fuzz/coins_view.cpp

+    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
+    auto db_params = DBParams{
+        .path = "", // Memory only.
+        .cache_bytes = 1 << 20, // 1MB.


nit: I know this was just copied over, but technically this is

Suggested change

.cache_bytes = 1 << 20, // 1MB.

.cache_bytes = 1 << 20, // 1MiB.

l0rinc · 2025-02-19T15:09:57Z

src/test/fuzz/coins_view.cpp

+    TestCoinsView(fuzzed_data_provider, backend_coins_view);
+}
+
+FUZZ_TARGET(coins_db, .init = initialize_coins_view)


The file name previously coincided with the only fuzz target - now a coins_view contains a coins_db target as well.
Given that the type is CCoinsViewDB, we could rename this to:

Suggested change

FUZZ_TARGET(coins_db, .init = initialize_coins_view)

FUZZ_TARGET(coins_view_db, .init = initialize_coins_view)

l0rinc · 2025-02-19T15:10:31Z

src/test/fuzz/coins_view.cpp

+{
+    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
+    auto db_params = DBParams{
+        .path = "", // Memory only.


This is already obvious from two lines below:

Suggested change

.path = "", // Memory only.

.path = "",

l0rinc · 2025-02-19T15:14:45Z

src/test/fuzz/coins_view.cpp

+                        // of cachedCoinsUsage would have been incorrectly decreased, leading to an underflow later on.
+                        // To avoid this, use Flush() to reset the value of cachedCoinsUsage in sync with the cacheCoins
+                        // mapping.
+                        (void)coins_view_cache.Flush();


What's the purpose of the comment?
Why not add that to the commit message only?

This comment seems to me to document relevant and lasting context for what is otherwise a non-sequitur, anyone touching this line in the future should know what motivated putting it here.

marcofleon · 2025-03-26T15:47:41Z

I explored the instablilty of the coins_db target some more and it seems to be in LevelDB, so I think we can ignore it as an issue for this fuzz test.

I generated two coverage reports: one with a corpus of inputs that were all stable (afl++ showing 99.3%) and then one with that same corpus but just a single added unstable input (afl++ showing ~75%). You can see that basically all of the differences in coverage are in LevelDB. If you look at the coverage for db_impl.cc you'll see that the one unstable input reaches the compaction code (probably seen clearest in MaybeScheduleCompaction()).

Anyway, all this to say that I don't think the instability should be a blocker for this harness. Unless we want to start changing LevelDB...

darosior · 2025-03-27T20:18:51Z

Closing my old open PRs. I have intended to come back to it for a while but as a matter of fact i did not. Anyone interested feel free to pick it up.

fanquake · 2025-05-23T15:03:55Z

Picked up in #32602.

cfc42ae fuzz: add a target for the coins database (Antoine Poinsot) 46e1463 fuzz: move the coins_view target's body into a standalone function (Antoine Poinsot) 56d878c fuzz: avoid underflow in coins_view target (Antoine Poinsot) Pull request description: This reopens #28216. The current `coins_view` target only tests `CCoinsViewCache` using a basic `CCoinsView` instance. The addition of the `coins_view_db` target enables testing with an actual `CCoinsViewDB` as the backend. ACKs for top commit: maflcko: lgtm ACK cfc42ae l0rinc: code review ACK cfc42ae TheCharlatan: ACK cfc42ae Tree-SHA512: d3a92f122629f075767453a1abd9819a1c9716db53b997418993fef62d27683324740d0a8f84df76d8a7a45e508ccadeb69553b6f69e29a1238cd7c0be5276ca

DrahtBot added the Tests label Aug 4, 2023

darosior force-pushed the fuzz_coins_db branch from 021dace to 43a766b Compare August 4, 2023 16:38

DrahtBot added the CI failed label Aug 4, 2023

darosior mentioned this pull request Aug 5, 2023

Flaky wallet_transactiontime_rescan.py --legacy-wallet functional test #28221

Closed

DrahtBot removed the CI failed label Aug 6, 2023

Allah1king approved these changes Aug 6, 2023

View reviewed changes

dergoegge mentioned this pull request Aug 10, 2023

fuzz: fix a couple incorrect assertions in the coins_view target #28215

Merged

darosior force-pushed the fuzz_coins_db branch from 43a766b to d776491 Compare August 15, 2023 10:33

DrahtBot added the CI failed label Aug 15, 2023

DrahtBot mentioned this pull request Aug 17, 2023

Don't empty dbcache on prune flushes: >30% faster IBD #28280

Merged

darosior force-pushed the fuzz_coins_db branch from d776491 to ad9cfb5 Compare August 17, 2023 09:12

DrahtBot removed the CI failed label Aug 17, 2023

DrahtBot added CI failed and removed CI failed labels Sep 4, 2023

DrahtBot added CI failed and removed CI failed labels Oct 19, 2023

DrahtBot mentioned this pull request Nov 8, 2023

fuzz: Avoid timeout and bloat in fuzz targets #28815

Merged

DrahtBot added the Needs rebase label Nov 8, 2023

darosior mentioned this pull request Jan 2, 2024

fuzz: avoid underflow in coins_view target #29164

Closed

darosior force-pushed the fuzz_coins_db branch from ad9cfb5 to a77c29a Compare January 2, 2024 10:54

maflcko mentioned this pull request Aug 7, 2024

fuzz: a target for the block index database #28209

Merged

DrahtBot added the Needs rebase label Aug 8, 2024

darosior added 2 commits February 18, 2025 13:55

fuzz: avoid underflow in coins_view target

16aaad3

fuzz: move the coins_view target's body into a standalone function

85b9b8d

We'll reuse it for a target where the coins view is a DB.

darosior force-pushed the fuzz_coins_db branch from e79784e to 4dafb32 Compare February 18, 2025 19:04

DrahtBot added the CI failed label Feb 18, 2025

fuzz: add a target for the coins database

ea0bcbc

It reuses the logic from the `coins_view` target, except it uses an in-memory CCoinsViewDB as the backend. Note `CCoinsViewDB` will assert the best block hash is never null, so we slightly modify the coins_view fuzz logic to take care of this.

darosior force-pushed the fuzz_coins_db branch from 4dafb32 to ea0bcbc Compare February 18, 2025 19:21

DrahtBot removed Needs rebase CI failed labels Feb 18, 2025

DrahtBot mentioned this pull request Feb 18, 2025

Use number of dirty cache entries in flush warnings/logs #31703

Open

davidgumberg reviewed Feb 19, 2025

View reviewed changes

l0rinc reviewed Feb 19, 2025

View reviewed changes

darosior closed this Mar 27, 2025

marcofleon mentioned this pull request May 23, 2025

fuzz: Add target for coins database #32602

Merged

	assert(is_db ? !!coins_view_cursor : !coins_view_cursor);
	assert(is_db == !!coins_view_cursor);

	assert(is_db ? !!coins_view_cursor : !coins_view_cursor);
	assert(is_db != !coins_view_cursor);

	.cache_bytes = 1 << 20, // 1MB.
	.cache_bytes = 1 << 20, // 1MiB.

	FUZZ_TARGET(coins_db, .init = initialize_coins_view)
	FUZZ_TARGET(coins_view_db, .init = initialize_coins_view)

fuzz: a new target for the coins database #28216

fuzz: a new target for the coins database #28216

Uh oh!

Conversation

darosior commented Aug 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

fanquake commented Aug 15, 2023

Uh oh!

darosior commented Aug 15, 2023

Uh oh!

darosior commented Aug 17, 2023

Uh oh!

darosior commented Dec 31, 2023

Uh oh!

darosior commented Jan 2, 2024

Uh oh!

darosior commented Jun 27, 2024

Uh oh!

darosior commented Jul 28, 2024

Uh oh!

dergoegge commented Jul 30, 2024

Uh oh!

maflcko commented Nov 8, 2024

Uh oh!

maflcko commented Jan 22, 2025

Uh oh!

DrahtBot commented Feb 18, 2025

Uh oh!

davidgumberg Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidgumberg commented Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

l0rinc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcofleon commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darosior commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented May 23, 2025

Uh oh!

Uh oh!

darosior commented Aug 4, 2023 •

edited

Loading

DrahtBot commented Aug 4, 2023 •

edited

Loading

davidgumberg Feb 19, 2025 •

edited

Loading

davidgumberg commented Feb 19, 2025 •

edited

Loading

l0rinc left a comment •

edited

Loading

marcofleon commented Mar 26, 2025 •

edited

Loading

darosior commented Mar 27, 2025 •

edited

Loading