Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Precision of invariant() is inconsistent #8

Open
robertleifke opened this issue Jun 29, 2024 · 0 comments · Fixed by #14
Open

Precision of invariant() is inconsistent #8

robertleifke opened this issue Jun 29, 2024 · 0 comments · Fixed by #14
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation
Milestone

Comments

@robertleifke
Copy link
Member

robertleifke commented Jun 29, 2024

Description:

The invariant function of the Pair.sol contract does not function as intended due to a mismatch in fixed-point precision.
As shown in figure 6.1, the invariant method multiplies and divides values by 1e18 to manage precision. For example, the scale0 and scale1 values are multiplied by 1e18 before dividing by the value of the liquidity variable to maintain a precision of 1e18, the same as that of the function’s input values.

The a, b, and c terms of the core invariant logic increase the precision to 1e36, which is appropriate for arithmetic that is sensitive to rounding errors. The d term, however, uses native exponentiation to raise a value with 1e18 precision to the fourth power, yielding a number with 1e72 precision. The underlying value of the d term is therefore orders of magnitude larger than that of the others, preventing the invariant from being satisfied under expected circumstances (e.g., the arithmetic operations can easily revert due to overflow protection).

Action items:

  • Consider a fixed-point power function to raise the upperBound value to the fourth power.
  • Alternatively, or in addition, use the FullMath.mulDiv function and native multiplication calls to produce a result with 1e36 precision.
  • Measure each option due to different rounding properties, so test all possibilities and choose the one that best fits the requirements of the new invariant.
  • Write thorough tests that confirm the solution behaves and rounds as intended.
  • Document the precision used for all critical values. This will facilitate a review of the codebase and help prevent similar issues.
  • LHS and RHS need to match.
  • Sufficient unit and integration tests exist for all components, and use fuzzing tests to check proper behavior at the bounds.
@robertleifke robertleifke self-assigned this Jun 29, 2024
@robertleifke robertleifke added the bug Something isn't working label Jun 29, 2024
@robertleifke robertleifke added this to the Review #2 milestone Jun 29, 2024
@robertleifke robertleifke added the documentation Improvements or additions to documentation label Jun 29, 2024
@robertleifke robertleifke linked a pull request Jul 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant