fix: deregister guardian from AllGuardians and slot index in resign_g…

src/contracts/vault_ops.rs

@@ -131,8 +131,9 @@ pub(crate) fn resign_guardian(env: &Env, guardian: Address) -> Result<(), Contra
     // Check if timelock has expired
     timelock::check_timelock_expired(env, &guardian)?;
 
-    let g_key = DataKey::Guardian(guardian.clone());
-    env.storage().instance().remove(&g_key);
+    // Deregister guardian from all membership structures (AllGuardians, slot index)
+    guardian::deregister_guardian(env, guardian.clone())?;
+
     let key = DataKey::LockedBalance(guardian.clone());
     let amount: i128 = env.storage().instance().get(&key).unwrap_or(0);
     if amount > 0 {

src/guardian.rs

@@ -55,11 +55,16 @@ pub fn add_guardian(env: &Env, _admin: Address, guardian: Address) -> Result<(),
     Ok(())
 }
 
-/// Removes an existing guardian from the contract.
+/// Internal helper that deregisters a guardian from all membership structures.
 ///
-/// Address validation (`admin`, `guardian`) is performed by the calling
-/// entrypoint; this helper must not repeat it.
-pub fn remove_guardian(env: &Env, _admin: Address, guardian: Address) -> Result<(), ContractError> {
+/// This removes the guardian from:
+/// - The Guardian flag (DataKey::Guardian)
+/// - The AllGuardians set
+/// - The dense slot index (GuardianIndexAt/GuardianIndexOf/GuardianIndexCount)
+///
+/// Caller is responsible for any validation (e.g., NotGuardian check) and
+/// peripheral cleanup (e.g., token refunds, timelock clearing).
+pub(crate) fn deregister_guardian(env: &Env, guardian: Address) -> Result<(), ContractError> {
     let key = DataKey::Guardian(guardian.clone());
     if !env.storage().instance().has(&key) {
         return Err(ContractError::NotGuardian);
@@ -132,6 +137,14 @@ pub fn remove_guardian(env: &Env, _admin: Address, guardian: Address) -> Result<
     Ok(())
 }
 
+/// Removes an existing guardian from the contract.
+///
+/// Address validation (`admin`, `guardian`) is performed by the calling
+/// entrypoint; this helper must not repeat it.
+pub fn remove_guardian(env: &Env, _admin: Address, guardian: Address) -> Result<(), ContractError> {
+    deregister_guardian(env, guardian)
+}
+
 /// Checks if a given address is a registered guardian.
 pub fn is_guardian(env: &Env, guardian: &Address) -> bool {
     let key = DataKey::Guardian(guardian.clone());

tests/resign_guardian_deregister.rs

@@ -0,0 +1,71 @@
+use soroban_sdk::token::StellarAssetClient as TestTokenClient;
+use soroban_sdk::{
+    testutils::{Address as _, Ledger as _},
+    Address, Env,
+};
+use vero_core_contracts::VeroContractClient;
+use vero_core_contracts::Role;
+
+fn setup() -> (Env, Address, Address, Address, VeroContractClient<'static>) {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let contract_id = env.register_contract(None, vero_core_contracts::VeroContract);
+    let client = VeroContractClient::new(&env, &contract_id);
+
+    let admin = Address::generate(&env);
+    let token_admin = Address::generate(&env);
+    let token = env.register_stellar_asset_contract_v2(token_admin.clone());
+    let token_addr = token.address();
+
+    client.initialize(&admin, &token_addr, &100i128);
+
+    (env, contract_id, admin, token_addr, client)
+}
+
+fn lock_for_guardian(
+    env: &Env,
+    token: &Address,
+    client: &VeroContractClient,
+    guardian: &Address,
+    amount: i128,
+) {
+    let sac = TestTokenClient::new(env, token);
+    sac.mint(guardian, &amount);
+    client.lock_tokens(guardian, &amount);
+}
+
+#[test]
+fn test_resigned_guardian_can_be_readded() {
+    let (env, _contract_id, admin, token, client) = setup();
+    let g = Address::generate(&env);
+
+    client.grant_role(&admin, &admin, &Role::GuardianManager);
+    client.add_guardian(&admin, &g);
+    lock_for_guardian(&env, &token, &client, &g, 200);
+
+    // Initiate the 24-hour timelock, then advance ledger past it
+    client.request_unlock(&g);
+    let timelock = client.get_withdrawal_timelock(&g).unwrap();
+    env.ledger().set_timestamp(timelock + 86401u64);
+
+    client.resign_guardian(&g);
+
+    // Verify guardian is deregistered from all structures
+    assert!(!client.is_guardian(&g));
+    let snapshot_meta = client.get_snapshot_meta();
+    assert_eq!(snapshot_meta.guardian_count, 0);
+
+    let guardians_page = client.get_guardians_page(&0u32, &50u32);
+    assert_eq!(guardians_page.len(), 0);
+
+    // Re-add the guardian - should succeed after deregistration
+    client.add_guardian(&admin, &g);
+    assert!(client.is_guardian(&g));
+
+    let snapshot_meta_after = client.get_snapshot_meta();
+    assert_eq!(snapshot_meta_after.guardian_count, 1);
+
+    let guardians_page_after = client.get_guardians_page(&0u32, &50u32);
+    assert_eq!(guardians_page_after.len(), 1);
+}