Skip to content

Commit ff8e907

Browse files
committed
fix(lending_pool): base share accounting on tracked managed assets, not raw balance
Share value for both minting and redemption was derived from the raw TokenClient::balance of the pool. That let the price swing while loan principal was out (a receivable) and let anyone inflate it by transferring tokens directly to the pool address (donation / inflation attack), while redeem_shares subtracted the full yield-inclusive payout from TotalDeposits, causing principal drift. Introduce TotalManagedAssets (principal + realized yield, net of withdrawals) as the single source of truth for the share<->asset exchange rate, moved only by deposit, redeem, and an admin-gated record_yield. The raw balance is now a liquidity gate only (InsufficientLiquidity on under-funded redemptions). redeem_shares reduces TotalDeposits by the principal portion only. Adds record_yield (emitting YieldDistributed), bumps version 3->4, documents the model, and adds tests for outstanding loans, direct transfers, the inflation attack, and principal reconciliation. Closes #2
1 parent 2011656 commit ff8e907

152 files changed

Lines changed: 7421 additions & 254 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

lending_pool/src/events.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ pub fn emergency_withdraw(
2424
env.events().publish(topics, (amount, shares_burned));
2525
}
2626

27-
#[allow(dead_code)]
2827
pub fn yield_distributed(env: &Env, token: Address, amount: i128) {
2928
let topics = (Symbol::new(env, "YieldDistributed"), token);
3029
env.events().publish(topics, amount);

lending_pool/src/lib.rs

Lines changed: 138 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,27 @@ pub enum PoolError {
2727
///
2828
/// v2 replaces the accumulator-style keys (Deposit, RewardDebt, ClaimableYield,
2929
/// AccYieldPerDeposit, UnclaimedYieldPool) with a share-based (LP-token) model.
30-
/// Yield is now implicit in the exchange rate between shares and underlying
31-
/// assets — no separate accumulation or claim step is required.
30+
/// Yield is implicit in the exchange rate between shares and underlying assets —
31+
/// no separate accumulation or claim step is required.
32+
///
33+
/// ## Accounting source of truth (v3, issue #2)
34+
///
35+
/// Share value is derived **exclusively** from `TotalManagedAssets`, an
36+
/// internally-tracked figure equal to deposited principal plus realized yield,
37+
/// net of withdrawals. It is deliberately *not* derived from the contract's raw
38+
/// `TokenClient::balance`, because the raw balance:
39+
/// * drops while principal is out on loan (the principal is still a pool asset,
40+
/// just a receivable — share value must not swing with utilisation), and
41+
/// * can be inflated by anyone transferring tokens directly to the pool
42+
/// address (a donation / inflation attack that would otherwise let an
43+
/// attacker arbitrarily move existing holders' redeemable value).
44+
///
45+
/// The raw balance is used only as a *liquidity* gate: a redemption that cannot
46+
/// currently be serviced from on-hand tokens fails with `InsufficientLiquidity`
47+
/// rather than mis-pricing shares. Realized yield (e.g. loan interest repaid
48+
/// into the pool) is folded into `TotalManagedAssets` exclusively through the
49+
/// admin-gated `record_yield` entry point, so unsolicited transfers never change
50+
/// the exchange rate.
3251
///
3352
/// All per-token keys carry the token address so one contract instance can
3453
/// serve multiple token liquidity pools.
@@ -47,8 +66,15 @@ pub enum DataKey {
4766
/// (provider, token) → ledger sequence of the most recent deposit
4867
DepositTimestamp(Address, Address),
4968
/// token → total principal deposited (net of withdrawals); used for
50-
/// utilisation stats and the MaxPoolSize cap
69+
/// utilisation stats and the MaxPoolSize cap. Tracks principal only — it is
70+
/// never moved by yield, so it cannot drift above what was actually
71+
/// deposited.
5172
TotalDeposits(Address),
73+
/// token → total underlying assets backing LP shares (principal + realized
74+
/// yield, net of withdrawals). Single source of truth for the share↔asset
75+
/// exchange rate. Unaffected by direct token transfers or by principal
76+
/// temporarily out on loan. See the module-level accounting note.
77+
TotalManagedAssets(Address),
5278
/// token → number of active depositors
5379
DepositorCount(Address),
5480
ProposedAdmin,
@@ -77,7 +103,7 @@ impl LendingPool {
77103
const INSTANCE_TTL_BUMP: u32 = 518400;
78104
const PERSISTENT_TTL_THRESHOLD: u32 = 17280;
79105
const PERSISTENT_TTL_BUMP: u32 = 518400;
80-
const CURRENT_VERSION: u32 = 3;
106+
const CURRENT_VERSION: u32 = 4;
81107
const DEFAULT_WITHDRAWAL_COOLDOWN: u32 = 1_440;
82108
const SHARE_PRICE_SCALE: i128 = 1_000_000;
83109
const MAX_WITHDRAWAL_COOLDOWN_LEDGERS: u32 = 17_280 * 30;
@@ -128,6 +154,24 @@ impl LendingPool {
128154
.unwrap_or(0)
129155
}
130156

157+
/// Total underlying assets backing LP shares (principal + realized yield,
158+
/// net of withdrawals). The single source of truth for the share↔asset
159+
/// exchange rate — see the module-level accounting note.
160+
fn total_managed_assets(env: &Env, token: &Address) -> i128 {
161+
Self::bump_instance_ttl(env);
162+
env.storage()
163+
.instance()
164+
.get(&DataKey::TotalManagedAssets(token.clone()))
165+
.unwrap_or(0)
166+
}
167+
168+
fn set_total_managed_assets(env: &Env, token: &Address, value: i128) {
169+
env.storage()
170+
.instance()
171+
.set(&DataKey::TotalManagedAssets(token.clone()), &value);
172+
Self::bump_instance_ttl(env);
173+
}
174+
131175
fn read_shares(env: &Env, provider: &Address, token: &Address) -> i128 {
132176
let key = DataKey::Shares(provider.clone(), token.clone());
133177
let shares: i128 = env.storage().persistent().get(&key).unwrap_or(0);
@@ -272,13 +316,32 @@ impl LendingPool {
272316
}
273317

274318
let cur_total_shares = Self::total_shares(env, token);
275-
let total_assets = Self::read_pool_balance(env, token);
319+
// Redemption value is derived from internally-tracked managed assets, not
320+
// the raw token balance, so it cannot be manipulated by direct transfers
321+
// and does not swing while principal is out on loan (issue #2).
322+
let total_assets = Self::total_managed_assets(env, token);
276323
let assets_to_return = Self::calc_assets_to_redeem(shares, total_assets, cur_total_shares)?;
277324

278325
if assets_to_return <= 0 {
279326
return Err(PoolError::InvalidAmount);
280327
}
281328

329+
// Liquidity gate: the redemption must be serviceable from tokens the pool
330+
// physically holds. If principal is currently out on loan the share value
331+
// is unchanged, but the redemption is deferred rather than mis-priced.
332+
let liquid_balance = Self::read_pool_balance(env, token);
333+
if liquid_balance < assets_to_return {
334+
return Err(PoolError::InsufficientLiquidity);
335+
}
336+
337+
// Principal portion being redeemed, used to keep TotalDeposits tracking
338+
// principal only (it must not absorb the yield portion of the payout).
339+
let cur_total_deposits = Self::total_deposits(env, token);
340+
let principal_redeemed = shares
341+
.checked_mul(cur_total_deposits)
342+
.and_then(|v| v.checked_div(cur_total_shares))
343+
.expect("principal redeem overflow");
344+
282345
TokenClient::new(env, token).transfer(
283346
&env.current_contract_address(),
284347
provider,
@@ -309,7 +372,15 @@ impl LendingPool {
309372
.instance()
310373
.set(&DataKey::TotalShares(token.clone()), &new_total_shares);
311374

312-
let new_total_deposits = Self::total_deposits(env, token).saturating_sub(assets_to_return);
375+
// Managed assets shrink by the full payout (principal + yield portion).
376+
let new_managed_assets = total_assets
377+
.checked_sub(assets_to_return)
378+
.expect("managed assets underflow");
379+
Self::set_total_managed_assets(env, token, new_managed_assets);
380+
381+
// TotalDeposits shrinks by the principal portion only, so it never drifts
382+
// away from actual net principal (issue #2, acceptance criterion 4).
383+
let new_total_deposits = cur_total_deposits.saturating_sub(principal_redeemed);
313384
env.storage()
314385
.instance()
315386
.set(&DataKey::TotalDeposits(token.clone()), &new_total_deposits);
@@ -426,6 +497,14 @@ impl LendingPool {
426497
Self::total_shares(&env, &token)
427498
}
428499

500+
/// Total underlying assets backing LP shares (principal + realized yield),
501+
/// i.e. the value the outstanding shares collectively redeem to. This is the
502+
/// accounting figure that drives the share price — distinct from
503+
/// `pool_balance` (raw on-hand tokens) and `get_total_deposits` (principal).
504+
pub fn get_total_managed_assets(env: Env, token: Address) -> i128 {
505+
Self::total_managed_assets(&env, &token)
506+
}
507+
429508
pub fn get_withdrawal_cooldown(env: Env) -> u32 {
430509
Self::withdrawal_cooldown(&env)
431510
}
@@ -464,9 +543,11 @@ impl LendingPool {
464543
}
465544
}
466545

467-
// Snapshot pool state *before* the transfer so the share price
468-
// reflects the pre-deposit pool composition.
469-
let total_assets_before = Self::read_pool_balance(&env, &token);
546+
// Snapshot pool state *before* the transfer so the share price reflects
547+
// the pre-deposit pool composition. Uses internally-tracked managed
548+
// assets (not the raw balance) so a direct transfer made just before the
549+
// deposit cannot dilute or inflate the minted shares (issue #2).
550+
let total_assets_before = Self::total_managed_assets(&env, &token);
470551
let cur_total_shares = Self::total_shares(&env, &token);
471552

472553
// Issue #1: first depositor must commit at least MINIMUM_INITIAL_DEPOSIT
@@ -524,6 +605,12 @@ impl LendingPool {
524605
.instance()
525606
.set(&DataKey::TotalDeposits(token.clone()), &new_total_deposits);
526607

608+
// Deposited principal joins the managed-asset base 1:1.
609+
let new_managed_assets = total_assets_before
610+
.checked_add(amount)
611+
.expect("managed assets overflow");
612+
Self::set_total_managed_assets(&env, &token, new_managed_assets);
613+
527614
Self::bump_instance_ttl(&env);
528615
deposit(
529616
&env,
@@ -551,7 +638,7 @@ impl LendingPool {
551638
}
552639
let asset_value = Self::calc_assets_to_redeem(
553640
shares,
554-
Self::read_pool_balance(&env, &token),
641+
Self::total_managed_assets(&env, &token),
555642
cur_total_shares,
556643
)
557644
.unwrap_or(0);
@@ -570,7 +657,7 @@ impl LendingPool {
570657
}
571658
Self::calc_assets_to_redeem(
572659
shares,
573-
Self::read_pool_balance(&env, &token),
660+
Self::total_managed_assets(&env, &token),
574661
cur_total_shares,
575662
)
576663
.unwrap_or(0)
@@ -583,13 +670,16 @@ impl LendingPool {
583670

584671
/// Current LP share price scaled by `SHARE_PRICE_SCALE`.
585672
/// `1_000_000` means 1.0 underlying asset per share.
673+
///
674+
/// Derived from internally-tracked managed assets, so it is stable while
675+
/// principal is out on loan and immune to direct-transfer manipulation.
586676
pub fn get_share_price(env: Env, token: Address) -> i128 {
587677
let total_shares = Self::total_shares(&env, &token);
588678
if total_shares <= 0 {
589679
return Self::SHARE_PRICE_SCALE;
590680
}
591681

592-
Self::read_pool_balance(&env, &token)
682+
Self::total_managed_assets(&env, &token)
593683
.checked_mul(Self::SHARE_PRICE_SCALE)
594684
.and_then(|v| v.checked_div(total_shares))
595685
.expect("share price overflow")
@@ -644,6 +734,42 @@ impl LendingPool {
644734
Ok(())
645735
}
646736

737+
/// Record `amount` of realized yield (e.g. loan interest repaid into the
738+
/// pool), raising every outstanding share's value pro-rata.
739+
///
740+
/// This is the *only* way assets enter share value besides deposits. Because
741+
/// a contract cannot distinguish a legitimate interest repayment from an
742+
/// unsolicited donation by looking at its balance, yield is recognised
743+
/// through this deliberate, admin-gated call rather than by reading the raw
744+
/// balance. That is precisely what stops a direct transfer from arbitrarily
745+
/// changing existing holders' redeemable value (issue #2).
746+
///
747+
/// Yield is not principal, so it does not move `TotalDeposits` or count
748+
/// against the `MaxPoolSize` cap. The caller is responsible for ensuring the
749+
/// corresponding tokens are actually present in the pool; otherwise later
750+
/// redemptions will hit the `InsufficientLiquidity` gate.
751+
pub fn record_yield(env: Env, token: Address, amount: i128) -> Result<(), PoolError> {
752+
Self::admin(&env).require_auth();
753+
Self::assert_not_paused(&env)?;
754+
755+
if amount <= 0 {
756+
return Err(PoolError::InvalidAmount);
757+
}
758+
759+
// Yield is only meaningful once shares exist to distribute it to.
760+
if Self::total_shares(&env, &token) <= 0 {
761+
return Err(PoolError::InvalidAmount);
762+
}
763+
764+
let new_managed_assets = Self::total_managed_assets(&env, &token)
765+
.checked_add(amount)
766+
.expect("managed assets overflow");
767+
Self::set_total_managed_assets(&env, &token, new_managed_assets);
768+
769+
yield_distributed(&env, token, amount);
770+
Ok(())
771+
}
772+
647773
// ── Queries ───────────────────────────────────────────────────────────
648774

649775
pub fn get_pool_stats(env: Env, token: Address) -> PoolStats {

0 commit comments

Comments
 (0)