Merge #11372: Address encoding cleanup

92f1f8b Split off key_io_tests from base58_tests (Pieter Wuille)
119b0f8 Split key_io (address/key encodings) off from base58 (Pieter Wuille)
ebfe217 Stop using CBase58Data for ext keys (Pieter Wuille)
32e69fa Replace CBitcoinSecret with {Encode,Decode}Secret (Pieter Wuille)

Pull request description:

  This PR contains some of the changes left as TODO in #11167 (and built on top of that PR). They are not intended for backporting.

  This removes the `CBase58`, `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey` classes, in favor of simple `Encode`/`Decode` functions. Furthermore, all Bitcoin-specific logic (addresses, WIF, BIP32) is moved to `key_io.{h,cpp}`, leaving `base58.{h,cpp}` as a pure utility that implements the base58 encoding/decoding logic.

Tree-SHA512: a5962c0ed27ad53cbe00f22af432cf11aa530e3efc9798e25c004bc9ed1b5673db5df3956e398ee2c085e3a136ac8da69fe7a7d97a05fb2eb3be0b60d0479655

Author: laanwj

src/Makefile.am

@@ -105,6 +105,7 @@ BITCOIN_CORE_H = \
   indirectmap.h \
   init.h \
   key.h \
+  key_io.h \
   keystore.h \
   dbwrapper.h \
   limitedmap.h \
@@ -327,6 +328,7 @@ libbitcoin_common_a_SOURCES = \
   core_read.cpp \
   core_write.cpp \
   key.cpp \
+  key_io.cpp \
   keystore.cpp \
   netaddress.cpp \
   netbase.cpp \

src/Makefile.test.include

@@ -9,13 +9,13 @@ TEST_SRCDIR = test
 TEST_BINARY=test/test_bitcoin$(EXEEXT)
 
 JSON_TEST_FILES = \
-  test/data/script_tests.json \
-  test/data/base58_keys_valid.json \
   test/data/base58_encode_decode.json \
-  test/data/base58_keys_invalid.json \
+  test/data/key_io_valid.json \
+  test/data/key_io_invalid.json \
+  test/data/script_tests.json \
+  test/data/sighash.json \
   test/data/tx_invalid.json \
-  test/data/tx_valid.json \
-  test/data/sighash.json
+  test/data/tx_valid.json
 
 RAW_TEST_FILES =
 
@@ -45,6 +45,7 @@ BITCOIN_TESTS =\
   test/DoS_tests.cpp \
   test/getarg_tests.cpp \
   test/hash_tests.cpp \
+  test/key_io_tests.cpp \
   test/key_tests.cpp \
   test/limitedmap_tests.cpp \
   test/dbwrapper_tests.cpp \

src/base58.cpp

@@ -4,20 +4,12 @@
 
 #include <base58.h>
 
-#include <bech32.h>
 #include <hash.h>
-#include <script/script.h>
 #include <uint256.h>
-#include <utilstrencodings.h>
 
-#include <boost/variant/apply_visitor.hpp>
-#include <boost/variant/static_visitor.hpp>
-
-#include <algorithm>
 #include <assert.h>
 #include <string.h>
 
-
 /** All alphanumeric characters except for "0", "I", "O", and "l" */
 static const char* pszBase58 = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz";
 
@@ -151,227 +143,3 @@ bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRe
 {
     return DecodeBase58Check(str.c_str(), vchRet);
 }
