diff --git a/contracts/Cargo.toml b/contracts/Cargo.toml index 59351db..4930e57 100644 --- a/contracts/Cargo.toml +++ b/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 = [] diff --git a/contracts/src/lib.rs b/contracts/src/lib.rs index f18100a..e3070c5 100644 --- a/contracts/src/lib.rs +++ b/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) } diff --git a/contracts/src/test.rs b/contracts/src/test.rs index f47d9f7..82c3e9d 100644 --- a/contracts/src/test.rs +++ b/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); +} diff --git a/contracts/src/types.rs b/contracts/src/types.rs index 5420804..85ae82c 100644 --- a/contracts/src/types.rs +++ b/contracts/src/types.rs @@ -19,6 +19,7 @@ pub enum DataKey { ReflectorAddress, EmergencyStop, Initialized, + PortfolioCount, Portfolio(u64), }