From 44e2edff1da32a73bcb532d45cc32c8ee269a68d Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 16 Sep 2022 10:26:57 +0100 Subject: [PATCH 1/3] Merge bitcoin/bitcoin#26105: Use ReadLE64 in uint256::GetUint64 instead of duplicating logic 04fee75bacb9ec3bceff1246ba6c8ed8a8759548 Use ReadLE64 in uint256::GetUint64() instead of duplicating logic (Pieter Wuille) Pull request description: No need to have a (naive) copy of the `ReadLE64` logic inside `uint256::GetUint64`, when we have an optimized function for exactly that. ACKs for top commit: davidgumberg: ACK 04fee75bacb9ec3bceff1246ba6c8ed8a8759548 jonatack: ACK 04fee75bacb9ec3bceff1246ba6c8ed8a8759548 review, this use of ReadLE64() is similar to the existing invocation by Num3072::Num3072(), sanity checked that before and after this change GetUint64() returns the same result (debug build, clang 13) Tree-SHA512: 0fc2681536a18d82408411bcc6d5c6445fb96793fa43ff4021cd2933d46514c725318da35884f428d1799023921f33f8af091ef428ceb96a50866ac53a345356 --- src/uint256.h | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/uint256.h b/src/uint256.h index 101f617f5e..826ce581aa 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -7,6 +7,8 @@ #ifndef BITCOIN_UINT256_H #define BITCOIN_UINT256_H +#include + #include #include #include @@ -83,15 +85,7 @@ public: uint64_t GetUint64(int pos) const { - const uint8_t* ptr = m_data + pos * 8; - return ((uint64_t)ptr[0]) | \ - ((uint64_t)ptr[1]) << 8 | \ - ((uint64_t)ptr[2]) << 16 | \ - ((uint64_t)ptr[3]) << 24 | \ - ((uint64_t)ptr[4]) << 32 | \ - ((uint64_t)ptr[5]) << 40 | \ - ((uint64_t)ptr[6]) << 48 | \ - ((uint64_t)ptr[7]) << 56; + return ReadLE64(m_data + pos * 8); } template From 5ad6088c93d90f5933d8caa1569eee900ab44a6b Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 6 Feb 2023 13:56:32 -0500 Subject: [PATCH 2/3] Merge bitcoin/bitcoin#26345: refactor: modernize the implementation of uint256.* 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 refactor: modernize the implementation of uint256.* (pasta) Pull request description: - 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 This may also be replaced by std::memset, but I think that std::fill is more idiomatic of modern c++ and readable. - 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) This could also likely be replaced by std::memcpy, but as said above, I believe the using std::copy is the more c++ way to do anything and is almost guaranteed to compile to the same asm - memcmp -> std::memcmp ACKs for top commit: achow101: ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 hebasto: Approach ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329. aureleoules: reACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 john-moffett: ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 stickies-v: Approach ACK 935acdcc7 Tree-SHA512: 4f1ba54ff2198eea0e505d41e73d552c84c60f6878d5c85a94a8ab57f39afc94ef8d79258e7afd01fa84ec2a99f4404bb877eecd671f65e1ee9273f3129fc650 --- src/consensus/params.h | 1 + src/random.h | 1 + src/uint256.cpp | 15 +------ src/uint256.h | 89 ++++++++++++++++++------------------------ 4 files changed, 42 insertions(+), 64 deletions(-) diff --git a/src/consensus/params.h b/src/consensus/params.h index f06f4352de..650133c6d2 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -10,6 +10,7 @@ #include #include +#include namespace Consensus { diff --git a/src/random.h b/src/random.h index 3ddc964165..d5455bcffd 100644 --- a/src/random.h +++ b/src/random.h @@ -13,6 +13,7 @@ #include // For std::chrono::microseconds #include #include +#include /** * Overall design of the RNG and entropy sources. diff --git a/src/uint256.cpp b/src/uint256.cpp index e5d429d645..4043fec8c9 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 826ce581aa..05847da6a6 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -8,12 +8,14 @@ #define BITCOIN_UINT256_H #include +#include -#include +#include +#include +#include #include #include #include -#include /** Template base class for fixed-sized opaque blobs. */ template @@ -21,7 +23,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,75 +33,58 @@ public: /* constructor for constants between 1 and 255 */ constexpr explicit base_blob(uint8_t v) : m_data{v} {} - explicit base_blob(const std::vector& vch); - - bool IsNull() const + constexpr explicit base_blob(Span vch) { - for (int i = 0; i < WIDTH; i++) - if (m_data[i] != 0) - return false; - return true; + assert(vch.size() == WIDTH); + std::copy(vch.begin(), vch.end(), m_data.begin()); } - void SetNull() + constexpr bool IsNull() const { - memset(m_data, 0, sizeof(m_data)); + return std::all_of(m_data.begin(), m_data.end(), [](uint8_t val) { + return val == 0; + }); } - inline int Compare(const base_blob& other) const { return memcmp(m_data, other.m_data, sizeof(m_data)); } + constexpr void SetNull() + { + std::fill(m_data.begin(), m_data.end(), 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 inline bool operator<(const base_blob& a, const base_blob& b) { return a.Compare(b) < 0; } + constexpr int Compare(const base_blob& other) const { return std::memcmp(m_data.data(), other.m_data.data(), WIDTH); } + + 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]; - } - - unsigned int size() const - { - 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 { - s.write((char*)m_data, sizeof(m_data)); + s.write((char*)m_data.data(), sizeof(m_data)); } template void Unserialize(Stream& s) { - s.read((char*)m_data, sizeof(m_data)); + s.read((char*)m_data.data(), sizeof(m_data)); } }; @@ -107,8 +94,8 @@ public: */ 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 +105,9 @@ public: */ 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; static const uint256 TWO; @@ -157,7 +144,7 @@ public: uint256 trim256() const { uint256 result; - memcpy((void*)&result, (void*)m_data, 32); + memcpy((void*)&result, (void*)m_data.data(), 32); return result; } }; From a2c3031dcc60f8305245245e8daf92222ea36e91 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 5 Oct 2023 08:24:16 -0500 Subject: [PATCH 3/3] refactor: extend changes to uint512 --- src/uint256.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uint256.h b/src/uint256.h index 05847da6a6..455238db2d 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -137,9 +137,9 @@ inline uint256 uint256S(const std::string& str) /** 512-bit unsigned big integer. */ class uint512 : public base_blob<512> { public: - uint512() {} - uint512(const base_blob<512>& b) : base_blob<512>(b) {} - explicit uint512(const std::vector& vch) : base_blob<512>(vch) {} + constexpr uint512() = default; + constexpr uint512(const base_blob<512>& b) : base_blob<512>(b) {} + constexpr explicit uint512(Span vch) : base_blob<512>(vch) {} uint256 trim256() const {