fix: Portfolio ID collision — use incrementing nonce instead of ledger sequence

contracts/Cargo.toml

@@ -7,10 +7,10 @@ edition = "2021"
 crate-type = ["cdylib"]
 
 [dependencies]
-soroban-sdk = "21.0.0"
+soroban-sdk = "21.7.7"
 
 [dev-dependencies]
-soroban-sdk = { version = "21.0.0", features = ["testutils"] }
+soroban-sdk = { version = "21.7.7", features = ["testutils"] }
 
 [features]
 default = []

contracts/src/lib.rs

@@ -40,8 +40,19 @@ impl PortfolioRebalancer {
         if rebalance_threshold < 1 || rebalance_threshold > 50 {
             return Err(Error::InvalidThreshold);
         }
-
-        let portfolio_id = env.ledger().sequence() as u64; // Convert u32 to u64
+
+        // Generate unique portfolio ID using an incrementing nonce.
+        // This prevents ID collisions when multiple users create portfolios in the same ledger.
+        let count: u64 = env
+            .storage()
+            .instance()
+            .get(&DataKey::PortfolioCount)
+            .unwrap_or(0);
+        let portfolio_id = count + 1;
+        env.storage()
+            .instance()
+            .set(&DataKey::PortfolioCount, &portfolio_id);
+
         let portfolio = Portfolio {
             user: user.clone(),
             target_allocations,
@@ -51,12 +62,12 @@ impl PortfolioRebalancer {
             total_value: 0,
             is_active: true,
         };
-        
-        env.storage().persistent().set(&DataKey::Portfolio(portfolio_id), &portfolio);
-        env.events().publish(
-            ("portfolio", "created"),
-            (portfolio_id, user)
-        );
+
+        env.storage()
+            .persistent()
+            .set(&DataKey::Portfolio(portfolio_id), &portfolio);
+        env.events()
+            .publish(("portfolio", "created"), (portfolio_id, user));
         Ok(portfolio_id)
     }
 

contracts/src/test.rs

@@ -445,3 +445,77 @@ fn test_create_portfolio_threshold_too_high() {
     allocations.set(Address::generate(&env), 100);
     client.create_portfolio(&user, &allocations, &51); // threshold 51 is invalid
 }
+
+#[test]
+fn test_concurrent_portfolio_creation() {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    // Set sequence to a fixed value to simulate same ledger
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 42;
+    });
+
+    let contract_id = env.register_contract(None, PortfolioRebalancer);
+    let client = PortfolioRebalancerClient::new(&env, &contract_id);
+    let reflector_id = env.register_contract(None, reflector_contract::MockReflector);
+    let admin = Address::generate(&env);
+    client.initialize(&admin, &reflector_id);
+
+    // Two different users create portfolios in the same ledger
+    let user_a = Address::generate(&env);
+    let user_b = Address::generate(&env);
+
+    let mut allocations_a = Map::new(&env);
+    let asset_a = Address::generate(&env);
+    allocations_a.set(asset_a, 100);
+
+    let mut allocations_b = Map::new(&env);
+    let asset_b = Address::generate(&env);
+    allocations_b.set(asset_b, 100);
+
+    let pid_a = client.create_portfolio(&user_a, &allocations_a, &5);
+    let pid_b = client.create_portfolio(&user_b, &allocations_b, &5);
+
+    // Both portfolios should have different IDs even though created in same ledger
+    assert_ne!(pid_a, pid_b);
+
+    // Both portfolios should be retrievable and belong to their respective users
+    let portfolio_a = client.get_portfolio(&pid_a);
+    let portfolio_b = client.get_portfolio(&pid_b);
+    assert_eq!(portfolio_a.user, user_a);
+    assert_eq!(portfolio_b.user, user_b);
+}
+
+#[test]
+fn test_same_user_two_portfolios() {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    env.ledger().with_mut(|li| {
+        li.sequence_number = 100;
+    });
+
+    let contract_id = env.register_contract(None, PortfolioRebalancer);
+    let client = PortfolioRebalancerClient::new(&env, &contract_id);
+    let reflector_id = env.register_contract(None, reflector_contract::MockReflector);
+    let admin = Address::generate(&env);
+    client.initialize(&admin, &reflector_id);
+
+    let user = Address::generate(&env);
+
+    let mut allocations_1 = Map::new(&env);
+    let asset1 = Address::generate(&env);
+    allocations_1.set(asset1, 100);
+
+    let mut allocations_2 = Map::new(&env);
+    let asset2 = Address::generate(&env);
+    allocations_2.set(asset2, 100);
+
+    // Same user creates two portfolios
+    let pid_1 = client.create_portfolio(&user, &allocations_1, &5);
+    let pid_2 = client.create_portfolio(&user, &allocations_2, &5);
+
+    // They should have different IDs
+    assert_ne!(pid_1, pid_2);
+}

contracts/src/types.rs

@@ -19,6 +19,7 @@ pub enum DataKey {
     ReflectorAddress,
     EmergencyStop,
     Initialized,
+    PortfolioCount,
     Portfolio(u64),
 }