Merge pull request #5713

bf6cdeb Increase coverage of DERSIG edge cases (Pieter Wuille) 819bcf9 Add RPC test for DERSIG BIP switchover logic (Pieter Wuille) 5a47811 BIP66 changeover logic (Pieter Wuille) 092e9fe Example unit tests from BIP66 (Pieter Wuille) 80ad135 Change IsDERSignature to BIP66 implementation (Pieter Wuille)
TrezarCoin · Feb 3, 2015 · 41e6e4c · 41e6e4c
2 parents 9c4a5a5 + bf6cdeb
commit 41e6e4c
Show file tree

Hide file tree

Showing 9 changed files with 405 additions and 64 deletions.
diff --git a/qa/rpc-tests/bipdersig.py b/qa/rpc-tests/bipdersig.py
@@ -0,0 +1,90 @@
+#!/usr/bin/env python2
+# Copyright (c) 2014 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#
+# Test the BIP66 changeover logic
+#
+
+from test_framework import BitcoinTestFramework
+from bitcoinrpc.authproxy import AuthServiceProxy, JSONRPCException
+from util import *
+import os
+import shutil
+
+class BIP66Test(BitcoinTestFramework):
+
+    def setup_network(self):
+        self.nodes = []
+        self.nodes.append(start_node(0, self.options.tmpdir, []))
+        self.nodes.append(start_node(1, self.options.tmpdir, ["-blockversion=2"]))
+        self.nodes.append(start_node(2, self.options.tmpdir, ["-blockversion=3"]))
+        connect_nodes(self.nodes[1], 0)
+        connect_nodes(self.nodes[2], 0)
+        self.is_network_split = False
+        self.sync_all()
+
+    def run_test(self):
+        cnt = self.nodes[0].getblockcount()
+
+        # Mine some old-version blocks
+        self.nodes[1].setgenerate(True, 100)
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 100):
+            raise AssertionError("Failed to mine 100 version=2 blocks")
+
+        # Mine 750 new-version blocks
+        for i in xrange(15):
+            self.nodes[2].setgenerate(True, 50)
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 850):
+            raise AssertionError("Failed to mine 750 version=3 blocks")
+
+        # TODO: check that new DERSIG rules are not enforced
+
+        # Mine 1 new-version block
+        self.nodes[2].setgenerate(True, 1)
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 851):
+            raise AssertionFailure("Failed to mine a version=3 blocks")
+
+        # TODO: check that new DERSIG rules are enforced
+
+        # Mine 198 new-version blocks
+        for i in xrange(2):
+            self.nodes[2].setgenerate(True, 99)
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 1049):
+            raise AssertionError("Failed to mine 198 version=3 blocks")
+
+        # Mine 1 old-version block
+        self.nodes[1].setgenerate(True, 1)
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 1050):
+            raise AssertionError("Failed to mine a version=2 block after 949 version=3 blocks")
+
+        # Mine 1 new-version blocks
+        self.nodes[2].setgenerate(True, 1)
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 1051):
+            raise AssertionError("Failed to mine a version=3 block")
+
+        # Mine 1 old-version blocks
+        try:
+            self.nodes[1].setgenerate(True, 1)
+            raise AssertionError("Succeeded to mine a version=2 block after 950 version=3 blocks")
+        except JSONRPCException:
+            pass
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 1051):
+            raise AssertionError("Accepted a version=2 block after 950 version=3 blocks")
+
+        # Mine 1 new-version blocks
+        self.nodes[2].setgenerate(True, 1)
+        self.sync_all()
+        if (self.nodes[0].getblockcount() != cnt + 1052):
+            raise AssertionError("Failed to mine a version=3 block")
+
+if __name__ == '__main__':
+    BIP66Test().main()
diff --git a/src/main.cpp b/src/main.cpp
@@ -1764,6 +1764,11 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
 
     unsigned int flags = fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE;
 
+    // Start enforcing the DERSIG (BIP66) rules, for block.nVersion=3 blocks, when 75% of the network has upgraded:
+    if (block.nVersion >= 3 && IsSuperMajority(3, pindex->pprev, Params().EnforceBlockUpgradeMajority())) {
+        flags |= SCRIPT_VERIFY_DERSIG;
+    }
+
     CBlockUndo blockundo;
 
     CCheckQueueControl<CScriptCheck> control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL);
