From b194386d8bce3f65b142ccc7b8f5c651521149cd Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 1 Mar 2021 16:50:30 +0100 Subject: [PATCH 1/6] [Refactor] Rename CCSV opcode to OP_CHECKCOLDSTAKEVERIFY_LOF Last Output Free --- src/script/interpreter.cpp | 2 +- src/script/script.cpp | 4 ++-- src/script/script.h | 2 +- src/script/standard.cpp | 2 +- src/test/script_P2CS_tests.cpp | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 847f9f55cc808..557e3efef6502 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -960,7 +960,7 @@ bool EvalScript(std::vector >& stack, const CScript& } break; - case OP_CHECKCOLDSTAKEVERIFY: + case OP_CHECKCOLDSTAKEVERIFY_LOF: { if (g_IsV6Active) { // the stack can contain only at this point diff --git a/src/script/script.cpp b/src/script/script.cpp index 06cd172558aa6..a1a847533a608 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -148,7 +148,7 @@ const char* GetOpName(opcodetype opcode) case OP_ZEROCOINPUBLICSPEND : return "OP_ZEROCOINPUBLICSPEND"; // cold staking - case OP_CHECKCOLDSTAKEVERIFY : return "OP_CHECKCOLDSTAKEVERIFY"; + case OP_CHECKCOLDSTAKEVERIFY_LOF : return "OP_CHECKCOLDSTAKEVERIFY_LOF"; case OP_INVALIDOPCODE : return "OP_INVALIDOPCODE"; @@ -236,7 +236,7 @@ bool CScript::IsPayToColdStaking() const (!g_IsV6Active || (*this)[1] == OP_HASH160) && (*this)[2] == OP_ROT && (!g_IsV6Active || (*this)[3] == OP_IF) && - (*this)[4] == OP_CHECKCOLDSTAKEVERIFY && + (*this)[4] == OP_CHECKCOLDSTAKEVERIFY_LOF && (*this)[5] == 0x14 && (!g_IsV6Active || (*this)[26] == OP_ELSE) && (*this)[27] == 0x14 && diff --git a/src/script/script.h b/src/script/script.h index aa5a4d3aa3137..3b80d50554370 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -184,7 +184,7 @@ enum opcodetype OP_ZEROCOINPUBLICSPEND = 0xc3, // cold staking - OP_CHECKCOLDSTAKEVERIFY = 0xd1, + OP_CHECKCOLDSTAKEVERIFY_LOF = 0xd1, // last output free for masternode/budget payments OP_INVALIDOPCODE = 0xff, }; diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 266d9d30b4c95..d390818ccbe8b 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -299,7 +299,7 @@ CScript GetScriptForStakeDelegation(const CKeyID& stakingKey, const CKeyID& spen { CScript script; script << OP_DUP << OP_HASH160 << OP_ROT << - OP_IF << OP_CHECKCOLDSTAKEVERIFY << ToByteVector(stakingKey) << + OP_IF << OP_CHECKCOLDSTAKEVERIFY_LOF << ToByteVector(stakingKey) << OP_ELSE << ToByteVector(spendingKey) << OP_ENDIF << OP_EQUALVERIFY << OP_CHECKSIG; return script; diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index 8309c37f3668d..a8813f3519825 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -206,7 +206,7 @@ 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_IF << OP_CHECKCOLDSTAKEVERIFY_LOF << ToByteVector(staker) << OP_ELSE << ToByteVector(owner) << OP_DROP << OP_EQUALVERIFY << OP_CHECKSIG; From df116317ecb5afe8b754207c269cf9f5c0005d80 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 1 Mar 2021 18:41:45 +0100 Subject: [PATCH 2/6] [Script] Introduce new OP_CHECKCOLDSTAKEVERIFY opcode without free outputs --- src/script/interpreter.cpp | 75 +++++++++++++---------- src/script/interpreter.h | 4 +- src/script/script.cpp | 9 ++- src/script/script.h | 2 + src/script/standard.cpp | 10 +++ src/script/standard.h | 1 + src/test/script_P2CS_tests.cpp | 18 +++--- src/validation.cpp | 20 ++++-- test/functional/mining_pos_coldStaking.py | 4 +- 9 files changed, 90 insertions(+), 53 deletions(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 557e3efef6502..837bb84cdd9ac 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -960,30 +960,16 @@ bool EvalScript(std::vector >& stack, const CScript& } break; + case OP_CHECKCOLDSTAKEVERIFY: + { + return checker.CheckColdStake(false, script, stack, flags, serror); + } + break; + case OP_CHECKCOLDSTAKEVERIFY_LOF: { - if (g_IsV6Active) { - // 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); - } + // Allow last output script "free" + return checker.CheckColdStake(true, script, stack, flags, serror); } break; @@ -1357,36 +1343,59 @@ bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) con return true; } -bool TransactionSignatureChecker::CheckColdStake(const CScript& prevoutScript) const +bool TransactionSignatureChecker::CheckColdStake(bool fAllowLastOutputFree, const CScript& prevoutScript, std::vector& stack, unsigned int flags, ScriptError* serror) const { + if (g_IsV6Active) { + // 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); + } + } + + // check it is used in a valid cold stake transaction. // 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(); assert(outs >= 2); - // All outputs, except the first, and (for cold stakes with outs >=3) the last one, - // must have the same pubKeyScript, and it must match the script we are spending. - // If the coinstake has at least 3 outputs, the last one is left free, to be used for - // budget/masternode payments, and is checked in CheckColdstakeFreeOutput(). + // All outputs must have the same pubKeyScript, and it must match the script we are spending. + // If the coinstake has at least 3 outputs, the last one can be left free, to be used for + // budget/masternode payments (before v6.0 enforcement), and is checked in CheckColdstakeFreeOutput(). // Here we verify only that input amount goes to the non-free outputs. CAmount outValue{0}; for (unsigned int i = 1; i < outs; i++) { 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; + // Only the last one can be different (and only when outs >=3 and fAllowLastOutputFree=true) + if (!fAllowLastOutputFree || i != outs-1 || outs < 3) { + 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 3fe927eb99fc4..0224756111bda 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(bool fAllowLastOutputFree, 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(bool fAllowLastOutputFree, const CScript& prevoutScript, std::vector& stack, unsigned int flags, ScriptError* serror) const override; }; class MutableTransactionSignatureChecker : public TransactionSignatureChecker diff --git a/src/script/script.cpp b/src/script/script.cpp index a1a847533a608..1318285830f09 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -149,6 +149,7 @@ const char* GetOpName(opcodetype opcode) // cold staking case OP_CHECKCOLDSTAKEVERIFY_LOF : return "OP_CHECKCOLDSTAKEVERIFY_LOF"; + case OP_CHECKCOLDSTAKEVERIFY : return "OP_CHECKCOLDSTAKEVERIFY"; case OP_INVALIDOPCODE : return "OP_INVALIDOPCODE"; @@ -229,6 +230,7 @@ bool CScript::IsPayToScriptHash() const // can be removed once v6 enforcement is activated. std::atomic g_IsV6Active{false}; +// P2CS script: either with or without last output free bool CScript::IsPayToColdStaking() const { return (this->size() == 51 && @@ -236,7 +238,7 @@ bool CScript::IsPayToColdStaking() const (!g_IsV6Active || (*this)[1] == OP_HASH160) && (*this)[2] == OP_ROT && (!g_IsV6Active || (*this)[3] == OP_IF) && - (*this)[4] == OP_CHECKCOLDSTAKEVERIFY_LOF && + ((*this)[4] == OP_CHECKCOLDSTAKEVERIFY || (*this)[4] == OP_CHECKCOLDSTAKEVERIFY_LOF) && (*this)[5] == 0x14 && (!g_IsV6Active || (*this)[26] == OP_ELSE) && (*this)[27] == 0x14 && @@ -245,6 +247,11 @@ bool CScript::IsPayToColdStaking() const (*this)[50] == OP_CHECKSIG); } +bool CScript::IsPayToColdStakingLOF() const +{ + return IsPayToColdStaking() && (*this)[4] == OP_CHECKCOLDSTAKEVERIFY_LOF; +} + bool CScript::StartsWithOpcode(const opcodetype opcode) const { return (!this->empty() && (*this)[0] == opcode); diff --git a/src/script/script.h b/src/script/script.h index 3b80d50554370..87406c57f687d 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -185,6 +185,7 @@ enum opcodetype // cold staking OP_CHECKCOLDSTAKEVERIFY_LOF = 0xd1, // last output free for masternode/budget payments + OP_CHECKCOLDSTAKEVERIFY = 0xd2, OP_INVALIDOPCODE = 0xff, }; @@ -632,6 +633,7 @@ class CScript : public CScriptBase bool IsPayToPublicKeyHash() const; bool IsPayToScriptHash() const; bool IsPayToColdStaking() const; + bool IsPayToColdStakingLOF() const; bool StartsWithOpcode(const opcodetype opcode) const; bool IsZerocoinMint() const; bool IsZerocoinSpend() const; diff --git a/src/script/standard.cpp b/src/script/standard.cpp index d390818ccbe8b..a2c5bd2382bd0 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -296,6 +296,16 @@ CScript GetScriptForRawPubKey(const CPubKey& pubKey) } CScript GetScriptForStakeDelegation(const CKeyID& stakingKey, const CKeyID& spendingKey) +{ + CScript script; + script << OP_DUP << OP_HASH160 << OP_ROT << + OP_IF << OP_CHECKCOLDSTAKEVERIFY << ToByteVector(stakingKey) << + OP_ELSE << ToByteVector(spendingKey) << OP_ENDIF << + OP_EQUALVERIFY << OP_CHECKSIG; + return script; +} + +CScript GetScriptForStakeDelegationLOF(const CKeyID& stakingKey, const CKeyID& spendingKey) { CScript script; script << OP_DUP << OP_HASH160 << OP_ROT << diff --git a/src/script/standard.h b/src/script/standard.h index 307c7cc9b964e..dd738dc79b4b1 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -84,6 +84,7 @@ CScript GetScriptForDestination(const CTxDestination& dest); CScript GetScriptForRawPubKey(const CPubKey& pubKey); CScript GetScriptForMultisig(int nRequired, const std::vector& keys); CScript GetScriptForStakeDelegation(const CKeyID& stakingKey, const CKeyID& spendingKey); +CScript GetScriptForStakeDelegationLOF(const CKeyID& stakingKey, const CKeyID& spendingKey); CScript GetScriptForOpReturn(const uint256& message); #endif // BITCOIN_SCRIPT_STANDARD_H diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index a8813f3519825..b42545e7a5ac4 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -57,12 +57,12 @@ BOOST_AUTO_TEST_CASE(extract_cold_staking_destination_keys) CheckValidKeyId(destVector[1], ownerId); } -static CScript GetNewP2CS(CKey& stakerKey, CKey& ownerKey) +static CScript GetNewP2CS(CKey& stakerKey, CKey& ownerKey, bool fLastOutFree) { stakerKey = DecodeSecret("91yo52JPHDVUG3jXWLKGyzEdjn1a9nbnurLdmQEf2UzbgzkTc2c"); ownerKey = DecodeSecret("92KgNFNfmVVJRQuzssETc7NhwufGuHsLvPQxW9Nwmxs7PB4ByWB"); - return GetScriptForStakeDelegation(stakerKey.GetPubKey().GetID(), - ownerKey.GetPubKey().GetID()); + return fLastOutFree ? GetScriptForStakeDelegationLOF(stakerKey.GetPubKey().GetID(), ownerKey.GetPubKey().GetID()) + : GetScriptForStakeDelegation(stakerKey.GetPubKey().GetID(), ownerKey.GetPubKey().GetID()); } static CScript GetDummyP2CS(const CKeyID& dummyKeyID) @@ -78,9 +78,9 @@ static CScript GetDummyP2PKH(const CKeyID& dummyKeyID) static const CAmount amtIn = 200 * COIN; static const unsigned int flags = STANDARD_SCRIPT_VERIFY_FLAGS; -static CMutableTransaction CreateNewColdStakeTx(CScript& scriptP2CS, CKey& stakerKey, CKey& ownerKey) +static CMutableTransaction CreateNewColdStakeTx(CScript& scriptP2CS, CKey& stakerKey, CKey& ownerKey, bool fLastOutFree) { - scriptP2CS = GetNewP2CS(stakerKey, ownerKey); + scriptP2CS = GetNewP2CS(stakerKey, ownerKey, fLastOutFree); // Create prev transaction: CMutableTransaction txFrom; @@ -122,14 +122,14 @@ static bool CheckP2CSScript(const CScript& scriptSig, const CScript& scriptPubKe return VerifyScript(scriptSig, scriptPubKey, flags, MutableTransactionSignatureChecker(&tx, 0, amtIn), tx.GetRequiredSigVersion(), &err); } -BOOST_AUTO_TEST_CASE(coldstake_script) +BOOST_AUTO_TEST_CASE(coldstake_lof_script) { SelectParams(CBaseChainParams::REGTEST); CScript scriptP2CS; CKey stakerKey, ownerKey; // create unsigned coinstake transaction - CMutableTransaction good_tx = CreateNewColdStakeTx(scriptP2CS, stakerKey, ownerKey); + CMutableTransaction good_tx = CreateNewColdStakeTx(scriptP2CS, stakerKey, ownerKey, true); // sign the input with the staker key SignColdStake(good_tx, 0, scriptP2CS, stakerKey, true); @@ -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)); diff --git a/src/validation.cpp b/src/validation.cpp index 9028ce9f2d407..db625a30b883d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2705,19 +2705,27 @@ bool FindUndoPos(CValidationState& state, int nFile, CDiskBlockPos& pos, unsigne bool CheckColdStakeFreeOutput(const CTransaction& tx, const int nHeight) { - if (!tx.HasP2CSOutputs()) + assert(tx.IsCoinStake()); + // This check applies only to coinstakes spending a P2CS_LOF script. + // The script-check ensures that all but the first and the last output + // (if the coinstake has more than 3 outputs) have the same scriptPubKey. + // If the second script is not a P2CS_LOF, then either this is a "regular" + // P2PKH stake, or it fails the script verification. + if (!tx.vout[1].scriptPubKey.IsPayToColdStakingLOF()) { return true; - + } + // If the last output is different, then it can be either a masternode + // or a budget proposal payment const unsigned int outs = tx.vout.size(); const CTxOut& lastOut = tx.vout[outs-1]; if (outs >=3 && lastOut.scriptPubKey != tx.vout[outs-2].scriptPubKey) { + if (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V6_0)) { + // after v6.0, masternode and budgets are paid in the coinbase. No more free outputs allowed. + return false; + } if (lastOut.nValue == GetMasternodePayment()) return true; - // This could be a budget block. - if (Params().IsRegTestNet()) - return false; - // if mnsync is incomplete, we cannot verify if this is a budget block. // so we check that the staker is not transferring value to the free output if (!masternodeSync.IsSynced()) { diff --git a/test/functional/mining_pos_coldStaking.py b/test/functional/mining_pos_coldStaking.py index bdd182702be49..fdb93a65daa6b 100755 --- a/test/functional/mining_pos_coldStaking.py +++ b/test/functional/mining_pos_coldStaking.py @@ -25,7 +25,7 @@ # filter utxos based on first 5 bytes of scriptPubKey def getDelegatedUtxos(utxos): - return [x for x in utxos if x["scriptPubKey"][:10] == '76a97b63d1'] + return [x for x in utxos if x["scriptPubKey"][:10] == '76a97b63d1' or x["scriptPubKey"][:10] == '76a97b63d2'] class PIVX_ColdStakingTest(PivxTestFramework): @@ -340,7 +340,7 @@ def run_test(self): # Try to submit the block ret = self.nodes[1].submitblock(bytes_to_hex_str(new_block.serialize())) self.log.info("Block %s submitted." % new_block.hash) - assert_equal(ret, "bad-p2cs-outs") + assert ret in ["bad-p2cs-outs", "rejected"] # Verify that nodes[0] rejects it self.sync_blocks() From 8eefab50cb90d01f81e6133315680c4bddd8a787 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 1 Mar 2021 19:18:06 +0100 Subject: [PATCH 3/6] [Tests] Add script test for new opcode --- src/test/script_P2CS_tests.cpp | 75 +++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index b42545e7a5ac4..45889f4166ea1 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -199,6 +199,79 @@ BOOST_AUTO_TEST_CASE(coldstake_lof_script) BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); } +BOOST_AUTO_TEST_CASE(coldstake_script) +{ + SelectParams(CBaseChainParams::REGTEST); + CScript scriptP2CS; + CKey stakerKey, ownerKey; + + // create unsigned coinstake transaction + CMutableTransaction good_tx = CreateNewColdStakeTx(scriptP2CS, stakerKey, ownerKey, false); + + // sign the input with the staker key + SignColdStake(good_tx, 0, scriptP2CS, stakerKey, true); + + // check the signature and script + ScriptError err = SCRIPT_ERR_OK; + CMutableTransaction tx(good_tx); + BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); + + // pay less than expected + tx.vout[1].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)); + + // Add another p2cs out + tx.vout.emplace_back(3 * COIN, scriptP2CS); + SignColdStake(tx, 0, scriptP2CS, stakerKey, true); + BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); + + const CKey& dummyKey = DecodeSecret("91t7cwPGevo885Uccg87nVjzUxKhXta9JprHM3R21PQkBFMFg2i"); + const CKeyID& dummyKeyID = dummyKey.GetPubKey().GetID(); + const CScript& dummyP2PKH = GetDummyP2PKH(dummyKeyID); + + // Add a dummy P2PKH out at the end + tx.vout.emplace_back(3 * COIN, dummyP2PKH); + 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)); + // -- but the owner can + SignColdStake(tx, 0, scriptP2CS, ownerKey, false); + BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); + + // Add a dummy P2PKH out at the beginning + tx = good_tx; + tx.vout[1] = CTxOut(3 * COIN, dummyP2PKH); + tx.vout.emplace_back(3 * COIN, scriptP2CS); + 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)); + // -- but the owner can + SignColdStake(tx, 0, scriptP2CS, ownerKey, false); + BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); + + // Replace with new p2cs + tx = good_tx; + tx.vout[1].scriptPubKey = GetDummyP2CS(dummyKeyID); + 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)); + // -- but the owner can + SignColdStake(tx, 0, scriptP2CS, ownerKey, false); + BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); + + // Replace with single dummy out + tx = good_tx; + tx.vout[1] = CTxOut(COIN, dummyP2PKH); + 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)); + // -- but the owner can + SignColdStake(tx, 0, scriptP2CS, ownerKey, false); + 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: @@ -206,7 +279,7 @@ static CScript GetFakeLockingScript(const CKeyID staker, const CKeyID& owner) { CScript script; script << opcodetype(0x2F) << opcodetype(0x01) << OP_ROT << - OP_IF << OP_CHECKCOLDSTAKEVERIFY_LOF << ToByteVector(staker) << + OP_IF << OP_CHECKCOLDSTAKEVERIFY << ToByteVector(staker) << OP_ELSE << ToByteVector(owner) << OP_DROP << OP_EQUALVERIFY << OP_CHECKSIG; From 45ebfee6bb0c0fb7f721e7c6b7c7e86e7f5ca6e5 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 1 Mar 2021 21:31:07 +0100 Subject: [PATCH 4/6] [RPC] Use new opcode for P2CS after v6 enforcement --- src/sapling/sapling_operation.h | 5 +++-- src/wallet/rpcwallet.cpp | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/sapling/sapling_operation.h b/src/sapling/sapling_operation.h index 2df64a1c8abc5..38ff8aa978772 100644 --- a/src/sapling/sapling_operation.h +++ b/src/sapling/sapling_operation.h @@ -51,8 +51,9 @@ struct SendManyRecipient {} // Transparent recipient: P2CS - SendManyRecipient(const CKeyID& ownerKey, const CKeyID& stakerKey, const CAmount& amount): - transparentRecipient(CTxOut(amount, GetScriptForStakeDelegation(stakerKey, ownerKey))) + SendManyRecipient(const CKeyID& ownerKey, const CKeyID& stakerKey, const CAmount& amount, bool fV6Enforced): + transparentRecipient(CTxOut(amount, fV6Enforced ? GetScriptForStakeDelegation(stakerKey, ownerKey) + : GetScriptForStakeDelegationLOF(stakerKey, ownerKey))) {} // Transparent recipient: multisig diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 12237803e83b0..e75e86d209390 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1147,12 +1147,16 @@ UniValue CreateColdStakeDelegation(const UniValue& params, CTransactionRef& txNe ownerAddressStr = EncodeDestination(ownerAddr); } + // Use new opcode after v6.0 enforcement (!TODO: remove after enforcement) + bool fV6Enforced = Params().GetConsensus().NetworkUpgradeActive(chainActive.Height(), Consensus::UPGRADE_V6_0); + // Create the transaction const bool fUseShielded = (params.size() > 5) && params[5].get_bool(); if (!fUseShielded) { // Delegate transparent coins CAmount nFeeRequired; - CScript scriptPubKey = GetScriptForStakeDelegation(*stakeKey, ownerKey); + CScript scriptPubKey = fV6Enforced ? GetScriptForStakeDelegation(*stakeKey, ownerKey) + : GetScriptForStakeDelegationLOF(*stakeKey, ownerKey); if (!pwalletMain->CreateTransaction(scriptPubKey, nValue, txNew, reservekey, nFeeRequired, strError, nullptr, ALL_COINS, (CAmount)0, fUseDelegated)) { if (nValue + nFeeRequired > currBalance) strError = strprintf("Error: This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds!", FormatMoney(nFeeRequired)); @@ -1167,7 +1171,7 @@ UniValue CreateColdStakeDelegation(const UniValue& params, CTransactionRef& txNe if (!consensus.NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_V5_0)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling not active yet"); } - std::vector recipients = {SendManyRecipient(ownerKey, *stakeKey, nValue)}; + std::vector recipients = {SendManyRecipient(ownerKey, *stakeKey, nValue, fV6Enforced)}; SaplingOperation operation(consensus, nextBlockHeight, pwalletMain); OperationResult res = operation.setSelectShieldedCoins(true) ->setRecipients(recipients) From c19416fbfd5c1bcf70eee83cea7ec5bac7b65fbf Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 1 Mar 2021 21:31:22 +0100 Subject: [PATCH 5/6] [GUI] Use new opcode for P2CS after v6 enforcement --- src/qt/walletmodel.cpp | 8 +++++++- src/qt/walletmodel.h | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index bbd34d7bb9846..9b8f0028eb3dc 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -90,6 +90,11 @@ bool WalletModel::isSaplingEnforced() const return Params().GetConsensus().NetworkUpgradeActive(cachedNumBlocks, Consensus::UPGRADE_V5_0); } +bool WalletModel::isV6Enforced() const +{ + return Params().GetConsensus().NetworkUpgradeActive(cachedNumBlocks, Consensus::UPGRADE_V6_0); +} + bool WalletModel::isStakingStatusActive() const { return wallet && wallet->pStakerStatus && wallet->pStakerStatus->IsActive(); @@ -480,7 +485,8 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact return InvalidAddress; } - scriptPubKey = GetScriptForStakeDelegation(*stakerId, *ownerId); + scriptPubKey = isV6Enforced() ? GetScriptForStakeDelegation(*stakerId, *ownerId) + : GetScriptForStakeDelegationLOF(*stakerId, *ownerId); } else { // Regular P2PK or P2PKH scriptPubKey = GetScriptForDestination(out); diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 4306a5ed2a262..f7808e111edf4 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -156,6 +156,7 @@ class WalletModel : public QObject bool isColdStakingNetworkelyEnabled() const; bool isSaplingInMaintenance() const; bool isSaplingEnforced() const; + bool isV6Enforced() const; CAmount getMinColdStakingAmount() const; /* current staking status from the miner thread **/ bool isStakingStatusActive() const; From 4a5baf3b7a0829f1bc91635ac8061ddb7441228e Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 8 May 2021 15:27:34 +0200 Subject: [PATCH 6/6] [Doc] Add cold-staking changes to release notes --- doc/release-notes.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index e60eab687be9f..cac3a40e0ea74 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -44,6 +44,19 @@ Notable Changes (Developers: add your notes here as part of your pull requests whenever possible) +Cold-Staking Re-Activation +-------------------------- +PIVX Core v6.0.0 includes a fix for the vulnerability identified within the cold-staking protocol (see PR [#2258](https://github.com/PIVX-Project/PIVX/pull/2258)). +Therefore the feature will be re-enabled on the network, via `SPORK_19`, shortly after the upgrade enforcement. + +#### Protocol changes + +A new opcode (`0xd2`) is introduced (see PR [#2275](https://github.com/PIVX-Project/PIVX/pull/2275)). It enforces the same rules as the legacy cold-staking opcode, but without allowing a "free" script for the last output of the transaction. +This is in accord with the consensus change introduced with the "Deterministic Masternodes" update, as masternode/budget payments are now outputs of the *coinbase* transaction (rather than the *coinstake*), therefore a "free" output for the coinstake is no longer needed. +The new opcode takes the name of `OP_CHECKCOLDSTAKEVERIFY`, and the legacy opcode (`0xd1`) is renamed to `OP_CHECKCOLDSTAKEVERIFY_LOF` (last-output-free). +Scripts with the old opcode are still accepted on the network (the restriction on the last-output is enforced after the script validation in this case), but the client creates new delegations with the new opcode, by default, after the upgrade enforcement. + + GUI changes -----------