Implement BIP66 #5713

pull sipa wants to merge 5 commits into bitcoin:master from sipa:bip66 changing 9 files +405 −64

sipa commented at 3:06 am on January 27, 2015: member

See the corresponding BIP text (see bitcoin/bips#135) and mailing list discussion for details.
laanwj added the label Priority High on Jan 27, 2015
laanwj added the label Validation on Jan 27, 2015
laanwj commented at 12:05 pm on January 27, 2015: member

Cool, looks good to me. I’ll have a try at filing in the TODOs in the RPC test.
sipa commented at 2:08 pm on January 27, 2015: member

@laanwj That’d be awesome, but I don’t see how to easily do that without adding functionality to the RPC framework to build its own blocks. Bitcoin Core won’t mine blocks with transactions that violate the rules it wants. I guess there could be a -overridemempoolvalidationflags or something, but that gets really ugly.
laanwj commented at 3:13 pm on January 27, 2015: member

@sipa Bleh, yes, I just noticed; added python code to fuzz a transaction into non-canonical-DER, but it’s impossible to get it into the mempool. Building blocks from the test framework would be neat, of course.

in qa/rpc-tests/bipdersig.py: in 864913e0bb outdated

60+
61+        # Mine 1 old-version block
62+        self.nodes[1].setgenerate(True, 1)
63+        self.sync_all()
64+        if (self.nodes[0].getblockcount() != cnt + 1050):
65+            raise AssertionFailure("Failed to mine a version=2 block after 949 version=3 blocks")

laanwj commented at 7:11 pm on January 27, 2015:

s/AssertionFailure/AssertionError/

laanwj commented at 6:07 am on January 28, 2015: member

tested ACK. Verified that the deployment behavior as mentioned in BIP66 is indeed what is enforced.
Change IsDERSignature to BIP66 implementation 80ad135a5e
Example unit tests from BIP66 092e9fe5fb
BIP66 changeover logic 5a47811da5
Add RPC test for DERSIG BIP switchover logic 819bcf9b99
Increase coverage of DERSIG edge cases bf6cdebdc5
sipa force-pushed on Feb 1, 2015
sipa commented at 10:02 pm on February 1, 2015: member

Addressed comments and rebased.
luke-jr commented at 11:34 pm on February 1, 2015: member

utACK: read over non-test commits (80ad135a5e54e8a065fee5ef36e57034679111ab and 5a47811da5158df763aa2fca09ce646ee0c51e7b), looks reasonable (I did not compare to DER spec).
sdaftuar commented at 9:23 pm on February 2, 2015: member

I was able to test deployment behavior, submitting regtest blocks with a couple kinds of non-DER signatures over the p2p network to a node running the new code – worked as expected.
laanwj merged this on Feb 3, 2015
laanwj closed this on Feb 3, 2015
laanwj referenced this in commit 41e6e4caba on Feb 3, 2015
fanatid commented at 5:38 am on March 17, 2016: contributor
Why BIP66 not checked that r or s less more than 33 bytes? related: https://github.com/bitcoinjs/bip66/pull/2

Invalid DER signature for secp256k1 (but still valid by BIP66):
03044021458a2f39bd87f0000000506030000000000050603022c402dde9afe7f0000010000000100000004000000040000000000000000000000000000000a00000000000000
vietlq commented at 8:13 am on July 14, 2018: none

@fanatid the example you posted should be invalidated as an invalid signature even before coming to BIP66 validation code. Layered filters I’d say, so no worries.
fanquake referenced this in commit cffbf1eb9a on May 27, 2020
sidhujag referenced this in commit bf5ca922fd on May 27, 2020
PastaPastaPasta referenced this in commit eb91540645 on Jun 27, 2021
PastaPastaPasta referenced this in commit aaa99155ea on Jun 28, 2021
PastaPastaPasta referenced this in commit 7e867e95e1 on Jun 29, 2021
PastaPastaPasta referenced this in commit e1dc813f55 on Jul 1, 2021
PastaPastaPasta referenced this in commit c333a6fc91 on Jul 1, 2021
PastaPastaPasta referenced this in commit f3f00f446e on Jul 14, 2021
PastaPastaPasta referenced this in commit 7ac32ac79f on Jul 15, 2021
DrahtBot locked this on Sep 8, 2021

Contributors
sipa laanwj luke-jr sdaftuar fanatid vietlq

Labels
Validation