Skip to content

log10 implementation and tests - #104

Merged
immrsd merged 7 commits into
mainfrom
feat/log10-impl
Nov 28, 2025
Merged

log10 implementation and tests#104
immrsd merged 7 commits into
mainfrom
feat/log10-impl

Conversation

@immrsd

@immrsd immrsd commented Nov 28, 2025

Copy link
Copy Markdown
Collaborator

Resolves #23

PR Checklist

  • Tests
  • Documentation
  • Changelog

Summary by CodeRabbit

Release Notes

  • New Features

    • Base-10 logarithm (log10) function now available for all unsigned integer types with configurable rounding modes (down, up, nearest).
    • Zero values return 0; exact powers of 10 return precise exponents.
  • Tests

    • Comprehensive test coverage added for log10 across all integer types and rounding scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@immrsd immrsd self-assigned this Nov 28, 2025
@coderabbitai

coderabbitai Bot commented Nov 28, 2025

Copy link
Copy Markdown

Walkthrough

This PR implements base-10 logarithm functionality (log10) across the OpenZeppelin Math library. It introduces core logarithm computation macros with configurable rounding modes, supporting constants, and helper functions in macros.move, then exposes public API wrappers for u8, u16, u32, u64, u128, and u256 integer types. Comprehensive test suites validate correctness across zero inputs, powers of 10, rounding modes, and edge cases.

Changes

Cohort / File(s) Summary
Core macros implementation
math/core/sources/internal/macros.move
Adds log10 macro with configurable RoundingMode for unsigned integers; introduces log10_floor helper (binary-search division by precomputed powers of 10) and round_log10_to_nearest for rounding; adds constants MAX_LOG_10 (77) and TEN_POW_X cascade; implements fast path (u256) and slow path (u512 arithmetic) for large values.
Public API wrappers for integer types
math/core/sources/u8.move
math/core/sources/u16.move
math/core/sources/u32.move
math/core/sources/u64.move
math/core/sources/u128.move
math/core/sources/u256.move
Each module adds a public log10(value: $Int, rounding_mode: RoundingMode): u8 function delegating to macros::log10!; functions positioned after existing log-related utilities and mirror behavior of log2 and log256.
Test coverage for macros and integer types
math/core/tests/macros_tests.move
math/core/tests/u8_tests.move
math/core/tests/u16_tests.move
math/core/tests/u32_tests.move
math/core/tests/u64_tests.move
math/core/tests/u128_tests.move
math/core/tests/u256_tests.move
Comprehensive test suites validate: zero input returns, exact powers of 10 across all rounding modes (down, up, nearest), floor/ceiling/nearest rounding behavior, edge cases near powers of 10, max-value handling, and large-value scenarios exercising both fast and slow execution paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • macros.move: Requires careful verification of the binary-search division logic, TEN_POW_X constant precomputation accuracy, fast/slow path arithmetic (u256 vs. u512), and rounding correctness for boundary cases.
  • Public wrapper functions: Straightforward delegation pattern, but verify consistency across all six integer types.
  • Test coverage: Extensive and structured, but verify no duplication or missing rounding-mode combinations in macro tests.

Possibly related PRs

  • log2 implementation and tests #46: Adds analogous logarithm implementations (e.g., log2) with similar macro patterns and rounding-helper design; log10 mirrors this established architecture.

Suggested reviewers

  • bidzyyys
  • ericnordelo

Poem

🐰 Ten times, ten times, we count the way,
Log base ten through night and day!
Floor and nearest, up we go,
Powers of ten put on a show! ✨
Rounding true from u8 to u256,
Mathematics' magic we traverse!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description partially follows the template by referencing issue #23 and including a PR checklist, but is vague about actual changes made. Expand the description to explain what was implemented, including the macro-based approach, integer type coverage, and rounding modes supported.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: implementation of log10 function and its tests.
Linked Issues check ✅ Passed The PR implements a complete log10 function following the Solidity Math Library reference, covering all required integer types and rounding modes with comprehensive tests.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the log10 feature: internal macros, public API functions across integer types, and corresponding test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/log10-impl

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a7cddd and 8a8f9ca.

