-
Notifications
You must be signed in to change notification settings - Fork 37.8k
fuzz: compact block harness #33300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
fuzz: compact block harness #33300
Conversation
This allows other fuzz tests to use the function to create valid block headers.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33300. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
ea293e3
to
c426b80
Compare
Is my understanding correct that AFL will run init (create the static dir), then fork into several fuzz processes, which run for N iterations and then shut down and delete the static dir? (On the next re-fork, it will fail to find the static dir)? If yes, a solution could be to lazy-init the static dir after init, after the fork. Also, you may have to use the pid (or os-rand) to name the static folder to avoid collisions. As for deleting the dirs, maybe it is possible to spin up several testing setups at the same time:
(Not sure if this is possible, though) |
We removed the |
My description was a bit vague. Since the harness uses
I considered this, but I think this introduces some (slight) non-determinism as the directory may/may not exist?
Yup, I agree. I think this randomization can only be done if the static datadir can be deleted at the end of fuzzing, otherwise there will be lots of these lying around?
Why is the dummy testing setup needed in this example? Could it instead just be two testing setups (one for the static datadir, one for the copied datadir)? I considered something similar to this as well where there is a |
c426b80
to
c76f475
Compare
I was able to get AFL++ multi-core fuzzing to work by adding a static |
Disable implicit string conversion.
This harness does one of the following operations in a loop: - send a cmpctblock message to the test node - send a blocktxn message to the test node - send a headers message to the test node - send a sendcmpct message to the test node - send a tx message to the test node - mine a block - set mock time The initialize function creates a TestingSetup and mines 200 blocks. Each fuzz iteration will then create a TestingSetup and copy the cached data directory via -fuzzcopydatadir.
c76f475
to
0de9143
Compare
Posting up to get feedback, there are some design flaws with the approach in this PR. Coverage is here (look in
src/blockencodings.cpp
, relevant compact block bits insrc/net_processing.cpp
).This harness can make (in)valid blocks, reconstruct blocks with in-mempool txns, mark peers as HB, and has high stability in AFL++ (~98-99%).
The main downside is that there are filesystem operations. In the
.init
functioninitialize_cmpctblock
, a chain of 200 blocks is created. Each fuzzing iteration then copies this statically-named, "cached" data directory to a temporary directory that gets deleted at the end of the iteration. If each fuzzing iteration instead mines its own chain, the execs/s slows down to a crawl (~0.5/s or less, which would also make CI runs really slow).