Normalize approximation native token decimals#4125
Conversation
squadgazzz
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request normalizes price approximations for tokens with different decimals. It also includes an unrelated logic change to preserve the KeepPriceUpdated flag in the cache. This unrelated change should be extracted into a separate pull-request, in line with the guideline to avoid mixing logic changes with refactoring.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces decimal normalization for native price approximations and includes an unrelated bug fix for preserving the KeepPriceUpdated cache flag. The decimal normalization logic appears correct and is well-tested. However, the unrelated bug fix should be moved to a separate pull request to adhere to the single responsibility principle for commits, as per the rule: 'Avoid mixing logic changes with refactoring in the same pull request. Defer logic changes to a separate PR.'
MartinquaXD
left a comment
MartinquaXD
left a comment
There was a problem hiding this comment.
Math checks out.
Only minor nits.
| // 5e-22 * 10^12 = 5e-10 | ||
| assert!((price - 5e-10).abs() < 1e-18); |
There was a problem hiding this comment.
Why don't we have a strict equality assertion here? Is there any loss of precision in the relevant code paths?
There was a problem hiding this comment.
There is precision loss after all: 10f64.powi(-12) isn't exactly representable. Updated the test.
squadgazzz
force-pushed
the
normalize-approx-tokens
branch
from
7da2820 to
941cb88
Compare
3 participants
Description
Instead of validating that native price approximation token pairs have matching decimals at startup, this PR normalizes prices based on the decimal difference between tokens, as was suggested in another PR comment[link].
Changes
ApproximationTokentype that stores both the approximation address and a normalization factor(10^(to_decimals - from_decimals))How to test
Added unit tests for normalization in both directions (source > target decimals and target > source decimals)