[Backport] 5.2.0 cs backports

src/chainparams.cpp

@@ -198,6 +198,8 @@ class CMainParams : public CChainParams
         consensus.vUpgrades[Consensus::UPGRADE_V3_4].nActivationHeight          = 1967000;
         consensus.vUpgrades[Consensus::UPGRADE_V4_0].nActivationHeight          = 2153200;
         consensus.vUpgrades[Consensus::UPGRADE_V5_0].nActivationHeight          = 2700500;
+        consensus.vUpgrades[Consensus::UPGRADE_V5_2].nActivationHeight =
+                Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;
 
         consensus.vUpgrades[Consensus::UPGRADE_ZC].hashActivationBlock =
                 uint256S("0x5b2482eca24caf2a46bb22e0545db7b7037282733faa3a42ec20542509999a64");
@@ -329,6 +331,8 @@ class CTestNetParams : public CChainParams
         consensus.vUpgrades[Consensus::UPGRADE_V3_4].nActivationHeight          = 201;
         consensus.vUpgrades[Consensus::UPGRADE_V4_0].nActivationHeight          = 201;
         consensus.vUpgrades[Consensus::UPGRADE_V5_0].nActivationHeight          = 201;
+        consensus.vUpgrades[Consensus::UPGRADE_V5_2].nActivationHeight =
+                Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;
 
         /**
          * The message start string is designed to be unlikely to occur in normal data.
@@ -454,6 +458,7 @@ class CRegTestParams : public CChainParams
         consensus.vUpgrades[Consensus::UPGRADE_V4_0].nActivationHeight          =
                 Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
         consensus.vUpgrades[Consensus::UPGRADE_V5_0].nActivationHeight          = 300;
+        consensus.vUpgrades[Consensus::UPGRADE_V5_2].nActivationHeight          = 300;
 
         /**
          * The message start string is designed to be unlikely to occur in normal data.

src/consensus/params.h

@@ -34,6 +34,7 @@ enum UpgradeIndex : uint32_t {
     UPGRADE_V3_4,
     UPGRADE_V4_0,
     UPGRADE_V5_0,
+    UPGRADE_V5_2,
     UPGRADE_TESTDUMMY,
     // NOTE: Also add new upgrades to NetworkUpgradeInfo in upgrades.cpp
     MAX_NETWORK_UPGRADES

src/consensus/upgrades.cpp

@@ -53,6 +53,10 @@ const struct NUInfo NetworkUpgradeInfo[Consensus::MAX_NETWORK_UPGRADES] = {
                 /*.strName =*/ "v5_shield",
                 /*.strInfo =*/ "Sapling Shield - start block v8 - start transaction v3",
         },
+        {
+                /*.strName =*/ "PIVX_v5.2",
+                /*.strInfo =*/ "new cold-staking rules",
+        },
         {
                 /*.strName =*/ "Test_dummy",
                 /*.strInfo =*/ "Test dummy info",

src/script/interpreter.cpp

@@ -962,10 +962,7 @@ 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(script)) {
-                        return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY);
-                    }
+                    return checker.CheckColdStake(script, stack, flags, serror);
                 }
                 break;
 
@@ -1339,15 +1336,35 @@ bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) con
     return true;
 }
 
