From d76aa3717c6ce5dc4d53c298537e1580f99afdfc Mon Sep 17 00:00:00 2001 From: Ogunmodede Joel Taiwo Date: Thu, 18 Jun 2026 09:04:07 +0100 Subject: [PATCH 1/6] [Contracts] LoanManager deposit_collateral CEI fix and regression test --- loan_manager/src/lib.rs | 25 ++++++++------ loan_manager/src/test.rs | 74 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/loan_manager/src/lib.rs b/loan_manager/src/lib.rs index bee04dc..ae6abfd 100644 --- a/loan_manager/src/lib.rs +++ b/loan_manager/src/lib.rs @@ -1380,25 +1380,30 @@ impl LoanManager { .storage() .instance() .get(&DataKey::Token) - .expect("token not set"); + .ok_or(LoanError::NotInitialized)?; let token_client = TokenClient::new(&env, &token); - token_client.transfer(&loan.borrower, &env.current_contract_address(), &amount); - - let loan_key = DataKey::Loan(loan_id); - let mut loan: Loan = env - .storage() - .persistent() - .get(&loan_key) - .expect("loan not found"); let updated_collateral = loan .collateral_amount .checked_add(amount) - .expect("collateral overflow"); + .ok_or(LoanError::AmountTooLarge)?; loan.collateral_amount = updated_collateral; env.storage().persistent().set(&loan_key, &loan); Self::bump_persistent_ttl(&env, &loan_key); + // CEI: commit the updated loan state before the external token transfer. + token_client.transfer(&loan.borrower, &env.current_contract_address(), &amount); + + // Re-validate after the external interaction with typed errors rather than panicking. + let stored_loan: Loan = env + .storage() + .persistent() + .get(&loan_key) + .ok_or(LoanError::LoanNotFound)?; + if stored_loan.status != LoanStatus::Approved { + return Err(LoanError::LoanNotActive); + } + env.events().publish( (symbol_short!("ColDep"), loan_id, loan.borrower), updated_collateral, diff --git a/loan_manager/src/test.rs b/loan_manager/src/test.rs index 9552f98..3ff9be5 100644 --- a/loan_manager/src/test.rs +++ b/loan_manager/src/test.rs @@ -3,7 +3,43 @@ use lending_pool::{LendingPool, LendingPoolClient}; use remittance_nft::{RemittanceNFT, RemittanceNFTClient}; use soroban_sdk::testutils::Ledger as _; use soroban_sdk::token::{Client as TokenClient, StellarAssetClient}; -use soroban_sdk::{testutils::Address as _, Address, BytesN, Env, String}; +use soroban_sdk::{contract, contractclient, contractimpl, testutils::Address as _, Address, BytesN, Env, String, Symbol}; + +#[contractclient(name = "MaliciousTokenClient")] +pub trait MaliciousTokenInterface { + fn set_attack_target(env: Env, manager: Address, loan_id: u32); +} + +#[contract] +pub struct MaliciousToken; + +#[contractimpl] +impl MaliciousToken { + pub fn set_attack_target(env: Env, manager: Address, loan_id: u32) { + env.storage() + .persistent() + .set(&Symbol::new(&env, "manager"), &manager); + env.storage() + .persistent() + .set(&Symbol::new(&env, "loan_id"), &loan_id); + } + + pub fn transfer(env: Env, from: Address, to: Address, amount: i128) { + let manager: Address = env + .storage() + .persistent() + .get(&Symbol::new(&env, "manager")) + .unwrap(); + let loan_id: u32 = env + .storage() + .persistent() + .get(&Symbol::new(&env, "loan_id")) + .unwrap(); + env.as_contract(&manager, || { + env.storage().persistent().remove(&DataKey::Loan(loan_id)); + }); + } +} fn setup_test<'a>( env: &Env, @@ -1335,6 +1371,42 @@ fn test_deposit_collateral_and_auto_release_on_full_repayment() { ); } +#[test] +fn test_deposit_collateral_rejects_loan_removed_during_token_transfer() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + + let (manager, nft_client, pool_client, _token_id, _token_admin) = setup_test(&env); + let borrower = Address::generate(&env); + + let history_hash = BytesN::from_array(&env, &[0u8; 32]); + nft_client.mint( + &borrower, + &650, + &history_hash, + &String::from_str(&env, "ipfs://QmTest"), + &None, + ); + + let stellar_token = StellarAssetClient::new(&env, &token_id); + stellar_token.mint(&pool_client, &20_000); + stellar_token.mint(&borrower, &20_000); + + let loan_id = manager.request_loan(&borrower, &1_000, &17280); + manager.approve_loan(&loan_id); + + let malicious = env.register(MaliciousToken, ()); + let malicious_client = MaliciousTokenClient::new(&env, &malicious); + malicious_client.set_attack_target(&manager.address, &loan_id); + + env.as_contract(&manager.address, || { + env.storage().instance().set(&DataKey::Token, &malicious); + }); + + let result = manager.deposit_collateral(&loan_id, &300); + assert_eq!(result, Err(LoanError::LoanNotFound)); +} + #[test] fn test_collateral_is_seized_on_default() { let env = Env::default(); From 269bb312856cfc682f28435f8222fd83ce5e5596 Mon Sep 17 00:00:00 2001 From: Ogunmodede Joel Taiwo Date: Sat, 20 Jun 2026 10:41:55 +0100 Subject: [PATCH 2/6] fix(loan-manager): correct loan retrieval and update event publishing --- loan_manager/src/lib.rs | 4 ++-- loan_manager/src/test.rs | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/loan_manager/src/lib.rs b/loan_manager/src/lib.rs index b622d4d..7f0daef 100644 --- a/loan_manager/src/lib.rs +++ b/loan_manager/src/lib.rs @@ -1392,7 +1392,7 @@ impl LoanManager { } let loan_key = DataKey::Loan(loan_id); - let loan: Loan = env + let mut loan: Loan = env .storage() .persistent() .get(&loan_key) @@ -1444,7 +1444,7 @@ impl LoanManager { } env.events().publish( - (symbol_short!("ColDep"), loan_id, loan.borrower), + (symbol_short!("ColDep"), loan_id, stored_loan.borrower), updated_collateral, ); diff --git a/loan_manager/src/test.rs b/loan_manager/src/test.rs index 79fed96..7a7a73b 100644 --- a/loan_manager/src/test.rs +++ b/loan_manager/src/test.rs @@ -3,7 +3,17 @@ use lending_pool::{LendingPool, LendingPoolClient}; use remittance_nft::{RemittanceNFT, RemittanceNFTClient}; use soroban_sdk::testutils::Ledger as _; use soroban_sdk::token::{Client as TokenClient, StellarAssetClient}; -use soroban_sdk::{contract, contractclient, contractimpl, testutils::Address as _, Address, BytesN, Env, String, Symbol}; +use soroban_sdk::{ + contract, + contractclient, + contractimpl, + testutils::Address as _, + Address, + BytesN, + Env, + String, + Symbol, +}; #[contractclient(name = "MaliciousTokenClient")] pub trait MaliciousTokenInterface { From def37ba0abb6b483c51d089c4e50d1ed78481448 Mon Sep 17 00:00:00 2001 From: Ogunmodede Joel Taiwo Date: Sat, 20 Jun 2026 10:43:44 +0100 Subject: [PATCH 3/6] refactor(test): simplify import statements in test.rs --- loan_manager/src/test.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/loan_manager/src/test.rs b/loan_manager/src/test.rs index 7a7a73b..79fed96 100644 --- a/loan_manager/src/test.rs +++ b/loan_manager/src/test.rs @@ -3,17 +3,7 @@ use lending_pool::{LendingPool, LendingPoolClient}; use remittance_nft::{RemittanceNFT, RemittanceNFTClient}; use soroban_sdk::testutils::Ledger as _; use soroban_sdk::token::{Client as TokenClient, StellarAssetClient}; -use soroban_sdk::{ - contract, - contractclient, - contractimpl, - testutils::Address as _, - Address, - BytesN, - Env, - String, - Symbol, -}; +use soroban_sdk::{contract, contractclient, contractimpl, testutils::Address as _, Address, BytesN, Env, String, Symbol}; #[contractclient(name = "MaliciousTokenClient")] pub trait MaliciousTokenInterface { From b7c329217dc195f01261edb123eebf3d51520f8c Mon Sep 17 00:00:00 2001 From: Ogunmodede Joel Taiwo Date: Sat, 20 Jun 2026 10:45:27 +0100 Subject: [PATCH 4/6] refactor(test): format import statements for better readability --- loan_manager/src/test.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/loan_manager/src/test.rs b/loan_manager/src/test.rs index 79fed96..e6b53e2 100644 --- a/loan_manager/src/test.rs +++ b/loan_manager/src/test.rs @@ -3,7 +3,10 @@ use lending_pool::{LendingPool, LendingPoolClient}; use remittance_nft::{RemittanceNFT, RemittanceNFTClient}; use soroban_sdk::testutils::Ledger as _; use soroban_sdk::token::{Client as TokenClient, StellarAssetClient}; -use soroban_sdk::{contract, contractclient, contractimpl, testutils::Address as _, Address, BytesN, Env, String, Symbol}; +use soroban_sdk::{ + contract, contractclient, contractimpl, testutils::Address as _, Address, BytesN, Env, String, + Symbol, +}; #[contractclient(name = "MaliciousTokenClient")] pub trait MaliciousTokenInterface { From 50d46a5ac6310146a0ac65af4cdd836bf61acf40 Mon Sep 17 00:00:00 2001 From: Ogunmodede Joel Taiwo Date: Sat, 20 Jun 2026 10:51:57 +0100 Subject: [PATCH 5/6] refactor(test): clean up unused trait and improve transfer function parameters --- loan_manager/src/test.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/loan_manager/src/test.rs b/loan_manager/src/test.rs index e6b53e2..3c38ea8 100644 --- a/loan_manager/src/test.rs +++ b/loan_manager/src/test.rs @@ -4,15 +4,9 @@ use remittance_nft::{RemittanceNFT, RemittanceNFTClient}; use soroban_sdk::testutils::Ledger as _; use soroban_sdk::token::{Client as TokenClient, StellarAssetClient}; use soroban_sdk::{ - contract, contractclient, contractimpl, testutils::Address as _, Address, BytesN, Env, String, - Symbol, + contract, contractimpl, testutils::Address as _, Address, BytesN, Env, String, Symbol, }; -#[contractclient(name = "MaliciousTokenClient")] -pub trait MaliciousTokenInterface { - fn set_attack_target(env: Env, manager: Address, loan_id: u32); -} - #[contract] pub struct MaliciousToken; @@ -27,7 +21,7 @@ impl MaliciousToken { .set(&Symbol::new(&env, "loan_id"), &loan_id); } - pub fn transfer(env: Env, from: Address, to: Address, amount: i128) { + pub fn transfer(env: Env, _from: Address, _to: Address, _amount: i128) { let manager: Address = env .storage() .persistent() @@ -1379,7 +1373,7 @@ fn test_deposit_collateral_rejects_loan_removed_during_token_transfer() { let env = Env::default(); env.mock_all_auths_allowing_non_root_auth(); - let (manager, nft_client, pool_client, _token_id, _token_admin) = setup_test(&env); + let (manager, nft_client, pool_client, token_id, _token_admin) = setup_test(&env); let borrower = Address::generate(&env); let history_hash = BytesN::from_array(&env, &[0u8; 32]); @@ -1406,7 +1400,7 @@ fn test_deposit_collateral_rejects_loan_removed_during_token_transfer() { env.storage().instance().set(&DataKey::Token, &malicious); }); - let result = manager.deposit_collateral(&loan_id, &300); + let result: Result<(), LoanError> = manager.try_deposit_collateral(&loan_id, &300); assert_eq!(result, Err(LoanError::LoanNotFound)); } From 0f9f82e224f2e2c28d560b9b1eea4ad4049b0901 Mon Sep 17 00:00:00 2001 From: Ogunmodede Joel Taiwo Date: Sat, 20 Jun 2026 10:59:37 +0100 Subject: [PATCH 6/6] fix(test): correct error handling in deposit collateral test --- loan_manager/src/test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/loan_manager/src/test.rs b/loan_manager/src/test.rs index 3c38ea8..05dd20b 100644 --- a/loan_manager/src/test.rs +++ b/loan_manager/src/test.rs @@ -1400,8 +1400,8 @@ fn test_deposit_collateral_rejects_loan_removed_during_token_transfer() { env.storage().instance().set(&DataKey::Token, &malicious); }); - let result: Result<(), LoanError> = manager.try_deposit_collateral(&loan_id, &300); - assert_eq!(result, Err(LoanError::LoanNotFound)); + let result = manager.try_deposit_collateral(&loan_id, &300); + assert_eq!(result, Err(Ok(LoanError::LoanNotFound))); } #[test]