fix: correct what the group a review found

AGENTS.md

@@ -28,8 +28,10 @@ What the contracts enforce:
    allowlist place. Priced claims are never re-priced and never identity-gated;
    a delisted, non-frozen investor leaves through the exit-only cash path,
    including a deposit that was priced before the delisting.
-3. **Cancellation is atomic and single-step**, open only until the attestation
-   that prices the request is accepted. There is no instant exit.
+3. **Cancellation is atomic and single-step**, open only while the vault cannot
+   price the request: the attestation that prices it closes cancellation, and
+   cancellation reopens if that report expires, the NAV is paused or the vault
+   is paused. There is no instant exit.
 
 Five authorities, each a native Stellar multisig: governance, compliance,
 attestation, treasury, guardian. Testnet only. Not audited.

contracts/async-vault/src/error.rs

@@ -123,9 +123,9 @@ pub enum VaultError {
     UseTimelock = 6065,
     /// Setup has finished, and it never reopens.
     SetupClosed = 6066,
-    /// The token, the oracle, the manager role, the allowlist or the
-    /// compliance binding is not wired to this vault yet, or the token already
-    /// has shares outstanding, so setup cannot finish.
+    /// A wiring check `health()` reports is false (token, oracle and compliance
+    /// bound, sole token manager, allowlisted, not frozen) or the token has
+    /// shares outstanding, so setup cannot finish.
     SetupIncomplete = 6067,
     /// The holder fails the share token's identity check, so the vault takes no
     /// deposit request from them and returns no escrowed shares to them.

contracts/async-vault/src/governance.rs

@@ -60,7 +60,8 @@ pub struct VaultPendingChange {
 const TOKEN_MANAGER_ROLE: Symbol = symbol_short!("manager");
 
 /// What `finish_setup` needs to see before deposits open. `health()` reports
-/// the same checks.
+/// every check but `no_shares_outstanding`: a healthy vault has shares
+/// outstanding from its first claim on.
 pub(crate) struct Wiring {
     pub token_bound: bool,
     pub oracle_bound: bool,

contracts/async-vault/src/health.rs

@@ -6,7 +6,8 @@ use crate::keys::DataKey;
 use crate::state;
 
 /// What a deployment checks before it trusts the vault, and what monitoring
-/// keeps checking after. Every field is true on a healthy vault.
+/// keeps checking after. On a healthy vault `setup_open` is false and every
+/// other field is true.
 #[contracttype]
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Health {

contracts/async-vault/src/lib.rs

@@ -286,10 +286,10 @@ impl AsyncVault {
         governance::set_notice(e, secs);
     }
 
-    /// Ends the window in which admin changes are instant, once the token,
-    /// the oracle, the manager role, the allowlist and the compliance binding
-    /// are wired to this vault and the token has no shares outstanding.
-    /// Deposits open from here on, and it never reopens.
+    /// Ends the window in which admin changes are instant. Needs every wiring
+    /// check `health()` reports (token, oracle and compliance bound, sole token
+    /// 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();
@@ -300,8 +300,9 @@ impl AsyncVault {
         timelock::setup::is_open(e)
     }
 
-    /// The wiring `finish_setup` checks, plus the feed: whether it can price
-    /// now, and whether its reports outlive the notice.
+    /// Whether setup is open, the wiring `finish_setup` checks except the share
+    /// supply, and the feed: whether it can price now, and whether its reports
+    /// outlive the notice.
     pub fn health(e: &Env) -> Health {
         health::health(e)
     }

contracts/async-vault/src/redeem.rs

@@ -208,9 +208,9 @@ fn deliver(e: &Env, controller: &Address, shares: i128) {
     let vault = e.current_contract_address();
     let token = state::get_addr(e, &DataKey::ShareToken);
     let share = RWATokenClient::new(e, &token);
-    let rules = RulesClient::new(e, &share.compliance());
     if lock_policy::standing(e, controller) != Standing::Listed
-        || !rules.can_transfer(&vault, controller, &shares, &token)
+        || !RulesClient::new(e, &share.compliance())
+            .can_transfer(&vault, controller, &shares, &token)
     {
         panic_with_error!(e, VaultError::HolderNotEligible);
     }

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

@@ -4,7 +4,7 @@ use storage::{INSTANCE_EXTEND_AMOUNT, PERSISTENT_EXTEND_AMOUNT};
 use super::*;
 use crate::keys::DataKey;
 
-const TWO_DAYS: u64 = 2 * 24 * 60 * 60;
+const TWO_DAYS: u64 = 2 * DAY_DURATION;
 
 fn persistent_ttl(f: &Fixture, key: &DataKey) -> u32 {
     f.e.as_contract(&f.vault.address, || f.e.storage().persistent().get_ttl(key))
@@ -93,7 +93,7 @@ fn a_pending_request_still_claims_after_most_of_a_month() {
     f.vault.request_deposit(&user, &400);
     let epoch = f.close_epoch();
 
-    f.advance(29 * 24 * 60 * 60);
+    f.advance(29 * DAY_DURATION);
     f.oracle.attest(
         &NavReport {
             nav_per_share: wad(2),

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

@@ -293,16 +293,32 @@ fn only_governance_applies() {
     assert_eq!(f.vault.pending_changes().len(), 1);
 }
 
+/// Whether the host refused a call to `function` because the vault has no
+/// function of that name. The reason is only in the diagnostic events.
+fn has_no_function(e: &Env, function: &str) -> bool {
+    use soroban_sdk::xdr::{ContractEventBody, ScError, ScErrorCode, ScSymbol, ScVal};
+    let missing = ScVal::Error(ScError::Context(ScErrorCode::MissingValue));
+    let name = ScVal::Symbol(ScSymbol(function.try_into().unwrap()));
+    e.host()
+        .get_diagnostic_events()
+        .unwrap()
+        .0
+        .iter()
+        .any(|event| {
+            let ContractEventBody::V0(body) = &event.event.body;
+            body.topics.contains(&missing)
+                && matches!(&body.data, ScVal::Vec(Some(args)) if args.contains(&name))
+        })
+}
+
 #[test]
 fn the_vault_exposes_no_immediate_upgrade() {
     let f = setup();
-    let hash = wasm(&f, 7);
-
-    // If the Upgradeable trait were implemented, this client would work
-    // against the vault and replace the code with no delay at all.
-    let client =
+    let upgrader =
         stellar_contract_utils::upgradeable::UpgradeableClient::new(&f.e, &f.vault.address);
-    assert!(client.try_upgrade(&hash, &f.admin).is_err());
+
+    assert!(upgrader.try_upgrade(&wasm(&f, 7), &f.admin).is_err());
+    assert!(has_no_function(&f.e, "upgrade"));
 }
 
 #[test]

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

@@ -48,7 +48,7 @@ fn a_fresh_vault_is_not_winding_down() {
     assert_eq!(f.vault.wind_down(), None);
 }
 
-const WEEK: u64 = 7 * 24 * 60 * 60;
+const WEEK: u64 = 7 * DAY_DURATION;
 
 #[test]
 fn governance_proposes_and_anyone_activates_after_the_delay() {
@@ -593,7 +593,7 @@ fn a_position_survives_the_ledger_advancing() {
     f.vault.finalize_wind_down_round();
     f.vault.claim_wind_down(&user);
 
-    f.advance(29 * 24 * 60 * 60);
+    f.advance(29 * DAY_DURATION);
 
     f.vault.fund(&backer, &800);
     f.vault.finalize_wind_down_round();

contracts/nav-oracle/src/lib.rs

@@ -39,7 +39,8 @@ pub enum OracleError {
     NavOutOfBand = 3002,
     /// Attested before `cooldown_secs` elapsed since the last record.
     CooldownActive = 3003,
-    /// Per-share move exceeds `max_deviation_bps`.
+    /// The move from the last record exceeds `max_up_bps` (a rise) or
+    /// `max_down_bps` (a fall).
     DeviationExceeded = 3004,
     /// `expires_at` not in the future — the record would be born stale.
     ExpiresInPast = 3005,

contracts/nav-oracle/src/test_governance.rs

@@ -109,6 +109,20 @@ impl Fixture<'_> {
         call();
         assert_eq!(self.instance_ttl(), INSTANCE_EXTEND_AMOUNT);
     }
+
+    /// Signs one call to the oracle as `who` and nothing else, so the call
+    /// carries the caller's signature but not the admin's.
+    fn sign_only_as(&self, who: &Address, fn_name: &str, args: soroban_sdk::Vec<soroban_sdk::Val>) {
+        self.e.mock_auths(&[MockAuth {
+            address: who,
+            invoke: &MockAuthInvoke {
+                contract: &self.oracle.address,
+                fn_name,
+                args,
+                sub_invokes: &[],
+            },
+        }]);
+    }
 }
 
 fn config() -> OracleConfig {
@@ -465,12 +479,25 @@ fn an_address_that_reports_no_clock_cannot_be_bound() {
 fn only_the_admin_binds_and_finishes() {
     let f = setup();
     let stranger = Address::generate(&f.e);
-    f.e.set_auths(&[]);
 
+    f.sign_only_as(
+        &stranger,
+        "bind_vault",
+        (f.vault.address.clone(), stranger.clone()).into_val(&f.e),
+    );
     assert!(f
         .oracle
         .try_bind_vault(&f.vault.address, &stranger)
         .is_err());
+    assert_eq!(f.oracle.vault(), None);
+
+    f.e.mock_all_auths();
+    f.oracle.bind_vault(&f.vault.address, &f.admin);
+    f.sign_only_as(
+        &stranger,
+        "finish_setup",
+        (stranger.clone(),).into_val(&f.e),
+    );
     assert!(f.oracle.try_finish_setup(&stranger).is_err());
     assert!(f.oracle.setup_open());
 }
@@ -820,9 +847,23 @@ fn only_the_admin_proposes_cancels_and_applies() {
     let id = f.propose(change.clone());
     f.advance(MIN_DELAY);
 
-    f.e.set_auths(&[]);
+    f.sign_only_as(
+        &stranger,
+        "propose_change",
+        (change.clone(), stranger.clone()).into_val(&f.e),
+    );
     assert!(f.oracle.try_propose_change(&change, &stranger).is_err());
+    f.sign_only_as(
+        &stranger,
+        "cancel_change",
+        (id, stranger.clone()).into_val(&f.e),
+    );
     assert!(f.oracle.try_cancel_change(&id, &stranger).is_err());
+    f.sign_only_as(
+        &stranger,
+        "apply_change",
+        (id, stranger.clone()).into_val(&f.e),
+    );
     assert!(f.oracle.try_apply_change(&id, &stranger).is_err());
     assert_eq!(f.oracle.pending_changes().len(), 1);
 }

contracts/share-token/src/test_governance.rs

@@ -98,6 +98,20 @@ impl Fixture<'_> {
         call();
         assert_eq!(self.instance_ttl(), INSTANCE_EXTEND_AMOUNT);
     }
+
+    /// Signs one call to the token as `who` and nothing else, so the call
+    /// carries the caller's signature but not the admin's.
+    fn sign_only_as(&self, who: &Address, fn_name: &str, args: soroban_sdk::Vec<soroban_sdk::Val>) {
+        self.e.mock_auths(&[MockAuth {
+            address: who,
+            invoke: &MockAuthInvoke {
+                contract: &self.token.address,
+                fn_name,
+                args,
+                sub_invokes: &[],
+            },
+        }]);
+    }
 }
 
 fn setup<'a>() -> Fixture<'a> {
@@ -197,9 +211,22 @@ fn an_address_that_reports_no_clock_cannot_be_bound() {
 fn only_the_admin_binds_and_finishes() {
     let f = setup();
     let stranger = Address::generate(&f.e);
-    f.e.set_auths(&[]);
 
+    f.sign_only_as(
+        &stranger,
+        "bind_vault",
+        (f.vault.address.clone(), stranger.clone()).into_val(&f.e),
+    );
     assert!(f.token.try_bind_vault(&f.vault.address, &stranger).is_err());
+    assert_eq!(f.token.vault(), None);
+
+    f.e.mock_all_auths();
+    f.token.bind_vault(&f.vault.address, &f.admin);
+    f.sign_only_as(
+        &stranger,
+        "finish_setup",
+        (stranger.clone(),).into_val(&f.e),
+    );
     assert!(f.token.try_finish_setup(&stranger).is_err());
     assert!(f.token.setup_open());
 }
@@ -481,9 +508,23 @@ fn only_the_admin_proposes_cancels_and_applies() {
     let id = f.propose(change.clone());
     f.advance(MIN_DELAY);
 
-    f.e.set_auths(&[]);
+    f.sign_only_as(
+        &stranger,
+        "propose_change",
+        (change.clone(), stranger.clone()).into_val(&f.e),
+    );
     assert!(f.token.try_propose_change(&change, &stranger).is_err());
+    f.sign_only_as(
+        &stranger,
+        "cancel_change",
+        (id, stranger.clone()).into_val(&f.e),
+    );
     assert!(f.token.try_cancel_change(&id, &stranger).is_err());
+    f.sign_only_as(
+        &stranger,
+        "apply_change",
+        (id, stranger.clone()).into_val(&f.e),
+    );
     assert!(f.token.try_apply_change(&id, &stranger).is_err());
     assert_eq!(f.token.pending_changes().len(), 1);
 }