Implement BIP62 rules 2, 3 and 4 #5065
Conversation
sipa
force-pushed
the
bip62b
branch
2 times, most recently
from
b1d99c5
to
c839843
Compare
| @@ -156,6 +156,29 @@ bool static CheckPubKeyEncoding(const valtype &vchSig, unsigned int flags) { | |||
| return true; | |||
| } | |||
|
|
|||
| bool static CheckMinimalPush(const valtype& data, opcodetype opcode) { | |||
There was a problem hiding this comment.
Just asking, why you and Gavin like to use
void foo() {
...
}
over
void foo()
{
...
}
AFAIK the clang style will change it.
There was a problem hiding this comment.
It will.
But whenever I write code I try to mimick the style of the code around it. Once we effectively apply clang-format, that will be the same style everywhere.
sipa
force-pushed
the
bip62b
branch
3 times, most recently
from
4bb9a47
to
6a1f425
Compare
|
Made a few small comments on the commits d54b8e40908b3e9ca33d728d8f0ab921f856e1dc and 6a1f425942701b916bb5fe39e5ec76b2b5585b40. Other than those, LGTM |
|
I made some minor improvements to this: https://github.com/petertodd/bitcoin/tree/bip62b-improvements Better comments for CScriptNum(), OP_PUSHDATA4 handling, changed the invalid unittests to make them more likely to pass, and added bunch of unit tests to get ~100% coverage, and clearly separated them into PUSHDATA and numeric argument categories. That said, other than those minor improvements, I can't find anything wrong with this patch after a few hours of thinking about it; strong ACK. Also, I found out something really odd while tearing it apart: if we hadn't disabled OP_RIGHT you could have this valid pair of redeemScript's and scriptSigs: This works because OP_1NEGATE pushes 0x81 to the stack, which happens to also be a perfectly valid redeemScript! |
Also use the new flag as a standard rule, and replace the IsCanonicalPush standardness check with it (as it is more complete).
Edited-by: Pieter Wuille <pieter.wuille@gmail.com>
Removes the need for the 'negated' versions of the tests, and ensures other failures don't mask what we're trying to test.
…ic minimal encoding rule
|
Rebased and included some of @petertodd's commits (fixed typo's in the comment improvement, and skipped the PUSHDATA4 one, because it looks controversial). |
|
Added another test. EDIT: nevermind; petertodd's cases already cover this. |
|
utACK. |
|
16d78bd Add valid invert of invalid every numeric opcode tests (Peter Todd) 2b62e17 Clearly separate PUSHDATA and numeric argument MINIMALDATA tests (Peter Todd) dfeec18 Test every numeric-accepting opcode for correct handling of the numeric minimal encoding rule (Peter Todd) 554147a Ensure MINIMALDATA invalid tests can only fail one way (Peter Todd) 6004e77 Improve CScriptNum() comment (Peter Todd) 698c6ab Add SCRIPT_VERIFY_MINIMALDATA (BIP62 rules 3 and 4) (Pieter Wuille) d752ba8 Add SCRIPT_VERIFY_SIGPUSHONLY (BIP62 rule 2) (Pieter Wuille)
|
@sipa FWIW I wasn't actually expecting you to add in all those commits individually; if you had wanted to squash them all or whatever that'd be fine by me. I just had it all split out to make it easy for you to review. But equally, totally ok for you to just pull them in directly too. |
No description provided.