doc + test: Correct uint256 hex string endianness

Follow-up to bitcoin#30436.

uint256 string representation was wrongfully documented as little-endian due to them being reversed by GetHex() etc, and base_blob::Compare() giving most significance to the beginning of the internal array. They are closer to "big-endian", but this commit tries to be even more precise than that.

uint256_tests.cpp - Avoid using variable from the left side of the condition in the right side.

setup_common.cpp - Skip needless ArithToUint256-conversion.

Author: hodlinator

src/arith_uint256.h

@@ -196,6 +196,7 @@ class base_uint
         return ret;
     }
 
+    /** Numeric ordering (unlike \ref base_blob::Compare) */
     int CompareTo(const base_uint& b) const;
     bool EqualTo(uint64_t b) const;
 
@@ -218,6 +219,7 @@ class base_uint
     friend inline bool operator==(const base_uint& a, uint64_t b) { return a.EqualTo(b); }
     friend inline bool operator!=(const base_uint& a, uint64_t b) { return !a.EqualTo(b); }
 
+    /** Hex encoding of the number (with the most significant digits first). */
     std::string GetHex() const;
     std::string ToString() const;
 

src/test/arith_uint256_tests.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <arith_uint256.h>
+#include <test/util/setup_common.h>
 #include <uint256.h>
 
 #include <boost/test/unit_test.hpp>
@@ -22,7 +23,8 @@ static inline arith_uint256 arith_uint256V(const std::vector<unsigned char>& vch
 {
     return UintToArith256(uint256(vch));
 }
-static inline arith_uint256 arith_uint256S(const std::string& str) { return UintToArith256(uint256S(str)); }
+// Takes a number written in hex (with most significant digits first).
+static inline arith_uint256 arith_uint256S(std::string_view str) { return UintToArith256(uint256S(str)); }
 
 const unsigned char R1Array[] =
     "\x9c\x52\x4a\xdb\xcf\x56\x11\x12\x2b\x29\x12\x5e\x5d\x35\xd2\xd2"
@@ -104,6 +106,7 @@ BOOST_AUTO_TEST_CASE( basics ) // constructors, equality, inequality
     BOOST_CHECK(arith_uint256S(R1L.ToString()) == R1L);
     BOOST_CHECK(arith_uint256S("   0x" + R1L.ToString() + "   ") == R1L);
     BOOST_CHECK(arith_uint256S("") == ZeroL);
+    BOOST_CHECK(arith_uint256S("1") == OneL);
     BOOST_CHECK(R1L == arith_uint256S(R1ArrayHex));
     BOOST_CHECK(arith_uint256(R1L) == R1L);
     BOOST_CHECK((arith_uint256(R1L^R2L)^R2L) == R1L);
@@ -278,6 +281,12 @@ BOOST_AUTO_TEST_CASE( comparison ) // <= >= < >
         BOOST_CHECK( R1L <= TmpL ); BOOST_CHECK( (R1L == TmpL) != (R1L < TmpL)); BOOST_CHECK( (TmpL == R1L) || !( R1L >= TmpL));
         BOOST_CHECK(! (TmpL < R1L)); BOOST_CHECK(! (R1L > TmpL));
     }
+
+    BOOST_CHECK_LT(ZeroL,
+                   OneL);
+    // Verify hex number representation has the most significant digits first.
+    BOOST_CHECK_LT(arith_uint256S("0000000000000000000000000000000000000000000000000000000000000001"),
+                   arith_uint256S("1000000000000000000000000000000000000000000000000000000000000000"));
 }
 
 BOOST_AUTO_TEST_CASE( plusMinus )

src/test/uint256_tests.cpp

@@ -58,7 +58,7 @@ static std::string ArrayToString(const unsigned char A[], unsigned int width)
     return Stream.str();
 }
 