📒 Files selected for processing (14)
  • math/core/sources/internal/macros.move (4 hunks)
  • math/core/sources/u128.move (1 hunks)
  • math/core/sources/u16.move (1 hunks)
  • math/core/sources/u256.move (1 hunks)
  • math/core/sources/u32.move (1 hunks)
  • math/core/sources/u64.move (1 hunks)
  • math/core/sources/u8.move (1 hunks)
  • math/core/tests/macros_tests.move (1 hunks)
  • math/core/tests/u128_tests.move (1 hunks)
  • math/core/tests/u16_tests.move (1 hunks)
  • math/core/tests/u256_tests.move (1 hunks)
  • math/core/tests/u32_tests.move (1 hunks)
  • math/core/tests/u64_tests.move (1 hunks)
  • math/core/tests/u8_tests.move (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-04T14:38:12.309Z
Learnt from: ericnordelo
Repo: OpenZeppelin/contracts-sui PR: 40
File: math/core/sources/u8.move:50-57
Timestamp: 2025-11-04T14:38:12.309Z
Learning: In math/core/sources/{u8,u16,u32,u64,u128}.move files, the mul_shr implementations correctly ignore the overflow flag from macros::mul_shr! and rely on try_as_*() to detect when the result doesn't fit in the target type. Only u256.move needs to explicitly check the overflow flag because u256 operations can exceed u256 capacity.
<!-- [/add_learning]

Applied to files:

  • math/core/sources/u8.move
  • math/core/tests/u8_tests.move
  • math/core/tests/u64_tests.move
  • math/core/tests/u16_tests.move
  • math/core/sources/u256.move
  • math/core/sources/u128.move
  • math/core/sources/u64.move
  • math/core/tests/u128_tests.move
  • math/core/tests/u256_tests.move
  • math/core/sources/u16.move
  • math/core/tests/macros_tests.move
  • math/core/sources/u32.move
  • math/core/sources/internal/macros.move
  • math/core/tests/u32_tests.move
📚 Learning: 2025-11-04T14:37:40.142Z
Learnt from: ericnordelo
Repo: OpenZeppelin/contracts-sui PR: 40
File: math/core/sources/u16.move:49-56
Timestamp: 2025-11-04T14:37:40.142Z
Learning: In the mul_shr! macro (and similar macros like mul_div!), the overflow flag indicates whether the result overflows u256, not the target type. For smaller types like u8, u16, u32, u64, and u128, the overflow flag will never be true because intermediate calculations always fit in u256. The try_as_uX() methods handle cases where the u256 result doesn't fit in the target type.

Applied to files:

  • math/core/sources/internal/macros.move
📚 Learning: 2025-11-04T14:39:02.711Z
Learnt from: ericnordelo
Repo: OpenZeppelin/contracts-sui PR: 40
File: math/core/sources/u64.move:49-56
Timestamp: 2025-11-04T14:39:02.711Z
Learning: In mul_shr implementations for integer types smaller than u256 (u8, u16, u32, u64, u128), the overflow flag from macros::mul_shr! indicates u256 overflow, which is mathematically impossible for these smaller types. The pattern `let (_, result) = macros::mul_shr!(...); result.try_as_X()` is correct, as try_as_X() handles the conversion to Option<X> and returns None when the result doesn't fit in the target type. Only u256::mul_shr needs to explicitly check the overflow flag.

Applied to files:

  • math/core/sources/internal/macros.move
📚 Learning: 2025-11-04T14:38:12.309Z
Learnt from: ericnordelo
Repo: OpenZeppelin/contracts-sui PR: 40
File: math/core/sources/u8.move:50-57
Timestamp: 2025-11-04T14:38:12.309Z
Learning: In the OpenZeppelin Sui contracts math library, the `mul_shr!` and `mul_div!` macros return (overflow, result) tuples where overflow indicates the result cannot fit in u256. For integer types smaller than u256 (u8, u16, u32, u64, u128), the overflow flag can be safely ignored because the product of two values of these types will always fit in u256. The `try_as_*()` methods on the result handle the case where the u256 result doesn't fit in the target type by returning None.
<!-- [/add_learning]

Applied to files:

  • math/core/sources/internal/macros.move
🔇 Additional comments (16)
math/core/sources/u32.move (1)

90-95: u32::log10 wrapper matches existing patterns and types

The log10 wrapper’s signature, zero-handling doc comment, and delegation to macros::log10! are consistent with the existing log2/log256 APIs, and u8 is an appropriate return width for u32 logs.

math/core/tests/u32_tests.move (1)

527-629: u32::log10 tests thoroughly exercise rounding and edge cases

The new tests cover zero, powers of 10, near-boundary values (around 10 and 10⁶), and u32::max_value!() across all rounding modes, with expectations matching the actual base‑10 logarithm behavior.

math/core/tests/u128_tests.move (1)

537-640: u128::log10 test suite matches expected math and range

Zero, powers of 10 up to 10³⁸, large edge cases near 10²⁰, and u128::max_value!() are correctly validated for Down/Up/Nearest rounding, providing solid coverage for both normal and wide-path ranges.

math/core/sources/u16.move (1)

90-95: u16::log10 wrapper is consistent and correctly typed

The new log10 function cleanly delegates to macros::log10!, uses u8 as an appropriate result type for u16 logs, and follows the established documentation and layout conventions.

math/core/sources/u64.move (1)

90-95: u64::log10 wrapper cleanly extends the numeric API

The added log10 function matches the existing log API shape, delegates to the shared macro, and uses a safe u8 result type for the u64 domain.

math/core/tests/u16_tests.move (1)

510-613: u16::log10 tests give good coverage of domain and rounding

The tests accurately cover zero, in-range powers of 10, near-boundary values, and u16::max_value!() across all rounding modes, and the expected results align with the actual base‑10 logarithm semantics.

math/core/sources/u256.move (1)

84-89: u256::log10 wrapper correctly exposes macro implementation

The u256 log10 API is well-typed, follows the existing pattern (including zero behavior), and cleanly routes to macros::log10!, aligning with the intended shared implementation.

math/core/sources/u8.move (1)

91-96: u8::log10 wrapper is straightforward and consistent

The new log10 function for u8 mirrors the existing log APIs, correctly delegates to the shared macro, and is appropriately typed.

math/core/sources/u128.move (1)

90-95: LGTM!

The log10 function follows the established pattern for logarithm functions in this module. The delegation to macros::log10! is consistent with log2 and log256, and the return type u8 is appropriate since log10(u128::MAX) ≈ 38.5.

math/core/tests/u8_tests.move (1)

484-587: LGTM!

Comprehensive test coverage for u8::log10 including:

  • Zero handling across all rounding modes
  • Exact powers of 10 (1, 10, 100)
  • Rounding behavior with correct midpoint calculations (√10 ≈ 3.162, 10×√10 ≈ 31.62)
  • Edge cases near power-of-10 boundaries
  • Max value (255) behavior
math/core/tests/u64_tests.move (1)

536-638: LGTM!

Thorough test coverage for u64::log10 including:

  • Zero handling and exact powers of 10 up to 10^16
  • Rounding modes with correct expected values
  • Edge cases near 10^1 and 10^12 boundaries
  • Max value (18446744073709551615) correctly yields log10 ≈ 19.266
math/core/tests/u256_tests.move (1)

596-715: LGTM!

Excellent test coverage for u256::log10 including:

  • Both the fast path (floor_log ≤ 38, value ≤ u128::MAX) and slow path (u512 arithmetic)
  • Edge cases around the 10^38 threshold where paths diverge
  • Large value tests (10^77 + 1) that exercise 512-bit arithmetic
  • Max value (u256::MAX) behavior with log10 ≈ 77.064
math/core/tests/macros_tests.move (1)

834-1003: LGTM!

Comprehensive macro-level test coverage for log10 including:

  • All unsigned integer types (u8 through u256)
  • Zero handling, exact powers of 10, and all rounding modes
  • Detailed midpoint tests with correct thresholds (√10 ≈ 3.162, 10×√10 ≈ 31.62, etc.)
  • Max value tests for each type with accurate expected log10 values
  • Large u256 values exercising both the u256 fast path and u512 slow path
math/core/sources/internal/macros.move (3)

343-374: LGTM!

The log10 macro implementation is well-structured:

  • Correctly handles zero input by returning 0
  • Uses log10_floor for efficient floor computation via binary search
  • Properly detects exact powers of 10 using std::u256::pow(10, floor_result)
  • Delegates nearest-rounding logic to round_log10_to_nearest

The pattern mirrors the existing log2 and log256 implementations while accommodating the base-10 specific requirements.


376-426: LGTM!

The constants and log10_floor function are well-designed:

  • MAX_LOG_10 = 77 is correct since log10(2^256-1) ≈ 77.06
  • The cascading TEN_POW_* constants efficiently build powers of 10 at compile time
  • The binary search algorithm in log10_floor mirrors the approach used in OpenZeppelin's Solidity implementation, checking progressively smaller powers (64, 32, 16, 8, 4, 2, 1)

754-816: LGTM!

The round_log10_to_nearest implementation is mathematically sound:

  • The early return at floor_log >= MAX_LOG_10 correctly handles the boundary case
  • The fast path condition (floor_log <= 38 && value <= u128::MAX) ensures both value² and 10^(2*floor_log+1) fit in u256
  • The slow path correctly uses u512 arithmetic, computing 10^(2*floor_log+1) as (10 * 10^floor_log) * 10^floor_log to avoid intermediate overflow
  • The threshold computation in the slow path is safe because floor_log is at most 76 when reaching this code, so 10 * 10^76 = 10^77 fits in u256

The documentation clearly explains the mathematical basis for the squaring approach to avoid floating-point √10.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Nov 28, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.87755% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.35%. Comparing base (7a7cddd) to head (8a8f9ca).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
math/core/sources/internal/macros.move 94.44% 0 Missing and 2 partials ⚠️
math/core/sources/u128.move 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #104      +/-   ##
==========================================
+ Coverage   94.92%   95.35%   +0.42%     
==========================================
  Files          13       13              
  Lines        1104     1227     +123     
  Branches      303      350      +47     
==========================================
+ Hits         1048     1170     +122     
+ Misses         41       40       -1     
- Partials       15       17       +2     
Flag Coverage Δ
contracts/access 53.50% <0.00%> (ø)
math/core 94.84% <93.87%> (+0.54%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bidzyyys bidzyyys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @immrsd 👏
Left one theoretical question, the decision is up to you.

Comment thread math/core/sources/u128.move
Comment thread math/core/sources/internal/macros.move
@immrsd
immrsd merged commit e61daa9 into main Nov 28, 2025
11 checks passed
@bidzyyys
bidzyyys deleted the feat/log10-impl branch November 28, 2025 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: log10 function

2 participants