diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b88fb7f..db89148c 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/contracts/vault/src/admin.rs b/contracts/vault/src/admin.rs new file mode 100644 index 00000000..4f185a7a --- /dev/null +++ b/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 { + 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); + } +} diff --git a/contracts/vault/src/lib.rs b/contracts/vault/src/lib.rs index 1194c1b7..2b4e1e17 100644 --- a/contracts/vault/src/lib.rs +++ b/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::read_proposal(&env, proposal_id) } pub fn admin(env: Env) -> Option
{ @@ -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"),), diff --git a/contracts/vault/src/proxy_tests.rs b/contracts/vault/src/proxy_tests.rs index a9afc932..3482f60d 100644 --- a/contracts/vault/src/proxy_tests.rs +++ b/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(); diff --git a/contracts/vault/src/storage_registry.rs b/contracts/vault/src/storage_registry.rs index 29563491..4ba9ab17 100644 --- a/contracts/vault/src/storage_registry.rs +++ b/contracts/vault/src/storage_registry.rs @@ -64,6 +64,12 @@ pub fn registered_vault_keys(env: &soroban_sdk::Env) -> soroban_sdk::Vec