Merge pull request #2634 from div72/cleanup-key-io

refactor, misc: remove CBitcoin(Address|Secret)

Author: jamescowens

src/Makefile.test.include

@@ -8,9 +8,9 @@ TEST_SRCDIR = test
 TEST_BINARY=test/test_gridcoin$(EXEEXT)
 
 JSON_TEST_FILES = \
-	test/data/base58_keys_valid.json \
 	test/data/base58_encode_decode.json \
-	test/data/base58_keys_invalid.json \
+	test/data/key_io_invalid.json \
+	test/data/key_io_valid.json \
 	test/data/script_valid.json \
 	test/data/script_invalid.json \
 	test/data/tx_invalid.json \
@@ -59,6 +59,7 @@ GRIDCOIN_TESTS =\
 	test/gridcoin/sidestake_tests.cpp \
 	test/gridcoin/superblock_tests.cpp \
 	test/key_tests.cpp \
+	test/key_io_tests.cpp \
 	test/merkle_tests.cpp \
 	test/mruset_tests.cpp \
 	test/multisig_tests.cpp \

src/base58.h

@@ -60,292 +60,4 @@ std::string EncodeBase58Check(const std::vector<unsigned char>& vchIn);
 [[nodiscard]] bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet,
                                      int max_ret_len = std::numeric_limits<int>::max() - 4);
 
