Skip to content

Commit e6dd8b2

Browse files
committed
address review: wire record_yield into repayment path; CEI + invariant guard
Once share value tracks TotalManagedAssets instead of the raw balance, loan interest transferred into the pool no longer reaches LPs automatically. Wire it up: - lending_pool: add a per-token authorized yield reporter (set_loan_manager/get_loan_manager, admin-gated, emits LoanManagerUpdated) and gate record_yield to that reporter, falling back to admin when unset. - loan_manager: report the interest + late-fee portion of a repayment and the extension fee as yield via record_yield. Best-effort and non-fatal (only when configured as the pool's reporter; pool-side errors are swallowed) so it can never block a repayment. liquidate is intentionally left unwired and documented, since recovered interest needs a paired principal write-off (record_loss) to avoid inflating share value during a loss event. Review nits: - calc_shares_to_mint documents the managed-assets-positive invariant with a debug_assert instead of silently minting 1:1 when total_assets_before == 0. - redeem_shares now commits all accounting before the token transfer (CEI). Adds pool tests for reporter gating and end-to-end loan_manager tests proving repaid interest reaches LP share value and that repayment still succeeds when no reporter is configured.
1 parent 720bef2 commit e6dd8b2

9 files changed

Lines changed: 6241 additions & 9 deletions

lending_pool/src/events.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ pub fn yield_distributed(env: &Env, token: Address, amount: i128) {
2929
env.events().publish(topics, amount);
3030
}
3131

32+
pub fn loan_manager_updated(env: &Env, token: Address, loan_manager: Address) {
33+
let topics = (Symbol::new(env, "LoanManagerUpdated"), token);
34+
env.events().publish(topics, loan_manager);
35+
}
36+
3237
pub fn deposit_cap_updated(
3338
env: &Env,
3439
token: Address,

lending_pool/src/lib.rs

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ pub enum DataKey {
7575
/// exchange rate. Unaffected by direct token transfers or by principal
7676
/// temporarily out on loan. See the module-level accounting note.
7777
TotalManagedAssets(Address),
78+
/// token → address authorized to report realized yield via `record_yield`
79+
/// (normally the LoanManager that repayments flow through). Lets the
80+
/// repayment path credit interest to LPs automatically while keeping
81+
/// `record_yield` gated, so arbitrary callers still cannot move the share
82+
/// price.
83+
LoanManager(Address),
7884
/// token → number of active depositors
7985
DepositorCount(Address),
8086
ProposedAdmin,
@@ -172,6 +178,14 @@ impl LendingPool {
172178
Self::bump_instance_ttl(env);
173179
}
174180

181+
/// Address authorized to report realized yield for `token`, if configured.
182+
fn loan_manager(env: &Env, token: &Address) -> Option<Address> {
183+
Self::bump_instance_ttl(env);
184+
env.storage()
185+
.instance()
186+
.get(&DataKey::LoanManager(token.clone()))
187+
}
188+
175189
fn read_shares(env: &Env, provider: &Address, token: &Address) -> i128 {
176190
let key = DataKey::Shares(provider.clone(), token.clone());
177191
let shares: i128 = env.storage().persistent().get(&key).unwrap_or(0);
@@ -231,9 +245,20 @@ impl LendingPool {
231245
total_assets_before: i128,
232246
cur_total_shares: i128,
233247
) -> i128 {
234-
if cur_total_shares == 0 || total_assets_before == 0 {
248+
if cur_total_shares == 0 {
249+
// First depositor into an empty share pool: 1:1 allocation.
235250
amount
236251
} else {
252+
// Invariant: once shares exist, managed assets are strictly positive.
253+
// deposit and redeem move shares and managed assets together, and
254+
// record_yield only ever increases managed assets (and requires
255+
// shares > 0), so `total_assets_before == 0` here is unreachable. The
256+
// checked_div below would surface any violation as a panic rather
257+
// than silently minting a diluting 1:1 allocation.
258+
debug_assert!(
259+
total_assets_before > 0,
260+
"managed assets must be positive while shares are outstanding"
261+
);
237262
amount
238263
.checked_mul(cur_total_shares)
239264
.and_then(|v| v.checked_div(total_assets_before))
@@ -319,12 +344,6 @@ impl LendingPool {
319344
.and_then(|v| v.checked_div(cur_total_shares))
320345
.expect("principal redeem overflow");
321346

322-
TokenClient::new(env, token).transfer(
323-
&env.current_contract_address(),
324-
provider,
325-
&assets_to_return,
326-
);
327-
328347
let share_key = DataKey::Shares(provider.clone(), token.clone());
329348
let deposit_key = DataKey::DepositTimestamp(provider.clone(), token.clone());
330349
let remaining = cur_shares.checked_sub(shares).expect("share underflow");
@@ -363,6 +382,15 @@ impl LendingPool {
363382
.set(&DataKey::TotalDeposits(token.clone()), &new_total_deposits);
364383

365384
Self::bump_instance_ttl(env);
385+
386+
// Interaction last (checks-effects-interactions): all accounting is
387+
// committed before the token leaves the pool.
388+
TokenClient::new(env, token).transfer(
389+
&env.current_contract_address(),
390+
provider,
391+
&assets_to_return,
392+
);
393+
366394
Ok(assets_to_return)
367395
}
368396

@@ -702,22 +730,49 @@ impl LendingPool {
702730
Ok(())
703731
}
704732

733+
/// Authorize `loan_manager` to report realized yield for `token` via
734+
/// [`record_yield`](Self::record_yield). Set this to the LoanManager that
735+
/// repayments flow through so interest is credited to LPs automatically.
736+
/// Admin-gated.
737+
pub fn set_loan_manager(env: Env, token: Address, loan_manager: Address) {
738+
Self::admin(&env).require_auth();
739+
env.storage()
740+
.instance()
741+
.set(&DataKey::LoanManager(token.clone()), &loan_manager);
742+
Self::bump_instance_ttl(&env);
743+
loan_manager_updated(&env, token, loan_manager);
744+
}
745+
746+
/// The address authorized to report yield for `token`, if any.
747+
pub fn get_loan_manager(env: Env, token: Address) -> Option<Address> {
748+
Self::loan_manager(&env, &token)
749+
}
750+
705751
/// Record `amount` of realized yield (e.g. loan interest repaid into the
706752
/// pool), raising every outstanding share's value pro-rata.
707753
///
708754
/// This is the *only* way assets enter share value besides deposits. Because
709755
/// a contract cannot distinguish a legitimate interest repayment from an
710756
/// unsolicited donation by looking at its balance, yield is recognised
711-
/// through this deliberate, admin-gated call rather than by reading the raw
757+
/// through this deliberate, access-gated call rather than by reading the raw
712758
/// balance. That is precisely what stops a direct transfer from arbitrarily
713759
/// changing existing holders' redeemable value (issue #2).
714760
///
761+
/// Authorization: the configured [`LoanManager`] reporter for `token` (so the
762+
/// repayment path can credit interest automatically), or the admin when no
763+
/// reporter is configured (manual / keeper operation).
764+
///
715765
/// Yield is not principal, so it does not move `TotalDeposits` or count
716766
/// against the `MaxPoolSize` cap. The caller is responsible for ensuring the
717767
/// corresponding tokens are actually present in the pool; otherwise later
718768
/// redemptions will hit the `InsufficientLiquidity` gate.
719769
pub fn record_yield(env: Env, token: Address, amount: i128) -> Result<(), PoolError> {
720-
Self::admin(&env).require_auth();
770+
// Gate on the configured yield reporter, falling back to admin so the
771+
// function is still usable manually before a LoanManager is wired up.
772+
match Self::loan_manager(&env, &token) {
773+
Some(reporter) => reporter.require_auth(),
774+
None => Self::admin(&env).require_auth(),
775+
}
721776
Self::assert_not_paused(&env)?;
722777

723778
if amount <= 0 {

lending_pool/src/test.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,3 +1683,87 @@ fn test_record_yield_rejected_when_no_shares_outstanding() {
16831683
let res = pool_client.try_record_yield(&token_id, &100);
16841684
assert_eq!(res, Err(Ok(crate::PoolError::InvalidAmount)));
16851685
}
1686+
1687+
#[test]
1688+
fn test_record_yield_gated_to_configured_reporter() {
1689+
// Once a loan-manager reporter is configured for a token, it (and not the
1690+
// admin) is the authority for record_yield — this is what lets the
1691+
// repayment path credit interest automatically while still blocking
1692+
// arbitrary callers.
1693+
let env = Env::default();
1694+
1695+
let admin = Address::generate(&env);
1696+
let (token_id, stellar_asset_client, _token_client) = create_token_contract(&env, &admin);
1697+
1698+
let pool_id = env.register(LendingPool, ());
1699+
let pool_client = LendingPoolClient::new(&env, &pool_id);
1700+
1701+
env.mock_all_auths();
1702+
pool_client.initialize(&admin);
1703+
pool_client.set_withdrawal_cooldown(&0);
1704+
1705+
let provider = Address::generate(&env);
1706+
stellar_asset_client.mint(&provider, &1_000);
1707+
pool_client.deposit(&provider, &token_id, &1_000);
1708+
1709+
let reporter = Address::generate(&env);
1710+
pool_client.set_loan_manager(&token_id, &reporter);
1711+
assert_eq!(
1712+
pool_client.get_loan_manager(&token_id),
1713+
Some(reporter.clone())
1714+
);
1715+
1716+
// The configured reporter can record yield.
1717+
env.mock_auths(&[soroban_sdk::testutils::MockAuth {
1718+
address: &reporter,
1719+
invoke: &soroban_sdk::testutils::MockAuthInvoke {
1720+
contract: &pool_id,
1721+
fn_name: "record_yield",
1722+
args: (token_id.clone(), 100i128).into_val(&env),
1723+
sub_invokes: &[],
1724+
},
1725+
}]);
1726+
pool_client.record_yield(&token_id, &100);
1727+
assert_eq!(pool_client.get_total_managed_assets(&token_id), 1_100);
1728+
1729+
// The admin is no longer the authority once a reporter is configured.
1730+
env.mock_auths(&[soroban_sdk::testutils::MockAuth {
1731+
address: &admin,
1732+
invoke: &soroban_sdk::testutils::MockAuthInvoke {
1733+
contract: &pool_id,
1734+
fn_name: "record_yield",
1735+
args: (token_id.clone(), 50i128).into_val(&env),
1736+
sub_invokes: &[],
1737+
},
1738+
}]);
1739+
assert!(pool_client.try_record_yield(&token_id, &50).is_err());
1740+
}
1741+
1742+
#[test]
1743+
fn test_set_loan_manager_requires_admin() {
1744+
let env = Env::default();
1745+
1746+
let admin = Address::generate(&env);
1747+
let (token_id, _stellar_asset_client, _token_client) = create_token_contract(&env, &admin);
1748+
1749+
let pool_id = env.register(LendingPool, ());
1750+
let pool_client = LendingPoolClient::new(&env, &pool_id);
1751+
1752+
env.mock_all_auths();
1753+
pool_client.initialize(&admin);
1754+
1755+
let reporter = Address::generate(&env);
1756+
let attacker = Address::generate(&env);
1757+
env.mock_auths(&[soroban_sdk::testutils::MockAuth {
1758+
address: &attacker,
1759+
invoke: &soroban_sdk::testutils::MockAuthInvoke {
1760+
contract: &pool_id,
1761+
fn_name: "set_loan_manager",
1762+
args: (token_id.clone(), reporter.clone()).into_val(&env),
1763+
sub_invokes: &[],
1764+
},
1765+
}]);
1766+
assert!(pool_client
1767+
.try_set_loan_manager(&token_id, &reporter)
1768+
.is_err());
1769+
}

0 commit comments

Comments
 (0)