fuzz: bolster ExtractDestination(s) checks #20772

pull mjdietzx wants to merge 1 commits into bitcoin:master from mjdietzx:test-fuzz-extract-destinations changing 1 files +38 −10

mjdietzx commented at 3:31 PM on December 26, 2020: contributor

No description provided.
DrahtBot added the label Tests on Dec 26, 2020
DrahtBot cross-referenced this on Dec 26, 2020 from issue [POC/DRAFT] - Finalize remove reqsigs deprecation from rpcs by mjdietzx
DrahtBot commented at 6:05 PM on December 26, 2020: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

No conflicts as of last run.
DrahtBot cross-referenced this on Dec 26, 2020 from issue fuzz: check that certain script TxoutType are nonstandard by mjdietzx
mjdietzx force-pushed on Dec 26, 2020
DrahtBot added the label Needs rebase on Dec 26, 2020
mjdietzx force-pushed on Dec 27, 2020
DrahtBot removed the label Needs rebase on Dec 27, 2020
mjdietzx force-pushed on Dec 27, 2020

in src/test/fuzz/script.cpp:62 in 0a7f327027 outdated

  67 | -    (void)InferDescriptor(script, signing_provider);
  68 | -
  69 | -    (void)IsSegWitOutput(signing_provider, script);
  70 | -
  71 | -    (void)IsSolvable(signing_provider, script);
  72 | +    if (!ExtractDestinations(script, type_ret, addresses, required_ret)) {

sipa commented at 5:02 AM on December 27, 2020:

This code could be significantly more efficient by just calling ExtractDestination(s) once each, caching the result, and then making assertions on those results.

sipa commented at 5:03 AM on December 27, 2020: member

Concept ACK, but probably not worth spending too much time on if ExtractDestinations is going away anyway.
mjdietzx cross-referenced this on Dec 27, 2020 from issue rpc: deprecate `addresses` and `reqSigs` from rpc outputs by mjdietzx
practicalswift commented at 10:39 PM on December 27, 2020: contributor

Concept ACK

Thanks for improving the fuzzing harnesses!
mjdietzx force-pushed on Dec 29, 2020
fuzz: bolster ExtractDestination(s) checks a29f522ba4
mjdietzx force-pushed on Jan 3, 2021
mjdietzx commented at 5:40 PM on January 3, 2021: contributor

rebased to master - fixed conflicts after #20765 was merged

in src/test/fuzz/script.cpp:75 in a29f522ba4

  80 | +                   type_ret == TxoutType::NONSTANDARD ||
  81 | +                   type_ret == TxoutType::NULL_DATA);
  82 | +        }
  83 | +    } else {
  84 | +        assert(required_ret >= 1 && required_ret <= 16);
  85 | +        assert((unsigned long)required_ret == addresses.size());

practicalswift commented at 10:36 PM on March 20, 2021:

Nit: size_t instead of unsigned long to match the return type of size()?

practicalswift commented at 10:38 PM on March 20, 2021: contributor

Tested ACK a29f522ba4aa71582b54025c5682b4c1687ae9f3

Thanks for adding assertions to existing fuzzing harnesses: that's a great way to improve the value provided by them!

Feel free to ignore nit.
adamjonas commented at 5:13 PM on April 27, 2021: member

Tested a29f522ba4aa71582b54025c5682b4c1687ae9f3 with FUZZ=script src/test/fuzz/fuzz qa-assets/fuzz_seed_corpus/script. No problems.
MarcoFalke merged this on Apr 28, 2021
MarcoFalke closed this on Apr 28, 2021
sidhujag referenced this in commit 095694c116 on Apr 28, 2021
mjdietzx cross-referenced this on Sep 24, 2021 from issue Remove -deprecatedrpc=addresses flag and corresponding code/logic by mjdietzx
gwillen referenced this in commit ae471dc6a5 on Jun 1, 2022
bitcoin locked this on Aug 18, 2022

Contributors

Labels

Linked (view graph)

#20286 rpc: deprecate `addresses` and `reqSigs` from rpc outputs #22650 Remove -deprecatedrpc=addresses flag and corresponding code/logic

fuzz: bolster ExtractDestination(s) checks #20772

Conflicts