-
-/** Base class for all base58-encoded data */
-class CBase58Data
-{
-protected:
-    // the version byte
-    unsigned char nVersion;
-
-    // the actually encoded data
-    std::vector<unsigned char> vchData;
-
-    CBase58Data()
-    {
-        nVersion = 0;
-        vchData.clear();
-    }
-
-    ~CBase58Data()
-    {
-        // zero the memory, as it may contain sensitive data
-        if (!vchData.empty())
-            memory_cleanse(&vchData[0], vchData.size());
-    }
-
-    void SetData(int nVersionIn, const void* pdata, size_t nSize)
-    {
-        nVersion = nVersionIn;
-        vchData.resize(nSize);
-        if (!vchData.empty())
-            memcpy(&vchData[0], pdata, nSize);
-    }
-
-    void SetData(int nVersionIn, const unsigned char *pbegin, const unsigned char *pend)
-    {
-        SetData(nVersionIn, (void*)pbegin, pend - pbegin);
-    }
-
-public:
-    bool SetString(const char* psz)
-    {
-        std::vector<unsigned char> vchTemp;
-        if (!DecodeBase58Check(psz, vchTemp))
-        {
-            vchData.clear();
-            nVersion = 0;
-            return false;
-        }
-        nVersion = vchTemp[0];
-        vchData.resize(vchTemp.size() - 1);
-        if (!vchData.empty())
-            memcpy(&vchData[0], &vchTemp[1], vchData.size());
-        memory_cleanse(&vchTemp[0], vchTemp.size());
-        return true;
-    }
-
-    bool SetString(const std::string& str)
-    {
-        return SetString(str.c_str());
-    }
-
-    std::string ToString() const
-    {
-        std::vector<unsigned char> vch(1, nVersion);
-        vch.insert(vch.end(), vchData.begin(), vchData.end());
-        return EncodeBase58Check(vch);
-    }
-
-    int CompareTo(const CBase58Data& b58) const
-    {
-        if (nVersion < b58.nVersion) return -1;
-        if (nVersion > b58.nVersion) return  1;
-        if (vchData < b58.vchData)   return -1;
-        if (vchData > b58.vchData)   return  1;
-        return 0;
-    }
-
-    bool operator==(const CBase58Data& b58) const { return CompareTo(b58) == 0; }
-    bool operator<=(const CBase58Data& b58) const { return CompareTo(b58) <= 0; }
-    bool operator>=(const CBase58Data& b58) const { return CompareTo(b58) >= 0; }
-    bool operator< (const CBase58Data& b58) const { return CompareTo(b58) <  0; }
-    bool operator> (const CBase58Data& b58) const { return CompareTo(b58) >  0; }
-};
-
-/** base58-encoded addresses.
- * Public-key-hash-addresses have version 25 (or 111 testnet).
- * The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key.
- * Script-hash-addresses have version 85 (or 196 testnet).
- * The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script.
- */
-class CBitcoinAddress;
-class CBitcoinAddressVisitor
-{
-private:
-    CBitcoinAddress *addr;
-public:
-    CBitcoinAddressVisitor(CBitcoinAddress *addrIn) : addr(addrIn) { }
-    bool operator()(const CKeyID &id) const;
-    bool operator()(const CScriptID &id) const;
-    bool operator()(const CNoDestination &no) const;
-};
-
-class CBitcoinAddress : public CBase58Data
-{
-public:
-    enum
-    {
-		//Base58Gridcoin:
-        PUBKEY_ADDRESS = 62, // Gridcoin Research addresses start with R - 62,  (Classic Starts with G: 37)
-        SCRIPT_ADDRESS = 85,
-        PUBKEY_ADDRESS_TEST = 111,
-        SCRIPT_ADDRESS_TEST = 196,
-    };
-
-    bool Set(const CKeyID &id) {
-        SetData(fTestNet ? PUBKEY_ADDRESS_TEST : PUBKEY_ADDRESS, &id, 20);
-        return true;
-    }
-
-    bool Set(const CScriptID &id) {
-        SetData(fTestNet ? SCRIPT_ADDRESS_TEST : SCRIPT_ADDRESS, &id, 20);
-        return true;
-    }
-
-    bool Set(const CTxDestination &dest)
-    {
-        return std::visit(CBitcoinAddressVisitor(this), dest);
-    }
-
-    bool IsValid() const
-    {
-        unsigned int nExpectedSize = 20;
-        bool fExpectTestNet = false;
-        switch(nVersion)
-        {
-            case PUBKEY_ADDRESS:
-                nExpectedSize = 20; // Hash of public key
-                fExpectTestNet = false;
-                break;
-            case SCRIPT_ADDRESS:
-                nExpectedSize = 20; // Hash of CScript
-                fExpectTestNet = false;
-                break;
-
-            case PUBKEY_ADDRESS_TEST:
-                nExpectedSize = 20;
-                fExpectTestNet = true;
-                break;
-            case SCRIPT_ADDRESS_TEST:
-                nExpectedSize = 20;
-                fExpectTestNet = true;
-                break;
-
-            default:
-                return false;
-        }
-        return fExpectTestNet == fTestNet && vchData.size() == nExpectedSize;
-    }
-
-    CBitcoinAddress()
-    {
-    }
-
-    CBitcoinAddress(const CTxDestination &dest)
-    {
-        Set(dest);
-    }
-
-    CBitcoinAddress(const std::string& strAddress)
-    {
-        SetString(strAddress);
-    }
-
-    CBitcoinAddress(const char* pszAddress)
-    {
-        SetString(pszAddress);
-    }
-
-    CTxDestination Get() const {
-        if (!IsValid())
-            return CNoDestination();
-        switch (nVersion) {
-        case PUBKEY_ADDRESS:
-        case PUBKEY_ADDRESS_TEST: {
-            uint160 id;
-            memcpy(&id, &vchData[0], 20);
-            return CKeyID(id);
-        }
-        case SCRIPT_ADDRESS:
-        case SCRIPT_ADDRESS_TEST: {
-            uint160 id;
-            memcpy(&id, &vchData[0], 20);
-            return CScriptID(id);
-        }
-        }
-        return CNoDestination();
-    }
-
-    bool GetKeyID(CKeyID &keyID) const {
-        if (!IsValid())
-            return false;
-        switch (nVersion) {
-        case PUBKEY_ADDRESS:
-        case PUBKEY_ADDRESS_TEST: {
-            uint160 id;
-            memcpy(&id, &vchData[0], 20);
-            keyID = CKeyID(id);
-            return true;
-        }
-        default: return false;
-        }
-    }
-
-    bool IsScript() const {
-        if (!IsValid())
-            return false;
-        switch (nVersion) {
-        case SCRIPT_ADDRESS:
-        case SCRIPT_ADDRESS_TEST: {
-            return true;
-        }
-        default: return false;
-        }
-    }
-};
-
-bool inline CBitcoinAddressVisitor::operator()(const CKeyID &id) const         { return addr->Set(id); }
-bool inline CBitcoinAddressVisitor::operator()(const CScriptID &id) const      { return addr->Set(id); }
-bool inline CBitcoinAddressVisitor::operator()(const CNoDestination &id) const { return false; }
-
-/** A base58-encoded secret key */
-class CBitcoinSecret : public CBase58Data
-{
-public:
-    void SetSecret(const CSecret& vchSecret, bool fCompressed)
-    {
-        assert(vchSecret.size() == 32);
-        SetData(128 + (fTestNet ? CBitcoinAddress::PUBKEY_ADDRESS_TEST : CBitcoinAddress::PUBKEY_ADDRESS), &vchSecret[0], vchSecret.size());
-        if (fCompressed)
-            vchData.push_back(1);
-    }
-
-    CSecret GetSecret(bool &fCompressedOut)
-    {
-        CSecret vchSecret;
-        vchSecret.resize(32);
-        memcpy(&vchSecret[0], &vchData[0], 32);
-        fCompressedOut = vchData.size() == 33;
-        return vchSecret;
-    }
-
-    bool IsValid() const
-    {
-        bool fExpectTestNet = false;
-        switch(nVersion)
-        {
-            case (128 + CBitcoinAddress::PUBKEY_ADDRESS):
-                break;
-
-            case (128 + CBitcoinAddress::PUBKEY_ADDRESS_TEST):
-                fExpectTestNet = true;
-                break;
-
-            default:
-                return false;
-        }
-        return fExpectTestNet == fTestNet && (vchData.size() == 32 || (vchData.size() == 33 && vchData[32] == 1));
-    }
-
-    bool SetString(const char* pszSecret)
-    {
-        return CBase58Data::SetString(pszSecret) && IsValid();
-    }
-
-    bool SetString(const std::string& strSecret)
-    {
-        return SetString(strSecret.c_str());
-    }
-
-    CBitcoinSecret(const CSecret& vchSecret, bool fCompressed)
-    {
-        SetSecret(vchSecret, fCompressed);
-    }
-
-    CBitcoinSecret()
-    {
-    }
-};
-
 #endif

