From 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 19 Oct 2022 15:16:04 -0500 Subject: [PATCH] refactor: modernize the implementation of uint256.* - Constructors of uint256 to utilize Span instead of requiring a std::vector - converts m_data into a std::array - Prefers using `WIDTH` instead of `sizeof(m_data)` - make all the things constexpr - replace C style functions with c++ equivalents - memset -> std::fill - memcpy -> std::copy Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy) - memcmp -> std::memcmp --- ci/test/06_script_b.sh | 1 + src/consensus/params.h | 1 + src/psbt.h | 2 +- src/random.h | 1 + src/uint256.cpp | 15 ++------ src/uint256.h | 80 +++++++++++++++++------------------------- 6 files changed, 39 insertions(+), 61 deletions(-) diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 46312b50eb..d52ae2228e 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -57,6 +57,7 @@ if [ "${RUN_TIDY}" = "true" ]; then " src/rpc/signmessage.cpp"\ " src/test/fuzz/txorphan.cpp"\ " src/test/fuzz/util/"\ + " src/uint256.cpp"\ " src/util/bip32.cpp"\ " src/util/bytevectorhash.cpp"\ " src/util/check.cpp"\ diff --git a/src/consensus/params.h b/src/consensus/params.h index 7c35222713..3b5eb1034d 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -11,6 +11,7 @@ #include #include #include +#include namespace Consensus { diff --git a/src/psbt.h b/src/psbt.h index b636f0348b..f1f04cccef 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -889,7 +889,7 @@ struct PSBTOutput } else if (key.size() != 33) { throw std::ios_base::failure("Output Taproot BIP32 keypath key is not at 33 bytes"); } - XOnlyPubKey xonly(uint256({key.begin() + 1, key.begin() + 33})); + XOnlyPubKey xonly(uint256(Span(key).last(32))); std::set leaf_hashes; uint64_t value_len = ReadCompactSize(s); size_t before_hashes = s.size(); diff --git a/src/random.h b/src/random.h index 5fe20c5f76..082ccd4047 100644 --- a/src/random.h +++ b/src/random.h @@ -15,6 +15,7 @@ #include #include #include +#include /** * Overall design of the RNG and entropy sources. diff --git a/src/uint256.cpp b/src/uint256.cpp index cd9cbb566a..7f81c3c448 100644 --- a/src/uint256.cpp +++ b/src/uint256.cpp @@ -7,15 +7,6 @@ #include -#include - -template -base_blob::base_blob(const std::vector& vch) -{ - assert(vch.size() == sizeof(m_data)); - memcpy(m_data, vch.data(), sizeof(m_data)); -} - template std::string base_blob::GetHex() const { @@ -29,7 +20,7 @@ std::string base_blob::GetHex() const template void base_blob::SetHex(const char* psz) { - memset(m_data, 0, sizeof(m_data)); + std::fill(m_data.begin(), m_data.end(), 0); // skip leading spaces while (IsSpace(*psz)) @@ -43,7 +34,7 @@ void base_blob::SetHex(const char* psz) size_t digits = 0; while (::HexDigit(psz[digits]) != -1) digits++; - unsigned char* p1 = (unsigned char*)m_data; + unsigned char* p1 = m_data.data(); unsigned char* pend = p1 + WIDTH; while (digits > 0 && p1 < pend) { *p1 = ::HexDigit(psz[--digits]); @@ -67,14 +58,12 @@ std::string base_blob::ToString() const } // Explicit instantiations for base_blob<160> -template base_blob<160>::base_blob(const std::vector&); template std::string base_blob<160>::GetHex() const; template std::string base_blob<160>::ToString() const; template void base_blob<160>::SetHex(const char*); template void base_blob<160>::SetHex(const std::string&); // Explicit instantiations for base_blob<256> -template base_blob<256>::base_blob(const std::vector&); template std::string base_blob<256>::GetHex() const; template std::string base_blob<256>::ToString() const; template void base_blob<256>::SetHex(const char*); diff --git a/src/uint256.h b/src/uint256.h index e74b9ff7b1..782fec8e51 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -9,11 +9,12 @@ #include #include -#include +#include +#include +#include #include #include #include -#include /** Template base class for fixed-sized opaque blobs. */ template @@ -21,7 +22,9 @@ class base_blob { protected: static constexpr int WIDTH = BITS / 8; - uint8_t m_data[WIDTH]; + std::array m_data; + static_assert(WIDTH == sizeof(m_data), "Sanity check"); + public: /* construct 0 value by default */ constexpr base_blob() : m_data() {} @@ -29,64 +32,47 @@ class base_blob /* constructor for constants between 1 and 255 */ constexpr explicit base_blob(uint8_t v) : m_data{v} {} - explicit base_blob(const std::vector& vch); + constexpr explicit base_blob(Span vch) + { + assert(vch.size() == WIDTH); + std::copy(vch.begin(), vch.end(), m_data.begin()); + } - bool IsNull() const + constexpr bool IsNull() const { - for (int i = 0; i < WIDTH; i++) - if (m_data[i] != 0) - return false; - return true; + return std::all_of(m_data.begin(), m_data.end(), [](uint8_t val) { + return val == 0; + }); } - void SetNull() + constexpr void SetNull() { - memset(m_data, 0, sizeof(m_data)); + std::fill(m_data.begin(), m_data.end(), 0); } - inline int Compare(const base_blob& other) const { return memcmp(m_data, other.m_data, sizeof(m_data)); } + constexpr int Compare(const base_blob& other) const { return std::memcmp(m_data.data(), other.m_data.data(), WIDTH); } - friend inline bool operator==(const base_blob& a, const base_blob& b) { return a.Compare(b) == 0; } - friend inline bool operator!=(const base_blob& a, const base_blob& b) { return a.Compare(b) != 0; } - friend inline bool operator<(const base_blob& a, const base_blob& b) { return a.Compare(b) < 0; } + friend constexpr bool operator==(const base_blob& a, const base_blob& b) { return a.Compare(b) == 0; } + friend constexpr bool operator!=(const base_blob& a, const base_blob& b) { return a.Compare(b) != 0; } + friend constexpr bool operator<(const base_blob& a, const base_blob& b) { return a.Compare(b) < 0; } std::string GetHex() const; void SetHex(const char* psz); void SetHex(const std::string& str); std::string ToString() const; - const unsigned char* data() const { return m_data; } - unsigned char* data() { return m_data; } + constexpr const unsigned char* data() const { return m_data.data(); } + constexpr unsigned char* data() { return m_data.data(); } - unsigned char* begin() - { - return &m_data[0]; - } + constexpr unsigned char* begin() { return m_data.data(); } + constexpr unsigned char* end() { return m_data.data() + WIDTH; } - unsigned char* end() - { - return &m_data[WIDTH]; - } + constexpr const unsigned char* begin() const { return m_data.data(); } + constexpr const unsigned char* end() const { return m_data.data() + WIDTH; } - const unsigned char* begin() const - { - return &m_data[0]; - } + static constexpr unsigned int size() { return WIDTH; } - const unsigned char* end() const - { - return &m_data[WIDTH]; - } - - static constexpr unsigned int size() - { - return sizeof(m_data); - } - - uint64_t GetUint64(int pos) const - { - return ReadLE64(m_data + pos * 8); - } + constexpr uint64_t GetUint64(int pos) const { return ReadLE64(m_data.data() + pos * 8); } template void Serialize(Stream& s) const @@ -107,8 +93,8 @@ class base_blob */ class uint160 : public base_blob<160> { public: - constexpr uint160() {} - explicit uint160(const std::vector& vch) : base_blob<160>(vch) {} + constexpr uint160() = default; + constexpr explicit uint160(Span vch) : base_blob<160>(vch) {} }; /** 256-bit opaque blob. @@ -118,9 +104,9 @@ class uint160 : public base_blob<160> { */ class uint256 : public base_blob<256> { public: - constexpr uint256() {} + constexpr uint256() = default; constexpr explicit uint256(uint8_t v) : base_blob<256>(v) {} - explicit uint256(const std::vector& vch) : base_blob<256>(vch) {} + constexpr explicit uint256(Span vch) : base_blob<256>(vch) {} static const uint256 ZERO; static const uint256 ONE; };