From 0434d1d81e2a05455cddc23a46932198290c361a Mon Sep 17 00:00:00 2001
From: jamilahmadzai <jamilurrehman722@gmail.com>
Date: Wed, 15 Jul 2026 11:52:57 +0200
Subject: [PATCH 1/2] fix: guard factory pool count overflow

---
 soroban/contracts/factory/src/lib.rs   |  9 ++++++--
 soroban/contracts/factory/src/test.rs  | 29 +++++++++++++++++++++++++-
 soroban/contracts/factory/src/types.rs |  2 ++
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/soroban/contracts/factory/src/lib.rs b/soroban/contracts/factory/src/lib.rs
index cab440a..b737a20 100644
--- a/soroban/contracts/factory/src/lib.rs
+++ b/soroban/contracts/factory/src/lib.rs
@@ -2,7 +2,9 @@
 
 mod types;
 
-use soroban_sdk::{contract, contractimpl, symbol_short, vec, Address, BytesN, Env, IntoVal, Symbol, Val, Vec};
+use soroban_sdk::{
+    contract, contractimpl, symbol_short, vec, Address, BytesN, Env, IntoVal, Symbol, Val, Vec,
+};
 use types::{DataKey, FactoryError, ListPoolsResponse, PoolRecord};
 
 // ~30 days at ~5 s/ledger; extend to ~60 days when below threshold.
