Discourage NOPs reserved for soft-fork upgrades

NOP1 to NOP10 are reserved for future soft-fork upgrades. In the event
of an upgrade such NOPs have *VERIFY behavior, meaning that if their
arguments are not correct the script fails. Discouraging these NOPs by
rejecting transactions containing them from the mempool ensures that
we'll never accept transactions, nor mine blocks, with scripts that are
now invalid according to the majority of hashing power even if we're not
yet upgraded. Previously this wasn't an issue as the IsStandard() rules
didn't allow upgradable NOPs anyway, but 7f3b4e95 relaxed the
IsStandard() rules for P2SH redemptions allowing any redeemScript to be
spent.

We *do* allow upgradable NOPs in scripts so long as they are not
executed. This is harmless as there is no opportunity for the script to
be invalid post-upgrade.

8 files changed, 49 insertions, 3 deletions

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index cf81fe30a2..760086eaba 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -329,8 +329,14 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, un
                 // Control
                 //
                 case OP_NOP:
+                break;
+
                 case OP_NOP1: case OP_NOP2: case OP_NOP3: case OP_NOP4: case OP_NOP5:
                 case OP_NOP6: case OP_NOP7: case OP_NOP8: case OP_NOP9: case OP_NOP10:
+                {
+                    if (flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS)
+                        return set_error(serror, SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS);
+                }
                 break;
 
                 case OP_IF:
diff --git a/src/script/interpreter.h b/src/script/interpreter.h
index 14cccc558f..12b2719414 100644
--- a/src/script/interpreter.h
+++ b/src/script/interpreter.h
@@ -57,7 +57,18 @@ enum
     // any other push causes the script to fail (BIP62 rule 3).
     // In addition, whenever a stack element is interpreted as a number, it must be of minimal length (BIP62 rule 4).
     // (softfork safe)
-    SCRIPT_VERIFY_MINIMALDATA = (1U << 6)
+    SCRIPT_VERIFY_MINIMALDATA = (1U << 6),
+
+    // Discourage use of NOPs reserved for upgrades (NOP1-10)
+    //
+    // Provided so that nodes can avoid accepting or mining transactions
+    // containing executed NOP's whose meaning may change after a soft-fork,
+    // thus rendering the script invalid; with this flag set executing
+    // 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)
+
 };
 
 uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType);
diff --git a/src/script/script_error.cpp b/src/script/script_error.cpp
index 4a3df268ec..793fc0da41 100644
--- a/src/script/script_error.cpp
+++ b/src/script/script_error.cpp
@@ -59,6 +59,8 @@ const char* ScriptErrorString(const ScriptError serror)
             return "Non-canonical signature: S value is unnecessarily high";
         case SCRIPT_ERR_SIG_NULLDUMMY:
             return "Dummy CHECKMULTISIG argument must be zero";
+        case SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS:
+            return "NOPx reserved for soft-fork upgrades";
         case SCRIPT_ERR_UNKNOWN_ERROR:
         case SCRIPT_ERR_ERROR_COUNT:
         default: break;
diff --git a/src/script/script_error.h b/src/script/script_error.h
index ae6626b257..21153f1bd1 100644
--- a/src/script/script_error.h
+++ b/src/script/script_error.h
@@ -43,6 +43,9 @@ typedef enum ScriptError_t
     SCRIPT_ERR_SIG_HIGH_S,
     SCRIPT_ERR_SIG_NULLDUMMY,
 
+    /* softfork safeness */
+    SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS,
+
     SCRIPT_ERR_ERROR_COUNT
 } ScriptError;
 
diff --git a/src/script/standard.h b/src/script/standard.h
index f3dcc75fdc..c4b82b4c45 100644
--- a/src/script/standard.h
+++ b/src/script/standard.h
@@ -47,7 +47,8 @@ static const unsigned int MANDATORY_SCRIPT_VERIFY_FLAGS = SCRIPT_VERIFY_P2SH;
 static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY_FLAGS |
                                                          SCRIPT_VERIFY_STRICTENC |
                                                          SCRIPT_VERIFY_MINIMALDATA |
-                                                         SCRIPT_VERIFY_NULLDUMMY;
+                                                         SCRIPT_VERIFY_NULLDUMMY |
+                                                         SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS;
 
 /** 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
index 6f451a36ee..0356d0be1c 100644
--- a/src/test/data/script_invalid.json
+++ b/src/test/data/script_invalid.json
@@ -163,6 +163,23 @@ nSequences are max.
 ["1","NOP1 NOP2 NOP3 NOP4 NOP5 NOP6 NOP7 NOP8 NOP9 NOP10 2 EQUAL", "P2SH,STRICTENC"],
 ["'NOP_1_to_10' NOP1 NOP2 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", "NOP2",  "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"],
diff --git a/src/test/data/script_valid.json b/src/test/data/script_valid.json
index 439c82ef32..0cf1563163 100644
--- a/src/test/data/script_valid.json
+++ b/src/test/data/script_valid.json
@@ -235,6 +235,11 @@ nSequences are max.
 ["1","NOP1 NOP2 NOP3 NOP4 NOP5 NOP6 NOP7 NOP8 NOP9 NOP10 1 EQUAL", "P2SH,STRICTENC"],
 ["'NOP_1_to_10' NOP1 NOP2 NOP3 NOP4 NOP5 NOP6 NOP7 NOP8 NOP9 NOP10","'NOP_1_to_10' EQUAL", "P2SH,STRICTENC"],
 
+["1", "NOP", "P2SH,STRICTENC,DISCOURAGE_UPGRADABLE_NOPS", "Discourage NOPx flag allows OP_NOP"],
+
+["0", "IF NOP10 ENDIF 1", "P2SH,STRICTENC,DISCOURAGE_UPGRADABLE_NOPS",
+ "Discouraged NOPs are allowed if not executed"],
+
 ["0", "IF 0xba ELSE 1 ENDIF", "P2SH,STRICTENC", "opcodes above NOP10 invalid if executed"],
 ["0", "IF 0xbb ELSE 1 ENDIF", "P2SH,STRICTENC"],
 ["0", "IF 0xbc ELSE 1 ENDIF", "P2SH,STRICTENC"],
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index bf3a60c04f..e939e89972 100644
--- 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("LOW_S"), (unsigned int)SCRIPT_VERIFY_LOW_S)
     (string("SIGPUSHONLY"), (unsigned int)SCRIPT_VERIFY_SIGPUSHONLY)
     (string("MINIMALDATA"), (unsigned int)SCRIPT_VERIFY_MINIMALDATA)
-    (string("NULLDUMMY"), (unsigned int)SCRIPT_VERIFY_NULLDUMMY);
+    (string("NULLDUMMY"), (unsigned int)SCRIPT_VERIFY_NULLDUMMY)
+    (string("DISCOURAGE_UPGRADABLE_NOPS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS);
 
 unsigned int ParseScriptFlags(string strFlags)
 {