[Script] Unify OP_CHECKCOLDSTAKEVERIFY checks

random-zebra · random-zebra · commit 75b8861bf17f · 2019-10-23T02:22:52.000+02:00
Pass the script that we are spending to CTransaction::CheckColdStake(), directly from the interpreter. This way, there's no need for additional verifications in CheckTransaction (except for enforcement/activation).
diff --git a/src/main.cpp b/src/main.cpp
@@ -1174,25 +1174,6 @@ bool CheckZerocoinSpend(const CTransaction& tx, bool fVerifySignature, CValidati
     return fValidated;
 }
 
-bool CheckColdStake(const CTransaction& tx, CValidationState& state, bool fColdStakingActive)
-{
-    CTxOut prevOut;
-    if(!GetOutput(tx.vin[0].prevout.hash, tx.vin[0].prevout.n, state, prevOut))
-        return state.DoS(100, error("%s : invalid input", __func__), REJECT_INVALID, "bad-txns-inputs");
-
-    if (!prevOut.scriptPubKey.IsPayToColdStaking())
-        return true;
-
-    if (!fColdStakingActive)
-        return state.DoS(100, error("%s : invalid input", __func__), REJECT_INVALID, "coldstake-not-active");
-
-    // spending to the same contract
-    if (prevOut.scriptPubKey != tx.vout[1].scriptPubKey)
-        return state.DoS(100, error("%s : invalid scripts", __func__), REJECT_INVALID, "bad-txns-cold-stake");
-
-    return true;
-}
-
 bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, bool fRejectBadUTXO, CValidationState& state, bool fFakeSerialAttack, bool fColdStakingActive)
 {
     // Basic checks that don't depend on any context
@@ -1241,10 +1222,6 @@ bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, bool fReject
         }
     }
 
-    // Additional check for cold staking
-    if (tx.IsCoinStake() && !tx.HasZerocoinSpendInputs() && !CheckColdStake(tx, state, fColdStakingActive))
-        return state.DoS(100, error("%s: invalid cold stake", __func__), REJECT_INVALID, "bad-txns-cold-stake");
-
     std::set<COutPoint> vInOutPoints;
     std::set<CBigNum> vZerocoinSpendSerials;
     int nZCSpendCount = 0;
@@ -4474,8 +4451,9 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
         if (!IsInitialBlockDownload() && block.vtx[1].HasP2CSOutputs() && !sporkManager.IsSporkActive(SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT)) {
             const int outputs = block.vtx[1].vout.size();
             if (outputs >=3 && block.vtx[1].vout[outputs-1].scriptPubKey != block.vtx[1].vout[outputs-2].scriptPubKey)
-                return state.DoS(100, error("%s: Wrong cold staking outputs when masternode payment enforcement is disabled on tx %s",
-                        __func__, block.vtx[1].ToString().c_str()));
+                return state.DoS(100, error("%s: Wrong cold staking outputs when masternode payment enforcement is disabled: "
+                        "script[-1] (%s) != script[-2] (%s)", __func__,
+                        HexStr(block.vtx[1].vout[outputs-1].scriptPubKey), HexStr(block.vtx[1].vout[outputs-2].scriptPubKey)));
         }
     }
 
diff --git a/src/main.h b/src/main.h
@@ -351,11 +351,6 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma
  */
 bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector<CScriptCheck>* pvChecks = NULL);
 
-/*
- * Check cold stakes P2CS script rules and enforcement activation
- */
-bool CheckColdStake(const CTransaction& tx, CValidationState& state, bool fColdStakingActive);
-
 /** Apply the effects of this transaction on the UTXO set represented by view */
 void UpdateCoins(const CTransaction& tx, CValidationState& state, CCoinsViewCache& inputs, CTxUndo& txundo, int nHeight);
 
diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp
@@ -209,28 +209,28 @@ bool CTransaction::IsCoinStake() const
     return (vout.size() >= 2 && vout[0].IsEmpty());
 }
 
