See specification at https://github.com/bitcoincashorg/bitcoincash.org/pull/375
deadalnix Mengerian jasonbcox Fabien
- Group Reviewers
- Maniphest Tasks
- T528: Add Schnorr support to OP_CHECKMULTISIG (new mechanics)
You are much better off working with unsigned when you handle bitfields.
This is not the check you want to be doing.
You want to check that:
Use proper variable instead of mutating other variable that mean something other than being a loop counter.
You also probably want to have two loops, one to validate pubkey encoding and one to validate sigs. There is no point starting to validate sigs. More generally, you shoudl try to simplify this loop by shaving stuff out of it.
When you nest most of the logic in an if, you indicate that you probably should bail the opposite branch early.
It's not clear what this error stands for. It needs a better name.
Having a separate loop here to check pubkey encoding results is strange behavior: It would check all pubkeys for successful Schnorr multisigs, but not for the "0-of-N" or "false" cases. (Or for the legacy case).
Two potential alternatives are:
A separate loop to validate pubkey encoding would likely mean a change in behaviour -- all pubkeys must be correctly encoded. Is that intended?
When the predicate here is false, we aren't bailing. There are still statements that need to be executed.
I agree, any suggestions?
It doesn't check for 0 of n anyways and what is the false case ? If there is a discrepancy in behavior, this can make a good test case. I can't derive one from that comment.
Execute them before and them continue.
Thinking about it, it's a nullfail or a nulldummy error, not really a new kind of error.
The appropriate test cases are in place. If you want to arc patch this Diff and parents, and try the modifications you suggest, you can see how it affects the test cases.
They can't be executed before since they modify values used in the body of this if-statement. It is possible to rewrite this to use if(!x) continue; but it just gets more verbose.
Indeed, that's exactly what I had in a prior version of this Diff. :-D I'm happy to revert if that's desirable.