Skip to content

Commit f1dabd5

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 ff8e907 commit f1dabd5

11 files changed

Lines changed: 7153 additions & 235 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);
@@ -242,9 +256,20 @@ impl LendingPool {
242256
total_assets_before: i128,
243257
cur_total_shares: i128,
244258
) -> i128 {
245-
if cur_total_shares == 0 || total_assets_before == 0 {
259+
if cur_total_shares == 0 {
260+
// First depositor into an empty share pool: 1:1 allocation.
246261
amount
247262
} else {
263+
// Invariant: once shares exist, managed assets are strictly positive.
264+
// deposit and redeem move shares and managed assets together, and
265+
// record_yield only ever increases managed assets (and requires
266+
// shares > 0), so `total_assets_before == 0` here is unreachable. The
267+
// checked_div below would surface any violation as a panic rather
268+
// than silently minting a diluting 1:1 allocation.
269+
debug_assert!(
270+
total_assets_before > 0,
271+
"managed assets must be positive while shares are outstanding"
272+
);
248273
amount
249274
.checked_mul(cur_total_shares)
250275
.and_then(|v| v.checked_div(total_assets_before))
@@ -342,12 +367,6 @@ impl LendingPool {
342367
.and_then(|v| v.checked_div(cur_total_shares))
343368
.expect("principal redeem overflow");
344369

345-
TokenClient::new(env, token).transfer(
346-
&env.current_contract_address(),
347-
provider,
348-
&assets_to_return,
349-
);
350-
351370
let share_key = DataKey::Shares(provider.clone(), token.clone());
352371
let deposit_key = DataKey::DepositTimestamp(provider.clone(), token.clone());
353372
let remaining = cur_shares.checked_sub(shares).expect("share underflow");
@@ -386,6 +405,15 @@ impl LendingPool {
386405
.set(&DataKey::TotalDeposits(token.clone()), &new_total_deposits);
387406

388407
Self::bump_instance_ttl(env);
408+
409+
// Interaction last (checks-effects-interactions): all accounting is
410+
// committed before the token leaves the pool.
411+
TokenClient::new(env, token).transfer(
412+
&env.current_contract_address(),
413+
provider,
414+
&assets_to_return,
415+
);
416+
389417
Ok(assets_to_return)
390418
}
391419

@@ -734,22 +762,49 @@ impl LendingPool {
734762
Ok(())
735763
}
736764

765+
/// Authorize `loan_manager` to report realized yield for `token` via
766+
/// [`record_yield`](Self::record_yield). Set this to the LoanManager that
767+
/// repayments flow through so interest is credited to LPs automatically.
768+
/// Admin-gated.
769+
pub fn set_loan_manager(env: Env, token: Address, loan_manager: Address) {
770+
Self::admin(&env).require_auth();
771+
env.storage()
772+
.instance()
773+
.set(&DataKey::LoanManager(token.clone()), &loan_manager);
774+
Self::bump_instance_ttl(&env);
775+
loan_manager_updated(&env, token, loan_manager);
776+
}
777+
778+
/// The address authorized to report yield for `token`, if any.
779+
pub fn get_loan_manager(env: Env, token: Address) -> Option<Address> {
780+
Self::loan_manager(&env, &token)
781+
}
782+
737783
/// Record `amount` of realized yield (e.g. loan interest repaid into the
738784
/// pool), raising every outstanding share's value pro-rata.
739785
///
740786
/// This is the *only* way assets enter share value besides deposits. Because
741787
/// a contract cannot distinguish a legitimate interest repayment from an
742788
/// unsolicited donation by looking at its balance, yield is recognised
743-
/// through this deliberate, admin-gated call rather than by reading the raw
789+
/// through this deliberate, access-gated call rather than by reading the raw
744790
/// balance. That is precisely what stops a direct transfer from arbitrarily
745791
/// changing existing holders' redeemable value (issue #2).
746792
///
793+
/// Authorization: the configured [`LoanManager`] reporter for `token` (so the
794+
/// repayment path can credit interest automatically), or the admin when no
795+
/// reporter is configured (manual / keeper operation).
796+
///
747797
/// Yield is not principal, so it does not move `TotalDeposits` or count
748798
/// against the `MaxPoolSize` cap. The caller is responsible for ensuring the
749799
/// corresponding tokens are actually present in the pool; otherwise later
750800
/// redemptions will hit the `InsufficientLiquidity` gate.
751801
pub fn record_yield(env: Env, token: Address, amount: i128) -> Result<(), PoolError> {
752-
Self::admin(&env).require_auth();
802+
// Gate on the configured yield reporter, falling back to admin so the
803+
// function is still usable manually before a LoanManager is wired up.
804+
match Self::loan_manager(&env, &token) {
805+
Some(reporter) => reporter.require_auth(),
806+
None => Self::admin(&env).require_auth(),
807+
}
753808
Self::assert_not_paused(&env)?;
754809

755810
if amount <= 0 {

lending_pool/src/test.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,3 +1707,87 @@ fn test_record_yield_rejected_when_no_shares_outstanding() {
17071707
let res = pool_client.try_record_yield(&token_id, &100);
17081708
assert_eq!(res, Err(Ok(crate::PoolError::InvalidAmount)));
17091709
}
1710+
1711+
#[test]
1712+
fn test_record_yield_gated_to_configured_reporter() {
1713+
// Once a loan-manager reporter is configured for a token, it (and not the
1714+
// admin) is the authority for record_yield — this is what lets the
1715+
// repayment path credit interest automatically while still blocking
1716+
// arbitrary callers.
1717+
let env = Env::default();
1718+
1719+
let admin = Address::generate(&env);
1720+
let (token_id, stellar_asset_client, _token_client) = create_token_contract(&env, &admin);
1721+
1722+
let pool_id = env.register(LendingPool, ());
1723+
let pool_client = LendingPoolClient::new(&env, &pool_id);
1724+
1725+
env.mock_all_auths();
1726+
pool_client.initialize(&admin);
1727+
pool_client.set_withdrawal_cooldown(&0);
1728+
1729+
let provider = Address::generate(&env);
1730+
stellar_asset_client.mint(&provider, &1_000);
1731+
pool_client.deposit(&provider, &token_id, &1_000);
1732+
1733+
let reporter = Address::generate(&env);
1734+
pool_client.set_loan_manager(&token_id, &reporter);
1735+
assert_eq!(
1736+
pool_client.get_loan_manager(&token_id),
1737+
Some(reporter.clone())
1738+
);
1739+
1740+
// The configured reporter can record yield.
1741+
env.mock_auths(&[soroban_sdk::testutils::MockAuth {
1742+
address: &reporter,
1743+
invoke: &soroban_sdk::testutils::MockAuthInvoke {
1744+
contract: &pool_id,
1745+
fn_name: "record_yield",
1746+
args: (token_id.clone(), 100i128).into_val(&env),
1747+
sub_invokes: &[],
1748+
},
1749+
}]);
1750+
pool_client.record_yield(&token_id, &100);
1751+
assert_eq!(pool_client.get_total_managed_assets(&token_id), 1_100);
1752+
1753+
// The admin is no longer the authority once a reporter is configured.
1754+
env.mock_auths(&[soroban_sdk::testutils::MockAuth {
1755+
address: &admin,
1756+
invoke: &soroban_sdk::testutils::MockAuthInvoke {
1757+
contract: &pool_id,
1758+
fn_name: "record_yield",
1759+
args: (token_id.clone(), 50i128).into_val(&env),
1760+
sub_invokes: &[],
1761+
},
1762+
}]);
1763+
assert!(pool_client.try_record_yield(&token_id, &50).is_err());
1764+
}
1765+
1766+
#[test]
1767+
fn test_set_loan_manager_requires_admin() {
1768+
let env = Env::default();
1769+
1770+
let admin = Address::generate(&env);
1771+
let (token_id, _stellar_asset_client, _token_client) = create_token_contract(&env, &admin);
1772+
1773+
let pool_id = env.register(LendingPool, ());
1774+
let pool_client = LendingPoolClient::new(&env, &pool_id);
1775+
1776+
env.mock_all_auths();
1777+
pool_client.initialize(&admin);
1778+
1779+
let reporter = Address::generate(&env);
1780+
let attacker = Address::generate(&env);
1781+
env.mock_auths(&[soroban_sdk::testutils::MockAuth {
1782+
address: &attacker,
1783+
invoke: &soroban_sdk::testutils::MockAuthInvoke {
1784+
contract: &pool_id,
1785+
fn_name: "set_loan_manager",
1786+
args: (token_id.clone(), reporter.clone()).into_val(&env),
1787+
sub_invokes: &[],
1788+
},
1789+
}]);
1790+
assert!(pool_client
1791+
.try_set_loan_manager(&token_id, &reporter)
1792+
.is_err());
1793+
}

0 commit comments

Comments
 (0)