fuzz: avoid underflow in coins_view target #29164

1. 
   darosior commented at 10:54 am on January 2, 2024: member

   Catching the exception in AddCoin and continuing will lead to the calculated memory usage of the cache to be out of sync (lower by the size of the existing coin). If we later remove more coins, this can lead to an underflow and the sanitizer will trip on a later overflow when we add back to cachedCoinsUsage. See #28216 (comment).

   I found this while rebasing #28216 but another way of exposing this mistake is to simply run the cache sanity check after the LIMITED_WHILE loop. This PR fixes the underflow by not continuing when hitting this exception and adds the sanity check.

   The crash can be reproduced with the following seed:

   0q6urNzc3NzfHx8fHx8fHx8fHx8cBpwAAAAD//wD/////7/+CAAAAAAAAAAGCgv9Z/////////8dDbwBcXW+XbyEhL01BTklGRVNULTAwMCEhBQUFBQUFBQX///////8hISMjIyP/ISHd//9rQ0P/AQAAISEhISMjIyP/ISHd//9rQ0MhISEFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQAFBf8BAAAhISEhIyMjI/8hISEhISMjIyP/ISHd//9rQ0P/AQAAISEhISMjIyP/ISHd//9rQ0MhIUM/ISEhIf//////AH//////AQAAISEhISMjIyP/ISHd//9rQ0P/AQAAISEhISMjIyP/ISEhISEjIyMj/yEh3f//a0ND/wEAACEhISEjIyMj/yEh3f//a0NDISEhBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUABQUFBQUFx/9rQ0P/AQAAISEhISMjIyP/ISHd//9rQ0MhISEFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQAFBQUFBQXHx8fHx8fHx8fHx8fHxzc3Nzc3NzfHx8fHx8cFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFIf//ISEhISMjIyP/ISHd//////////////8x////////////ISHd//////////////8y/////+//////////////////f2vHx0ND/8fHxzc3Nzc3Nzc=
   

2. fuzz: avoid underflow in coins_view target 67d02fb5bf
3. fuzz: sanity check the coin cache memory usage in coins_view target bc22ae1a68
4. 
   DrahtBot commented at 10:54 am on January 2, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

5. DrahtBot added the label Tests on Jan 2, 2024
6. fanquake requested review from dergoegge on Jan 2, 2024
7. 
   maflcko commented at 11:37 am on January 2, 2024: member

   0fuzz: coins.cpp:338: void CCoinsViewCache::SanityCheck() const: Assertion `recomputed_usage == cachedCoinsUsage' failed.
   

8. DrahtBot added the label CI failed on Jan 2, 2024
9. 
   darosior commented at 11:46 am on January 2, 2024: member

   My bad, i ran the new coins_db introduced in #28216 for a while but not this one.. Will debug to find the other source of inconsistency.
10. 
    maflcko commented at 11:54 am on January 2, 2024: member

    The crash can be reproduced with the following seed:

    On what commit? For me it seems to pass on current master.

11. 
    darosior commented at 11:55 am on January 2, 2024: member

    You need to add the sanity check to trigger it.
12. 
    darosior commented at 12:28 pm on January 2, 2024: member

    Sigh… I’m tired of this target just doing random things. Turns out you can’t perform the sanity checks because it’s using a dummy backend where Flush() won’t actually erase the cache. I’ll just use a hack to avoid the crash in #28216.
13. darosior closed this on Jan 2, 2024

    ---

14. bitcoin locked this on Jan 1, 2025

Contributors
darosior DrahtBot maflcko

Review Requested
dergoegge

Labels
Tests CI failed