src/chainparams.cpp

@@ -102,7 +102,7 @@ class CMainParams : public CChainParams {
 
         base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,62);
         base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,85);
-        base58Prefixes[SECRET_KEY] = std::vector<unsigned char>(1,0); // TODO: What should be the exact value here? 128 + PUBKEY_ADDRESS as 4 bytes?
+        base58Prefixes[SECRET_KEY] = std::vector<unsigned char>(1,190);
         base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x88, 0xB2, 0x1E};
         base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x88, 0xAD, 0xE4};
 
@@ -213,7 +213,7 @@ class CTestNetParams : public CChainParams {
 
         base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,111);
         base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,196);
-        base58Prefixes[SECRET_KEY] = std::vector<unsigned char>(1,0); // TODO: What should be the exact value here? 128 + PUBKEY_ADDRESS as 4 bytes?
+        base58Prefixes[SECRET_KEY] = std::vector<unsigned char>(1,239);
         base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF};
         base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94};
 

src/crypter.cpp

@@ -114,7 +114,7 @@ bool CCrypter::Decrypt(const std::vector<unsigned char>& vchCiphertext, CKeyingM
 }
 
 
-bool EncryptSecret(CKeyingMaterial& vMasterKey, const CSecret &vchPlaintext, const uint256& nIV, std::vector<unsigned char> &vchCiphertext)
+bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vchPlaintext, const uint256& nIV, std::vector<unsigned char> &vchCiphertext)
 {
     CCrypter cKeyCrypter;
     std::vector<unsigned char> chIV(WALLET_CRYPTO_KEY_SIZE);
@@ -124,7 +124,7 @@ bool EncryptSecret(CKeyingMaterial& vMasterKey, const CSecret &vchPlaintext, con
     return cKeyCrypter.Encrypt((CKeyingMaterial)vchPlaintext, vchCiphertext);
 }
 
-bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char>& vchCiphertext, const uint256& nIV, CSecret& vchPlaintext)
+bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char>& vchCiphertext, const uint256& nIV, CKeyingMaterial& vchPlaintext)
 {
     CCrypter cKeyCrypter;
     std::vector<unsigned char> chIV(WALLET_CRYPTO_KEY_SIZE);

src/crypter.h

@@ -120,8 +120,8 @@ class CCrypter
 };
 
 
