fix: ask the admin to sign each call once

contracts/async-vault/src/error.rs

@@ -52,8 +52,10 @@ pub enum VaultError {
     /// The epoch is priced, so the request can no longer be cancelled. The
     /// holder claims instead.
     AlreadyPriced = 6039,
-    /// The vault must always have an admin. Governance hands over in two steps
-    /// rather than stepping down into nobody.
+    /// The call needs the admin. `unpause` refuses any other caller.
+    /// `renounce_admin` is always refused, because the vault must always have
+    /// an admin: governance hands over in two steps rather than stepping down
+    /// into nobody.
     AdminRequired = 6040,
     /// The epoch is sealed and the feed can price it, so the price it will take
     /// is already knowable. Cancelling now would be declining a price after

contracts/async-vault/src/guards.rs

@@ -1,5 +1,8 @@
 use soroban_sdk::{panic_with_error, Address, Env, Symbol};
-use stellar_access::{access_control::AccessControlStorageKey, role_transfer};
+use stellar_access::{
+    access_control::{self, AccessControlStorageKey},
+    role_transfer,
+};
 use stellar_tokens::{
     fungible::FungibleTokenClient,
     rwa::{compliance::ComplianceClient as RulesClient, RWATokenClient},
@@ -94,6 +97,13 @@ pub(crate) fn no_hand_over_pending(e: &Env) {
     }
 }
 
+/// Stands in for `#[only_admin]` where an OpenZeppelin trait fixes the caller.
+pub(crate) fn caller_is_admin(e: &Env, caller: &Address) {
+    if access_control::get_admin(e).as_ref() != Some(caller) {
+        panic_with_error!(e, VaultError::AdminRequired);
+    }
+}
+
 pub(crate) fn wiring_complete(e: &Env) {
     if !governance::wiring(e).complete() {
         panic_with_error!(e, VaultError::SetupIncomplete);

contracts/async-vault/src/lib.rs

@@ -178,8 +178,7 @@ impl AsyncVault {
     }
 
     #[only_admin]
-    pub fn set_deposit_cap(e: &Env, cap: Option<i128>, caller: Address) {
-        caller.require_auth();
+    pub fn set_deposit_cap(e: &Env, cap: Option<i128>) {
         guards::non_negative_cap(e, cap);
         let old_cap = state::deposit_cap(e);
         state::set_deposit_cap(e, cap);
@@ -193,17 +192,15 @@ impl AsyncVault {
     /// Instant during setup; afterwards the custodian changes through the
     /// timelock.
     #[only_admin]
-    pub fn set_custodian(e: &Env, custodian: Address, caller: Address) {
-        caller.require_auth();
+    pub fn set_custodian(e: &Env, custodian: Address) {
         guards::direct_change_allowed(e);
         treasury::set_custodian(e, &custodian);
     }
 
     /// Raising is instant. Lowering shortens the warning holders get before a
     /// wind-down starts, so after setup it waits for the timelock.
     #[only_admin]
-    pub fn set_wind_down_delay(e: &Env, secs: u64, caller: Address) {
-        caller.require_auth();
+    pub fn set_wind_down_delay(e: &Env, secs: u64) {
         guards::wind_down_delay_in_bounds(e, secs);
         guards::not_lowered_after_setup(e, secs, wind_down::delay(e));
         wind_down::set_delay(e, secs);
@@ -218,14 +215,12 @@ impl AsyncVault {
     }
 
     #[only_admin]
-    pub fn propose_wind_down(e: &Env, caller: Address) {
-        caller.require_auth();
+    pub fn propose_wind_down(e: &Env) {
         wind_down::propose(e);
     }
 
     #[only_admin]
-    pub fn cancel_wind_down_proposal(e: &Env, caller: Address) {
-        caller.require_auth();
+    pub fn cancel_wind_down_proposal(e: &Env) {
         wind_down::cancel_proposal(e);
     }
 
@@ -267,8 +262,7 @@ impl AsyncVault {
     /// Raising is instant. Lowering shortens the wait every closed epoch gives
     /// before it prices, so after setup it waits for the timelock.
     #[only_admin]
-    pub fn set_notice(e: &Env, secs: u64, caller: Address) {
-        caller.require_auth();
+    pub fn set_notice(e: &Env, secs: u64) {
         guards::not_lowered_after_setup(e, secs, state::notice(e));
         guards::notice_in_bounds(e, secs);
         governance::set_notice(e, secs);
@@ -279,8 +273,7 @@ impl AsyncVault {
     /// manager, allowlisted, not frozen), no shares outstanding and no admin
     /// hand-over pending. Deposits open from here on, and it never reopens.
     #[only_admin]
-    pub fn finish_setup(e: &Env, caller: Address) {
-        caller.require_auth();
+    pub fn finish_setup(e: &Env) {
         governance::finish_setup(e);
     }
 
@@ -297,22 +290,19 @@ impl AsyncVault {
 
     /// Queues a change behind the timelock and returns its id.
     #[only_admin]
-    pub fn propose_change(e: &Env, change: VaultChange, caller: Address) -> u32 {
-        caller.require_auth();
+    pub fn propose_change(e: &Env, change: VaultChange) -> u32 {
         governance::propose(e, change)
     }
 
     #[only_admin]
-    pub fn cancel_change(e: &Env, id: u32, caller: Address) {
-        caller.require_auth();
+    pub fn cancel_change(e: &Env, id: u32) {
         governance::cancel(e, id);
     }
 
     /// Lands a change whose delay has run. Refused while paused or winding
     /// down.
     #[only_admin]
-    pub fn apply_change(e: &Env, id: u32, caller: Address) {
-        caller.require_auth();
+    pub fn apply_change(e: &Env, id: u32) {
         governance::apply(e, id);
     }
 
@@ -452,8 +442,8 @@ impl Pausable for AsyncVault {
         strata_timelock::pause::start(e);
     }
 
-    #[only_admin]
     fn unpause(e: &Env, caller: Address) {
+        guards::caller_is_admin(e, &caller);
         caller.require_auth();
         pausable::unpause(e);
         strata_timelock::pause::end(e);

contracts/async-vault/src/test/compliance.rs

@@ -24,7 +24,7 @@ fn an_investor_who_was_never_allowlisted_is_refused_before_any_cash_moves() {
 fn a_delisted_investor_cannot_request_a_deposit() {
     let f = setup();
     let investor = f.investor(1_000);
-    f.identity.allow(&investor, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
 
     refused(
         f.vault.try_request_deposit(&investor, &400),
@@ -48,9 +48,9 @@ fn relisting_after_pricing_lets_the_claim_through() {
     let investor = f.investor(1_000);
     f.vault.request_deposit(&investor, &400);
     f.fulfill_epoch(wad(2));
-    f.identity.allow(&investor, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
 
-    f.identity.allow(&investor, &true, &f.compliance);
+    f.identity.allow(&investor, &true);
 
     assert_eq!(f.vault.claim_deposit(&investor, &1), 200);
     assert_eq!(f.shares(&investor), 200);
@@ -65,14 +65,14 @@ fn a_holder_delisted_after_pricing_is_paid_through_a_wind_down() {
     f.fulfill_epoch(wad(2));
     assert_eq!(f.vault.cancellable_escrow(), 0);
     assert_eq!(f.vault.pending_mint_shares(), 200);
-    f.identity.allow(&investor, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
 
     refused(
         f.vault.try_cancel_deposit(&investor, &epoch),
         VaultError::AlreadyPriced,
     );
 
-    f.vault.propose_wind_down(&f.admin);
+    f.vault.propose_wind_down();
     f.advance(MIN_WIND_DOWN_DELAY);
     f.vault.activate_wind_down();
     assert_eq!(f.vault.claim_deposit(&investor, &epoch), 0);
@@ -119,7 +119,7 @@ fn a_delisted_holder_can_still_queue_an_exit() {
     let user = f.holder(500);
     let epoch = f.vault.current_epoch();
 
-    f.identity.allow(&user, &false, &f.compliance);
+    f.identity.allow(&user, &false);
 
     assert_eq!(f.vault.request_redeem(&user, &200), epoch);
     assert_eq!(f.shares(&f.vault.address), 200);
@@ -132,7 +132,7 @@ fn a_delisted_holder_can_still_claim_their_exit() {
     let epoch = f.vault.request_redeem(&user, &200);
     f.fulfill_epoch(wad(2));
 
-    f.identity.allow(&user, &false, &f.compliance);
+    f.identity.allow(&user, &false);
 
     assert_eq!(f.vault.claim_redeem(&user, &epoch), 400);
     assert_eq!(f.balance(&user), 400);
@@ -145,7 +145,7 @@ fn a_delisted_investor_can_cancel_deposit_and_receive_asset() {
     let epoch = f.vault.request_deposit(&investor, &400);
 
     // Investor gets delisted
-    f.identity.allow(&investor, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
 
     // Delisted investor can still cancel deposit and receive settlement asset
     assert_eq!(f.vault.cancel_deposit(&investor, &epoch), 400);
@@ -159,7 +159,7 @@ fn a_delisted_holder_cannot_cancel_redemption_back_to_shares() {
     let epoch = f.vault.request_redeem(&user, &200);
 
     // Delisted before pricing
-    f.identity.allow(&user, &false, &f.compliance);
+    f.identity.allow(&user, &false);
 
     refused(
         f.vault.try_cancel_redeem(&user, &epoch),
@@ -202,11 +202,11 @@ fn a_frozen_or_delisted_holder_cannot_transfer_shares() {
     assert_eq!(f.shares(&recipient), 100);
 
     // Delisted sender cannot transfer
-    f.identity.allow(&user, &false, &f.compliance);
+    f.identity.allow(&user, &false);
     assert!(f.share.try_transfer(&user, &recipient, &100).is_err());
 
     // Restore allowlist, then freeze sender
-    f.identity.allow(&user, &true, &f.compliance);
+    f.identity.allow(&user, &true);
     f.share.set_address_frozen(&user, &true, &f.compliance);
     assert!(f.share.try_transfer(&user, &recipient, &100).is_err());
 }
@@ -321,7 +321,7 @@ fn a_frozen_holder_is_not_moved_into_the_exit_queue() {
     let investor = f.investor(1_000);
     let epoch = f.vault.request_deposit(&investor, &400);
     f.fulfill_epoch(wad(2));
-    f.identity.allow(&investor, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
     f.share.set_address_frozen(&investor, &true, &f.compliance);
 
     refused(

contracts/async-vault/src/test/controls.rs

@@ -44,6 +44,17 @@ fn the_guardian_pauses_but_only_governance_unpauses() {
     assert!(!f.vault.paused());
 }
 
+#[test]
+fn a_caller_other_than_the_admin_cannot_unpause_even_when_it_signs() {
+    let f = setup();
+    f.vault.pause(&f.guardian);
+
+    for who in [f.guardian.clone(), Address::generate(&f.e)] {
+        refused(f.vault.try_unpause(&who), VaultError::AdminRequired);
+    }
+    assert!(f.vault.paused());
+}
+
 #[test]
 fn a_paused_vault_still_lets_investors_claim() {
     let f = setup();
@@ -108,17 +119,17 @@ fn governance_can_replace_the_guardian() {
     let f = setup();
     let next = Address::generate(&f.e);
 
-    let grant = f.vault.propose_change(
-        &VaultChange::GrantRole(symbol_short!("guardian"), next.clone()),
-        &f.admin,
-    );
-    let revoke = f.vault.propose_change(
-        &VaultChange::RevokeRole(symbol_short!("guardian"), f.guardian.clone()),
-        &f.admin,
-    );
+    let grant = f.vault.propose_change(&VaultChange::GrantRole(
+        symbol_short!("guardian"),
+        next.clone(),
+    ));
+    let revoke = f.vault.propose_change(&VaultChange::RevokeRole(
+        symbol_short!("guardian"),
+        f.guardian.clone(),
+    ));
     f.advance(MIN_UPGRADE_DELAY);
-    f.vault.apply_change(&grant, &f.admin);
-    f.vault.apply_change(&revoke, &f.admin);
+    f.vault.apply_change(&grant);
+    f.vault.apply_change(&revoke);
 
     assert_eq!(f.vault.guardian(), Some(next.clone()));
     assert!(f.vault.try_pause(&f.guardian).is_err());
@@ -242,11 +253,9 @@ fn governance_hands_over_in_two_steps_instead() {
     let f = setup();
     let next = Address::generate(&f.e);
 
-    let id = f
-        .vault
-        .propose_change(&VaultChange::Admin(next.clone()), &f.admin);
+    let id = f.vault.propose_change(&VaultChange::Admin(next.clone()));
     f.advance(MIN_UPGRADE_DELAY);
-    f.vault.apply_change(&id, &f.admin);
+    f.vault.apply_change(&id);
     f.vault.accept_admin_transfer();
 
     assert_eq!(f.vault.governance(), Some(next.clone()));

contracts/async-vault/src/test/delisted.rs

@@ -9,7 +9,7 @@ fn delisted_with_a_priced_deposit(f: &Fixture) -> (Address, u64) {
     let investor = f.investor(1_000);
     let epoch = f.vault.request_deposit(&investor, &400);
     f.fulfill_epoch(wad(2));
-    f.identity.allow(&investor, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
     (investor, epoch)
 }
 
@@ -88,7 +88,7 @@ fn a_conversion_adds_to_a_redemption_already_open_in_the_epoch() {
     f.fulfill_epoch(wad(2));
     let open = f.vault.current_epoch();
     f.vault.request_redeem(&investor, &100);
-    f.identity.allow(&investor, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
 
     f.vault.claim_deposit(&investor, &epoch);
 
@@ -107,7 +107,7 @@ fn a_conversion_adds_to_a_redemption_already_open_in_the_epoch() {
 fn in_a_wind_down_the_shares_are_credited_before_the_first_round() {
     let f = setup();
     let (investor, epoch) = delisted_with_a_priced_deposit(&f);
-    f.vault.propose_wind_down(&f.admin);
+    f.vault.propose_wind_down();
     f.advance(MIN_WIND_DOWN_DELAY);
     f.vault.activate_wind_down();
 
@@ -140,7 +140,7 @@ fn in_a_wind_down_the_shares_are_credited_before_the_first_round() {
 fn in_a_wind_down_the_shares_are_credited_after_the_first_round() {
     let f = setup();
     let (investor, epoch) = delisted_with_a_priced_deposit(&f);
-    f.vault.propose_wind_down(&f.admin);
+    f.vault.propose_wind_down();
     f.advance(MIN_WIND_DOWN_DELAY);
     f.vault.activate_wind_down();
     assert_eq!(f.vault.finalize_wind_down_round(), 400);
@@ -157,7 +157,7 @@ fn a_credit_before_the_first_round_leaves_every_holder_their_share() {
     let f = setup();
     let other = f.holder(200);
     let (investor, epoch) = delisted_with_a_priced_deposit(&f);
-    f.vault.propose_wind_down(&f.admin);
+    f.vault.propose_wind_down();
     f.advance(MIN_WIND_DOWN_DELAY);
     f.vault.activate_wind_down();
     f.vault.claim_deposit(&investor, &epoch);
@@ -193,7 +193,7 @@ fn a_conversion_caught_by_a_wind_down_is_paid_from_the_rounds() {
     let f = setup();
     let other = f.holder(200);
     let (investor, epoch) = delisted_with_a_priced_deposit(&f);
-    f.vault.propose_wind_down(&f.admin);
+    f.vault.propose_wind_down();
     f.vault.claim_deposit(&investor, &epoch);
     let open = f.vault.current_epoch();
     f.advance(MIN_WIND_DOWN_DELAY);
@@ -223,7 +223,7 @@ fn a_redemption_surrendered_after_the_first_round_keeps_the_rounds_solvent() {
     let f = setup();
     let other = f.holder(200);
     let (investor, epoch) = delisted_with_a_priced_deposit(&f);
-    f.vault.propose_wind_down(&f.admin);
+    f.vault.propose_wind_down();
     f.vault.claim_deposit(&investor, &epoch);
     let open = f.vault.current_epoch();
     f.advance(MIN_WIND_DOWN_DELAY);
@@ -243,8 +243,8 @@ fn a_delisted_holders_unpriced_redemption_joins_the_wind_down_when_cancelled() {
     let investor = f.holder(200);
     let other = f.holder(200);
     let epoch = f.vault.request_redeem(&investor, &200);
-    f.identity.allow(&investor, &false, &f.compliance);
-    f.vault.propose_wind_down(&f.admin);
+    f.identity.allow(&investor, &false);
+    f.vault.propose_wind_down();
     f.advance(MIN_WIND_DOWN_DELAY);
     f.vault.activate_wind_down();
 
@@ -261,16 +261,16 @@ fn a_conversion_waits_while_the_vault_is_delisted() {
     let investor = f.investor(1_000);
     let epoch = f.vault.request_deposit(&investor, &1_000);
     f.fulfill_epoch(wad(2));
-    f.identity.allow(&investor, &false, &f.compliance);
-    f.identity.allow(&f.vault.address, &false, &f.compliance);
+    f.identity.allow(&investor, &false);
+    f.identity.allow(&f.vault.address, &false);
 
     assert_eq!(
         f.vault.try_claim_deposit(&investor, &epoch),
         Err(Ok(soroban_sdk::Error::from_contract_error(304)))
     );
     assert!(f.vault.get_deposit_request(&epoch, &investor).is_some());
 
-    f.identity.allow(&f.vault.address, &true, &f.compliance);
+    f.identity.allow(&f.vault.address, &true);
     assert_eq!(f.vault.claim_deposit(&investor, &epoch), 0);
     let open = f.vault.current_epoch();
     assert_eq!(
@@ -309,7 +309,7 @@ proptest! {
         for (_, i) in claims {
             let who = &investors[i];
             if delisted[i] {
-                f.identity.allow(who, &false, &f.compliance);
+                f.identity.allow(who, &false);
             }
             let queued = || f.vault.get_redeem_request(&open, who).map_or(0, |r| r.shares);
             let before = queued();