Skip to content

ref: sd29x9::pow intermediate positive values are strictly < min. neg. value#280

Open
0xNeshi wants to merge 3 commits intorelease-v1.1from
fix/audit-l-01
Open

ref: sd29x9::pow intermediate positive values are strictly < min. neg. value#280
0xNeshi wants to merge 3 commits intorelease-v1.1from
fix/audit-l-01

Conversation

@0xNeshi
Copy link
Copy Markdown
Collaborator

@0xNeshi 0xNeshi commented Mar 30, 2026

Adjusts SD29x9::pow overflow validation to respect the asymmetric signed boundary: positive results must be strictly less than the magnitude reserved for -2^127, while negative results may equal it (to allow min()).

The overflow assertion now uses a local min_negative variable (consistent with wrap_components) and is expressed in the clearer form res_mag < min_negative || (res_neg && res_mag == min_negative).

PR Checklist

  • Tests
  • Documentation
  • Changelog

@0xNeshi 0xNeshi self-assigned this Mar 30, 2026
Copilot AI review requested due to automatic review settings March 30, 2026 14:16
@0xNeshi 0xNeshi linked an issue Mar 30, 2026 that may be closed by this pull request
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (e8dea76) to head (d3aa978).

Files with missing lines Patch % Lines
math/fixed_point/sources/sd29x9/sd29x9_base.move 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release-v1.1     #280      +/-   ##
================================================
- Coverage         89.87%   89.82%   -0.06%     
================================================
  Files                19       19              
  Lines              1787     1788       +1     
  Branches            484      484              
================================================
  Hits               1606     1606              
  Misses              168      168              
- Partials             13       14       +1     
Flag Coverage Δ
contracts/access 44.87% <ø> (ø)
math/core 86.12% <ø> (ø)
math/fixed_point 58.37% <50.00%> (-0.11%) ⬇️

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.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9bfe8ed8-b856-43bb-95f1-575393354bd4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/audit-l-01

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

@0xNeshi 0xNeshi changed the title ref: sd29x9::pow intermediate pos. value must be < min. neg. value ref: sd29x9::pow intermediate positive values are strictly < min. neg. value Mar 30, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts SD29x9::pow overflow validation to respect the asymmetric signed boundary: positive results must be strictly less than the magnitude reserved for -2^127, while negative results may equal it (to allow min()).

Changes:

  • Introduces a local limit for the MIN_NEGATIVE_VALUE magnitude boundary.
  • Updates the per-iteration overflow assertion in pow to enforce < limit for non-negative results and <= limit for negative results.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ericnordelo
Copy link
Copy Markdown
Member

Is this going to be superceded by the binary exponentiation implementation?

@0xNeshi
Copy link
Copy Markdown
Collaborator Author

0xNeshi commented Mar 31, 2026

re: https://github.com/OpenZeppelin/contracts-sui/pull/280/checks?check_run_id=69324773853

Very difficult to find base + exponent that results exactly in sd29x9::min(), so test case doesn't yet exist.

@0xNeshi
Copy link
Copy Markdown
Collaborator Author

0xNeshi commented Mar 31, 2026

Is this going to be superceded by the binary exponentiation implementation?

Not superseded, the PRs solve different problems, responded there

Copy link
Copy Markdown
Member

@ericnordelo ericnordelo left a comment

Choose a reason for hiding this comment

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

LGTM!

Copilot AI review requested due to automatic review settings April 1, 2026 16:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

[L-01]: SD29x9 pow Uses Asymmetric Boundary Validation

5 participants