descriptor_parse: extend coverage with Miniscript Descriptors inputs #92

pull darosior wants to merge 2 commits into bitcoin-core:main from darosior:miniscript_descriptor_parse changing 56912 files +56890 −0

darosior commented at 12:19 PM on July 4, 2022: member

No description provided.
descriptor_parse: base miniscript coverage 1e3c7cd69f
darosior cross-referenced this on Jul 4, 2022 from issue Miniscript corpus for `descriptor_parse` by darosior
MarcoFalke commented at 12:27 PM on July 4, 2022: none

There is already basic coverage: https://github.com/bitcoin-core/qa-assets/tree/main/fuzz_seed_corpus/descriptor_parse

So I presume you are extending it?
darosior commented at 12:30 PM on July 4, 2022: member

Hmm, i'm not so good at english. What i meant here was to add initial coverage for miniscript descriptors, but yeah, it is extending the descriptors coverage with new inputs (those that contain miniscripts inside).
darosior renamed this:
~~descriptor_parse: base miniscript coverage~~
descriptor_parse: extend coverage with Miniscript Descriptors inputs
on Jul 4, 2022
MarcoFalke commented at 12:35 PM on July 4, 2022: none

Oh, I see. So this is exclusively adding inputs with miniscript coverage after https://github.com/bitcoin/bitcoin/pull/24148 is merged?
darosior commented at 12:38 PM on July 4, 2022: member

Yes. I initially seeded a folder with Miniscript descriptors and ran the fuzzer on it for a while. This is the minified version of the resulted folder.
Rspigler cross-referenced this on Jul 4, 2022 from issue Awesome multisig PR labyrinth guide by Sjors
darosior cross-referenced this on Jul 5, 2022 from issue Miniscript support in Output Descriptors by darosior
achow101 referenced this in commit 85b601e043 on Jul 14, 2022
darosior commented at 11:57 AM on July 15, 2022: member

Added some more seeds i've been generating in the background the past week(s). As a commit since it's not specific to Miniscript descriptors, it's extending the whole descriptor_parse corpus.

Marking as ready now that https://github.com/bitcoin/bitcoin/pull/24148 is merged (:tada:).
Extend the descriptor_parse corpus 8df0eb8c12
darosior marked this as ready for review on Jul 15, 2022
darosior commented at 12:01 PM on July 15, 2022: member

(I used a rebased https://github.com/bitcoin/bitcoin/pull/25540 to generate and merge those.)

MarcoFalke commented at 3:01 PM on July 15, 2022: none

pubkey.cpp:368:18: runtime error: implicit conversion from type 'int' of value 256 (32-bit, signed) to type 'unsigned char' changed the value to 0 (8-bit, unsigned)

darosior commented at 4:31 PM on July 18, 2022: member
How did you trigger this? I couldn't reproduce with --with-sanitizers=fuzzer,undefined running
```
FUZZ=descriptor_parse ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/descriptor_parse -runs=0
```
on 088e5b0b9317bd1d7c9755fb839b7326d99bc910 (https://github.com/bitcoin/bitcoin/pull/25540's tip).
sidhujag referenced this in commit 2a8811a52a on Jul 18, 2022
MarcoFalke commented at 7:19 AM on July 19, 2022: none

It needs the integer sanitizer, which is not the UB sanitizer. :sweat_smile:

The input that triggers this is ./4a30defd5a644721859db1781153d0fb34decd94.
darosior commented at 10:28 AM on July 19, 2022: member

Err, right.. :sweat_smile: I've opened a PR fixing the wrap-around: https://github.com/bitcoin/bitcoin/pull/25642.
MarcoFalke commented at 1:49 PM on August 11, 2022: none

Now it times out due to the timeout issue
darosior commented at 1:51 PM on August 11, 2022: member

Yes, we need https://github.com/bitcoin/bitcoin/pull/25540 in first
darosior cross-referenced this on Sep 17, 2022 from issue miniscript: avoid wasteful computation, prevent memory blowup when fuzzing by darosior
MarcoFalke merged this on Sep 20, 2022
MarcoFalke closed this on Sep 20, 2022

Contributors

darosior

MarcoFalke

Linked (view graph)

#86 Miniscript corpus for `descriptor_parse`