@@ -2601,6 +2606,13 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta
                              REJECT_OBSOLETE, "bad-version");
     }
 
+    // Reject block.nVersion=2 blocks when 95% (75% on testnet) of the network has upgraded:
+    if (block.nVersion < 3 && IsSuperMajority(3, pindexPrev, Params().RejectBlockOutdatedMajority()))
+    {
+        return state.Invalid(error("%s : rejected nVersion=2 block", __func__),
+                             REJECT_OBSOLETE, "bad-version");
+    }
+
     return true;
 }
 

diff --git a/src/primitives/block.h b/src/primitives/block.h
@@ -24,7 +24,7 @@ class CBlockHeader
 {
 public:
     // header
-    static const int32_t CURRENT_VERSION=2;
+    static const int32_t CURRENT_VERSION=3;
     int32_t nVersion;
     uint256 hashPrevBlock;
     uint256 hashMerkleRoot;

diff --git a/src/script/bitcoinconsensus.h b/src/script/bitcoinconsensus.h
@@ -46,6 +46,7 @@ enum
 {
     bitcoinconsensus_SCRIPT_FLAGS_VERIFY_NONE      = 0,
     bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH      = (1U << 0), // evaluate P2SH (BIP16) subscripts
+    bitcoinconsensus_SCRIPT_FLAGS_VERIFY_DERSIG    = (1U << 2), // enforce strict DER (BIP66) compliance
 };
 
 /// Returns 1 if the input nIn of the serialized transaction pointed to by

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
@@ -93,76 +93,76 @@ bool static IsCompressedOrUncompressedPubKey(const valtype &vchPubKey) {
  * in which case a single 0 byte is necessary and even required).
  * 
  * See https://bitcointalk.org/index.php?topic=8392.msg127623#msg127623
+ *
+ * This function is consensus-critical since BIP66.
  */
-bool static IsDERSignature(const valtype &vchSig) {
+bool static IsValidSignatureEncoding(const std::vector<unsigned char> &sig) {
+    // Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S] [sighash]
+    // * total-length: 1-byte length descriptor of everything that follows,
+    //   excluding the sighash byte.
+    // * R-length: 1-byte length descriptor of the R value that follows.
+    // * R: arbitrary-length big-endian encoded R value. It must use the shortest
+    //   possible encoding for a positive integers (which means no null bytes at
+    //   the start, except a single one when the next byte has its highest bit set).
+    // * S-length: 1-byte length descriptor of the S value that follows.
+    // * S: arbitrary-length big-endian encoded S value. The same rules apply.
+    // * sighash: 1-byte value indicating what data is hashed (not part of the DER
+    //   signature)
 
-    if (vchSig.size() < 9) {
-        //  Non-canonical signature: too short
-        return false;
-    }
-    if (vchSig.size() > 73) {
-        // Non-canonical signature: too long
-        return false;
-    }
-    if (vchSig[0] != 0x30) {
-        //  Non-canonical signature: wrong type
-        return false;
-    }
-    if (vchSig[1] != vchSig.size()-3) {
-        //  Non-canonical signature: wrong length marker
-        return false;
-    }
-    unsigned int nLenR = vchSig[3];
-    if (5 + nLenR >= vchSig.size()) {
-        //  Non-canonical signature: S length misplaced
-        return false;
-    }
-    unsigned int nLenS = vchSig[5+nLenR];
-    if ((unsigned long)(nLenR+nLenS+7) != vchSig.size()) {
-        //  Non-canonical signature: R+S length mismatch
-        return false;
-    }
+    // Minimum and maximum size constraints.
+    if (sig.size() < 9) return false;
+    if (sig.size() > 73) return false;
 
-    const unsigned char *R = &vchSig[4];
-    if (R[-2] != 0x02) {
-        //  Non-canonical signature: R value type mismatch
-        return false;
-    }
-    if (nLenR == 0) {
-        //  Non-canonical signature: R length is zero
-        return false;
-    }
-    if (R[0] & 0x80) {
-        //  Non-canonical signature: R value negative
-        return false;
-    }
-    if (nLenR > 1 && (R[0] == 0x00) && !(R[1] & 0x80)) {
-        //  Non-canonical signature: R value excessively padded
-        return false;
-    }
+    // A signature is of type 0x30 (compound).
+    if (sig[0] != 0x30) return false;
+
+    // Make sure the length covers the entire signature.
+    if (sig[1] != sig.size() - 3) return false;
+
+    // Extract the length of the R element.
+    unsigned int lenR = sig[3];
+
+    // Make sure the length of the S element is still inside the signature.
+    if (5 + lenR >= sig.size()) return false;
+
+    // Extract the length of the S element.
+    unsigned int lenS = sig[5 + lenR];
+
+    // Verify that the length of the signature matches the sum of the length
+    // of the elements.
+    if ((size_t)(lenR + lenS + 7) != sig.size()) return false;
+
+    // Check whether the R element is an integer.
+    if (sig[2] != 0x02) return false;
+
+    // Zero-length integers are not allowed for R.
+    if (lenR == 0) return false;
+
+    // Negative numbers are not allowed for R.
+    if (sig[4] & 0x80) return false;
+
+    // Null bytes at the start of R are not allowed, unless R would
+    // otherwise be interpreted as a negative number.
+    if (lenR > 1 && (sig[4] == 0x00) && !(sig[5] & 0x80)) return false;
+
+    // Check whether the S element is an integer.
+    if (sig[lenR + 4] != 0x02) return false;
+
+    // Zero-length integers are not allowed for S.
+    if (lenS == 0) return false;
+
+    // Negative numbers are not allowed for S.
+    if (sig[lenR + 6] & 0x80) return false;
+
+    // Null bytes at the start of S are not allowed, unless S would otherwise be
+    // interpreted as a negative number.
+    if (lenS > 1 && (sig[lenR + 6] == 0x00) && !(sig[lenR + 7] & 0x80)) return false;
 
-    const unsigned char *S = &vchSig[6+nLenR];
-    if (S[-2] != 0x02) {
-        //  Non-canonical signature: S value type mismatch
-        return false;
-    }
-    if (nLenS == 0) {
-        //  Non-canonical signature: S length is zero
-        return false;
-    }
-    if (S[0] & 0x80) {
-        //  Non-canonical signature: S value negative
-        return false;
-    }
-    if (nLenS > 1 && (S[0] == 0x00) && !(S[1] & 0x80)) {
-        //  Non-canonical signature: S value excessively padded
-        return false;
-    }
     return true;
 }
 
 bool static IsLowDERSignature(const valtype &vchSig, ScriptError* serror) {
-    if (!IsDERSignature(vchSig)) {
+    if (!IsValidSignatureEncoding(vchSig)) {
         return set_error(serror, SCRIPT_ERR_SIG_DER);
     }
     unsigned int nLenR = vchSig[3];
@@ -194,7 +194,7 @@ bool static CheckSignatureEncoding(const valtype &vchSig, unsigned int flags, Sc
     if (vchSig.size() == 0) {
         return true;
     }
-    if ((flags & (SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_STRICTENC)) != 0 && !IsDERSignature(vchSig)) {
+    if ((flags & (SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_STRICTENC)) != 0 && !IsValidSignatureEncoding(vchSig)) {
         return set_error(serror, SCRIPT_ERR_SIG_DER);
     } else if ((flags & SCRIPT_VERIFY_LOW_S) != 0 && !IsLowDERSignature(vchSig, serror)) {
         // serror is set

diff --git a/src/script/standard.h b/src/script/standard.h
@@ -45,6 +45,7 @@ static const unsigned int MANDATORY_SCRIPT_VERIFY_FLAGS = SCRIPT_VERIFY_P2SH;
  * blocks and we must accept those blocks.
  */
 static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY_FLAGS |
+                                                         SCRIPT_VERIFY_DERSIG |
                                                          SCRIPT_VERIFY_STRICTENC |
                                                          SCRIPT_VERIFY_MINIMALDATA |
                                                          SCRIPT_VERIFY_NULLDUMMY |