refactor: name every precondition as a guard

contracts/async-vault/src/deposit.rs

@@ -1,57 +1,26 @@
 use soroban_sdk::{panic_with_error, token::TokenClient, Address, Env};
 use stellar_tokens::rwa::RWATokenClient;
 
-use crate::epoch;
 use crate::error::VaultError;
 use crate::event::{DepositCancelled, DepositClaimed, DepositConvertedToExit, DepositRequested};
-use crate::governance;
+use crate::guards;
 use crate::keys::DataKey;
 use crate::lock_policy::{self, Decision, Exit, Standing};
 use crate::pricing::{Pricing, PricingScheme};
 use crate::redeem;
 use crate::settlement;
-use crate::state::{self, DepositRequest, EpochStatus};
-use crate::treasury;
+use crate::state::{self, DepositRequest};
 use crate::wind_down;
 
-fn refuse_unless_allowed_in(e: &Env, holder: &Address) {
-    let standing = lock_policy::standing(e, holder);
-    if lock_policy::decide(Exit::RequestDeposit, standing, wind_down::is_active(e))
-        != Decision::Proceed
-    {
-        panic_with_error!(e, lock_policy::refused_with(standing));
-    }
-}
-
-fn refuse_if_exceeds_cap(e: &Env, amount: i128) {
-    let Some(cap) = state::deposit_cap(e) else {
-        return;
-    };
-    let current = treasury::deposited_capital(e);
-    let new_total = current
-        .checked_add(amount)
-        .unwrap_or_else(|| panic_with_error!(e, VaultError::AmountTooLarge));
-    if new_total > cap {
-        panic_with_error!(e, VaultError::DepositCapExceeded);
-    }
-}
-
 pub(crate) fn request(e: &Env, holder: &Address, amount: i128) -> u64 {
-    wind_down::refuse_if_active(e);
-    governance::refuse_if_setup_open(e);
+    guards::not_winding_down(e);
+    guards::setup_finished(e);
     holder.require_auth();
-
-    if amount <= 0 {
-        panic_with_error!(e, VaultError::InvalidAmount);
-    }
-    refuse_unless_allowed_in(e, holder);
-    refuse_if_exceeds_cap(e, amount);
-
+    guards::positive_amount(e, amount);
+    guards::allowed_to(e, Exit::RequestDeposit, holder);
+    guards::within_cap(e, amount);
     let epoch_id = state::current_epoch(e);
-
-    if state::get_deposit_request(e, epoch_id, holder).is_some() {
-        panic_with_error!(e, VaultError::RequestOutstanding);
-    }
+    guards::no_open_deposit(e, epoch_id, holder);
 
     let mut epoch = state::get_epoch(e, epoch_id)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::EpochNotFound));
@@ -71,7 +40,10 @@ pub(crate) fn request(e: &Env, holder: &Address, amount: i128) -> u64 {
         },
     );
     state::set_epoch(e, epoch_id, &epoch);
-    state::set_cancellable_escrow(e, state::cancellable_escrow(e) + amount);
+    let escrow = state::cancellable_escrow(e)
+        .checked_add(amount)
+        .unwrap_or_else(|| panic_with_error!(e, VaultError::AmountTooLarge));
+    state::set_cancellable_escrow(e, escrow);
     settlement::on_deposit_requested(e, epoch_id);
 
     let asset = state::get_addr(e, &DataKey::Asset);
@@ -89,40 +61,21 @@ pub(crate) fn request(e: &Env, holder: &Address, amount: i128) -> u64 {
 
 pub(crate) fn claim(e: &Env, holder: &Address, epoch_id: u64) -> i128 {
     holder.require_auth();
-
     let epoch = state::get_epoch(e, epoch_id)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::EpochNotFound));
-
-    if epoch.status != EpochStatus::Fulfilled {
-        panic_with_error!(e, VaultError::EpochNotFulfilled);
-    }
-
+    guards::epoch_fulfilled(e, &epoch);
     let mut request = state::get_deposit_request(e, epoch_id, holder)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::RequestNotFound));
-
-    if request.claimed {
-        panic_with_error!(e, VaultError::AlreadyClaimed);
-    }
-
+    guards::not_claimed(e, request.claimed);
     let standing = lock_policy::standing(e, holder);
     let decision = lock_policy::decide(Exit::ClaimDeposit, standing, wind_down::is_active(e));