-
-CBase58Data::CBase58Data()
-{
-    vchVersion.clear();
-    vchData.clear();
-}
-
-void CBase58Data::SetData(const std::vector<unsigned char>& vchVersionIn, const void* pdata, size_t nSize)
-{
-    vchVersion = vchVersionIn;
-    vchData.resize(nSize);
-    if (!vchData.empty())
-        memcpy(vchData.data(), pdata, nSize);
-}
-
-void CBase58Data::SetData(const std::vector<unsigned char>& vchVersionIn, const unsigned char* pbegin, const unsigned char* pend)
-{
-    SetData(vchVersionIn, (void*)pbegin, pend - pbegin);
-}
-
-bool CBase58Data::SetString(const char* psz, unsigned int nVersionBytes)
-{
-    std::vector<unsigned char> vchTemp;
-    bool rc58 = DecodeBase58Check(psz, vchTemp);
-    if ((!rc58) || (vchTemp.size() < nVersionBytes)) {
-        vchData.clear();
-        vchVersion.clear();
-        return false;
-    }
-    vchVersion.assign(vchTemp.begin(), vchTemp.begin() + nVersionBytes);
-    vchData.resize(vchTemp.size() - nVersionBytes);
-    if (!vchData.empty())
-        memcpy(vchData.data(), vchTemp.data() + nVersionBytes, vchData.size());
-    memory_cleanse(vchTemp.data(), vchTemp.size());
-    return true;
-}
-
-bool CBase58Data::SetString(const std::string& str)
-{
-    return SetString(str.c_str());
-}
-
-std::string CBase58Data::ToString() const
-{
-    std::vector<unsigned char> vch = vchVersion;
-    vch.insert(vch.end(), vchData.begin(), vchData.end());
-    return EncodeBase58Check(vch);
-}
-
-int CBase58Data::CompareTo(const CBase58Data& b58) const
-{
-    if (vchVersion < b58.vchVersion)
-        return -1;
-    if (vchVersion > b58.vchVersion)
-        return 1;
-    if (vchData < b58.vchData)
-        return -1;
-    if (vchData > b58.vchData)
-        return 1;
-    return 0;
-}
-
-namespace
-{
-class DestinationEncoder : public boost::static_visitor<std::string>
-{
-private:
-    const CChainParams& m_params;
-
-public:
-    DestinationEncoder(const CChainParams& params) : m_params(params) {}
-
-    std::string operator()(const CKeyID& id) const
-    {
-        std::vector<unsigned char> data = m_params.Base58Prefix(CChainParams::PUBKEY_ADDRESS);
-        data.insert(data.end(), id.begin(), id.end());
-        return EncodeBase58Check(data);
-    }
-
-    std::string operator()(const CScriptID& id) const
-    {
-        std::vector<unsigned char> data = m_params.Base58Prefix(CChainParams::SCRIPT_ADDRESS);
-        data.insert(data.end(), id.begin(), id.end());
-        return EncodeBase58Check(data);
-    }
-
-    std::string operator()(const WitnessV0KeyHash& id) const
-    {
-        std::vector<unsigned char> data = {0};
-        ConvertBits<8, 5, true>(data, id.begin(), id.end());
-        return bech32::Encode(m_params.Bech32HRP(), data);
-    }
-
-    std::string operator()(const WitnessV0ScriptHash& id) const
-    {
-        std::vector<unsigned char> data = {0};
-        ConvertBits<8, 5, true>(data, id.begin(), id.end());
-        return bech32::Encode(m_params.Bech32HRP(), data);
-    }
-
-    std::string operator()(const WitnessUnknown& id) const
-    {
-        if (id.version < 1 || id.version > 16 || id.length < 2 || id.length > 40) {
-            return {};
-        }
-        std::vector<unsigned char> data = {(unsigned char)id.version};
-        ConvertBits<8, 5, true>(data, id.program, id.program + id.length);
-        return bech32::Encode(m_params.Bech32HRP(), data);
-    }
-
-    std::string operator()(const CNoDestination& no) const { return {}; }
-};
-
-CTxDestination DecodeDestination(const std::string& str, const CChainParams& params)
-{
-    std::vector<unsigned char> data;
-    uint160 hash;
-    if (DecodeBase58Check(str, data)) {
-        // base58-encoded Bitcoin addresses.
-        // Public-key-hash-addresses have version 0 (or 111 testnet).
-        // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key.
-        const std::vector<unsigned char>& pubkey_prefix = params.Base58Prefix(CChainParams::PUBKEY_ADDRESS);
-        if (data.size() == hash.size() + pubkey_prefix.size() && std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin())) {
-            std::copy(data.begin() + pubkey_prefix.size(), data.end(), hash.begin());
-            return CKeyID(hash);
-        }
-        // Script-hash-addresses have version 5 (or 196 testnet).
-        // The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script.
-        const std::vector<unsigned char>& script_prefix = params.Base58Prefix(CChainParams::SCRIPT_ADDRESS);
-        if (data.size() == hash.size() + script_prefix.size() && std::equal(script_prefix.begin(), script_prefix.end(), data.begin())) {
-            std::copy(data.begin() + script_prefix.size(), data.end(), hash.begin());
-            return CScriptID(hash);
-        }
-    }
-    data.clear();
-    auto bech = bech32::Decode(str);
-    if (bech.second.size() > 0 && bech.first == params.Bech32HRP()) {
-        // Bech32 decoding
-        int version = bech.second[0]; // The first 5 bit symbol is the witness version (0-16)
-        // The rest of the symbols are converted witness program bytes.
-        if (ConvertBits<5, 8, false>(data, bech.second.begin() + 1, bech.second.end())) {
-            if (version == 0) {
-                {
-                    WitnessV0KeyHash keyid;
-                    if (data.size() == keyid.size()) {
-                        std::copy(data.begin(), data.end(), keyid.begin());
-                        return keyid;
-                    }
-                }
-                {
-                    WitnessV0ScriptHash scriptid;
-                    if (data.size() == scriptid.size()) {
-                        std::copy(data.begin(), data.end(), scriptid.begin());
-                        return scriptid;
-                    }
-                }
-                return CNoDestination();
-            }
-            if (version > 16 || data.size() < 2 || data.size() > 40) {
-                return CNoDestination();
-            }
-            WitnessUnknown unk;
-            unk.version = version;
-            std::copy(data.begin(), data.end(), unk.program);
-            unk.length = data.size();
-            return unk;
-        }
-    }
-    return CNoDestination();
-}
-} // namespace
-
-void CBitcoinSecret::SetKey(const CKey& vchSecret)
-{
-    assert(vchSecret.IsValid());
-    SetData(Params().Base58Prefix(CChainParams::SECRET_KEY), vchSecret.begin(), vchSecret.size());
-    if (vchSecret.IsCompressed())
-        vchData.push_back(1);
-}
-
-CKey CBitcoinSecret::GetKey()
-{
-    CKey ret;
-    assert(vchData.size() >= 32);
-    ret.Set(vchData.begin(), vchData.begin() + 32, vchData.size() > 32 && vchData[32] == 1);
-    return ret;
-}
-
-bool CBitcoinSecret::IsValid() const
-{
-    bool fExpectedFormat = vchData.size() == 32 || (vchData.size() == 33 && vchData[32] == 1);
-    bool fCorrectVersion = vchVersion == Params().Base58Prefix(CChainParams::SECRET_KEY);
-    return fExpectedFormat && fCorrectVersion;
-}
-
-bool CBitcoinSecret::SetString(const char* pszSecret)
-{
-    return CBase58Data::SetString(pszSecret) && IsValid();
-}
-
-bool CBitcoinSecret::SetString(const std::string& strSecret)
-{
-    return SetString(strSecret.c_str());
-}
-
-std::string EncodeDestination(const CTxDestination& dest)
-{
-    return boost::apply_visitor(DestinationEncoder(Params()), dest);
-}
-
-CTxDestination DecodeDestination(const std::string& str)
-{
-    return DecodeDestination(str, Params());
-}
-
-bool IsValidDestinationString(const std::string& str, const CChainParams& params)
-{
-    return IsValidDestination(DecodeDestination(str, params));
-}
-
-bool IsValidDestinationString(const std::string& str)
-{
-    return IsValidDestinationString(str, Params());
-}