Fix: LendingPool MaxPoolSize cap and DepositorCount drifting

[Lansa-18]

Closes #9

Fix: TotalDeposits Tracks Net Principal; Harden DepositorCount

Problem

Two accounting bugs in lending_pool/src/lib.rs caused TotalDeposits and DepositorCount to drift from real state whenever yield had accrued in the pool.

Bug 1 — TotalDeposits Saturates Under Yield (critical)

redeem_shares() decremented TotalDeposits by assets_to_return:

// assets_to_return = shares × pool_balance / total_shares
//                  = principal + yield portion
let new_total_deposits = Self::total_deposits(env, token).saturating_sub(assets_to_return);

TotalDeposits is supposed to track net deposited principal — the sum of all deposit() amount arguments minus what has been withdrawn. But assets_to_return includes accrued yield, so each yield-bearing withdrawal over-subtracted, eroding TotalDeposits faster than actual principal left the pool.

Concrete example: Provider A and Provider B each deposit 1 000 (TotalDeposits = 2 000). 1 000 tokens of interest arrive (pool_balance = 3 000, total_shares = 2 000). A redeems all 1 000 shares and receives 1 500 assets (principal + yield). The buggy code then sets:

TotalDeposits = 2 000 − 1 500 = 500

But B's original principal is still entirely in the pool — TotalDeposits should be 1 000. With three or more providers, the compounding error cascades: each successive yield-bearing withdrawal shaves more from TotalDeposits than the corresponding principal, eventually saturating it to 0 via saturating_sub while shares and depositors remain.

Bug 2 — DepositorCount Decrement Swallows Invariant Violations Silently

On full withdrawal, the count was decremented via saturating_sub(1):

&count.saturating_sub(1)

If count is somehow 0 when this branch is reached (indicating upstream state corruption), saturating_sub writes 0 back to storage — committing the corrupted value without any signal. The transaction succeeds, the bad state persists, and every future operation builds on it.

---

Fix

Bug 1 — Subtract Principal Fraction, Not Asset Value

Replace the assets_to_return subtraction with the pro-rata principal fraction:

principal_to_remove = total_deposits × shares_burned / total_shares_before

This keeps TotalDeposits strictly in the "net principal" space regardless of how much yield has accumulated. When the last depositor fully exits (shares == total_shares), the formula reduces to total_deposits × total_shares / total_shares = total_deposits, so TotalDeposits reaches exactly 0. For partial withdrawals it removes only the proportional principal slice, leaving yield invisible to the cap.

let cur_total_deposits = Self::total_deposits(env, token);
let principal_to_remove = cur_total_deposits
    .checked_mul(shares)
    .and_then(|v| v.checked_div(cur_total_shares))
    .expect("principal computation overflow");
let new_total_deposits = cur_total_deposits.saturating_sub(principal_to_remove);

cur_total_shares is already captured earlier in the function (pre-withdrawal) and reused here — no extra reads required.

Bug 2 — Fail Loudly on Count Underflow

&count.checked_sub(1).expect("depositor_count underflow")

A panic! in Soroban aborts and reverts the entire transaction. If count is 0 when a full-withdrawal branch is reached, the withdrawal fails cleanly rather than persisting a 0 that future operations silently build on. In normal operation this branch is only reachable when a depositor just burned their last shares, so count ≥ 1 is a hard invariant — making violations visible is the right call.

---

Documentation

• DataKey::TotalDeposits — expanded comment clarifying it holds net deposited principal only, that yield is excluded (it is implicit in the share price), and distinguishing it from pool_token_balance (the live on-chain balance that includes yield but excludes outstanding loans)
• PoolStats::utilization_bps — added note that accrued yield inflates pool_token_balance, which partially offsets outstanding loans in the utilisation formula, so utilisation may understate the true loan fraction when significant yield has accumulated

---

Tests Added

Four regression tests in a dedicated Issue #9 section at the bottom of test.rs. Each test is written to fail on the old code and pass on the fix.

Test	Coverage
test_total_deposits_tracks_principal_not_yield	Single depositor, 1 000 principal, 300 tokens of yield arrive. Redeeming 500 shares (half) must reduce TotalDeposits by 500 (principal half), not by 650 (the yield-inflated asset value). Minimal reproduction of Bug 1.
test_cap_enforced_after_yield_bearing_withdrawal	Two providers each deposit 1 000 against a cap of 2 000. 1 000 yield arrives. A fully withdraws. Asserts TotalDeposits == 1 000 (B's principal, not 500); a deposit of 1 001 is rejected with PoolSizeExceeded; a deposit of exactly 1 000 succeeds and fills the cap.
test_depositor_count_across_deposit_partial_full_withdraw_redeposit	Two providers. Asserts depositor_count at every state transition: after both deposit → 2; after A partial-withdraws → still 2; after A fully withdraws → 1; after A re-deposits → 2; after B fully withdraws → 1; after A fully withdraws → 0.
test_total_deposits_zero_after_all_shares_redeemed_with_yield	Two providers, both fully withdraw after 1 000 tokens of yield arrive. Asserts TotalDeposits == 0, total_shares == 0, and depositor_count == 0 — confirming the pool returns to a clean empty state even when the share price has risen above 1:1.

---

Files Changed

File	Changes
lending_pool/src/lib.rs	redeem_shares(): principal-fraction formula, checked_sub hardening; doc updates for DataKey::TotalDeposits and PoolStats::utilization_bps
lending_pool/src/test.rs	4 new regression tests (51 → 55 tests)

[ogazboiz]

the core fix is correct, on main redeem_shares subtracted the yield-inflated payout from the principal-only TotalDeposits tracker so the MaxPoolSize cap drifted, and you fix it by removing only the pro-rata principal (checked mul/div), with good tests for the cap math and DepositorCount. but it now conflicts with #36, which just merged.

#36 reworked the same redeem_shares accounting (it switched share value to total_managed_assets and also computes principal_redeemed pro-rata), and it's the more complete fix since it also closes the donation vector and the out-on-loan mispricing that this pr leaves untouched. so:

1. please rebase on current main. your TotalDeposits rewrite is now largely subsumed by #36, so keep just the parts #36 doesn't cover, mainly the DepositorCount saturating_sub -> checked_sub underflow hardening and any of its tests not already covered by #36's suite.
2. github shows this DIRTY/conflicting now, so the rebase is required to proceed.

once it's rebased down to the DepositorCount hardening on top of #36, i'll merge.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

[Lansa-18]

@ogazboiz I've made the necessary changes, kindly review.

[ogazboiz]

this is exactly what i asked for last round, you rebased onto #36 and trimmed it down to just the DepositorCount underflow hardening. the net change now is the utilization_bps doc comment plus lending_pool/src/lib.rs:382 going from saturating_sub(1) to checked_sub(1).expect("depositor_count underflow"). the MaxPoolSize cap is enforced on deposit (PoolSizeExceeded, checked against principal total_deposits with a checked_add overflow guard), and DepositorCount only increments when existing_shares == 0 and only decrements on full redemption, so no drift on repeated deposit/withdraw. good test coverage too (cap-exceeded panic, cap after yield-bearing withdrawal, and the count walk across deposit/partial/full/redeposit). ci is green and it's up to date with main. merging.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0