-bool TransactionSignatureChecker::CheckColdStake(const CScript& prevoutScript) const
+bool TransactionSignatureChecker::CheckColdStake(const CScript& prevoutScript, std::vector<valtype>& stack, unsigned int flags, ScriptError* serror) const
 {
+    if (g_newP2CSRules) {
+        // the stack can contain only <sig> <pk> <pkh> at this point
+        if ((int)stack.size() != 3) {
+            return set_error(serror, SCRIPT_ERR_INVALID_STACK_OPERATION);
+        }
+        // check pubkey/signature encoding
+        valtype& vchSig    = stacktop(-3);
+        valtype& vchPubKey = stacktop(-2);
+        if (!CheckSignatureEncoding(vchSig, flags, serror) ||
+                !CheckPubKeyEncoding(vchPubKey, flags, serror)) {
+            // serror is set
+            return false;
+        }
+        // check hash size
+        valtype& vchPubKeyHash = stacktop(-1);
+        if ((int)vchPubKeyHash.size() != 20) {
+            return set_error(serror, SCRIPT_ERR_SCRIPT_SIZE);
+        }
+    }
+
     // Transaction must be a coinstake tx
     if (!txTo->IsCoinStake()) {
-        return false;
+        return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY);
     }
     // There must be one single input
     if (txTo->vin.size() != 1) {
-        return false;
+        return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY);
     }
     // Since this is a coinstake, it has at least 2 outputs
     const unsigned int outs = txTo->vout.size();
@@ -1362,13 +1379,16 @@ bool TransactionSignatureChecker::CheckColdStake(const CScript& prevoutScript) c
         if (txTo->vout[i].scriptPubKey != prevoutScript) {
             // Only the last one can be different (and only when outs >=3)
             if (i != outs-1 || outs < 3) {
-                return false;
+                return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY);
             }
         } else {
             outValue += txTo->vout[i].nValue;
         }
     }
-    return outValue > amount;
+    if (outValue < amount) {
+        return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY);
+    }
+    return true;
 }
 
 

src/script/interpreter.h

@@ -107,7 +107,7 @@ class BaseSignatureChecker
          return false;
     }
 
-    virtual bool CheckColdStake(const CScript& script) const
+    virtual bool CheckColdStake(const CScript& prevoutScript, std::vector<valtype>& stack, unsigned int flags, ScriptError* error) const
     {
          return false;
     }
@@ -132,7 +132,7 @@ class TransactionSignatureChecker : public BaseSignatureChecker
 
     bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override ;
     bool CheckLockTime(const CScriptNum& nLockTime) const override;
-    bool CheckColdStake(const CScript& prevoutScript) const override;
+    bool CheckColdStake(const CScript& prevoutScript, std::vector<valtype>& stack, unsigned int flags, ScriptError* error) const override;
 };
 
 class MutableTransactionSignatureChecker : public TransactionSignatureChecker

src/script/script.cpp

@@ -223,14 +223,23 @@ bool CScript::IsPayToScriptHash() const
             (*this)[22] == OP_EQUAL);
 }
 
+// contextual flag to guard the new rules for P2CS.
+// can be removed once v5.2 enforcement is activated.
+std::atomic<bool> g_newP2CSRules{false};
+
+// P2CS script: either with or without last output free
 bool CScript::IsPayToColdStaking() const
 {
-    // Extra-fast test for pay-to-cold-staking CScripts:
     return (this->size() == 51 &&
+            (!g_newP2CSRules || (*this)[0] == OP_DUP) &&
+            (!g_newP2CSRules || (*this)[1] == OP_HASH160) &&
             (*this)[2] == OP_ROT &&
+            (!g_newP2CSRules || (*this)[3] == OP_IF) &&
             (*this)[4] == OP_CHECKCOLDSTAKEVERIFY &&
             (*this)[5] == 0x14 &&
+            (!g_newP2CSRules || (*this)[26] == OP_ELSE) &&
             (*this)[27] == 0x14 &&
+            (!g_newP2CSRules || (*this)[48] == OP_ENDIF) &&
             (*this)[49] == OP_EQUALVERIFY &&
             (*this)[50] == OP_CHECKSIG);
 }

src/script/script.h

@@ -658,4 +658,8 @@ class CScript : public CScriptBase
     size_t DynamicMemoryUsage() const;
 };
 
+// contextual flag to guard the new rules for P2CS.
+// can be removed once v5.2 enforcement is activated.
+extern std::atomic<bool> g_newP2CSRules;
+
 #endif // BITCOIN_SCRIPT_SCRIPT_H

src/test/script_P2CS_tests.cpp

