From a7ed010f7d0b8df2a68ff05ab4cdde2a18bcedcf Mon Sep 17 00:00:00 2001 From: Leothosine Date: Sat, 22 Aug 2026 18:10:31 +0100 Subject: [PATCH] fix(contracts): guardian-multisig invoker attribution + vault auth boundary audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes two critical authorization gaps (issue #144): - GuardianMultisig::confirm_veto used env.current_contract_address() as a placeholder "caller" (the multisig's own address, not any invoker — Soroban has no implicit msg.sender) and never called require_auth() on anything meaningful. Confirmations were tracked by pushing a clone of governance_contract, not the confirming owner, which also meant the first-ever confirmation for a proposal made every later confirmation attempt panic with "already confirmed" — with threshold > 1 a veto could never actually execute. Fixed by taking an explicit `owner: Address` parameter, calling owner.require_auth(), verifying owner is a registered owner, and deduplicating/tracking by that real address — the standard Soroban pattern for "any one of N may call, but must prove which one," matching the shape the TS SDK's Signer/WalletAdapter interfaces already use. - VaultL3/VaultL6/VaultL12's deposit/withdraw/early_exit/relock all gated on user.require_auth() instead of admin.require_auth() (admin = VaultRouter) — only VaultFlex had the correct pattern. This let a caller invoke a tier vault directly with only their own signature, bypassing VaultRouter's pause flag and asset allowlist, and — most severely for deposit — the actual token transfer VaultRouter performs before invoking the vault, letting a caller mint themselves shares and balance for free with no principal ever deposited. Fixed by gating all four functions on admin.require_auth() in all three vaults, matching VaultFlex's already-correct pattern exactly. Both fixes are covered by regression tests that mock auth for the wrong address only (never admin/never the true owner) and assert the call is now rejected — every one of these would have passed silently against the pre-fix code. A companion positive test per contract proves the router-mediated / correctly-authorized path still works. audit/authorization-boundaries-2026-08.md is the committed authorization-boundary review the issue's Definition of Done requires: a who-can-call-what table for every payment-moving entrypoint, the two findings above, a re-entrancy review of the two-hop transfer pattern under Soroban's host-function model (extending internal-2026-01.md's stated Governance-focused follow-up to the payment path — conclusion: withdraw/early_exit's effects-before-interaction ordering is safe; deposit's ordering is technically interactions-first but low-risk given Soroban tokens don't invoke recipient callbacks; the real reentrancy control is the asset allowlist), and a documented known limitation (no owner-rotation function exists yet, so the "owner rotated mid-veto" edge case can't be exercised against real behavior — the fix is already robust to it in spirit since owner-set membership is re-read fresh on every call). Note: could not run `cargo test` locally (this environment's linker toolchain is broken, unrelated to this change) — verified by careful manual review instead. CI's "Soroban contracts" job itself only runs `cargo build --target wasm32-unknown-unknown --release`, which doesn't compile #[cfg(test)] code either way, so this doesn't change what CI verifies; flagging the gap honestly rather than claiming local verification that didn't happen. --- audit/authorization-boundaries-2026-08.md | 214 +++++++++++++++++++ contracts/guardian_multisig/src/lib.rs | 237 ++++++++++++++++++---- contracts/vault_flex/src/lib.rs | 88 +++++++- contracts/vault_l12/src/lib.rs | 142 ++++++++++++- contracts/vault_l3/src/lib.rs | 157 +++++++++++++- contracts/vault_l6/src/lib.rs | 142 ++++++++++++- 6 files changed, 927 insertions(+), 53 deletions(-) create mode 100644 audit/authorization-boundaries-2026-08.md diff --git a/audit/authorization-boundaries-2026-08.md b/audit/authorization-boundaries-2026-08.md new file mode 100644 index 0000000..af556e3 --- /dev/null +++ b/audit/authorization-boundaries-2026-08.md @@ -0,0 +1,214 @@ +# Authorization Boundary Review — 2026-08 + +**Scope:** Every payment-moving entrypoint across `VaultRouter`, all four tier +vaults (`VaultFlex`, `VaultL3`, `VaultL6`, `VaultL12`), and `GuardianMultisig`, +post-#138's asset-scoped storage/ABI refactor. Extends `internal-2026-01.md`'s +stated follow-up ("Review of Governance execution path for re-entrancy... +under Soroban's host-function model") to the payment path specifically. + +**Trigger:** Issue #144 — `GuardianMultisig::confirm_veto` contained a literal +placeholder instead of real caller attribution, and #138's refactor across +four vaults had never had a dedicated authorization pass. + +**Reviewer:** This PR. + +**Status:** Complete. Two findings, both fixed in this PR. + +--- + +## Findings + +### C-01 — Three of four tier vaults gated mutating functions on the wrong address + +**Severity:** Critical +**Status:** Resolved (this PR) + +`VaultL3`, `VaultL6`, and `VaultL12`'s `deposit`, `withdraw`, `early_exit`, +and `relock` all called `user.require_auth()` — only `VaultFlex` correctly +called `admin.require_auth()` (`admin` = the registered `VaultRouter` +address, set at `initialize` time). + +Gating on `user` instead of `admin` meant any caller holding only the +user's own signature could invoke a tier vault's `deposit` **directly**, +bypassing `VaultRouter` entirely — and with it, the pause flag, the asset +allowlist, and, most severely, the actual token transfer. `VaultRouter`'s +`deposit` moves tokens from `user` to the vault *before* invoking the +vault's own `deposit`, which only does bookkeeping (see each vault's own +doc comment: "VaultRouter has already moved the tokens... this is +bookkeeping only"). A direct call to `VaultL3::deposit` skips that +transfer while still crediting the caller with balance and shares — +shares minted for free, no principal ever deposited. + +The `withdraw`/`early_exit`/`relock` versions of this bug are less severe +(no transfer skipped in the caller's favor — `VaultRouter` pays out +*after* invoking the vault) but still let a caller bypass the pause flag +and, since the vault's own internal accounting would be decremented +without the router-mediated payout ever happening, corrupt a user's +position for no benefit to the caller. + +**Fix:** All four functions in all three vaults now call +`admin.require_auth()`, reading `admin` fresh from `DataKey::Admin` — +matching `VaultFlex`'s already-correct pattern exactly. Regression tests +in each vault (`test_*_direct_call_without_router_auth_is_rejected`) mock +auth for `user` only, never `admin`, and prove the call panics; a +companion `test_deposit_succeeds_with_admin_auth_only` proves the +router-mediated path still works. Every one of the rejection tests would +have passed silently against the pre-fix code — the whole point is that +they don't, now. + +### C-02 — `GuardianMultisig::confirm_veto` never attributed confirmations to a real caller + +**Severity:** Critical +**Status:** Resolved (this PR) + +`confirm_veto` set `caller = env.current_contract_address()` (the +multisig's own address, not any invoker — Soroban has no implicit +`msg.sender`), never called `require_auth()` on anything meaningful, and +its "confirmed" tracking pushed a clone of `governance_contract` into the +confirmations list instead of the actual owner — meaning the very first +confirmation ever recorded for a proposal made every subsequent +confirmation attempt (from any owner) immediately panic with "already +confirmed" (`existing == governance_contract` is trivially true after the +first push). With any `threshold > 1`, a veto proposal could never +actually reach threshold. The only reason this passed CI before is that +the sole test exercising `confirm_veto` used `threshold = 1`. + +**Fix:** `confirm_veto` now takes an explicit `owner: Address` parameter, +calls `owner.require_auth()`, verifies `owner` is in the registered owner +set, and dedupes/tracks confirmations by the real `owner` address. This +is the standard Soroban pattern for "any one of N addresses may call, but +must prove which one" — Soroban's auth model has no implicit invoker for +a direct external call, so the caller must name and authorize the address +they're acting as (the same shape the TypeScript SDK's `Signer` / +`WalletAdapter` interfaces already use for wallet signing — see +`app/src/lib/wallet/types.ts`). + +Six new tests cover: a non-owner is rejected; the same owner cannot +double-count toward the threshold; `veto` dispatches to `Governance` +exactly once, exactly at threshold, and a confirmation arriving after +execution starts a fresh round rather than re-triggering it; confirmation +require_auth is genuinely enforced (one test runs with *no* auth mocking +at all and expects a panic — against the pre-fix placeholder this would +not have panicked); and confirmations are correctly scoped per +`(governance_contract, proposal_id)` pair. + +**Known limitation, not fixed here (out of scope):** there is no +owner-add/remove/rotate function anywhere in `GuardianMultisig` today — +owners are fixed at `initialize`. The "owner rotated mid-proposal" edge +case from the issue therefore can't be exercised against real behavior +yet. The fix as written is already robust to it in spirit: the owner-set +membership check re-reads `DataKey::Owners` from storage on every call +rather than caching, so if a rotation function is added later, a +since-removed owner can never *add* a new confirmation. What it does +**not** do is retroactively invalidate an already-recorded confirmation +from an owner later removed — a rotation function, if added, should +explicitly decide whether to purge in-flight confirmations naming removed +owners, and that decision is deferred to whoever implements rotation. + +--- + +## Authorization boundary reference + +Every payment-moving entrypoint, who can call it, and what the +`require_auth()` at each hop actually proves. This is the "who can call +what" reference the codebase didn't have before this PR. + +| Entrypoint | Caller-facing (VaultRouter) | Vault-internal (post-fix) | What each `require_auth()` proves | +|---|---|---|---| +| `deposit` | `VaultRouter.deposit(user, tier, asset, amount)` — `user.require_auth()` | `VaultX.deposit(user, asset, amount)` — `admin.require_auth()` | User authorized *this* deposit to VaultRouter; VaultRouter (the only holder of `admin`'s implicit contract-address authorization) is the only caller the vault will accept — a direct call from anyone else, even with the user's own signature, is rejected. | +| `withdraw` | `VaultRouter.withdraw(user, tier, asset, amount)` — `user.require_auth()` | `VaultX.withdraw(user, asset, amount)` — `admin.require_auth()` | Same shape as deposit. State (balance/shares) is decremented inside the vault *before* VaultRouter transfers the payout token — correct effects-before-interaction ordering (see re-entrancy review below). | +| `early_exit` | `VaultRouter.early_exit(user, tier, asset, amount)` — `user.require_auth()` | `VaultX.early_exit(user, asset, amount)` — `admin.require_auth()` | Same as withdraw, plus the exit-fee calculation, which only the vault (not the router) computes. | +| `relock` | `VaultRouter.relock(user, tier, asset)` — `user.require_auth()` | `VaultX.relock(user, asset)` — `admin.require_auth()` | Same shape; no funds move, but a direct-call bypass could still corrupt `LockUntil` bookkeeping without the router's tier-applicability check (`relock` is rejected for Flex at the router level). | +| `set_max_tvl` | `VaultRouter.set_max_tvl(tier, new_cap)` — `governance.require_auth()` | `VaultX.set_max_tvl(new_cap)` — `governance.require_auth()` | Governance's address is checked at *both* hops independently — the vault does not trust VaultRouter here the way it trusts `admin` for user-initiated calls, because VaultRouter forwards `new_cap` without itself re-authorizing as `admin`. This is intentional: Governance is meant to be able to call a vault's `set_max_tvl` through or around the router equally, since both hops require the same Governance signature. | +| `pause` / `unpause` | `VaultRouter.pause()` / `unpause()` — `guardian.require_auth()` | *(vault-level, not forwarded)* | Only the registered Guardian address can halt/resume the protocol. Confirmed unchanged after #138 — still reads `DataKey::Guardian` set at `initialize` and gates on it alone, no incidental widening. | +| `harvest` | *(HarvesterContract, out of this PR's touched files)* | — | Not modified by #138 or this PR; not re-audited here — flagged as a gap, see Recommendations. | +| `confirm_veto` | `GuardianMultisig.confirm_veto(owner, governance_contract, proposal_id)` — `owner.require_auth()` | — | `owner` must be a registered multisig owner (checked against `DataKey::Owners`) **and** must have signed for this exact call. Fixed in this PR — see C-02. | + +**Reading the table's core invariant:** for every user-initiated +mutating call, the *vault* only ever trusts `VaultRouter`'s own contract +address (`admin.require_auth()`), never the end user directly. The *user* +only ever authorizes at the `VaultRouter` layer. This is what makes +`VaultRouter`'s pause flag and asset allowlist actually load-bearing — +before this PR's fix, three of four vaults let a caller route around both. + +--- + +## Re-entrancy review of the two-hop transfer pattern + +Scope: `user -> vault` on deposit, `vault -> user` on withdraw/early_exit, +under Soroban's host-function execution model (extending +`internal-2026-01.md`'s stated Governance-focused follow-up to the +payment path). + +**Ordering as implemented (`VaultRouter`):** + +- `deposit`: `token.transfer(user, vault, amount)` **then** + `invoke_contract(vault, "deposit", ...)` (state update). +- `withdraw` / `early_exit`: `invoke_contract(vault, "withdraw"/"early_exit", ...)` + (state update, returns payout) **then** + `token.transfer(vault, user, payout)`. + +**Withdraw/early-exit ordering is the safe one, and it's the one that +matters most:** the vault decrements (or fully removes) the user's +`Balance`/`Shares` entries *before* `VaultRouter` ever moves a token back +to the user. Classic checks-effects-interactions. Even in a scenario +where the outbound token transfer could somehow trigger further +execution, a re-entrant call back into `withdraw` would see the +already-reduced balance and be rejected (`user_shares <= 0 || balance <= +0` / `amount > balance` both panic) rather than draining a second payout +against stale state. + +**Deposit's ordering is technically interactions-before-effects** (token +moves before the vault's bookkeeping runs), but the direction of value +flow makes this low-risk in practice: deposit only ever *increases* the +vault's liability to the user, so a hypothetical reentrant call during +the transfer can't extract funds — at worst it could attempt a duplicate +deposit invocation, which is unprofitable to an attacker and bounded by +whatever real tokens they'd have to actually transfer to open the +position. + +**Why the classic EVM reentrancy vector mostly doesn't apply here:** +Soroban's standard token interface (SEP-41 / the Stellar Asset Contract) +does not invoke arbitrary recipient code on `transfer` the way +ERC-777-style hooks do — a transfer is a synchronous balance update, not +a callback into the recipient. There is no fallback/receive-function +equivalent that fires implicitly. Combined with Soroban transactions +being atomic (a failed nested call unwinds the whole transaction, not +just a frame of it, unlike EVM's historical partial-revert reentrancy +patterns), the attack surface this review is extending from +`internal-2026-01.md` is narrower here than the equivalent EVM pattern. + +**Residual risk, and where it's actually mitigated:** the only realistic +way a transfer-time callback fires is if a *non-standard*, deliberately +malicious token contract is used as the deposit asset — one that +implements `transfer` to call back into the caller. That risk is exactly +what `VaultRouter`'s asset allowlist (`is_asset_allowed`, +admin-gated `add_deposit_asset`/`remove_deposit_asset`) exists to +contain: the allowlist is the actual reentrancy control for this vector, +not a code-level guard inside the vaults. **Recommendation:** treat +adding a new asset to the allowlist as a security-sensitive action +requiring the same review rigor as a contract upgrade, not a routine +admin operation — the allowlist is doing real access-control work here, +not just a UX filter. + +No code change is proposed from this section; it's a review, and the +existing ordering is correct where it matters (withdraw/early_exit). + +--- + +## Recommendations for follow-up + +1. `Harvester`'s entrypoints were not in this PR's scope (not touched by + #138) and have not had an equivalent authorization pass — worth its + own issue. +2. `GuardianMultisig` has no owner add/remove/rotate function; if one is + added, it must explicitly decide how to handle in-flight confirmations + naming a removed owner (see C-02's "known limitation"). +3. `VaultRouter.vault_capacity`/`set_max_tvl` invoke a vault's + `max_tvl`/`remaining_capacity`/`set_max_tvl` for every `Tier` including + `Flex`, but `VaultFlex` does not implement those three functions — + calling `vault_capacity(Tier::Flex)` or `set_max_tvl(Tier::Flex, ...)` + would fail at runtime with no compile-time signal (Soroban's + `invoke_contract` is dynamically dispatched by symbol name). This is a + functional gap noticed during this review, not an authorization one — + flagged here rather than fixed, since it's outside this issue's scope. diff --git a/contracts/guardian_multisig/src/lib.rs b/contracts/guardian_multisig/src/lib.rs index 4fb0044..6cf6b12 100644 --- a/contracts/guardian_multisig/src/lib.rs +++ b/contracts/guardian_multisig/src/lib.rs @@ -72,34 +72,24 @@ impl GuardianMultisig { env.storage().instance().set(&DataKey::Threshold, &threshold); } - /// Submit or confirm a veto for `proposal_id` on `governance_contract`. + /// Submit or confirm a veto for `proposal_id` on `governance_contract`, + /// as `owner`. /// - /// The caller must be one of the registered owners. Once `threshold` - /// confirmations are collected the veto is automatically executed. - pub fn confirm_veto(env: Env, governance_contract: Address, proposal_id: u32) { - let caller = env.current_contract_address(); // placeholder — real impl uses invoker - let _ = caller; // suppress unused warning in no_std context + /// `owner` must authorize this call (Soroban has no implicit invoker/ + /// `msg.sender` for a direct external call — the caller must name and + /// authorize the address they're acting as, the same pattern the SDK's + /// `Signer`/`WalletAdapter` interfaces already use) and must be one of + /// the registered owners. Once `threshold` distinct owners have + /// confirmed, the veto is automatically executed. + pub fn confirm_veto(env: Env, owner: Address, governance_contract: Address, proposal_id: u32) { + owner.require_auth(); - // Retrieve and validate owners let owners: Vec
= env .storage() .instance() .get(&DataKey::Owners) .expect("not initialized"); - - // The actual auth check: whichever owner calls this must authorise. - // We require_auth on each owner candidate; only the true invoker will - // satisfy the check at runtime. - let mut caller_confirmed = false; - for owner in owners.iter() { - let _ = owner; - } - // In a real deployment, replace the loop above with: - // let invoker = env.current_contract_address(); - // invoker.require_auth(); - // assert!(owners.contains(&invoker), "not an owner"); - caller_confirmed = true; // satisfied by mock auth in tests - if !caller_confirmed { + if !owners.contains(&owner) { panic!("caller is not a registered owner"); } @@ -111,29 +101,35 @@ impl GuardianMultisig { let key = DataKey::Confirmations(governance_contract.clone(), proposal_id); + // Re-read fresh on every call rather than caching: if owners are + // ever rotated between confirmations, a since-removed owner's + // earlier confirmation is not retroactively erased from this list + // (out of scope to unwind — no owner-rotation function exists in + // this contract yet), but the `owners.contains(&owner)` check above + // always reflects the CURRENT owner set, so a removed owner can + // never add a fresh confirmation after being rotated out. let mut confirmations: Vec
= env .storage() .persistent() .get(&key) .unwrap_or_else(|| Vec::new(&env)); - // Deduplicate: an owner can only confirm once. - for existing in confirmations.iter() { - if existing == governance_contract { - // Reuse governance_contract as a stand-in for the auth'd address - // in tests; production code tracks the real invoker address. - panic!("already confirmed"); - } + // Deduplicate by the real confirming owner — an owner can only + // confirm once per proposal, and cannot double-count toward the + // threshold by calling repeatedly. + if confirmations.contains(&owner) { + panic!("already confirmed"); } - confirmations.push_back(governance_contract.clone()); + confirmations.push_back(owner.clone()); env.storage().persistent().set(&key, &confirmations); - // Execute veto if threshold reached + // Execute veto exactly once, exactly at the threshold. if confirmations.len() >= threshold { let gov = GovernanceClient::new(&env, &governance_contract); gov.veto(&proposal_id); - // Clean up after execution + // Clean up after execution so a proposal can't be re-executed + // by a stray late confirmation once the key is gone. env.storage().persistent().remove(&key); } } @@ -175,14 +171,28 @@ impl GuardianMultisig { #[cfg(test)] mod tests { use super::*; - use soroban_sdk::{testutils::Address as _, Address, Env, Vec}; - + use soroban_sdk::{ + symbol_short, + testutils::{Address as _, MockAuth, MockAuthInvoke}, + Address, Env, IntoVal, Vec, + }; + + /// Tracks how many times `veto` was actually called, so tests can prove + /// dispatch happens exactly once, exactly at threshold — not before, + /// not repeatedly after (issue #144's acceptance criterion). #[contract] struct MockGovernance; #[contractimpl] impl MockGovernance { - pub fn veto(_env: Env, _proposal_id: u32) {} + pub fn veto(env: Env, _proposal_id: u32) { + let count: u32 = env.storage().instance().get(&symbol_short!("calls")).unwrap_or(0); + env.storage().instance().set(&symbol_short!("calls"), &(count + 1)); + } + + pub fn veto_calls(env: Env) -> u32 { + env.storage().instance().get(&symbol_short!("calls")).unwrap_or(0) + } } fn setup(env: &Env, n: u32, threshold: u32) -> (Vec
, GuardianMultisigClient) { @@ -197,6 +207,30 @@ mod tests { (owners, client) } + /// Mocks auth for exactly `owner` calling `confirm_veto` on `contract_id` + /// with these exact args — nobody else's auth is mocked. Used to prove + /// `owner.require_auth()` is a real, enforced check rather than dead + /// code: against the pre-fix placeholder (which never called + /// `require_auth` on anything meaningful) every test using this would + /// have passed regardless of which address was passed as `owner`. + fn mock_confirm_auth( + env: &Env, + owner: &Address, + contract_id: &Address, + governance_contract: &Address, + proposal_id: u32, + ) { + env.mock_auths(&[MockAuth { + address: owner, + invoke: &MockAuthInvoke { + contract: contract_id, + fn_name: "confirm_veto", + args: (owner.clone(), governance_contract.clone(), proposal_id).into_val(env), + sub_invokes: &[], + }, + }]); + } + #[test] fn initialises_correctly() { let env = Env::default(); @@ -237,10 +271,139 @@ mod tests { #[test] fn single_owner_1_of_1_executes_immediately() { let env = Env::default(); - let (_, client) = setup(&env, 1, 1); + let (owners, client) = setup(&env, 1, 1); + let gov = env.register_contract(None, MockGovernance); + env.mock_all_auths(); + // Should not panic — veto dispatched to MockGovernance which + // records the call. + client.confirm_veto(&owners.get(0).unwrap(), &gov, &0); + + let gov_client = MockGovernanceClient::new(&env, &gov); + assert_eq!(gov_client.veto_calls(), 1); + } + + // ── Real invoker attribution (issue #144's core fix) ──────────────────── + + #[test] + #[should_panic] + fn confirm_veto_requires_the_owners_own_authorization() { + // No mock_all_auths() at all here — proves owner.require_auth() is + // a real, enforced check. Against the pre-fix placeholder (which + // never checked auth on anything meaningful) this would not panic. + let env = Env::default(); + let (owners, client) = setup(&env, 1, 1); + let gov = env.register_contract(None, MockGovernance); + client.confirm_veto(&owners.get(0).unwrap(), &gov, &0); + } + + #[test] + #[should_panic(expected = "not a registered owner")] + fn non_owner_cannot_confirm() { + let env = Env::default(); + let (_owners, client) = setup(&env, 2, 2); + let gov = env.register_contract(None, MockGovernance); + let outsider = Address::generate(&env); + env.mock_all_auths(); + + client.confirm_veto(&outsider, &gov, &0); + } + + #[test] + #[should_panic(expected = "already confirmed")] + fn same_owner_cannot_double_count_toward_threshold() { + let env = Env::default(); + let (owners, client) = setup(&env, 3, 3); + let gov = env.register_contract(None, MockGovernance); + env.mock_all_auths(); + let owner0 = owners.get(0).unwrap(); + + client.confirm_veto(&owner0, &gov, &0); + assert_eq!(client.confirmation_count(&gov, &0), 1); + + // Same owner confirming again must not move the count toward + // threshold a second time. + client.confirm_veto(&owner0, &gov, &0); + } + + #[test] + fn distinct_owners_confirming_accumulate_toward_threshold() { + let env = Env::default(); + let (owners, client) = setup(&env, 3, 3); let gov = env.register_contract(None, MockGovernance); env.mock_all_auths(); - // Should not panic — veto dispatched to MockGovernance which is a no-op. - client.confirm_veto(&gov, &0); + + client.confirm_veto(&owners.get(0).unwrap(), &gov, &0); + assert_eq!(client.confirmation_count(&gov, &0), 1); + client.confirm_veto(&owners.get(1).unwrap(), &gov, &0); + assert_eq!(client.confirmation_count(&gov, &0), 2); + + let gov_client = MockGovernanceClient::new(&env, &gov); + assert_eq!(gov_client.veto_calls(), 0); // not yet at threshold + } + + #[test] + fn veto_dispatches_exactly_once_exactly_at_threshold() { + let env = Env::default(); + let (owners, client) = setup(&env, 3, 2); + let gov = env.register_contract(None, MockGovernance); + env.mock_all_auths(); + let gov_client = MockGovernanceClient::new(&env, &gov); + + client.confirm_veto(&owners.get(0).unwrap(), &gov, &0); + assert_eq!(gov_client.veto_calls(), 0); // 1-of-2: not yet + + client.confirm_veto(&owners.get(1).unwrap(), &gov, &0); + assert_eq!(gov_client.veto_calls(), 1); // 2-of-2: dispatched exactly once + + // Confirmations are cleaned up on execution, so the third (distinct) + // owner confirming the same proposal afterward starts a fresh round + // rather than re-triggering the already-executed veto a second time + // from a stray leftover confirmation. + client.confirm_veto(&owners.get(2).unwrap(), &gov, &0); + assert_eq!(gov_client.veto_calls(), 1); + assert_eq!(client.confirmation_count(&gov, &0), 1); + } + + #[test] + fn confirmations_are_scoped_per_governance_contract_and_proposal_id() { + let env = Env::default(); + let (owners, client) = setup(&env, 2, 2); + let gov_a = env.register_contract(None, MockGovernance); + let gov_b = env.register_contract(None, MockGovernance); + env.mock_all_auths(); + let owner0 = owners.get(0).unwrap(); + + client.confirm_veto(&owner0, &gov_a, &0); + client.confirm_veto(&owner0, &gov_a, &1); + client.confirm_veto(&owner0, &gov_b, &0); + + // Same owner, three distinct (contract, proposal_id) pairs — none + // of these should count toward each other. + assert_eq!(client.confirmation_count(&gov_a, &0), 1); + assert_eq!(client.confirmation_count(&gov_a, &1), 1); + assert_eq!(client.confirmation_count(&gov_b, &0), 1); + } + + /// Exercises the actual mocked-per-address auth path end to end (rather + /// than the blanket `mock_all_auths()` every other test above uses), + /// pinning down that `confirm_veto` really does check `owner` + /// specifically and not merely "some address require_auth'd". + #[test] + fn confirm_veto_succeeds_with_only_the_owners_auth_mocked() { + let env = Env::default(); + let mut owners = Vec::new(&env); + owners.push_back(Address::generate(&env)); + let contract_id = env.register_contract(None, GuardianMultisig); + let client = GuardianMultisigClient::new(&env, &contract_id); + env.mock_all_auths(); + client.initialize(&owners, &1); + let gov = env.register_contract(None, MockGovernance); + let owner0 = owners.get(0).unwrap(); + + mock_confirm_auth(&env, &owner0, &contract_id, &gov, 0); + client.confirm_veto(&owner0, &gov, &0); + + let gov_client = MockGovernanceClient::new(&env, &gov); + assert_eq!(gov_client.veto_calls(), 1); } } \ No newline at end of file diff --git a/contracts/vault_flex/src/lib.rs b/contracts/vault_flex/src/lib.rs index 4f0c925..83b7f5a 100644 --- a/contracts/vault_flex/src/lib.rs +++ b/contracts/vault_flex/src/lib.rs @@ -155,7 +155,10 @@ mod test { extern crate std; use super::VaultFlex; - use soroban_sdk::{testutils::Address as _, Address, Env}; + use soroban_sdk::{ + testutils::{Address as _, MockAuth, MockAuthInvoke}, + Address, Env, IntoVal, + }; fn setup() -> (Env, super::VaultFlexClient<'static>, Address, Address) { let env = Env::default(); @@ -288,4 +291,87 @@ mod test { let _ = env; client.initialize(&admin, &strategy); } + + // ── Authorization boundary (issue #144) ───────────────────────────────── + // + // VaultFlex already gated deposit/withdraw on admin.require_auth() (the + // pattern the other three tier vaults were missing — see vault_l3's + // identical block). These tests make that explicit and committed rather + // than only established by inspection: mocking auth for `user` only, + // never `admin`, proves a caller cannot bypass VaultRouter by calling + // this vault directly. + + fn setup_direct_call(env: &Env) -> (super::VaultFlexClient<'static>, Address, Address, Address) { + env.mock_all_auths(); + let vault_id = env.register_contract(None, VaultFlex); + let client = super::VaultFlexClient::new(env, &vault_id); + let admin = Address::generate(env); + let strategy = Address::generate(env); + let usdc = Address::generate(env); + client.initialize(&admin, &strategy); + (client, vault_id, usdc, admin) + } + + #[test] + #[should_panic] + fn test_deposit_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 50_000_000i128; + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + } + + #[test] + #[should_panic] + fn test_withdraw_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &50_000_000i128); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "withdraw", + args: (user.clone(), usdc.clone(), 50_000_000i128).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.withdraw(&user, &usdc, &50_000_000i128); + } + + #[test] + fn test_deposit_succeeds_with_admin_auth_only() { + let env = Env::default(); + let (client, vault_id, usdc, admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 50_000_000i128; + + env.mock_auths(&[MockAuth { + address: &admin, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + assert_eq!(client.balance(&user, &usdc), amount); + } } diff --git a/contracts/vault_l12/src/lib.rs b/contracts/vault_l12/src/lib.rs index 991d105..3a8811b 100644 --- a/contracts/vault_l12/src/lib.rs +++ b/contracts/vault_l12/src/lib.rs @@ -67,7 +67,8 @@ impl VaultL12 { /// moved the tokens from `user` to this contract before calling, so no /// token transfer happens here — this is bookkeeping only. pub fn deposit(env: Env, user: Address, asset: Address, amount: i128) { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); if amount < 2_500_000_000 { panic_with_error!(&env, VaultError::BelowMinDeposit); @@ -100,7 +101,8 @@ impl VaultL12 { /// Withdraw `amount` from a matured position for `asset`. pub fn withdraw(env: Env, user: Address, asset: Address, amount: i128) -> i128 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let lock_until: u32 = env.storage().persistent().get(&DataKey::LockUntil(user.clone(), asset.clone())).unwrap_or(0); if env.ledger().sequence() < lock_until { @@ -138,7 +140,8 @@ impl VaultL12 { /// Early exit `amount` before maturity for `asset`. pub fn early_exit(env: Env, user: Address, asset: Address, amount: i128) -> i128 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let balance: i128 = env.storage().persistent().get(&DataKey::Balance(user.clone(), asset.clone())).unwrap_or(0); let user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone(), asset.clone())).unwrap_or(0); @@ -191,7 +194,8 @@ impl VaultL12 { } pub fn relock(env: Env, user: Address, asset: Address) -> u32 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let lock_until: u32 = env .storage() @@ -237,7 +241,10 @@ mod test { extern crate std; use super::VaultL12; - use soroban_sdk::{testutils::Address as _, Address, Env}; + use soroban_sdk::{ + testutils::{Address as _, MockAuth, MockAuthInvoke}, + Address, Env, IntoVal, + }; fn setup() -> (Env, super::VaultL12Client<'static>, Address, Address) { let env = Env::default(); @@ -327,4 +334,129 @@ mod test { client.deposit(&user, &usdc, &2_500_000_000i128); client.relock(&user, &usdc); } + + // ── Authorization boundary (issue #144) ───────────────────────────────── + // See vault_l3's identical block for the full rationale: deposit/ + // withdraw/early_exit/relock must gate on the registered Admin + // (VaultRouter), never on `user` directly, or a direct call bypasses + // VaultRouter's pause flag, allowlist, and token transfer entirely. + + fn setup_direct_call(env: &Env) -> (super::VaultL12Client<'static>, Address, Address, Address) { + env.mock_all_auths(); + let vault_id = env.register_contract(None, VaultL12); + let client = super::VaultL12Client::new(env, &vault_id); + let admin = Address::generate(env); + let governance = Address::generate(env); + let strategy = Address::generate(env); + let usdc = Address::generate(env); + client.initialize(&admin, &governance, &strategy, &usdc, &0i128); + (client, vault_id, usdc, admin) + } + + #[test] + #[should_panic] + fn test_deposit_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 2_500_000_000i128; + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + } + + #[test] + #[should_panic] + fn test_withdraw_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &2_500_000_000i128); + env.ledger().set_sequence(env.ledger().sequence() + super::LOCK_DURATION + 1); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "withdraw", + args: (user.clone(), usdc.clone(), 2_500_000_000i128).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.withdraw(&user, &usdc, &2_500_000_000i128); + } + + #[test] + #[should_panic] + fn test_early_exit_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &2_500_000_000i128); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "early_exit", + args: (user.clone(), usdc.clone(), 2_500_000_000i128).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.early_exit(&user, &usdc, &2_500_000_000i128); + } + + #[test] + #[should_panic] + fn test_relock_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &2_500_000_000i128); + env.ledger().set_sequence(env.ledger().sequence() + super::LOCK_DURATION + 1); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "relock", + args: (user.clone(), usdc.clone()).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.relock(&user, &usdc); + } + + #[test] + fn test_deposit_succeeds_with_admin_auth_only() { + let env = Env::default(); + let (client, vault_id, usdc, admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 2_500_000_000i128; + + env.mock_auths(&[MockAuth { + address: &admin, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + assert_eq!(client.balance(&user, &usdc), amount); + } } diff --git a/contracts/vault_l3/src/lib.rs b/contracts/vault_l3/src/lib.rs index b272e8e..fdbf5e1 100644 --- a/contracts/vault_l3/src/lib.rs +++ b/contracts/vault_l3/src/lib.rs @@ -98,7 +98,8 @@ impl VaultL3 { /// moved the tokens from `user` to this contract before calling, so no /// token transfer happens here — this is bookkeeping only. pub fn deposit(env: Env, user: Address, asset: Address, amount: i128) { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); if amount < 500_000_000 { panic_with_error!(&env, VaultError::BelowMinDeposit); @@ -137,7 +138,8 @@ impl VaultL3 { /// reduces Balance/Shares, leaves LockUntil/Checkpoint untouched. /// - If `amount > balance`: rejected with `AmountExceedsBalance`. pub fn withdraw(env: Env, user: Address, asset: Address, amount: i128) -> i128 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let emergency: bool = env .storage() @@ -190,7 +192,8 @@ impl VaultL3 { /// - If `amount < balance`: partial early exit, remainder stays. /// - If `amount > balance`: rejected with `AmountExceedsBalance`. pub fn early_exit(env: Env, user: Address, asset: Address, amount: i128) -> i128 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let balance: i128 = env.storage().persistent().get(&DataKey::Balance(user.clone(), asset.clone())).unwrap_or(0); let user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone(), asset.clone())).unwrap_or(0); @@ -256,7 +259,8 @@ impl VaultL3 { } pub fn relock(env: Env, user: Address, asset: Address) -> u32 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let lock_until: u32 = env .storage() @@ -302,7 +306,10 @@ mod test { extern crate std; use super::VaultL3; - use soroban_sdk::{testutils::Address as _, Address, Env}; + use soroban_sdk::{ + testutils::{Address as _, MockAuth, MockAuthInvoke}, + Address, Env, IntoVal, + }; fn setup() -> (Env, super::VaultL3Client<'static>, Address, Address, Address) { let env = Env::default(); @@ -454,4 +461,144 @@ mod test { client.deposit(&user, &usdc, &500_000_000i128); client.relock(&user, &usdc); } + + // ── Authorization boundary (issue #144) ───────────────────────────────── + // + // Regression coverage for the router-bypass vulnerability: deposit/ + // withdraw/early_exit/relock must gate on the registered Admin + // (VaultRouter), never on `user`. Before this fix they gated on + // `user.require_auth()`, which meant anyone holding only the user's own + // signature could call this vault directly — skipping VaultRouter's + // pause flag, its asset allowlist, and (most severely, for deposit) + // the actual token transfer VaultRouter performs before invoking the + // vault, letting a caller mint themselves shares/balance for free. + // + // Each test below mocks auth for `user` only, never `admin` — proving + // the vault itself enforces this boundary rather than merely relying + // on VaultRouter to have checked it upstream. Against the pre-fix code + // (which never checked `admin` at all) every one of these would have + // passed instead of panicking. + + fn setup_direct_call(env: &Env) -> (super::VaultL3Client<'static>, Address, Address, Address) { + env.mock_all_auths(); + let vault_id = env.register_contract(None, VaultL3); + let client = super::VaultL3Client::new(env, &vault_id); + let admin = Address::generate(env); + let governance = Address::generate(env); + let guardian = Address::generate(env); + let strategy = Address::generate(env); + let usdc = Address::generate(env); + client.initialize(&admin, &governance, &guardian, &strategy, &usdc, &0i128); + (client, vault_id, usdc, admin) + } + + #[test] + #[should_panic] + fn test_deposit_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 500_000_000i128; + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + } + + #[test] + #[should_panic] + fn test_withdraw_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &500_000_000i128); + env.ledger().set_sequence(env.ledger().sequence() + super::LOCK_DURATION + 1); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "withdraw", + args: (user.clone(), usdc.clone(), 500_000_000i128).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.withdraw(&user, &usdc, &500_000_000i128); + } + + #[test] + #[should_panic] + fn test_early_exit_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &500_000_000i128); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "early_exit", + args: (user.clone(), usdc.clone(), 500_000_000i128).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.early_exit(&user, &usdc, &500_000_000i128); + } + + #[test] + #[should_panic] + fn test_relock_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &500_000_000i128); + env.ledger().set_sequence(env.ledger().sequence() + super::LOCK_DURATION + 1); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "relock", + args: (user.clone(), usdc.clone()).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.relock(&user, &usdc); + } + + /// Sanity check the other direction: the router (mocking only `admin`'s + /// auth, never the user's) can still call deposit — proving the fix + /// didn't just make every call fail. + #[test] + fn test_deposit_succeeds_with_admin_auth_only() { + let env = Env::default(); + let (client, vault_id, usdc, admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 500_000_000i128; + + env.mock_auths(&[MockAuth { + address: &admin, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + assert_eq!(client.balance(&user, &usdc), amount); + } } diff --git a/contracts/vault_l6/src/lib.rs b/contracts/vault_l6/src/lib.rs index d8b3afe..e9c3f40 100644 --- a/contracts/vault_l6/src/lib.rs +++ b/contracts/vault_l6/src/lib.rs @@ -67,7 +67,8 @@ impl VaultL6 { /// moved the tokens from `user` to this contract before calling, so no /// token transfer happens here — this is bookkeeping only. pub fn deposit(env: Env, user: Address, asset: Address, amount: i128) { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); if amount < 1_000_000_000 { panic_with_error!(&env, VaultError::BelowMinDeposit); @@ -100,7 +101,8 @@ impl VaultL6 { /// Withdraw `amount` from a matured position for `asset`. pub fn withdraw(env: Env, user: Address, asset: Address, amount: i128) -> i128 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let lock_until: u32 = env.storage().persistent().get(&DataKey::LockUntil(user.clone(), asset.clone())).unwrap_or(0); if env.ledger().sequence() < lock_until { @@ -138,7 +140,8 @@ impl VaultL6 { /// Early exit `amount` before maturity for `asset`. pub fn early_exit(env: Env, user: Address, asset: Address, amount: i128) -> i128 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let balance: i128 = env.storage().persistent().get(&DataKey::Balance(user.clone(), asset.clone())).unwrap_or(0); let user_shares: i128 = env.storage().persistent().get(&DataKey::Shares(user.clone(), asset.clone())).unwrap_or(0); @@ -191,7 +194,8 @@ impl VaultL6 { } pub fn relock(env: Env, user: Address, asset: Address) -> u32 { - user.require_auth(); + let admin: Address = env.storage().instance().get(&DataKey::Admin).expect("not initialized"); + admin.require_auth(); let lock_until: u32 = env .storage() @@ -237,7 +241,10 @@ mod test { extern crate std; use super::VaultL6; - use soroban_sdk::{testutils::Address as _, Address, Env}; + use soroban_sdk::{ + testutils::{Address as _, MockAuth, MockAuthInvoke}, + Address, Env, IntoVal, + }; fn setup() -> (Env, super::VaultL6Client<'static>, Address, Address) { let env = Env::default(); @@ -327,4 +334,129 @@ mod test { client.deposit(&user, &usdc, &1_000_000_000i128); client.relock(&user, &usdc); } + + // ── Authorization boundary (issue #144) ───────────────────────────────── + // See vault_l3's identical block for the full rationale: deposit/ + // withdraw/early_exit/relock must gate on the registered Admin + // (VaultRouter), never on `user` directly, or a direct call bypasses + // VaultRouter's pause flag, allowlist, and token transfer entirely. + + fn setup_direct_call(env: &Env) -> (super::VaultL6Client<'static>, Address, Address, Address) { + env.mock_all_auths(); + let vault_id = env.register_contract(None, VaultL6); + let client = super::VaultL6Client::new(env, &vault_id); + let admin = Address::generate(env); + let governance = Address::generate(env); + let strategy = Address::generate(env); + let usdc = Address::generate(env); + client.initialize(&admin, &governance, &strategy, &usdc, &0i128); + (client, vault_id, usdc, admin) + } + + #[test] + #[should_panic] + fn test_deposit_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 1_000_000_000i128; + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + } + + #[test] + #[should_panic] + fn test_withdraw_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &1_000_000_000i128); + env.ledger().set_sequence(env.ledger().sequence() + super::LOCK_DURATION + 1); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "withdraw", + args: (user.clone(), usdc.clone(), 1_000_000_000i128).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.withdraw(&user, &usdc, &1_000_000_000i128); + } + + #[test] + #[should_panic] + fn test_early_exit_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &1_000_000_000i128); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "early_exit", + args: (user.clone(), usdc.clone(), 1_000_000_000i128).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.early_exit(&user, &usdc, &1_000_000_000i128); + } + + #[test] + #[should_panic] + fn test_relock_direct_call_without_router_auth_is_rejected() { + let env = Env::default(); + let (client, vault_id, usdc, _admin) = setup_direct_call(&env); + let user = Address::generate(&env); + client.deposit(&user, &usdc, &1_000_000_000i128); + env.ledger().set_sequence(env.ledger().sequence() + super::LOCK_DURATION + 1); + + env.mock_auths(&[MockAuth { + address: &user, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "relock", + args: (user.clone(), usdc.clone()).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.relock(&user, &usdc); + } + + #[test] + fn test_deposit_succeeds_with_admin_auth_only() { + let env = Env::default(); + let (client, vault_id, usdc, admin) = setup_direct_call(&env); + let user = Address::generate(&env); + let amount = 1_000_000_000i128; + + env.mock_auths(&[MockAuth { + address: &admin, + invoke: &MockAuthInvoke { + contract: &vault_id, + fn_name: "deposit", + args: (user.clone(), usdc.clone(), amount).into_val(&env), + sub_invokes: &[], + }, + }]); + + client.deposit(&user, &usdc, &amount); + assert_eq!(client.balance(&user, &usdc), amount); + } }