Skip to content

Commit d41a17c

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 bbb52f3 commit d41a17c

149 files changed

Lines changed: 7380 additions & 226 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
@@ -10,7 +10,6 @@ pub fn withdraw(env: &Env, provider: Address, token: Address, amount: i128, shar
1010
env.events().publish(topics, (amount, shares_burned));
1111
}
1212

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

lending_pool/src/lib.rs

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

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

241285
let cur_total_shares = Self::total_shares(env, token);
242-
let total_assets = Self::read_pool_balance(env, token);
286+
// Redemption value is derived from internally-tracked managed assets, not
287+
// the raw token balance, so it cannot be manipulated by direct transfers
288+
// and does not swing while principal is out on loan (issue #2).
289+
let total_assets = Self::total_managed_assets(env, token);
243290
let assets_to_return = Self::calc_assets_to_redeem(shares, total_assets, cur_total_shares);
244291

245292
if assets_to_return <= 0 {
246293
return Err(PoolError::InvalidAmount);
247294
}
248295

296+
// Liquidity gate: the redemption must be serviceable from tokens the pool
297+
// physically holds. If principal is currently out on loan the share value
298+
// is unchanged, but the redemption is deferred rather than mis-priced.
299+
let liquid_balance = Self::read_pool_balance(env, token);
300+
if liquid_balance < assets_to_return {
301+
return Err(PoolError::InsufficientLiquidity);
302+
}
303+
304+
// Principal portion being redeemed, used to keep TotalDeposits tracking
305+
// principal only (it must not absorb the yield portion of the payout).
306+
let cur_total_deposits = Self::total_deposits(env, token);
307+
let principal_redeemed = shares
308+
.checked_mul(cur_total_deposits)
309+
.and_then(|v| v.checked_div(cur_total_shares))
310+
.expect("principal redeem overflow");
311+
249312
TokenClient::new(env, token).transfer(
250313
&env.current_contract_address(),
251314
provider,
@@ -276,7 +339,15 @@ impl LendingPool {
276339
.instance()
277340
.set(&DataKey::TotalShares(token.clone()), &new_total_shares);
278341

279-
let new_total_deposits = Self::total_deposits(env, token).saturating_sub(assets_to_return);
342+
// Managed assets shrink by the full payout (principal + yield portion).
343+
let new_managed_assets = total_assets
344+
.checked_sub(assets_to_return)
345+
.expect("managed assets underflow");
346+
Self::set_total_managed_assets(env, token, new_managed_assets);
347+
348+
// TotalDeposits shrinks by the principal portion only, so it never drifts
349+
// away from actual net principal (issue #2, acceptance criterion 4).
350+
let new_total_deposits = cur_total_deposits.saturating_sub(principal_redeemed);
280351
env.storage()
281352
.instance()
282353
.set(&DataKey::TotalDeposits(token.clone()), &new_total_deposits);
@@ -400,6 +471,14 @@ impl LendingPool {
400471
Self::total_shares(&env, &token)
401472
}
402473

474+
/// Total underlying assets backing LP shares (principal + realized yield),
475+
/// i.e. the value the outstanding shares collectively redeem to. This is the
476+
/// accounting figure that drives the share price — distinct from
477+
/// `pool_balance` (raw on-hand tokens) and `get_total_deposits` (principal).
478+
pub fn get_total_managed_assets(env: Env, token: Address) -> i128 {
479+
Self::total_managed_assets(&env, &token)
480+
}
481+
403482
pub fn get_withdrawal_cooldown(env: Env) -> u32 {
404483
Self::withdrawal_cooldown(&env)
405484
}
@@ -438,9 +517,11 @@ impl LendingPool {
438517
}
439518
}
440519

441-
// Snapshot pool state *before* the transfer so the share price
442-
// reflects the pre-deposit pool composition.
443-
let total_assets_before = Self::read_pool_balance(&env, &token);
520+
// Snapshot pool state *before* the transfer so the share price reflects
521+
// the pre-deposit pool composition. Uses internally-tracked managed
522+
// assets (not the raw balance) so a direct transfer made just before the
523+
// deposit cannot dilute or inflate the minted shares (issue #2).
524+
let total_assets_before = Self::total_managed_assets(&env, &token);
444525
let cur_total_shares = Self::total_shares(&env, &token);
445526

446527
let shares_to_mint =
@@ -491,6 +572,12 @@ impl LendingPool {
491572
.instance()
492573
.set(&DataKey::TotalDeposits(token.clone()), &new_total_deposits);
493574

575+
// Deposited principal joins the managed-asset base 1:1.
576+
let new_managed_assets = total_assets_before
577+
.checked_add(amount)
578+
.expect("managed assets overflow");
579+
Self::set_total_managed_assets(&env, &token, new_managed_assets);
580+
494581
Self::bump_instance_ttl(&env);
495582
deposit(
496583
&env,
@@ -518,7 +605,7 @@ impl LendingPool {
518605
}
519606
let asset_value = Self::calc_assets_to_redeem(
520607
shares,
521-
Self::read_pool_balance(&env, &token),
608+
Self::total_managed_assets(&env, &token),
522609
cur_total_shares,
523610
);
524611
(shares, asset_value)
@@ -536,7 +623,7 @@ impl LendingPool {
536623
}
537624
Self::calc_assets_to_redeem(
538625
shares,
539-
Self::read_pool_balance(&env, &token),
626+
Self::total_managed_assets(&env, &token),
540627
cur_total_shares,
541628
)
542629
}
@@ -548,13 +635,16 @@ impl LendingPool {
548635

549636
/// Current LP share price scaled by `SHARE_PRICE_SCALE`.
550637
/// `1_000_000` means 1.0 underlying asset per share.
638+
///
639+
/// Derived from internally-tracked managed assets, so it is stable while
640+
/// principal is out on loan and immune to direct-transfer manipulation.
551641
pub fn get_share_price(env: Env, token: Address) -> i128 {
552642
let total_shares = Self::total_shares(&env, &token);
553643
if total_shares <= 0 {
554644
return Self::SHARE_PRICE_SCALE;
555645
}
556646

557-
Self::read_pool_balance(&env, &token)
647+
Self::total_managed_assets(&env, &token)
558648
.checked_mul(Self::SHARE_PRICE_SCALE)
559649
.and_then(|v| v.checked_div(total_shares))
560650
.expect("share price overflow")
@@ -587,6 +677,42 @@ impl LendingPool {
587677
Self::redeem_shares(&env, &provider, &token, shares)
588678
}
589679

680+
/// Record `amount` of realized yield (e.g. loan interest repaid into the
681+
/// pool), raising every outstanding share's value pro-rata.
682+
///
683+
/// This is the *only* way assets enter share value besides deposits. Because
684+
/// a contract cannot distinguish a legitimate interest repayment from an
685+
/// unsolicited donation by looking at its balance, yield is recognised
686+
/// through this deliberate, admin-gated call rather than by reading the raw
687+
/// balance. That is precisely what stops a direct transfer from arbitrarily
688+
/// changing existing holders' redeemable value (issue #2).
689+
///
690+
/// Yield is not principal, so it does not move `TotalDeposits` or count
691+
/// against the `MaxPoolSize` cap. The caller is responsible for ensuring the
692+
/// corresponding tokens are actually present in the pool; otherwise later
693+
/// redemptions will hit the `InsufficientLiquidity` gate.
694+
pub fn record_yield(env: Env, token: Address, amount: i128) -> Result<(), PoolError> {
695+
Self::admin(&env).require_auth();
696+
Self::assert_not_paused(&env)?;
697+
698+
if amount <= 0 {
699+
return Err(PoolError::InvalidAmount);
700+
}
701+
702+
// Yield is only meaningful once shares exist to distribute it to.
703+
if Self::total_shares(&env, &token) <= 0 {
704+
return Err(PoolError::InvalidAmount);
705+
}
706+
707+
let new_managed_assets = Self::total_managed_assets(&env, &token)
708+
.checked_add(amount)
709+
.expect("managed assets overflow");
710+
Self::set_total_managed_assets(&env, &token, new_managed_assets);
711+
712+
yield_distributed(&env, token, amount);
713+
Ok(())
714+
}
715+
590716
// ── Queries ───────────────────────────────────────────────────────────
591717

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

0 commit comments

Comments
 (0)