From dc425633da353ec3d1c7feec7d749502f66a3481 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 28 Aug 2019 15:12:51 -0700 Subject: [PATCH 01/12] Add some general std::vector utility functions Added are: * Vector(arg1,arg2,arg3,...) constructs a vector with the specified arguments as elements. The vector's type is derived from the arguments. If some of the arguments are rvalue references, they will be moved into place rather than copied (which can't be achieved using list initialization). * Cat(vector1,vector2) returns a concatenation of the two vectors, efficiently moving elements when relevant. Vector generalizes (and replaces) the Singleton function in src/descriptor.cpp, and Cat replaces the Cat function in bech32.cpp --- src/Makefile.am | 1 + src/bech32.cpp | 10 +++------- src/txdb.cpp | 3 ++- src/util/vector.h | 51 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 src/util/vector.h diff --git a/src/Makefile.am b/src/Makefile.am index b4a257dbb03a..c09b335e8487 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -290,6 +290,7 @@ BITCOIN_CORE_H = \ utilstrencodings.h \ utilmoneystr.h \ utiltime.h \ + util/vector.h \ validation.h \ validationinterface.h \ version.h \ diff --git a/src/bech32.cpp b/src/bech32.cpp index 78c35b976a5c..b675250594d8 100644 --- a/src/bech32.cpp +++ b/src/bech32.cpp @@ -3,6 +3,9 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "bech32.h" +#include "util/vector.h" + +#include namespace { @@ -24,13 +27,6 @@ const int8_t CHARSET_REV[128] = { 1, 0, 3, 16, 11, 28, 12, 14, 6, 4, 2, -1, -1, -1, -1, -1 }; -/** Concatenate two byte arrays. */ -data Cat(data x, const data& y) -{ - x.insert(x.end(), y.begin(), y.end()); - return x; -} - /** This function will compute what 6 5-bit values to XOR into the last 6 input values, in order to * make the checksum 0. These 6 values are packed together in a single 30-bit integer. The higher * bits correspond to earlier values. */ diff --git a/src/txdb.cpp b/src/txdb.cpp index 5b894ce5b59a..4847803aee3c 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -11,6 +11,7 @@ #include "uint256.h" #include "util/system.h" #include "zpiv/zerocoin.h" +#include <"til/vector.h" #include @@ -113,7 +114,7 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap& mapCoins, // A vector is used for future extensibility, as we may want to support // interrupting after partial writes from multiple independent reorgs. batch.Erase(DB_BEST_BLOCK); - batch.Write(DB_HEAD_BLOCKS, std::vector{hashBlock, old_tip}); + batch.Write(DB_HEAD_BLOCKS, Vector(hashBlock, old_tip)); for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { diff --git a/src/util/vector.h b/src/util/vector.h new file mode 100644 index 000000000000..dab65ded2ace --- /dev/null +++ b/src/util/vector.h @@ -0,0 +1,51 @@ +// Copyright (c) 2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_VECTOR_H +#define BITCOIN_UTIL_VECTOR_H + +#include +#include +#include + +/** Construct a vector with the specified elements. + * + * This is preferable over the list initializing constructor of std::vector: + * - It automatically infers the element type from its arguments. + * - If any arguments are rvalue references, they will be moved into the vector + * (list initialization always copies). + */ +template +inline std::vector::type> Vector(Args&&... args) +{ + std::vector::type> ret; + ret.reserve(sizeof...(args)); + // The line below uses the trick from https://www.experts-exchange.com/articles/32502/None-recursive-variadic-templates-with-std-initializer-list.html + (void)std::initializer_list{(ret.emplace_back(std::forward(args)), 0)...}; + return ret; +} + +/** Concatenate two vectors, moving elements. */ +template +inline V Cat(V v1, V&& v2) +{ + v1.reserve(v1.size() + v2.size()); + for (auto& arg : v2) { + v1.push_back(std::move(arg)); + } + return v1; +} + +/** Concatenate two vectors. */ +template +inline V Cat(V v1, const V& v2) +{ + v1.reserve(v1.size() + v2.size()); + for (const auto& arg : v2) { + v1.push_back(arg); + } + return v1; +} + +#endif // BITCOIN_UTIL_VECTOR_H From 32c1e42c27dd119a1b0a2cc085847dcd5de2ae5f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 10 Oct 2019 11:23:41 -0700 Subject: [PATCH 02/12] Add tests for util/vector.h's Cat and Vector --- src/test/util_tests.cpp | 106 ++++++++++++++++++++++++++++++++++++++++ src/txdb.cpp | 2 +- 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index a66a1ffa7285..d2be290f26b7 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -11,6 +11,7 @@ #include "utilstrencodings.h" #include "utilmoneystr.h" #include "test/test_pivx.h" +#include "util/vector.h" #include #include @@ -948,4 +949,109 @@ BOOST_AUTO_TEST_CASE(test_Capitalize) BOOST_CHECK_EQUAL(Capitalize("\x00\xfe\xff"), "\x00\xfe\xff"); } +namespace { + + struct Tracker + { + //! Points to the original object (possibly itself) we moved/copied from + const Tracker* origin; + //! How many copies where involved between the original object and this one (moves are not counted) + int copies; + + Tracker() noexcept : origin(this), copies(0) {} + Tracker(const Tracker& t) noexcept : origin(t.origin), copies(t.copies + 1) {} + Tracker(Tracker&& t) noexcept : origin(t.origin), copies(t.copies) {} + Tracker& operator=(const Tracker& t) noexcept + { + origin = t.origin; + copies = t.copies + 1; + return *this; + } + Tracker& operator=(Tracker&& t) noexcept + { + origin = t.origin; + copies = t.copies; + return *this; + } + }; + +} + +BOOST_AUTO_TEST_CASE(test_tracked_vector) +{ + Tracker t1; + Tracker t2; + Tracker t3; + + BOOST_CHECK(t1.origin == &t1); + BOOST_CHECK(t2.origin == &t2); + BOOST_CHECK(t3.origin == &t3); + + auto v1 = Vector(t1); + BOOST_CHECK_EQUAL(v1.size(), 1); + BOOST_CHECK(v1[0].origin == &t1); + BOOST_CHECK_EQUAL(v1[0].copies, 1); + + auto v2 = Vector(std::move(t2)); + BOOST_CHECK_EQUAL(v2.size(), 1); + BOOST_CHECK(v2[0].origin == &t2); + BOOST_CHECK_EQUAL(v2[0].copies, 0); + + auto v3 = Vector(t1, std::move(t2)); + BOOST_CHECK_EQUAL(v3.size(), 2); + BOOST_CHECK(v3[0].origin == &t1); + BOOST_CHECK(v3[1].origin == &t2); + BOOST_CHECK_EQUAL(v3[0].copies, 1); + BOOST_CHECK_EQUAL(v3[1].copies, 0); + + auto v4 = Vector(std::move(v3[0]), v3[1], std::move(t3)); + BOOST_CHECK_EQUAL(v4.size(), 3); + BOOST_CHECK(v4[0].origin == &t1); + BOOST_CHECK(v4[1].origin == &t2); + BOOST_CHECK(v4[2].origin == &t3); + BOOST_CHECK_EQUAL(v4[0].copies, 1); + BOOST_CHECK_EQUAL(v4[1].copies, 1); + BOOST_CHECK_EQUAL(v4[2].copies, 0); + + auto v5 = Cat(v1, v4); + BOOST_CHECK_EQUAL(v5.size(), 4); + BOOST_CHECK(v5[0].origin == &t1); + BOOST_CHECK(v5[1].origin == &t1); + BOOST_CHECK(v5[2].origin == &t2); + BOOST_CHECK(v5[3].origin == &t3); + BOOST_CHECK_EQUAL(v5[0].copies, 2); + BOOST_CHECK_EQUAL(v5[1].copies, 2); + BOOST_CHECK_EQUAL(v5[2].copies, 2); + BOOST_CHECK_EQUAL(v5[3].copies, 1); + + auto v6 = Cat(std::move(v1), v3); + BOOST_CHECK_EQUAL(v6.size(), 3); + BOOST_CHECK(v6[0].origin == &t1); + BOOST_CHECK(v6[1].origin == &t1); + BOOST_CHECK(v6[2].origin == &t2); + BOOST_CHECK_EQUAL(v6[0].copies, 1); + BOOST_CHECK_EQUAL(v6[1].copies, 2); + BOOST_CHECK_EQUAL(v6[2].copies, 1); + + auto v7 = Cat(v2, std::move(v4)); + BOOST_CHECK_EQUAL(v7.size(), 4); + BOOST_CHECK(v7[0].origin == &t2); + BOOST_CHECK(v7[1].origin == &t1); + BOOST_CHECK(v7[2].origin == &t2); + BOOST_CHECK(v7[3].origin == &t3); + BOOST_CHECK_EQUAL(v7[0].copies, 1); + BOOST_CHECK_EQUAL(v7[1].copies, 1); + BOOST_CHECK_EQUAL(v7[2].copies, 1); + BOOST_CHECK_EQUAL(v7[3].copies, 0); + + auto v8 = Cat(std::move(v2), std::move(v3)); + BOOST_CHECK_EQUAL(v8.size(), 3); + BOOST_CHECK(v8[0].origin == &t2); + BOOST_CHECK(v8[1].origin == &t1); + BOOST_CHECK(v8[2].origin == &t2); + BOOST_CHECK_EQUAL(v8[0].copies, 0); + BOOST_CHECK_EQUAL(v8[1].copies, 1); + BOOST_CHECK_EQUAL(v8[2].copies, 0); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txdb.cpp b/src/txdb.cpp index 4847803aee3c..2035c1c75081 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -11,7 +11,7 @@ #include "uint256.h" #include "util/system.h" #include "zpiv/zerocoin.h" -#include <"til/vector.h" +#include "util/vector.h" #include From f6c28729ce51fe84bd64d6b64943289b1682f5b6 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 20 Aug 2019 14:51:43 -0400 Subject: [PATCH 03/12] util: Add Join helper to join a list of strings --- src/Makefile.am | 2 ++ src/test/util_tests.cpp | 14 ++++++++++++++ src/util/string.cpp | 5 +++++ src/util/string.h | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 src/util/string.cpp create mode 100644 src/util/string.h diff --git a/src/Makefile.am b/src/Makefile.am index c09b335e8487..80ccee8cbd7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -286,6 +286,7 @@ BITCOIN_CORE_H = \ util/memory.h \ util/system.h \ util/macros.h \ + util/string.h \ util/threadnames.h \ utilstrencodings.h \ utilmoneystr.h \ @@ -558,6 +559,7 @@ libbitcoin_util_a_SOURCES = \ utilmoneystr.cpp \ util/threadnames.cpp \ utilstrencodings.cpp \ + util/string.cpp \ utiltime.cpp \ $(BITCOIN_CORE_H) \ $(LIBSAPLING_H) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index d2be290f26b7..1c0937763c02 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -9,6 +9,7 @@ #include "primitives/transaction.h" #include "sync.h" #include "utilstrencodings.h" +#include "util/string.h" #include "utilmoneystr.h" #include "test/test_pivx.h" #include "util/vector.h" @@ -85,6 +86,19 @@ BOOST_AUTO_TEST_CASE(util_HexStr) "04 67 8a fd b0"); } +BOOST_AUTO_TEST_CASE(util_Join) +{ + // Normal version + BOOST_CHECK_EQUAL(Join({}, ", "), ""); + BOOST_CHECK_EQUAL(Join({"foo"}, ", "), "foo"); + BOOST_CHECK_EQUAL(Join({"foo", "bar"}, ", "), "foo, bar"); + + // Version with unary operator + const auto op_upper = [](const std::string& s) { return ToUpper(s); }; + BOOST_CHECK_EQUAL(Join({}, ", ", op_upper), ""); + BOOST_CHECK_EQUAL(Join({"foo"}, ", ", op_upper), "FOO"); + BOOST_CHECK_EQUAL(Join({"foo", "bar"}, ", ", op_upper), "FOO, BAR"); +} BOOST_AUTO_TEST_CASE(util_FormatISO8601DateTime) { diff --git a/src/util/string.cpp b/src/util/string.cpp new file mode 100644 index 000000000000..8ea3a1afc6a0 --- /dev/null +++ b/src/util/string.cpp @@ -0,0 +1,5 @@ +// Copyright (c) 2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include diff --git a/src/util/string.h b/src/util/string.h new file mode 100644 index 000000000000..dec0c19b0829 --- /dev/null +++ b/src/util/string.h @@ -0,0 +1,35 @@ +// Copyright (c) 2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_STRING_H +#define BITCOIN_UTIL_STRING_H + +#include +#include +#include + +/** + * Join a list of items + * + * @param list The list to join + * @param separator The separator + * @param unary_op Apply this operator to each item in the list + */ +template +std::string Join(const std::vector& list, const std::string& separator, UnaryOp unary_op) +{ + std::string ret; + for (size_t i = 0; i < list.size(); ++i) { + if (i > 0) ret += separator; + ret += unary_op(list.at(i)); + } + return ret; +} + +inline std::string Join(const std::vector& list, const std::string& separator) +{ + return Join(list, separator, [](const std::string& i) { return i; }); +} + +#endif // BITCOIN_UTIL_STRENCODINGS_H From e861cda0f25d2e3f23e3961d7956511ae628a32a Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 5 Jun 2021 17:09:34 -0300 Subject: [PATCH 04/12] Backport string ToUpper and ToLower. --- src/utilstrencodings.cpp | 14 ++++++++++++++ src/utilstrencodings.h | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 93aed475976b..20d1714dd850 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -506,6 +506,20 @@ void Downcase(std::string& str) std::transform(str.begin(), str.end(), str.begin(), [](unsigned char c){return ToLower(c);}); } +std::string ToLower(const std::string& str) +{ + std::string r; + for (auto ch : str) r += ToLower((unsigned char)ch); + return r; +} + +std::string ToUpper(const std::string& str) +{ + std::string r; + for (auto ch : str) r += ToUpper((unsigned char)ch); + return r; +} + std::string Capitalize(std::string str) { if (str.empty()) return str; diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index 3a84b68f36e6..a3ef855db211 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -222,6 +222,17 @@ constexpr unsigned char ToLower(unsigned char c) return (c >= 'A' && c <= 'Z' ? (c - 'A') + 'a' : c); } +/** + * Returns the lowercase equivalent of the given string. + * This function is locale independent. It only converts uppercase + * characters in the standard 7-bit ASCII range. + * This is a feature, not a limitation. + * + * @param[in] str the string to convert to lowercase. + * @returns lowercased equivalent of str + */ +std::string ToLower(const std::string& str); + /** * Converts the given string to its lowercase equivalent. * This function is locale independent. It only converts uppercase @@ -243,6 +254,17 @@ constexpr unsigned char ToUpper(unsigned char c) return (c >= 'a' && c <= 'z' ? (c - 'a') + 'A' : c); } +/** + * Returns the uppercase equivalent of the given string. + * This function is locale independent. It only converts lowercase + * characters in the standard 7-bit ASCII range. + * This is a feature, not a limitation. + * + * @param[in] str the string to convert to uppercase. + * @returns UPPERCASED EQUIVALENT OF str + */ +std::string ToUpper(const std::string& str); + /** * Capitalizes the first character of the given string. * This function is locale independent. It only capitalizes the From 4d4160e4d5dd12641fea107fabe49e2ba017af58 Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 5 Jun 2021 17:33:22 -0300 Subject: [PATCH 05/12] Stop using CBase58Data for ext keys Adaptation of btc@ebfe217b15d21656a173e5c102f826d17c6c8be4 --- src/base58.cpp | 63 ----------------------------------------------- src/base58.h | 67 -------------------------------------------------- 2 files changed, 130 deletions(-) diff --git a/src/base58.cpp b/src/base58.cpp index 40e3b1cb336c..92818c90bb56 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -166,69 +166,6 @@ bool DecodeBase58Check(const std::string& str, std::vector& vchRe return DecodeBase58Check(str.c_str(), vchRet); } -CBase58Data::CBase58Data() -{ - vchVersion.clear(); - vchData.clear(); -} - -void CBase58Data::SetData(const std::vector& 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& 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 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) -{ - if (str.empty()) - return false; - return SetString(str.c_str()); -} - -std::string CBase58Data::ToString() const -{ - std::vector 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 diff --git a/src/base58.h b/src/base58.h index 63fa97b8620b..e4d788173341 100644 --- a/src/base58.h +++ b/src/base58.h @@ -70,76 +70,9 @@ inline bool DecodeBase58Check(const char* psz, std::vector& vchRe */ bool DecodeBase58Check(const std::string& str, std::vector& vchRet); -/** - * Base class for all base58-encoded data - */ -class CBase58Data -{ -protected: - //! the version byte(s) - std::vector vchVersion; - - //! the actually encoded data - typedef std::vector > vector_uchar; - vector_uchar vchData; - - CBase58Data(); - void SetData(const std::vector& vchVersionIn, const void* pdata, size_t nSize); - void SetData(const std::vector& vchVersionIn, const unsigned char* pbegin, const unsigned char* pend); - -public: - bool SetString(const char* psz, unsigned int nVersionBytes = 1); - bool SetString(const std::string& str); - std::string ToString() const; - int CompareTo(const CBase58Data& b58) const; - - 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; } -}; - CKey DecodeSecret(const std::string& str); std::string EncodeSecret(const CKey& key); -template -class CBitcoinExtKeyBase : public CBase58Data -{ -public: - void SetKey(const K& key) - { - unsigned char vch[Size]; - key.Encode(vch); - SetData(Params().Base58Prefix(Type), vch, vch + Size); - } - - K GetKey() - { - K ret; - if (vchData.size() == Size) { - // If base58 encoded data does not hold an ext key, return a !IsValid() key - ret.Decode(vchData.data()); - } - return ret; - } - - CBitcoinExtKeyBase(const K& key) - { - SetKey(key); - } - - CBitcoinExtKeyBase(const std::string& strBase58c) { - SetString(strBase58c.c_str(), Params().Base58Prefix(Type).size()); - } - - CBitcoinExtKeyBase() {} -}; - -typedef CBitcoinExtKeyBase CBitcoinExtKey; -typedef CBitcoinExtKeyBase CBitcoinExtPubKey; - - std::string EncodeDestination(const CTxDestination& dest, bool isStaking); std::string EncodeDestination(const CTxDestination& dest, const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS); // DecodeDestinationisStaking flag is set to true when the string arg is from an staking address From c93e19f16f2b972935bd7e1d841d9dcbe57c8232 Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 5 Jun 2021 18:21:44 -0300 Subject: [PATCH 06/12] Clean duplicate usage of DecodeSecret & EncodeSecret. The latest version of these two functions is inside the key_io files and not in base58. --- src/Makefile.am | 2 +- src/base58.cpp | 29 -------------------------- src/base58.h | 6 +----- src/messagesigner.cpp | 3 ++- src/pivx-tx.cpp | 4 ++-- src/qt/pivx/masternodewizarddialog.cpp | 3 ++- src/rpc/masternode.cpp | 4 ++-- src/rpc/rawtransaction.cpp | 3 ++- src/rpc/rpcevo.cpp | 3 ++- src/test/base58_tests.cpp | 11 +++++----- src/test/bloom_tests.cpp | 2 +- src/test/key_tests.cpp | 14 ++++++------- src/test/script_P2CS_tests.cpp | 11 +++++----- src/wallet/rpcdump.cpp | 10 ++++----- 14 files changed, 37 insertions(+), 68 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 80ccee8cbd7e..3aa9415a0265 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -396,7 +396,6 @@ libbitcoin_wallet_a_SOURCES = \ interfaces/wallet.cpp \ addressbook.cpp \ crypter.cpp \ - key_io.cpp \ legacy/stakemodifier.cpp \ kernel.cpp \ wallet/db.cpp \ @@ -498,6 +497,7 @@ libbitcoin_common_a_SOURCES = \ coins.cpp \ compressor.cpp \ consensus/merkle.cpp \ + key_io.cpp \ primitives/block.cpp \ primitives/transaction.cpp \ zpiv/zerocoin.cpp \ diff --git a/src/base58.cpp b/src/base58.cpp index 92818c90bb56..a5f996173764 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -227,35 +227,6 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par } // anon namespace -CKey DecodeSecret(const std::string& str) -{ - CKey key; - std::vector data; - if (DecodeBase58Check(str, data)) { - const std::vector& privkey_prefix = Params().Base58Prefix(CChainParams::SECRET_KEY); - if ((data.size() == 32 + privkey_prefix.size() || (data.size() == 33 + privkey_prefix.size() && data.back() == 1)) && - std::equal(privkey_prefix.begin(), privkey_prefix.end(), data.begin())) { - bool compressed = data.size() == 33 + privkey_prefix.size(); - key.Set(data.begin() + privkey_prefix.size(), data.begin() + privkey_prefix.size() + 32, compressed); - } - } - memory_cleanse(data.data(), data.size()); - return key; -} - -std::string EncodeSecret(const CKey& key) -{ - assert(key.IsValid()); - std::vector data = Params().Base58Prefix(CChainParams::SECRET_KEY); - data.insert(data.end(), key.begin(), key.end()); - if (key.IsCompressed()) { - data.push_back(1); - } - std::string ret = EncodeBase58Check(data); - memory_cleanse(data.data(), data.size()); - return ret; -} - std::string EncodeDestination(const CTxDestination& dest, bool isStaking) { return EncodeDestination(dest, isStaking ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS); diff --git a/src/base58.h b/src/base58.h index e4d788173341..f4ff35426ca0 100644 --- a/src/base58.h +++ b/src/base58.h @@ -70,16 +70,12 @@ inline bool DecodeBase58Check(const char* psz, std::vector& vchRe */ bool DecodeBase58Check(const std::string& str, std::vector& vchRet); -CKey DecodeSecret(const std::string& str); -std::string EncodeSecret(const CKey& key); - std::string EncodeDestination(const CTxDestination& dest, bool isStaking); std::string EncodeDestination(const CTxDestination& dest, const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS); // DecodeDestinationisStaking flag is set to true when the string arg is from an staking address CTxDestination DecodeDestination(const std::string& str, bool& isStaking); CTxDestination DecodeDestination(const std::string& str); -// Return true if the address is valid without care on the type. -bool IsValidDestinationString(const std::string& str); + // Return true if the address is valid and is following the fStaking flag type (true means that the destination must be a staking address, false the opposite). bool IsValidDestinationString(const std::string& str, bool fStaking); bool IsValidDestinationString(const std::string& str, bool fStaking, const CChainParams& params); diff --git a/src/messagesigner.cpp b/src/messagesigner.cpp index 18e2d3a5e00d..b542c21aef54 100644 --- a/src/messagesigner.cpp +++ b/src/messagesigner.cpp @@ -5,6 +5,7 @@ #include "base58.h" #include "hash.h" +#include "key_io.h" #include "messagesigner.h" #include "tinyformat.h" #include "util/system.h" @@ -14,7 +15,7 @@ const std::string strMessageMagic = "DarkNet Signed Message:\n"; bool CMessageSigner::GetKeysFromSecret(const std::string& strSecret, CKey& keyRet, CPubKey& pubkeyRet) { - keyRet = DecodeSecret(strSecret); + keyRet = KeyIO::DecodeSecret(strSecret); if (!keyRet.IsValid()) return false; diff --git a/src/pivx-tx.cpp b/src/pivx-tx.cpp index 1621aff68ecf..3d32e324bc62 100644 --- a/src/pivx-tx.cpp +++ b/src/pivx-tx.cpp @@ -8,8 +8,8 @@ #include "coins.h" #include "core_io.h" #include "keystore.h" +#include "key_io.h" #include "policy/policy.h" -#include "primitives/block.h" // for MAX_BLOCK_SIZE #include "primitives/transaction.h" #include "script/script.h" #include "script/sign.h" @@ -472,7 +472,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) for (unsigned int kidx = 0; kidx < keysObj.size(); kidx++) { if (!keysObj[kidx].isStr()) throw std::runtime_error("privatekey not a string"); - CKey key = DecodeSecret(keysObj[kidx].getValStr()); + CKey key = KeyIO::DecodeSecret(keysObj[kidx].getValStr()); if (!key.IsValid()) { throw std::runtime_error("privatekey not valid"); } diff --git a/src/qt/pivx/masternodewizarddialog.cpp b/src/qt/pivx/masternodewizarddialog.cpp index 74b4be15228b..8d5cb6297efd 100644 --- a/src/qt/pivx/masternodewizarddialog.cpp +++ b/src/qt/pivx/masternodewizarddialog.cpp @@ -7,6 +7,7 @@ #include "activemasternode.h" #include "clientmodel.h" +#include "key_io.h" #include "optionsmodel.h" #include "pairresult.h" #include "qt/pivx/mnmodel.h" @@ -212,7 +213,7 @@ bool MasterNodeWizardDialog::createMN() // create the mn key CKey secret; secret.MakeNewKey(false); - std::string mnKeyString = EncodeSecret(secret); + std::string mnKeyString = KeyIO::EncodeSecret(secret); // Look for a valid collateral utxo COutPoint collateralOut; diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 60cf12b912e9..c4a6604db726 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -7,13 +7,13 @@ #include "db.h" #include "evo/deterministicmns.h" #include "init.h" +#include "key_io.h" #include "masternode-payments.h" #include "masternode-sync.h" #include "masternodeconfig.h" #include "masternodeman.h" #include "netbase.h" #include "rpc/server.h" -#include "utilmoneystr.h" #ifdef ENABLE_WALLET #include "wallet/rpcwallet.h" #endif @@ -554,7 +554,7 @@ UniValue createmasternodekey(const JSONRPCRequest& request) CKey secret; secret.MakeNewKey(false); - return EncodeSecret(secret); + return KeyIO::EncodeSecret(secret); } UniValue getmasternodeoutputs(const JSONRPCRequest& request) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 82ab17016ab1..61f51aba206f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -11,6 +11,7 @@ #include "evo/providertx.h" #include "init.h" #include "keystore.h" +#include "key_io.h" #include "validationinterface.h" #include "net.h" #include "policy/policy.h" @@ -612,7 +613,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) UniValue keys = request.params[2].get_array(); for (unsigned int idx = 0; idx < keys.size(); idx++) { UniValue k = keys[idx]; - CKey key = DecodeSecret(k.get_str()); + CKey key = KeyIO::DecodeSecret(k.get_str()); if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); tempKeystore.AddKey(key); diff --git a/src/rpc/rpcevo.cpp b/src/rpc/rpcevo.cpp index a12b31821e99..bb7d06576bd1 100644 --- a/src/rpc/rpcevo.cpp +++ b/src/rpc/rpcevo.cpp @@ -8,6 +8,7 @@ #include "evo/deterministicmns.h" #include "evo/specialtx.h" #include "evo/providertx.h" +#include "key_io.h" #include "masternode.h" #include "messagesigner.h" #include "netbase.h" @@ -180,7 +181,7 @@ static CKey ParsePrivKey(CWallet* pwallet, const std::string &strKeyOrAddress, b #endif // ENABLE_WALLET } - CKey key = DecodeSecret(strKeyOrAddress); + CKey key = KeyIO::DecodeSecret(strKeyOrAddress); if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); return key; } diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 11b50bbe9f5d..701a56f8a863 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -10,9 +10,8 @@ #include "data/base58_keys_valid.json.h" #include "key.h" -#include "script/script.h" +#include "key_io.h" #include "uint256.h" -#include "util/system.h" #include "utilstrencodings.h" #include "test/test_pivx.h" @@ -146,7 +145,7 @@ BOOST_AUTO_TEST_CASE(base58_keys_valid_parse) if(isPrivkey) { bool isCompressed = find_value(metadata, "isCompressed").get_bool(); // Must be valid private key - privkey = DecodeSecret(exp_base58string); + privkey = KeyIO::DecodeSecret(exp_base58string); BOOST_CHECK_MESSAGE(privkey.IsValid(), "!IsValid:" + strTest); BOOST_CHECK_MESSAGE(privkey.IsCompressed() == isCompressed, "compressed mismatch:" + strTest); BOOST_CHECK_MESSAGE(privkey.size() == exp_payload.size() && std::equal(privkey.begin(), privkey.end(), exp_payload.begin()), "key mismatch:" + strTest); @@ -163,7 +162,7 @@ BOOST_AUTO_TEST_CASE(base58_keys_valid_parse) BOOST_CHECK_MESSAGE(boost::apply_visitor(TestAddrTypeVisitor(exp_addrType), destination), "addrType mismatch" + strTest); // Public key must be invalid private key - privkey = DecodeSecret(exp_base58string); + privkey = KeyIO::DecodeSecret(exp_base58string); BOOST_CHECK_MESSAGE(!privkey.IsValid(), "IsValid pubkey as privkey:" + strTest); } } @@ -198,7 +197,7 @@ BOOST_AUTO_TEST_CASE(base58_keys_valid_gen) CKey key; key.Set(exp_payload.begin(), exp_payload.end(), isCompressed); assert(key.IsValid()); - BOOST_CHECK_MESSAGE(EncodeSecret(key) == exp_base58string, "result mismatch: " + strTest); + BOOST_CHECK_MESSAGE(KeyIO::EncodeSecret(key) == exp_base58string, "result mismatch: " + strTest); } else { @@ -250,7 +249,7 @@ BOOST_AUTO_TEST_CASE(base58_keys_invalid) // must be invalid as public and as private key destination = DecodeDestination(exp_base58string); BOOST_CHECK_MESSAGE(!IsValidDestination(destination), "IsValid pubkey:" + strTest); - privkey = DecodeSecret(exp_base58string); + privkey = KeyIO::DecodeSecret(exp_base58string); BOOST_CHECK_MESSAGE(!privkey.IsValid(), "IsValid privkey:" + strTest); } } diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 63dba3864b0f..dc6c5e67cdf6 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -87,7 +87,7 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) BOOST_AUTO_TEST_CASE(bloom_create_insert_key) { std::string strSecret = std::string("5Kg1gnAjaLfKiwhhPpGS3QfRg2m6awQvaj98JCZBZQ5SuS2F15C"); - CKey key = DecodeSecret(strSecret); + CKey key = KeyIO::DecodeSecret(strSecret); CPubKey pubkey = key.GetPubKey(); std::vector vchPubKey(pubkey.begin(), pubkey.end()); diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index e33de3fe0f5b..a7c8c3739ed8 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -6,7 +6,7 @@ #include "key.h" #include "base58.h" -#include "script/script.h" +#include "key_io.h" #include "uint256.h" #include "util/system.h" #include "utilstrencodings.h" @@ -44,7 +44,7 @@ void dumpKeyInfo(uint256 privkey) for (int nCompressed=0; nCompressed<2; nCompressed++) { - printf(" * secret (base58): %s\n", EncodeSecret(secret)); + printf(" * secret (base58): %s\n", KeyIO::EncodeSecret(secret)); CKey key; key.SetSecret(secret, fCompressed); std::vector vchPubKey = key.GetPubKey(); @@ -58,15 +58,15 @@ BOOST_FIXTURE_TEST_SUITE(key_tests, TestingSetup) BOOST_AUTO_TEST_CASE(key_test1) { - CKey key1 = DecodeSecret(strSecret1); + CKey key1 = KeyIO::DecodeSecret(strSecret1); BOOST_CHECK(key1.IsValid() && !key1.IsCompressed()); - CKey key2 = DecodeSecret(strSecret2); + CKey key2 = KeyIO::DecodeSecret(strSecret2); BOOST_CHECK(key2.IsValid() && !key2.IsCompressed()); - CKey key1C = DecodeSecret(strSecret1C); + CKey key1C = KeyIO::DecodeSecret(strSecret1C); BOOST_CHECK(key1C.IsValid() && key1C.IsCompressed()); - CKey key2C = DecodeSecret(strSecret2C); + CKey key2C = KeyIO::DecodeSecret(strSecret2C); BOOST_CHECK(key2C.IsValid() && key2C.IsCompressed()); - CKey bad_key = DecodeSecret(strAddressBad); + CKey bad_key = KeyIO::DecodeSecret(strAddressBad); BOOST_CHECK(!bad_key.IsValid()); CPubKey pubkey1 = key1. GetPubKey(); diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index a2379900e401..fa055db4b03a 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -1,10 +1,9 @@ // Copyright (c) 2020 The PIVX developers // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. -#include "test/test_pivx.h" - #include "base58.h" #include "key.h" +#include "key_io.h" #include "policy/policy.h" #include "wallet/test/wallet_test_fixture.h" #include "wallet/wallet.h" @@ -59,8 +58,8 @@ BOOST_AUTO_TEST_CASE(extract_cold_staking_destination_keys) static CScript GetNewP2CS(CKey& stakerKey, CKey& ownerKey, bool fLastOutFree) { - stakerKey = DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E"); - ownerKey = DecodeSecret("YUo8oW3y8cUQdQxQxCdnUJ4Ww5H7nHBEMwD2bNDpBbuLM59t4rvd"); + stakerKey = KeyIO::DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E"); + ownerKey = KeyIO::DecodeSecret("YUo8oW3y8cUQdQxQxCdnUJ4Ww5H7nHBEMwD2bNDpBbuLM59t4rvd"); return fLastOutFree ? GetScriptForStakeDelegationLOF(stakerKey.GetPubKey().GetID(), ownerKey.GetPubKey().GetID()) : GetScriptForStakeDelegation(stakerKey.GetPubKey().GetID(), ownerKey.GetPubKey().GetID()); } @@ -149,7 +148,7 @@ BOOST_AUTO_TEST_CASE(coldstake_lof_script) SignColdStake(tx, 0, scriptP2CS, stakerKey, true); BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); - const CKey& dummyKey = DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E"); + const CKey& dummyKey = KeyIO::DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E"); const CKeyID& dummyKeyID = dummyKey.GetPubKey().GetID(); const CScript& dummyP2PKH = GetDummyP2PKH(dummyKeyID); @@ -225,7 +224,7 @@ BOOST_AUTO_TEST_CASE(coldstake_script) SignColdStake(tx, 0, scriptP2CS, stakerKey, true); BOOST_CHECK(CheckP2CSScript(tx.vin[0].scriptSig, scriptP2CS, tx, err)); - const CKey& dummyKey = DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E"); + const CKey& dummyKey = KeyIO::DecodeSecret("YNdsth3BsW53DYmCiR12SofWSAt2utXQUSGoin3PekVQCMbzfS7E"); const CKeyID& dummyKeyID = dummyKey.GetPubKey().GetID(); const CScript& dummyP2PKH = GetDummyP2PKH(dummyKeyID); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e8f7de0b9e71..e0fd551c0700 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -116,7 +116,7 @@ UniValue importprivkey(const JSONRPCRequest& request) const bool fStakingAddress = (request.params.size() > 3 ? request.params[3].get_bool() : false); - CKey key = DecodeSecret(strSecret); + CKey key = KeyIO::DecodeSecret(strSecret); if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); CPubKey pubkey = key.GetPubKey(); @@ -402,7 +402,7 @@ UniValue importwallet(const JSONRPCRequest& request) } } - CKey key = DecodeSecret(vstr[0]); + CKey key = KeyIO::DecodeSecret(vstr[0]); if (!key.IsValid()) continue; CPubKey pubkey = key.GetPubKey(); @@ -490,7 +490,7 @@ UniValue dumpprivkey(const JSONRPCRequest& request) CKey vchSecret; if (!pwallet->GetKey(*keyID, vchSecret)) throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); - return EncodeSecret(vchSecret); + return KeyIO::EncodeSecret(vchSecret); } UniValue dumpwallet(const JSONRPCRequest& request) @@ -761,7 +761,7 @@ static UniValue processImport(CWallet* const pwallet, const UniValue& data, cons if (keys.size()) { for (size_t i = 0; i < keys.size(); i++) { const std::string& privkey = keys[i].get_str(); - CKey key = DecodeSecret(privkey); + CKey key = KeyIO::DecodeSecret(privkey); if (!key.IsValid()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); @@ -858,7 +858,7 @@ static UniValue processImport(CWallet* const pwallet, const UniValue& data, cons // Import private keys. if (keys.size()) { const std::string& strPrivkey = keys[0].get_str(); - CKey key = DecodeSecret(strPrivkey); + CKey key = KeyIO::DecodeSecret(strPrivkey); if (!key.IsValid()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); From 2e9376c325d9158a1213ae2f1fb04cccf89ce532 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 18 Nov 2019 15:16:50 -0800 Subject: [PATCH 07/12] Pass a maximum output length to DecodeBase58 and DecodeBase58Check Also remove a needless loop in DecodeBase58 to prune zeroes in the base256 output of the conversion. The number of zeroes is implied by keeping track explicitly of the length during the loop. --- src/base58.cpp | 20 +++++++++++--------- src/base58.h | 9 +++++---- src/test/base58_tests.cpp | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/base58.cpp b/src/base58.cpp index a5f996173764..51930882266b 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -17,10 +17,12 @@ #include #include +#include + /** All alphanumeric characters except for "0", "I", "O", and "l" */ static const char* pszBase58 = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz"; -bool DecodeBase58(const char* psz, std::vector& vch) +bool DecodeBase58(const char* psz, std::vector& vch, int max_ret_len) { // Skip leading spaces. while (*psz && isspace(*psz)) @@ -30,6 +32,7 @@ bool DecodeBase58(const char* psz, std::vector& vch) int length = 0; while (*psz == '1') { zeroes++; + if (zeroes > max_ret_len) return false; psz++; } // Allocate enough space in big-endian base256 representation. @@ -51,6 +54,7 @@ bool DecodeBase58(const char* psz, std::vector& vch) } assert(carry == 0); length = i; + if (length + zeroes > max_ret_len) return false; psz++; } // Skip trailing spaces. @@ -60,8 +64,6 @@ bool DecodeBase58(const char* psz, std::vector& vch) return false; // Skip leading zeroes in b256. std::vector::iterator it = b256.begin() + (size - length); - while (it != b256.end() && *it == 0) - it++; // Copy result into output vector. vch.reserve(zeroes + (b256.end() - it)); vch.assign(zeroes, 0x00); @@ -130,9 +132,9 @@ std::string EncodeBase58(const std::vector& vch) return EncodeBase58(vch.data(), vch.data() + vch.size()); } -bool DecodeBase58(const std::string& str, std::vector& vchRet) +bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len) { - return DecodeBase58(str.c_str(), vchRet); + return DecodeBase58(str.c_str(), vchRet, max_ret_len); } std::string EncodeBase58Check(const std::vector& vchIn) @@ -144,9 +146,9 @@ std::string EncodeBase58Check(const std::vector& vchIn) return EncodeBase58(vch); } -bool DecodeBase58Check(const char* psz, std::vector& vchRet) +bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len) { - if (!DecodeBase58(psz, vchRet) || + if (!DecodeBase58(psz, vchRet, max_ret_len > std::numeric_limits::max() - 4 ? std::numeric_limits::max() : max_ret_len + 4) || (vchRet.size() < 4)) { vchRet.clear(); return false; @@ -161,9 +163,9 @@ bool DecodeBase58Check(const char* psz, std::vector& vchRet) return true; } -bool DecodeBase58Check(const std::string& str, std::vector& vchRet) +bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret) { - return DecodeBase58Check(str.c_str(), vchRet); + return DecodeBase58Check(str.c_str(), vchRet, max_ret); } namespace diff --git a/src/base58.h b/src/base58.h index f4ff35426ca0..a20dd86ae00a 100644 --- a/src/base58.h +++ b/src/base58.h @@ -20,6 +20,7 @@ #include "pubkey.h" #include "script/standard.h" +#include #include #include @@ -39,7 +40,7 @@ std::string EncodeBase58(const std::vector& vch); * return true if decoding is successful. * psz cannot be NULL. */ -bool DecodeBase58(const char* psz, std::vector& vchRet); +bool DecodeBase58(const char* psz, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); /** * Decode a base58-encoded string (psz) into a string. @@ -51,7 +52,7 @@ std::string DecodeBase58(const char* psz); * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ -bool DecodeBase58(const std::string& str, std::vector& vchRet); +bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); /** * Encode a byte vector into a base58-encoded string, including checksum @@ -62,13 +63,13 @@ std::string EncodeBase58Check(const std::vector& vchIn); * Decode a base58-encoded string (psz) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -inline bool DecodeBase58Check(const char* psz, std::vector& vchRet); +bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); /** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -bool DecodeBase58Check(const std::string& str, std::vector& vchRet); +bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); std::string EncodeDestination(const CTxDestination& dest, bool isStaking); std::string EncodeDestination(const CTxDestination& dest, const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS); diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 701a56f8a863..38b20e3374d1 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -14,6 +14,7 @@ #include "uint256.h" #include "utilstrencodings.h" #include "test/test_pivx.h" +#include "util/vector.h" #include @@ -254,6 +255,21 @@ BOOST_AUTO_TEST_CASE(base58_keys_invalid) } } +BOOST_AUTO_TEST_CASE(base58_random_encode_decode) +{ + for (int n = 0; n < 1000; ++n) { + unsigned int len = 1 + InsecureRandBits(8); + unsigned int zeroes = InsecureRandBool() ? InsecureRandRange(len + 1) : 0; + auto data = Cat(std::vector(zeroes, '\000'), insecure_rand_ctx.randbytes(len - zeroes)); + auto encoded = EncodeBase58Check(data); + std::vector decoded; + auto ok_too_small = DecodeBase58Check(encoded, decoded, InsecureRandRange(len)); + BOOST_CHECK(!ok_too_small); + auto ok = DecodeBase58Check(encoded, decoded, len + InsecureRandRange(257 - len)); + BOOST_CHECK(ok); + BOOST_CHECK(data == decoded); + } +} BOOST_AUTO_TEST_SUITE_END() From eac71b55ba7889ecbf96f7e1d5837027a1e15d85 Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 5 Jun 2021 19:37:17 -0300 Subject: [PATCH 08/12] Finish Encode/Decode destination functions move from base58 to key_io. --- src/base58.cpp | 98 +------------------ src/base58.h | 38 ------- src/bip38.cpp | 2 +- src/core_write.cpp | 2 +- src/destination_io.cpp | 2 +- src/evo/deterministicmns.cpp | 2 +- src/evo/providertx.cpp | 3 +- src/httprpc.cpp | 2 +- src/key_io.cpp | 94 ++++++++++++++++++ src/key_io.h | 38 +++++++ src/masternode.h | 2 +- src/masternodeman.h | 2 +- src/messagesigner.cpp | 1 - src/pivx-tx.cpp | 1 - src/qt/addresstablemodel.cpp | 2 +- src/qt/bitcoinaddressvalidator.cpp | 2 +- src/qt/paymentserver.cpp | 11 +-- .../pivx/settings/settingsbittoolwidget.cpp | 2 +- .../settings/settingssignmessagewidgets.cpp | 2 +- src/qt/transactionrecord.cpp | 2 +- src/qt/walletmodel.h | 1 + src/rpc/blockchain.cpp | 2 +- src/rpc/budget.cpp | 5 +- src/rpc/mining.cpp | 5 +- src/rpc/misc.cpp | 2 +- src/rpc/rawtransaction.cpp | 1 - src/rpc/server.cpp | 2 +- src/spork.h | 2 +- src/test/bloom_tests.cpp | 2 +- src/test/librust/sapling_rpc_wallet_tests.cpp | 2 +- src/test/librust/sapling_wallet_tests.cpp | 1 - src/test/rpc_tests.cpp | 2 +- src/test/script_P2CS_tests.cpp | 1 - src/wallet/hdchain.cpp | 2 +- src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.h | 2 +- src/wallet/walletdb.cpp | 2 +- 37 files changed, 163 insertions(+), 181 deletions(-) diff --git a/src/base58.cpp b/src/base58.cpp index 51930882266b..91e7685968ec 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -6,11 +6,8 @@ #include "base58.h" #include "hash.h" -#include "script/script.h" -#include "uint256.h" -#include -#include +#include "uint256.h" #include #include @@ -167,96 +164,3 @@ bool DecodeBase58Check(const std::string& str, std::vector& vchRe { return DecodeBase58Check(str.c_str(), vchRet, max_ret); } - -namespace -{ -class DestinationEncoder : public boost::static_visitor -{ -private: - const CChainParams& m_params; - const CChainParams::Base58Type m_addrType; - -public: - DestinationEncoder(const CChainParams& params, const CChainParams::Base58Type _addrType = CChainParams::PUBKEY_ADDRESS) : m_params(params), m_addrType(_addrType) {} - - std::string operator()(const CKeyID& id) const - { - std::vector data = m_params.Base58Prefix(m_addrType); - data.insert(data.end(), id.begin(), id.end()); - return EncodeBase58Check(data); - } - - std::string operator()(const CScriptID& id) const - { - std::vector data = m_params.Base58Prefix(CChainParams::SCRIPT_ADDRESS); - data.insert(data.end(), id.begin(), id.end()); - return EncodeBase58Check(data); - } - - std::string operator()(const CNoDestination& no) const { return ""; } -}; - -CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, bool& isStaking) -{ - std::vector data; - uint160 hash; - if (DecodeBase58Check(str, data)) { - // base58-encoded PIVX addresses. - // Public-key-hash-addresses have version 30 (or 139 testnet). - // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key. - const std::vector& 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); - } - // Public-key-hash-coldstaking-addresses have version 63 (or 73 testnet). - const std::vector& staking_prefix = params.Base58Prefix(CChainParams::STAKING_ADDRESS); - if (data.size() == hash.size() + staking_prefix.size() && std::equal(staking_prefix.begin(), staking_prefix.end(), data.begin())) { - isStaking = true; - std::copy(data.begin() + staking_prefix.size(), data.end(), hash.begin()); - return CKeyID(hash); - } - // Script-hash-addresses have version 13 (or 19 testnet). - // The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script. - const std::vector& 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); - } - } - return CNoDestination(); -} - -} // anon namespace - -std::string EncodeDestination(const CTxDestination& dest, bool isStaking) -{ - return EncodeDestination(dest, isStaking ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS); -} - -std::string EncodeDestination(const CTxDestination& dest, const CChainParams::Base58Type addrType) -{ - return boost::apply_visitor(DestinationEncoder(Params(), addrType), dest); -} - -CTxDestination DecodeDestination(const std::string& str) -{ - bool isStaking; - return DecodeDestination(str, Params(), isStaking); -} - -CTxDestination DecodeDestination(const std::string& str, bool& isStaking) -{ - return DecodeDestination(str, Params(), isStaking); -} - -bool IsValidDestinationString(const std::string& str, bool fStaking, const CChainParams& params) -{ - bool isStaking = false; - return IsValidDestination(DecodeDestination(str, params, isStaking)) && (isStaking == fStaking); -} - -bool IsValidDestinationString(const std::string& str, bool isStaking) -{ - return IsValidDestinationString(str, isStaking, Params()); -} diff --git a/src/base58.h b/src/base58.h index a20dd86ae00a..c44e871f532c 100644 --- a/src/base58.h +++ b/src/base58.h @@ -71,42 +71,4 @@ bool DecodeBase58Check(const char* psz, std::vector& vchRet, int */ bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); -std::string EncodeDestination(const CTxDestination& dest, bool isStaking); -std::string EncodeDestination(const CTxDestination& dest, const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS); -// DecodeDestinationisStaking flag is set to true when the string arg is from an staking address -CTxDestination DecodeDestination(const std::string& str, bool& isStaking); -CTxDestination DecodeDestination(const std::string& str); - -// Return true if the address is valid and is following the fStaking flag type (true means that the destination must be a staking address, false the opposite). -bool IsValidDestinationString(const std::string& str, bool fStaking); -bool IsValidDestinationString(const std::string& str, bool fStaking, const CChainParams& params); - -/** - * Wrapper class for every supported address - */ -struct Destination { -public: - explicit Destination() {} - explicit Destination(const CTxDestination& _dest, bool _isP2CS) : dest(_dest), isP2CS(_isP2CS) {} - - CTxDestination dest{CNoDestination()}; - bool isP2CS{false}; - - Destination& operator=(const Destination& from) - { - this->dest = from.dest; - this->isP2CS = from.isP2CS; - return *this; - } - - std::string ToString() - { - if (!IsValidDestination(dest)) { - // Invalid address - return ""; - } - return EncodeDestination(dest, isP2CS ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS); - } -}; - #endif // BITCOIN_BASE58_H diff --git a/src/bip38.cpp b/src/bip38.cpp index e83f81b1d09f..bf4a5df13cfa 100644 --- a/src/bip38.cpp +++ b/src/bip38.cpp @@ -6,9 +6,9 @@ #include "base58.h" #include "crypto/aes.h" +#include "key_io.h" #include "hash.h" #include "pubkey.h" -#include "util/system.h" #include "utilstrencodings.h" #include "random.h" diff --git a/src/core_write.cpp b/src/core_write.cpp index 041fcc5563cf..5f0588c82fc1 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -5,7 +5,7 @@ #include "core_io.h" -#include "base58.h" +#include "key_io.h" #include "primitives/transaction.h" #include "script/script.h" #include "script/standard.h" diff --git a/src/destination_io.cpp b/src/destination_io.cpp index 9e7c538864f3..f4816f52c59f 100644 --- a/src/destination_io.cpp +++ b/src/destination_io.cpp @@ -3,7 +3,7 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php. #include "destination_io.h" -#include "base58.h" +#include "key_io.h" #include "sapling/key_io_sapling.h" namespace Standard { diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 6d0af44ba473..520d3e3bdce4 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -5,11 +5,11 @@ #include "evo/deterministicmns.h" -#include "base58.h" #include "chainparams.h" #include "consensus/upgrades.h" #include "core_io.h" #include "evo/specialtx.h" +#include "key_io.h" #include "guiinterface.h" #include "masternode.h" // for MasternodeCollateralMinConf #include "masternodeman.h" // for mnodeman (!TODO: remove) diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index f01fa5305bb3..22e11f4caa38 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -5,10 +5,9 @@ #include "evo/providertx.h" -#include "base58.h" #include "core_io.h" #include "evo/deterministicmns.h" -#include "masternode.h" // MN_COLL_AMT +#include "key_io.h" #include "messagesigner.h" #include "evo/specialtx.h" #include "tinyformat.h" diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 6c9d6f25ec43..9992411d8d24 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -5,10 +5,10 @@ #include "httprpc.h" -#include "base58.h" #include "chainparams.h" #include "crypto/hmac_sha256.h" #include "httpserver.h" +#include "key_io.h" #include "rpc/protocol.h" #include "rpc/server.h" #include "random.h" diff --git a/src/key_io.cpp b/src/key_io.cpp index 03b71ecc17fd..3e24c763a4db 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -5,6 +5,7 @@ #include "key_io.h" #include "base58.h" +#include "script/script.h" #include #include @@ -12,6 +13,99 @@ #include #include +namespace +{ + class DestinationEncoder : public boost::static_visitor + { + private: + const CChainParams& m_params; + const CChainParams::Base58Type m_addrType; + + public: + DestinationEncoder(const CChainParams& params, const CChainParams::Base58Type _addrType = CChainParams::PUBKEY_ADDRESS) : m_params(params), m_addrType(_addrType) {} + + std::string operator()(const CKeyID& id) const + { + std::vector data = m_params.Base58Prefix(m_addrType); + data.insert(data.end(), id.begin(), id.end()); + return EncodeBase58Check(data); + } + + std::string operator()(const CScriptID& id) const + { + std::vector data = m_params.Base58Prefix(CChainParams::SCRIPT_ADDRESS); + data.insert(data.end(), id.begin(), id.end()); + return EncodeBase58Check(data); + } + + std::string operator()(const CNoDestination& no) const { return ""; } + }; + + CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, bool& isStaking) + { + std::vector data; + uint160 hash; + if (DecodeBase58Check(str, data)) { + // base58-encoded PIVX addresses. + // Public-key-hash-addresses have version 30 (or 139 testnet). + // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key. + const std::vector& 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); + } + // Public-key-hash-coldstaking-addresses have version 63 (or 73 testnet). + const std::vector& staking_prefix = params.Base58Prefix(CChainParams::STAKING_ADDRESS); + if (data.size() == hash.size() + staking_prefix.size() && std::equal(staking_prefix.begin(), staking_prefix.end(), data.begin())) { + isStaking = true; + std::copy(data.begin() + staking_prefix.size(), data.end(), hash.begin()); + return CKeyID(hash); + } + // Script-hash-addresses have version 13 (or 19 testnet). + // The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script. + const std::vector& 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); + } + } + return CNoDestination(); + } + +} // anon namespace + +std::string EncodeDestination(const CTxDestination& dest, bool isStaking) +{ + return EncodeDestination(dest, isStaking ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS); +} + +std::string EncodeDestination(const CTxDestination& dest, const CChainParams::Base58Type addrType) +{ + return boost::apply_visitor(DestinationEncoder(Params(), addrType), dest); +} + +CTxDestination DecodeDestination(const std::string& str) +{ + bool isStaking; + return DecodeDestination(str, Params(), isStaking); +} + +CTxDestination DecodeDestination(const std::string& str, bool& isStaking) +{ + return DecodeDestination(str, Params(), isStaking); +} + +bool IsValidDestinationString(const std::string& str, bool fStaking, const CChainParams& params) +{ + bool isStaking = false; + return IsValidDestination(DecodeDestination(str, params, isStaking)) && (isStaking == fStaking); +} + +bool IsValidDestinationString(const std::string& str, bool isStaking) +{ + return IsValidDestinationString(str, isStaking, Params()); +} + namespace KeyIO { CKey DecodeSecret(const std::string &str) { diff --git a/src/key_io.h b/src/key_io.h index 803c761eb915..5ded92522a7c 100644 --- a/src/key_io.h +++ b/src/key_io.h @@ -13,6 +13,16 @@ #include +std::string EncodeDestination(const CTxDestination& dest, bool isStaking); +std::string EncodeDestination(const CTxDestination& dest, const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS); +// DecodeDestinationisStaking flag is set to true when the string arg is from an staking address +CTxDestination DecodeDestination(const std::string& str, bool& isStaking); +CTxDestination DecodeDestination(const std::string& str); + +// Return true if the address is valid and is following the fStaking flag type (true means that the destination must be a staking address, false the opposite). +bool IsValidDestinationString(const std::string& str, bool fStaking); +bool IsValidDestinationString(const std::string& str, bool fStaking, const CChainParams& params); + namespace KeyIO { CKey DecodeSecret(const std::string &str); @@ -28,4 +38,32 @@ namespace KeyIO { } +/** + * Wrapper class for every supported address + */ +struct Destination { +public: + explicit Destination() {} + explicit Destination(const CTxDestination& _dest, bool _isP2CS) : dest(_dest), isP2CS(_isP2CS) {} + + CTxDestination dest{CNoDestination()}; + bool isP2CS{false}; + + Destination& operator=(const Destination& from) + { + this->dest = from.dest; + this->isP2CS = from.isP2CS; + return *this; + } + + std::string ToString() + { + if (!IsValidDestination(dest)) { + // Invalid address + return ""; + } + return EncodeDestination(dest, isP2CS ? CChainParams::STAKING_ADDRESS : CChainParams::PUBKEY_ADDRESS); + } +}; + #endif //PIVX_KEY_IO_H diff --git a/src/masternode.h b/src/masternode.h index 33977f3bedd5..55fc24b4d6bd 100644 --- a/src/masternode.h +++ b/src/masternode.h @@ -6,7 +6,7 @@ #ifndef MASTERNODE_H #define MASTERNODE_H -#include "base58.h" +#include "key_io.h" #include "key.h" #include "messagesigner.h" #include "net.h" diff --git a/src/masternodeman.h b/src/masternodeman.h index 9bc24a98428d..cec03ac478a0 100644 --- a/src/masternodeman.h +++ b/src/masternodeman.h @@ -7,9 +7,9 @@ #define MASTERNODEMAN_H #include "activemasternode.h" -#include "base58.h" #include "cyclingvector.h" #include "key.h" +#include "key_io.h" #include "masternode.h" #include "net.h" #include "sync.h" diff --git a/src/messagesigner.cpp b/src/messagesigner.cpp index b542c21aef54..bf6acdd94c39 100644 --- a/src/messagesigner.cpp +++ b/src/messagesigner.cpp @@ -3,7 +3,6 @@ // Distributed under the MIT/X11 software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "base58.h" #include "hash.h" #include "key_io.h" #include "messagesigner.h" diff --git a/src/pivx-tx.cpp b/src/pivx-tx.cpp index 3d32e324bc62..93ccb7ba17dd 100644 --- a/src/pivx-tx.cpp +++ b/src/pivx-tx.cpp @@ -3,7 +3,6 @@ // Distributed under the MIT/X11 software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "base58.h" #include "clientversion.h" #include "coins.h" #include "core_io.h" diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index f2967d519d50..01472ba72e14 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -10,7 +10,7 @@ #include "guiutil.h" #include "walletmodel.h" -#include "base58.h" +#include "key_io.h" #include "wallet/wallet.h" #include "askpassphrasedialog.h" diff --git a/src/qt/bitcoinaddressvalidator.cpp b/src/qt/bitcoinaddressvalidator.cpp index c771333ea306..5023d3a26d75 100644 --- a/src/qt/bitcoinaddressvalidator.cpp +++ b/src/qt/bitcoinaddressvalidator.cpp @@ -6,7 +6,7 @@ #include "bitcoinaddressvalidator.h" -#include "base58.h" +#include "key_io.h" /* Base58 characters are: "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index a9554e7c5b30..d4f51b316582 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -6,28 +6,19 @@ #include "paymentserver.h" -#include "bitcoinunits.h" #include "guiutil.h" #include "optionsmodel.h" -#include "policy/policy.h" -#include "base58.h" +#include "key_io.h" #include "chainparams.h" #include "guiinterface.h" #include "util/system.h" -#include "wallet/wallet.h" - -#include #include #include #include #include -#include -#include #include -#include -#include #include #include #include diff --git a/src/qt/pivx/settings/settingsbittoolwidget.cpp b/src/qt/pivx/settings/settingsbittoolwidget.cpp index 6b537f5fb14e..c9870fed90a5 100644 --- a/src/qt/pivx/settings/settingsbittoolwidget.cpp +++ b/src/qt/pivx/settings/settingsbittoolwidget.cpp @@ -9,9 +9,9 @@ #include "guiutil.h" #include "walletmodel.h" -#include "base58.h" #include "bip38.h" #include "init.h" +#include "key_io.h" #include "wallet/wallet.h" #include "askpassphrasedialog.h" diff --git a/src/qt/pivx/settings/settingssignmessagewidgets.cpp b/src/qt/pivx/settings/settingssignmessagewidgets.cpp index bc3413a83999..37bbd0be0194 100644 --- a/src/qt/pivx/settings/settingssignmessagewidgets.cpp +++ b/src/qt/pivx/settings/settingssignmessagewidgets.cpp @@ -8,7 +8,7 @@ #include "guiutil.h" #include "walletmodel.h" -#include "base58.h" +#include "key_io.h" #include "init.h" #include "wallet/wallet.h" #include "askpassphrasedialog.h" diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 6ef501c243fc..60ea2b62f362 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -6,7 +6,7 @@ #include "transactionrecord.h" -#include "base58.h" +#include "key_io.h" #include "sapling/key_io_sapling.h" #include "wallet/wallet.h" diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 2cdcec7575d2..ada0a107703f 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -14,6 +14,7 @@ #include "interfaces/wallet.h" #include "key.h" +#include "key_io.h" #include "operationresult.h" #include "support/allocators/zeroafterfree.h" #include "pairresult.h" diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index b59919cd5240..08fbb73efc62 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -5,13 +5,13 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. -#include "base58.h" #include "budget/budgetmanager.h" #include "checkpoints.h" #include "clientversion.h" #include "core_io.h" #include "consensus/upgrades.h" #include "kernel.h" +#include "key_io.h" #include "masternodeman.h" #include "policy/feerate.h" #include "policy/policy.h" diff --git a/src/rpc/budget.cpp b/src/rpc/budget.cpp index 4746367655eb..12d30c07a6c2 100644 --- a/src/rpc/budget.cpp +++ b/src/rpc/budget.cpp @@ -5,10 +5,11 @@ #include "activemasternode.h" #include "chainparams.h" -#include "db.h" -#include "init.h" #include "budget/budgetmanager.h" +#include "db.h" #include "evo/deterministicmns.h" +#include "init.h" +#include "key_io.h" #include "masternode-payments.h" #include "masternode-sync.h" #include "masternodeconfig.h" diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 28c0a62bf053..0015cfb0dfd6 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -6,14 +6,13 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php. #include "amount.h" -#include "base58.h" #include "blockassembler.h" #include "chainparams.h" #include "core_io.h" #include "init.h" +#include "key_io.h" #include "miner.h" #include "net.h" -#include "pow.h" #include "rpc/server.h" #include "validationinterface.h" #ifdef ENABLE_WALLET @@ -22,8 +21,6 @@ #include "wallet/wallet.h" #endif -#include - #include #ifdef ENABLE_WALLET diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 0e498e62a62e..88e1daf5a2d9 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -5,10 +5,10 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "base58.h" #include "clientversion.h" #include "httpserver.h" #include "init.h" +#include "key_io.h" #include "sapling/key_io_sapling.h" #include "masternode-sync.h" #include "net.h" diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 61f51aba206f..470bb9b6dd0b 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -5,7 +5,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "base58.h" #include "core_io.h" #include "evo/specialtx.h" #include "evo/providertx.h" diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 0a6a24451cbf..d94729028512 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -7,9 +7,9 @@ #include "rpc/server.h" -#include "base58.h" #include "fs.h" #include "init.h" +#include "key_io.h" #include "random.h" #include "sync.h" #include "guiinterface.h" diff --git a/src/spork.h b/src/spork.h index 814261e6daa2..2142948830e1 100644 --- a/src/spork.h +++ b/src/spork.h @@ -6,9 +6,9 @@ #ifndef SPORK_H #define SPORK_H -#include "base58.h" #include "hash.h" #include "key.h" +#include "key_io.h" #include "messagesigner.h" #include "net.h" #include "sporkid.h" diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index dc6c5e67cdf6..36dd0d4f4f6f 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -5,9 +5,9 @@ #include "bloom.h" -#include "base58.h" #include "clientversion.h" #include "key.h" +#include "key_io.h" #include "merkleblock.h" #include "random.h" #include "serialize.h" diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp index b56ad0b32b6f..6f40f8f95b45 100644 --- a/src/test/librust/sapling_rpc_wallet_tests.cpp +++ b/src/test/librust/sapling_rpc_wallet_tests.cpp @@ -9,9 +9,9 @@ #include "test/librust/utiltest.h" #include "rpc/server.h" -#include "rpc/client.h" #include "core_io.h" +#include "key_io.h" #include "consensus/merkle.h" #include "wallet/wallet.h" diff --git a/src/test/librust/sapling_wallet_tests.cpp b/src/test/librust/sapling_wallet_tests.cpp index 36c8681a94bf..915adda079a6 100644 --- a/src/test/librust/sapling_wallet_tests.cpp +++ b/src/test/librust/sapling_wallet_tests.cpp @@ -7,7 +7,6 @@ #include -#include "base58.h" #include "chainparams.h" #include "key_io.h" #include "validation.h" diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 9a5cb364098f..213cd7f67ee4 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -6,7 +6,7 @@ #include "rpc/server.h" #include "rpc/client.h" -#include "base58.h" +#include "key_io.h" #include "netbase.h" #include "util/system.h" diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index fa055db4b03a..b424f4a16dff 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -1,7 +1,6 @@ // Copyright (c) 2020 The PIVX developers // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. -#include "base58.h" #include "key.h" #include "key_io.h" #include "policy/policy.h" diff --git a/src/wallet/hdchain.cpp b/src/wallet/hdchain.cpp index 642fb8217e9a..b79884329b16 100644 --- a/src/wallet/hdchain.cpp +++ b/src/wallet/hdchain.cpp @@ -4,8 +4,8 @@ #include "wallet/hdchain.h" -#include "base58.h" #include "chainparams.h" +#include "key_io.h" #include "tinyformat.h" #include "util/system.h" #include "utilstrencodings.h" diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2e60550fc04a..bf2678f7c544 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -9,12 +9,12 @@ #include "addressbook.h" #include "amount.h" -#include "base58.h" #include "coincontrol.h" #include "core_io.h" #include "destination_io.h" #include "httpserver.h" #include "key_io.h" +#include "key_io.h" #include "masternode-sync.h" #include "net.h" #include "policy/feerate.h" diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7c8ab44df549..5b3c9fd6a22d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -10,13 +10,13 @@ #include "addressbook.h" #include "amount.h" -#include "base58.h" #include "consensus/tx_verify.h" #include "consensus/validation.h" #include "crypter.h" #include "destination_io.h" #include "kernel.h" #include "key.h" +#include "key_io.h" #include "keystore.h" #include "pairresult.h" #include "policy/feerate.h" diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c164b5b81fb8..e14f10995b8d 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -9,7 +9,7 @@ #include "fs.h" -#include "base58.h" +#include "key_io.h" #include "protocol.h" #include "reverse_iterate.h" #include "sapling/key_io_sapling.h" From 9d481beb39bcb4056b83f8cdb9f663ce3e51ab0b Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 18 Nov 2019 15:26:55 -0800 Subject: [PATCH 09/12] Add bounds checks in key_io before DecodeBase58Check --- src/base58.cpp | 4 ++-- src/base58.h | 11 +++++------ src/bench/base58.cpp | 2 +- src/key_io.cpp | 8 ++++---- src/test/base58_tests.cpp | 8 ++++---- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/base58.cpp b/src/base58.cpp index 91e7685968ec..a4b4e4e6afea 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -69,10 +69,10 @@ bool DecodeBase58(const char* psz, std::vector& vch, int max_ret_ return true; } -std::string DecodeBase58(const char* psz) +std::string DecodeBase58(const char* psz, int max_ret_len) { std::vector vch; - DecodeBase58(psz, vch); + DecodeBase58(psz, vch, max_ret_len); std::stringstream ss; ss << std::hex; diff --git a/src/base58.h b/src/base58.h index c44e871f532c..201a1e8b42be 100644 --- a/src/base58.h +++ b/src/base58.h @@ -20,7 +20,6 @@ #include "pubkey.h" #include "script/standard.h" -#include #include #include @@ -40,19 +39,19 @@ std::string EncodeBase58(const std::vector& vch); * return true if decoding is successful. * psz cannot be NULL. */ -bool DecodeBase58(const char* psz, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +bool DecodeBase58(const char* psz, std::vector& vchRet, int max_ret_len); /** * Decode a base58-encoded string (psz) into a string. * psz cannot be NULL. */ -std::string DecodeBase58(const char* psz); +std::string DecodeBase58(const char* psz, int max_ret_len = std::numeric_limits::max()); /** * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ -bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len); /** * Encode a byte vector into a base58-encoded string, including checksum @@ -63,12 +62,12 @@ std::string EncodeBase58Check(const std::vector& vchIn); * Decode a base58-encoded string (psz) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len); /** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len = std::numeric_limits::max()); +bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len); #endif // BITCOIN_BASE58_H diff --git a/src/bench/base58.cpp b/src/bench/base58.cpp index b213b41281ea..c107ece5aaf1 100644 --- a/src/bench/base58.cpp +++ b/src/bench/base58.cpp @@ -47,7 +47,7 @@ static void Base58Decode(benchmark::State& state) const char* addr = "D6ytFXbsnEh7Y8NKgo84KA5yD8Uk4YnBdE"; std::vector vch; while (state.KeepRunning()) { - DecodeBase58(addr, vch); + (void) DecodeBase58(addr, vch, 64); } } diff --git a/src/key_io.cpp b/src/key_io.cpp index 3e24c763a4db..e0b881743bf5 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -45,7 +45,7 @@ namespace { std::vector data; uint160 hash; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 21)) { // base58-encoded PIVX addresses. // Public-key-hash-addresses have version 30 (or 139 testnet). // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key. @@ -111,7 +111,7 @@ namespace KeyIO { CKey DecodeSecret(const std::string &str) { CKey key; std::vector data; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 34)) { const std::vector &privkey_prefix = Params().Base58Prefix(CChainParams::SECRET_KEY); if ((data.size() == 32 + privkey_prefix.size() || (data.size() == 33 + privkey_prefix.size() && data.back() == 1)) && @@ -141,7 +141,7 @@ namespace KeyIO { CExtKey DecodeExtKey(const std::string &str) { CExtKey key; std::vector data; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 78)) { const std::vector &prefix = Params().Base58Prefix(CChainParams::EXT_SECRET_KEY); if (data.size() == BIP32_EXTKEY_SIZE + prefix.size() && std::equal(prefix.begin(), prefix.end(), data.begin())) { @@ -165,7 +165,7 @@ namespace KeyIO { { CExtPubKey key; std::vector data; - if (DecodeBase58Check(str, data)) { + if (DecodeBase58Check(str, data, 78)) { const std::vector& prefix = Params().Base58Prefix(CChainParams::EXT_PUBLIC_KEY); if (data.size() == BIP32_EXTKEY_SIZE + prefix.size() && std::equal(prefix.begin(), prefix.end(), data.begin())) { key.Decode(data.data() + prefix.size()); diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 38b20e3374d1..7846d03cffb4 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -60,15 +60,15 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) } std::vector expected = ParseHex(test[0].get_str()); std::string base58string = test[1].get_str(); - BOOST_CHECK_MESSAGE(DecodeBase58(base58string, result), strTest); + BOOST_CHECK_MESSAGE(DecodeBase58(base58string, result, 256), strTest); BOOST_CHECK_MESSAGE(result.size() == expected.size() && std::equal(result.begin(), result.end(), expected.begin()), strTest); } - BOOST_CHECK(!DecodeBase58("invalid", result)); + BOOST_CHECK(!DecodeBase58("invalid", result, 100)); // check that DecodeBase58 skips whitespace, but still fails with unexpected non-whitespace at the end. - BOOST_CHECK(!DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t a", result)); - BOOST_CHECK( DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t ", result)); + BOOST_CHECK(!DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t a", result, 3)); + BOOST_CHECK( DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t ", result, 3)); std::vector expected = ParseHex("971a55"); BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); } From 70c480cc59e471efad6fd3073c1a477e7d12b161 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Wed, 11 Dec 2019 10:56:34 +0000 Subject: [PATCH 10/12] tests: Add tests for base58-decoding of std::string:s containing non-base58 characters --- src/test/base58_tests.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 7846d03cffb4..24dea9087be1 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -65,12 +65,24 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) } BOOST_CHECK(!DecodeBase58("invalid", result, 100)); + BOOST_CHECK(!DecodeBase58(std::string("invalid"), result, 100)); + BOOST_CHECK(!DecodeBase58(std::string("\0invalid", 8), result, 100)); + + BOOST_CHECK(DecodeBase58(std::string("good", 4), result, 100)); + BOOST_CHECK(!DecodeBase58(std::string("bad0IOl", 7), result, 100)); + BOOST_CHECK(!DecodeBase58(std::string("goodbad0IOl", 11), result, 100)); + BOOST_CHECK(!DecodeBase58(std::string("good\0bad0IOl", 12), result, 100)); // check that DecodeBase58 skips whitespace, but still fails with unexpected non-whitespace at the end. BOOST_CHECK(!DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t a", result, 3)); BOOST_CHECK( DecodeBase58(" \t\n\v\f\r skip \r\f\v\n\t ", result, 3)); std::vector expected = ParseHex("971a55"); BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); + + BOOST_CHECK(DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh", 21), result, 100)); + BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oi", 21), result, 100)); + BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh0IOl", 25), result, 100)); + BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh\00IOl", 26), result, 100)); } // Visitor to check address type From 0247f6f57e7d2aa455ab6504d9b6eb7b2a5146e8 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Wed, 11 Dec 2019 11:00:52 +0000 Subject: [PATCH 11/12] util: Don't allow base58-decoding of std::string:s containing non-base58 characters --- src/base58.cpp | 7 +++++++ src/util/string.h | 9 +++++++++ src/utilstrencodings.cpp | 3 ++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/base58.cpp b/src/base58.cpp index a4b4e4e6afea..422694cbc71a 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -6,6 +6,7 @@ #include "base58.h" #include "hash.h" +#include "util/string.h" #include "uint256.h" @@ -131,6 +132,9 @@ std::string EncodeBase58(const std::vector& vch) bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len) { + if (!ValidAsCString(str)) { + return false; + } return DecodeBase58(str.c_str(), vchRet, max_ret_len); } @@ -162,5 +166,8 @@ bool DecodeBase58Check(const char* psz, std::vector& vchRet, int bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret) { + if (!ValidAsCString(str)) { + return false; + } return DecodeBase58Check(str.c_str(), vchRet, max_ret); } diff --git a/src/util/string.h b/src/util/string.h index dec0c19b0829..d07cb04af0ad 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_UTIL_STRING_H #define BITCOIN_UTIL_STRING_H +#include #include #include #include @@ -32,4 +33,12 @@ inline std::string Join(const std::vector& list, const std::string& return Join(list, separator, [](const std::string& i) { return i; }); } +/** + * Check if a string does not contain any embedded NUL (\0) characters + */ +inline bool ValidAsCString(const std::string& str) noexcept +{ + return str.size() == strlen(str.c_str()); +} + #endif // BITCOIN_UTIL_STRENCODINGS_H diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 20d1714dd850..a07fd4e4d644 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -5,6 +5,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "utilstrencodings.h" +#include "util/string.h" #include "tinyformat.h" @@ -266,7 +267,7 @@ static bool ParsePrechecks(const std::string& str) return false; if (str.size() >= 1 && (isspace(str[0]) || isspace(str[str.size()-1]))) // No padding allowed return false; - if (str.size() != strlen(str.c_str())) // No embedded NUL characters allowed + if (!ValidAsCString(str)) // No embedded NUL characters allowed return false; return true; } From a470c3421a35251688e9d61807069b5587309868 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Tue, 25 Sep 2018 07:00:36 +0200 Subject: [PATCH 12/12] Backport attributes.h and connect it to base58.h functions only --- src/Makefile.am | 1 + src/attributes.h | 22 ++++++++++++++++++++++ src/base58.h | 9 +++++---- 3 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 src/attributes.h diff --git a/src/Makefile.am b/src/Makefile.am index 3aa9415a0265..a747e373eaa0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -147,6 +147,7 @@ BITCOIN_CORE_H = \ activemasternode.h \ addrdb.h \ addrman.h \ + attributes.h \ arith_uint256.h \ amount.h \ base58.h \ diff --git a/src/attributes.h b/src/attributes.h new file mode 100644 index 000000000000..45099bd8b880 --- /dev/null +++ b/src/attributes.h @@ -0,0 +1,22 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_ATTRIBUTES_H +#define BITCOIN_ATTRIBUTES_H + +#if defined(__has_cpp_attribute) +# if __has_cpp_attribute(nodiscard) +# define NODISCARD [[nodiscard]] +# endif +#endif +#ifndef NODISCARD +# if defined(_MSC_VER) && _MSC_VER >= 1700 +# define NODISCARD _Check_return_ +# else +# define NODISCARD __attribute__((warn_unused_result)) +# endif +#endif + +#endif // BITCOIN_ATTRIBUTES_H diff --git a/src/base58.h b/src/base58.h index 201a1e8b42be..04fc58e01971 100644 --- a/src/base58.h +++ b/src/base58.h @@ -15,6 +15,7 @@ #ifndef BITCOIN_BASE58_H #define BITCOIN_BASE58_H +#include "attributes.h" #include "chainparams.h" #include "key.h" #include "pubkey.h" @@ -39,7 +40,7 @@ std::string EncodeBase58(const std::vector& vch); * return true if decoding is successful. * psz cannot be NULL. */ -bool DecodeBase58(const char* psz, std::vector& vchRet, int max_ret_len); +NODISCARD bool DecodeBase58(const char* psz, std::vector& vchRet, int max_ret_len); /** * Decode a base58-encoded string (psz) into a string. @@ -51,7 +52,7 @@ std::string DecodeBase58(const char* psz, int max_ret_len = std::numeric_limits< * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ -bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len); +NODISCARD bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len); /** * Encode a byte vector into a base58-encoded string, including checksum @@ -62,12 +63,12 @@ std::string EncodeBase58Check(const std::vector& vchIn); * Decode a base58-encoded string (psz) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len); +NODISCARD bool DecodeBase58Check(const char* psz, std::vector& vchRet, int max_ret_len); /** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len); +NODISCARD bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret_len); #endif // BITCOIN_BASE58_H