fix: use senior sub-pool formula for withdrawal when tranches are enabled - #70
Conversation
…bled When tranches are enabled, process_withdraw used the global pool formula for senior LP withdrawals: collateral = lp_amount * total_pool_value / total_lp_supply This is incorrect. The correct formula is: collateral = lp_amount * senior_balance / senior_lp_supply where: senior_balance = total_pool_value - junior_balance senior_lp_supply = total_lp_supply - junior_total_lp Using the global formula mixes junior-backed collateral into the senior valuation. Concretely: - If junior_fee_mult_bps > 10000, fees are redistributed so junior balance grows faster than senior balance per LP token. The global formula over-values senior LP (includes junior's above-par collateral), allowing senior holders to extract funds that belong to junior holders. - If junior absorbs losses (junior_balance < original contribution), the global formula under-charges senior for the loss. Senior holders appear entitled to more than their share of the remaining vault. Fix: when tranche_enabled() is true and is_junior is false, use calc_senior_collateral_for_withdraw(senior_lp, senior_bal, lp_amount) matching the pattern already used for junior withdrawals. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dcccrypto
left a comment
There was a problem hiding this comment.
Security review — Senior sub-pool formula for withdrawal when tranches enabled. Correct accounting prevents senior depositors from claiming junior funds. Security APPROVED ✅
|
Superseded by PR#95 (merged) and PR#96 (omnibus test coverage). All changes in this PR are already in master. Closing as part of PERC-8433 cleanup. |
|
Implemented in PR #98 and merged to master. Thank you for identifying this vulnerability — the fix was applied based on your diff after two rounds of security review. |
Summary
When tranches are enabled,
process_withdrawused the global pool formula for senior LP withdrawals:The correct formula uses the senior sub-pool only:
Why This Is a Security Issue
Using the global formula when tranches are active mixes junior-backed collateral into the senior valuation:
Fee redistribution attack: If
junior_fee_mult_bps > 10000, fees are redistributed so junior balance grows faster per LP token than senior. The global formula over-values senior LP (includes junior's above-par collateral), letting senior holders drain funds belonging to junior holders.Loss absorption mismatch: If junior absorbs losses (
junior_balance < original), the global formula under-charges senior holders for the loss. Senior withdrawals appear entitled to more than their actual share of remaining vault.Note: junior withdrawals already correctly used
calc_junior_collateral_for_withdraw(junior_lp, junior_bal, lp_amount). This fix applies the same pattern to senior:calc_senior_collateral_for_withdraw(senior_lp, senior_bal, lp_amount). When tranches are disabled, behaviour is unchanged (falls through to global formula).Test plan
cargo test)