fix(lending_pool): base share accounting on tracked managed assets, n…

[xeladev4]

Fix LendingPool share accounting: track managed assets internally instead of raw balance

Closes #2

Problem

lending_pool derived total_assets for both share minting (deposit) and
redemption (redeem_shares / get_deposit / get_depositor_yield /
get_share_price) from read_pool_balance — the raw TokenClient::balance of
the contract. That made the share price:

• swing with outstanding loans — loan_manager::approve_loan transfers
  principal out of the pool and repayments transfer it back, so a share's value
  dropped while a loan was live even though the lent principal is still a pool
  asset (a receivable); and
• manipulable by anyone — a direct token transfer to the pool address
  inflates the balance, decoupling shares from the tracked TotalDeposits and
  enabling a classic donation / first-depositor inflation attack.

redeem_shares also subtracted the full payout (assets_to_return, which
includes yield) from TotalDeposits, so the principal counter drifted away from
actual net principal.

Decision (acceptance criterion 1)

Share value tracks an internally-tracked figure, never the raw balance.

A new per-token key TotalManagedAssets is the single source of truth for the
share↔asset exchange rate. It equals deposited principal + realized yield, net of
withdrawals, and only ever moves through three deliberate paths: deposit,
redeem_shares, and the admin-gated record_yield. It is not affected by
unsolicited transfers or by principal temporarily out on loan.

The raw balance is now used only as a liquidity gate: a redemption that
cannot be serviced from on-hand tokens fails with the (previously unused)
InsufficientLiquidity error rather than being mis-priced.

Changes (lending_pool/src/lib.rs)

• Add DataKey::TotalManagedAssets(Address) plus total_managed_assets /
  set_total_managed_assets accessors and a public get_total_managed_assets
  getter.
• deposit snapshots and increments TotalManagedAssets (instead of reading the
  balance) when computing shares to mint.
• redeem_shares:
  • prices the payout off TotalManagedAssets;
  • rejects with InsufficientLiquidity when on-hand balance can't cover it;
  • decrements TotalDeposits by the principal portion only
    (shares * total_deposits / total_shares), fixing the drift
    (acceptance criterion 4);
  • decrements TotalManagedAssets by the full payout.
• get_deposit, get_depositor_yield, and get_share_price read
  TotalManagedAssets.
• Add admin-gated record_yield(token, amount) — the only way realized yield
  (e.g. loan interest repaid into the pool) raises share value. Because a
  contract can't tell a legitimate repayment from a donation by inspecting its
  balance, yield is recognised through this explicit call; this is precisely what
  stops direct transfers from changing holders' redeemable value
  (acceptance criterion 2). Emits the existing YieldDistributed event.
• Bump CURRENT_VERSION 3 → 4 and document the accounting model in module/field
  doc comments.

Tests (lending_pool/src/test.rs) (acceptance criterion 3)

Existing yield tests now recognise interest via record_yield (mirroring a
repayment followed by the manager recording yield). New tests:

• test_direct_transfer_does_not_change_redeemable_value — a 10k unsolicited
  transfer leaves share price and redeemable value untouched.
• test_inflation_attack_via_donation_is_prevented — first-depositor inflation
  attack no longer rounds a victim's deposit to zero shares.
• test_deposit_and_redeem_while_loan_outstanding — deposits/redeems price off
  the stable base while a loan is live; over-liquidity redemptions are gated.
• test_total_deposits_tracks_principal_after_yield_then_redeem — TotalDeposits
  drops by principal only after a yield-bearing redemption.
• test_record_yield_requires_admin, test_record_yield_rejected_when_no_shares_outstanding.
• test_withdrawal_with_utilization rewritten: share value is no longer
  discounted by utilisation; over-balance redemptions return InsufficientLiquidity.

Automatic yield distribution (review follow-up)

Because share value now tracks TotalManagedAssets rather than the raw balance,
interest merely transferred into the pool no longer reaches LPs on its own —
it must be reported. This is now wired end-to-end:

• Pool authorizes a per-token yield reporter: set_loan_manager(token, manager) (admin-gated) + get_loan_manager(token). record_yield is gated to
  that reporter, falling back to the admin when none is configured (manual /
  keeper use). Emits LoanManagerUpdated.
• LoanManager reports realized yield after the funds land in the pool:
  • repay → reports the interest + late-fee portion of the repayment
    (principal just returns borrowed principal, so it is excluded).
  • extend_loan → reports the extension fee (pure income).
  • Reporting is best-effort and non-fatal: the manager only calls the pool
    when it is the configured reporter, and swallows any pool-side error, so
    yield accounting can never block a repayment.
• liquidate is intentionally NOT wired (documented in code): a liquidation
  can also leave a principal shortfall, and crediting recovered interest as yield
  without a paired principal write-off would inflate share value during a loss
  event. That needs a record_loss companion and is called out as a follow-up;
  recovered funds remain as conservative, uncounted surplus until then.

Review nits addressed