-bool CTransaction::CheckColdStake() const
+bool CTransaction::CheckColdStake(const CScript& script) const
 {
+
     // tx is a coinstake tx
     if (!IsCoinStake())
         return false;
 
-    // all inputs have the same pubKeyScript
+    // all inputs have the same scriptSig
     CScript firstScript = vin[0].scriptSig;
     if (vin.size() > 1) {
         for (unsigned int i=1; i<vin.size(); i++)
             if (vin[i].scriptSig != firstScript) return false;
     }
 
     // all outputs except first (coinstake marker) and last (masternode payout)
-    // have the same pubKeyScript
-    firstScript = vout[1].scriptPubKey;
-    if (vout.size() > 3) {
+    // have the same pubKeyScript and it matches the script we are spending
+    if (vout[1].scriptPubKey != script) return false;
+    if (vin.size() > 3) {
         for (unsigned int i=2; i<vout.size()-1; i++)
-            if (vout[i].scriptPubKey != firstScript) return false;
+            if (vout[i].scriptPubKey != script) return false;
     }
 
-    // additional checks in CheckTransaction
     return true;
 }
 
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
@@ -283,7 +283,7 @@ class CTransaction
     }
 
     bool IsCoinStake() const;
-    bool CheckColdStake() const;
+    bool CheckColdStake(const CScript& script) const;
     bool HasP2CSOutputs() const;
 
     friend bool operator==(const CTransaction& a, const CTransaction& b)
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
@@ -962,10 +962,9 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                 case OP_CHECKCOLDSTAKEVERIFY:
                 {
                     // check it is used in a valid cold stake transaction.
-                    if(!checker.CheckColdStake()) {
+                    if(!checker.CheckColdStake(script)) {
                         return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY);
                     }
-                    // CheckTransaction verifies that prevout scripts match with out scripts
                 }
                 break;
 
diff --git a/src/script/interpreter.h b/src/script/interpreter.h
@@ -98,7 +98,7 @@ class BaseSignatureChecker
          return false;
     }
 
-    virtual bool CheckColdStake() const
+    virtual bool CheckColdStake(const CScript& script) const
     {
          return false;
     }
@@ -119,8 +119,8 @@ class TransactionSignatureChecker : public BaseSignatureChecker
     TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn) : txTo(txToIn), nIn(nInIn) {}
     bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode) const;
     bool CheckLockTime(const CScriptNum& nLockTime) const;
-    bool CheckColdStake() const override {
-        return txTo->CheckColdStake();
+    bool CheckColdStake(const CScript& script) const override {
+        return txTo->CheckColdStake(script);
     }
 };
 

Original file line number	Diff line number	Diff line change
`@@ -283,7 +283,7 @@ class CTransaction`
`283`	`283`	`}`
`284`	`284`
`285`	`285`	`bool IsCoinStake() const;`
`286`		`- bool CheckColdStake() const;`
	`286`	`+ bool CheckColdStake(const CScript& script) const;`
`287`	`287`	`bool HasP2CSOutputs() const;`
`288`	`288`
`289`	`289`	`friend bool operator==(const CTransaction& a, const CTransaction& b)`
Original file line number	Diff line number	Diff line change
`@@ -962,10 +962,9 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&`
`962`	`962`	`case OP_CHECKCOLDSTAKEVERIFY:`
`963`	`963`	`{`
`964`	`964`	`// check it is used in a valid cold stake transaction.`
`965`		`- if(!checker.CheckColdStake()) {`
	`965`	`+ if(!checker.CheckColdStake(script)) {`
`966`	`966`	`return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY);`
`967`	`967`	`}`
`968`		`- // CheckTransaction verifies that prevout scripts match with out scripts`
`969`	`968`	`}`
`970`	`969`	`break;`
`971`	`970`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ class BaseSignatureChecker`
`98`	`98`	`return false;`
`99`	`99`	`}`
`100`	`100`
`101`		`- virtual bool CheckColdStake() const`
	`101`	`+ virtual bool CheckColdStake(const CScript& script) const`
`102`	`102`	`{`
`103`	`103`	`return false;`
`104`	`104`	`}`
`@@ -119,8 +119,8 @@ class TransactionSignatureChecker : public BaseSignatureChecker`
`119`	`119`	`TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn) : txTo(txToIn), nIn(nInIn) {}`
`120`	`120`	`bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode) const;`
`121`	`121`	`bool CheckLockTime(const CScriptNum& nLockTime) const;`
`122`		`- bool CheckColdStake() const override {`
`123`		`- return txTo->CheckColdStake();`
	`122`	`+ bool CheckColdStake(const CScript& script) const override {`
	`123`	`+ return txTo->CheckColdStake(script);`
`124`	`124`	`}`
`125`	`125`	`};`
`126`	`126`