Merge pull request #5143

da918ac Make SCRIPT_VERIFY_CLEANSTACK a standardness requirement (Pieter Wuille) b6e03cc Add SCRIPT_VERIFY_CLEANSTACK (BIP62 rule 6) (Pieter Wuille) ae4151b No semantic change: reuse stack variable in P2SH evaluation (Pieter Wuille)
TrezarCoin · Jan 8, 2015 · 48e1765 · 48e1765
2 parents e3f5727 + da918ac
commit 48e1765
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 14 deletions.
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
@@ -1098,7 +1098,6 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigne
         return false;
     if (stack.empty())
         return set_error(serror, SCRIPT_ERR_EVAL_FALSE);
-
     if (CastToBool(stack.back()) == false)
         return set_error(serror, SCRIPT_ERR_EVAL_FALSE);
 
@@ -1109,24 +1108,37 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigne
         if (!scriptSig.IsPushOnly())
             return set_error(serror, SCRIPT_ERR_SIG_PUSHONLY);
 
-        // stackCopy cannot be empty here, because if it was the
+        // Restore stack.
+        swap(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(!stackCopy.empty());
+        assert(!stack.empty());
 
-        const valtype& pubKeySerialized = stackCopy.back();
+        const valtype& pubKeySerialized = stack.back();
         CScript pubKey2(pubKeySerialized.begin(), pubKeySerialized.end());
-        popstack(stackCopy);
+        popstack(stack);
 
-        if (!EvalScript(stackCopy, pubKey2, flags, checker, serror))
+        if (!EvalScript(stack, pubKey2, flags, checker, serror))
             // serror is set
             return false;
-        if (stackCopy.empty())
+        if (stack.empty())
             return set_error(serror, SCRIPT_ERR_EVAL_FALSE);
-        if (!CastToBool(stackCopy.back()))
+        if (!CastToBool(stack.back()))
             return set_error(serror, SCRIPT_ERR_EVAL_FALSE);
-        else
-            return set_success(serror);
+    }
+
+    // The CLEANSTACK check is only performed after potential P2SH evaluation,
+    // as the non-P2SH evaluation of a P2SH script will obviously not result in
+    // a clean stack (the P2SH inputs remain).
+    if ((flags & SCRIPT_VERIFY_CLEANSTACK) != 0) {
+        // Disallow CLEANSTACK without P2SH, as otherwise a switch CLEANSTACK->P2SH+CLEANSTACK
+        // would be possible, which is not a softfork (and P2SH should be one).
+        assert((flags & SCRIPT_VERIFY_P2SH) != 0);
+        if (stack.size() != 1) {
+            return set_error(serror, SCRIPT_ERR_CLEANSTACK);
+        }
     }
 
     return set_success(serror);

diff --git a/src/script/interpreter.h b/src/script/interpreter.h
@@ -67,8 +67,14 @@ enum
     // discouraged NOPs fails the script. This verification flag will never be
     // a mandatory flag applied to scripts in a block. NOPs that are not
     // executed, e.g.  within an unexecuted IF ENDIF block, are *not* rejected.
-    SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS  = (1U << 7)
-
+    SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS  = (1U << 7),
+
+    // Require that only a single stack element remains after evaluation. This changes the success criterion from
+    // "At least one stack element must remain, and when interpreted as a boolean, it must be true" to
+    // "Exactly one stack element must remain, and when interpreted as a boolean, it must be true".
+    // (softfork safe, BIP62 rule 6)
+    // Note: CLEANSTACK should never be used without P2SH.
+    SCRIPT_VERIFY_CLEANSTACK = (1U << 8),
 };
 
 uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType);

diff --git a/src/script/script_error.h b/src/script/script_error.h
@@ -43,6 +43,7 @@ typedef enum ScriptError_t
     SCRIPT_ERR_SIG_HIGH_S,
     SCRIPT_ERR_SIG_NULLDUMMY,
     SCRIPT_ERR_PUBKEYTYPE,
+    SCRIPT_ERR_CLEANSTACK,
 
     /* softfork safeness */
     SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS,

diff --git a/src/script/standard.h b/src/script/standard.h
@@ -48,7 +48,8 @@ static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY
                                                          SCRIPT_VERIFY_STRICTENC |
                                                          SCRIPT_VERIFY_MINIMALDATA |
                                                          SCRIPT_VERIFY_NULLDUMMY |
-                                                         SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS;
+                                                         SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS |
+                                                         SCRIPT_VERIFY_CLEANSTACK;
 
 /** For convenience, standard but not mandatory verify flags. */
 static const unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS;

diff --git a/src/test/data/script_invalid.json b/src/test/data/script_invalid.json
@@ -667,6 +667,18 @@
     "SIGPUSHONLY",
     "P2SH(P2PK) with non-push scriptSig"
 ],
