fuzz: Deglobalize signature cache in sigcache test #30447
pull TheCharlatan wants to merge 1 commits into bitcoin:master from TheCharlatan:fuzzScriptCache changing 1 files +10 −13-
TheCharlatan commented at 11:03 am on July 13, 2024: contributorThe 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.
-
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.
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.
-
DrahtBot added the label Tests on Jul 13, 2024
-
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.TheCharlatan force-pushed on Jul 19, 2024TheCharlatan commented at 11:42 am on July 19, 2024: contributorUpdated eb3d9cd7ab2fa3d588325ec3352ce914c1d94880 -> ae2a4c8f6e0b44cd69b057ae8bc3d542786413e4 (fuzzScriptCache_0 -> fuzzScriptCache_1, compare)
- Addressed @dergoegge’s comment, keeping testing setup initialization without logging.
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.
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…TheCharlatan force-pushed on Jul 19, 2024TheCharlatan commented at 3:20 pm on July 19, 2024: contributorUpdated ae2a4c8f6e0b44cd69b057ae8bc3d542786413e4 -> fae0db0360466aed536f4ce96d357cf579100080 (fuzzScriptCache_1 -> fuzzScriptCache_2, compare)
- Applied @dergoegge’s suggestion, fixing call to init.
dergoegge approveddergoegge commented at 3:37 pm on July 19, 2024: memberutACK fae0db0360466aed536f4ce96d357cf579100080in 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.
ryanofsky approvedryanofsky commented at 11:18 am on July 26, 2024: contributorCode review ACK fae0db0360466aed536f4ce96d357cf579100080ryanofsky merged this on Jul 26, 2024ryanofsky closed this on Jul 26, 2024
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-08 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me