-// Input is treated as little-endian.
+// Takes hex string in reverse byte order.
 inline uint160 uint160S(std::string_view str)
 {
     uint160 rv;
@@ -99,6 +99,7 @@ BOOST_AUTO_TEST_CASE( basics ) // constructors, equality, inequality
     BOOST_CHECK_EQUAL(uint256S("   0x"+R1L.ToString()+"-trash;%^&   "), R1L);
     BOOST_CHECK_EQUAL(uint256S("\t \n  \n \f\n\r\t\v\t   0x"+R1L.ToString()+"  \t \n  \n \f\n\r\t\v\t "), R1L);
     BOOST_CHECK_EQUAL(uint256S(""), ZeroL);
+    BOOST_CHECK_EQUAL(uint256S("1"), OneL);
     BOOST_CHECK_EQUAL(R1L, uint256S(R1ArrayHex));
     BOOST_CHECK_EQUAL(uint256(R1L), R1L);
     BOOST_CHECK_EQUAL(uint256(ZeroL), ZeroL);
@@ -152,9 +153,15 @@ BOOST_AUTO_TEST_CASE( comparison ) // <= >= < >
     BOOST_CHECK_LT(R1S, MaxS);
     BOOST_CHECK_LT(R2S, MaxS);
 
-    // Verify hex strings are little-endian
-    BOOST_CHECK_LT(uint256S("2000000000000000000000000000000000000000000000000000000000000001"),
-                   uint256S("1000000000000000000000000000000000000000000000000000000000000002"));
+    // Non-arithmetic uint256s compare from the beginning of their inner arrays:
+    BOOST_CHECK_LT(R2L, R1L);
+    // Ensure first element comparisons give the same order as above:
+    BOOST_CHECK_LT(*R2L.begin(), *R1L.begin());
+    // Ensure last element comparisons give a different result (swapped params):
+    BOOST_CHECK_LT(*(R1L.end()-1), *(R2L.end()-1));
+    // Hex strings represent reverse-encoded bytes, with lexicographic ordering:
+    BOOST_CHECK_LT(uint256S("1000000000000000000000000000000000000000000000000000000000000000"),
+                   uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
 }
 
 BOOST_AUTO_TEST_CASE(methods) // GetHex SetHexDeprecated FromHex begin() end() size() GetLow64 GetSerializeSize, Serialize, Unserialize
@@ -172,11 +179,11 @@ BOOST_AUTO_TEST_CASE(methods) // GetHex SetHexDeprecated FromHex begin() end() s
     BOOST_CHECK_EQUAL(uint256::FromHex(ZeroL.ToString()).value(), uint256());
 
     TmpL = uint256::FromHex(R1L.ToString()).value();
-    BOOST_CHECK_EQUAL_COLLECTIONS(R1L.begin(), R1L.end(), R1Array, R1Array + R1L.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(TmpL.begin(), TmpL.end(), R1Array, R1Array + TmpL.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(R2L.begin(), R2L.end(), R2Array, R2Array + R2L.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(ZeroL.begin(), ZeroL.end(), ZeroArray, ZeroArray + ZeroL.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(OneL.begin(), OneL.end(), OneArray, OneArray + OneL.size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(R1L.begin(), R1L.end(), R1Array, R1Array + uint256::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(TmpL.begin(), TmpL.end(), R1Array, R1Array + uint256::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(R2L.begin(), R2L.end(), R2Array, R2Array + uint256::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(ZeroL.begin(), ZeroL.end(), ZeroArray, ZeroArray + uint256::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(OneL.begin(), OneL.end(), OneArray, OneArray + uint256::size());
     BOOST_CHECK_EQUAL(R1L.size(), sizeof(R1L));
     BOOST_CHECK_EQUAL(sizeof(R1L), 32);
     BOOST_CHECK_EQUAL(R1L.size(), 32);
@@ -218,11 +225,11 @@ BOOST_AUTO_TEST_CASE(methods) // GetHex SetHexDeprecated FromHex begin() end() s
     BOOST_CHECK_EQUAL(uint160::FromHex(ZeroS.ToString()).value(), uint160());
 
     TmpS = uint160::FromHex(R1S.ToString()).value();
-    BOOST_CHECK_EQUAL_COLLECTIONS(R1S.begin(), R1S.end(), R1Array, R1Array + R1S.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(TmpS.begin(), TmpS.end(), R1Array, R1Array + TmpS.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(R2S.begin(), R2S.end(), R2Array, R2Array + R2S.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(ZeroS.begin(), ZeroS.end(), ZeroArray, ZeroArray + ZeroS.size());
-    BOOST_CHECK_EQUAL_COLLECTIONS(OneS.begin(), OneS.end(), OneArray, OneArray + OneS.size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(R1S.begin(), R1S.end(), R1Array, R1Array + uint160::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(TmpS.begin(), TmpS.end(), R1Array, R1Array + uint160::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(R2S.begin(), R2S.end(), R2Array, R2Array + uint160::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(ZeroS.begin(), ZeroS.end(), ZeroArray, ZeroArray + uint160::size());
+    BOOST_CHECK_EQUAL_COLLECTIONS(OneS.begin(), OneS.end(), OneArray, OneArray + uint160::size());
     BOOST_CHECK_EQUAL(R1S.size(), sizeof(R1S));
     BOOST_CHECK_EQUAL(sizeof(R1S), 20);
     BOOST_CHECK_EQUAL(R1S.size(), 20);
@@ -307,23 +314,23 @@ BOOST_AUTO_TEST_CASE(parse)
     {
         std::string s_12{"0000000000000000000000000000000000000000000000000000000000000012"};
         BOOST_CHECK_EQUAL(uint256S("12\0").GetHex(), s_12);
-        BOOST_CHECK_EQUAL(uint256S(std::string{"12\0", 3}).GetHex(), s_12);
+        BOOST_CHECK_EQUAL(uint256S(std::string_view{"12\0", 3}).GetHex(), s_12);
         BOOST_CHECK_EQUAL(uint256S("0x12").GetHex(), s_12);
         BOOST_CHECK_EQUAL(uint256S(" 0x12").GetHex(), s_12);
         BOOST_CHECK_EQUAL(uint256S(" 12").GetHex(), s_12);
     }
     {
         std::string s_1{uint256::ONE.GetHex()};
         BOOST_CHECK_EQUAL(uint256S("1\0").GetHex(), s_1);
-        BOOST_CHECK_EQUAL(uint256S(std::string{"1\0", 2}).GetHex(), s_1);
+        BOOST_CHECK_EQUAL(uint256S(std::string_view{"1\0", 2}).GetHex(), s_1);
         BOOST_CHECK_EQUAL(uint256S("0x1").GetHex(), s_1);
         BOOST_CHECK_EQUAL(uint256S(" 0x1").GetHex(), s_1);
         BOOST_CHECK_EQUAL(uint256S(" 1").GetHex(), s_1);
     }
     {
         std::string s_0{uint256::ZERO.GetHex()};
         BOOST_CHECK_EQUAL(uint256S("\0").GetHex(), s_0);
-        BOOST_CHECK_EQUAL(uint256S(std::string{"\0", 1}).GetHex(), s_0);
+        BOOST_CHECK_EQUAL(uint256S(std::string_view{"\0", 1}).GetHex(), s_0);
         BOOST_CHECK_EQUAL(uint256S("0x").GetHex(), s_0);
         BOOST_CHECK_EQUAL(uint256S(" 0x").GetHex(), s_0);
         BOOST_CHECK_EQUAL(uint256S(" ").GetHex(), s_0);

src/test/util/setup_common.cpp

@@ -81,7 +81,7 @@ static FastRandomContext g_insecure_rand_ctx_temp_path;
 
 std::ostream& operator<<(std::ostream& os, const arith_uint256& num)
 {
-    os << ArithToUint256(num).ToString();
+    os << num.ToString();
     return os;
 }
 

src/uint256.h

@@ -53,17 +53,46 @@ class base_blob
         std::fill(m_data.begin(), m_data.end(), 0);
     }
 
+    /** Lexicographic ordering
+     * @note Does NOT match the ordering on the corresponding \ref
+     *       base_uint::CompareTo, which starts comparing from the end.
+     */
     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; }
 
-    // Hex string representations are little-endian.
+    /** @name Hex representation
+     *
+     * The reverse-byte hex representation is a convenient way to view the blob
+     * as a number, because it is consistent with the way the base_uint class
+     * converts blobs to numbers.
+     *
+     * @note base_uint treats the blob as an array of bytes with the numerically
+     * least significant byte first and the most significant byte last. Because
+     * numbers are typically written with the most significant digit first and
+     * the least significant digit last, the reverse hex display of the blob
+     * corresponds to the same numeric value that base_uint interprets from the
+     * blob.
+     * @{*/
     std::string GetHex() const;
-    /** Unlike FromHex this accepts any invalid input, thus it is fragile and deprecated */
+    /** Unlike FromHex this accepts any invalid input, thus it is fragile and deprecated!
+     *
+     * - Hex numbers that don't specify enough bytes to fill the internal array
+     *   will be treated as setting the beginning of it, which corresponds to
+     *   the least significant bytes when converted to base_uint.
+     *
+     * - Hex numbers specifying too many bytes will have the numerically most
+     *   significant bytes (the beginning of the string) narrowed away.
+     *
+     * - An odd count of hex digits will result in the high bits of the leftmost
+     *   byte being zero.
+     *   "0x123" => {0x23, 0x1, 0x0, ..., 0x0}
+     */
     void SetHexDeprecated(std::string_view str);
     std::string ToString() const;
+    /**@}*/
 
     constexpr const unsigned char* data() const { return m_data.data(); }
     constexpr unsigned char* data() { return m_data.data(); }
@@ -93,7 +122,7 @@ class base_blob
 
 namespace detail {
 /**
- * Writes the hex string (treated as little-endian) into a new uintN_t object
+ * Writes the hex string (in reverse byte order) into a new uintN_t object
  * and only returns a value iff all of the checks pass:
  *   - Input length is uintN_t::size()*2
  *   - All characters are hex
@@ -134,7 +163,7 @@ class uint256 : public base_blob<256> {
     static const uint256 ONE;
 };
 
-/* uint256 from std::string_view, treated as little-endian.
+/* uint256 from std::string_view, containing byte-reversed hex encoding.
  * DEPRECATED. Unlike FromHex this accepts any invalid input, thus it is fragile and deprecated!
  */
 inline uint256 uint256S(std::string_view str)