diff --git a/barretenberg/cpp/scripts/audit/audit_scopes/numeric_audit_scope.md b/barretenberg/cpp/scripts/audit/audit_scopes/numeric_audit_scope.md index 808e8cf90836..7a1f1bba84b8 100644 --- a/barretenberg/cpp/scripts/audit/audit_scopes/numeric_audit_scope.md +++ b/barretenberg/cpp/scripts/audit/audit_scopes/numeric_audit_scope.md @@ -12,22 +12,23 @@ Note: Paths relative to `aztec-packages/barretenberg/cpp/src/barretenberg` 3. `numeric/bitop/keep_n_lsb.hpp` 4. `numeric/bitop/pow.hpp` 5. `numeric/bitop/rotate.hpp` +6. `numeric/bitop/sparse_form.hpp` ### Random Number Generation -6. `numeric/random/engine.cpp` -7. `numeric/random/engine.hpp` +7. `numeric/random/engine.cpp` +8. `numeric/random/engine.hpp` ### Unsigned Integer Types -8. `numeric/uint128/uint128.hpp` -9. `numeric/uint128/uint128_impl.hpp` -10. `numeric/uint256/uint256.hpp` -11. `numeric/uint256/uint256_impl.hpp` -12. `numeric/uintx/uintx.cpp` -13. `numeric/uintx/uintx.hpp` -14. `numeric/uintx/uintx_impl.hpp` +9. `numeric/uint128/uint128.hpp` +10. `numeric/uint128/uint128_impl.hpp` +11. `numeric/uint256/uint256.hpp` +12. `numeric/uint256/uint256_impl.hpp` +13. `numeric/uintx/uintx.cpp` +14. `numeric/uintx/uintx.hpp` +15. `numeric/uintx/uintx_impl.hpp` ### General Utilities -15. `numeric/general/general.hpp` +16. `numeric/general/general.hpp` ## Summary of Module diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/count_leading_zeros.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/count_leading_zeros.hpp index a371c0692f83..236dc1ca19e3 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/count_leading_zeros.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/count_leading_zeros.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -35,7 +35,10 @@ template <> constexpr inline size_t count_leading_zeros(uint128_t con return static_cast(__builtin_clzll(hi)); } auto lo = static_cast(u); - return static_cast(__builtin_clzll(lo)) + 64; + if (lo != 0U) { + return static_cast(__builtin_clzll(lo)) + 64; + } + return 128; } template <> constexpr inline size_t count_leading_zeros(uint256_t const& u) diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp index a18e43975645..6815dc2a5e67 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.hpp @@ -1,13 +1,15 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== #pragma once #include +#include #include #include +#include namespace bb::numeric { // from http://supertech.csail.mit.edu/papers/debruijn.pdf @@ -72,8 +74,17 @@ template constexpr inline T get_lsb(const T in) template constexpr inline T round_up_power_2(const T in) { + if (in == 0) { + return 0; + } auto lower_bound = T(1) << get_msb(in); - return (lower_bound == in || lower_bound == 1) ? in : lower_bound * 2; + if (lower_bound == in) { + return in; + } + // Overflow check: lower_bound is the highest power of 2 <= in, + // so lower_bound * 2 would overflow if lower_bound is already the top bit. + assert(lower_bound <= (std::numeric_limits::max() >> 1)); + return lower_bound * 2; } } // namespace bb::numeric \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.test.cpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.test.cpp index 3abdd73b155e..3b8c420c4f7d 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.test.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/get_msb.test.cpp @@ -33,3 +33,97 @@ TEST(bitop, GetMsbSizeT7) auto r = numeric::get_msb(a); EXPECT_EQ(r, 7U); } + +// Verify De Bruijn lookup tables by testing every bit position with multiple input patterns +TEST(bitop, GetMsbUint32AllPositions) +{ + for (uint32_t i = 0; i < 32; i++) { + // Power of 2: exactly one bit set + EXPECT_EQ(numeric::get_msb(uint32_t(1U << i)), i); + // All bits set up to position i (exercises the post-smearing pattern) + uint32_t all_ones = (i == 31) ? 0xFFFFFFFF : ((1U << (i + 1)) - 1); + EXPECT_EQ(numeric::get_msb(all_ones), i); + // MSB set plus random low bit + if (i > 0) { + EXPECT_EQ(numeric::get_msb(uint32_t((1U << i) | 1U)), i); + } + } +} + +TEST(bitop, GetMsbUint64AllPositions) +{ + for (uint64_t i = 0; i < 64; i++) { + // Power of 2 + EXPECT_EQ(numeric::get_msb(uint64_t(1ULL << i)), i); + // All bits set up to position i + uint64_t all_ones = (i == 63) ? 0xFFFFFFFFFFFFFFFFULL : ((1ULL << (i + 1)) - 1); + EXPECT_EQ(numeric::get_msb(all_ones), i); + // MSB set plus low bit + if (i > 0) { + EXPECT_EQ(numeric::get_msb(uint64_t((1ULL << i) | 1ULL)), i); + } + } +} + +// get_lsb tests +TEST(bitop, GetLsbZero) +{ + EXPECT_EQ(numeric::get_lsb(uint32_t(0)), 0U); + EXPECT_EQ(numeric::get_lsb(uint64_t(0)), 0U); +} + +TEST(bitop, GetLsbOne) +{ + EXPECT_EQ(numeric::get_lsb(uint32_t(1)), 0U); + EXPECT_EQ(numeric::get_lsb(uint64_t(1)), 0U); +} + +TEST(bitop, GetLsbPowersOfTwo) +{ + for (uint32_t i = 0; i < 32; i++) { + EXPECT_EQ(numeric::get_lsb(uint32_t(1U << i)), i); + } + for (uint64_t i = 0; i < 64; i++) { + EXPECT_EQ(numeric::get_lsb(uint64_t(1ULL << i)), i); + } +} + +TEST(bitop, GetLsbComposite) +{ + // LSB of 0b1100 is bit 2 + EXPECT_EQ(numeric::get_lsb(uint32_t(0b1100)), 2U); + // LSB of 0xFF00 is bit 8 + EXPECT_EQ(numeric::get_lsb(uint64_t(0xFF00)), 8U); +} + +// round_up_power_2 tests +TEST(bitop, RoundUpPower2Zero) +{ + EXPECT_EQ(numeric::round_up_power_2(uint32_t(0)), 0U); + EXPECT_EQ(numeric::round_up_power_2(uint64_t(0)), 0ULL); +} + +TEST(bitop, RoundUpPower2PowersOfTwo) +{ + // Powers of two should be returned unchanged + for (uint32_t i = 0; i < 31; i++) { + uint32_t val = 1U << i; + EXPECT_EQ(numeric::round_up_power_2(val), val); + } +} + +TEST(bitop, RoundUpPower2NonPowers) +{ + EXPECT_EQ(numeric::round_up_power_2(uint32_t(3)), 4U); + EXPECT_EQ(numeric::round_up_power_2(uint32_t(5)), 8U); + EXPECT_EQ(numeric::round_up_power_2(uint32_t(7)), 8U); + EXPECT_EQ(numeric::round_up_power_2(uint32_t(9)), 16U); + EXPECT_EQ(numeric::round_up_power_2(uint32_t(100)), 128U); + EXPECT_EQ(numeric::round_up_power_2(uint64_t(1000)), 1024ULL); +} + +TEST(bitop, RoundUpPower2LargestValid) +{ + // Largest non-power-of-2 that doesn't overflow: 2^30 + 1 -> 2^31 + EXPECT_EQ(numeric::round_up_power_2(uint32_t((1U << 30) + 1)), 1U << 31); +} diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/keep_n_lsb.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/keep_n_lsb.hpp index e3fe0434fa3e..9bfa6689ac00 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/keep_n_lsb.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/keep_n_lsb.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/pow.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/pow.hpp index 38f06712b7c2..ea6816fd196e 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/pow.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/pow.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/rotate.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/rotate.hpp index dea0d0e0ca82..cc7075d21b8b 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/rotate.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/rotate.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/bitop/sparse_form.hpp b/barretenberg/cpp/src/barretenberg/numeric/bitop/sparse_form.hpp index 1bb271fe510a..8ae55f3bd6b6 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/bitop/sparse_form.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/bitop/sparse_form.hpp @@ -5,6 +5,7 @@ // ===================== #pragma once +#include "barretenberg/common/assert.hpp" #include "barretenberg/common/throw_or_abort.hpp" #include #include @@ -15,8 +16,13 @@ namespace bb::numeric { +/** + * @brief Decompose a uint256_t into digits in the given base (least-significant digit first). + * If num_slices > 0, returns exactly that many digits. If num_slices == 0, returns as many as needed. + */ inline std::vector slice_input(const uint256_t& input, const uint64_t base, const size_t num_slices) { + BB_ASSERT(base > 0); uint256_t target = input; std::vector slices; if (num_slices > 0) { @@ -33,12 +39,17 @@ inline std::vector slice_input(const uint256_t& input, const uint64_t return slices; } +/** + * @brief Decompose a uint256_t using a different base for each digit position (least-significant first). + * Throws if the input is too large to be fully represented by the given bases. + */ inline std::vector slice_input_using_variable_bases(const uint256_t& input, const std::vector& bases) { uint256_t target = input; std::vector slices; for (size_t i = 0; i < bases.size(); ++i) { + BB_ASSERT(bases[i] > 0); if (target >= bases[i] && i == bases.size() - 1) { throw_or_abort(format("Last key slice greater than ", bases[i])); } @@ -48,6 +59,9 @@ inline std::vector slice_input_using_variable_bases(const uint256_t& i return slices; } +/** + * @brief Compute [1, base, base^2, ..., base^(num_slices-1)] as uint256_t values. + */ template constexpr std::array get_base_powers() { std::array output{}; @@ -58,6 +72,11 @@ template constexpr std::array constexpr uint256_t map_into_sparse_form(const uint64_t input) { uint256_t out = 0UL; @@ -73,6 +92,11 @@ template constexpr uint256_t map_into_sparse_form(const uint64_t return out; } +/** + * @brief Decode a sparse-form uint256_t back to a 32-bit value. + * Extracts the base-adic digits from most-significant to least-significant, and recovers the original + * binary value by reading the low bit of each digit. + */ template constexpr uint64_t map_from_sparse_form(const uint256_t& input) { uint256_t target = input; @@ -102,6 +126,12 @@ template constexpr uint64_t map_from_sparse_form(const uint256_t return output; } +/** + * @brief Integer type that stores each bit as a separate digit in the given base. + * Supports addition with single-pass carry propagation. Used to build plookup tables + * for bitwise operations (XOR, AND) where two sparse_ints are added and the resulting + * per-digit values encode the operation's truth table. + */ template class sparse_int { public: sparse_int(const uint64_t input = 0) @@ -118,6 +148,8 @@ template class sparse_int { sparse_int& operator=(sparse_int&& other) noexcept = default; ~sparse_int() noexcept = default; + // Single-pass carry propagation: correct when all input limbs are < base, which is guaranteed + // by the constructor (limbs are 0 or 1) and maintained by this operator (carry produces values < base). sparse_int operator+(const sparse_int& other) const { sparse_int result(*this); @@ -126,6 +158,9 @@ template class sparse_int { if (result.limbs[i] >= base) { result.limbs[i] -= base; ++result.limbs[i + 1]; + // After carry: result.limbs[i] < base (since both inputs were < base, sum < 2*base, + // so subtracting base gives a value < base). The carry of 1 into limbs[i+1] cannot + // cascade because limbs[i+1] hasn't been added to other.limbs[i+1] yet. } } result.limbs[num_bits - 1] += other.limbs[num_bits - 1]; diff --git a/barretenberg/cpp/src/barretenberg/numeric/general/general.hpp b/barretenberg/cpp/src/barretenberg/numeric/general/general.hpp index b147ce6f1182..df44e75deb48 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/general/general.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/general/general.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp b/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp index 6d30a12a75e6..8a86f9fc75a7 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/random/engine.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/random/engine.hpp b/barretenberg/cpp/src/barretenberg/numeric/random/engine.hpp index d2e3e8b0b821..2ef64d5bceb9 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/random/engine.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/random/engine.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp index 17c79af7eb1d..ad2762d16f87 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -37,12 +37,15 @@ class alignas(32) uint128_t { return { static_cast(a), static_cast(a >> 32), 0, 0 }; } - constexpr explicit operator uint64_t() { return (static_cast(data[1]) << 32) + data[0]; } + constexpr explicit operator uint64_t() const { return (static_cast(data[1]) << 32) + data[0]; } constexpr uint128_t& operator=(const uint128_t& other) = default; constexpr uint128_t& operator=(uint128_t&& other) = default; constexpr ~uint128_t() = default; - explicit constexpr operator bool() const { return static_cast(data[0]); }; + explicit constexpr operator bool() const + { + return (data[0] != 0) || (data[1] != 0) || (data[2] != 0) || (data[3] != 0); + }; template explicit constexpr operator T() const { return static_cast(data[0]); }; diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128_impl.hpp b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128_impl.hpp index 590c67ef6a5d..2d5214b63f8d 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128_impl.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp index 156407981dfa..f679dfd1c4b5 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -109,9 +109,12 @@ class alignas(32) uint256_t { constexpr uint256_t& operator=(uint256_t&& other) noexcept = default; constexpr ~uint256_t() noexcept = default; - explicit constexpr operator bool() const { return static_cast(data[0]); }; + explicit constexpr operator bool() const + { + return (data[0] != 0) || (data[1] != 0) || (data[2] != 0) || (data[3] != 0); + }; - constexpr explicit operator uint128_t() { return (static_cast(data[1]) << 64) + data[0]; } + constexpr explicit operator uint128_t() const { return (static_cast(data[1]) << 64) + data[0]; } template explicit constexpr operator T() const { return static_cast(data[0]); }; [[nodiscard]] constexpr bool get_bit(uint64_t bit_index) const; diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.test.cpp b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.test.cpp index ae72524449cd..710d3e0d83f6 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.test.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.test.cpp @@ -336,3 +336,148 @@ TEST(uint256, ToFromBuffer) auto b = from_buffer(buf); EXPECT_EQ(a, b); } + +// operator bool: verify all limbs are checked (not just data[0]) +TEST(uint256, BoolConversion) +{ + EXPECT_FALSE(static_cast(uint256_t(0))); + EXPECT_TRUE(static_cast(uint256_t(1))); + // These cases caught the old bug where only data[0] was checked + EXPECT_TRUE(static_cast(uint256_t{ 0, 1, 0, 0 })); + EXPECT_TRUE(static_cast(uint256_t{ 0, 0, 1, 0 })); + EXPECT_TRUE(static_cast(uint256_t{ 0, 0, 0, 1 })); +} + +// operator uint128_t: verify both lower limbs are preserved +TEST(uint256, Uint128Conversion) +{ + constexpr uint256_t a{ 0xaaaaaaaaaaaaaaaa, 0xbbbbbbbbbbbbbbbb, 0xcccccccccccccccc, 0xdddddddddddddddd }; + auto lo128 = static_cast(a); + EXPECT_EQ(static_cast(lo128), 0xaaaaaaaaaaaaaaaa); + EXPECT_EQ(static_cast(lo128 >> 64), 0xbbbbbbbbbbbbbbbb); + + // Verify const objects use the correct overload (not the integral template) + const uint256_t b{ 0x1111111111111111, 0x2222222222222222, 0, 0 }; + auto b128 = static_cast(b); + EXPECT_EQ(static_cast(b128 >> 64), 0x2222222222222222); +} + +// Addition with carry propagation across all limbs +TEST(uint256, AddCarryPropagation) +{ + // Carry from limb 0 to limb 1 + uint256_t a{ UINT64_MAX, 0, 0, 0 }; + uint256_t b{ 1, 0, 0, 0 }; + uint256_t c = a + b; + EXPECT_EQ(c, (uint256_t{ 0, 1, 0, 0 })); + + // Carry propagates through all limbs + uint256_t d{ UINT64_MAX, UINT64_MAX, UINT64_MAX, 0 }; + uint256_t e = d + uint256_t(1); + EXPECT_EQ(e, (uint256_t{ 0, 0, 0, 1 })); + + // Full overflow wraps to zero + uint256_t f{ UINT64_MAX, UINT64_MAX, UINT64_MAX, UINT64_MAX }; + uint256_t g = f + uint256_t(1); + EXPECT_EQ(g, uint256_t(0)); +} + +// mul_extended: verify full 512-bit product is correct +TEST(uint256, MulExtended) +{ + // Simple case: known product + uint256_t a{ 0, 0, 0, 1 }; // 2^192 + uint256_t b{ 0, 0, 0, 1 }; // 2^192 + auto [lo, hi] = a.mul_extended(b); + // 2^192 * 2^192 = 2^384, which is hi.data[2] bit 0 + EXPECT_EQ(lo, uint256_t(0)); + EXPECT_EQ(hi, (uint256_t{ 0, 0, 1, 0 })); + + // Verify with random values + a = engine.get_random_uint256(); + b = engine.get_random_uint256(); + auto [ab_lo, ab_hi] = a.mul_extended(b); + + // Truncated product should match low half + EXPECT_EQ(a * b, ab_lo); + + // Verify commutativity + auto [ba_lo, ba_hi] = b.mul_extended(a); + EXPECT_EQ(ab_lo, ba_lo); + EXPECT_EQ(ab_hi, ba_hi); + + // Verify hi is zero when inputs are small + uint256_t small_a{ 0xFFFFFFFF, 0, 0, 0 }; + uint256_t small_b{ 0xFFFFFFFF, 0, 0, 0 }; + auto [sm_lo, sm_hi] = small_a.mul_extended(small_b); + EXPECT_EQ(sm_hi, uint256_t(0)); + EXPECT_EQ(sm_lo, small_a * small_b); +} + +// Single-limb divmod +TEST(uint256, DivModSingleLimb) +{ + for (size_t i = 0; i < 64; ++i) { + uint256_t a = engine.get_random_uint256(); + uint64_t b = engine.get_random_uint256().data[0]; + if (b == 0) { + b = 1; + } + auto [q, r] = a.divmod(b); + // Verify roundtrip: q * b + r == a + uint256_t reconstructed = q * uint256_t(b) + uint256_t(r); + EXPECT_EQ(reconstructed, a); + // Remainder must be less than divisor + EXPECT_LT(r, b); + } +} + +// slice +TEST(uint256, Slice) +{ + constexpr uint256_t a{ 0xaaaaaaaaaaaaaaaa, 0xbbbbbbbbbbbbbbbb, 0xcccccccccccccccc, 0xdddddddddddddddd }; + + // Slice bottom 64 bits + uint256_t bottom = a.slice(0, 64); + EXPECT_EQ(bottom, uint256_t(0xaaaaaaaaaaaaaaaa)); + + // Slice bits [64, 128) + uint256_t mid = a.slice(64, 128); + EXPECT_EQ(mid, uint256_t(0xbbbbbbbbbbbbbbbb)); + + // Slice across limb boundary [32, 96) + uint256_t cross = a.slice(32, 96); + uint256_t expected = (a >> 32) & ((uint256_t(1) << 64) - 1); + EXPECT_EQ(cross, expected); + + // Full slice + uint256_t full = a.slice(0, 256); + EXPECT_EQ(full, a); +} + +// pow +TEST(uint256, Pow) +{ + // x^0 = 1 + uint256_t a{ 12345, 0, 0, 0 }; + EXPECT_EQ(a.pow(uint256_t(0)), uint256_t(1)); + + // x^1 = x + EXPECT_EQ(a.pow(uint256_t(1)), a); + + // 0^n = 0 for n > 0 + EXPECT_EQ(uint256_t(0).pow(uint256_t(5)), uint256_t(0)); + + // 2^10 = 1024 + EXPECT_EQ(uint256_t(2).pow(uint256_t(10)), uint256_t(1024)); + + // 3^20 = 3486784401 + EXPECT_EQ(uint256_t(3).pow(uint256_t(20)), uint256_t(3486784401ULL)); + + // Verify a^2 == a * a for random value + uint256_t b = engine.get_random_uint256(); + EXPECT_EQ(b.pow(uint256_t(2)), b * b); + + // Verify a^3 == a * a * a + EXPECT_EQ(b.pow(uint256_t(3)), b * b * b); +} diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256_impl.hpp b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256_impl.hpp index 4a399d984797..4434db6f69a0 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256_impl.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.cpp b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.cpp index 9026c2237dc3..aa6081b6fd14 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -12,12 +12,4 @@ namespace bb::numeric { template class uintx; template class uintx; -// NOTE: this instantiation is only used to maintain a 1024 barrett reduction test. -// The simpler route would have been to delete that test as this modulus is otherwise not used, but this was more -// conservative. -constexpr uint512_t TEST_MODULUS(uint256_t{ "0x04689e957a1242c84a50189c6d96cadca602072d09eac1013b5458a2275d69b1" }, - uint256_t{ "0x0925c4b8763cbf9c599a6f7c0348d21cb00b85511637560626edfa5c34c6b38d" }); - -template std::pair uintx::barrett_reduction() const; - } // namespace bb::numeric diff --git a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp index 5c9068c8e0c9..5a42fb866fb8 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp @@ -1,19 +1,18 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== /** - * uintx - * Copyright Aztec 2020 + * uintx: a wide unsigned integer formed by pairing two `base_uint` halves (lo, hi). * - * An unsigned 512 bit integer type. + * Instantiated as: + * uintx = uint512_t (two 256-bit halves) + * uintx = uint1024_t (two 512-bit halves) * - * Constructor and all methods are constexpr. Ideally, uintx should be able to be treated like any other literal - *type. - * - * Not optimized for performance, this code doesn"t touch any of our hot paths when constructing PLONK proofs + * Constructor and all methods are constexpr. + * Not optimized for performance; does not touch hot paths when constructing proofs. **/ #pragma once @@ -57,7 +56,7 @@ template class uintx { uintx& operator=(uintx&& other) noexcept = default; ~uintx() = default; - constexpr explicit operator bool() const { return static_cast(lo); }; + constexpr explicit operator bool() const { return static_cast(lo) || static_cast(hi); }; constexpr explicit operator uint8_t() const { return static_cast(lo); }; constexpr explicit operator uint16_t() const { return static_cast(lo); }; constexpr explicit operator uint32_t() const { return static_cast(lo); }; @@ -82,7 +81,7 @@ template class uintx { constexpr uintx slice(const uint64_t start, const uint64_t end) const { const uint64_t range = end - start; - const uintx mask = range == base_uint::length() ? -uintx(1) : (uintx(1) << range) - 1; + const uintx mask = (uintx(1) << range) - 1; return ((*this) >> start) & mask; } diff --git a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp index ee88020b2e76..bbbb85696b8d 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp @@ -1,10 +1,18 @@ #include "./uintx.hpp" #include "../random/engine.hpp" +#include "./uintx_impl.hpp" #include "barretenberg/numeric/uint256/uint256.hpp" #include using namespace bb; +// Explicit instantiation of barrett_reduction for the test-only 1024-bit modulus. +namespace bb::numeric { +constexpr uint512_t TEST_MODULUS(uint256_t{ "0x04689e957a1242c84a50189c6d96cadca602072d09eac1013b5458a2275d69b1" }, + uint256_t{ "0x0925c4b8763cbf9c599a6f7c0348d21cb00b85511637560626edfa5c34c6b38d" }); +template std::pair uintx::barrett_reduction() const; +} // namespace bb::numeric + namespace { auto& engine = numeric::get_debug_randomness(); } // namespace @@ -294,6 +302,37 @@ TEST(uintx, DISABLEDRInv) */ } +TEST(uintx, Slice) +{ + // Construct a uint512_t with known lo and hi halves + uint256_t lo_val(1, 2, 3, 4); + uint256_t hi_val(5, 6, 7, 8); + uint512_t val(lo_val, hi_val); + + // Slice the lower 256 bits + uint512_t lower = val.slice(0, 256); + EXPECT_EQ(lower.lo, lo_val); + EXPECT_EQ(lower.hi, uint256_t(0)); + + // Slice the upper 256 bits + uint512_t upper = val.slice(256, 512); + EXPECT_EQ(upper.lo, hi_val); + EXPECT_EQ(upper.hi, uint256_t(0)); + + // Slice a sub-limb range within lo + uint512_t small_slice = val.slice(0, 64); + EXPECT_EQ(static_cast(small_slice), uint64_t(1)); + + // Slice crossing the lo/hi boundary + uint512_t cross_boundary = val.slice(128, 384); + // Should get bits [128..256) from lo (limbs 2,3) and bits [256..384) from hi (limbs 0,1) + uint512_t expected_cross(uint256_t(3, 4, 5, 6)); + EXPECT_EQ(cross_boundary, expected_cross); + + // Full-width slice should return the original value + EXPECT_EQ(val.slice(0, 512), val); +} + TEST(uintx, BarrettReductionRegression) { // Test specific modulus and self values that may cause issues diff --git a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp index 0af4869a56a3..77327e096f07 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: Planned, auditors: [], commit: } +// internal: { status: Complete, auditors: [Luke], commit: } // external_1: { status: not started, auditors: [], commit: } // external_2: { status: not started, auditors: [], commit: } // ===================== @@ -303,9 +303,9 @@ std::pair, uintx> uintx::barrett_reductio } uintx remainder = x - qm_lo; - // because redc_parameter is an imperfect representation of 2^{2k} / n (might be too small), - // the computed quotient may be off by up to 4 (classic algorithm should be up to 1, - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1051): investigate, why) + // The quotient estimate can be off by up to 4. Classic Barrett guarantees at most 1 correction + // when k = ceil(log2(modulus)) and x < modulus^2. Here k = base_uint::length() - 1 (a fixed, + // conservative choice), so x / 2^{2k} can be up to 3, giving an error bound of 4. size_t i = 0; while (remainder >= uintx(modulus)) { BB_ASSERT_LT(i, 4U);