• calc_shares_to_mint no longer silently mints 1:1 when total_assets_before == 0 while shares exist; it documents the (unreachable) invariant with a
  debug_assert! and would surface a violation as a panic via checked_div.
• redeem_shares now follows checks-effects-interactions: all accounting is
  committed before the token transfer out.

Verification

• cargo fmt --check — clean
• cargo clippy --all-targets -- -D warnings — clean
• cargo test — all workspace tests pass (58 in lending_pool, 90 in
  loan_manager)
• cargo build --target wasm32-unknown-unknown — builds

Notes / follow-ups (out of scope here)

• Credit-loss accounting (record_loss) for default/liquidation principal
  shortfalls is the natural companion to record_yield and is left as a
  follow-up (see the in-code note in loan_manager::liquidate).
• The storage-model change bumps the contract version; an already-deployed pool
  would need a one-time migration seeding TotalManagedAssets from existing
  balance/principal. No migration entry point exists in this contract today, so
  none is added here.
• Per issue scope, no oracle pricing or multi-asset rebalancing was added.

[xeladev4]

This was a really tough one boss

[ogazboiz]

this is genuinely strong work and ci is green (fmt, clippy -D warnings, tests and the wasm build all pass). the donation/inflation fix is mathematically sound: share value is derived from a tracked TotalManagedAssets figure rather than the raw token balance, deposits price off managed-assets-before and redeem off managed-assets, and direct token transfers never touch it, so the first-depositor inflation attack is neutralized (test_inflation_attack_via_donation_is_prevented confirms it). it also correctly preserves #31 (the redeem_shares core, emergency_withdraw gate and NotPaused are all intact, not reverted), and tracking TotalDeposits by principal-only on redeem with a liquidity gate is a nice touch. checked math throughout, and record_yield is admin + pause gated.

one blocking issue before this can land:

1. automatic yield distribution is silently broken by this change. today repaid loan interest reaches depositors automatically because share value reads the pool balance, which grows when interest is repaid. once share value tracks only TotalManagedAssets, that figure moves only via deposit and the new admin-gated record_yield. but loan_manager repays with a raw token transfer (loan_manager/src/lib.rs:1304) and never calls record_yield, and there are zero non-test callers of record_yield in the repo. so after this merge, repaid interest sits as uncounted surplus and depositors stop earning until an admin manually calls record_yield. the tests pass only because they call it by hand. please wire record_yield into the repayment path (loan_manager should call pool.record_yield(token, interest_portion) after transferring the interest), or add an automated keeper and document it.

two smaller things:
2. calc_shares_to_mint mints 1:1 when total_assets_before == 0 even if cur_total_shares > 0. probably unreachable since redeem decrements both together, but worth a guard or an assert documenting the invariant.
3. redeem_shares transfers before the state writes (inherited from #31, recipient is the provider so no re-entry risk here), not strictly checks-effects-interactions. optional to tighten while you're in there.

heads up on coordination: #33 is also fixing the donation attack on this same file (with a MINIMUM_INITIAL_DEPOSIT guard, bundled with a loan-term accrual change). your managed-assets approach here is the more complete and cleaner fix for the donation vector, so i'd like to canonicalize the share-accounting fix on this pr and have #33 keep just its accrual-by-term work, otherwise the two collide on the share math. once record_yield is wired in, this is the one i want for the donation fix.

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

[xeladev4]

What I changed in response to the review
Blocking item — automatic yield distribution restored (now end-to-end):

Pool (lending_pool/src/lib.rs): added a per-token authorized yield reporter — set_loan_manager(token, manager) (admin-gated) + get_loan_manager. record_yield is now gated to that reporter, falling back to admin when unset (manual/keeper). Emits LoanManagerUpdated.
LoanManager (loan_manager/src/lib.rs): repay reports the interest + late-fee portion (principal excluded); extend_loan reports the extension fee. Reporting is best-effort/non-fatal — it only calls the pool when this manager is the configured reporter, and swallows pool-side errors via try_record_yield, so it can never block a repayment.
Deliberate scope call on liquidate: I did not wire it, and documented why in-code: a liquidation can leave a principal shortfall, so crediting recovered interest as yield without a paired record_loss would inflate share value during a loss event. Recovered funds stay as conservative uncounted surplus; record_loss is flagged as the follow-up. I'd rather not ship a half-fix there — happy to do the full default/liquidation value-accounting in a follow-up PR if you want it.

The two nits:

calc_shares_to_mint no longer silently mints 1:1 when total_assets_before == 0 with shares outstanding — documents the invariant via debug_assert! and would surface a violation as a panic.
redeem_shares now does all state writes before the transfer (CEI).
Tests: pool reporter-gating tests + two loan_manager end-to-end tests (repaid interest reaches LP share value; repayment still succeeds when no reporter configured).

I noted your coordination point with #33 — this PR keeps the share-accounting/donation fix; #33 can drop its share-math and keep accrual-by-term.