-    match decision {
-        Decision::Proceed | Decision::ConvertToExit | Decision::Surrender => {}
-        Decision::Refuse | Decision::Hold => {
-            panic_with_error!(e, lock_policy::refused_with(standing))
-        }
-    }
+    guards::not_refused(e, decision, standing);
 
     let shares = Pricing::deposit_shares(e, request.amount, epoch.share_price)
         .unwrap_or_else(|err| panic_with_error!(e, err));
 
     if shares == 0 {
-        // Pricing already released this epoch's escrow into the reserve, so a
-        // refund now is an extra payout rather than part of committed. It comes
-        // out of free reserve and must not reach money owed to a priced exit.
-        if treasury::free_reserve(e) < request.amount {
-            panic_with_error!(e, VaultError::ClaimNotCovered);
-        }
+        guards::refund_covered(e, request.amount);
         refund_deposit(e, holder, epoch_id, request.amount);
         settlement::on_deposit_claimed(e, epoch_id, 0);
         return 0;
@@ -209,32 +162,13 @@ fn refund_deposit(e: &Env, holder: &Address, epoch_id: u64, amount: i128) {
 
 pub(crate) fn cancel(e: &Env, holder: &Address, epoch_id: u64) -> i128 {
     holder.require_auth();
-
     let epoch = state::get_epoch(e, epoch_id)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::EpochNotFound));
-
-    if epoch.status == EpochStatus::Fulfilled {
-        panic_with_error!(e, VaultError::AlreadyPriced);
-    }
-
-    // Cancellation is an escape hatch, not a choice. Once the feed can price a
-    // sealed epoch, the price is knowable and the only way out is to take it.
-    // Fulfilment is closed during a wind-down, so this epoch will never take a
-    // price. There is nothing to decline.
-    if !wind_down::is_active(e) && epoch::is_priceable(e, &epoch) {
-        panic_with_error!(e, VaultError::PriceAvailable);
-    }
-
+    guards::not_priced(e, &epoch);
+    guards::no_price_available(e, &epoch);
     let request = state::get_deposit_request(e, epoch_id, holder)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::RequestNotFound));
-
-    let standing = lock_policy::standing(e, holder);
-    match lock_policy::decide(Exit::CancelDeposit, standing, wind_down::is_active(e)) {
-        Decision::Proceed => {}
-        Decision::Refuse | Decision::Hold | Decision::ConvertToExit | Decision::Surrender => {
-            panic_with_error!(e, lock_policy::refused_with(standing))
-        }
-    }
+    guards::allowed_to(e, Exit::CancelDeposit, holder);
 
     state::set_cancellable_escrow(
         e,

contracts/async-vault/src/epoch.rs

@@ -4,12 +4,12 @@ use strata_bindings::{OracleFeedClient, OracleState};
 
 use crate::error::VaultError;
 use crate::event::{EpochClosed, EpochFulfilled};
+use crate::guards;
 use crate::keys::DataKey;
 use crate::pricing::{Pricing, PricingScheme};
 use crate::settlement;
 use crate::state::{self, EpochInfo, EpochStatus};
 use crate::timing::{FulfilmentTiming, StandardTiming};
-use crate::wind_down;
 
 /// The schedule this vault fulfils on.
 type Timing = StandardTiming;
@@ -59,14 +59,11 @@ pub(crate) fn is_priceable(e: &Env, epoch: &EpochInfo) -> bool {
 }
 
 pub(crate) fn close(e: &Env) -> u64 {
-    wind_down::refuse_if_active(e);
+    guards::not_winding_down(e);
     let current = state::current_epoch(e);
     let mut epoch = state::get_epoch(e, current)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::EpochNotFound));
-
-    if epoch.status != EpochStatus::Open {
-        panic_with_error!(e, VaultError::EpochNotOpen);
-    }
+    guards::epoch_open(e, &epoch);
 
     let next = current
         .checked_add(1)
@@ -93,13 +90,10 @@ pub(crate) fn close(e: &Env) -> u64 {
 }
 
 pub(crate) fn fulfill(e: &Env, epoch_id: u64) -> i128 {
-    wind_down::refuse_if_active(e);
+    guards::not_winding_down(e);
     let mut epoch = state::get_epoch(e, epoch_id)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::EpochNotFound));
-
-    if let Err(refusal) = fulfilable(e, &epoch) {
-        panic_with_error!(e, refusal);
-    }
+    guards::fulfilable(e, &epoch);
 
     let share_price =
         Pricing::resolve_share_price(e).unwrap_or_else(|err| panic_with_error!(e, err));

contracts/async-vault/src/governance.rs

@@ -21,13 +21,13 @@ use strata_timelock::{queue, setup, Clock, Refusal};
 
 use crate::error::VaultError;
 use crate::event::{ChangeApplied, ChangeCancelled, ChangeProposed, NoticeSet, SetupFinished};
+use crate::guards;
 use crate::keys::DataKey;
-use crate::roles::MANAGER_ROLE;
 use crate::state;
 use crate::treasury;
 use crate::upgrade;
 use crate::wind_down;
-use crate::{AsyncVault, MAX_NOTICE_SECS};
+use crate::AsyncVault;
 
 /// Seven days of five-second ledgers: how long the next admin has to accept a
 /// hand-over.
