From 9531fb8b6af42f30746010af5366a6540032d236 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 22 Mar 2021 16:07:54 +0100 Subject: [PATCH 1/6] [Tests] Proof of concept for P2CS vulnerability >>> Backports dced6c0f3fc749d7c825b9a939b6a33697085a79 (#2258) The validation for P2CS scriptPubKey is incomplete and doesn't check all the opcodes. This opens a vulnerability. A script could be crafted so that: * it is identified as P2CS (passing IsPayToColdStaking), and recognized by the wallet as ISMINE_SPENDABLE_DELEGATED. * the assumed owner is not actually the owner (or not the only one) of the coins. In this Proof of concept, we craft a script that is recognized as own P2CS by the owner wallet, but can actually be spent with **any** key. This is achieved by including OP_DROP in a strategic position, so that, during the script evaluation, part of the locking condition (included only to fake IsPayToColdStaking) is removed, and replaced by a new condition embedded in the spending scriptSig. --- src/test/script_P2CS_tests.cpp | 87 +++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index a62bed9560cd..7c19b4a8a0ba 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/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_FIXTURE_TEST_SUITE(script_P2CS_tests, TestingSetup) +BOOST_FIXTURE_TEST_SUITE(script_P2CS_tests, WalletTestingSetup) void CheckValidKeyId(const CTxDestination& dest, const CKeyID& expectedKey) { @@ -197,4 +199,87 @@ 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. +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 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) +{ + 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: + CMutableTransaction txFrom; + txFrom.vout.resize(1); + txFrom.vout[0].nValue = amtIn; + txFrom.vout[0].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), 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); + BOOST_CHECK_MESSAGE(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err), ScriptErrorString(err)); + wallet.AddToWallet({&wallet, MakeTransactionRef(CTransaction(tx))}); + BOOST_CHECK_EQUAL(wallet.GetWalletTx(txFrom.GetHash())->GetAvailableCredit(false, ISMINE_SPENDABLE_TRANSPARENT), 0); +} + BOOST_AUTO_TEST_SUITE_END() From 84ed2d49788c46025bd19d2a2c1d46bc1b52e70f Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 22 Mar 2021 15:03:25 +0100 Subject: [PATCH 2/6] [Script] Strict checks for OP_CHECKCOLDSTAKEVERIFY >>> Backports 889a9e74cc8a7c20af68862b1f9de4a9cb09a549 (#2258) Ensure stack consistency (size, signature and pubkey encoding) during evaluation --- src/script/interpreter.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 7e7312b003f3..f26842d2b598 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -962,7 +962,23 @@ bool EvalScript(std::vector >& stack, const CScript& case OP_CHECKCOLDSTAKEVERIFY: { - // check it is used in a valid cold stake transaction. + // the stack can contain only 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); + } if(!checker.CheckColdStake(script)) { return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY); } From fecbb4a1544dbe888885e0df1a3022ff81733533 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 22 Mar 2021 15:34:42 +0100 Subject: [PATCH 3/6] [Script] Strict test for IsPayToColdStaking >>> Backports 74bc415d038c49d93a9959fe252089b5bffca912 (#2258) check the whole script template (leave only the 20 bytes for the staker keyID and 20 for the owner keyID). --- src/script/script.cpp | 6 +++++- src/test/script_P2CS_tests.cpp | 9 +++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/script/script.cpp b/src/script/script.cpp index b4c367a8e5ce..81ec9dc87b61 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -225,12 +225,16 @@ bool CScript::IsPayToScriptHash() const bool CScript::IsPayToColdStaking() const { - // Extra-fast test for pay-to-cold-staking CScripts: return (this->size() == 51 && + (*this)[0] == OP_DUP && + (*this)[1] == OP_HASH160 && (*this)[2] == OP_ROT && + (*this)[3] == OP_IF && (*this)[4] == OP_CHECKCOLDSTAKEVERIFY && (*this)[5] == 0x14 && + (*this)[26] == OP_ELSE && (*this)[27] == 0x14 && + (*this)[48] == OP_ENDIF && (*this)[49] == OP_EQUALVERIFY && (*this)[50] == OP_CHECKSIG); } diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index 7c19b4a8a0ba..b028de1b1fe2 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -200,7 +200,8 @@ BOOST_AUTO_TEST_CASE(coldstake_script) } // Check that it's not possible to "fake" a P2CS script for the owner by splitting the locking -// and unlocking parts. +// and unlocking parts. This particular script can be spent by any key, with a +// unlocking script composed like: static CScript GetFakeLockingScript(const CKeyID staker, const CKeyID& owner) { CScript script; @@ -231,6 +232,7 @@ static void setupWallet(CWallet& wallet) wallet.SetupSPKM(false); } +/* !TODO: check before/after v6 enforcement BOOST_AUTO_TEST_CASE(fake_script_test) { CWallet& wallet = *pwalletMain; @@ -277,9 +279,12 @@ BOOST_AUTO_TEST_CASE(fake_script_test) // ... but it can be spent by the staker (or any) key, with the fake unlocking script FakeUnlockColdStake(tx, scriptP2CS, stakerKey); - BOOST_CHECK_MESSAGE(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err), ScriptErrorString(err)); + 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), 0); } +*/ BOOST_AUTO_TEST_SUITE_END() From 7bec5312902a07055e66d59b3dff446daa642469 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Fri, 18 Jun 2021 18:41:14 +0200 Subject: [PATCH 4/6] [Consensus] Introduce V5.2 network-upgrade params >>> Backports 4e0c0b6790d06a983a534f6c79745e12a1cefcab (#2428) --- src/chainparams.cpp | 5 +++++ src/consensus/params.h | 1 + src/consensus/upgrades.cpp | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 430eecb455da..6d77370e8df0 100644 --- a/src/chainparams.cpp +++ b/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. diff --git a/src/consensus/params.h b/src/consensus/params.h index c5b2a76a13bd..fd0aa52c9253 100644 --- a/src/consensus/params.h +++ b/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 diff --git a/src/consensus/upgrades.cpp b/src/consensus/upgrades.cpp index 2de9cc45f37f..a22ae32e58a8 100644 --- a/src/consensus/upgrades.cpp +++ b/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", From 5358c50212348a61ca571d4c925adef5e905a5a3 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Fri, 18 Jun 2021 19:36:39 +0200 Subject: [PATCH 5/6] [Refactor] Move stack check inside CheckColdStake >>> adapted from df116317ecb5afe8b754207c269cf9f5c0005d80 (#2275) without introducing the new opcode --- src/script/interpreter.cpp | 52 ++++++++++++++++++---------------- src/script/interpreter.h | 4 +-- src/test/script_P2CS_tests.cpp | 4 +-- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index f26842d2b598..6635eb7df0d5 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -962,26 +962,7 @@ bool EvalScript(std::vector >& stack, const CScript& case OP_CHECKCOLDSTAKEVERIFY: { - // the stack can contain only 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); - } - if(!checker.CheckColdStake(script)) { - return set_error(serror, SCRIPT_ERR_CHECKCOLDSTAKEVERIFY); - } + return checker.CheckColdStake(script, stack, flags, serror); } break; @@ -1355,15 +1336,33 @@ bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) con return true; } -bool TransactionSignatureChecker::CheckColdStake(const CScript& prevoutScript) const +bool TransactionSignatureChecker::CheckColdStake(const CScript& prevoutScript, std::vector& stack, unsigned int flags, ScriptError* serror) const { + // the stack can contain only 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(); @@ -1378,13 +1377,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; } diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 3fe927eb99fc..8da9272caa83 100644 --- a/src/script/interpreter.h +++ b/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& stack, unsigned int flags, ScriptError* error) const { return false; } @@ -132,7 +132,7 @@ class TransactionSignatureChecker : public BaseSignatureChecker bool CheckSig(const std::vector& scriptSig, const std::vector& 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& stack, unsigned int flags, ScriptError* error) const override; }; class MutableTransactionSignatureChecker : public TransactionSignatureChecker diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index b028de1b1fe2..1252f67c4d39 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -160,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)); From 62bf09573b082d51eb1c5b5be9053a0fe3752958 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Fri, 18 Jun 2021 18:56:00 +0200 Subject: [PATCH 6/6] [Consensus] use v5.2 params to guard new cold-staking rules >>> Backports caad3fb57fa8d7e4b33405be21dee8fa3659932c (#2428) --- src/script/interpreter.cpp | 34 ++++++++++++++++++---------------- src/script/script.cpp | 15 ++++++++++----- src/script/script.h | 4 ++++ src/test/script_P2CS_tests.cpp | 26 ++++++++++++++++++++------ src/validation.cpp | 4 ++++ 5 files changed, 56 insertions(+), 27 deletions(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 6635eb7df0d5..adba8d0a741b 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1338,22 +1338,24 @@ bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) con bool TransactionSignatureChecker::CheckColdStake(const CScript& prevoutScript, std::vector& stack, unsigned int flags, ScriptError* serror) const { - // the stack can contain only 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); + if (g_newP2CSRules) { + // the stack can contain only 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 diff --git a/src/script/script.cpp b/src/script/script.cpp index 81ec9dc87b61..8b9c7073578f 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -223,18 +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 g_newP2CSRules{false}; + +// P2CS script: either with or without last output free bool CScript::IsPayToColdStaking() const { return (this->size() == 51 && - (*this)[0] == OP_DUP && - (*this)[1] == OP_HASH160 && + (!g_newP2CSRules || (*this)[0] == OP_DUP) && + (!g_newP2CSRules || (*this)[1] == OP_HASH160) && (*this)[2] == OP_ROT && - (*this)[3] == OP_IF && + (!g_newP2CSRules || (*this)[3] == OP_IF) && (*this)[4] == OP_CHECKCOLDSTAKEVERIFY && (*this)[5] == 0x14 && - (*this)[26] == OP_ELSE && + (!g_newP2CSRules || (*this)[26] == OP_ELSE) && (*this)[27] == 0x14 && - (*this)[48] == OP_ENDIF && + (!g_newP2CSRules || (*this)[48] == OP_ENDIF) && (*this)[49] == OP_EQUALVERIFY && (*this)[50] == OP_CHECKSIG); } diff --git a/src/script/script.h b/src/script/script.h index a40990c7df05..93115363224f 100644 --- a/src/script/script.h +++ b/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 g_newP2CSRules; + #endif // BITCOIN_SCRIPT_SCRIPT_H diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index 1252f67c4d39..82346d19587e 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -232,9 +232,10 @@ static void setupWallet(CWallet& wallet) wallet.SetupSPKM(false); } -/* !TODO: check before/after v6 enforcement BOOST_AUTO_TEST_CASE(fake_script_test) { + BOOST_ASSERT(!g_newP2CSRules); + CWallet& wallet = *pwalletMain; LOCK(wallet.cs_wallet); setupWallet(wallet); @@ -250,17 +251,21 @@ BOOST_AUTO_TEST_CASE(fake_script_test) 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(1); - txFrom.vout[0].nValue = amtIn; - txFrom.vout[0].scriptPubKey = scriptP2CS; + 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), amtIn); + BOOST_CHECK_EQUAL(wallet.GetWalletTx(txFrom.GetHash())->GetAvailableCredit(false, ISMINE_SPENDABLE_TRANSPARENT), 2 * amtIn); // create spend tx CMutableTransaction tx; @@ -283,8 +288,17 @@ BOOST_AUTO_TEST_CASE(fake_script_test) 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() diff --git a/src/validation.cpp b/src/validation.cpp index ee6b90377b8a..ba792c3f788f 100644 --- a/src/validation.cpp +++ b/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()),