[#736] Contracts: Add deterministic admin proposal nonce for governance action replay protection

CHANGELOG.md

@@ -9,6 +9,7 @@ Versioning follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ## [Unreleased]
 
 ### Features
+- Add deterministic admin proposal nonces with replay rejection for admin rotation (#736)
 - Add empty-state deposit and withdraw intent actions across dashboard pages (#734)
 - CORS configuration for cross-origin API access
 - Add canonical `VaultError` namespace module and replace contract panics with stable error codes (#754)

contracts/vault/src/admin.rs

@@ -0,0 +1,69 @@
+//! Admin rotation proposals with deterministic nonces and replay protection.
+
+use soroban_sdk::{contracttype, Address, Env};
+
+#[contracttype]
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct AdminProposal {
+    pub new_admin: Address,
+    pub proposer: Address,
+    pub accepted: bool,
+    pub cancelled: bool,
+    pub created_at: u64,
+}
+
+pub fn read_proposal(env: &Env, id: u32) -> Option<AdminProposal> {
+    env.storage()
+        .instance()
+        .get(&crate::DataKey::AdminProposal(id))
+}
+
+pub fn write_proposal(env: &Env, id: u32, proposal: &AdminProposal) {
+    env.storage()
+        .instance()
+        .set(&crate::DataKey::AdminProposal(id), proposal);
+}
+
+pub fn next_proposal_id(env: &Env) -> u32 {
+    let nonce: u32 = env
+        .storage()
+        .instance()
+        .get(&crate::DataKey::AdminProposalNonce)
+        .unwrap_or(0);
+    let next = nonce.checked_add(1).expect("admin proposal nonce overflow");
+    env.storage()
+        .instance()
+        .set(&crate::DataKey::AdminProposalNonce, &next);
+    next
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use soroban_sdk::{testutils::Address as _, Address, Env};
+
+    #[test]
+    fn test_admin_proposal_nonce_is_monotonic() {
+        let env = Env::default();
+        assert_eq!(next_proposal_id(&env), 1);
+        assert_eq!(next_proposal_id(&env), 2);
+        assert_eq!(next_proposal_id(&env), 3);
+    }
+
+    #[test]
+    fn test_admin_proposal_round_trip() {
+        let env = Env::default();
+        let proposer = Address::generate(&env);
+        let new_admin = Address::generate(&env);
+        let proposal = AdminProposal {
+            new_admin: new_admin.clone(),
+            proposer: proposer.clone(),
+            accepted: false,
+            cancelled: false,
+            created_at: 42,
+        };
+        write_proposal(&env, 1, &proposal);
+        let loaded = read_proposal(&env, 1).expect("proposal stored");
+        assert_eq!(loaded, proposal);
+    }
+}

contracts/vault/src/lib.rs

@@ -53,6 +53,7 @@
 //! See `DEPLOYMENT.md` for step-by-step deployment to Stellar testnet/mainnet.
 
 #[cfg(not(target_arch = "wasm32"))]
+pub mod admin;
 pub mod benji_strategy;
 pub mod errors;
 pub use errors::VaultError;
@@ -235,6 +236,8 @@ pub enum DataKey {
     Emergency(EmergencyStorageKey),
     EmergencyProposalNonce,
     EmergencyProposal(u32),
+    AdminProposalNonce,
+    AdminProposal(u32),
 
     Proposal(u32),
     Vote(VoteKey),
@@ -439,6 +442,10 @@ pub enum VaultError {
     /// Treasury claim quota exceeded for the current epoch.
     ClaimQuotaExceeded = 28,
     StrategyHeartbeatExpired = 29,
+    /// Referenced admin or governance proposal does not exist.
+    ProposalNotFound = 30,
+    /// Proposal has already been executed or accepted.
+    ProposalAlreadyExecuted = 31,
     /// Invalid RWA shipment status transition (violates lifecycle rules).
     InvalidShipmentStatusTransition = 30,
 }
@@ -555,43 +562,87 @@ impl YieldVault {
 
     /// Propose a new admin.
     /// Only the current Admin can call this.
-    pub fn propose_admin(env: Env, new_admin: Address) {
+    ///
+    /// Returns a monotonically increasing proposal ID used for replay-safe accept/cancel.
+    pub fn propose_admin(env: Env, new_admin: Address) -> u32 {
         let admin = get_admin(&env).expect("Admin not set");
         admin.require_auth();
 
         let previous_pending = get_pending_admin(&env);
-        set_pending_admin(&env, &Some(new_admin));
+        let proposal_id = admin::next_proposal_id(&env);
+        let proposal = admin::AdminProposal {
+            new_admin: new_admin.clone(),
+            proposer: admin.clone(),
+            accepted: false,
+            cancelled: false,
+            created_at: env.ledger().timestamp(),
+        };
+        admin::write_proposal(&env, proposal_id, &proposal);
+        set_pending_admin(&env, &Some(new_admin.clone()));
         env.events().publish(
             (symbol_short!("adminprop"),),
-            (admin, previous_pending, get_pending_admin(&env).unwrap()),
+            (proposal_id, admin, previous_pending, new_admin),
         );
+        proposal_id
     }
 
-    /// Accept the admin role.
+    /// Accept the admin role for a specific proposal.
     /// Only the pending Admin can call this.
-    pub fn accept_admin(env: Env) {
-        let pending_admin = get_pending_admin(&env).expect("No pending admin");
-        pending_admin.require_auth();
+    pub fn accept_admin(env: Env, proposal_id: u32) -> Result<(), VaultError> {
+        let mut proposal = admin::read_proposal(&env, proposal_id).ok_or(VaultError::ProposalNotFound)?;
+
+        if proposal.cancelled {
+            return Err(VaultError::ProposalCancelled);
+        }
+        if proposal.accepted {
+            return Err(VaultError::ProposalAlreadyExecuted);
+        }
+
+        proposal.new_admin.require_auth();
 
         let previous_admin = get_admin(&env).expect("Admin not set");
-        set_admin(&env, &pending_admin);
+        proposal.accepted = true;
+        admin::write_proposal(&env, proposal_id, &proposal);
+
+        set_admin(&env, &proposal.new_admin);
         set_pending_admin(&env, &None);
         env.events().publish(
             (symbol_short!("adminxfer"),),
-            (previous_admin, pending_admin),
+            (proposal_id, previous_admin, proposal.new_admin),
         );
+        Ok(())
     }
 
-    /// Cancel an in-flight admin rotation.
+    /// Cancel an in-flight admin rotation proposal.
     /// Only the current Admin can call this.
-    pub fn cancel_admin_rotation(env: Env) {
+    pub fn cancel_admin_rotation(env: Env, proposal_id: u32) -> Result<(), VaultError> {
         let admin = get_admin(&env).expect("Admin not set");
         admin.require_auth();
 
+        let mut proposal = admin::read_proposal(&env, proposal_id).ok_or(VaultError::ProposalNotFound)?;
+
+        if proposal.accepted {
+            return Err(VaultError::ProposalAlreadyExecuted);
+        }
+        if proposal.cancelled {
+            return Err(VaultError::ProposalCancelled);
+        }
+
+        proposal.cancelled = true;
+        admin::write_proposal(&env, proposal_id, &proposal);
+
         let previous_pending = get_pending_admin(&env);
         set_pending_admin(&env, &None);
-        env.events()
-            .publish((symbol_short!("admincncl"),), (admin, previous_pending));
+        env.events().publish(
+            (symbol_short!("admincncl"),),
+            (proposal_id, admin, previous_pending),
+        );
+        Ok(())
+    }
+
+    /// Returns the stored admin rotation proposal for the given ID.
+    pub fn admin_proposal(env: Env, proposal_id: u32) -> Option<admin::AdminProposal> {
+        admin::read_proposal(&env, proposal_id)
     }
 
     pub fn admin(env: Env) -> Option<Address> {
@@ -3404,6 +3455,23 @@ impl YieldVault {
             );
         }
 
+        if current_version < 3 && target_version >= 3 {
+            if let Some(pending_admin) = get_pending_admin(env) {
+                let proposer = get_admin(env).expect("admin must exist for pending rotation");
+                let proposal = admin::AdminProposal {
+                    new_admin: pending_admin.clone(),
+                    proposer,
+                    accepted: false,
+                    cancelled: false,
+                    created_at: env.ledger().timestamp(),
+                };
+                admin::write_proposal(env, 1, &proposal);
+                env.storage()
+                    .instance()
+                    .set(&DataKey::AdminProposalNonce, &1u32);
+            }
+        }
+
         set_storage_version(env, target_version);
         env.events().publish(
             (symbol_short!("migrate"),),

contracts/vault/src/proxy_tests.rs

@@ -58,8 +58,8 @@ fn test_storage_migration_version_guard() {
     let vault = YieldVaultClient::new(&env, &vault_id);
     vault.initialize(&admin, &token);
 
-    assert_eq!(vault.storage_version(), 2);
-    vault.migrate_storage(&2);
+    assert_eq!(vault.storage_version(), 3);
+    vault.migrate_storage(&3);
 
     let result = vault.try_migrate_storage(&1);
     assert!(matches!(
@@ -84,10 +84,10 @@ fn test_admin_rotation_handover_flow() {
     assert_eq!(vault.admin(), Some(admin.clone()));
     assert_eq!(vault.pending_admin(), None);
 
-    vault.propose_admin(&next_admin);
+    let proposal_id = vault.propose_admin(&next_admin);
     assert_eq!(vault.pending_admin(), Some(next_admin.clone()));
 
-    vault.accept_admin();
+    vault.accept_admin(&proposal_id);
     assert_eq!(vault.admin(), Some(next_admin));
     assert_eq!(vault.pending_admin(), None);
 }
@@ -105,14 +105,75 @@ fn test_admin_rotation_can_be_cancelled() {
     let vault = YieldVaultClient::new(&env, &vault_id);
     vault.initialize(&admin, &token);
 
-    vault.propose_admin(&next_admin);
+    let proposal_id = vault.propose_admin(&next_admin);
     assert_eq!(vault.pending_admin(), Some(next_admin));
 
-    vault.cancel_admin_rotation();
+    vault.cancel_admin_rotation(&proposal_id);
     assert_eq!(vault.admin(), Some(admin));
     assert_eq!(vault.pending_admin(), None);
 }
 
+#[test]
+fn test_admin_proposal_nonce_is_monotonic() {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let admin = Address::generate(&env);
+    let token = Address::generate(&env);
+    let candidate_a = Address::generate(&env);
+    let candidate_b = Address::generate(&env);
+
+    let vault_id = env.register(YieldVault, ());
+    let vault = YieldVaultClient::new(&env, &vault_id);
+    vault.initialize(&admin, &token);
+
+    let pid_a = vault.propose_admin(&candidate_a);
+    vault.cancel_admin_rotation(&pid_a);
+    let pid_b = vault.propose_admin(&candidate_b);
+    assert_ne!(pid_a, pid_b);
+    assert_eq!(pid_b, pid_a + 1);
+}
+
+#[test]
+fn test_admin_accept_replay_is_rejected() {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let admin = Address::generate(&env);
+    let next_admin = Address::generate(&env);
+    let token = Address::generate(&env);
+
+    let vault_id = env.register(YieldVault, ());
+    let vault = YieldVaultClient::new(&env, &vault_id);
+    vault.initialize(&admin, &token);
+
+    let proposal_id = vault.propose_admin(&next_admin);
+    vault.accept_admin(&proposal_id);
+
+    let replay = vault.try_accept_admin(&proposal_id);
+    assert_eq!(replay, Err(Ok(VaultError::ProposalAlreadyExecuted)));
+}
+
+#[test]
+fn test_admin_cancel_then_accept_is_rejected() {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let admin = Address::generate(&env);
+    let next_admin = Address::generate(&env);
+    let token = Address::generate(&env);
+
+    let vault_id = env.register(YieldVault, ());
+    let vault = YieldVaultClient::new(&env, &vault_id);
+    vault.initialize(&admin, &token);
+
+    let proposal_id = vault.propose_admin(&next_admin);
+    vault.cancel_admin_rotation(&proposal_id);
+
+    let result = vault.try_accept_admin(&proposal_id);
+    assert_eq!(result, Err(Ok(VaultError::ProposalCancelled)));
+}
+
 #[test]
 fn test_storage_layout_integrity() {
     let env = Env::default();

contracts/vault/src/storage_registry.rs

@@ -64,6 +64,12 @@ pub fn registered_vault_keys(env: &soroban_sdk::Env) -> soroban_sdk::Vec<Storage
 
     keys.push_back(scalar(StorageNamespace::Governance, "DaoThreshold"));
     keys.push_back(scalar(StorageNamespace::Governance, "ProposalNonce"));
+    keys.push_back(scalar(StorageNamespace::Governance, "AdminProposalNonce"));
+    keys.push_back(StorageKeyDescriptor {
+        namespace: StorageNamespace::Governance,
+        name: symbol_short!("AdminProp"),
+        parameterized: true,
+    });
     keys.push_back(scalar(StorageNamespace::Governance, "GovernanceConfig"));
     keys.push_back(scalar(StorageNamespace::Governance, "BenjiStrategy"));
     keys.push_back(scalar(StorageNamespace::Governance, "KoreanDebtStrategy"));