[fold] Resolve misc review issues 1

Builds/CMake/RippledCore.cmake

@@ -300,8 +300,10 @@ install (
   DESTINATION include/ripple/protocol)
 install (
   FILES
+    src/ripple/protocol/impl/b58_utils.h
     src/ripple/protocol/impl/STVar.h
     src/ripple/protocol/impl/secp256k1.h
+    src/ripple/protocol/impl/token_errors.h
   DESTINATION include/ripple/protocol/impl)
 
 #[===================================[

src/ripple/protocol/impl/b58_utils.h

@@ -137,7 +137,7 @@ inplace_bigint_div_rem(std::span<uint64_t> numerator, std::uint64_t divisor)
         return {static_cast<std::uint64_t>(d), static_cast<std::uint64_t>(r)};
     };
 
-    std::uint64_t prev_rem;
+    std::uint64_t prev_rem = 0;
     std::size_t const last_index = numerator.size() - 1;
     std::tie(numerator[last_index], prev_rem) =
         div_rem(numerator[last_index], divisor);
@@ -155,6 +155,12 @@ inplace_bigint_div_rem(std::span<uint64_t> numerator, std::uint64_t divisor)
 [[nodiscard]] inline std::array<std::uint8_t, 10>
 b58_10_to_b58_be(std::uint64_t input)
 {
+    constexpr std::uint64_t B_58_10 = 430804206899405824;  // 58^10;
+    if (input >= B_58_10)
+    {
+        LogicError("Input to b58_10_to_b58_be equals or exceeds 58^10.");
+    }
+
     constexpr std::size_t resultSize = 10;
     std::array<std::uint8_t, resultSize> result{};
     int i = 0;
@@ -165,7 +171,6 @@ b58_10_to_b58_be(std::uint64_t input)
         result[resultSize - 1 - i] = rem;
         i += 1;
     }
-    assert(i <= 10);
 
     return result;
 }

src/ripple/protocol/impl/tokens.cpp

@@ -99,8 +99,8 @@ coefficient 9 is represented in base 2 with the coefficients 1,0,0,1. The base
 the final answer, just concatenate those two independent conversions together.
 The base 16 number "93" is the binary number "10010011".
 
-The original algorithm to convert from base 58 to a number to a binary number
-used the
+The original (new reference) algorithm to convert from base 58 to a number to a
+binary number used the
 
 ```
 N = 0;
@@ -120,7 +120,9 @@ The original algorithm in essence converted from base 58 to base 256 (base
 2^8). The new, faster algorithm converts from base 58 to base 58^10 (this is
 fast using the shortcut described above), then from base 58^10 to base 2^64
 (this is slow, and requires multi-precision arithmetic), and then from base 2^64
-to base 2^8 (this is fast, using the shortcut described above).
+to base 2^8 (this is fast, using the shortcut described above). Base 58^10 is
+chosen because it is the largest power of 58 that will fit into a 64-bit
+register.
 
 While it may seem counter-intuitive that converting from base 58 -> base 58^10
 -> base 2^64 -> base 2^8 is faster than directly converting from base 58 -> base
@@ -204,7 +206,7 @@ decodeBase58Token(std::string const& s, TokenType type)
 #ifndef _MSC_VER
     return b58_fast::decodeBase58Token(s, type);
 #else
-    return b58_ref::decodeBase58Token_ms(s, type);
+    return b58_ref::decodeBase58Token(s, type);
 #endif
 }
 
@@ -362,15 +364,14 @@ decodeBase58Token(std::string const& s, TokenType type)
 
 #ifndef _MSC_VER
 // The algorithms use gcc's int128 (fast MS version will have to wait, in the
-// meantime MS falls back to the slow code)
+// meantime MS falls back to the slower reference implementation)
 namespace b58_fast {
 namespace detail {
 B58Result<std::span<std::uint8_t>>
 b256_to_b58(std::span<std::uint8_t const> input, std::span<std::uint8_t> out)
 {
-    // Allocate enough base 58^10 coeff for encoding 38 bytes
+    // Max valid input is 38 bytes:
     // (33 bytes for nodepublic + 1 byte token + 4 bytes checksum)
-    // log(2^(38*8),58^10)) ~= 5.18. So 6 coeff are enough
     if (input.size() > 38)
     {
         return Unexpected(TokenCodecErrc::inputTooLarge);
@@ -393,6 +394,8 @@ b256_to_b58(std::span<std::uint8_t const> input, std::span<std::uint8_t> out)
     auto const input_zeros = count_leading_zeros(input);
     input = input.subspan(input_zeros);
 
+    // Allocate enough base 2^64 coeff for encoding 38 bytes
+    // log(2^(38*8),2^64)) ~= 4.75. So 5 coeff are enough
     std::array<std::uint64_t, 5> base_2_64_coeff_buf{};
     std::span<std::uint64_t> const base_2_64_coeff =
         [&]() -> std::span<std::uint64_t> {
@@ -427,11 +430,13 @@ b256_to_b58(std::span<std::uint8_t const> input, std::span<std::uint8_t> out)
         return std::span(base_2_64_coeff_buf.data(), num_coeff);
     }();
 
+    // Allocate enough base 58^10 coeff for encoding 38 bytes
+    // log(2^(38*8),58^10)) ~= 5.18. So 6 coeff are enough
     std::array<std::uint64_t, 6> base_58_10_coeff{};
     constexpr std::uint64_t B_58_10 = 430804206899405824;  // 58^10;
     std::size_t num_58_10_coeffs = 0;
     std::size_t cur_2_64_end = base_2_64_coeff.size();
-    // compute the base 58 10 coeffs
+    // compute the base 58^10 coeffs
     while (cur_2_64_end > 0)
     {
         base_58_10_coeff[num_58_10_coeffs] =

src/test/basics/base58_test.cpp

@@ -245,8 +245,7 @@ class base58_test : public beast::unit_test::suite
             std::array<std::span<std::uint8_t>, 2> b256Result;
             for (int i = 0; i < 2; ++i)
             {
-                std::span const outBuf{
-                    b58ResultBuf[i].data(), b58ResultBuf[i].size()};
+                std::span const outBuf{b58ResultBuf[i]};
                 if (i == 0)
                 {
                     auto const r =
@@ -408,7 +407,7 @@ class base58_test : public beast::unit_test::suite
         {
             std::array<std::uint8_t, 128> b256DataBuf;
             auto const [tokType, tokSize] = tokenTypeAndSize(i);
-            for (int d = 0; d < 255; ++d)
+            for (int d = 0; d <= 255; ++d)
             {
                 memset(b256DataBuf.data(), d, tokSize);
                 testIt(tokType, std::span(b256DataBuf.data(), tokSize));
@@ -420,8 +419,7 @@ class base58_test : public beast::unit_test::suite
         for (int i = 0; i < iters; ++i)
         {
             std::array<std::uint8_t, 128> b256DataBuf;
-            auto const [tokType, b256Data] = randomB256TestData(
-                std::span(b256DataBuf.data(), b256DataBuf.size()));
+            auto const [tokType, b256Data] = randomB256TestData(b256DataBuf);
             testIt(tokType, b256Data);
         }
     }