Merge branch 'script-verify-flags'

Author: petertodd

bitcoin/core/script.py

@@ -772,11 +772,6 @@ def GetSigOpCount(self, fAccurate):
         return n
 
 
-SCRIPT_VERIFY_P2SH = object()
-SCRIPT_VERIFY_STRICTENC = object()
-SCRIPT_VERIFY_EVEN_S = object()
-SCRIPT_VERIFY_NOCACHE = object()
-
 SIGHASH_ALL = 1
 SIGHASH_NONE = 2
 SIGHASH_SINGLE = 3
@@ -1051,10 +1046,6 @@ def SignatureHash(script, txTo, inIdx, hashtype):
         'CScriptInvalidError',
         'CScriptTruncatedPushDataError',
         'CScript',
-        'SCRIPT_VERIFY_P2SH',
-        'SCRIPT_VERIFY_STRICTENC',
-        'SCRIPT_VERIFY_EVEN_S',
-        'SCRIPT_VERIFY_NOCACHE',
         'SIGHASH_ALL',
         'SIGHASH_NONE',
         'SIGHASH_SINGLE',

bitcoin/core/scripteval.py

@@ -40,8 +40,27 @@
 
 SCRIPT_VERIFY_P2SH = object()
 SCRIPT_VERIFY_STRICTENC = object()
