From 40f70af019cd4400695985101e094e08b91de911 Mon Sep 17 00:00:00 2001 From: sublime247 Date: Wed, 26 Aug 2026 08:42:31 +0100 Subject: [PATCH 1/2] fix(creditline): chunk borrower loan index into fixed-size persistent pages and audit extend_ttl --- context/progress-tracker.md | 10 +++ contracts/creditline-contract/src/lib.rs | 4 + contracts/creditline-contract/src/storage.rs | 50 ++++++++++--- contracts/creditline-contract/src/tests.rs | 78 ++++++++++++++++++++ 4 files changed, 133 insertions(+), 9 deletions(-) diff --git a/context/progress-tracker.md b/context/progress-tracker.md index a43add4..73f2863 100644 --- a/context/progress-tracker.md +++ b/context/progress-tracker.md @@ -16,6 +16,16 @@ Update this file after every completed contract change, fix, or architectural de ## Completed +### Storage Layout & Bounded Per-User Loan Index Chunking (creditline-contract) +- **Problem:** Per-borrower loan indexes were previously stored without index vector caps, risking footprint and entry size bloat under high loan counts. +- **Fix:** Changed storage key layout in `creditline-contract/src/storage.rs` from individual key index to fixed-size persistent pages `DataKey::UserLoanPage(Address, u32)` with `PAGE_SIZE = 32`. +- Updated `append_user_loan_index` to append loan IDs to the current `UserLoanPage(borrower, page_num)` and immediately call `extend_ttl`. +- Updated `get_user_loan_ids_paginated` to fetch across chunked page boundaries dynamically given zero-indexed `start` and `limit`. +- Audited all persistent storage write paths (`write_loan`, `increase_user_active_debt`, `decrease_user_active_debt`, `append_user_loan_index`) ensuring every `.set()` call is paired with an immediate `extend_ttl`. +- Documented pagination storage contract (`PAGE_SIZE = 32`) in `storage.rs` and `get_user_loans` API doc comments in `lib.rs`. +- Added `test_200_loan_borrower_stress_and_storage_layout_regression` in `tests.rs` asserting that a 200-loan borrower can create loan #201, query loans across page boundaries (`0..32`, `32..64`, `30..40`, `200..201`), and repay loans without footprint or capacity errors. +- Verified test suite: 125 tests passing in `creditline-contract`, full workspace suite green. + ### Issue #58 — Principal-Interest-Fee Repayment Waterfall - Added `RepaymentAllocation` struct and `apply_waterfall()` helper in `lib.rs` with correct priority: late fees → interest → service fee → principal - Fixed `repay_loan()` to use the corrected waterfall order (was principal-first, now late-fees-first) diff --git a/contracts/creditline-contract/src/lib.rs b/contracts/creditline-contract/src/lib.rs index 665d2d2..f74b9a3 100644 --- a/contracts/creditline-contract/src/lib.rs +++ b/contracts/creditline-contract/src/lib.rs @@ -167,6 +167,10 @@ impl CreditLineContract { Ok(loan_id) } + /// Retrieve loans for a borrower paginated across fixed-size persistent index pages (`PAGE_SIZE = 32`). + /// + /// - `start`: zero-based loan index offset. + /// - `limit`: max number of loans to return. pub fn get_user_loans(env: Env, borrower: Address, start: u64, limit: u32) -> Vec { storage::get_user_loans_paginated(&env, &borrower, start, limit) .unwrap_or_else(|err| panic_with_error!(&env, err)) diff --git a/contracts/creditline-contract/src/storage.rs b/contracts/creditline-contract/src/storage.rs index 99d2c17..01a6c90 100644 --- a/contracts/creditline-contract/src/storage.rs +++ b/contracts/creditline-contract/src/storage.rs @@ -13,6 +13,10 @@ pub const TOKEN: Symbol = symbol_short!("TOKEN"); pub const PARAMETERS_CONTRACT: Symbol = symbol_short!("PARAMS"); pub const REENTRANCY_LOCK: Symbol = symbol_short!("LOCKED"); +/// Fixed max capacity per per-borrower persistent index page entry. +/// Chunking index entries into 32-element vectors prevents footprint & instance/persistent size bloat. +pub const PAGE_SIZE: u32 = 32; + const LOAN_SHARD_COUNT: u32 = 32; #[contracttype] @@ -20,7 +24,7 @@ const LOAN_SHARD_COUNT: u32 = 32; enum DataKey { Loan(u32, u64), UserLoanCount(Address), - UserLoanAt(Address, u64), + UserLoanPage(Address, u32), UserActiveDebt(Address), } @@ -59,7 +63,7 @@ pub fn read_loan(env: &Env, loan_id: u64) -> Result { .ok_or(CreditLineError::LoanNotFound) } -/// Write a loan to storage +/// Write a loan to storage and extend persistent TTL pub fn write_loan(env: &Env, loan: &Loan) { let shard = loan_shard(loan.loan_id); let key = DataKey::Loan(shard, loan.loan_id); @@ -80,6 +84,12 @@ pub fn get_user_loan_count(env: &Env, borrower: &Address) -> Result(&key) { - result.push_back(loan_id); + let page_num = (idx / (PAGE_SIZE as u64)) as u32; + let page_offset = (idx % (PAGE_SIZE as u64)) as u32; + let key = DataKey::UserLoanPage(borrower.clone(), page_num); + + if let Some(page) = env.storage().persistent().get::>(&key) { + let page_len = page.len(); + let mut offset = page_offset; + while offset < page_len && idx < end { + if let Some(loan_id) = page.get(offset) { + result.push_back(loan_id); + } + idx += 1; + offset += 1; + } + } else { + return Err(CreditLineError::LoanNotFound); } - idx += 1; } Ok(result) @@ -163,9 +185,19 @@ pub fn decrease_user_active_debt( fn append_user_loan_index(env: &Env, borrower: &Address, loan_id: u64) { let count = get_user_loan_count(env, borrower) .unwrap_or_else(|err| soroban_sdk::panic_with_error!(env, err)); - let loan_at_key = DataKey::UserLoanAt(borrower.clone(), count); - env.storage().persistent().set(&loan_at_key, &loan_id); - extend_persistent_ttl(env, &loan_at_key); + + let page_num = (count / (PAGE_SIZE as u64)) as u32; + let page_key = DataKey::UserLoanPage(borrower.clone(), page_num); + + let mut page: Vec = env + .storage() + .persistent() + .get(&page_key) + .unwrap_or_else(|| Vec::new(env)); + page.push_back(loan_id); + + env.storage().persistent().set(&page_key, &page); + extend_persistent_ttl(env, &page_key); let count_key = DataKey::UserLoanCount(borrower.clone()); let next_count = count diff --git a/contracts/creditline-contract/src/tests.rs b/contracts/creditline-contract/src/tests.rs index f99158b..41d911f 100644 --- a/contracts/creditline-contract/src/tests.rs +++ b/contracts/creditline-contract/src/tests.rs @@ -3915,3 +3915,81 @@ fn test_mark_defaulted_loss_absorption_share_price_impact() { assert_eq!(pool_stats.locked_liquidity, 0); assert!(pool_stats.total_liquidity > 0); } + +#[test] +fn test_200_loan_borrower_stress_and_storage_layout_regression() { + let ctx = TestCtx::setup(); + let borrower = Address::generate(&ctx.env); + let vendor = Address::generate(&ctx.env); + + ctx.register_vendor(&vendor, "High Volume Vendor"); + + // Mint enough balance for 205 loan guarantees (205 * 200 = 41,000) + ctx.mint(&borrower, 50_000); + + let due_date = ctx.env.ledger().timestamp() + 10_000; + let schedule = ctx.single_installment(DEFAULT_TOTAL_DUE, due_date); + + // Create 200 loans for the single borrower (repaying each so active debt stays within exposure limit) + for i in 0..200 { + let loan_id = ctx.client.create_loan( + &borrower, + &vendor, + &DEFAULT_PRINCIPAL, + &DEFAULT_GUARANTEE, + &schedule, + &LoanType::Standard, + ); + assert_eq!(loan_id, (i + 1) as u64); + ctx.mint(&borrower, DEFAULT_TOTAL_DUE); + ctx.client.repay_loan(&borrower, &loan_id, &DEFAULT_TOTAL_DUE); + } + + assert_eq!(ctx.client.get_user_loan_count(&borrower), 200); + + // Verify 201st loan creation succeeds without storage footprint or capacity failure + let loan_id_201 = ctx.client.create_loan( + &borrower, + &vendor, + &DEFAULT_PRINCIPAL, + &DEFAULT_GUARANTEE, + &schedule, + &LoanType::Standard, + ); + assert_eq!(loan_id_201, 201); + assert_eq!(ctx.client.get_user_loan_count(&borrower), 201); + + // Test paginated retrieval across page boundaries (PAGE_SIZE = 32) + // Page 0 (0..32) + let page_0_loans = ctx.client.get_user_loans(&borrower, &0, &32); + assert_eq!(page_0_loans.len(), 32); + assert_eq!(page_0_loans.get(0).unwrap().loan_id, 1); + assert_eq!(page_0_loans.get(31).unwrap().loan_id, 32); + + // Page 1 (32..64) + let page_1_loans = ctx.client.get_user_loans(&borrower, &32, &32); + assert_eq!(page_1_loans.len(), 32); + assert_eq!(page_1_loans.get(0).unwrap().loan_id, 33); + + // Cross-page boundary request (start=30, limit=10 -> loans 31..41) + let cross_page_loans = ctx.client.get_user_loans(&borrower, &30, &10); + assert_eq!(cross_page_loans.len(), 10); + assert_eq!(cross_page_loans.get(0).unwrap().loan_id, 31); + assert_eq!(cross_page_loans.get(9).unwrap().loan_id, 40); + + // Tail page request (start=200, limit=10 -> loan 201) + let tail_loans = ctx.client.get_user_loans(&borrower, &200, &10); + assert_eq!(tail_loans.len(), 1); + assert_eq!(tail_loans.get(0).unwrap().loan_id, 201); + + // Verify repaying a loan works cleanly for a 200+ loan borrower + ctx.mint(&borrower, DEFAULT_TOTAL_DUE); + let remaining = ctx + .client + .repay_loan(&borrower, &loan_id_201, &DEFAULT_TOTAL_DUE); + assert_eq!(remaining, 0); + + let loan_201 = ctx.client.get_loan(&loan_id_201); + assert_eq!(loan_201.status, LoanStatus::Paid); +} + From 4d53270d3478d4dcc9c0f97e58f1940397f84300 Mon Sep 17 00:00:00 2001 From: sublime247 Date: Wed, 26 Aug 2026 09:16:43 +0100 Subject: [PATCH 2/2] fix(storage): add legacy index fallback, storage entry class regression tests, and active debt TTL coverage --- .github/workflows/contracts-ci.yml | 4 +- contracts/creditline-contract/src/storage.rs | 17 ++- contracts/creditline-contract/src/tests.rs | 115 +++++++++++++++++++ 3 files changed, 132 insertions(+), 4 deletions(-) diff --git a/.github/workflows/contracts-ci.yml b/.github/workflows/contracts-ci.yml index 48e7320..9d53b29 100644 --- a/.github/workflows/contracts-ci.yml +++ b/.github/workflows/contracts-ci.yml @@ -2,9 +2,9 @@ name: Contracts CI on: push: - branches: [ main, develop ] + branches: [ "**" ] pull_request: - branches: [ main, develop ] + branches: [ "**" ] env: CARGO_TERM_COLOR: always diff --git a/contracts/creditline-contract/src/storage.rs b/contracts/creditline-contract/src/storage.rs index 01a6c90..0cd3cca 100644 --- a/contracts/creditline-contract/src/storage.rs +++ b/contracts/creditline-contract/src/storage.rs @@ -21,11 +21,12 @@ const LOAN_SHARD_COUNT: u32 = 32; #[contracttype] #[derive(Clone)] -enum DataKey { +pub enum DataKey { Loan(u32, u64), UserLoanCount(Address), UserLoanPage(Address, u32), UserActiveDebt(Address), + UserLoanAt(Address, u64), // Legacy un-chunked index key for backward compatibility } /// Get the admin address from storage @@ -121,7 +122,19 @@ pub fn get_user_loan_ids_paginated( offset += 1; } } else { - return Err(CreditLineError::LoanNotFound); + // Backward compatibility fallback for legacy un-chunked UserLoanAt(borrower, idx) entries + let legacy_key = DataKey::UserLoanAt(borrower.clone(), idx); + if let Some(loan_id) = env + .storage() + .persistent() + .get::(&legacy_key) + .or_else(|| env.storage().instance().get::(&legacy_key)) + { + result.push_back(loan_id); + idx += 1; + } else { + return Err(CreditLineError::LoanNotFound); + } } } diff --git a/contracts/creditline-contract/src/tests.rs b/contracts/creditline-contract/src/tests.rs index 41d911f..f7b56ee 100644 --- a/contracts/creditline-contract/src/tests.rs +++ b/contracts/creditline-contract/src/tests.rs @@ -3993,3 +3993,118 @@ fn test_200_loan_borrower_stress_and_storage_layout_regression() { assert_eq!(loan_201.status, LoanStatus::Paid); } +#[test] +fn test_storage_layout_entry_classes_instance_vs_persistent() { + let ctx = TestCtx::setup(); + let borrower = Address::generate(&ctx.env); + let vendor = Address::generate(&ctx.env); + ctx.register_vendor(&vendor, "Layout Vendor"); + ctx.mint(&borrower, 10_000); + + let due_date = ctx.env.ledger().timestamp() + 10_000; + let schedule = ctx.single_installment(DEFAULT_TOTAL_DUE, due_date); + + let loan_id = ctx.client.create_loan( + &borrower, + &vendor, + &DEFAULT_PRINCIPAL, + &DEFAULT_GUARANTEE, + &schedule, + &LoanType::Standard, + ); + + // Verify storage entry classification inside contract context + ctx.env.as_contract(&ctx.client.address, || { + let shard = (loan_id % 32) as u32; + let loan_key = crate::storage::DataKey::Loan(shard, loan_id); + let page_key = crate::storage::DataKey::UserLoanPage(borrower.clone(), 0); + let count_key = crate::storage::DataKey::UserLoanCount(borrower.clone()); + let debt_key = crate::storage::DataKey::UserActiveDebt(borrower.clone()); + + // Assert all per-borrower and per-loan data keys reside strictly in PERSISTENT storage + assert!(ctx.env.storage().persistent().has(&loan_key), "Loan key must be in persistent storage"); + assert!(ctx.env.storage().persistent().has(&page_key), "UserLoanPage key must be in persistent storage"); + assert!(ctx.env.storage().persistent().has(&count_key), "UserLoanCount key must be in persistent storage"); + assert!(ctx.env.storage().persistent().has(&debt_key), "UserActiveDebt key must be in persistent storage"); + + // Assert NONE of these keys leak into INSTANCE storage + assert!(!ctx.env.storage().instance().has(&loan_key), "Loan key must NOT be in instance storage"); + assert!(!ctx.env.storage().instance().has(&page_key), "UserLoanPage key must NOT be in instance storage"); + assert!(!ctx.env.storage().instance().has(&count_key), "UserLoanCount key must NOT be in instance storage"); + assert!(!ctx.env.storage().instance().has(&debt_key), "UserActiveDebt key must NOT be in instance storage"); + }); +} + +#[test] +fn test_legacy_user_loan_at_backward_compatibility() { + let ctx = TestCtx::setup(); + let borrower = Address::generate(&ctx.env); + + // Manually write legacy UserLoanAt(borrower, idx) entries in persistent and instance storage + ctx.env.as_contract(&ctx.client.address, || { + let count_key = crate::storage::DataKey::UserLoanCount(borrower.clone()); + ctx.env.storage().persistent().set(&count_key, &2u64); + + let legacy_key_0 = crate::storage::DataKey::UserLoanAt(borrower.clone(), 0); + let legacy_key_1 = crate::storage::DataKey::UserLoanAt(borrower.clone(), 1); + + ctx.env.storage().persistent().set(&legacy_key_0, &101u64); + ctx.env.storage().instance().set(&legacy_key_1, &102u64); + + let ids = crate::storage::get_user_loan_ids_paginated(&ctx.env, &borrower, 0, 10).unwrap(); + assert_eq!(ids.len(), 2); + assert_eq!(ids.get(0).unwrap(), 101); + assert_eq!(ids.get(1).unwrap(), 102); + }); +} + +#[test] +fn test_concurrent_active_debt_and_ttl_extension_mid_flow() { + let ctx = TestCtx::setup(); + let borrower = Address::generate(&ctx.env); + let vendor = Address::generate(&ctx.env); + ctx.register_vendor(&vendor, "Active Debt Vendor"); + + let num_loans = 4; + let guarantee_per_loan = 200i128; + let principal_per_loan = 1_000i128; + let total_due_per_loan = 1_050i128; + + ctx.mint(&borrower, (num_loans as i128) * guarantee_per_loan); + + let due_date = ctx.env.ledger().timestamp() + 10_000; + let schedule = ctx.single_installment(total_due_per_loan, due_date); + + let mut created_ids = soroban_sdk::Vec::new(&ctx.env); + for i in 0..num_loans { + let loan_id = ctx.client.create_loan( + &borrower, + &vendor, + &principal_per_loan, + &guarantee_per_loan, + &schedule, + &LoanType::Standard, + ); + assert_eq!(loan_id, (i + 1) as u64); + created_ids.push_back(loan_id); + } + + // Verify concurrent active debt is accumulated across all 4 active loans + let expected_active_debt = (num_loans as i128) * total_due_per_loan; + assert_eq!(ctx.client.get_user_active_debt(&borrower), expected_active_debt); + + // Advance ledger timestamp to exercise TTL extension mid-flow + ctx.env.ledger().set_timestamp(ctx.env.ledger().timestamp() + 5_000); + + // Repay 2 of the active loans mid-flow + ctx.mint(&borrower, 2 * total_due_per_loan); + for i in 0..2 { + let loan_id = created_ids.get(i).unwrap(); + ctx.client.repay_loan(&borrower, &loan_id, &total_due_per_loan); + } + + // Active debt should decrease by exactly 2 * total_due_per_loan + let updated_active_debt = ((num_loans - 2) as i128) * total_due_per_loan; + assert_eq!(ctx.client.get_user_active_debt(&borrower), updated_active_debt); +} +