Merge pull request #602 from Lekan101/fix/issues-587-597-598-600

feat: add read_creator_supply helper, storage key collision tests, and integration tests

Author: Chucks1093

creator-keys/src/lib.rs

@@ -751,7 +751,15 @@ pub fn read_registered_creator_profile(
 /// Use this helper wherever repeated key balance read logic is needed to keep
 /// missing-balance behavior consistent across the contract.
 pub fn read_key_balance(env: &Env, creator: &Address) -> u32 {
-    read_creator_profile(env, creator)
+    read_creator_supply(env, creator)
+}
+
+/// Reads a creator's current key supply from persistent storage.
+///
+/// Returns `0` if the creator has not been registered (no supply record exists).
+/// Centralises the supply read so the storage key format is defined once.
+pub fn read_creator_supply(env: &Env, creator_id: &Address) -> u32 {
+    read_creator_profile(env, creator_id)
         .map(|p| p.supply)
         .unwrap_or(0)
 }
@@ -2181,9 +2189,9 @@ impl CreatorKeysContract {
     /// Read-only view: returns the total key supply for a creator.
     ///
     /// Returns `0` if the creator is not registered, avoiding panics for
-    /// invalid lookups. Delegates to the shared [`read_key_balance`] helper.
+    /// invalid lookups. Delegates to the shared [`read_creator_supply`] helper.
     pub fn get_total_key_supply(env: Env, creator: Address) -> u32 {
-        read_key_balance(&env, &creator)
+        read_creator_supply(&env, &creator)
     }
 
     /// Read-only view: returns the current supply for a registered creator.
@@ -3627,6 +3635,78 @@ mod tests {
         let result = super::validate_non_zero_address(&env, &valid);
         assert_eq!(result, Ok(()));
     }
+
+    // --- read_creator_supply helper tests (#587) ---
+
+    #[test]
+    fn test_read_creator_supply_returns_correct_supply_for_initialized_creator() {
+        use soroban_sdk::{testutils::Address as _, Address, Env};
+
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let contract_id = env.register(super::CreatorKeysContract, ());
+
+        let profile = super::CreatorProfile {
+            creator: creator.clone(),
+            handle: soroban_sdk::String::from_str(&env, "alice"),
+            supply: 42,
+            holder_count: 5,
+            fee_recipient: creator.clone(),
+            registered_at: 0,
+        };
+
+        let supply = env.as_contract(&contract_id, || {
+            env.storage()
+                .persistent()
+                .set(&super::constants::storage::creator(&creator), &profile);
+
+            super::read_creator_supply(&env, &creator)
+        });
+        assert_eq!(supply, 42);
+    }
+
+    #[test]
+    fn test_read_creator_supply_returns_zero_for_uninitialized_creator() {
+        use soroban_sdk::{testutils::Address as _, Address, Env};
+
+        let env = Env::default();
+        let missing_creator = Address::generate(&env);
+        let contract_id = env.register(super::CreatorKeysContract, ());
+
+        let supply = env.as_contract(&contract_id, || {
+            super::read_creator_supply(&env, &missing_creator)
+        });
+        assert_eq!(supply, 0);
+    }
+
+    #[test]
+    fn test_read_creator_supply_matches_read_key_balance() {
+        use soroban_sdk::{testutils::Address as _, Address, Env};
+
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let contract_id = env.register(super::CreatorKeysContract, ());
+
+        let profile = super::CreatorProfile {
+            creator: creator.clone(),
+            handle: soroban_sdk::String::from_str(&env, "alice"),
+            supply: 15,
+            holder_count: 3,
+            fee_recipient: creator.clone(),
+            registered_at: 0,
+        };
+
+        env.as_contract(&contract_id, || {
+            env.storage()
+                .persistent()
+                .set(&super::constants::storage::creator(&creator), &profile);
+
+            let supply_from_new = super::read_creator_supply(&env, &creator);
+            let supply_from_old = super::read_key_balance(&env, &creator);
+            assert_eq!(supply_from_new, supply_from_old);
+            assert_eq!(supply_from_new, 15);
+        });
+    }
 }
 
 #[cfg(test)]

creator-keys/src/test_issues.rs

@@ -450,4 +450,255 @@ mod issue_tests {
     fn test_bonding_curve_step_5() {
         test_bonding_curve_step_helper(5, 100, 150, 350);
     }
+
+    // =========================================================================
+    // Storage key collision tests (#597)
+    //
+    // Confirm that two distinct inputs never produce the same storage key.
+    // =========================================================================
+
+    #[test]
+    fn test_holder_balance_key_different_holders_differ_for_same_creator() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder_a = Address::generate(&env);
+        let holder_b = Address::generate(&env);
+
+        let key_a = constants::storage::holder_balance_key(&creator, &holder_a);
+        let key_b = constants::storage::holder_balance_key(&creator, &holder_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different holders must produce different keys"
+        );
+    }
+
+    #[test]
+    fn test_holder_balance_key_argument_order_matters() {
+        let env = Env::default();
+        let addr_a = Address::generate(&env);
+        let addr_b = Address::generate(&env);
+
+        let key_ab = constants::storage::holder_balance_key(&addr_a, &addr_b);
+        let key_ba = constants::storage::holder_balance_key(&addr_b, &addr_a);
+
+        assert_ne!(
+            key_ab, key_ba,
+            "swapping creator and holder must produce different keys"
+        );
+    }
+
+    #[test]
+    fn test_holder_balance_key_never_equals_creator_key() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder = Address::generate(&env);
+
+        let balance_key = constants::storage::holder_balance_key(&creator, &holder);
+        let creator_key = constants::storage::creator(&creator);
+
+        assert_ne!(
+            balance_key, creator_key,
+            "holder balance key must not collide with creator profile key"
+        );
+    }
+
+    #[test]
+    fn test_holder_balance_key_never_equals_dividend_accumulator_key() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder = Address::generate(&env);
+
+        let balance_key = constants::storage::holder_balance_key(&creator, &holder);
+        let dividend_key = constants::storage::dividend_accumulator(&creator);
+
+        assert_ne!(
+            balance_key, dividend_key,
+            "holder balance key must not collide with dividend accumulator key"
+        );
+    }
+
+    #[test]
+    fn test_holder_dividend_checkpoint_different_holders_differ() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder_a = Address::generate(&env);
+        let holder_b = Address::generate(&env);
+
+        let key_a = constants::storage::holder_dividend_checkpoint(&creator, &holder_a);
+        let key_b = constants::storage::holder_dividend_checkpoint(&creator, &holder_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different holders must produce different dividend checkpoint keys"
+        );
+    }
+
+    #[test]
+    fn test_holder_dividend_pending_different_holders_differ() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder_a = Address::generate(&env);
+        let holder_b = Address::generate(&env);
+
+        let key_a = constants::storage::holder_dividend_pending(&creator, &holder_a);
+        let key_b = constants::storage::holder_dividend_pending(&creator, &holder_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different holders must produce different dividend pending keys"
+        );
+    }
+
+    #[test]
+    fn test_co_creator_fee_balance_different_pairs_differ() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let co_a = Address::generate(&env);
+        let co_b = Address::generate(&env);
+
+        let key_a = constants::storage::co_creator_fee_balance(&creator, &co_a);
+        let key_b = constants::storage::co_creator_fee_balance(&creator, &co_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different co-creator addresses must produce different fee balance keys"
+        );
+    }
+
+    #[test]
+    fn test_co_creator_fee_balance_different_creators_differ() {
+        let env = Env::default();
+        let creator_a = Address::generate(&env);
+        let creator_b = Address::generate(&env);
+        let co_creator = Address::generate(&env);
+
+        let key_a = constants::storage::co_creator_fee_balance(&creator_a, &co_creator);
+        let key_b = constants::storage::co_creator_fee_balance(&creator_b, &co_creator);
+
+        assert_ne!(
+            key_a, key_b,
+            "different creators with same co-creator must produce different keys"
+        );
+    }
+
+    #[test]
+    fn test_creator_fee_balance_different_creators_differ() {
+        let env = Env::default();
+        let creator_a = Address::generate(&env);
+        let creator_b = Address::generate(&env);
+
+        let key_a = constants::storage::creator_fee_balance(&creator_a);
+        let key_b = constants::storage::creator_fee_balance(&creator_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different creators must produce different fee balance keys"
+        );
+    }
+
+    #[test]
+    fn test_holder_balance_key_never_equals_creator_fee_balance_key() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder = Address::generate(&env);
+
+        let balance_key = constants::storage::holder_balance_key(&creator, &holder);
+        let fee_key = constants::storage::creator_fee_balance(&creator);
+
+        assert_ne!(
+            balance_key, fee_key,
+            "holder balance key must not collide with creator fee balance key"
+        );
+    }
+
+    #[test]
+    fn test_holder_dividend_checkpoint_never_equals_holder_balance_key() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder = Address::generate(&env);
+
+        let checkpoint_key = constants::storage::holder_dividend_checkpoint(&creator, &holder);
+        let balance_key = constants::storage::holder_balance_key(&creator, &holder);
+
+        assert_ne!(
+            checkpoint_key, balance_key,
+            "dividend checkpoint key must not collide with holder balance key"
+        );
+    }
+
+    #[test]
+    fn test_holder_dividend_pending_never_equals_holder_balance_key() {
+        let env = Env::default();
+        let creator = Address::generate(&env);
+        let holder = Address::generate(&env);
+
+        let pending_key = constants::storage::holder_dividend_pending(&creator, &holder);
+        let balance_key = constants::storage::holder_balance_key(&creator, &holder);
+
+        assert_ne!(
+            pending_key, balance_key,
+            "dividend pending key must not collide with holder balance key"
+        );
+    }
+
+    #[test]
+    fn test_curve_preset_different_creators_differ() {
+        let env = Env::default();
+        let creator_a = Address::generate(&env);
+        let creator_b = Address::generate(&env);
+
+        let key_a = constants::storage::curve_preset(&creator_a);
+        let key_b = constants::storage::curve_preset(&creator_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different creators must produce different curve preset keys"
+        );
+    }
+
+    #[test]
+    fn test_max_supply_different_creators_differ() {
+        let env = Env::default();
+        let creator_a = Address::generate(&env);
+        let creator_b = Address::generate(&env);
+
+        let key_a = constants::storage::max_supply(&creator_a);
+        let key_b = constants::storage::max_supply(&creator_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different creators must produce different max supply keys"
+        );
+    }
+
+    #[test]
+    fn test_max_keys_per_wallet_different_creators_differ() {
+        let env = Env::default();
+        let creator_a = Address::generate(&env);
+        let creator_b = Address::generate(&env);
+
+        let key_a = constants::storage::max_keys_per_wallet(&creator_a);
+        let key_b = constants::storage::max_keys_per_wallet(&creator_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different creators must produce different max keys per wallet keys"
+        );
+    }
+
+    #[test]
+    fn test_locked_allocation_different_creators_differ() {
+        let env = Env::default();
+        let creator_a = Address::generate(&env);
+        let creator_b = Address::generate(&env);
+
+        let key_a = constants::storage::locked_allocation(&creator_a);
+        let key_b = constants::storage::locked_allocation(&creator_b);
+
+        assert_ne!(
+            key_a, key_b,
+            "different creators must produce different locked allocation keys"
+        );
+    }
 }