Skip to content

Widen ulp tolerance#397

Merged
grod220 merged 2 commits into
mainfrom
widen-ulp-tolerance
May 27, 2026
Merged

Widen ulp tolerance#397
grod220 merged 2 commits into
mainfrom
widen-ulp-tolerance

Conversation

@grod220
Copy link
Copy Markdown
Member

@grod220 grod220 commented May 26, 2026

During audit was provided a case where the ULP diff was > 4. Recommended widening to 10:

With 5 axioms on f64's behavior, I've formally proven, assuming the triple multiply does not overflow and no division by 0 occurs, that a tolerance of 10*ulp is maintained regardless of the account_portion, cluster_effective, or rate_bps in all scenarios.

Practically speaking, this means the integer and float implementations (not including highly unlikely overflow cases) do not differ more than 1280 lamports.

No impact to product code, just tests.

@grod220 grod220 requested review from 2501babe and joncinque May 26, 2026 20:51
@grod220 grod220 force-pushed the widen-ulp-tolerance branch from 55ae2d3 to a3e5f3b Compare May 26, 2026 22:02
Copy link
Copy Markdown
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Makes sense! Can you add a worst-case test where it's possible to get close to the maximum ULP? Or something that fails on 9?

@grod220
Copy link
Copy Markdown
Member Author

grod220 commented May 26, 2026

Can you add a worst-case test where it's possible to get close to the maximum ULP?

Was not able to generate an example larger than 5 (proves the current allowance is too small). Added that test.

Or something that fails on 9?

Anything 10 and lower will now pass. The audit provided a proof it is not possible higher.

@grod220 grod220 requested a review from joncinque May 26, 2026 23:29
@grod220 grod220 enabled auto-merge (squash) May 26, 2026 23:29
Copy link
Copy Markdown
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Ah ok, I thought they had an example for 9. This works though!

@grod220 grod220 merged commit 25edff5 into main May 27, 2026
25 checks passed
@grod220 grod220 deleted the widen-ulp-tolerance branch May 27, 2026 13:54
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.

2 participants