@@ -6,10 +6,12 @@
 #include "base58.h"
 #include "key.h"
 #include "policy/policy.h"
+#include "wallet/test/wallet_test_fixture.h"
+#include "wallet/wallet.h"
 
 #include <boost/test/unit_test.hpp>
 
-BOOST_FIXTURE_TEST_SUITE(script_P2CS_tests, TestingSetup)
+BOOST_FIXTURE_TEST_SUITE(script_P2CS_tests, WalletTestingSetup)
 
 void CheckValidKeyId(const CTxDestination& dest, const CKeyID& expectedKey)
 {
@@ -158,8 +160,8 @@ BOOST_AUTO_TEST_CASE(coldstake_script)
     BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err));
 
     // Transfer more coins to the masternode
-    tx.vout[2].nValue -= 2 * COIN;
-    tx.vout[3].nValue += 2 * COIN;
+    tx.vout[2].nValue -= 3 * COIN;
+    tx.vout[3].nValue += 3 * COIN;
     SignColdStake(tx, 0, scriptP2CS, stakerKey, true);
     BOOST_CHECK(!CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err));
     BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_CHECKCOLDSTAKEVERIFY, ScriptErrorString(err));
@@ -197,4 +199,106 @@ BOOST_AUTO_TEST_CASE(coldstake_script)
     BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err));
 }
 
