fuzz: Remove expensive and redundant muhash from crypto fuzz target #21185

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2102-speedupFuzz changing 2 files +10 −13

MarcoFalke commented at 11:25 AM on February 15, 2021: member

Remove because it is redundant with src/test/fuzz/muhash.cpp and incredibly expensive
MarcoFalke renamed this:
~~fuzz: Remove expensive and redundant from cryto fuzz target~~
fuzz: Remove expensive and redundant muhash from cryto fuzz target
on Feb 15, 2021
fanquake added the label Tests on Feb 15, 2021
MarcoFalke force-pushed on Feb 15, 2021
MarcoFalke commented at 11:35 AM on February 15, 2021: member

On my system the times are 2s vs 567s (in valgrind)
MarcoFalke renamed this:
~~fuzz: Remove expensive and redundant muhash from cryto fuzz target~~
fuzz: Remove expensive and redundant muhash from crypto fuzz target
on Feb 15, 2021
MarcoFalke force-pushed on Feb 15, 2021
MarcoFalke commented at 12:42 PM on February 15, 2021: member

(fixed typos is commit message. Apparently I can't spell)
practicalswift commented at 1:01 PM on February 15, 2021: contributor

Concept ACK on removing redundancy (in relation to src/test/fuzz/muhash.cpp)
fuzz: Remove expensive and redundant muhash from crypto fuzz target ffff84a9cb
in src/test/fuzz/crypto.cpp:68 in fabda1036c outdated
```
  60 | @@ -63,12 +61,6 @@ FUZZ_TARGET(crypto)
  61 |                  (void)Hash(data);
  62 |                  (void)Hash160(data);
  63 |                  (void)sha512.Size();
  64 | -
  65 | -                if (fuzzed_data_provider.ConsumeBool()) {
  66 | -                    muhash *= MuHash3072(data);
```
practicalswift commented at 1:07 PM on February 15, 2021:

Nit: I don't think operator*=(const MuHash3072&) is exercised in src/test/fuzz/muhash.cpp. Perhaps worth expanding src/test/fuzz/muhash.cpp to keep coverage intact? :)

If doing so, consider fuzz covering the remaining currently non covered member functions of MuHash3072.

MarcoFalke commented at 1:50 PM on February 15, 2021:

thanks, done
MarcoFalke force-pushed on Feb 15, 2021
practicalswift commented at 8:53 PM on February 15, 2021: contributor

Tested ACK ffff84a9cb659562b3f560d3a489d4a62c71f793

FUZZ=crypto now super fast (as expected) and FUZZ=muhash super slow (as expected).

ffff prefix the new fa prefix? Such proof of work! 🕵️‍♀️
MarcoFalke merged this on Feb 16, 2021
MarcoFalke closed this on Feb 16, 2021
MarcoFalke deleted the branch on Feb 16, 2021
sidhujag referenced this in commit e53991358b on Feb 16, 2021
DrahtBot locked this on Aug 18, 2022

Contributors

MarcoFalke

practicalswift

Labels

Tests