@@ -74,7 +74,7 @@ pub(crate) struct Wiring {
 }
 
 impl Wiring {
-    fn complete(&self) -> bool {
+    pub(crate) fn complete(&self) -> bool {
         self.token_bound
             && self.oracle_bound
             && self.sole_token_manager
@@ -113,29 +113,6 @@ fn refuse(e: &Env, refusal: Refusal) -> ! {
     }
 }
 
-/// Direct admin calls are instant only before setup finishes.
-pub(crate) fn require_setup_open(e: &Env) {
-    if !setup::is_open(e) {
-        panic_with_error!(e, VaultError::UseTimelock);
-    }
-}
-
-/// Nothing is taken in or queued until every admin change on the kit waits for
-/// the timelock.
-pub(crate) fn refuse_if_setup_open(e: &Env) {
-    if setup::is_open(e) {
-        panic_with_error!(e, VaultError::SetupOpen);
-    }
-}
-
-/// After setup, revoking `manager` waits like a grant; any other revoke stays
-/// instant.
-pub(crate) fn require_revoke_allowed(e: &Env, role: &Symbol) {
-    if *role == MANAGER_ROLE {
-        require_setup_open(e);
-    }
-}
-
 /// True when `contract` reads this vault's clock and its own setup has
 /// finished, so its admin changes wait for this timelock.
 pub(crate) fn bound_here(e: &Env, contract: &Address) -> bool {
@@ -193,78 +170,24 @@ pub(crate) fn wiring(e: &Env) -> Wiring {
     }
 }
 
-/// No nominee from setup may accept once changes wait, so a hand-over offered
-/// during setup holds it open.
 pub(crate) fn finish_setup(e: &Env) {
-    if !setup::is_open(e) {
-        panic_with_error!(e, VaultError::SetupClosed);
-    }
-    if role_transfer::has_active_pending_transfer(e, &AccessControlStorageKey::PendingAdmin) {
-        panic_with_error!(e, VaultError::AdminTransferPending);
-    }
-    if !wiring(e).complete() {
-        panic_with_error!(e, VaultError::SetupIncomplete);
-    }
+    guards::setup_open(e);
+    guards::no_hand_over_pending(e);
+    guards::wiring_complete(e);
     setup::finish::<AsyncVault>(e).unwrap_or_else(|r| refuse(e, r));
     SetupFinished {}.publish(e);
 }
 
-fn check_notice(e: &Env, secs: u64) {
-    if secs > MAX_NOTICE_SECS {
-        panic_with_error!(e, VaultError::NoticeTooLong);
-    }
-    if secs > upgrade::delay(e) {
-        panic_with_error!(e, VaultError::NoticeAboveUpgradeDelay);
-    }
-}
-
 pub(crate) fn set_notice(e: &Env, secs: u64) {
-    check_notice(e, secs);
     state::set_notice(e, secs);
     NoticeSet { secs }.publish(e);
 }
 
-fn changes_code(change: &VaultChange) -> bool {
-    matches!(
-        change,
-        VaultChange::Upgrade(_) | VaultChange::UpgradeDelay(_)
-    )
-}
-
-/// One code change at a time, so a standing eta is never quietly replaced.
-fn code_change_pending(e: &Env) -> bool {
-    queue::open_ids(e).iter().any(|id| {
-        queue::pending::<VaultChange>(e, id).is_some_and(|(change, _)| changes_code(&change))
-    })
-}
-
-/// The bounds a change must meet when it is proposed and again when it
-/// applies.
-fn check(e: &Env, change: &VaultChange) {
-    match change {
-        VaultChange::UpgradeDelay(secs) => upgrade::check_delay(e, *secs),
-        VaultChange::Notice(secs) => check_notice(e, *secs),
-        VaultChange::WindDownDelay(secs) => wind_down::check_delay(e, *secs),
-        VaultChange::Oracle(feed) => {
-            if !bound_here(e, feed) {
-                panic_with_error!(e, VaultError::FeedNotBound);
-            }
-        }
-        VaultChange::Upgrade(_)
-        | VaultChange::Custodian(_)
-        | VaultChange::GrantRole(..)
-        | VaultChange::RevokeRole(..)
-        | VaultChange::Admin(_) => {}
-    }
-}
-
 pub(crate) fn propose(e: &Env, change: VaultChange) -> u32 {
-    wind_down::refuse_if_active(e);
-    refuse_if_setup_open(e);
-    check(e, &change);
-    if changes_code(&change) && code_change_pending(e) {
-        panic_with_error!(e, VaultError::UpgradeProposalExists);
-    }
+    guards::not_winding_down(e);
+    guards::setup_finished(e);
+    guards::change_in_bounds(e, &change);
+    guards::not_a_second_code_change(e, &change);
     let id = queue::propose::<AsyncVault, _>(e, &change).unwrap_or_else(|r| refuse(e, r));
     ChangeProposed {
         id,
@@ -284,7 +207,7 @@ pub(crate) fn apply(e: &Env, id: u32) {
     let change: VaultChange =
         queue::take_ready::<AsyncVault, _>(e, id).unwrap_or_else(|r| refuse(e, r));
     // The notice or the delay may have moved since the proposal; this check decides.
-    check(e, &change);
+    guards::change_in_bounds(e, &change);
     let admin = access_control::get_admin(e)
         .unwrap_or_else(|| panic_with_error!(e, VaultError::AdminRequired));
     match &change {