diff --git a/src/blocksignature.cpp b/src/blocksignature.cpp index e42342a9acc3..3eaf6db71b5a 100644 --- a/src/blocksignature.cpp +++ b/src/blocksignature.cpp @@ -39,7 +39,7 @@ bool SignBlock(CBlock& block, const CKeyStore& keystore) return SignBlockWithKey(block, key); } -bool CheckBlockSignature(const CBlock& block) +bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH) { if (block.IsProofOfWork()) return block.vchBlockSig.empty(); @@ -62,9 +62,29 @@ bool CheckBlockSignature(const CBlock& block) const CTxOut& txout = block.vtx[1].vout[1]; if (!Solver(txout.scriptPubKey, whichType, vSolutions)) return false; - if (whichType == TX_PUBKEY || whichType == TX_PUBKEYHASH) { + + if (!enableP2PKH) { + // Before v5 activation, P2PKH was always failing. + if (whichType == TX_PUBKEYHASH) { + return false; + } + } + + if (whichType == TX_PUBKEY) { valtype& vchPubKey = vSolutions[0]; pubkey = CPubKey(vchPubKey); + } else if (whichType == TX_PUBKEYHASH) { + const CTxIn& txin = block.vtx[1].vin[0]; + // Check if the scriptSig is for a p2pk or a p2pkh + if (txin.scriptSig.size() == 73) { // Sig size + DER signature size. + // If the input is for a p2pk and the output is a p2pkh. + // We don't have the pubkey to verify the block sig anywhere in this block. + // p2pk scriptsig only contains the signature and p2pkh scriptpubkey only contain the hash. + return false; + } else { + int start = 1 + (int) *txin.scriptSig.begin(); // skip sig + pubkey = CPubKey(txin.scriptSig.begin()+start+1, txin.scriptSig.end()); + } } else if (whichType == TX_COLDSTAKE) { // pick the public key from the P2CS input const CTxIn& txin = block.vtx[1].vin[0]; diff --git a/src/blocksignature.h b/src/blocksignature.h index ad399b85dbc0..feed6af20d5a 100644 --- a/src/blocksignature.h +++ b/src/blocksignature.h @@ -11,6 +11,6 @@ bool SignBlockWithKey(CBlock& block, const CKey& key); bool SignBlock(CBlock& block, const CKeyStore& keystore); -bool CheckBlockSignature(const CBlock& block); +bool CheckBlockSignature(const CBlock& block, const bool enableP2PKH); #endif //PIVX_BLOCKSIGNATURE_H diff --git a/src/main.cpp b/src/main.cpp index b549fd76af39..634736e4d558 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4072,14 +4072,19 @@ bool ProcessNewBlock(CValidationState& state, CNode* pfrom, const CBlock* pblock // Preliminary checks int64_t nStartTime = GetTimeMillis(); + const Consensus::Params& consensus = Params().GetConsensus(); // check block bool checked = CheckBlock(*pblock, state); - if (!CheckBlockSignature(*pblock)) + // For now, we need the tip to know whether p2pkh block signatures are accepted or not. + // After 5.0, this can be removed and replaced by the enforcement block time. + const CBlockIndex* pindexPrev = chainActive.Tip(); + const bool enableP2PKH = (pindexPrev) ? consensus.NetworkUpgradeActive(pindexPrev->nHeight + 1, Consensus::UPGRADE_V5_DUMMY) : false; + if (!CheckBlockSignature(*pblock, enableP2PKH)) return error("%s : bad proof-of-stake block signature", __func__); - if (pblock->GetHash() != Params().GetConsensus().hashGenesisBlock && pfrom != NULL) { + if (pblock->GetHash() != consensus.hashGenesisBlock && pfrom != NULL) { //if we get this far, check if the prev block is our prev block, if not then request sync and return false BlockMap::iterator mi = mapBlockIndex.find(pblock->hashPrevBlock); if (mi == mapBlockIndex.end()) { diff --git a/src/miner.cpp b/src/miner.cpp index 48e24f770fa4..c3fd8fddc449 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -501,7 +501,7 @@ CBlockTemplate* CreateNewBlockWithKey(CReserveKey& reservekey, CWallet* pwallet) return nullptr; } - CScript scriptPubKey = CScript() << ToByteVector(pubkey) << OP_CHECKSIG; + CScript scriptPubKey = GetScriptForDestination(pubkey.GetID()); return CreateNewBlock(scriptPubKey, pwallet, false); } diff --git a/src/stakeinput.cpp b/src/stakeinput.cpp index 1f6ea2d0fca4..e628ee11bc8f 100644 --- a/src/stakeinput.cpp +++ b/src/stakeinput.cpp @@ -69,7 +69,7 @@ CAmount CPivStake::GetValue() const return txFrom.vout[nPosition].nValue; } -bool CPivStake::CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal) +bool CPivStake::CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal, const bool onlyP2PK) { std::vector vSolutions; txnouttype whichType; @@ -82,19 +82,18 @@ bool CPivStake::CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmoun CScript scriptPubKey; CKey key; - if (whichType == TX_PUBKEYHASH) { - // if P2PKH check that we have the input private key + if (whichType == TX_PUBKEYHASH || whichType == TX_COLDSTAKE) { + // if P2PKH or P2CS check that we have the input private key if (!pwallet->GetKey(CKeyID(uint160(vSolutions[0])), key)) return error("%s: Unable to get staking private key", __func__); + } + // Consensus check: P2PKH block signatures were not accepted before v5 update. + // This can be removed after v5.0 enforcement + if (whichType == TX_PUBKEYHASH && onlyP2PK) { // convert to P2PK inputs scriptPubKey << key.GetPubKey() << OP_CHECKSIG; - } else { - // if P2CS, check that we have the coldstaking private key - if ( whichType == TX_COLDSTAKE && !pwallet->GetKey(CKeyID(uint160(vSolutions[0])), key) ) - return error("%s: Unable to get cold staking private key", __func__); - // keep the same script scriptPubKey = scriptPubKeyKernel; } diff --git a/src/stakeinput.h b/src/stakeinput.h index e488a54f67c9..c1519c38e7bf 100644 --- a/src/stakeinput.h +++ b/src/stakeinput.h @@ -26,7 +26,7 @@ class CStakeInput virtual bool GetTxFrom(CTransaction& tx) const = 0; virtual bool GetTxOutFrom(CTxOut& out) const = 0; virtual CAmount GetValue() const = 0; - virtual bool CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal) = 0; + virtual bool CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal, const bool onlyP2PK) = 0; virtual bool IsZPIV() const = 0; virtual CDataStream GetUniqueness() const = 0; virtual bool ContextCheck(int nHeight, uint32_t nTime) = 0; @@ -51,7 +51,7 @@ class CPivStake : public CStakeInput CAmount GetValue() const override; CDataStream GetUniqueness() const override; bool CreateTxIn(CWallet* pwallet, CTxIn& txIn, uint256 hashTxOut = UINT256_ZERO) override; - bool CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal) override; + bool CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal, const bool onlyP2PK) override; bool IsZPIV() const override { return false; } bool ContextCheck(int nHeight, uint32_t nTime) override; }; diff --git a/src/test/main_tests.cpp b/src/test/main_tests.cpp index 029a23cc2790..343caa848cf4 100644 --- a/src/test/main_tests.cpp +++ b/src/test/main_tests.cpp @@ -4,14 +4,113 @@ // Distributed under the MIT/X11 software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "primitives/transaction.h" +#include "blocksignature.h" #include "main.h" +#include "primitives/transaction.h" +#include "script/sign.h" #include "test_pivx.h" - #include BOOST_FIXTURE_TEST_SUITE(main_tests, TestingSetup) +enum BlockSignatureType{ + P2PK, + P2PKH, + P2CS +}; + +CScript GetScriptForType(CPubKey pubKey, BlockSignatureType type) +{ + switch(type){ + case P2PK: + return CScript() << pubKey << OP_CHECKSIG; + default: + return GetScriptForDestination(pubKey.GetID()); + } +} + +std::vector CreateDummyScriptSigWithKey(CPubKey pubKey) +{ + std::vector vchSig; + const CScript scriptCode; + DummySignatureCreator(nullptr).CreateSig(vchSig, pubKey.GetID(), scriptCode); + return vchSig; +} + +CScript GetDummyScriptSigByType(CPubKey pubKey, bool isP2PK) +{ + CScript script = CScript() << CreateDummyScriptSigWithKey(pubKey); + if (!isP2PK) + script << ToByteVector(pubKey); + return script; +} + +CBlock CreateDummyBlockWithSignature(CKey stakingKey, BlockSignatureType type, bool useInputP2PK) +{ + CMutableTransaction txCoinStake; + // Dummy input + CTxIn input(uint256(), 0); + // P2PKH input + input.scriptSig = GetDummyScriptSigByType(stakingKey.GetPubKey(), useInputP2PK); + // Add dummy input + txCoinStake.vin.emplace_back(input); + // Empty first output + txCoinStake.vout.emplace_back(CTxOut(0, CScript())); + // P2PK staking output + CScript scriptPubKey = GetScriptForType(stakingKey.GetPubKey(), type); + txCoinStake.vout.emplace_back(CTxOut(0, scriptPubKey)); + + // Now the block. + CBlock block; + block.vtx.emplace_back(CTransaction()); // dummy first tx + block.vtx.emplace_back(txCoinStake); + SignBlockWithKey(block, stakingKey); + + return block; +} + +bool TestBlockSignaturePreEnforcementV5(const CBlock& block) +{ + return CheckBlockSignature(block, false); +} + +bool TestBlockSignaturePostEnforcementV5(const CBlock& block) +{ + return CheckBlockSignature(block, true); +} + +BOOST_AUTO_TEST_CASE(block_signature_test) +{ + for (int i = 0; i < 20; ++i) { + CKey stakingKey; + stakingKey.MakeNewKey(true); + bool useInputP2PK = i % 2 == 0; + + // Test P2PK block signature pre enforcement. + CBlock block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PK, useInputP2PK); + BOOST_CHECK(TestBlockSignaturePreEnforcementV5(block)); + + // Test P2PK block signature post enforcement + block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PK, useInputP2PK); + BOOST_CHECK(TestBlockSignaturePostEnforcementV5(block)); + + // Test P2PKH block signature pre enforcement ---> must fail. + block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PKH, useInputP2PK); + BOOST_CHECK(!TestBlockSignaturePreEnforcementV5(block)); + + // Test P2PKH block signature post enforcement + block = CreateDummyBlockWithSignature(stakingKey, BlockSignatureType::P2PKH, useInputP2PK); + if (useInputP2PK) { + // If it's using a P2PK scriptsig as input and a P2PKH output + // The block doesn't contain the public key to verify the sig anywhere. + // Must fail. + BOOST_CHECK(!TestBlockSignaturePostEnforcementV5(block)); + } else { + BOOST_CHECK(TestBlockSignaturePostEnforcementV5(block)); + } + } +} + CAmount nMoneySupplyPoWEnd = 43199500 * COIN; BOOST_AUTO_TEST_CASE(subsidy_limit_test) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 981721758332..b121143f74b8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -761,6 +761,21 @@ int64_t CWallet::IncOrderPosNext(CWalletDB* pwalletdb) return nRet; } +bool CWallet::IsKeyUsed(const CPubKey& vchPubKey) { + if (vchPubKey.IsValid()) { + CScript scriptPubKey = GetScriptForDestination(vchPubKey.GetID()); + for (std::map::iterator it = mapWallet.begin(); + it != mapWallet.end() && vchPubKey.IsValid(); + ++it) { + const CWalletTx& wtx = (*it).second; + for (const CTxOut& txout : wtx.vout) + if (txout.scriptPubKey == scriptPubKey) + return true; + } + } + return false; +} + bool CWallet::GetLabelDestination(CTxDestination& dest, const std::string& label, bool bForceNew) { CWalletDB walletdb(strWalletFile); @@ -768,29 +783,17 @@ bool CWallet::GetLabelDestination(CTxDestination& dest, const std::string& label walletdb.ReadAccount(label, account); if (!bForceNew) { - if (!account.vchPubKey.IsValid()) - bForceNew = true; - else { - // Check if the current key has been used - CScript scriptPubKey = GetScriptForDestination(account.vchPubKey.GetID()); - for (std::map::iterator it = mapWallet.begin(); - it != mapWallet.end() && account.vchPubKey.IsValid(); - ++it) { - const CWalletTx& wtx = (*it).second; - for (const CTxOut& txout : wtx.vout) { - if (txout.scriptPubKey == scriptPubKey) { - bForceNew = true; - break; - } - } - } - } + // Check if the key is invalid or has been used + bForceNew = !account.vchPubKey.IsValid() || IsKeyUsed(account.vchPubKey); } // Generate a new key if (bForceNew) { - if (!GetKeyFromPool(account.vchPubKey)) - return false; + // double check for used keys. todo: research why we have sometimes used keys in the keypool.. + do { + if (!GetKeyFromPool(account.vchPubKey)) + return false; + } while (IsKeyUsed(account.vchPubKey)); dest = account.vchPubKey.GetID(); SetAddressBook(dest, label, AddressBook::AddressBookPurpose::RECEIVE); @@ -2786,6 +2789,9 @@ bool CWallet::CreateCoinStake( pStakerStatus->SetLastTip(pindexPrev); pStakerStatus->SetLastCoins(listInputs.size()); + // P2PKH block signatures were not accepted before v5 update. + bool onlyP2PK = !consensus.NetworkUpgradeActive(pindexPrev->nHeight + 1, Consensus::UPGRADE_V5_DUMMY); + // Kernel Search CAmount nCredit; CScript scriptPubKeyKernel; @@ -2826,7 +2832,7 @@ bool CWallet::CreateCoinStake( // Create the output transaction(s) std::vector vout; - if (!stakeInput->CreateTxOuts(this, vout, nCredit)) { + if (!stakeInput->CreateTxOuts(this, vout, nCredit, onlyP2PK)) { LogPrintf("%s : failed to create output\n", __func__); continue; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cc135c86e515..06f38ad934df 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -253,6 +253,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void SyncMetaData(std::pair); + bool IsKeyUsed(const CPubKey& vchPubKey); + public: static const CAmount DEFAULT_STAKE_SPLIT_THRESHOLD = 500 * COIN; diff --git a/src/zpiv/zpos.h b/src/zpiv/zpos.h index c83bea737bfb..4d13403ee9f0 100644 --- a/src/zpiv/zpos.h +++ b/src/zpiv/zpos.h @@ -27,7 +27,7 @@ class CLegacyZPivStake : public CStakeInput CAmount GetValue() const override; CDataStream GetUniqueness() const override; bool CreateTxIn(CWallet* pwallet, CTxIn& txIn, uint256 hashTxOut = UINT256_ZERO) override { return false; /* creation disabled */} - bool CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal) override { return false; /* creation disabled */} + bool CreateTxOuts(CWallet* pwallet, std::vector& vout, CAmount nTotal, const bool onlyP2PK) override { return false; /* creation disabled */} bool GetTxFrom(CTransaction& tx) const override { return false; /* not available */ } bool GetTxOutFrom(CTxOut& out) const override { return false; /* not available */ } virtual bool ContextCheck(int nHeight, uint32_t nTime) override; diff --git a/test/functional/mining_v5_upgrade.py b/test/functional/mining_v5_upgrade.py new file mode 100755 index 000000000000..85c7e834fa3a --- /dev/null +++ b/test/functional/mining_v5_upgrade.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python3 + +from test_framework.test_framework import PivxTestFramework +from test_framework.util import * + +""" +Simple test checking chain movement after v5 enforcement. +""" + +class MiningV5UpgradeTest(PivxTestFramework): + + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [[]] + self.setup_clean_chain = True + + def run_test(self): + assert_equal(self.nodes[0].getblockchaininfo()['upgrades']['v5 dummy']['status'], 'pending') + self.nodes[0].generate(300) # v5 activation height + assert_equal(self.nodes[0].getblockchaininfo()['upgrades']['v5 dummy']['status'], 'active') + self.nodes[0].generate(25) # 25 more to check chain movement + assert_equal(self.nodes[0].getblockchaininfo()['upgrades']['v5 dummy']['status'], 'active') + assert_equal(self.nodes[0].getblockcount(), 325) + + +if __name__ == '__main__': + MiningV5UpgradeTest().main() \ No newline at end of file diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 0a79256d7792..2d6bb60ddbfe 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -69,7 +69,7 @@ def _test_gettxoutsetinfo(self): assert_equal(res['transactions'], 200) assert_equal(res['height'], 200) assert_equal(res['txouts'], 200) - assert_equal(res['bytes_serialized'], 14073), + assert_equal(res['bytes_serialized'], 11673), assert_equal(len(res['bestblock']), 64) assert_equal(len(res['hash_serialized']), 64) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 4f516bf53b9a..98ffdb63b4b7 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -107,6 +107,7 @@ 'rpc_decodescript.py', # ~ 50 sec 'rpc_blockchain.py', # ~ 50 sec 'wallet_disable.py', # ~ 50 sec + 'mining_v5_upgrade.py', # ~ 48 sec 'feature_help.py', # ~ 30 sec # Don't append tests at the end to avoid merge conflicts