fuzz: fix checks on number of required sigs and keys in multisig scripts #23091
Conversation
Fuzz test coverage was added around the ExtractDestination(s) functions in a29f522. This commit contained an incorrect assertion that the number of required signatures in a multisig script was equal to the number of addresses. This is incorrect, as for an m-of-n multisig, m <= n. a29f522 also had an incorrect assertion on the maximum number of public keys per multisig. It checked that the number of keys was less than or equal to 16. This is incorrect, as it should be <= 20 (see MAX_PUBKEYS_PER_MULTISIG). Both of these incorrect assertions are fixed in this commit accordingly. Note: this is sort of moot because this behavior is deprecated and these fuzz tests are slated for removal in v23 when the deprecation period for -deprecatedrpc=addresses ends. However, for correctness and just in case, it's fixed here.
c81b149 to
09d29b5
Compare
Nice catch @darosior, I fixed as suggested in this commit |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
|
PR #22650 got merged before this, which deleted all of the code this would have corrected, therefore this PR is unnecessary and I'm closing it |
Fuzz test coverage was added around the ExtractDestination(s) functions in
a29f522. This commit contained an incorrect
assertion that the number of required signatures in a multisig script was equal
to the number of addresses. This is incorrect, as for an m-of-n multisig, m <= n.
a29f522 also had an incorrect assertion on
the maximum number of public keys per multisig. It checked that the number of
keys was less than or equal to 16. This is incorrect, as it should be <= 20
(see MAX_PUBKEYS_PER_MULTISIG).
Both of these incorrect assertions are fixed in this commit accordingly.
Note: this is sort of moot because this behavior is deprecated and these
fuzz tests are slated for removal in v23 when the deprecation period for
-deprecatedrpc=addresses ends. However, for correctness and just in case,
it's fixed here.
Noticed here: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=39152
Moot because this PR #22650 gets rid of all this code anyways