@@ -229,6 +231,9 @@ impl Factory {
             .map_err(|_| FactoryError::MinLockPeriodOutOfRange)?;
 
         let pool_id: u32 = env.storage().instance().get(&DataKey::PoolCount).unwrap();
+        let next_count = pool_id
+            .checked_add(1)
+            .ok_or(FactoryError::PoolCountOverflow)?;
         let wasm_hash: BytesN<32> = env.storage().instance().get(&DataKey::WasmHash).unwrap();
         let salt = pool_salt(&env, pool_id);
 
@@ -268,7 +273,7 @@ impl Factory {
         bump_pool(&env, pool_id);
         env.storage()
             .instance()
-            .set(&DataKey::PoolCount, &(pool_id + 1));
+            .set(&DataKey::PoolCount, &next_count);
 
         // Emit enriched event so indexers get the full pool parameters in one shot.
         #[allow(deprecated)]
diff --git a/soroban/contracts/factory/src/test.rs b/soroban/contracts/factory/src/test.rs
index d31a237..2280127 100644
--- a/soroban/contracts/factory/src/test.rs
+++ b/soroban/contracts/factory/src/test.rs
@@ -495,6 +495,31 @@ fn test_create_pool_increments_count_after_each_pool() {
     assert_eq!(t.client.pool_count(), 2);
 }
 
+#[test]
+fn test_create_pool_returns_typed_error_when_pool_count_overflows() {
+    let t = setup();
+
+    t.env.as_contract(&t.factory_addr, || {
+        t.env
+            .storage()
+            .instance()
+            .set(&DataKey::PoolCount, &u32::MAX);
+    });
+
+    let result =
+        t.client
+            .try_create_pool(&Address::generate(&t.env), &1_728_000u128, &2u32, &100u64);
+
+    assert_eq!(result, Err(Ok(FactoryError::PoolCountOverflow)));
+    assert_eq!(t.client.pool_count(), u32::MAX);
+    assert_eq!(
+        t.env.as_contract(&t.factory_addr, || {
+            t.env.storage().persistent().has(&DataKey::Pool(u32::MAX))
+        }),
+        false
+    );
+}
+
 #[test]
 fn test_create_pool_uses_deterministic_pool_addresses() {
     let t = setup();
@@ -651,7 +676,9 @@ fn test_create_pool_configures_deployed_pool_matching_factory_record() {
     let t = setup();
     let asset = Address::generate(&t.env);
 
-    let id = t.client.create_pool(&asset, &17_280_000u128, &3u32, &86_400u64);
+    let id = t
+        .client
+        .create_pool(&asset, &17_280_000u128, &3u32, &86_400u64);
     let record = t.client.get_pool(&id);
 
     assert_eq!(record.credit_rate, 1_000);
diff --git a/soroban/contracts/factory/src/types.rs b/soroban/contracts/factory/src/types.rs
index d8ab301..6ab6888 100644
--- a/soroban/contracts/factory/src/types.rs
+++ b/soroban/contracts/factory/src/types.rs
@@ -73,4 +73,6 @@ pub enum FactoryError {
     InvalidCreditRate = 5,
     /// `create_pool`'s `min_lock_period` does not fit in the pool's native `u32`.
     MinLockPeriodOutOfRange = 6,
+    /// `create_pool` cannot allocate another monotonically increasing pool ID.
+    PoolCountOverflow = 7,
 }

From 9d365848bbc7260a97d9c943ba033ad1ce58131c Mon Sep 17 00:00:00 2001
From: jamilahmadzai <jamilurrehman722@gmail.com>
Date: Thu, 23 Jul 2026 19:14:59 +0200
Subject: [PATCH 2/2] fix: restore contract CI after upstream merge

---
 .github/workflows/ci.yml                      |  7 ++--
 soroban/Cargo.lock                            |  1 +
 soroban/contracts/factory/Cargo.toml          |  1 +
 soroban/contracts/factory/src/lib.rs          | 32 ++++++++++-----
 soroban/contracts/factory/src/test.rs         | 33 ++++++++-------
 soroban/contracts/factory/src/types.rs        |  2 +-
 .../factory/tests/factory_pool_integration.rs | 41 ++++---------------
 soroban/contracts/farming-pool/src/lib.rs     | 11 +++--
 .../farming-pool/src/mock_reentrant_token.rs  |  2 -
 soroban/contracts/farming-pool/src/test.rs    |  6 +--
 soroban/contracts/farming-pool/src/types.rs   |  1 -
 11 files changed, 63 insertions(+), 74 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index e2dd004..822e953 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -33,6 +33,9 @@ jobs:
             ${{ runner.os }}-cargo-
 
       - name: Build WASM fixtures for factory integration tests
+        working-directory: soroban
+        run: cargo build --workspace --target wasm32v1-none --release
+
       - name: Check formatting
         working-directory: soroban
         run: cargo fmt --all -- --check
@@ -41,10 +44,6 @@ jobs:
         working-directory: soroban
         run: cargo clippy --workspace --all-targets -- -D warnings
 
-      - name: Build farming-pool WASM fixture for factory integration tests
-        working-directory: soroban
-        run: cargo build --workspace --target wasm32v1-none --release
-
       - name: Run tests
         working-directory: soroban
         run: cargo test --workspace
diff --git a/soroban/Cargo.lock b/soroban/Cargo.lock
index a9f3532..2eb3c9d 100644
--- a/soroban/Cargo.lock
+++ b/soroban/Cargo.lock
@@ -602,6 +602,7 @@ checksum = "40404c3f5f511ec4da6fe866ddf6a717c309fdbb69fbbad7b0f3edab8f2e835f"
 name = "factory"
 version = "0.1.0"
 dependencies = [
+ "farming-pool",
  "soroban-sdk",
 ]
 
diff --git a/soroban/contracts/factory/Cargo.toml b/soroban/contracts/factory/Cargo.toml
index ddd5cc0..e364161 100644
--- a/soroban/contracts/factory/Cargo.toml
+++ b/soroban/contracts/factory/Cargo.toml
@@ -12,4 +12,5 @@ doctest    = false
 soroban-sdk = { workspace = true }
 
 [dev-dependencies]
+farming-pool = { path = "../farming-pool" }
 soroban-sdk = { workspace = true, features = ["testutils"] }
diff --git a/soroban/contracts/factory/src/lib.rs b/soroban/contracts/factory/src/lib.rs
index 0a47a77..e24e6a3 100644
--- a/soroban/contracts/factory/src/lib.rs
+++ b/soroban/contracts/factory/src/lib.rs
@@ -174,9 +174,18 @@ impl Factory {
     /// maintained incrementally in create_pool) would avoid full-registry scans entirely.
     /// This would be a more robust long-term fix but requires changes to create_pool's
     /// write path and potentially a migration/backfill for existing pools.
-    pub fn get_pools_by_asset(env: Env, asset: Address, start_id: u32, limit: u32) -> ListPoolsResponse {
+    pub fn get_pools_by_asset(
+        env: Env,
+        asset: Address,
+        start_id: u32,
+        limit: u32,
+    ) -> ListPoolsResponse {
         bump_instance(&env);
-        let count: u32 = env.storage().instance().get(&DataKey::PoolCount).unwrap_or(0);
+        let count: u32 = env
+            .storage()
+            .instance()
+            .get(&DataKey::PoolCount)
+            .unwrap_or(0);
         let capped_limit = limit.min(20);
         let mut records: Vec<(u32, PoolRecord)> = vec![&env];
         let mut next_start_id = count;
@@ -230,7 +239,11 @@ impl Factory {
     /// ~60 days) to ensure all pool records remain accessible.
     pub fn refresh_pool_ttls(env: Env, start_id: u32, limit: u32) -> Result<(), FactoryError> {
         bump_instance(&env);
-        let count: u32 = env.storage().instance().get(&DataKey::PoolCount).unwrap_or(0);
+        let count: u32 = env
+            .storage()
+            .instance()
+            .get(&DataKey::PoolCount)
+            .unwrap_or(0);
         let capped_limit = limit.min(20);
         let end = start_id.saturating_add(capped_limit).min(count);
         for pool_id in start_id..end {
@@ -286,6 +299,10 @@ impl Factory {
         env.events().publish(
             (symbol_short!("factory"), symbol_short!("pool_upg")),
             (pool_id, record.address, new_wasm_hash),
+        );
+        Ok(())
+    }
+
     /// Update the WASM hash used for future `create_pool` deployments. Admin-only.
     ///
     /// Allows the admin to point future pool deployments at a corrected or upgraded
@@ -294,19 +311,14 @@ impl Factory {
     ///
     /// Emits a `wasm_set` event with `(old_hash, new_hash)` so that the previous
     /// hash is discoverable off-chain for rollback scenarios.
-    pub fn set_pool_wasm_hash(
-        env: Env,
-        new_hash: BytesN<32>,
-    ) -> Result<(), FactoryError> {
+    pub fn set_pool_wasm_hash(env: Env, new_hash: BytesN<32>) -> Result<(), FactoryError> {
         require_initialized(&env)?;
         let admin: Address = load_admin(&env);
         admin.require_auth();
         bump_instance(&env);
 
         let old_hash: BytesN<32> = env.storage().instance().get(&DataKey::WasmHash).unwrap();
-        env.storage()
-            .instance()
-            .set(&DataKey::WasmHash, &new_hash);
+        env.storage().instance().set(&DataKey::WasmHash, &new_hash);
         #[allow(deprecated)]
         env.events().publish(
             (symbol_short!("factory"), symbol_short!("wasm_set")),
diff --git a/soroban/contracts/factory/src/test.rs b/soroban/contracts/factory/src/test.rs
index bade132..6c4e94e 100644
--- a/soroban/contracts/factory/src/test.rs
+++ b/soroban/contracts/factory/src/test.rs
@@ -236,7 +236,11 @@ fn test_get_pools_by_asset_returns_empty_when_no_pools() {
     let t = setup();
     let asset = Address::generate(&t.env);
     let page = t.client.get_pools_by_asset(&asset, &0u32, &10u32);
-    assert_eq!(page.records.len(), 0, "expected no pools for a fresh factory");
+    assert_eq!(
+        page.records.len(),
+        0,
+        "expected no pools for a fresh factory"
+    );
 }
 
 // ── transfer_admin ────────────────────────────────────────────────────────────
@@ -518,7 +522,7 @@ fn test_get_pools_by_asset_paginates_large_matching_registry() {
     let env = Env::default();
     env.mock_all_auths();
     let admin = Address::generate(&env);
-    let wasm_hash = upload_mock_pool_wasm(&env);
+    let wasm_hash = upload_farming_pool_wasm(&env);
     let factory_addr = env.register(Factory, ());
     let client = FactoryClient::new(&env, &factory_addr);
     client.initialize(&admin, &wasm_hash);
@@ -526,7 +530,12 @@ fn test_get_pools_by_asset_paginates_large_matching_registry() {
     // Create 25 pools all sharing the same asset
     let asset = Address::generate(&env);
     for i in 0..25 {
-        client.create_pool(&asset, &(100 + i as u128), &(10 + i as u64));
+        client.create_pool(
+            &asset,
+            &(1_728_000 + i as u128 * 17_280),
+            &2u32,
+            &(10 + i as u64),
+        );
     }
 
     // First page should return 20 records
@@ -651,12 +660,9 @@ fn test_create_pool_returns_typed_error_when_pool_count_overflows() {
 
     assert_eq!(result, Err(Ok(FactoryError::PoolCountOverflow)));
     assert_eq!(t.client.pool_count(), u32::MAX);
-    assert_eq!(
-        t.env.as_contract(&t.factory_addr, || {
-            t.env.storage().persistent().has(&DataKey::Pool(u32::MAX))
-        }),
-        false
-    );
+    assert!(!t.env.as_contract(&t.factory_addr, || {
+        t.env.storage().persistent().has(&DataKey::Pool(u32::MAX))
+    }));
 }
 
 #[test]
@@ -732,7 +738,7 @@ fn test_get_pool_bumps_pool_record_ttl() {
     advance_ledgers(&t.env, TTL_EXTEND_TO - TTL_THRESHOLD + 1);
     assert!(pool_record_ttl(&t.env, &t.factory_addr, id) < TTL_THRESHOLD);
 
-    assert_eq!(t.client.try_get_pool(&id).is_ok(), true);
+    assert!(t.client.try_get_pool(&id).is_ok());
     assert_eq!(pool_record_ttl(&t.env, &t.factory_addr, id), TTL_EXTEND_TO);
 }
 
@@ -741,7 +747,7 @@ fn test_refresh_pool_ttls_restores_ttl_for_unqueried_pool() {
     let t = setup();
     let id = t
         .client
-        .create_pool(&Address::generate(&t.env), &250u128, &50u64);
+        .create_pool(&Address::generate(&t.env), &1_728_000u128, &2u32, &50u64);
 
     // Initial TTL after creation
     assert_eq!(pool_record_ttl(&t.env, &t.factory_addr, id), TTL_EXTEND_TO);
@@ -751,10 +757,7 @@ fn test_refresh_pool_ttls_restores_ttl_for_unqueried_pool() {
     assert!(pool_record_ttl(&t.env, &t.factory_addr, id) < TTL_THRESHOLD);
 
     // Call refresh_pool_ttls to restore TTL without a specific get_pool query
-    assert_eq!(
-        t.client.try_refresh_pool_ttls(&id, &1u32),
-        Ok(Ok(()))
-    );
+    assert_eq!(t.client.try_refresh_pool_ttls(&id, &1u32), Ok(Ok(())));
 
     // Verify TTL is restored
     assert_eq!(pool_record_ttl(&t.env, &t.factory_addr, id), TTL_EXTEND_TO);
diff --git a/soroban/contracts/factory/src/types.rs b/soroban/contracts/factory/src/types.rs
index f6ed670..29c8aa4 100644
--- a/soroban/contracts/factory/src/types.rs
+++ b/soroban/contracts/factory/src/types.rs
@@ -74,7 +74,7 @@ pub enum FactoryError {
     /// `create_pool`'s `min_lock_period` does not fit in the pool's native `u32`.
     MinLockPeriodOutOfRange = 6,
     /// `create_pool` cannot allocate another monotonically increasing pool ID.
-    PoolCountOverflow = 7,
+    PoolCountOverflow = 8,
     /// A function requiring initialization was called on an uninitialized factory.
     NotInitialized = 7,
 }
diff --git a/soroban/contracts/factory/tests/factory_pool_integration.rs b/soroban/contracts/factory/tests/factory_pool_integration.rs
index 828266e..650a47d 100644
--- a/soroban/contracts/factory/tests/factory_pool_integration.rs
+++ b/soroban/contracts/factory/tests/factory_pool_integration.rs
@@ -87,6 +87,7 @@ fn deploy_pool_via_factory(
     admin: &Address,
     asset: &Address,
     daily_rate: u128,
+    global_multiplier: u32,
     min_lock_period: u64,
 ) -> (FarmingPoolClient<'static>, Address) {
     let wasm_hash = env.deployer().upload_contract_wasm(FARMING_POOL_WASM);
@@ -95,7 +96,8 @@ fn deploy_pool_via_factory(
     let factory_client = FactoryClient::new(env, &factory_addr);
     factory_client.initialize(admin, &wasm_hash);
 
-    let pool_id = factory_client.create_pool(asset, &daily_rate, &min_lock_period);
+    let pool_id =
+        factory_client.create_pool(asset, &daily_rate, &global_multiplier, &min_lock_period);
     let record = factory_client.get_pool(&pool_id);
     let pool_address = record.address.clone();
 
@@ -121,18 +123,14 @@ fn smoke_create_pool_returns_live_pool_address() {
     let factory_client = FactoryClient::new(&env, &factory_addr);
     factory_client.initialize(&admin, &wasm_hash);
 
-    let pool_id = factory_client.create_pool(&asset, &100u128, &10u64);
+    let pool_id = factory_client.create_pool(&asset, &1_728_000u128, &2u32, &10u64);
     let record = factory_client.get_pool(&pool_id);
 
     // The deployed pool exists at a real address the factory tracked, and it
     // is reachable via FarmingPoolClient (a bogus/undeployed address would
     // panic on the first call below).
     let pool_client = FarmingPoolClient::new(&env, &record.address);
-    // workaround for #78 — remove once create_pool initializes pools.
-    // create_pool's `deploy_v2(wasm_hash, ())` never calls the pool's own
-    // `initialize`, so the pool starts life uninitialized; prove that by
-    // reading its admin only after initializing it here.
-    pool_client.initialize(&admin, &asset, &1u32, &1i128, &0u32);
+    // `create_pool` initializes the deployed contract atomically.
     assert_eq!(pool_client.admin(), admin);
 }
 
@@ -160,7 +158,7 @@ fn end_to_end_create_pool_then_stake_and_unstake() {
 
     let global_multiplier = 2u32;
     let credit_rate = 1i128;
-    let daily_rate = 500u128;
+    let daily_rate = 17_280u128;
     let min_lock_period: u32 = 0;
 
     let (pool_client, pool_address) = deploy_pool_via_factory(
@@ -168,20 +166,10 @@ fn end_to_end_create_pool_then_stake_and_unstake() {
         &admin,
         &asset.address(),
         daily_rate,
+        global_multiplier,
         min_lock_period as u64,
     );
 
-    // workaround for #78 — remove once create_pool initializes pools.
-    // Factory::create_pool deploys the pool's WASM but never invokes its
-    // `initialize`, so it must be initialized manually here before use.
-    pool_client.initialize(
-        &admin,
-        &asset.address(),
-        &global_multiplier,
-        &credit_rate,
-        &min_lock_period,
-    );
-
     let stake_amount: i128 = 1_000;
     pool_client.stake(&user, &stake_amount);
 
@@ -248,8 +236,7 @@ fn end_to_end_create_pool_then_lock_and_unlock() {
     token_sac.mint(&user, &INITIAL_MINT);
 
     let global_multiplier = 1u32;
-    let credit_rate = 2i128;
-    let daily_rate = 300u128;
+    let daily_rate = 34_560u128;
     let min_lock_period_ledgers = 50u32;
 
     let (pool_client, pool_address) = deploy_pool_via_factory(
@@ -257,20 +244,10 @@ fn end_to_end_create_pool_then_lock_and_unlock() {
         &admin,
         &asset.address(),
         daily_rate,
+        global_multiplier,
         min_lock_period_ledgers as u64,
     );
 
-    // workaround for #78 — remove once create_pool initializes pools.
-    // Factory::create_pool deploys the pool's WASM but never invokes its
-    // `initialize`, so it must be initialized manually here before use.
-    pool_client.initialize(
-        &admin,
-        &asset.address(),
-        &global_multiplier,
-        &credit_rate,
-        &min_lock_period_ledgers,
-    );
-
     let lock_amount: i128 = 2_000;
     pool_client.lock_assets(&user, &lock_amount);
 
diff --git a/soroban/contracts/farming-pool/src/lib.rs b/soroban/contracts/farming-pool/src/lib.rs
index 691782a..dd2044c 100644
--- a/soroban/contracts/farming-pool/src/lib.rs
+++ b/soroban/contracts/farming-pool/src/lib.rs
@@ -1,8 +1,9 @@
 #![no_std]
+#![allow(deprecated)]
 
-mod types;
 #[cfg(test)]
 mod mock_reentrant_token;
+mod types;
 
 use soroban_sdk::{contract, contractimpl, symbol_short, token, Address, BytesN, Env};
 pub use types::PoolError;
@@ -40,7 +41,6 @@ fn require_not_paused(env: &Env) -> Result<(), PoolError> {
     Ok(())
 }
 
-
 fn get_admin(env: &Env) -> Result<Address, PoolError> {
     env.storage()
         .instance()
@@ -316,7 +316,7 @@ impl FarmingPool {
         let stake_token = get_stake_token(&env)?;
         token::TokenClient::new(&env, &stake_token).transfer(
             &user,
-            &env.current_contract_address(),
+            env.current_contract_address(),
             &amount,
         );
 
@@ -494,7 +494,7 @@ impl FarmingPool {
         let stake_token = get_stake_token(&env)?;
         token::TokenClient::new(&env, &stake_token).transfer(
             &from,
-            &env.current_contract_address(),
+            env.current_contract_address(),
             &amount,
         );
 
@@ -528,10 +528,9 @@ impl FarmingPool {
         user.require_auth();
         require_not_paused(&env)?;
 
-
         require_initialized(&env)?;
         assert!(
-            allocation_pct >= 1 && allocation_pct <= 100,
+            (1..=100).contains(&allocation_pct),
             "allocation_pct must be 1-100"
         );
         bump_instance(&env);
diff --git a/soroban/contracts/farming-pool/src/mock_reentrant_token.rs b/soroban/contracts/farming-pool/src/mock_reentrant_token.rs
index 2f71b09..c05627e 100644
--- a/soroban/contracts/farming-pool/src/mock_reentrant_token.rs
+++ b/soroban/contracts/farming-pool/src/mock_reentrant_token.rs
@@ -1,5 +1,3 @@
-#![cfg(test)]
-
 //! A minimal token-interface contract for exercising checks-effects-
 //! interactions (CEI) reentrancy scenarios in tests (#69). Configured with a
 //! target contract + user, its `transfer` attempts to call back into the
diff --git a/soroban/contracts/farming-pool/src/test.rs b/soroban/contracts/farming-pool/src/test.rs
index adab5a3..5ab5667 100644
--- a/soroban/contracts/farming-pool/src/test.rs
+++ b/soroban/contracts/farming-pool/src/test.rs
@@ -253,6 +253,9 @@ fn test_upgrade_requires_admin_auth() {
 
     assert!(result.is_err(), "non-admin upgrade must be rejected");
     assert_eq!(client.admin(), admin);
+}
+
+#[test]
 fn test_admin_uninitialized_returns_not_initialized() {
     let (_env, client, _user) = setup_uninitialized();
     match client.try_admin() {
@@ -997,7 +1000,6 @@ fn test_pause_blocks_lock_assets() {
     }
 }
 
-
 #[test]
 fn test_pause_blocks_unlock_assets() {
     let t = setup(1, 1);
@@ -1010,7 +1012,6 @@ fn test_pause_blocks_unlock_assets() {
     }
 }
 
-
 #[test]
 fn test_unpause_restores_operations() {
     let t = setup(1, 1);
@@ -1054,7 +1055,6 @@ fn test_pause_blocks_stake() {
     }
 }
 
-
 #[test]
 fn test_unpause_restores_stake() {
     let t = setup(1, 1);
diff --git a/soroban/contracts/farming-pool/src/types.rs b/soroban/contracts/farming-pool/src/types.rs
index d8dcffa..e881bf0 100644
--- a/soroban/contracts/farming-pool/src/types.rs
+++ b/soroban/contracts/farming-pool/src/types.rs
@@ -11,7 +11,6 @@ pub enum PoolError {
     NotPaused = 13,
     Paused = 20,
     NoActiveStake = 14,
-
 }
 
 /// Per-user boost configuration returned by `get_boost_config`.