Add fuzz input maintenance docs to the readme #121

dergoegge commented at 11:23 AM on May 2, 2023: member

No description provided.

in README.md:19 in 8785839d87 outdated

  15 | +
  16 | +If you want to contribute fuzz inputs, please "merge" the inputs before
  17 | +submitting a pull request. You can use the libFuzzer option `-merge=1`
  18 | +(recommended with `-use_value_profile=1`) or the `--m_dir` option of the fuzz
  19 | +runner:
  20 | +[`test_runner.py`](https://github.com/bitcoin/bitcoin/blob/master/test/fuzz/test_runner.py).

maflcko commented at 11:28 AM on May 2, 2023:

Can modify PULL_REQUEST_TEMPLATE to link to this section to avoid duplication?

LadyS8214 commented at 3:15 AM on September 27, 2023:

LadyS8214

in README.md:25 in 8785839d87 outdated

  21 | +
  22 | +### Pruning non-reduced inputs 
  23 | +
  24 | +Over time fuzz engines reduce inputs (produce a smaller input that yields the
  25 | +same coverage statistics). This causes our copora to accumulate larger
  26 | +non-reduced inputs.

maflcko commented at 9:04 AM on May 3, 2023:

Could also mention that inputs (or their coverage) may get stale over time due to code changes?

in README.md:30 in 8785839d87 outdated

  26 | +non-reduced inputs.
  27 | +
  28 | +To avoid corpora bloat and potential CI timeouts, we usually prune/minimize our
  29 | +copora around the branch-off point using the
  30 | +[`delete_nonreduced_fuzz_inputs.sh`](https://raw.githubusercontent.com/bitcoin-core/bitcoin-maintainer-tools/main/delete_nonreduced_fuzz_inputs.sh)
  31 | +script (Recommended to run in a fresh VM, see documentation in the script).

maflcko commented at 9:04 AM on May 3, 2023:

Maybe also doc that one should keep an eye on coverage to ensure it doesn´t drop?

in README.md:37 in 8785839d87 outdated

  27 | +
  28 | +To avoid corpora bloat and potential CI timeouts, we usually prune/minimize our
  29 | +copora around the branch-off point using the
  30 | +[`delete_nonreduced_fuzz_inputs.sh`](https://raw.githubusercontent.com/bitcoin-core/bitcoin-maintainer-tools/main/delete_nonreduced_fuzz_inputs.sh)
  31 | +script (Recommended to run in a fresh VM, see documentation in the script).
  32 | +

maflcko commented at 9:05 AM on May 3, 2023:

Maybe also doc that it is recommended to run the script twice, even though it is not reproducible?

See also #119 (comment)

dergoegge commented at 10:51 AM on May 4, 2023:

By running twice you mean running it twice on the non-reduced corpus to try and see if the result is similar? Or running it again on the reduced corpus from the first run?

maflcko commented at 10:53 AM on May 4, 2023:

The script clones fresh, the current state, before the pull is merged. So it will run twice on the non-reduced corpora.

dergoegge force-pushed on May 4, 2023

in README.md:35 in 885a48525c outdated

  31 | +[`delete_nonreduced_fuzz_inputs.sh`](https://raw.githubusercontent.com/bitcoin-core/bitcoin-maintainer-tools/main/delete_nonreduced_fuzz_inputs.sh)
  32 | +script (Recommended to run in a fresh VM, see documentation in the script). The
  33 | +script is usually run twice to ensure that the results are "somewhat"
  34 | +reproducible.
  35 | +
  36 | +After pruning the corpora, the coverage should not have dropped significantly.

maflcko commented at 11:31 AM on May 4, 2023:

After pruning the corpora, the coverage should not have dropped at all.

dergoegge commented at 12:19 PM on May 4, 2023:

If the code for a target changed and that invalidated the inputs, then we might see a drop in coverage, no?

maflcko commented at 12:22 PM on May 4, 2023:

Yes, but that is unrelated to removing non-reduced and stale inputs. The coverage should be identical before and after removing non-reduced and stale inputs, because the code doesn't change.

in README.md:33 in 885a48525c outdated

  29 | +To avoid corpora bloat, stale inputs and potential CI timeouts, we usually
  30 | +prune/minimize our copora around the branch-off point using the
  31 | +[`delete_nonreduced_fuzz_inputs.sh`](https://raw.githubusercontent.com/bitcoin-core/bitcoin-maintainer-tools/main/delete_nonreduced_fuzz_inputs.sh)
  32 | +script (Recommended to run in a fresh VM, see documentation in the script). The
  33 | +script is usually run twice to ensure that the results are "somewhat"
  34 | +reproducible.

maflcko commented at 11:32 AM on May 4, 2023:

Could add a link to #119 (comment) ?

maflcko approved

maflcko commented at 11:32 AM on May 4, 2023: contributor

lgtm

Add fuzz input maintenance docs to the readme ce813b8fcd

Link to readme from pull request template 0b76987467

in README.md:26 in 885a48525c outdated

  22 | +### Pruning inputs 
  23 | +
  24 | +* Over time fuzz engines reduce inputs (produce a smaller input that yields the
  25 | +  same coverage statistics), which causes our copora to accumulate larger
  26 | +  non-reduced inputs. 
  27 | +* Code changes can lead to inputs loosing their coverage.

maflcko commented at 11:33 AM on May 4, 2023:

* Code changes can lead to inputs losing their coverage.

dergoegge force-pushed on May 4, 2023

maflcko approved

maflcko commented at 1:54 PM on May 4, 2023: contributor

lgtm

in README.md:29 in 0b76987467

  25 | +  same coverage statistics), which causes our copora to accumulate larger
  26 | +  non-reduced inputs. 
  27 | +* Code changes can lead to inputs losing their coverage.
  28 | +
  29 | +To avoid corpora bloat, stale inputs and potential CI timeouts, we usually
  30 | +prune/minimize our copora around the branch-off point using the

maflcko commented at 1:55 PM on May 4, 2023:

unrelated: Maybe create a link to this section in the release process docs?

dergoegge commented at 1:56 PM on May 4, 2023:

Will do once this is merged.

fanquake merged this on May 4, 2023

fanquake closed this on May 4, 2023

maflcko commented at 3:12 PM on May 5, 2023: contributor

Reminds me that it is also possible to cross-polinate inputs to sync spent CPU time between compatible fuzz target formats. See 3368bee6bd327f74cd43eda78593091b9e44cff3

maflcko commented at 10:50 AM on May 12, 2023: contributor

Also reminds me that wallet_notifications has all fuzz inputs removed (I have a local gitingore for the folder), because it would otherwise cause CI timeouts.

See also another fuzz target that may cause timeouts: https://github.com/bitcoin/bitcoin/pull/17860#issuecomment-1538252773

LadyS8214 approved