policy: Add new constant MAX_STANDARD_MULTISIG_KEYS #6902
Conversation
| // Support up to x-of-3 multisig txns as standard | ||
| if (n < 1 || n > 3) | ||
| // Support up to x-of-MAX_STANDARD_MULTISIG_KEYS multisig txns as | ||
| // standard |
There was a problem hiding this comment.
Maybe Support up to m-of-MAX_ ... (note, the m, not x)
|
utACK |
|
@dcousens Agree. Updated. |
|
Trivial ACK |
|
I think the result of this is confusing, if you go read policy.h post patch you think that bitcoin multisig will not allow more keys. But this isn't true, the restriction is specific to bare multisig-- something virtually no one cares about-- which was more or less apparent from the code in context but isn't otherwise; and in the updated version the new comment is arguably outright factually incorrect. FWIW, the constant movement of trivial bits of code behavior across multiple files is very frustrating to me. The net result of this is that I am less likely to review future changes that changes around that test in IsStandard, because I have to go scatter and read many different files (which involves first figuring out where its defined) to have any idea of what the code actually does. |
|
IMO this should become an option or bare multisig made unconditionally non-IsStandard. |
|
@gmaxwell Seems that at least the comment should be fixed to say it's about bare multisig. |
| * The maximum number of public keys allowed in a multi-signature transaction | ||
| * output script to be considered standard. | ||
| */ | ||
| static const unsigned int MAX_STANDARD_MULTISIG_KEYS = 3; |
There was a problem hiding this comment.
Maybe MAX_STANDARD_BARE_MULTISIG_KEYS, in light of @gmaxwell's comment
|
Agreed the indirection of extracting constants can be frustrating for review, but, IMHO the only [equally balanced] alternative is to remove the constants themselves. This is probably an exception though, given it is a 1:1 mapping and the motivation for extraction is solely a style choice. If we have an answer to that, this could be a trivial ACK or NACK. |
|
Too controversial for a trivial change, closing |
No description provided.