-SCRIPT_VERIFY_EVEN_S = object()
-SCRIPT_VERIFY_NOCACHE = object()
+SCRIPT_VERIFY_DERSIG = object()
+SCRIPT_VERIFY_LOW_S = object()
+SCRIPT_VERIFY_NULLDUMMY = object()
+SCRIPT_VERIFY_SIGPUSHONLY = object()
+SCRIPT_VERIFY_MINIMALDATA = object()
+SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS = object()
+SCRIPT_VERIFY_CLEANSTACK = object()
+SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY = object()
+
+SCRIPT_VERIFY_FLAGS_BY_NAME = {
+    'P2SH': SCRIPT_VERIFY_P2SH,
+    'STRICTENC': SCRIPT_VERIFY_STRICTENC,
+    'DERSIG': SCRIPT_VERIFY_DERSIG,
+    'LOW_S': SCRIPT_VERIFY_LOW_S,
+    'NULLDUMMY': SCRIPT_VERIFY_NULLDUMMY,
+    'SIGPUSHONLY': SCRIPT_VERIFY_SIGPUSHONLY,
+    'MINIMALDATA': SCRIPT_VERIFY_MINIMALDATA,
+    'DISCOURAGE_UPGRADABLE_NOPS': SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS,
+    'CLEANSTACK': SCRIPT_VERIFY_CLEANSTACK,
+    'CHECKLOCKTIMEVERIFY': SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY,
+}
 
 class EvalScriptError(bitcoin.core.ValidationError):
     """Base class for exceptions raised when a script fails during EvalScript()
@@ -133,7 +152,7 @@ def _CheckSig(sig, pubkey, script, txTo, inIdx, err_raiser):
     return key.verify(h, sig)
 
 
-def _CheckMultiSig(opcode, script, stack, txTo, inIdx, err_raiser, nOpCount):
+def _CheckMultiSig(opcode, script, stack, txTo, inIdx, flags, err_raiser, nOpCount):
     i = 1
     if len(stack) < i:
         err_raiser(MissingOpArgumentsError, opcode, stack, i)
@@ -190,14 +209,24 @@ def _CheckMultiSig(opcode, script, stack, txTo, inIdx, err_raiser, nOpCount):
             if opcode == OP_CHECKMULTISIGVERIFY:
                 err_raiser(VerifyOpFailedError, opcode)
 
-    while i > 0:
+    while i > 1:
         stack.pop()
         i -= 1
 
+    # Note how Bitcoin Core duplicates the len(stack) check, rather than
+    # letting pop() handle it; maybe that's wrong?
+    if len(stack) and SCRIPT_VERIFY_NULLDUMMY in flags:
+        if stack[-1] != b'':
+            raise err_raiser(ArgumentsInvalidError, opcode, "dummy value not OP_0")
+
+    stack.pop()
+
     if opcode == OP_CHECKMULTISIG:
         if success:
             stack.append(b"\x01")
         else:
+            # FIXME: this is incorrect, but not caught by existing
+            # test cases
             stack.append(b"\x00")
 
 
@@ -457,7 +486,7 @@ def check_args(n):
 
             elif sop == OP_CHECKMULTISIG or sop == OP_CHECKMULTISIGVERIFY:
                 tmpScript = CScript(scriptIn[pbegincodehash:])
-                _CheckMultiSig(sop, tmpScript, stack, txTo, inIdx, err_raiser, nOpCount)
+                _CheckMultiSig(sop, tmpScript, stack, txTo, inIdx, flags, err_raiser, nOpCount)
 
             elif sop == OP_CHECKSIG or sop == OP_CHECKSIGVERIFY:
                 check_args(2)
@@ -572,9 +601,15 @@ def check_args(n):
                 check_args(2)
                 del stack[-2]
 
-            elif sop == OP_NOP or (sop >= OP_NOP1 and sop <= OP_NOP10):
+            elif sop == OP_NOP:
                 pass
 
+            elif sop >= OP_NOP1 and sop <= OP_NOP10:
+                if SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS in flags:
+                    err_raiser(EvalScriptError, "%s reserved for soft-fork upgrades" % OPCODE_NAMES[sop])
+                else:
+                    pass
+
             elif sop == OP_OVER:
                 check_args(2)
                 vch = stack[-2]
@@ -735,21 +770,30 @@ def VerifyScript(scriptSig, scriptPubKey, txTo, inIdx, flags=()):
         if not scriptSig.is_push_only():
             raise VerifyScriptError("P2SH scriptSig not is_push_only()")
 
-        # stackCopy cannot be empty here, because if it was the
+        # restore stack
+        stack = stackCopy
+
+        # stack cannot be empty here, because if it was the
         # P2SH  HASH <> EQUAL  scriptPubKey would be evaluated with
         # an empty stack and the EvalScript above would return false.
-        assert len(stackCopy)
+        assert len(stack)
 
-        pubKey2 = CScript(stackCopy.pop())
+        pubKey2 = CScript(stack.pop())
 
-        EvalScript(stackCopy, pubKey2, txTo, inIdx, flags=flags)
+        EvalScript(stack, pubKey2, txTo, inIdx, flags=flags)
 
-        if not len(stackCopy):
+        if not len(stack):
             raise VerifyScriptError("P2SH inner scriptPubKey left an empty stack")
 
-        if not _CastToBool(stackCopy[-1]):
+        if not _CastToBool(stack[-1]):
             raise VerifyScriptError("P2SH inner scriptPubKey returned false")
 
+    if SCRIPT_VERIFY_CLEANSTACK in flags:
+        assert SCRIPT_VERIFY_P2SH in flags
+
+        if len(stack) != 1:
+            raise VerifyScriptError("scriptPubKey left extra items on stack")
+
 
 class VerifySignatureError(bitcoin.core.ValidationError):
     pass
@@ -782,8 +826,15 @@ def VerifySignature(txFrom, txTo, inIdx):
         'MAX_STACK_ITEMS',
         'SCRIPT_VERIFY_P2SH',
         'SCRIPT_VERIFY_STRICTENC',
-        'SCRIPT_VERIFY_EVEN_S',
-        'SCRIPT_VERIFY_NOCACHE',
+        'SCRIPT_VERIFY_DERSIG',
+        'SCRIPT_VERIFY_LOW_S',
+        'SCRIPT_VERIFY_NULLDUMMY',
+        'SCRIPT_VERIFY_SIGPUSHONLY',
+        'SCRIPT_VERIFY_MINIMALDATA',
+        'SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS',
+        'SCRIPT_VERIFY_CLEANSTACK',
+        'SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY',
+        'SCRIPT_VERIFY_FLAGS_BY_NAME',
         'EvalScriptError',
         'MaxOpCountError',
         'MissingOpArgumentsError',

bitcoin/tests/data/script_invalid.json

@@ -163,6 +163,23 @@
 ["1","NOP1 CHECKLOCKTIMEVERIFY NOP3 NOP4 NOP5 NOP6 NOP7 NOP8 NOP9 NOP10 2 EQUAL", "P2SH,STRICTENC"],
 ["'NOP_1_to_10' NOP1 CHECKLOCKTIMEVERIFY NOP3 NOP4 NOP5 NOP6 NOP7 NOP8 NOP9 NOP10","'NOP_1_to_11' EQUAL", "P2SH,STRICTENC"],
 
+["Ensure 100% coverage of discouraged NOPS"],
+["1", "NOP1",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "CHECKLOCKTIMEVERIFY",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP3",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP4",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP5",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP6",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP7",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP8",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP9",  "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+["1", "NOP10", "P2SH,DISCOURAGE_UPGRADABLE_NOPS"],
+
+["NOP10", "1", "P2SH,DISCOURAGE_UPGRADABLE_NOPS", "Discouraged NOP10 in scriptSig"],
+
+["1 0x01 0xb9", "HASH160 0x14 0x15727299b05b45fdaf9ac9ecf7565cfe27c3e567 EQUAL",
+ "P2SH,DISCOURAGE_UPGRADABLE_NOPS", "Discouraged NOP10 in redeemScript"],
+
 ["0x50","1", "P2SH,STRICTENC", "opcode 0x50 is reserved"],
 ["1", "IF 0xba ELSE 1 ENDIF", "P2SH,STRICTENC", "opcodes above NOP10 invalid if executed"],
 ["1", "IF 0xbb ELSE 1 ENDIF", "P2SH,STRICTENC"],
@@ -492,6 +509,18 @@
     "STRICTENC",
     "P2PK NOT with hybrid pubkey"
 ],
+[
+    "1 0x47 0x3044022051254b9fb476a52d85530792b578f86fea70ec1ffb4393e661bcccb23d8d63d3022076505f94a403c86097841944e044c70c2045ce90e36de51f7e9d3828db98a07501 0x47 0x304402200a358f750934b3feb822f1966bfcd8bbec9eeaa3a8ca941e11ee5960e181fa01022050bf6b5a8e7750f70354ae041cb68a7bade67ec6c3ab19eb359638974410626e01 0x47 0x304402200955d031fff71d8653221e85e36c3c85533d2312fc3045314b19650b7ae2f81002202a6bb8505e36201909d0921f01abff390ae6b7ff97bbf959f98aedeb0a56730901",
+    "3 0x21 0x0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798 0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 3 CHECKMULTISIG",
+    "NULLDUMMY",
+    "3-of-3 with nonzero dummy"
+],
+[
+    "1 0x47 0x304402201bb2edab700a5d020236df174fefed78087697143731f659bea59642c759c16d022061f42cdbae5bcd3e8790f20bf76687443436e94a634321c16a72aa54cbc7c2ea01 0x47 0x304402204bb4a64f2a6e5c7fb2f07fef85ee56fde5e6da234c6a984262307a20e99842d702206f8303aaba5e625d223897e2ffd3f88ef1bcffef55f38dc3768e5f2e94c923f901 0x47 0x3044022040c2809b71fffb155ec8b82fe7a27f666bd97f941207be4e14ade85a1249dd4d02204d56c85ec525dd18e29a0533d5ddf61b6b1bb32980c2f63edf951aebf7a27bfe01",
+    "3 0x21 0x0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798 0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 3 CHECKMULTISIG NOT",
+    "NULLDUMMY",
+    "3-of-3 NOT with invalid sig with nonzero dummy"
+],
 [
     "0x47 0x304402203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022054e1c258c2981cdfba5df1f46661fb6541c44f77ca0092f3600331abfffb125101 0x23 0x2103363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640ac",
     "0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG",
@@ -504,6 +533,18 @@
     "P2SH,STRICTENC",
     "2-of-3 with one valid and one invalid signature due to parse error, nSigs > validSigs"
 ],
+[
+    "11 0x47 0x304402200a5c6163f07b8d3b013c4d1d6dba25e780b39658d79ba37af7057a3b7f15ffa102201fd9b4eaa9943f734928b99a83592c2e7bf342ea2680f6a2bb705167966b742001",
+    "0x41 0x0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 CHECKSIG",
+    "CLEANSTACK,P2SH",
+    "P2PK with unnecessary input"
+],
+[
+    "11 0x47 0x304402202f7505132be14872581f35d74b759212d9da40482653f1ffa3116c3294a4a51702206adbf347a2240ca41c66522b1a22a41693610b76a8e7770645dc721d1635854f01 0x43 0x410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ac",
+    "HASH160 0x14 0x31edc23bdafda4639e669f89ad6b2318dd79d032 EQUAL",
+    "CLEANSTACK,P2SH",
+    "P2SH with unnecessary input"
+],
 
 ["The End"]
 ]

bitcoin/tests/data/script_valid.json

@@ -726,6 +726,12 @@
     "P2SH",
     "P2SH(P2PK)"
 ],
+[
+    "0x47 0x304402204e2eb034be7b089534ac9e798cf6a2c79f38bcb34d1b179efd6f2de0841735db022071461beb056b5a7be1819da6a3e3ce3662831ecc298419ca101eb6887b5dd6a401 0x19 0x76a9147cf9c846cd4882efec4bf07e44ebdad495c94f4b88ac",
+    "HASH160 0x14 0x2df519943d5acc0ef5222091f9dfe3543f489a82 EQUAL",
+    "",
+    "P2SH(P2PKH), bad sig but no VERIFY_P2SH"
+],
 [
     "0 0x47 0x3044022051254b9fb476a52d85530792b578f86fea70ec1ffb4393e661bcccb23d8d63d3022076505f94a403c86097841944e044c70c2045ce90e36de51f7e9d3828db98a07501 0x47 0x304402200a358f750934b3feb822f1966bfcd8bbec9eeaa3a8ca941e11ee5960e181fa01022050bf6b5a8e7750f70354ae041cb68a7bade67ec6c3ab19eb359638974410626e01 0x47 0x304402200955d031fff71d8653221e85e36c3c85533d2312fc3045314b19650b7ae2f81002202a6bb8505e36201909d0921f01abff390ae6b7ff97bbf959f98aedeb0a56730901",
     "3 0x21 0x0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798 0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 3 CHECKMULTISIG",

bitcoin/tests/test_scripteval.py

@@ -68,12 +68,23 @@ def load_test_vectors(name):
             scriptSig = parse_script(scriptSig)
             scriptPubKey = parse_script(scriptPubKey)
 
-            yield (scriptSig, scriptPubKey, comment, test_case)
+            flag_set = set()
+            for flag in flags.split(','):
+                if flag == '' or flag == 'NONE':
+                    pass
 
+                else:
+                    try:
+                        flag = SCRIPT_VERIFY_FLAGS_BY_NAME[flag]
+                    except IndexError:
+                        raise Exception('Unknown script verify flag %r' % flag)
 
-class Test_EvalScript(unittest.TestCase):
-    flags = (SCRIPT_VERIFY_P2SH, SCRIPT_VERIFY_STRICTENC)
+                    flag_set.add(flag)
+
+            yield (scriptSig, scriptPubKey, flag_set, comment, test_case)
 
+
+class Test_EvalScript(unittest.TestCase):
     def create_test_txs(self, scriptSig, scriptPubKey):
         txCredit = CTransaction([CTxIn(COutPoint(), CScript([OP_0, OP_0]), nSequence=0xFFFFFFFF)],
                                 [CTxOut(0, scriptPubKey)],
@@ -84,20 +95,20 @@ def create_test_txs(self, scriptSig, scriptPubKey):
         return (txCredit, txSpend)
 
     def test_script_valid(self):
-        for scriptSig, scriptPubKey, comment, test_case in load_test_vectors('script_valid.json'):
+        for scriptSig, scriptPubKey, flags, comment, test_case in load_test_vectors('script_valid.json'):
             (txCredit, txSpend) = self.create_test_txs(scriptSig, scriptPubKey)
 
             try:
-                VerifyScript(scriptSig, scriptPubKey, txSpend, 0, flags=self.flags)
+                VerifyScript(scriptSig, scriptPubKey, txSpend, 0, flags)
             except ValidationError as err:
                 self.fail('Script FAILED: %r %r %r with exception %r' % (scriptSig, scriptPubKey, comment, err))
 
     def test_script_invalid(self):
-        for scriptSig, scriptPubKey, comment, test_case in load_test_vectors('script_invalid.json'):
+        for scriptSig, scriptPubKey, flags, comment, test_case in load_test_vectors('script_invalid.json'):
             (txCredit, txSpend) = self.create_test_txs(scriptSig, scriptPubKey)
 
             try:
-                VerifyScript(scriptSig, scriptPubKey, txSpend, 0, flags=self.flags)
+                VerifyScript(scriptSig, scriptPubKey, txSpend, 0, flags)
             except ValidationError:
                 continue
 

release-notes.md

@@ -1,6 +1,21 @@
 python-bitcoinlib release notes
 ===============================
 
+v0.6.0
+======
+
+Breaking API changes:
+
+* Removed SCRIPT_VERIFY constants ``bitcoin.core.script``, leaving just the
+  constants in ``bitcoin.core.scripteval``; being singletons the redundant
+  constants were broken anyway.
+
+* SCRIPT_VERIFY_EVEN_S renamed to SCRIPT_VERIFY_LOW_S to match Bitcoin Core's naming
+
+* SCRIPT_VERIFY_NOCACHE removed as Bitcoin Core no longer has it (and we never
+  did anything with it anyway)
+
+
 v0.5.1
 ======