From b8eefae9f6bb076c096d2d0b0c31740b95cb0dfb Mon Sep 17 00:00:00 2001
From: Thompson <140930314+Godbrand0@users.noreply.github.com>
Date: Mon, 28 Sep 2026 18:36:53 -0400
Subject: [PATCH] fix(maintenance-pool): cap deposits per pool with
 MAX_DEPOSITS

Add MAX_DEPOSITS (50) and a TooManyDeposits error, mirroring MAX_SPONSORS in
escrow/milestones. deposit() and withdraw() loop over deposit_count to refresh
TTLs, and a Soroban transaction footprint is limited to 100 ledger entries, so
an unbounded count eventually makes those calls fail.

Also fix maintenance-pool test compilation: close a truncated test, re-export
the contract from the crate root, and dereference an address comparison.

Closes #94
---
 contracts/maintenance-pool/src/error.rs |  2 ++
 contracts/maintenance-pool/src/lib.rs   | 14 ++++++++++
 contracts/maintenance-pool/src/test.rs  | 35 ++++++++++++++++++++++---
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/contracts/maintenance-pool/src/error.rs b/contracts/maintenance-pool/src/error.rs
index 494631b..a3e7655 100644
--- a/contracts/maintenance-pool/src/error.rs
+++ b/contracts/maintenance-pool/src/error.rs
@@ -25,4 +25,6 @@ pub enum Error {
     ContractPaused = 13,
     /// The deposit count has reached its maximum limit (issue #45).
     DepositCountOverflow = 14,
+    /// The pool already holds `MAX_DEPOSITS` deposits (issue #94).
+    TooManyDeposits = 15,
 }
diff --git a/contracts/maintenance-pool/src/lib.rs b/contracts/maintenance-pool/src/lib.rs
index 1012ba6..53b7383 100644
--- a/contracts/maintenance-pool/src/lib.rs
+++ b/contracts/maintenance-pool/src/lib.rs
@@ -28,6 +28,14 @@ use mergefi_common::BPS_DENOMINATOR;
 /// concept but applied per-deposit rather than per-pool.
 pub const INACTIVITY_WINDOW: u64 = 90 * 24 * 60 * 60; // 90 days
 
+/// Maximum number of deposits a single pool may record (issue #94).
+/// `deposit` and `withdraw` refresh the TTL of every deposit sub-record in
+/// a loop over `deposit_count`, so an unbounded count would make those
+/// calls grow without limit, and Soroban caps a transaction footprint at
+/// 100 ledger entries (a cap of 100 would already exceed it). Mirrors
+/// `MAX_SPONSORS` in escrow/milestones.
+pub const MAX_DEPOSITS: u32 = 50;
+
 /// Current version of the storage schema. Incremented on breaking layout changes.
 const CONTRACT_VERSION: u32 = 1;
 
@@ -123,6 +131,10 @@ mod contract {
             return Err(Error::TokenMismatch);
         }
 
+        if pool.deposit_count >= MAX_DEPOSITS {
+            return Err(Error::TooManyDeposits);
+        }
+
         let token_client = token::Client::new(&env, &token);
         token_client.transfer(&sponsor, env.current_contract_address(), &amount);
 
@@ -515,6 +527,8 @@ mod contract {
 }
 } // mod contract
 
+pub use contract::*;
+
 fn require_admin(env: &Env) -> Result<Address, Error> {
     mergefi_common::require_admin::<DataKey>(env).ok_or(Error::NotInitialized)
 }
diff --git a/contracts/maintenance-pool/src/test.rs b/contracts/maintenance-pool/src/test.rs
index 83f7b2c..1b0c34c 100644
--- a/contracts/maintenance-pool/src/test.rs
+++ b/contracts/maintenance-pool/src/test.rs
@@ -770,9 +770,33 @@ fn test_deposit_rejects_when_deposit_count_would_overflow() {
         env.storage().persistent().set(&pkey, &pool);
     });
 
-    // Calling deposit should now fail with DepositCountOverflow
+    // The MAX_DEPOSITS cap rejects long before u32 overflow is reachable.
     let err = client.try_deposit(&10u64, &sponsor, &token_addr, &100i128);
-    assert_eq!(err, Err(Ok(Error::DepositCountOverflow)));
+    assert_eq!(err, Err(Ok(Error::TooManyDeposits)));
+}
+
+#[test]
+fn test_deposit_rejects_beyond_max_deposits() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let (_admin, _treasury, client) = setup(&env);
+
+    let token_admin = Address::generate(&env);
+    let (token_addr, asset_client, _token_client) = create_token(&env, &token_admin);
+    let sponsor = Address::generate(&env);
+    asset_client.mint(&sponsor, &1_000_000i128);
+
+    for _ in 0..crate::MAX_DEPOSITS {
+        client.deposit(&11u64, &sponsor, &token_addr, &1i128);
+    }
+    assert_eq!(client.get_pool(&11u64).deposit_count, crate::MAX_DEPOSITS);
+
+    let err = client.try_deposit(&11u64, &sponsor, &token_addr, &1i128);
+    assert_eq!(err, Err(Ok(Error::TooManyDeposits)));
+    assert_eq!(client.get_pool(&11u64).deposit_count, crate::MAX_DEPOSITS);
+
+    // Other pools are unaffected.
+    client.deposit(&12u64, &sponsor, &token_addr, &1i128);
 }
 
 // ─── upgrade (issue #246) ────────────────────────────────────────────────────
@@ -928,8 +952,11 @@ fn test_set_oracle_rotates_oracle_used_by_withdraw() {
 
     // withdraw now requires the rotated oracle's authorization, not the old one's.
     let auths = env.auths();
-    assert!(auths.iter().any(|(addr, _)| addr == new_oracle));
-    assert!(!auths.iter().any(|(addr, _)| addr == old_oracle));
+    assert!(auths.iter().any(|(addr, _)| *addr == new_oracle));
+    assert!(!auths.iter().any(|(addr, _)| *addr == old_oracle));
+}
+
+#[test]
 fn test_set_treasury_requires_admin_auth() {
     let env = Env::default();
     env.mock_all_auths();