+// Check that it's not possible to "fake" a P2CS script for the owner by splitting the locking
+// and unlocking parts. This particular script can be spent by any key, with a
+// unlocking script composed like: <sig> <pk> <DUP> <HASH160> <pkh>
+static CScript GetFakeLockingScript(const CKeyID staker, const CKeyID& owner)
+{
+    CScript script;
+    script << opcodetype(0x2F) << opcodetype(0x01) << OP_ROT <<
+            OP_IF << OP_CHECKCOLDSTAKEVERIFY << ToByteVector(staker) <<
+            OP_ELSE << ToByteVector(owner) << OP_DROP <<
+            OP_EQUALVERIFY << OP_CHECKSIG;
+
+    return script;
+}
+
+void FakeUnlockColdStake(CMutableTransaction& tx, const CScript& prevScript, const CKey& key)
+{
+    // sign the first input
+    tx.vin[0].scriptSig.clear();
+    const CTransaction _tx(tx);
+    SigVersion sv = _tx.GetRequiredSigVersion();
+    const uint256& hash = SignatureHash(prevScript, _tx, 0, SIGHASH_ALL, amtIn, sv);
+    std::vector<unsigned char> vchSig;
+    BOOST_CHECK(key.Sign(hash, vchSig));
+    vchSig.push_back((unsigned char)SIGHASH_ALL);
+    tx.vin[0].scriptSig << vchSig << ToByteVector(key.GetPubKey()) << OP_DUP << OP_HASH160 << ToByteVector(key.GetPubKey().GetID());
+}
+
+static void setupWallet(CWallet& wallet)
+{
+    wallet.SetMinVersion(FEATURE_SAPLING);
+    wallet.SetupSPKM(false);
+}
+
+BOOST_AUTO_TEST_CASE(fake_script_test)
+{
+    BOOST_ASSERT(!g_newP2CSRules);
+
+    CWallet& wallet = *pwalletMain;
+    LOCK(wallet.cs_wallet);
+    setupWallet(wallet);
+    CKey stakerKey;         // dummy staker key (not in the wallet)
+    stakerKey.MakeNewKey(true);
+    CKeyID stakerId = stakerKey.GetPubKey().GetID();
+    CPubKey ownerPubKey;
+    BOOST_ASSERT(wallet.GetKeyFromPool(ownerPubKey));
+    const CKeyID& ownerId = ownerPubKey.GetID();
+    CKey ownerKey;          // owner key (in the wallet)
+    BOOST_ASSERT(wallet.GetKey(ownerId, ownerKey));
+
+    const CScript& scriptP2CS = GetFakeLockingScript(stakerId, ownerId);
+
+    // Create prev transaction:
+    // It has two outputs. The first one is spent before v5.2.
+    // The second one is tested after v5.2 enforcement.
+    CMutableTransaction txFrom;
+    txFrom.vout.resize(2);
+    for (size_t i = 0; i < 2; i++) {
+        txFrom.vout[i].nValue = amtIn;
+        txFrom.vout[i].scriptPubKey = scriptP2CS;
+    }
+
+    // passes IsPayToColdStaking
+    BOOST_CHECK(scriptP2CS.IsPayToColdStaking());
+
+    // the output amount is credited to the owner wallet
+    wallet.AddToWallet({&wallet, MakeTransactionRef(CTransaction(txFrom))});
+    BOOST_CHECK_EQUAL(wallet.GetWalletTx(txFrom.GetHash())->GetAvailableCredit(false, ISMINE_SPENDABLE_TRANSPARENT), 2 * amtIn);
+
+    // create spend tx
+    CMutableTransaction tx;
+    tx.vin.resize(1);
+    tx.vout.resize(1);
+    tx.vin[0].prevout.n = 0;
+    tx.vin[0].prevout.hash = txFrom.GetHash();
+    tx.vout[0].nValue = amtIn - 10000;
+    tx.vout[0].scriptPubKey = GetScriptForDestination(stakerId);
+
+    // it cannot be spent with the owner key, using the P2CS unlocking script
+    SignColdStake(tx, 0, scriptP2CS, ownerKey, false);
+    ScriptError err = SCRIPT_ERR_OK;
+    BOOST_CHECK(!CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err));
+    BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EQUALVERIFY, ScriptErrorString(err));
+
+    // ... but it can be spent by the staker (or any) key, with the fake unlocking script
+    FakeUnlockColdStake(tx, scriptP2CS, stakerKey);
+    if (!CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)) {
+        BOOST_ERROR(strprintf("P2CS verification failed: %s", ScriptErrorString(err)));
+    }
+    wallet.AddToWallet({&wallet, MakeTransactionRef(CTransaction(tx))});
+    BOOST_CHECK_EQUAL(wallet.GetWalletTx(txFrom.GetHash())->GetAvailableCredit(false, ISMINE_SPENDABLE_TRANSPARENT), amtIn);
+
+    // Now let's activate new rules
+    g_newP2CSRules = true;
+
+    // it does NOT pass IsPayToColdStaking
+    BOOST_CHECK_MESSAGE(!scriptP2CS.IsPayToColdStaking(), "Fake script passes as P2CS");
+
+    // the output amount is NOT credited to the owner wallet
+    BOOST_CHECK_EQUAL(wallet.GetWalletTx(txFrom.GetHash())->GetAvailableCredit(false, ISMINE_SPENDABLE_TRANSPARENT), 0);
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

src/validation.cpp

@@ -1812,6 +1812,7 @@ void static UpdateTip(CBlockIndex* pindexNew)
 {
     AssertLockHeld(cs_main);
     chainActive.SetTip(pindexNew);
+    g_newP2CSRules = Params().GetConsensus().NetworkUpgradeActive(pindexNew->nHeight, Consensus::UPGRADE_V5_2);
 
     // New best block
     mempool.AddTransactionsUpdated(1);
@@ -3529,6 +3530,9 @@ bool LoadChainTip(const CChainParams& chainparams)
 
     const CBlockIndex* pChainTip = chainActive.Tip();
 
+    // initial global flag update
+    g_newP2CSRules = Params().GetConsensus().NetworkUpgradeActive(pChainTip->nHeight, Consensus::UPGRADE_V5_2);
+
     LogPrintf("Loaded best chain: hashBestChain=%s height=%d date=%s progress=%f\n",
             pChainTip->GetBlockHash().GetHex(), pChainTip->nHeight,
             DateTimeStrFormat("%Y-%m-%d %H:%M:%S", pChainTip->GetBlockTime()),