Move coinbase & coinstake to P2PKH

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];

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

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()) {

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);
 }
 

src/stakeinput.cpp

@@ -69,7 +69,7 @@ CAmount CPivStake::GetValue() const
     return txFrom.vout[nPosition].nValue;
 }
 
-bool CPivStake::CreateTxOuts(CWallet* pwallet, std::vector<CTxOut>& vout, CAmount nTotal)
+bool CPivStake::CreateTxOuts(CWallet* pwallet, std::vector<CTxOut>& vout, CAmount nTotal, const bool onlyP2PK)
 {
     std::vector<valtype> vSolutions;
     txnouttype whichType;
@@ -82,19 +82,18 @@ bool CPivStake::CreateTxOuts(CWallet* pwallet, std::vector<CTxOut>& 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;
     }

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<CTxOut>& vout, CAmount nTotal) = 0;
+    virtual bool CreateTxOuts(CWallet* pwallet, std::vector<CTxOut>& 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<CTxOut>& vout, CAmount nTotal) override;
+    bool CreateTxOuts(CWallet* pwallet, std::vector<CTxOut>& vout, CAmount nTotal, const bool onlyP2PK) override;
     bool IsZPIV() const override { return false; }
     bool ContextCheck(int nHeight, uint32_t nTime) override;
 };

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/test/unit_test.hpp>
 
 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<unsigned char> CreateDummyScriptSigWithKey(CPubKey pubKey)
+{
+    std::vector<unsigned char> 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)

src/wallet/wallet.cpp

@@ -761,36 +761,39 @@ 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<uint256, CWalletTx>::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);
     CAccount account;
     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<uint256, CWalletTx>::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<CTxOut> vout;
-        if (!stakeInput->CreateTxOuts(this, vout, nCredit)) {
+        if (!stakeInput->CreateTxOuts(this, vout, nCredit, onlyP2PK)) {
             LogPrintf("%s : failed to create output\n", __func__);
             continue;
         }

src/wallet/wallet.h

@@ -253,6 +253,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
 
     void SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator>);
 
+    bool IsKeyUsed(const CPubKey& vchPubKey);
+
 public:
 
     static const CAmount DEFAULT_STAKE_SPLIT_THRESHOLD = 500 * COIN;

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<CTxOut>& vout, CAmount nTotal) override { return false; /* creation disabled */}
+    bool CreateTxOuts(CWallet* pwallet, std::vector<CTxOut>& 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;

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()

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)
 

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