fix: junior withdrawal ordering unfairness during active insurance loss

[0x-SquidSol]

Summary

During active insurance loss (total_flushed > total_returned), partial junior withdrawals used a proportional share of the raw junior_balance as the decrease amount. But withdrawal_amount was computed from the loss-adjusted effective_junior_balance, which is smaller. This mismatch caused gross_senior (= gross_pool - junior_balance) to inflate after each junior withdrawal, making distribute_loss over-penalize remaining junior holders.

Reproduction

Initial state: junior_balance=1000, junior_total_lp=2, total_flushed=500, 1 senior LP.

Step	User	withdrawal_amount	raw_decrease (old)	junior_balance after	effective_jb after
1	A	250	500	500	0
2	B	0 (BLOCKED)	—	—	—

Fair outcome: A gets 250, B gets 250. Actual (buggy): A gets 250, B gets 0.

Root Cause

raw_decrease = lp_amount * junior_balance / junior_lp_before removes a proportional share of the gross balance (500), but total_withdrawn only increases by withdrawal_amount (250). The 250 discrepancy shifts into gross_senior, causing distribute_loss to re-apply the full net_loss against the diminished junior balance.

Fix

Subtract withdrawal_amount from junior_balance instead of the proportional raw share. Since total_withdrawn also increases by withdrawal_amount, gross_senior = (deposited - withdrawn) - junior_balance stays constant across partial junior withdrawals.

Verification with fix:

Step	User	withdrawal_amount	junior_balance after	effective_jb after
1	A	250	750	250
2	B	250	500	0 (then zeroed by new_junior_lp==0 branch)

Both get 250. Fair.

Severity

MEDIUM — first-mover advantage for junior tranche holders during loss periods; last withdrawer can receive zero and have funds permanently locked

Test plan

• cargo build — compiles cleanly
• cargo clippy — zero warnings
• 3 independent agents confirmed the bug with exact arithmetic traces
• 3 independent agents unanimously agreed on the fix approach

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved withdrawal accounting accuracy by updating how withdrawal amounts are calculated and applied to balance reconciliation. Withdrawal processing now correctly reflects the actual amounts withdrawn from the vault, ensuring more precise balance tracking.

[coderabbitai]

📝 Walkthrough

Walkthrough

The process_withdraw function in src/processor.rs has been modified to calculate junior-tranche balance decrements using the actual withdrawal amount (loss-adjusted collateral) rather than a proportional raw-balance share. The control flow remains identical, with only the arithmetic computation altered.

Changes

Cohort / File(s)	Summary
Junior Balance Withdrawal Accounting src/processor.rs	Modified process_withdraw to decrease pool.junior_balance by withdrawal_amount instead of a proportional raw-balance share, removing intermediate jb/raw_decrease computation and altering the checked_sub result dependency.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: junior withdrawal balance drift causes permanent fund lockup #103: Directly modifies the same junior-balance decrement logic in process_withdraw, switching accounting methodology for pool balance updates.
• fix: 9 HIGH-severity tranche/insurance security fixes #98: Alters junior-withdrawal accounting in process_withdraw via different effective balance and sub-pool logic approaches.

Poem

🐰 A hop, a skip, the accounts align,
Where loss-adjusted collateral defines the line,
No more proportional shares to confuse the way,
Just withdrawal amounts, crystal clear each day!
Whisker-twitching approval for this fix so fine! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description includes a clear summary, reproduction case, root cause analysis, fix explanation, and verification table. However, the checklist section is missing from the provided description—no indication of test results, Clippy status, or format check completion.	Add the required checklist from the template showing test status, Clippy results, format check, and confirmation of Kani verification if math/invariants were touched.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing an ordering unfairness bug in junior withdrawals during insurance loss periods, which directly corresponds to the core change in process_withdraw.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/processor.rs (1)

908-926: Please add a regression test for the reproduced loss case.

This arithmetic is subtle and easy to regress. A focused test for “2 junior LPs, active insurance loss, sequential 50% withdrawals” would pin the intended behavior: both users receive the same 250 payout, and the second withdraw does not collapse to zero.

I can draft that test if you'd like.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/processor.rs` around lines 908 - 926, Add a focused regression test that
reproduces the “2 junior LPs, active insurance loss, sequential 50% withdrawals”
scenario to ensure the arithmetic in pool.set_junior_balance and distribute_loss
remains correct: create a pool with known gross_pool and two junior LPs each
with 500 effective value, apply an insurance loss so effective_junior_balance <
raw balance, have the first junior withdraw 50% (expect payout 250), then the
second junior withdraw 50% and assert they also receive 250 (not zero); locate
helpers that construct pools and perform withdrawals (functions referencing
pool.set_junior_balance, withdrawal_amount, junior_balance, total_withdrawn, and
distribute_loss) and implement the test (e.g.,
test_sequential_junior_withdrawals_under_loss) asserting both payouts equal 250
and that pool balances update without inflating gross_senior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/processor.rs`:
- Around line 908-926: Add a focused regression test that reproduces the “2
junior LPs, active insurance loss, sequential 50% withdrawals” scenario to
ensure the arithmetic in pool.set_junior_balance and distribute_loss remains
correct: create a pool with known gross_pool and two junior LPs each with 500
effective value, apply an insurance loss so effective_junior_balance < raw
balance, have the first junior withdraw 50% (expect payout 250), then the second
junior withdraw 50% and assert they also receive 250 (not zero); locate helpers
that construct pools and perform withdrawals (functions referencing
pool.set_junior_balance, withdrawal_amount, junior_balance, total_withdrawn, and
distribute_loss) and implement the test (e.g.,
test_sequential_junior_withdrawals_under_loss) asserting both payouts equal 250
and that pool balances update without inflating gross_senior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e657619-791e-407a-b8aa-54ad2ec2c575

📥 Commits

Reviewing files that changed from the base of the PR and between 4fe119b and 75150f1.

📒 Files selected for processing (1)

• src/processor.rs