Skip to content

fix(lending): prevent protocol fee collection from draining depositor collateral (#457) #463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
huangji6693-max wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(lending): prevent protocol fee collection from draining depositor collateral (#457) #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 48 additions & 11 deletions contracts/lending/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ pub enum DataKey {
CollateralConfig(Address),
/// User's deposited collateral positions: map user -> Vec<CollateralEntry>.
UserCollateral(Address),
/// Aggregate deposited collateral across all borrowers for an asset (Issue #457).
TotalCollateral(Address),
/// Price oracle address that can post price feeds for collateral assets.
PriceOracle,
/// Aggregated oracle price samples for a collateral asset.
Expand Down Expand Up @@ -777,8 +779,12 @@ impl LendingContract {
.expect("Fee token not configured");
let token_client = token::Client::new(&env, &fee_token);
let contract_address = env.current_contract_address();
if token_client.balance(&contract_address) < uncollected {
panic!("Insufficient fee token balance to collect fees");
let collateral_total = Self::get_total_collateral_for_asset(env.clone(), fee_token.clone());
let available_fees = token_client
.balance(&contract_address)
.saturating_sub(collateral_total);
if available_fees < uncollected {
panic!("Insufficient unencumbered fee token balance to collect fees");
}

env.storage()
Expand Down Expand Up @@ -1082,6 +1088,13 @@ impl LendingContract {

env.storage().persistent().set(&key, &new_entries);

let total_key = DataKey::TotalCollateral(asset.clone());
let current_total: i128 = env.storage().persistent().get(&total_key).unwrap_or(0);
let updated_total = current_total
.checked_add(amount)
.expect("Overflow updating total collateral");
env.storage().persistent().set(&total_key, &updated_total);

// Check if health factor improved after deposit (Issue #157)
// If health factor is now >= 1.0, clear grace periods for all borrower's loans
let health_factor = Self::get_health_factor(env.clone(), borrower.clone());
Expand Down Expand Up @@ -1184,6 +1197,17 @@ impl LendingContract {
env.storage().persistent().set(&key, &new_entries);
}

let total_key = DataKey::TotalCollateral(asset.clone());
let current_total: i128 = env.storage().persistent().get(&total_key).unwrap_or(0);
let updated_total = current_total
.checked_sub(amount)
.expect("Underflow updating total collateral");
if updated_total == 0 {
env.storage().persistent().remove(&total_key);
} else {
env.storage().persistent().set(&total_key, &updated_total);
}

env.events().publish(
(symbol_short!("collat"), symbol_short!("withdraw")),
(borrower, asset, amount, remaining_amount),
Expand All @@ -1200,6 +1224,14 @@ impl LendingContract {
.unwrap_or(Vec::new(&env))
}

/// Get total deposited collateral across all borrowers for a given asset (Issue #457).
pub fn get_total_collateral_for_asset(env: Env, asset: Address) -> i128 {
env.storage()
.persistent()
.get(&DataKey::TotalCollateral(asset))
.unwrap_or(0)
}

/// Get the total borrowing power for a borrower in the base asset's
/// smallest unit (stroops for XLM).
///
Expand Down Expand Up @@ -1378,15 +1410,6 @@ impl LendingContract {
.persistent()
.set(&DataKey::LoanReputationTier(loan_id), &reputation_tier);

let current_fees: i128 = env
.storage()
.instance()
.get(&DataKey::UncollectedFees)
.unwrap_or(0);
env.storage()
.instance()
.set(&DataKey::UncollectedFees, &(current_fees + platform_fee));

// Track per-borrower list
Self::store_borrower_loan_id(&env, &borrower, loan_id);

Expand Down Expand Up @@ -1531,6 +1554,20 @@ impl LendingContract {
loan.remaining_due = 0;
loan.status = LoanStatus::Repaid;

if loan.platform_fee > 0 {
let current_fees: i128 = env
.storage()
.instance()
.get(&DataKey::UncollectedFees)
.unwrap_or(0);
let updated_fees = current_fees
.checked_add(loan.platform_fee)
.expect("Overflow accruing platform fees");
env.storage()
.instance()
.set(&DataKey::UncollectedFees, &updated_fees);
}

// Distribute loyalty rewards if repaid on time
let now = env.ledger().timestamp();
if now <= loan.due_at {
Expand Down
149 changes: 148 additions & 1 deletion contracts/lending/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3402,7 +3402,7 @@ fn setup_fee_collection(fund: bool) -> (Env, Address, Address, Address, Address,
client.set_treasury(&admin, &treasury_id);
client.set_fee_token(&admin, &fee_token);

client.create_loan_request(
let loan_id = client.create_loan_request(
&borrower,
&create_request_struct(
&env,
Expand All @@ -3415,6 +3415,14 @@ fn setup_fee_collection(fund: bool) -> (Env, Address, Address, Address, Address,
InterestRateModel::Fixed,
),
);
// Fees do not accrue upfront before repayment (#457)
assert_eq!(client.get_uncollected_fees(), 0);
let lender = Address::generate(&env);
client.approve_loan(&lender, &loan_id, &1);
client.activate_loan(&admin, &loan_id);
let loan = client.get_loan(&loan_id);
client.record_payment(&admin, &loan_id, &loan.total_due);

let uncollected = client.get_uncollected_fees();
assert!(uncollected > 0);
if fund {
Expand Down Expand Up @@ -3559,3 +3567,142 @@ fn test_sweep_cannot_take_uncollected_fees() {
client.sweep_tokens(&admin, &fee_token, &recipient, &100);
assert_eq!(token::Client::new(&env, &fee_token).balance(&contract_id), uncollected);
}

#[test]
fn test_collect_fees_cannot_drain_depositor_collateral() {
let (env, contract_id, admin, borrower, _dummy_asset) = setup();
let client = LendingContractClient::new(&env, &contract_id);

// Deploy a real SEP-41 asset contract that acts as both whitelisted collateral and fee token
let collateral_asset = env
.register_stellar_asset_contract_v2(admin.clone())
.address();
client.whitelist_asset(&admin, &collateral_asset);
client.set_asset_collateral_config(
&admin,
&collateral_asset,
&AssetCollateralConfig {
collateral_factor_bps: DEFAULT_COLLATERAL_FACTOR_BPS,
has_price_oracle: true,
volatility_bps: 0,
},
);
client.set_asset_oracle_prices(&admin, &collateral_asset, &vec![&env, PRICE_PRECISION]);

let treasury_id = env.register(mock_treasury::MockTreasury, ());
client.set_treasury(&admin, &treasury_id);
client.set_fee_token(&admin, &collateral_asset);

// Borrower deposits 50,000 tokens of collateral
let collateral_deposit = 50_000_0000000i128;
mint_tokens(&env, &collateral_asset, &borrower, collateral_deposit);
client.deposit_collateral(&borrower, &collateral_asset, &collateral_deposit);
assert_eq!(
client.get_total_collateral_for_asset(&collateral_asset),
collateral_deposit
);

// Create, approve, activate, and repay a loan so platform fees accrue
let loan_id = client.create_loan_request(
&borrower,
&create_request_struct(
&env,
1_000_0000000,
30,
1000,
100_000_0000000,
&collateral_asset,
collateral_deposit,
InterestRateModel::Fixed,
),
);
assert_eq!(client.get_uncollected_fees(), 0);

let lender = Address::generate(&env);
client.approve_loan(&lender, &loan_id, &1);
client.activate_loan(&admin, &loan_id);
let loan = client.get_loan(&loan_id);
client.record_payment(&admin, &loan_id, &loan.total_due);

let uncollected = client.get_uncollected_fees();
assert!(uncollected > 0);

// Contract holds only the borrower's collateral (no unencumbered fee tokens)
let token_client = token::Client::new(&env, &collateral_asset);
assert_eq!(token_client.balance(&contract_id), collateral_deposit);

// Fee collection must fail because all held tokens are encumbered collateral
assert!(client.try_collect_fees(&admin, &treasury_id).is_err());
assert_eq!(client.get_uncollected_fees(), uncollected);
assert_eq!(token_client.balance(&contract_id), collateral_deposit);
assert_eq!(token_client.balance(&treasury_id), 0);

// Borrower can still withdraw their full collateral position without panic
client.withdraw_collateral(&borrower, &collateral_asset, &collateral_deposit);
assert_eq!(client.get_total_collateral_for_asset(&collateral_asset), 0);
assert_eq!(token_client.balance(&borrower), collateral_deposit);
}

#[test]
fn test_collect_fees_succeeds_when_unencumbered_fee_tokens_cover_accrual() {
let (env, contract_id, admin, borrower, _dummy_asset) = setup();
let client = LendingContractClient::new(&env, &contract_id);

let collateral_asset = env
.register_stellar_asset_contract_v2(admin.clone())
.address();
client.whitelist_asset(&admin, &collateral_asset);
client.set_asset_collateral_config(
&admin,
&collateral_asset,
&AssetCollateralConfig {
collateral_factor_bps: DEFAULT_COLLATERAL_FACTOR_BPS,
has_price_oracle: true,
volatility_bps: 0,
},
);
client.set_asset_oracle_prices(&admin, &collateral_asset, &vec![&env, PRICE_PRECISION]);

let treasury_id = env.register(mock_treasury::MockTreasury, ());
client.set_treasury(&admin, &treasury_id);
client.set_fee_token(&admin, &collateral_asset);

let collateral_deposit = 50_000_0000000i128;
mint_tokens(&env, &collateral_asset, &borrower, collateral_deposit);
client.deposit_collateral(&borrower, &collateral_asset, &collateral_deposit);

let loan_id = client.create_loan_request(
&borrower,
&create_request_struct(
&env,
1_000_0000000,
30,
1000,
100_000_0000000,
&collateral_asset,
collateral_deposit,
InterestRateModel::Fixed,
),
);
let lender = Address::generate(&env);
client.approve_loan(&lender, &loan_id, &1);
client.activate_loan(&admin, &loan_id);
let loan = client.get_loan(&loan_id);
client.record_payment(&admin, &loan_id, &loan.total_due);

let uncollected = client.get_uncollected_fees();
assert!(uncollected > 0);

// Fund the contract with the unencumbered fee tokens backing the accrual
mint_tokens(&env, &collateral_asset, &contract_id, uncollected);

let collected = client.collect_fees(&admin, &treasury_id);
assert_eq!(collected, uncollected);
assert_eq!(client.get_uncollected_fees(), 0);

// Depositor collateral remains 100% intact and withdrawable
let token_client = token::Client::new(&env, &collateral_asset);
assert_eq!(token_client.balance(&contract_id), collateral_deposit);
client.withdraw_collateral(&borrower, &collateral_asset, &collateral_deposit);
assert_eq!(token_client.balance(&borrower), collateral_deposit);
}