fuzz: fix dead HD keypaths (de)serialization round-trip#35481
Conversation
`DeserializeHDKeypaths()` was writing into the original `hd_keypaths` map instead of `deserialized_hd_keypaths`. As a result the latter was always empty and the round-trip assertion following was trivially true, so the serialize/deserialize round-trip wasn't actually being exercised. That bug was introduced with the commit introducing the fuzz target (commit f898ef6, bitcoin#18994).
|
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/35481. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste |
| try { | ||
| DeserializeHDKeypaths(serialized, key, hd_keypaths); | ||
| DeserializeHDKeypaths(serialized, key, deserialized_hd_keypaths); | ||
| } catch (const std::ios_base::failure&) { |
There was a problem hiding this comment.
do we still need the try/catch? Doesn't that kinda' defeat the purpose of a fuzzer?
There was a problem hiding this comment.
afaict the key can still have a bad size here, which triggers a try/catch, no?
There was a problem hiding this comment.
shouldn't we guard for that specifically and abort in other cases? Otherwise what's the point of this call, to have fake coverage?
4 participants
DeserializeHDKeypaths()was writing into the originalhd_keypathsmap instead ofdeserialized_hd_keypaths. As a result the latter was always empty and the round-trip assertion following was trivially true, so the serialize/deserialize round-trip wasn't actually being exercised.That bug was introduced with the commit introducing the fuzz target (commit f898ef6, #18994).