#99 Medium: Precision Loss in Share Price Calculations#126
Open
coder45oj wants to merge 3 commits into
Open
Conversation
…on withdrawal Problem: Integer floor division in withdraw() silently loses value for LPs. - Deposit: usdc_amount * total_shares / total_deposits (floor → LP gets fewer shares) - Withdraw: shares * total_deposits / total_shares (floor → LP gets less USDC) Fix: Use ceiling division for withdrawal calculations so that remaining fractional value is returned to the withdrawing LP instead of being lost. - Deposit stays as floor (conservative, benefits existing LPs) - Withdraw uses ceiling: (product + total_shares - 1) / total_shares, implemented as floor + 1 when remainder > 0 to avoid overflow edge cases Tests added: - test_precision_withdraw_ceil_recovers_value: single deposit+withdraw after yield recovers 1000 USDC vs 999 with floor - test_precision_ceiling_gt_floor_when_remainder: dynamically verifies ceiling == floor + 1 when remainder exists - test_precision_exact_division_unchanged: exact division unchanged - test_precision_dust_share_gets_1_usdc: dust shares return >= 1 USDC - test_precision_multiple_withdrawals_accumulate: partial withdrawals each benefit from ceiling rounding Closes TrusTrove#99
|
@coder45oj Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
Fix Ci error @coder45oj |
) Re-wrap three long assert!/assert_eq! statements in precision-rounding tests: - test_precision_withdraw_ceil_recovers_value: assert_eq! with full deposit message - test_precision_dust_share_gets_1_usdc: assert! for dust share return >= 1 - test_precision_multiple_withdrawals_accumulate: two ceiling-rounding asserts Resolves cargo fmt --all --check failure on the issue-99-precision-loss branch.
Contributor
|
resolve Conflict @coder45oj |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes precision loss in share price calculations by applying ceiling rounding on withdrawal, minimizing value leakage for liquidity providers. The deposit side remains floor rounding (conservative, benefits existing LPs).
Problem
Two integer division calculations cause silent value loss for LPs due to floor rounding:
contracts/pool/src/lib.rs:130):usdc_amount * total_shares / total_deposits— LP receives slightly fewer shares than their deposit entitles them to.contracts/pool/src/lib.rs:221):shares * total_deposits / total_shares— LP receives slightly less USDC than their shares are worth.While each individual loss is tiny (often 1 unit), these accumulate over time across all LPs, creating systemic leakage.
Fix
Implemented ceiling division for the withdrawal calculation at
contracts/pool/src/lib.rs:208-221:This ensures that when the mathematical value of shares is not an integer number of USDC units, the withdrawing LP receives the ceiling (rounded up) instead of the floor, recovering value that would otherwise be lost.
Example
Precision Tests
Added 5 tests in
contracts/pool/src/test.rs:test_precision_withdraw_ceil_recovers_value— single deposit + withdraw after yield, verifies full deposit recovered via ceilingtest_precision_ceiling_gt_floor_when_remainder— dynamically confirms ceiling = floor + 1 when remainder existstest_precision_exact_division_unchanged— exact divisions unaffectedtest_precision_dust_share_gets_1_usdc— dust shares return ≥ 1 USDC instead of 0test_precision_multiple_withdrawals_accumulate— partial withdrawals each benefit from ceilingCloses #99