+[
+    "11 0x47 0x3044022057c4ba463d3b8e6848b3896be14c6953caf0528cd390ad15104a109c94b558ba02206fa3922154e1d0bfca92ce5f9adbe4991be33f3f5b7f6bf501e895b7e37fd72f01",
+    "0x41 0x0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 CHECKSIG",
+    "CLEANSTACK,P2SH",
+    "P2PK with unnecessary input"
+],
+[
+    "11 0x47 0x304402203407c26745ea95ee31fbb5074e730ff9be235d5a8d8e0a2c868358bf6a04797402205cc0e594dfd275583472168298d448be59c2530a4ea4d7c37311b82108f9f8bc01 0x43 0x410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ac",
+    "HASH160 0x14 0x31edc23bdafda4639e669f89ad6b2318dd79d032 EQUAL",
+    "CLEANSTACK,P2SH",
+    "P2SH with unnecessary input"
+],
 
 ["The End"]
 ]
diff --git a/src/test/data/script_valid.json b/src/test/data/script_valid.json
@@ -816,6 +816,24 @@
     "SIGPUSHONLY",
     "2-of-2 with two identical keys and sigs pushed"
 ],
+[
+    "11 0x47 0x304402204ba8b04dfe8657608427b996bd7c151ff8cd8579b3316c7314549a6c59f6bfb7022058cf052927fbc5e51e26dd4711c470bbf7f3adc8aaaf7bfa304eff6bb6e6399e01",
+    "0x41 0x0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 CHECKSIG",
+    "P2SH",
+    "P2PK with unnecessary input but no CLEANSTACK"
+],
+[
+    "11 0x47 0x304402202beaa2f6a4ec783091643797f9819b5ae39a03dfcf3b934746e96dd6b2ad5f7202200650a618fb2ce08b4edd160351172e016a041c81622d806390420d15cc6cece401 0x43 0x410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ac",
+    "HASH160 0x14 0x31edc23bdafda4639e669f89ad6b2318dd79d032 EQUAL",
+    "P2SH",
+    "P2SH with unnecessary input but no CLEANSTACK"
+],
+[
+    "0x47 0x3044022048505fd42afde400932558ea7fa76a52c2fff3130fa820a6a05647f64d5d780e022056a6bc823c95cedf68f6f67ad036533b48b9f3f706355a71f9f6e647e889f79201 0x43 0x410479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8ac",
+    "HASH160 0x14 0x31edc23bdafda4639e669f89ad6b2318dd79d032 EQUAL",
+    "CLEANSTACK,P2SH",
+    "P2SH with CLEANSTACK"
+],
 
 ["The End"]
 ]
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
@@ -483,6 +483,22 @@ BOOST_AUTO_TEST_CASE(script_build)
                                "2-of-2 with two identical keys and sigs pushed", SCRIPT_VERIFY_SIGPUSHONLY
                               ).Num(0).PushSig(keys.key1).PushSig(keys.key1));
 
+    good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG,
+                               "P2PK with unnecessary input but no CLEANSTACK", SCRIPT_VERIFY_P2SH
+                              ).Num(11).PushSig(keys.key0));
+    bad.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG,
+                              "P2PK with unnecessary input", SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH
+                             ).Num(11).PushSig(keys.key0));
+    good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG,
+                               "P2SH with unnecessary input but no CLEANSTACK", SCRIPT_VERIFY_P2SH, true
+                              ).Num(11).PushSig(keys.key0).PushRedeem());
+    bad.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG,
+                              "P2SH with unnecessary input", SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH, true
+                             ).Num(11).PushSig(keys.key0).PushRedeem());
+    good.push_back(TestBuilder(CScript() << ToByteVector(keys.pubkey0) << OP_CHECKSIG,
+                               "P2SH with CLEANSTACK", SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH, true
+                              ).PushSig(keys.key0).PushRedeem());
+
 
     std::set<std::string> tests_good;
     std::set<std::string> tests_bad;

diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
@@ -37,7 +37,8 @@ static std::map<string, unsigned int> mapFlagNames = boost::assign::map_list_of
     (string("SIGPUSHONLY"), (unsigned int)SCRIPT_VERIFY_SIGPUSHONLY)
     (string("MINIMALDATA"), (unsigned int)SCRIPT_VERIFY_MINIMALDATA)
     (string("NULLDUMMY"), (unsigned int)SCRIPT_VERIFY_NULLDUMMY)
-    (string("DISCOURAGE_UPGRADABLE_NOPS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS);
+    (string("DISCOURAGE_UPGRADABLE_NOPS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS)
+    (string("CLEANSTACK"), (unsigned int)SCRIPT_VERIFY_CLEANSTACK);
 
 unsigned int ParseScriptFlags(string strFlags)
 {