-bool EncryptSecret(CKeyingMaterial& vMasterKey, const CSecret &vchPlaintext, const uint256& nIV, std::vector<unsigned char> &vchCiphertext);
-bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char> &vchCiphertext, const uint256& nIV, CSecret &vchPlaintext);
+bool EncryptSecret(const CKeyingMaterial& vMasterKey, const CKeyingMaterial &vchPlaintext, const uint256& nIV, std::vector<unsigned char> &vchCiphertext);
+bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char> &vchCiphertext, const uint256& nIV, CKeyingMaterial& vchPlaintext);
 bool DecryptKey(const CKeyingMaterial& vMasterKey, const std::vector<unsigned char>& vchCryptedSecret, const CPubKey& vchPubKey, CKey& key);
 
 #endif // BITCOIN_CRYPTER_H

src/gridcoin/backup.cpp

@@ -336,29 +336,3 @@ bool GRC::MaintainBackups(fs::path wallet_backup_path, std::vector<std::string>
     return true;
 }
 
-bool GRC::BackupPrivateKeys(const CWallet& wallet, std::string& sTarget, std::string& sErrors)
-{
-    if (wallet.IsLocked() || fWalletUnlockStakingOnly)
-    {
-        sErrors = "Wallet needs to be fully unlocked to backup private keys.";
-        return false;
-    }
-    fs::path PrivateKeysTarget = GetBackupFilename("keys.dat");
-    fs::create_directories(PrivateKeysTarget.parent_path());
-    sTarget = PrivateKeysTarget.string();
-    fsbridge::ofstream myBackup;
-    myBackup.open(PrivateKeysTarget);
-    std::string sError;
-    for(const auto& keyPair : wallet.GetAllPrivateKeys(sError))
-    {
-        if (!sError.empty())
-        {
-            sErrors = sError;
-            return false;
-        }
-        myBackup << "Address: " << keyPair.first.ToString() << ", Secret: " << keyPair.second.ToString() << std::endl;
-    }
-    LogPrintf("BackupPrivateKeys: Backup made to %s", PrivateKeysTarget.string());
-    myBackup.close();
-    return true;
-}

src/gridcoin/backup.h

@@ -23,7 +23,6 @@ bool BackupConfigFile(const std::string& strDest);
 bool MaintainBackups(fs::path wallet_backup_path, std::vector<std::string> backup_file_type,
                    unsigned int retention_by_num, unsigned int retention_by_days, std::vector<std::string>& files_removed);
 bool BackupWallet(const CWallet& wallet, const std::string& strDest);
-bool BackupPrivateKeys(const CWallet& wallet, std::string& sTarget, std::string& sErrors);
 }
 
 #endif // GRIDCOIN_BACKUP_H