diff --git a/contract/vault/soroban/governance/src/lib.rs b/contract/vault/soroban/governance/src/lib.rs index 48d7fcbb1..628ba6d6c 100644 --- a/contract/vault/soroban/governance/src/lib.rs +++ b/contract/vault/soroban/governance/src/lib.rs @@ -44,6 +44,7 @@ enum ProposalKey { #[derive(Clone, Eq, PartialEq, Ord, PartialOrd)] enum GovernanceActionKey { + Admin, Pause, Curator, Governance, @@ -71,6 +72,7 @@ enum GovernanceActionKey { impl GovernanceAction { fn pending_key(&self) -> GovernanceActionKey { match self { + Self::SetAdmin(_) => GovernanceActionKey::Admin, Self::SetPaused(_) => GovernanceActionKey::Pause, Self::SetCurator(_) => GovernanceActionKey::Curator, Self::SetGovernance(_) => GovernanceActionKey::Governance, @@ -170,6 +172,14 @@ impl SorobanVaultGovernanceContract { Self::submit(env, caller, GovernanceAction::SetPaused(paused)) } + pub fn submit_set_admin( + env: Env, + caller: Address, + new_admin: Address, + ) -> Result { + Self::submit(env, caller, GovernanceAction::SetAdmin(new_admin)) + } + pub fn submit_set_curator( env: Env, caller: Address, @@ -513,6 +523,18 @@ impl SorobanVaultGovernanceContract { let now_ns = ledger_timestamp_ns(&env)?; let mut queue = load_queue(&env); + let mut matching = 0u32; + for entry in queue.iter() { + if action_kind(&entry.value.action) == kind { + matching = matching + .checked_add(1) + .ok_or(GovernanceError::ArithmeticOverflow)?; + } + } + if matching > 1 { + return Err(GovernanceError::DuplicatePending); + } + let proposal = match queue.take_by_key(now_ns, &kind, queued_proposal_kind) { TakePending::Ready(proposal) => proposal, TakePending::Missing => return Err(GovernanceError::ProposalNotFound), @@ -550,11 +572,29 @@ impl SorobanVaultGovernanceContract { ) -> Result { extend_instance_ttl(&env); require_revoker(&env, &caller)?; - let removed = revoke_where(&env, |action| action_kind(action) == kind); - if removed == 0 { + let mut queue = load_queue(&env); + let mut matching = 0u32; + for entry in queue.iter() { + if action_kind(&entry.value.action) == kind { + matching = matching + .checked_add(1) + .ok_or(GovernanceError::ArithmeticOverflow)?; + } + } + if matching == 0 { return Err(GovernanceError::ProposalNotFound); } - Ok(removed) + if matching > 1 { + return Err(GovernanceError::DuplicatePending); + } + + let removed = queue.revoke_by_key(&kind, queued_proposal_kind); + save_queue(&env, &queue); + let Some(proposal) = removed.first() else { + return Err(GovernanceError::ProposalNotFound); + }; + ProposalRevoked { id: proposal.id }.publish(&env); + Ok(1) } pub fn pending(env: Env, proposal_id: u64) -> Result { @@ -657,6 +697,7 @@ impl SorobanVaultGovernanceContract { fn action_kind(action: &GovernanceAction) -> GovernanceActionKind { match action { + GovernanceAction::SetAdmin(_) => GovernanceActionKind::Admin, GovernanceAction::SetPaused(_) => GovernanceActionKind::Pause, GovernanceAction::SetCurator(_) => GovernanceActionKind::Curator, GovernanceAction::SetGovernance(_) => GovernanceActionKind::Governance, @@ -685,6 +726,7 @@ fn action_kind(action: &GovernanceAction) -> GovernanceActionKind { fn timelock_kind_for_action(action: &GovernanceAction) -> TimelockKind { match action { + GovernanceAction::SetAdmin(_) => TimelockKind::Admin, GovernanceAction::SetPaused(_) => TimelockKind::Pause, GovernanceAction::SetCurator(_) => TimelockKind::Curator, GovernanceAction::SetGovernance(_) => TimelockKind::Governance, @@ -759,6 +801,13 @@ fn decide_submission( action: &GovernanceAction, ) -> Result { match action { + GovernanceAction::SetAdmin(new_admin) => { + let current = get_address(env, DataKey::Admin)?; + if ¤t == new_admin { + return Err(GovernanceError::NoChange); + } + Ok(TimelockDecision::Timelocked) + } GovernanceAction::SetPaused(paused) => { if *paused { return Err(GovernanceError::InvalidInput); @@ -870,6 +919,13 @@ fn decide_submission( } GovernanceAction::RemoveMarket(_) => Ok(TimelockDecision::from_requires_timelock(true)), GovernanceAction::SetGroupCap(cap_group_id, new_cap) => { + let known: Option = env + .storage() + .instance() + .get(&DataKey::KnownCapGroupCap(cap_group_id.clone())); + if known != Some(true) { + return Ok(TimelockDecision::Timelocked); + } let current: Option = env .storage() .instance() @@ -884,6 +940,13 @@ fn decide_submission( } } GovernanceAction::SetGroupRelCap(cap_group_id, new_relative_cap_wad) => { + let known: Option = env + .storage() + .instance() + .get(&DataKey::KnownCapGroupRelCap(cap_group_id.clone())); + if known != Some(true) { + return Ok(TimelockDecision::Timelocked); + } let current: Option = env .storage() .instance() @@ -901,6 +964,13 @@ fn decide_submission( } } GovernanceAction::SetGroupMember(market_id, cap_group_id) => { + let known: Option = env + .storage() + .instance() + .get(&DataKey::KnownCapGroupMembership(*market_id)); + if known != Some(true) { + return Ok(TimelockDecision::Timelocked); + } let current: Option = env .storage() .instance() @@ -1090,43 +1160,14 @@ fn revoke_by_action_key(env: &Env, key: &GovernanceActionKey) -> u32 { revoked_ids.len() } -fn revoke_where(env: &Env, pred: impl Fn(&GovernanceAction) -> bool) -> u32 { - let mut queue = load_queue(env); - let mut revoked_ids = Vec::new(env); - let mut keys = alloc::vec::Vec::new(); - - for entry in queue.iter() { - if pred(&entry.value.action) { - revoked_ids.push_back(entry.value.id); - let key = entry.value.action_key(); - if !keys.iter().any(|existing| existing == &key) { - keys.push(key); - } - } - } - - if revoked_ids.is_empty() { - return 0; - } - - for key in keys.iter() { - let _removed = queue.revoke_by_key(key, QueuedProposal::action_key); - } - - save_queue(env, &queue); - - for id in revoked_ids.iter() { - ProposalRevoked { id }.publish(env); - } - - revoked_ids.len() -} - #[allow(clippy::too_many_lines)] fn execute_action(env: &Env, action: &GovernanceAction) -> Result<(), GovernanceError> { let vault = get_address(env, DataKey::Vault)?; match action { + GovernanceAction::SetAdmin(new_admin) => { + env.storage().instance().set(&DataKey::Admin, new_admin); + } GovernanceAction::SetPaused(paused) => { execute_vault_governance_action(env, &vault, action)?; env.storage() @@ -1181,6 +1222,9 @@ fn execute_action(env: &Env, action: &GovernanceAction) -> Result<(), Governance env.storage() .instance() .set(&DataKey::CurrentCapGroupCap(cap_group_id.clone()), cap); + env.storage() + .instance() + .set(&DataKey::KnownCapGroupCap(cap_group_id.clone()), &true); } GovernanceAction::SetGroupRelCap(cap_group_id, relative_cap) => { execute_vault_governance_action(env, &vault, action)?; @@ -1188,6 +1232,9 @@ fn execute_action(env: &Env, action: &GovernanceAction) -> Result<(), Governance &DataKey::CurrentCapGroupRelCap(cap_group_id.clone()), relative_cap, ); + env.storage() + .instance() + .set(&DataKey::KnownCapGroupRelCap(cap_group_id.clone()), &true); } GovernanceAction::SetGroupMember(market_id, cap_group_id) => { execute_vault_governance_action(env, &vault, action)?; @@ -1197,6 +1244,9 @@ fn execute_action(env: &Env, action: &GovernanceAction) -> Result<(), Governance } else { env.storage().instance().set(&key, cap_group_id); } + env.storage() + .instance() + .set(&DataKey::KnownCapGroupMembership(*market_id), &true); } GovernanceAction::SetSkimRecipient(recipient) => { execute_vault_governance_action(env, &vault, action)?; @@ -1476,6 +1526,7 @@ fn governance_payload_for_action( GovernanceAction::Upgrade(_) | GovernanceAction::Migrate | GovernanceAction::CancelMigration + | GovernanceAction::SetAdmin(_) | GovernanceAction::SetTimelock(_, _) | GovernanceAction::Other(_, _) => None, }; diff --git a/contract/vault/soroban/governance/src/tests.rs b/contract/vault/soroban/governance/src/tests.rs index 5d8e38ef8..905da62d9 100644 --- a/contract/vault/soroban/governance/src/tests.rs +++ b/contract/vault/soroban/governance/src/tests.rs @@ -811,6 +811,169 @@ fn revoke_kind_removes_all_matching() { assert_eq!(pending.len(), 0); } +#[test] +fn accept_kind_rejects_ambiguous_broad_kind() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().set(LedgerInfo { + timestamp: 100, + protocol_version: 25, + ..Default::default() + }); + + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register( + SorobanVaultGovernanceContract, + (&admin, &vault, &(5_000_000_000u64)), + ); + + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_member( + env.clone(), + admin.clone(), + 1, + SdkString::from_str(&env, "group-a"), + ) + .unwrap(); + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_member( + env.clone(), + admin.clone(), + 2, + SdkString::from_str(&env, "group-b"), + ) + .unwrap(); + }); + + env.ledger().set(LedgerInfo { + timestamp: 106, + protocol_version: 25, + ..Default::default() + }); + + let accepted = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept_kind( + env.clone(), + admin.clone(), + GovernanceActionKind::CapGroup, + ) + }); + assert_eq!(accepted, Err(GovernanceError::DuplicatePending)); + + let pending = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::pending_ids(env.clone()) + }); + assert_eq!(pending.len(), 2); +} + +#[test] +fn revoke_kind_rejects_ambiguous_broad_kind() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().set(LedgerInfo { + timestamp: 100, + protocol_version: 25, + ..Default::default() + }); + + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register( + SorobanVaultGovernanceContract, + (&admin, &vault, &(5_000_000_000u64)), + ); + + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_timelock( + env.clone(), + admin.clone(), + TimelockKind::Curator, + 4_000_000_000, + ) + .unwrap(); + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_timelock( + env.clone(), + admin.clone(), + TimelockKind::Fees, + 4_000_000_000, + ) + .unwrap(); + }); + + let removed = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::revoke_kind( + env.clone(), + admin.clone(), + GovernanceActionKind::TimelockConfig, + ) + }); + assert_eq!(removed, Err(GovernanceError::DuplicatePending)); + + let pending = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::pending_ids(env.clone()) + }); + assert_eq!(pending.len(), 2); +} + +#[test] +fn in_flight_proposal_keeps_submit_time_timelock_after_timelock_raise() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().set(LedgerInfo { + timestamp: 100, + protocol_version: 25, + ..Default::default() + }); + + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register( + SorobanVaultGovernanceContract, + (&admin, &vault, &(5_000_000_000u64)), + ); + + let next_curator = Address::generate(&env); + let proposal_id = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_curator( + env.clone(), + admin.clone(), + next_curator.clone(), + ) + .unwrap() + }); + + env.ledger().set(LedgerInfo { + timestamp: 101, + protocol_version: 25, + ..Default::default() + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_timelock( + env.clone(), + admin.clone(), + TimelockKind::Curator, + 10_000_000_000, + ) + .unwrap(); + }); + + env.ledger().set(LedgerInfo { + timestamp: 106, + protocol_version: 25, + ..Default::default() + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id).unwrap() + }); + + let curator = env.as_contract(&vault, || MockVault::curator(env.clone())); + assert_eq!(curator, Some(next_curator)); +} + #[test] fn timelock_config_increase_immediate_decrease_timelocked() { let env = Env::default(); @@ -2139,7 +2302,7 @@ fn remove_market_is_timelocked_and_routes_to_vault() { } #[test] -fn group_cap_is_immediate_and_routes_to_vault() { +fn group_cap_unknown_state_is_timelocked_and_routes_after_maturity() { let env = Env::default(); env.mock_all_auths(); env.ledger().set(LedgerInfo { @@ -2158,7 +2321,7 @@ fn group_cap_is_immediate_and_routes_to_vault() { let group_id = SdkString::from_str(&env, "group-a"); let new_cap = 1_000_000i128; - let _proposal_id = env.as_contract(&governance, || { + let proposal_id = env.as_contract(&governance, || { SorobanVaultGovernanceContract::submit_set_group_cap( env.clone(), admin.clone(), @@ -2168,10 +2331,19 @@ fn group_cap_is_immediate_and_routes_to_vault() { .unwrap() }); - let pending = env.as_contract(&governance, || { - SorobanVaultGovernanceContract::pending_ids(env.clone()) + let early = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id) + }); + assert_eq!(early, Err(GovernanceError::ProposalNotMature)); + + env.ledger().set(LedgerInfo { + timestamp: 106, + protocol_version: 25, + ..Default::default() + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id).unwrap() }); - assert_eq!(pending.len(), 0); let on_vault_id = env.as_contract(&vault, || MockVault::last_group_cap_group_id(env.clone())); assert_eq!(on_vault_id, Some(group_id)); @@ -2236,7 +2408,7 @@ fn group_cap_raise_uses_mirrored_current_cap_and_is_timelocked() { } #[test] -fn group_rel_cap_is_immediate_and_routes_to_vault() { +fn group_rel_cap_unknown_state_is_timelocked_and_routes_after_maturity() { let env = Env::default(); env.mock_all_auths(); env.ledger().set(LedgerInfo { @@ -2255,7 +2427,7 @@ fn group_rel_cap_is_immediate_and_routes_to_vault() { let group_id = SdkString::from_str(&env, "group-b"); let rel_cap_wad = 500_000_000_000_000_000i128; // 0.5 wad - let _proposal_id = env.as_contract(&governance, || { + let proposal_id = env.as_contract(&governance, || { SorobanVaultGovernanceContract::submit_set_group_rel_cap( env.clone(), admin.clone(), @@ -2265,10 +2437,19 @@ fn group_rel_cap_is_immediate_and_routes_to_vault() { .unwrap() }); - let pending = env.as_contract(&governance, || { - SorobanVaultGovernanceContract::pending_ids(env.clone()) + let early = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id) + }); + assert_eq!(early, Err(GovernanceError::ProposalNotMature)); + + env.ledger().set(LedgerInfo { + timestamp: 106, + protocol_version: 25, + ..Default::default() + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id).unwrap() }); - assert_eq!(pending.len(), 0); let on_vault_id = env.as_contract(&vault, || { MockVault::last_group_rel_cap_group_id(env.clone()) @@ -2392,7 +2573,7 @@ fn group_member_assignment_is_timelocked_and_routes_to_vault() { } #[test] -fn group_member_removal_without_existing_membership_is_no_change() { +fn group_member_removal_without_known_membership_is_timelocked() { let env = Env::default(); env.mock_all_auths(); env.ledger().set(LedgerInfo { @@ -2411,21 +2592,25 @@ fn group_member_removal_without_existing_membership_is_no_change() { let market_id = 5u32; let empty_group = SdkString::from_str(&env, ""); - let proposal = env.as_contract(&governance, || { + let proposal_id = env.as_contract(&governance, || { SorobanVaultGovernanceContract::submit_set_group_member( env.clone(), admin.clone(), market_id, empty_group.clone(), ) + .unwrap() }); - assert_eq!(proposal, Err(GovernanceError::NoChange)); + let early = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id) + }); + assert_eq!(early, Err(GovernanceError::ProposalNotMature)); let pending = env.as_contract(&governance, || { SorobanVaultGovernanceContract::pending_ids(env.clone()) }); - assert_eq!(pending.len(), 0); + assert_eq!(pending.len(), 1); } #[test] @@ -2575,3 +2760,193 @@ fn sentinel_revoke_kind_clears_pending() { }); assert_eq!(pending_after.len(), 0); } + +#[test] +fn admin_rotation_is_timelocked_and_updates_admin() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().set(LedgerInfo { + timestamp: 100, + protocol_version: 25, + ..Default::default() + }); + + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register( + SorobanVaultGovernanceContract, + (&admin, &vault, &(5_000_000_000u64)), + ); + let next_admin = Address::generate(&env); + + let proposal_id = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_admin( + env.clone(), + admin.clone(), + next_admin.clone(), + ) + .unwrap() + }); + + let early = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id) + }); + assert_eq!(early, Err(GovernanceError::ProposalNotMature)); + + env.ledger().set(LedgerInfo { + timestamp: 106, + protocol_version: 25, + ..Default::default() + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), proposal_id).unwrap() + }); + + let stored_admin = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::admin(env.clone()).unwrap() + }); + assert_eq!(stored_admin, next_admin); + + let old_admin_result = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_curator( + env.clone(), + admin.clone(), + Address::generate(&env), + ) + }); + assert_eq!(old_admin_result, Err(GovernanceError::Unauthorized)); +} + +#[test] +fn unknown_group_membership_state_is_conservatively_timelocked() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().set(LedgerInfo { + timestamp: 100, + protocol_version: 25, + ..Default::default() + }); + + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register( + SorobanVaultGovernanceContract, + (&admin, &vault, &(5_000_000_000u64)), + ); + + let clear_id = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_member( + env.clone(), + admin.clone(), + 9, + SdkString::from_str(&env, ""), + ) + .unwrap() + }); + + let early = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), clear_id) + }); + assert_eq!(early, Err(GovernanceError::ProposalNotMature)); +} + +#[test] +fn unknown_cap_group_state_is_conservatively_timelocked() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().set(LedgerInfo { + timestamp: 100, + protocol_version: 25, + ..Default::default() + }); + + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register( + SorobanVaultGovernanceContract, + (&admin, &vault, &(5_000_000_000u64)), + ); + + let cap_id = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_cap( + env.clone(), + admin.clone(), + SdkString::from_str(&env, "legacy-group"), + 20, + ) + .unwrap() + }); + let early_cap = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), cap_id) + }); + assert_eq!(early_cap, Err(GovernanceError::ProposalNotMature)); + + let rel_id = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_rel_cap( + env.clone(), + admin.clone(), + SdkString::from_str(&env, "legacy-relative-group"), + 20, + ) + .unwrap() + }); + let early_rel = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept(env.clone(), admin.clone(), rel_id) + }); + assert_eq!(early_rel, Err(GovernanceError::ProposalNotMature)); +} + +#[test] +fn classifier_counts_distinct_cap_group_subtypes_as_ambiguous_broad_kind() { + let env = Env::default(); + env.mock_all_auths(); + env.ledger().set(LedgerInfo { + timestamp: 100, + protocol_version: 25, + ..Default::default() + }); + + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register( + SorobanVaultGovernanceContract, + (&admin, &vault, &(5_000_000_000u64)), + ); + + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_cap( + env.clone(), + admin.clone(), + SdkString::from_str(&env, "group-a"), + 5, + ) + .unwrap(); + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_member( + env.clone(), + admin.clone(), + 1, + SdkString::from_str(&env, "group-a"), + ) + .unwrap(); + }); + env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_group_cap( + env.clone(), + admin.clone(), + SdkString::from_str(&env, "group-a"), + 10, + ) + .unwrap(); + }); + + let accepted = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::accept_kind( + env.clone(), + admin.clone(), + GovernanceActionKind::CapGroup, + ) + }); + assert_eq!(accepted, Err(GovernanceError::DuplicatePending)); +} diff --git a/contract/vault/soroban/governance/src/types.rs b/contract/vault/soroban/governance/src/types.rs index 93d600138..711269e92 100644 --- a/contract/vault/soroban/governance/src/types.rs +++ b/contract/vault/soroban/governance/src/types.rs @@ -24,11 +24,15 @@ pub(crate) enum DataKey { CurrentCap(u32), CurrentCapGroupCap(String), CurrentCapGroupRelCap(String), + KnownCapGroupCap(String), + KnownCapGroupRelCap(String), + KnownCapGroupMembership(u32), } #[contracttype] #[derive(Clone, Copy, Eq, PartialEq, Ord, PartialOrd)] pub enum TimelockKind { + Admin, Pause, Curator, Governance, @@ -52,6 +56,7 @@ pub enum TimelockKind { #[contracttype] #[derive(Clone, Copy, Eq, PartialEq)] pub enum GovernanceActionKind { + Admin, Pause, Curator, Governance, @@ -76,6 +81,7 @@ pub enum GovernanceActionKind { #[contracttype] #[derive(Clone, Eq, PartialEq)] pub struct Timelocks { + pub admin_ns: u64, pub pause_ns: u64, pub curator_ns: u64, pub governance_ns: u64, @@ -99,6 +105,7 @@ pub struct Timelocks { impl Timelocks { pub(crate) fn from_default(default_ns: u64) -> Self { Self { + admin_ns: default_ns, pause_ns: default_ns, curator_ns: default_ns, governance_ns: default_ns, @@ -122,6 +129,7 @@ impl Timelocks { pub(crate) fn get(self, kind: TimelockKind) -> u64 { match kind { + TimelockKind::Admin => self.admin_ns, TimelockKind::Pause => self.pause_ns, TimelockKind::Curator => self.curator_ns, TimelockKind::Governance => self.governance_ns, @@ -145,6 +153,7 @@ impl Timelocks { pub(crate) fn set(&mut self, kind: TimelockKind, value: u64) { match kind { + TimelockKind::Admin => self.admin_ns = value, TimelockKind::Pause => self.pause_ns = value, TimelockKind::Curator => self.curator_ns = value, TimelockKind::Governance => self.governance_ns = value, @@ -207,6 +216,7 @@ impl RestrictionMode { #[contracttype] #[derive(Clone, Eq, PartialEq)] pub enum GovernanceAction { + SetAdmin(Address), SetPaused(bool), SetCurator(Address), SetGovernance(Address), diff --git a/contract/vault/soroban/src/contract/curator_vault.rs b/contract/vault/soroban/src/contract/curator_vault.rs index 04e24686c..4781247d3 100644 --- a/contract/vault/soroban/src/contract/curator_vault.rs +++ b/contract/vault/soroban/src/contract/curator_vault.rs @@ -995,7 +995,13 @@ where target_ids: Vec, ) -> Result<(), RuntimeError> { self.auth.authorize(ActionKind::PolicyAdmin, caller, None)?; + self.set_supply_queue_authorized(target_ids) + } + pub fn set_supply_queue_authorized( + &mut self, + target_ids: Vec, + ) -> Result<(), RuntimeError> { let mut entries = Vec::with_capacity(target_ids.len()); for target_id in target_ids { let config = self @@ -1064,7 +1070,14 @@ where new_cap: u128, ) -> Result<(), RuntimeError> { self.auth.authorize(ActionKind::PolicyAdmin, caller, None)?; + self.apply_governance_cap_authorized(market_id, new_cap) + } + pub fn apply_governance_cap_authorized( + &mut self, + market_id: TargetId, + new_cap: u128, + ) -> Result<(), RuntimeError> { if self.policy_state.market_config(market_id).is_some() { self.policy_state .set_market_cap(market_id, new_cap) @@ -1118,7 +1131,13 @@ where market_id: TargetId, ) -> Result<(), RuntimeError> { self.auth.authorize(ActionKind::PolicyAdmin, caller, None)?; + self.apply_governance_remove_market_authorized(market_id) + } + pub fn apply_governance_remove_market_authorized( + &mut self, + market_id: TargetId, + ) -> Result<(), RuntimeError> { let Some(config) = self.policy_state.market_config(market_id) else { return Err(RuntimeError::invalid_input("")); }; @@ -1225,7 +1244,13 @@ where update: CapGroupUpdate, ) -> Result<(), RuntimeError> { self.auth.authorize(ActionKind::PolicyAdmin, caller, None)?; + self.apply_governance_cap_group_update_authorized(update) + } + pub fn apply_governance_cap_group_update_authorized( + &mut self, + update: CapGroupUpdate, + ) -> Result<(), RuntimeError> { match update { CapGroupUpdate::SetCap { cap_group_id, diff --git a/contract/vault/soroban/src/contract/entrypoints.rs b/contract/vault/soroban/src/contract/entrypoints.rs index 762866da1..2eac2069d 100644 --- a/contract/vault/soroban/src/contract/entrypoints.rs +++ b/contract/vault/soroban/src/contract/entrypoints.rs @@ -157,6 +157,7 @@ fn apply_supply_queue_policy( env: &Env, caller_kernel: Address, target_ids: soroban_sdk::Vec, + caller_preauthorized: bool, ) -> Result<(), ContractError> { if let Some(adapters) = env .storage() @@ -178,7 +179,11 @@ fn apply_supply_queue_policy( let targets = queue_targets .take() .ok_or_else(|| RuntimeError::invalid_state(""))?; - vault.set_supply_queue(caller_kernel, targets) + if caller_preauthorized { + vault.set_supply_queue_authorized(targets) + } else { + vault.set_supply_queue(caller_kernel, targets) + } }; with_contract_vault_contract_error(env, &mut call) } @@ -188,10 +193,15 @@ fn apply_cap_policy( caller_kernel: Address, market_id: u32, new_cap: i128, + caller_preauthorized: bool, ) -> Result<(), ContractError> { let new_cap_u128 = to_u128(new_cap)?; let mut call = |vault: &mut ContractVault<'_>| -> Result<(), RuntimeError> { - vault.apply_governance_cap(caller_kernel, market_id, new_cap_u128) + if caller_preauthorized { + vault.apply_governance_cap_authorized(market_id, new_cap_u128) + } else { + vault.apply_governance_cap(caller_kernel, market_id, new_cap_u128) + } }; with_contract_vault_contract_error(env, &mut call) } @@ -200,9 +210,14 @@ fn apply_remove_market_policy( env: &Env, caller_kernel: Address, market_id: u32, + caller_preauthorized: bool, ) -> Result<(), ContractError> { let mut call = |vault: &mut ContractVault<'_>| -> Result<(), RuntimeError> { - vault.apply_governance_remove_market(caller_kernel, market_id) + if caller_preauthorized { + vault.apply_governance_remove_market_authorized(market_id) + } else { + vault.apply_governance_remove_market(caller_kernel, market_id) + } }; with_contract_vault_contract_error(env, &mut call) } @@ -291,6 +306,7 @@ fn apply_group_policy( market_id: Option, cap_group_id: Option, value: Option, + caller_preauthorized: bool, ) -> Result<(), ContractError> { fn parse_cap_group(raw: alloc::string::String) -> Result { CapGroupId::try_from(raw).map_err(|_| ContractError::InvalidInput) @@ -332,7 +348,11 @@ fn apply_group_policy( let update = internal .take() .ok_or_else(|| RuntimeError::invalid_state(""))?; - vault.apply_governance_cap_group_update(caller_kernel, update) + if caller_preauthorized { + vault.apply_governance_cap_group_update_authorized(update) + } else { + vault.apply_governance_cap_group_update(caller_kernel, update) + } }; with_contract_vault_contract_error(env, &mut call) } @@ -650,6 +670,7 @@ fn set_governance_policy_impl( env, caller_kernel, target_ids.ok_or(ContractError::InvalidInput)?, + caller_preauthorized, ) } GOVERNANCE_POLICY_KIND_CAP => apply_cap_policy( @@ -657,11 +678,13 @@ fn set_governance_policy_impl( governance_kernel()?, market_id.ok_or(ContractError::InvalidInput)?, required_i128(value)?, + caller_preauthorized, ), GOVERNANCE_POLICY_KIND_REMOVE_MARKET => apply_remove_market_policy( env, governance_kernel()?, market_id.ok_or(ContractError::InvalidInput)?, + caller_preauthorized, ), GOVERNANCE_POLICY_KIND_RESTRICTIONS => apply_restrictions_policy( env, @@ -677,6 +700,7 @@ fn set_governance_policy_impl( market_id, cap_group_id, value, + caller_preauthorized, ), GOVERNANCE_POLICY_KIND_PAUSED => { let paused = match mode.ok_or(ContractError::InvalidInput)? { diff --git a/contract/vault/soroban/src/contract/helpers.rs b/contract/vault/soroban/src/contract/helpers.rs index 1d112e418..7b96d868b 100644 --- a/contract/vault/soroban/src/contract/helpers.rs +++ b/contract/vault/soroban/src/contract/helpers.rs @@ -404,7 +404,7 @@ pub(crate) fn load_vault_bootstrap(env: &Env) -> Result, Runt migrate_legacy_paused(env); let curator: SdkAddress = require_config_address(env, &VaultDataKey::Curator, "curator not set")?; - let governance: SdkAddress = + let _governance: SdkAddress = require_config_address(env, &VaultDataKey::Governance, "governance not set")?; let asset_token: SdkAddress = require_config_address(env, &VaultDataKey::AssetToken, "asset token not set")?; @@ -414,7 +414,6 @@ pub(crate) fn load_vault_bootstrap(env: &Env) -> Result, Runt let vault_sdk = env.current_contract_address(); let vault_kernel = kernel_address_from_sdk(env, &vault_sdk); let curator_kernel = kernel_address_from_sdk(env, &curator); - let governance_kernel = kernel_address_from_sdk(env, &governance); let asset_kernel = kernel_address_from_sdk(env, &asset_token); let share_kernel = kernel_address_from_sdk(env, &share_token); @@ -434,7 +433,6 @@ pub(crate) fn load_vault_bootstrap(env: &Env) -> Result, Runt let storage = SorobanStorage::new(env); let paused = storage.is_paused(); let mut rbac_config = RbacConfig::with_curator(curator_kernel); - rbac_config.add_role(governance_kernel, Role::Curator); load_rbac_addresses( env, diff --git a/contract/vault/soroban/src/tests.rs b/contract/vault/soroban/src/tests.rs index 353839dba..b9138918f 100644 --- a/contract/vault/soroban/src/tests.rs +++ b/contract/vault/soroban/src/tests.rs @@ -2000,7 +2000,7 @@ mod storage_tests { GOVERNANCE_CONFIG_KIND_GOVERNANCE, GOVERNANCE_CONFIG_KIND_GUARDIANS, GOVERNANCE_CONFIG_KIND_SENTINEL, GOVERNANCE_POLICY_KIND_CAP, GOVERNANCE_POLICY_KIND_GROUP, GOVERNANCE_POLICY_KIND_PAUSED, GOVERNANCE_POLICY_KIND_REMOVE_MARKET, - GOVERNANCE_POLICY_KIND_RESTRICTIONS, + GOVERNANCE_POLICY_KIND_RESTRICTIONS, GOVERNANCE_POLICY_KIND_SUPPLY_QUEUE, }; use templar_vault_kernel::{ Address as KernelAddress, AllocationPlanEntry, FeeAccrualAnchor, OpState, @@ -2801,6 +2801,132 @@ mod storage_tests { }); } + #[test] + fn test_execute_governance_supply_queue_applies_without_curator_role() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + let contract_id = env.register(SorobanVaultContract, ()); + let curator = SdkAddress::generate(&env); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); + + env.as_contract(&contract_id, || { + SorobanVaultContract::initialize( + env.clone(), + curator, + governance.clone(), + asset, + share, + 0, + 0, + ) + .unwrap(); + + let mut storage = SorobanStorage::new(&env); + let mut policy_state = PolicyState::default(); + policy_state + .set_market_config(7, MarketConfig::new(true, 100, None)) + .unwrap(); + Storage::save_policy_state(&mut storage, &policy_state).unwrap(); + + let target_ids = alloc::vec![7u32]; + let payload = Bytes::from_slice( + &env, + &GovernanceCommand::SetGovernancePolicy { + kind: GOVERNANCE_POLICY_KIND_SUPPLY_QUEUE, + target_ids: Some(target_ids), + mode: None, + accounts: None, + market_id: None, + cap_group_id: None, + value: None, + value_b: None, + value_c: None, + } + .encode(), + ); + SorobanVaultContract::execute_governance(env.clone(), governance.clone(), payload) + .unwrap(); + + let reloaded = Storage::load_policy_state(&storage) + .unwrap() + .unwrap_or_default(); + let queue = reloaded.supply_queue(); + assert_eq!(queue.entries().len(), 1); + assert_eq!(queue.entries()[0].target_id, 7); + }); + } + + #[test] + fn test_governance_policy_execution_does_not_grant_curator_role() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + let contract_id = env.register(SorobanVaultContract, ()); + let curator = SdkAddress::generate(&env); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); + + env.as_contract(&contract_id, || { + SorobanVaultContract::initialize( + env.clone(), + curator, + governance.clone(), + asset, + share, + 0, + 0, + ) + .unwrap(); + + let bootstrap = crate::contract::helpers::load_vault_bootstrap(&env).unwrap(); + assert!(!bootstrap.auth.config().has_role( + &crate::contract::helpers::kernel_address_from_sdk(&env, &governance), + templar_curator_primitives::rbac::Role::Curator, + )); + }); + } + + #[test] + fn test_sentinel_cannot_execute_governance_cap_policy() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + let contract_id = env.register(SorobanVaultContract, ()); + let curator = SdkAddress::generate(&env); + let sentinel = SdkAddress::generate(&env); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); + + env.as_contract(&contract_id, || { + SorobanVaultContract::initialize(env.clone(), curator, governance, asset, share, 0, 0) + .unwrap(); + set_config_address(&env, &crate::contract::VaultDataKey::Sentinel, &sentinel); + + let mut storage = SorobanStorage::new(&env); + let mut policy_state = PolicyState::default(); + policy_state + .set_market_config(7, MarketConfig::new(true, 100, None)) + .unwrap(); + Storage::save_policy_state(&mut storage, &policy_state).unwrap(); + + let payload = Bytes::from_slice( + &env, + &GovernanceCommand::SetGovernancePolicy { + kind: GOVERNANCE_POLICY_KIND_CAP, + target_ids: None, + mode: None, + accounts: None, + market_id: Some(7), + cap_group_id: None, + value: Some(200), + value_b: None, + value_c: None, + } + .encode(), + ); + + assert!( + SorobanVaultContract::execute_governance(env.clone(), sentinel, payload).is_err() + ); + }); + } + #[test] fn test_execute_governance_cap_creates_new_runtime_market_after_timelock() { let env = Env::default();