fuzz: Deglobalize signature cache in sigcache test #30447

1. 
   TheCharlatan commented at 11:03 am on July 13, 2024: contributor

   The body of the fuzz test should ideally be a pure function. If data is persisted in the cache over many iterations, and there is a crash, reproducing it from the input might be difficult. Solve this by getting rid of the global state. This is a follow-up from #30425.
2. 
   DrahtBot commented at 11:03 am on July 13, 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.

   Type	Reviewers
   ACK	dergoegge, ryanofsky

   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:

   • #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)

   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.

3. DrahtBot added the label Tests on Jul 13, 2024
4. in src/test/fuzz/script_sigcache.cpp:31 in eb3d9cd7ab outdated

   39+FUZZ_TARGET(script_sigcache)
   40 {
   41     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
   42 
   43+    const auto max_sigcache_bytes{fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, DEFAULT_SIGNATURE_CACHE_BYTES)};
   44+    SignatureCache signature_cache{max_sigcache_bytes};
   

   ---

   ---

   

   dergoegge commented at 8:42 am on July 15, 2024:

   I think we still need to setup logging or this test will oom because this constructor logs.
5. TheCharlatan force-pushed on Jul 19, 2024
6. 
   TheCharlatan commented at 11:42 am on July 19, 2024: contributor

   Updated eb3d9cd7ab2fa3d588325ec3352ce914c1d94880 -> ae2a4c8f6e0b44cd69b057ae8bc3d542786413e4 (fuzzScriptCache_0 -> fuzzScriptCache_1, compare)

   • Addressed @dergoegge’s comment, keeping testing setup initialization without logging.
7. fuzz: Deglobalize signature cache in sigcache test

   The body of the fuzz test should ideally be a pure function. If data is
   persisted in the cache over many iterations, and there is a crash,
   reproducing it from the input might be difficult.

   fae0db0360
8. in src/test/fuzz/script_sigcache.cpp:26 in ae2a4c8f6e outdated

   34-    g_setup = testing_setup.get();
   35-    g_signature_cache = &signature_cache;
   36 }
   37 
   38-FUZZ_TARGET(script_sigcache, .init = initialize_script_sigcache)
   39+FUZZ_TARGET(script_sigcache)
   

   ---

   ---

   

   dergoegge commented at 1:26 pm on July 19, 2024:

   0FUZZ_TARGET(script_sigcache, .init = initialize_script_sigcache)
   

   ---

   

   TheCharlatan commented at 3:17 pm on July 19, 2024:

   damn…
9. TheCharlatan force-pushed on Jul 19, 2024
10. 
    TheCharlatan commented at 3:20 pm on July 19, 2024: contributor

    Updated ae2a4c8f6e0b44cd69b057ae8bc3d542786413e4 -> fae0db0360466aed536f4ce96d357cf579100080 (fuzzScriptCache_1 -> fuzzScriptCache_2, compare)

    • Applied @dergoegge’s suggestion, fixing call to init.
11. dergoegge approved
12. 
    dergoegge commented at 3:37 pm on July 19, 2024: member

    utACK fae0db0360466aed536f4ce96d357cf579100080
13. in src/test/fuzz/script_sigcache.cpp:30 in fae0db0360

    37 
    38 FUZZ_TARGET(script_sigcache, .init = initialize_script_sigcache)
    39 {
    40     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
    41 
    42+    const auto max_sigcache_bytes{fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, DEFAULT_SIGNATURE_CACHE_BYTES)};
    

    ---

    ---

    

    darosior commented at 10:14 am on July 22, 2024:

    nit: this would invalidate existing seeds.

    ---

    

    dergoegge commented at 12:30 pm on July 23, 2024:

    Should be fine for this test since there isn’t a lot of reachable code here, i.e. coverage quickly plateaus.

    Could be avoided by fixing the cache size but I don’t think it matters too much.

14. ryanofsky approved
15. 
    ryanofsky commented at 11:18 am on July 26, 2024: contributor

    Code review ACK fae0db0360466aed536f4ce96d357cf579100080
16. ryanofsky merged this on Jul 26, 2024
17. ryanofsky closed this on Jul 26, 2024

    ---

Contributors
TheCharlatan DrahtBot dergoegge darosior ryanofsky

Labels
Tests