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:None 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).
fanquake cross-referenced this on Feb 2, 2015 from issue Add list of implemented BIPs by sipa
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
sdaftuar cross-referenced this on Feb 6, 2015 from issue Implement BIP66 (0.9) by sipa
TheBlueMatt cross-referenced this on Sep 25, 2015 from issue De-neuter NODE_BLOOM by TheBlueMatt
fanatid commented at 5:38 AM on March 17, 2016: contributor
Why BIP66 not checked that r or s <s>less</s> more than 33 bytes? related: https://github.com/bitcoinjs/bip66/pull/2

Invalid DER signature for secp256k1 (but still valid by BIP66):
```
3044021458a2f39bd87f0000000506030000000000050603022c402dde9afe7f0000010000000100000004000000040000000000000000000000000000000a00000000000000
```
Stouse49 cross-referenced this on Jan 11, 2018 from issue Code Missing to activate and check for the activation of BIP34, 65 and 66 softforks. by Stouse49
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.
elichai cross-referenced this on May 26, 2020 from issue Remove outdated comment about DER encoding by elichai
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
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#5708 Add list of implemented BIPs #5762 Implement BIP66 (0.9)#6641 De-neuter NODE_BLOOM #19073 Remove outdated comment about DER encoding