From 785c902e4db9e0957827dd57375e1e28c44905bd Mon Sep 17 00:00:00 2001 From: hpmaxi <358059+hpmaxi@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:05:12 -0300 Subject: [PATCH 1/5] fix: refuse the vault as its own custodian set_custodian and a Custodian change accepted the vault's own address. Capital deployed there never leaves, and the books count it as both held and deployed. Every path that sets the custodian now refuses it with InvalidCustodian: the direct setter during setup and after, and a timelocked change when it is proposed and again when it applies. --- contracts/async-vault/src/error.rs | 3 +++ contracts/async-vault/src/guards.rs | 8 +++++++- contracts/async-vault/src/lib.rs | 3 ++- contracts/async-vault/src/test/governance.rs | 20 ++++++++++++++++++++ contracts/async-vault/src/test/guards.rs | 13 +++++++++++++ contracts/async-vault/src/test/treasury.rs | 12 ++++++++++++ scripts/harness/guards.test.ts | 18 ++++++++++++++++++ 7 files changed, 75 insertions(+), 2 deletions(-) diff --git a/contracts/async-vault/src/error.rs b/contracts/async-vault/src/error.rs index 8c246d06..0e1ca536 100644 --- a/contracts/async-vault/src/error.rs +++ b/contracts/async-vault/src/error.rs @@ -152,4 +152,7 @@ pub enum VaultError { /// once it is accepted, cancelled or expired, so no nominee from setup can /// accept after changes start to wait. AdminTransferPending = 6074, + /// The custodian would be the vault itself. Capital deployed to it would + /// never leave, and the books would count it as both held and deployed. + InvalidCustodian = 6075, } diff --git a/contracts/async-vault/src/guards.rs b/contracts/async-vault/src/guards.rs index 784917d6..e072de91 100644 --- a/contracts/async-vault/src/guards.rs +++ b/contracts/async-vault/src/guards.rs @@ -148,6 +148,12 @@ pub(crate) fn upgrade_delay_in_bounds(e: &Env, secs: u64) { } } +pub(crate) fn custodian_not_the_vault(e: &Env, custodian: &Address) { + if *custodian == e.current_contract_address() { + panic_with_error!(e, VaultError::InvalidCustodian); + } +} + pub(crate) fn feed_bound_here(e: &Env, feed: &Address) { if !governance::bound_here(e, feed) { panic_with_error!(e, VaultError::FeedNotBound); @@ -162,8 +168,8 @@ pub(crate) fn change_in_bounds(e: &Env, change: &VaultChange) { VaultChange::Notice(secs) => notice_in_bounds(e, *secs), VaultChange::WindDownDelay(secs) => wind_down_delay_in_bounds(e, *secs), VaultChange::Oracle(feed) => feed_bound_here(e, feed), + VaultChange::Custodian(custodian) => custodian_not_the_vault(e, custodian), VaultChange::Upgrade(_) - | VaultChange::Custodian(_) | VaultChange::GrantRole(..) | VaultChange::RevokeRole(..) | VaultChange::Admin(_) => {} diff --git a/contracts/async-vault/src/lib.rs b/contracts/async-vault/src/lib.rs index 11dc124a..2e269555 100644 --- a/contracts/async-vault/src/lib.rs +++ b/contracts/async-vault/src/lib.rs @@ -190,9 +190,10 @@ impl AsyncVault { } /// Instant during setup; afterwards the custodian changes through the - /// timelock. + /// timelock. Never the vault itself (`InvalidCustodian`). #[only_admin] pub fn set_custodian(e: &Env, custodian: Address) { + guards::custodian_not_the_vault(e, &custodian); guards::direct_change_allowed(e); treasury::set_custodian(e, &custodian); } diff --git a/contracts/async-vault/src/test/governance.rs b/contracts/async-vault/src/test/governance.rs index f90bd1d0..9b23d0dd 100644 --- a/contracts/async-vault/src/test/governance.rs +++ b/contracts/async-vault/src/test/governance.rs @@ -48,6 +48,26 @@ fn applying_a_custodian_change_publishes_both_events() { ); } +#[test] +fn a_custodian_change_to_the_vault_is_refused_when_proposed_and_when_applied() { + let f = setup(); + let change = VaultChange::Custodian(f.vault.address.clone()); + + refused( + f.vault.try_propose_change(&change), + VaultError::InvalidCustodian, + ); + + // Skips the proposal's check, as a change queued by an earlier version would. + let id = f.e.as_contract(&f.vault.address, || { + strata_timelock::queue::propose::(&f.e, &change).unwrap() + }); + f.advance(MIN_UPGRADE_DELAY); + + refused(f.vault.try_apply_change(&id), VaultError::InvalidCustodian); + assert_eq!(f.vault.custodian(), Some(f.custodian.clone())); +} + #[test] fn raising_the_notice_is_instant_and_lowering_waits() { let f = setup(); diff --git a/contracts/async-vault/src/test/guards.rs b/contracts/async-vault/src/test/guards.rs index 6cd429b4..40220177 100644 --- a/contracts/async-vault/src/test/guards.rs +++ b/contracts/async-vault/src/test/guards.rs @@ -271,6 +271,15 @@ fn upgrade_delay_in_bounds_refuses_a_delay_outside_the_bounds_or_below_the_notic }); } +#[test] +fn custodian_not_the_vault_refuses_the_vault_itself() { + let f = setup(); + holds(&f, || guards::custodian_not_the_vault(&f.e, &f.custodian)); + refuses(&f, VaultError::InvalidCustodian, || { + guards::custodian_not_the_vault(&f.e, &f.vault.address) + }); +} + #[test] fn feed_bound_here_refuses_a_feed_not_bound_to_this_vault() { let f = setup(); @@ -325,6 +334,10 @@ fn change_in_bounds_checks_the_bounds_of_each_change() { VaultChange::Oracle(stranger.clone()), VaultError::FeedNotBound, ), + ( + VaultChange::Custodian(f.vault.address.clone()), + VaultError::InvalidCustodian, + ), ] { refuses(&f, error, || guards::change_in_bounds(&f.e, &change)); } diff --git a/contracts/async-vault/src/test/treasury.rs b/contracts/async-vault/src/test/treasury.rs index c81af58a..dea1410f 100644 --- a/contracts/async-vault/src/test/treasury.rs +++ b/contracts/async-vault/src/test/treasury.rs @@ -127,6 +127,18 @@ fn setting_the_custodian_needs_the_admin_signature() { assert_eq!(f.vault.custodian(), None); } +#[test] +fn setting_the_vault_as_its_own_custodian_is_refused() { + for f in [setup_unfinished(), setup()] { + let before = f.vault.custodian(); + refused( + f.vault.try_set_custodian(&f.vault.address), + VaultError::InvalidCustodian, + ); + assert_eq!(f.vault.custodian(), before); + } +} + #[test] fn deploying_a_non_positive_amount_is_rejected() { let f = setup(); diff --git a/scripts/harness/guards.test.ts b/scripts/harness/guards.test.ts index 3cc0639f..f5d283da 100644 --- a/scripts/harness/guards.test.ts +++ b/scripts/harness/guards.test.ts @@ -332,6 +332,24 @@ describe("the vault after setup", () => { expect((await reader.custodian()).result).toBe(custodian.publicKey()) }) + it("governance cannot make the vault its own custodian", async () => { + const v = vault(c, governance) + const vaultId = c.deployment.contracts.async_vault + expectRefused( + await v.set_custodian({ custodian: vaultId }), + v, + "InvalidCustodian", + ) + expectRefused( + await v.propose_change({ + change: { tag: "Custodian", values: [vaultId] }, + }), + v, + "InvalidCustodian", + ) + expect((await reader.custodian()).result).toBe(custodian.publicKey()) + }) + it("queues a custodian change behind its delay", async () => { const v = vault(c, governance) const id = await send( From 2bb65cc8e48d1667b2380870967db00582ffc4a9 Mon Sep 17 00:00:00 2001 From: hpmaxi <358059+hpmaxi@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:07:49 -0300 Subject: [PATCH 2/5] feat: check the whole wiring before setup closes finish_setup checked the bindings, the token's manager role, the share supply, the allowlist and the freeze, but not the rest of the wiring. It now also needs the asset to have the share token's decimals, four different addresses for the asset, the token, the oracle and the vault, an oracle that answers state and freshness_duration, and a custodian that is not the vault. Each check calls out with try_ and counts an error as false, and health() reports each one as a new field. The harness deploy stops if any of them is false. --- contracts/async-vault/src/error.rs | 3 +- contracts/async-vault/src/governance.rs | 36 +++++++++++++++--- contracts/async-vault/src/health.rs | 12 ++++++ contracts/async-vault/src/lib.rs | 3 +- contracts/async-vault/src/test/health.rs | 44 +++++++++++++++++++++- contracts/async-vault/src/test/mod.rs | 44 +++++++++++++++++++--- contracts/async-vault/src/test/setup.rs | 6 +++ contracts/async-vault/src/test/treasury.rs | 4 +- scripts/harness/deploy.ts | 7 +++- 9 files changed, 143 insertions(+), 16 deletions(-) diff --git a/contracts/async-vault/src/error.rs b/contracts/async-vault/src/error.rs index 0e1ca536..5394f89e 100644 --- a/contracts/async-vault/src/error.rs +++ b/contracts/async-vault/src/error.rs @@ -126,7 +126,8 @@ pub enum VaultError { /// Setup has finished, and it never reopens. SetupClosed = 6066, /// A wiring check `health()` reports is false (token, oracle and compliance - /// bound, sole token manager, allowlisted, not frozen) or the token has + /// bound, sole token manager, allowlisted, not frozen, matching decimals, + /// distinct addresses, a feed that answers, a custodian) 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 diff --git a/contracts/async-vault/src/governance.rs b/contracts/async-vault/src/governance.rs index 45c44728..f5b2a0ca 100644 --- a/contracts/async-vault/src/governance.rs +++ b/contracts/async-vault/src/governance.rs @@ -1,5 +1,6 @@ use soroban_sdk::{ - contracttype, panic_with_error, symbol_short, Address, BytesN, Env, Symbol, Vec, + contracttype, panic_with_error, symbol_short, token::TokenClient, Address, BytesN, Env, Symbol, + Vec, }; use stellar_access::{ access_control::{self, AccessControlClient, AccessControlStorageKey}, @@ -71,6 +72,11 @@ pub(crate) struct Wiring { pub allowlisted: bool, pub vault_unfrozen: bool, pub compliance_bound: bool, + pub decimals_match: bool, + pub addresses_distinct: bool, + pub custodian_set: bool, + /// A new feed has no report yet, so this asks only that the feed answers. + pub feed_answers: bool, } impl Wiring { @@ -82,6 +88,10 @@ impl Wiring { && self.allowlisted && self.vault_unfrozen && self.compliance_bound + && self.decimals_match + && self.addresses_distinct + && self.custodian_set + && self.feed_answers } } @@ -135,12 +145,22 @@ fn compliance_bound(e: &Env, compliance: &Address, token: &Address) -> bool { linked && on_this_clock } +fn all_different(addresses: &[&Address]) -> bool { + addresses + .iter() + .enumerate() + .all(|(i, a)| addresses[i + 1..].iter().all(|b| a != b)) +} + pub(crate) fn wiring(e: &Env) -> Wiring { let me = e.current_contract_address(); + let asset = state::get_addr(e, &DataKey::Asset); let token = state::get_addr(e, &DataKey::ShareToken); let oracle = state::get_addr(e, &DataKey::Oracle); let roles = AccessControlClient::new(e, &token); let rwa = RWATokenClient::new(e, &token); + let shares = FungibleTokenClient::new(e, &token); + let feed = OracleFeedClient::new(e, &oracle); Wiring { token_bound: bound_here(e, &token), oracle_bound: bound_here(e, &oracle), @@ -151,10 +171,7 @@ pub(crate) fn wiring(e: &Env) -> Wiring { roles.try_get_role_member(&TOKEN_MANAGER_ROLE, &0), Ok(Ok(holder)) if holder == me ), - no_shares_outstanding: matches!( - FungibleTokenClient::new(e, &token).try_total_supply(), - Ok(Ok(0)) - ), + no_shares_outstanding: matches!(shares.try_total_supply(), Ok(Ok(0))), allowlisted: match rwa.try_identity_verifier() { Ok(Ok(verifier)) => matches!( IdentityVerifierClient::new(e, &verifier).try_verify_identity(&me), @@ -167,6 +184,15 @@ pub(crate) fn wiring(e: &Env) -> Wiring { Ok(Ok(compliance)) => compliance_bound(e, &compliance, &token), _ => false, }, + decimals_match: matches!( + (TokenClient::new(e, &asset).try_decimals(), shares.try_decimals()), + (Ok(Ok(of_asset)), Ok(Ok(of_shares))) if of_asset == of_shares + ), + addresses_distinct: all_different(&[&asset, &token, &oracle, &me]), + custodian_set: state::get_addr_opt(e, &DataKey::Custodian) + .is_some_and(|custodian| custodian != me), + feed_answers: matches!(feed.try_state(), Ok(Ok(_))) + && matches!(feed.try_freshness_duration(), Ok(Ok(_))), } } diff --git a/contracts/async-vault/src/health.rs b/contracts/async-vault/src/health.rs index 8f7884c7..cef963dc 100644 --- a/contracts/async-vault/src/health.rs +++ b/contracts/async-vault/src/health.rs @@ -20,6 +20,14 @@ pub struct Health { pub allowlisted: bool, pub vault_unfrozen: bool, pub compliance_bound: bool, + pub decimals_match: bool, + /// The asset, the share token, the oracle and the vault are four + /// different addresses. + pub addresses_distinct: bool, + /// A custodian is set, and it is not the vault. + pub custodian_set: bool, + /// The oracle answers `state` and `freshness_duration`, report or not. + pub feed_answers: bool, pub feed_valid: bool, /// A report outlives the notice, so an epoch's cancellation window does /// not reopen on its own while the notice runs. @@ -38,6 +46,10 @@ pub(crate) fn health(e: &Env) -> Health { allowlisted: wiring.allowlisted, vault_unfrozen: wiring.vault_unfrozen, compliance_bound: wiring.compliance_bound, + decimals_match: wiring.decimals_match, + addresses_distinct: wiring.addresses_distinct, + custodian_set: wiring.custodian_set, + feed_answers: wiring.feed_answers, feed_valid: matches!(feed.try_state(), Ok(Ok(OracleState::Valid))), freshness_covers_notice: matches!( feed.try_freshness_duration(), diff --git a/contracts/async-vault/src/lib.rs b/contracts/async-vault/src/lib.rs index 2e269555..1dcb990a 100644 --- a/contracts/async-vault/src/lib.rs +++ b/contracts/async-vault/src/lib.rs @@ -271,7 +271,8 @@ impl AsyncVault { /// 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 + /// manager, allowlisted, not frozen, matching decimals, distinct addresses, + /// a feed that answers, a custodian), 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) { diff --git a/contracts/async-vault/src/test/health.rs b/contracts/async-vault/src/test/health.rs index afcf2589..4ee84791 100644 --- a/contracts/async-vault/src/test/health.rs +++ b/contracts/async-vault/src/test/health.rs @@ -10,6 +10,10 @@ fn all_healthy() -> Health { allowlisted: true, vault_unfrozen: true, compliance_bound: true, + decimals_match: true, + addresses_distinct: true, + custodian_set: true, + feed_answers: true, feed_valid: true, freshness_covers_notice: true, } @@ -103,6 +107,43 @@ fn each_missing_wiring_step_shows_in_its_own_flag() { ..all_healthy() }, ), + ( + Miswire::AssetDecimalsDiffer, + Health { + decimals_match: false, + ..all_healthy() + }, + ), + ( + Miswire::AssetIsShareToken, + Health { + addresses_distinct: false, + ..all_healthy() + }, + ), + ( + Miswire::OracleNotAFeed, + Health { + feed_answers: false, + feed_valid: false, + freshness_covers_notice: false, + ..all_healthy() + }, + ), + ( + Miswire::NoCustodian, + Health { + custodian_set: false, + ..all_healthy() + }, + ), + ( + Miswire::CustodianIsVault, + Health { + custodian_set: false, + ..all_healthy() + }, + ), ]; for (miswire, expected) in cases { let f = miswired(miswire); @@ -113,7 +154,8 @@ fn each_missing_wiring_step_shows_in_its_own_flag() { Health { setup_open: true, ..expected - } + }, + "{miswire:?}" ); } } diff --git a/contracts/async-vault/src/test/mod.rs b/contracts/async-vault/src/test/mod.rs index cb451efa..87b8cfe7 100644 --- a/contracts/async-vault/src/test/mod.rs +++ b/contracts/async-vault/src/test/mod.rs @@ -31,6 +31,7 @@ mod wind_down; extern crate std; +use soroban_sdk::{contract, contractimpl}; pub(crate) use soroban_sdk::{ symbol_short, testutils::{Address as _, Ledger as _}, @@ -275,6 +276,23 @@ enum Miswire { ComplianceUnbound, ComplianceBoundElsewhere, ComplianceSetupOpen, + AssetDecimalsDiffer, + AssetIsShareToken, + /// The compliance contract passed as the oracle: bound here, but no feed. + OracleNotAFeed, + NoCustodian, + CustodianIsVault, +} + +/// An asset with six decimals, where the share token has seven. +#[contract] +struct SixDecimalAsset; + +#[contractimpl] +impl SixDecimalAsset { + pub fn decimals(_e: Env) -> u32 { + 6 + } } /// A vault left in setup, wired except for `miswire`. @@ -306,8 +324,8 @@ fn setup_with<'a>( /// The share token's constructor arguments, in order. type TokenArgs = (String, String, Address, Address, Address, Address); -/// Wires the set the way the deploy does, apart from `miswire`, then sets the -/// custodian and finishes the vault's setup when `finish` is set. +/// Wires the set the way the deploy does, custodian included, apart from +/// `miswire`, then finishes the vault's setup when `finish` is set. fn build<'a>( feed: OracleConfig, register_token: impl FnOnce(&Env, TokenArgs) -> Address, @@ -360,12 +378,21 @@ fn build<'a>( ), ); + let vault_asset = match miswire { + Miswire::AssetDecimalsDiffer => e.register(SixDecimalAsset, ()), + Miswire::AssetIsShareToken => share.address.clone(), + _ => asset.clone(), + }; + let vault_oracle = match miswire { + Miswire::OracleNotAFeed => compliance.address.clone(), + _ => oracle.address.clone(), + }; let contract_id = e.register( AsyncVault, ( - &asset, + &vault_asset, &share.address, - &oracle.address, + &vault_oracle, VaultRoles { governance: admin.clone(), manager: manager.clone(), @@ -439,8 +466,15 @@ fn build<'a>( } } let vault = AsyncVaultClient::new(&e, &contract_id); + match miswire { + Miswire::NoCustodian => {} + // No entrypoint takes the vault as its own custodian, so the test stores it. + Miswire::CustodianIsVault => e.as_contract(&contract_id, || { + crate::state::set_addr(&e, &crate::keys::DataKey::Custodian, &contract_id) + }), + _ => vault.set_custodian(&custodian), + } if finish { - vault.set_custodian(&custodian); vault.finish_setup(); } diff --git a/contracts/async-vault/src/test/setup.rs b/contracts/async-vault/src/test/setup.rs index 2221ab8d..bd798e68 100644 --- a/contracts/async-vault/src/test/setup.rs +++ b/contracts/async-vault/src/test/setup.rs @@ -64,6 +64,11 @@ fn every_wiring_step_is_checked_before_setup_finishes() { Miswire::ComplianceUnbound, Miswire::ComplianceBoundElsewhere, Miswire::ComplianceSetupOpen, + Miswire::AssetDecimalsDiffer, + Miswire::AssetIsShareToken, + Miswire::OracleNotAFeed, + Miswire::NoCustodian, + Miswire::CustodianIsVault, ] { let f = miswired(miswire); @@ -172,6 +177,7 @@ fn a_token_that_cannot_answer_fails_its_checks_instead_of_trapping() { assert!(!health.allowlisted); assert!(!health.vault_unfrozen); assert!(!health.compliance_bound); + assert!(!health.decimals_match); refused(vault.try_finish_setup(), VaultError::SetupIncomplete); } diff --git a/contracts/async-vault/src/test/treasury.rs b/contracts/async-vault/src/test/treasury.rs index dea1410f..d51883a3 100644 --- a/contracts/async-vault/src/test/treasury.rs +++ b/contracts/async-vault/src/test/treasury.rs @@ -101,7 +101,7 @@ fn deploying_needs_the_treasury_role() { #[test] fn deploying_without_a_custodian_is_rejected() { - let f = setup_unfinished(); + let f = miswired(Miswire::NoCustodian); let backer = f.investor(1_000); f.vault.fund(&backer, &1_000); @@ -114,7 +114,7 @@ fn deploying_without_a_custodian_is_rejected() { #[test] fn setting_the_custodian_needs_the_admin_signature() { - let f = setup_unfinished(); + let f = miswired(Miswire::NoCustodian); f.e.set_auths(&[]); assert!(f.vault.try_set_custodian(&f.custodian).is_err()); diff --git a/scripts/harness/deploy.ts b/scripts/harness/deploy.ts index 6f3da744..fd5d72d8 100644 --- a/scripts/harness/deploy.ts +++ b/scripts/harness/deploy.ts @@ -223,7 +223,8 @@ async function grantAndVerify( } // The custodian is set while setup is open; afterwards it changes only through -// the timelock. finish_setup refuses unless every wiring step above landed. +// the timelock. finish_setup refuses until the custodian and every wiring step +// above are in place. async function finishVaultSetup( net: Network, accounts: Accounts, @@ -255,6 +256,10 @@ async function finishVaultSetup( "allowlisted", "vault_unfrozen", "compliance_bound", + "decimals_match", + "addresses_distinct", + "custodian_set", + "feed_answers", ] as const const broken = wiring.filter((flag) => !health[flag]) if (health.setup_open || broken.length > 0) From 83470d6d51bf19503ba52e7776a051106bde03a7 Mon Sep 17 00:00:00 2001 From: hpmaxi <358059+hpmaxi@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:08:13 -0300 Subject: [PATCH 3/5] docs: list the new setup checks The kit spec's Health table, invariant, custodian rows, runbook and adopter interfaces, the authorization matrix's deploy steps and the testing guide list the new wiring checks and the refusal of the vault as its own custodian. The README shows a deploy run that prints the new health() fields. --- README.md | 14 +++++++------- docs/AUTHORIZATION_MATRIX.md | 2 +- docs/TESTING.md | 3 ++- docs/kit-spec.md | 29 +++++++++++++++++------------ 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 11c655b8..9481dbbb 100644 --- a/README.md +++ b/README.md @@ -112,12 +112,12 @@ Output of a real run from a fresh clone. Contract IDs differ on every run. deploying to local (http://localhost:8000/rpc) funded 13 accounts -asset contract CCAE3ZUI4B2LSOF7HBS4F35SOHYDG7FOLSQ4DPKAS7YU3JB7IN2U2WOR -compliance CAFIN446IPMTL7E2RMHSSDFS5BRYKU6BJS7L7DBIDHLUDGPWEHVOSE2D -identity_verifier CCNJIIWUXN76GHMGNJW4WIDWYOLBGL6JPUT2J5FU2F6T74VUYNT6M7PW -share_token CCXMRDX67EUA563K3Z7BYJU5HM2PACTYQRHCGI7AXKZZDKEVZNPBYMDT -nav_oracle CDNW5MROHAB2ULG3ODWEVSMSVBTOAHDATES74HYAEZIPQCJB4HUB7KXD -async_vault CC2GSF7NZYFSUXX6UP7FFSIDXCKJV7SLN2HS7SD4XO42WHPXHQW3SJQM +asset contract CCW7OKVOXBBBX3YK6O7F6TZPNIFG32OG4UY7R2K6NSWZUI4R75QCEPVT +compliance CDPCD2BQQKVQBQFAFI4YBLN3TRX34J5NZUQQMUBGHPYNY64LCEM63DTL +identity_verifier CC4SRUIMKOBZSEO2GKGVQMDWPWCBJBBIMYYE7BBCUEZE6VMD3SFQDAMW +share_token CDC7SRTDZIMJSLC5AONUD3FIFYZGMFOHRAS7LLYV2ATJRT7RMTMQO2DG +nav_oracle CCUUBP3RGOIZZL4DUGRL4FBOSQJZKBPNQ7VUWIN6UOOYBJYYKGNT5E52 +async_vault CB2KVM2WHLXDZRHGCYPTGE7PZM6OORXCNVBUMC3L7GY5KX4BA6NMADXE clients generated vault holds share_token manager role share_token bound to the vault, setup finished @@ -126,7 +126,7 @@ share_token bound to compliance compliance bound to the vault, setup finished nav_oracle bound to the vault, setup finished vault setup finished -health {"allowlisted":true,"compliance_bound":true,"feed_valid":false,"freshness_covers_notice":true,"oracle_bound":true,"setup_open":false,"sole_token_manager":true,"token_bound":true,"vault_unfrozen":true} +health {"addresses_distinct":true,"allowlisted":true,"compliance_bound":true,"custodian_set":true,"decimals_match":true,"feed_answers":true,"feed_valid":false,"freshness_covers_notice":true,"oracle_bound":true,"setup_open":false,"sole_token_manager":true,"token_bound":true,"vault_unfrozen":true} wrote deployed.local.json ``` diff --git a/docs/AUTHORIZATION_MATRIX.md b/docs/AUTHORIZATION_MATRIX.md index 877f5bfa..f4abf5c1 100644 --- a/docs/AUTHORIZATION_MATRIX.md +++ b/docs/AUTHORIZATION_MATRIX.md @@ -43,7 +43,7 @@ In `scripts/harness/deploy.ts` and automated deployment scripts, the separation - It then calls `token.bind_vault({ vault })` and `token.finish_setup()`, each signed by governance, and stops if `token.setup_open()` still reads true. From then on the token's admin changes wait for the vault's timelock (§5.2). - It calls `compliance.bind_token({ token, operator: governance })`, then `compliance.bind_vault({ vault })` and `compliance.finish_setup()`, and stops if `compliance.setup_open()` still reads true. From then on the compliance contract's module, token-binding and admin changes wait for the vault's timelock (§5.5). - It calls `oracle.bind_vault({ vault })` and `oracle.finish_setup()`, and stops if `oracle.setup_open()` still reads true. From then on the oracle's admin changes wait for the vault's timelock (§5.3). -- Last, it calls `vault.set_custodian({ custodian })` and `vault.finish_setup()`, and stops if `vault.setup_open()` still reads true. `finish_setup` refuses with `SetupIncomplete` unless the token and the oracle report this vault with their setup closed, the vault is the token's only `manager`, the token has no shares outstanding, the vault passes the identity check, its address is not frozen on the token and the token is bound to its compliance contract. A compliance contract that answers `vault()` and `setup_open()` must report this vault with its setup closed; one that does not answer them is an external module and is accepted. From then on the vault takes deposits and its admin changes wait for its timelock (§5.1). +- Last, it calls `vault.set_custodian({ custodian })` and `vault.finish_setup()`, and stops if `vault.setup_open()` still reads true. `finish_setup` refuses with `SetupIncomplete` unless the token and the oracle report this vault with their setup closed, the vault is the token's only `manager`, the token has no shares outstanding, the vault passes the identity check, its address is not frozen on the token and the token is bound to its compliance contract. It also needs the asset to have the token's decimals, four different addresses for the asset, the token, the oracle and the vault, an oracle that answers as a feed, and a custodian other than the vault. A compliance contract that answers `vault()` and `setup_open()` must report this vault with its setup closed; one that does not answer them is an external module and is accepted. The script then reads `vault.health()` and stops if any wiring field is false. From then on the vault takes deposits and its admin changes wait for its timelock (§5.1). --- diff --git a/docs/TESTING.md b/docs/TESTING.md index 00669be6..68070a7c 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -132,7 +132,8 @@ fixture wires them as the deploy does and finishes the vault's setup; | The vault checks the identity verifier at request; the share token checks it on mint and transfer | [`contracts/async-vault/src/test/compliance.rs`](../contracts/async-vault/src/test/compliance.rs), [`contracts/share-token/src/test.rs`](../contracts/share-token/src/test.rs) | `an_investor_who_was_never_allowlisted_is_refused_before_any_cash_moves`, `a_frozen_or_delisted_holder_cannot_transfer_shares`, `manager_mints_to_allowlisted_account` | | The vault returns escrowed shares as the token's `manager` after checking the receiver itself, including the compliance contract's `can_transfer`; neither its own allowlist place nor the token pause stops a return | [`contracts/async-vault/src/test/share_returns.rs`](../contracts/async-vault/src/test/share_returns.rs) | `a_delisted_vault_still_returns_escrowed_shares`, `a_token_pause_stops_trading_but_not_a_share_return`, `a_return_the_compliance_rules_refuse_stays_escrowed` | | The vault reads the oracle and refuses a stale or paused feed | [`contracts/async-vault/src/test/oracle_pricing.rs`](../contracts/async-vault/src/test/oracle_pricing.rs) | `the_epoch_is_priced_at_the_attestation`, `fulfill_is_rejected_once_the_feed_goes_stale`, `a_paused_feed_is_refused_by_the_vault_not_the_oracle` | -| The vault finishes setup only once the token and the oracle report it with their own setup closed, it is the token's only `manager`, the token has no shares outstanding, it passes the identity check, its address is not frozen and the token is bound to compliance, which runs on this vault's clock with its setup closed unless it is an external module that does not answer `VaultBinding`; before that it takes no deposit | [`contracts/async-vault/src/test/setup.rs`](../contracts/async-vault/src/test/setup.rs) | `every_wiring_step_is_checked_before_setup_finishes`, `a_compliance_contract_from_outside_the_kit_is_accepted`, `a_deposit_before_setup_finishes_is_refused`, `setup_cannot_finish_while_a_hand_over_is_pending` | +| The vault finishes setup only once the token and the oracle report it with their own setup closed, it is the token's only `manager`, the token has no shares outstanding, it passes the identity check, its address is not frozen, the token is bound to compliance (which runs on this vault's clock with its setup closed unless it is an external module that does not answer `VaultBinding`), the asset has the token's decimals, the asset, token, oracle and vault have four different addresses, the oracle answers as a feed, and a custodian other than the vault is set. `health()` reports each check, and before setup finishes the vault takes no deposit | [`contracts/async-vault/src/test/setup.rs`](../contracts/async-vault/src/test/setup.rs), [`contracts/async-vault/src/test/health.rs`](../contracts/async-vault/src/test/health.rs) | `every_wiring_step_is_checked_before_setup_finishes`, `each_missing_wiring_step_shows_in_its_own_flag`, `a_token_that_cannot_answer_fails_its_checks_instead_of_trapping`, `a_compliance_contract_from_outside_the_kit_is_accepted`, `a_deposit_before_setup_finishes_is_refused`, `setup_cannot_finish_while_a_hand_over_is_pending` | +| The vault is never its own custodian: the direct setter, during setup and after, and a `Custodian` change, when proposed and when applied, refuse it with `InvalidCustodian` | [`contracts/async-vault/src/test/treasury.rs`](../contracts/async-vault/src/test/treasury.rs), [`contracts/async-vault/src/test/governance.rs`](../contracts/async-vault/src/test/governance.rs), [`contracts/async-vault/src/test/guards.rs`](../contracts/async-vault/src/test/guards.rs) | `setting_the_vault_as_its_own_custodian_is_refused`, `a_custodian_change_to_the_vault_is_refused_when_proposed_and_when_applied`, `custodian_not_the_vault_refuses_the_vault_itself` | | Upgrading replaces the vault's code and migrates once | [`contracts/async-vault/src/test/upgrade.rs`](../contracts/async-vault/src/test/upgrade.rs) (needs `upgrade_wasm`) | `applying_a_wasm_change_replaces_the_code`, `a_migration_refuses_a_second_run` | | The share token upgrades through the vault's timelock under live balances, an open deposit and an open redemption, and migrates once; called directly, `upgrade` does not exist | [`contracts/async-vault/src/test/upgrade.rs`](../contracts/async-vault/src/test/upgrade.rs) (needs `upgrade_wasm`) | `the_token_upgrades_under_live_balances_and_open_requests`, `the_token_exposes_no_immediate_upgrade` | | The oracle upgrades through the vault's timelock under a live valuation, keeps its config, roles and vault, and migrates once; called directly, `upgrade` does not exist | [`contracts/nav-oracle/src/test_governance.rs`](../contracts/nav-oracle/src/test_governance.rs) (needs `upgrade_wasm`) | `the_oracle_upgrades_under_a_live_valuation`, `the_oracle_exposes_no_immediate_upgrade` | diff --git a/docs/kit-spec.md b/docs/kit-spec.md index b15c9e13..a685abd6 100644 --- a/docs/kit-spec.md +++ b/docs/kit-spec.md @@ -85,7 +85,7 @@ The contracts enforce a set of mathematical and state-machine invariants across - **Testable Property**: - Upgrade delay $\in [7 \text{ days}, 90 \text{ days}]$ and $\text{delay} \ge \text{notice}$, checked when a delay change is proposed and again when it applies. - Wind-down delay $\in [7 \text{ days}, 90 \text{ days}]$ (`WindDownDelayTooShort`, `WindDownDelayTooLong`); 7 days when none was set. - - The vault takes no deposit until its one-time setup closes (`SetupOpen`). `finish_setup` refuses with `SetupIncomplete` until the share token and the oracle report this vault with their own setup closed, the vault is the token's only `manager`, the token has no shares outstanding, the vault passes the token's identity check, the token has not frozen the vault's address, and the token is bound to its compliance contract. A compliance contract that answers `vault()` and `setup_open()` must report this vault with its own setup closed; one that does not answer them is an external module and is accepted. Shares minted before setup closes would redeem against depositors' money. It refuses with `AdminTransferPending` while an admin hand-over offered during setup is pending. Setup never reopens (`SetupClosed`). + - The vault takes no deposit until its one-time setup closes (`SetupOpen`). `finish_setup` refuses with `SetupIncomplete` until the share token and the oracle report this vault with their own setup closed, the vault is the token's only `manager`, the token has no shares outstanding, the vault passes the token's identity check, the token has not frozen the vault's address, and the token is bound to its compliance contract. It also needs the asset to have the token's decimals, four different addresses for the asset, the token, the oracle and the vault, an oracle that answers `state()` and `freshness_duration()`, and a custodian other than the vault. A compliance contract that answers `vault()` and `setup_open()` must report this vault with its own setup closed; one that does not answer them is an external module and is accepted. Shares minted before setup closes would redeem against depositors' money. It refuses with `AdminTransferPending` while an admin hand-over offered during setup is pending. Setup never reopens (`SetupClosed`). - After setup, the vault's custodian, role grants, revoking `manager`, admin hand-overs and lowering the notice or the wind-down delay go through the queue; a direct call is refused with `UseTimelock`. Raising the notice or the wind-down delay, the deposit cap, `pause`/`unpause` and revoking any other role stay instant. A `Notice` or `WindDownDelay` change is checked against its bounds when proposed and again when it applies. - Every change waits in the queue (`propose_change` → `apply_change`). Applying before its eta reverts with `ChangeNotReady`. - A change whose delay is shorter than the notice standing at apply time reverts with `UpgradeDelayBelowNotice`. @@ -157,7 +157,7 @@ The `async_vault` constructor strictly verifies pairwise separation across autho | Parameter | Method | Authority | Constraints & Defaults | |---|---|---|---| | **Standing Notice** | `set_notice(secs: u64)`; after setup, lowering goes through `propose_change(Notice(secs))` → `apply_change(id)` | Governance | Notice wait before pricing. Default: `0`. Must satisfy $\text{secs} \le \text{upgrade\_delay}$ and $\text{secs} \le 30 \text{ days}$ (`MAX_NOTICE_SECS`), checked again when a `Notice` change applies. Raising is instant. | -| **Custodian Address** | `set_custodian(custodian: Address)` during setup; afterwards `propose_change(Custodian(address))` → `apply_change(id)` | Governance | Recipient address for off-chain capital deployment. Required before `deploy_to_custodian`. The deploy sets it before setup closes. | +| **Custodian Address** | `set_custodian(custodian: Address)` during setup; afterwards `propose_change(Custodian(address))` → `apply_change(id)` | Governance | Recipient address for off-chain capital deployment. Required before `finish_setup` and `deploy_to_custodian`. Never the vault itself (`InvalidCustodian`), checked when it is set, proposed and applied. The deploy sets it before setup closes. | | **Deposit Cap** | `set_deposit_cap(cap: Option)` | Governance | Maximum capacity for deposited capital. `None` = unbounded. Refuses negative cap. Instant both ways, also after setup. | | **Wind-Down Delay** | `set_wind_down_delay(secs: u64)`; after setup, lowering goes through `propose_change(WindDownDelay(secs))` → `apply_change(id)` | Governance | Minimum notice before wind-down activation. Default and minimum 7 days (`MIN_WIND_DOWN_DELAY`), maximum 90 days (`MAX_WIND_DOWN_DELAY`). Raising is instant. | | **Upgrade Delay** | `propose_change(UpgradeDelay(secs))` → `apply_change(id)` | Governance | Timelock delay for wasm upgrades. Must be $\ge 7 \text{ days}$, $\le 90 \text{ days}$, and $\ge \text{notice}$. | @@ -166,7 +166,7 @@ The `async_vault` constructor strictly verifies pairwise separation across autho ### Health View -`health()` returns one flag per check. A deployment is ready when `setup_open` is false and every other flag is true. `health()` leaves out two `finish_setup` checks: that the share token has no shares outstanding, which stops being true once the vault has holders, and that no admin hand-over offered during setup is pending (`AdminTransferPending`). If `finish_setup` refuses with `SetupIncomplete` while `token_bound`, `oracle_bound`, `sole_token_manager`, `allowlisted`, `vault_unfrozen` and `compliance_bound` are all true, shares were minted before setup; the token's `total_supply()` shows them. +`health()` returns one flag per check. A deployment is ready when `setup_open` is false and every other flag is true. `health()` leaves out two `finish_setup` checks: that the share token has no shares outstanding, which stops being true once the vault has holders, and that no admin hand-over offered during setup is pending (`AdminTransferPending`). If `finish_setup` refuses with `SetupIncomplete` while every field from `token_bound` to `feed_answers` is true, shares were minted before setup; the token's `total_supply()` shows them. | Field | True when | |---|---| @@ -177,6 +177,10 @@ The `async_vault` constructor strictly verifies pairwise separation across autho | `allowlisted` | This vault passes the share token's identity verifier. Escrowing and returning shares do not need it. Only a delisted holder's conversion does: it mints the shares into the vault's own escrow. While this flag is false, those conversions wait until the vault is allowlisted again, and nothing is lost. | | `vault_unfrozen` | The share token has not frozen this vault's address. The vault's own share moves ignore an address freeze, so freezing the vault stops nothing; `finish_setup` still refuses a frozen vault. | | `compliance_bound` | The share token is in its compliance contract's `linked_tokens`, and a compliance contract that answers `vault()` and `setup_open()` names this vault with its own setup finished. One that does not answer them is an external module and passes; its own governance is a trust assumption. | +| `decimals_match` | The asset's `decimals()` equals the share token's. Pricing converts base units at the attested price, so a mismatch would put every deposit and exit off by a power of ten. | +| `addresses_distinct` | The asset, the share token, the oracle and the vault are four different addresses. | +| `custodian_set` | A custodian is set, and it is not the vault itself. | +| `feed_answers` | The oracle answers `state()` and `freshness_duration()`. A new feed with no report passes. `oracle_bound` cannot tell a feed from another contract bound to this vault, such as the compliance contract. | | `feed_valid` | The oracle reports `Valid`: a report exists, it has not expired and the NAV is not paused. | | `freshness_covers_notice` | The oracle's `freshness_duration` is at least the vault's notice. | @@ -217,7 +221,7 @@ Strata distributes control across five native Stellar multisig authorities and p | **`AsyncVault`** | `pause` | Guardian | `#[only_role(caller, "guardian")]` | N/A | | **`AsyncVault`** | `unpause` | Governance | `caller` must be the admin (`AdminRequired`), then `caller.require_auth()` | N/A | | **`AsyncVault`** | `set_notice` | Governance | `#[only_admin]`; raising is instant, lowering is instant during setup, then `propose_change` | No | -| **`AsyncVault`** | `set_custodian` | Governance | `#[only_admin]`; instant during setup, then `propose_change` | No | +| **`AsyncVault`** | `set_custodian` | Governance | `#[only_admin]`; instant during setup, then `propose_change`; never the vault itself (`InvalidCustodian`) | No | | **`AsyncVault`** | `set_deposit_cap` | Governance | `#[only_admin]`; instant both ways | No | | **`AsyncVault`** | `set_wind_down_delay` | Governance | `#[only_admin]`; raising is instant, lowering is instant during setup, then `propose_change`; never below 7 days | No | | **`AsyncVault`** | `finish_setup` | Governance | `#[only_admin]`; needs the wiring complete and no pending admin hand-over; once | No | @@ -295,8 +299,8 @@ A clean deployment executes in three sequenced phases: - Call `IdentityVerifier::allow(vault_address, true)`, signed by the compliance key, so a delisted holder's priced deposit can be minted into the vault's escrow; `finish_setup` requires it. - Call `Compliance::bind_token(share_token_address, governance)` to link SEP-57 rule verification, then `Compliance::bind_vault(vault_address)` and `Compliance::finish_setup()`. Its module and binding changes are instant until then and wait for the vault's timelock afterwards. - Call `NavOracle::bind_vault(vault_address)`, then `NavOracle::finish_setup()`. The oracle's admin changes are instant until then and wait for the vault's timelock afterwards. - - On the vault, call `set_custodian(custodian)`, `set_notice(secs)` and, if initial capital intake is bounded, `set_deposit_cap(cap)`. All three are instant during setup, so deposits never open without the cap. - - Then call `finish_setup()` on the vault. It refuses with `SetupIncomplete` until every wiring step above is in place, the token has no shares outstanding and the vault's address is not frozen on it, and with `AdminTransferPending` while an admin hand-over offered during setup is pending. The vault takes no deposit before it. + - On the vault, call `set_custodian(custodian)`, `set_notice(secs)` and, if initial capital intake is bounded, `set_deposit_cap(cap)`. All three are instant during setup, so deposits never open without the cap. The custodian is required, and the vault's own address is refused (`InvalidCustodian`). + - Then call `finish_setup()` on the vault. It refuses with `SetupIncomplete` until every wiring step above is in place, the token has no shares outstanding and the vault's address is not frozen on it. It also needs the asset to have the token's decimals, four different addresses for the asset, the token, the oracle and the vault, and an oracle that answers `state()` and `freshness_duration()`. It refuses with `AdminTransferPending` while an admin hand-over offered during setup is pending. `health()` shows which check is false. The vault takes no deposit before it. 3. **Phase 3: Operational Parameterization** - Oracle reporter submits the genesis NAV attestation (`attest`). @@ -376,7 +380,7 @@ implementation against what the vault calls. | Interface | Defined in | Implemented here by | What the vault calls | |---|---|---|---| | `OracleFeed` | `crates/strata-bindings` | `nav-oracle` | `state`, `nav_per_share`, `attested_at`, `freshness_duration` | -| `FungibleToken` | OpenZeppelin `stellar-tokens` 0.7.2 | `share-token` | `transfer`, `balance`, `total_supply` | +| `FungibleToken` | OpenZeppelin `stellar-tokens` 0.7.2 | `share-token` | `transfer`, `balance`, `total_supply`, `decimals` | | `RWAToken` | OpenZeppelin `stellar-tokens` 0.7.2 | `share-token` | `mint`, `burn`, `forced_transfer` | | `IdentityVerifier` | OpenZeppelin `stellar-tokens` 0.7.2 | `identity-verifier` (testnet stub) | `verify_identity`, called by the share token | @@ -396,8 +400,9 @@ The vault's `finish_setup` reads the token and the oracle through closed. It reads the compliance contract the same way, but accepts one that does not answer `VaultBinding`: such an external module keeps its own governance, which is a trust assumption. It also reads the token's -`manager` members (OpenZeppelin `AccessControl`) and its `total_supply` -(`FungibleToken`), runs `verify_identity` on its own address, and reads -`linked_tokens` from the token's compliance contract (OpenZeppelin -`TokenBinder`). A replacement for any of these must answer those calls, or the -vault's setup cannot finish. +`manager` members (OpenZeppelin `AccessControl`), its `total_supply` and +`decimals` (`FungibleToken`) and the asset's `decimals`, runs +`verify_identity` on its own address, reads `linked_tokens` from the token's +compliance contract (OpenZeppelin `TokenBinder`), and calls the oracle's +`state` and `freshness_duration` (`OracleFeed`). A replacement for any of these +must answer those calls, or the vault's setup cannot finish. From 9f7dc20f70783618fd1a2ebfccd097159074e1c9 Mon Sep 17 00:00:00 2001 From: hpmaxi <358059+hpmaxi@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:44:19 -0300 Subject: [PATCH 4/5] fix: refuse an oracle swap to a non-feed An Oracle change checked only that the new address names this vault with its setup closed. Another kit contract bound to the vault, such as the compliance contract, passed at proposal and then trapped at apply on a raw state() call. feed_bound_here now also needs state() and freshness_duration() to answer, so such a swap is refused with FeedNotBound when it is proposed and again when it applies. The setup check and the swap guard share one helper. --- contracts/async-vault/src/error.rs | 6 +++-- contracts/async-vault/src/governance.rs | 18 ++++++------- contracts/async-vault/src/guards.rs | 3 ++- contracts/async-vault/src/test/guards.rs | 15 ++++++++--- contracts/async-vault/src/test/oracle_swap.rs | 25 +++++++++++++++++++ docs/AUTHORIZATION_MATRIX.md | 4 +-- docs/kit-spec.md | 2 +- 7 files changed, 54 insertions(+), 19 deletions(-) diff --git a/contracts/async-vault/src/error.rs b/contracts/async-vault/src/error.rs index 5394f89e..6d8420b4 100644 --- a/contracts/async-vault/src/error.rs +++ b/contracts/async-vault/src/error.rs @@ -142,8 +142,10 @@ pub enum VaultError { /// returned or moved, until compliance lifts the freeze. A partial freeze /// refuses only what would touch frozen shares. Frozen = 6071, - /// The proposed feed does not name this vault as its own, or its setup is - /// still open. Its admin could then change its limits with no delay. + /// The proposed feed does not name this vault as its own, its setup is + /// still open, or it does not answer `state()` and `freshness_duration()`. + /// Its admin could then change its limits with no delay, or no epoch could + /// be priced from it. FeedNotBound = 6072, /// The holder may not receive shares (delisted, frozen or refused by the /// compliance rules), so the escrowed shares stay with the vault and the diff --git a/contracts/async-vault/src/governance.rs b/contracts/async-vault/src/governance.rs index f5b2a0ca..47909e2e 100644 --- a/contracts/async-vault/src/governance.rs +++ b/contracts/async-vault/src/governance.rs @@ -75,7 +75,6 @@ pub(crate) struct Wiring { pub decimals_match: bool, pub addresses_distinct: bool, pub custodian_set: bool, - /// A new feed has no report yet, so this asks only that the feed answers. pub feed_answers: bool, } @@ -131,6 +130,13 @@ pub(crate) fn bound_here(e: &Env, contract: &Address) -> bool { && matches!(binding.try_setup_open(), Ok(Ok(false))) } +/// True when `feed` answers the views the vault prices from. A new feed has no +/// report yet, so this asks only for answers, not for a valid price. +pub(crate) fn answers_as_feed(e: &Env, feed: &Address) -> bool { + let feed = OracleFeedClient::new(e, feed); + matches!(feed.try_state(), Ok(Ok(_))) && matches!(feed.try_freshness_duration(), Ok(Ok(_))) +} + fn compliance_bound(e: &Env, compliance: &Address, token: &Address) -> bool { let linked = matches!( TokenBinderClient::new(e, compliance).try_linked_tokens(), @@ -160,7 +166,6 @@ pub(crate) fn wiring(e: &Env) -> Wiring { let roles = AccessControlClient::new(e, &token); let rwa = RWATokenClient::new(e, &token); let shares = FungibleTokenClient::new(e, &token); - let feed = OracleFeedClient::new(e, &oracle); Wiring { token_bound: bound_here(e, &token), oracle_bound: bound_here(e, &oracle), @@ -191,8 +196,7 @@ pub(crate) fn wiring(e: &Env) -> Wiring { addresses_distinct: all_different(&[&asset, &token, &oracle, &me]), custodian_set: state::get_addr_opt(e, &DataKey::Custodian) .is_some_and(|custodian| custodian != me), - feed_answers: matches!(feed.try_state(), Ok(Ok(_))) - && matches!(feed.try_freshness_duration(), Ok(Ok(_))), + feed_answers: answers_as_feed(e, &oracle), } } @@ -249,11 +253,7 @@ pub(crate) fn apply(e: &Env, id: u32) { access_control::revoke_role_no_auth(e, account, role, &admin) } VaultChange::Admin(next) => offer_admin(e, &admin, next), - VaultChange::Oracle(feed) => { - // A feed that cannot answer would stall every epoch after the swap. - OracleFeedClient::new(e, feed).state(); - state::set_addr(e, &DataKey::Oracle, feed); - } + VaultChange::Oracle(feed) => state::set_addr(e, &DataKey::Oracle, feed), } ChangeApplied { id, change }.publish(e); } diff --git a/contracts/async-vault/src/guards.rs b/contracts/async-vault/src/guards.rs index e072de91..2eab6ec7 100644 --- a/contracts/async-vault/src/guards.rs +++ b/contracts/async-vault/src/guards.rs @@ -154,8 +154,9 @@ pub(crate) fn custodian_not_the_vault(e: &Env, custodian: &Address) { } } +/// A feed that cannot answer would stall every epoch after the swap. pub(crate) fn feed_bound_here(e: &Env, feed: &Address) { - if !governance::bound_here(e, feed) { + if !governance::bound_here(e, feed) || !governance::answers_as_feed(e, feed) { panic_with_error!(e, VaultError::FeedNotBound); } } diff --git a/contracts/async-vault/src/test/guards.rs b/contracts/async-vault/src/test/guards.rs index 40220177..f3b8226d 100644 --- a/contracts/async-vault/src/test/guards.rs +++ b/contracts/async-vault/src/test/guards.rs @@ -281,7 +281,7 @@ fn custodian_not_the_vault_refuses_the_vault_itself() { } #[test] -fn feed_bound_here_refuses_a_feed_not_bound_to_this_vault() { +fn feed_bound_here_refuses_an_unbound_feed_or_a_contract_that_is_not_a_feed() { let f = setup(); holds(&f, || guards::feed_bound_here(&f.e, &f.oracle.address)); @@ -294,9 +294,12 @@ fn feed_bound_here_refuses_a_feed_not_bound_to_this_vault() { open_feed(), ), ); - refuses(&f, VaultError::FeedNotBound, || { - guards::feed_bound_here(&f.e, &unbound) - }); + // The compliance contract is bound to this vault but answers no feed view. + for not_a_feed_here in [unbound, f.share.compliance()] { + refuses(&f, VaultError::FeedNotBound, || { + guards::feed_bound_here(&f.e, ¬_a_feed_here) + }); + } } #[test] @@ -334,6 +337,10 @@ fn change_in_bounds_checks_the_bounds_of_each_change() { VaultChange::Oracle(stranger.clone()), VaultError::FeedNotBound, ), + ( + VaultChange::Oracle(f.share.compliance()), + VaultError::FeedNotBound, + ), ( VaultChange::Custodian(f.vault.address.clone()), VaultError::InvalidCustodian, diff --git a/contracts/async-vault/src/test/oracle_swap.rs b/contracts/async-vault/src/test/oracle_swap.rs index 2748ea0a..365fbdcd 100644 --- a/contracts/async-vault/src/test/oracle_swap.rs +++ b/contracts/async-vault/src/test/oracle_swap.rs @@ -15,6 +15,10 @@ impl FeedStub { pub fn state(_e: Env) -> strata_bindings::OracleState { strata_bindings::OracleState::Valid } + + pub fn freshness_duration(_e: Env) -> u64 { + 3_600 + } } #[contractimpl(contracttrait)] @@ -158,3 +162,24 @@ fn a_feed_that_stops_naming_this_vault_is_never_installed() { refused(f.vault.try_apply_change(&id), VaultError::FeedNotBound); assert_eq!(f.vault.oracle(), f.oracle.address); } + +#[test] +fn a_contract_that_is_not_a_feed_is_refused_when_proposed_and_when_applied() { + let f = setup(); + // Bound to this vault with its setup closed, but it answers no feed view. + let change = VaultChange::Oracle(f.share.compliance()); + + refused( + f.vault.try_propose_change(&change), + VaultError::FeedNotBound, + ); + + // Skips the proposal's check, as a change queued by an earlier version would. + let id = f.e.as_contract(&f.vault.address, || { + strata_timelock::queue::propose::(&f.e, &change).unwrap() + }); + f.advance(MIN_UPGRADE_DELAY); + + refused(f.vault.try_apply_change(&id), VaultError::FeedNotBound); + assert_eq!(f.vault.oracle(), f.oracle.address); +} diff --git a/docs/AUTHORIZATION_MATRIX.md b/docs/AUTHORIZATION_MATRIX.md index f4abf5c1..f5d2f3ba 100644 --- a/docs/AUTHORIZATION_MATRIX.md +++ b/docs/AUTHORIZATION_MATRIX.md @@ -106,8 +106,8 @@ Tests: [`controls.rs`](../contracts/async-vault/src/test/controls.rs), [`epochs. | `propose_change` | `#[only_admin]` | Not winding down, setup finished, one code change at a time, an `UpgradeDelay` in `[MIN, MAX]` and `>= notice`, a `Notice` within its bounds, a `WindDownDelay` in `[MIN_WIND_DOWN_DELAY, MAX_WIND_DOWN_DELAY]` | `test::upgrade::only_governance_proposes_and_cancels`, `test::upgrade::nothing_is_proposed_during_a_wind_down`, `test::setup::nothing_is_proposed_during_setup` | | `cancel_change` | `#[only_admin]` | The change must be pending | `test::upgrade::only_governance_proposes_and_cancels` | | `apply_change` | `#[only_admin]` | Eta passed, not paused, not winding down, delay `>= notice`; an `UpgradeDelay`, `Notice` or `WindDownDelay` change is checked against its bounds again | `test::upgrade::only_governance_applies`, `test::governance::a_change_shorter_than_a_raised_notice_never_lands`, `test::governance::nothing_timelocked_moves_during_a_wind_down_a_manager_revoke_included` | -| `propose_change` (`Oracle(address)`) | `#[only_admin]` | The feed names this vault and its setup is closed | `test::oracle_swap::a_feed_outside_this_vault_is_never_proposed` | -| `apply_change` (`Oracle(address)`) | `#[only_admin]` | Delay run; the feed still names this vault with its setup closed, and answers `state()` | `test::oracle_swap::the_oracle_changes_only_through_the_timelock`, `test::oracle_swap::a_feed_that_stops_naming_this_vault_is_never_installed` | +| `propose_change` (`Oracle(address)`) | `#[only_admin]` | The feed names this vault, its setup is closed, and it answers `state()` and `freshness_duration()` (`FeedNotBound`) | `test::oracle_swap::a_feed_outside_this_vault_is_never_proposed`, `test::oracle_swap::a_contract_that_is_not_a_feed_is_refused_when_proposed_and_when_applied` | +| `apply_change` (`Oracle(address)`) | `#[only_admin]` | Delay run; the feed still names this vault with its setup closed, and still answers `state()` and `freshness_duration()` (`FeedNotBound`) | `test::oracle_swap::the_oracle_changes_only_through_the_timelock`, `test::oracle_swap::a_feed_that_stops_naming_this_vault_is_never_installed`, `test::oracle_swap::a_contract_that_is_not_a_feed_is_refused_when_proposed_and_when_applied` | | `pending_changes` | Open view | None | None (view) | | `renounce_admin` | Refused always (`VaultError::AdminRequired`) | None | `test::controls::governance_cannot_renounce_itself_out_of_the_vault`; `test::controls::governance_hands_over_in_two_steps_instead`; `test::governance::an_applied_admin_change_starts_a_hand_over_the_next_admin_accepts`. Hand over by proposing and applying an `Admin` change (`transfer_admin_role` during setup), then the nominee calls `accept_admin_transfer`. | diff --git a/docs/kit-spec.md b/docs/kit-spec.md index a685abd6..c6a45e72 100644 --- a/docs/kit-spec.md +++ b/docs/kit-spec.md @@ -162,7 +162,7 @@ The `async_vault` constructor strictly verifies pairwise separation across autho | **Wind-Down Delay** | `set_wind_down_delay(secs: u64)`; after setup, lowering goes through `propose_change(WindDownDelay(secs))` → `apply_change(id)` | Governance | Minimum notice before wind-down activation. Default and minimum 7 days (`MIN_WIND_DOWN_DELAY`), maximum 90 days (`MAX_WIND_DOWN_DELAY`). Raising is instant. | | **Upgrade Delay** | `propose_change(UpgradeDelay(secs))` → `apply_change(id)` | Governance | Timelock delay for wasm upgrades. Must be $\ge 7 \text{ days}$, $\le 90 \text{ days}$, and $\ge \text{notice}$. | | **Oracle Configuration** | `set_config(config: OracleConfig)`; after setup, `propose_change(Config(config))` → `apply_change(id)` | Governance | Updates oracle parameters within the constructor's bounds. `set_config` is instant during the oracle's setup. After it, `set_config` is instant only for a shorter `freshness_duration`: later reports go stale sooner, and no value moves. Every other change, tighter limits included, goes through `propose_change(Config(config))` → `apply_change(id)`, and the bounds are checked again when it applies. A tighter fall cap or a longer cooldown can hold the price above a real loss, and a narrower band can stop pricing. Applying a `Config` change sets every field, freshness included; to keep a cut made after the proposal, cancel the queued change and propose again. | -| **Oracle Feed** | `propose_change(Oracle(address))` → `apply_change(id)` | Governance | Waits for the timelock. The new feed must name this vault through `vault()` with its own setup finished, at proposal and again at apply, and must answer `state()`. The next fulfilment prices from it, including an epoch sealed before the swap. | +| **Oracle Feed** | `propose_change(Oracle(address))` → `apply_change(id)` | Governance | Waits for the timelock. The new feed must name this vault through `vault()` with its own setup finished, and must answer `state()` and `freshness_duration()`, at proposal and again at apply (`FeedNotBound`). The next fulfilment prices from it, including an epoch sealed before the swap. | ### Health View From 8138a343b48fd2e860ecac6fd199e7e9f85dbab1 Mon Sep 17 00:00:00 2001 From: hpmaxi <358059+hpmaxi@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:45:11 -0300 Subject: [PATCH 5/5] docs: list the new setup checks in every table The design doc's governance row lists the conditions setup closes on, and the authorization matrix lists each vault entrypoint's guards. Both now include matching decimals, four distinct addresses, an oracle that answers the feed interface and a custodian other than the vault. The matrix rows for set_custodian, propose_change and apply_change name InvalidCustodian and its tests. --- docs/AUTHORIZATION_MATRIX.md | 8 ++++---- docs/strata-product-and-architecture.md | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/AUTHORIZATION_MATRIX.md b/docs/AUTHORIZATION_MATRIX.md index f5d2f3ba..b1174021 100644 --- a/docs/AUTHORIZATION_MATRIX.md +++ b/docs/AUTHORIZATION_MATRIX.md @@ -91,11 +91,11 @@ Tests: [`controls.rs`](../contracts/async-vault/src/test/controls.rs), [`epochs. | `deploy_to_custodian` | `#[only_role(caller, "treasury")]` | `assets <= free_reserve`, not winding down | `test::treasury::deploying_needs_the_treasury_role` | | `pause` | `#[only_role(caller, "guardian")]` | Contract not already paused | `test::controls::only_the_guardian_pauses` | | `unpause` | `caller` must be the stored admin (`AdminRequired`), then `caller.require_auth()` | Contract must be paused | `test::controls::the_guardian_pauses_but_only_governance_unpauses`, `test::controls::a_caller_other_than_the_admin_cannot_unpause_even_when_it_signs` | -| `set_custodian` | `#[only_admin]` | Instant during setup; `UseTimelock` after | `test::governance::the_custodian_changes_through_the_timelock`, `test::treasury::setting_the_custodian_needs_the_admin_signature` | +| `set_custodian` | `#[only_admin]` | Never the vault itself (`InvalidCustodian`, checked first). Instant during setup; `UseTimelock` after | `test::governance::the_custodian_changes_through_the_timelock`, `test::treasury::setting_the_custodian_needs_the_admin_signature`, `test::treasury::setting_the_vault_as_its_own_custodian_is_refused` | | `set_notice` | `#[only_admin]` | `secs <= MAX_NOTICE_SECS`, `secs <= upgrade_delay`. Raising is instant; lowering is `UseTimelock` after setup | `test::governance::raising_the_notice_is_instant_and_lowering_waits`, `test::notice::setting_the_notice_needs_governance_authorisation` | | `set_wind_down_delay` | `#[only_admin]` | `MIN_WIND_DOWN_DELAY <= secs <= MAX_WIND_DOWN_DELAY`. Raising is instant; lowering is `UseTimelock` after setup | `test::governance::raising_the_wind_down_delay_is_instant_and_lowering_waits`, `test::governance::a_wind_down_delay_below_the_minimum_cannot_be_proposed`, `test::wind_down::only_governance_sets_the_delay` | | `set_deposit_cap` | `#[only_admin]` | Not negative. Instant both ways, also after setup | `test::governance::the_deposit_cap_stays_instant_both_ways` | -| `finish_setup` | `#[only_admin]` | Once. The token and the oracle report this vault with their setup closed, the vault is the token's only `manager`, the token's `total_supply` is zero, the vault passes `verify_identity`, the token's `is_frozen` is false for the vault, and the token is in its compliance contract's `linked_tokens`; a compliance contract that answers `vault()` and `setup_open()` reports this vault with its setup closed (one that does not answer them is an external module and is accepted); no admin hand-over offered during setup is pending | `test::setup::every_wiring_step_is_checked_before_setup_finishes`, `test::setup::a_compliance_contract_from_outside_the_kit_is_accepted`, `test::setup::setup_cannot_finish_while_a_hand_over_is_pending`, `test::setup::setup_never_finishes_twice`, `test::setup::only_governance_finishes_setup` | +| `finish_setup` | `#[only_admin]` | Once. The token and the oracle report this vault with their setup closed, the vault is the token's only `manager`, the token's `total_supply` is zero, the vault passes `verify_identity`, the token's `is_frozen` is false for the vault, and the token is in its compliance contract's `linked_tokens`; a compliance contract that answers `vault()` and `setup_open()` reports this vault with its setup closed (one that does not answer them is an external module and is accepted); the asset's `decimals` equal the token's; the asset, the token, the oracle and the vault are four different addresses; the oracle answers `state()` and `freshness_duration()`; a custodian other than the vault is set; no admin hand-over offered during setup is pending | `test::setup::every_wiring_step_is_checked_before_setup_finishes`, `test::setup::a_compliance_contract_from_outside_the_kit_is_accepted`, `test::setup::setup_cannot_finish_while_a_hand_over_is_pending`, `test::setup::setup_never_finishes_twice`, `test::setup::only_governance_finishes_setup` | | `grant_role` / `transfer_admin_role` | Admin | Instant during setup, `UseTimelock` after. A zero `live_until_ledger` cancels instantly | `test::governance::grants_and_manager_revokes_wait_for_the_timelock`, `test::governance::an_admin_hand_over_waits_but_cancelling_one_does_not` | | `accept_admin_transfer` | The pending admin's auth | A hand-over is pending; an applied `Admin` change stays open for seven days of ledgers | `test::governance::an_applied_admin_change_starts_a_hand_over_the_next_admin_accepts`, `test::governance::the_hand_over_offer_lasts_seven_days_of_ledgers`, `test::governance::an_admin_hand_over_waits_but_cancelling_one_does_not`, `test::setup::setup_cannot_finish_while_a_hand_over_is_pending` | | `revoke_role` | Admin | `treasury`, `guardian`: instant. `manager`: instant during setup, `UseTimelock` after | `test::governance::grants_and_manager_revokes_wait_for_the_timelock` (instant case: `test::governance::revoking_treasury_or_guardian_stays_instant`; caller check: `test::governance::only_the_admin_revokes_a_role`) | @@ -103,9 +103,9 @@ Tests: [`controls.rs`](../contracts/async-vault/src/test/controls.rs), [`epochs. | `set_role_admin` | — | Always refused (`UseTimelock`) | `test::governance::no_role_ever_gets_a_role_admin` | | `propose_wind_down` | `#[only_admin]` | No proposal standing, not active | `test::wind_down::only_governance_proposes` | | `cancel_wind_down_proposal` | `#[only_admin]` | Must be proposed, not active | `test::wind_down::governance_cancels_a_proposal_before_activation` | -| `propose_change` | `#[only_admin]` | Not winding down, setup finished, one code change at a time, an `UpgradeDelay` in `[MIN, MAX]` and `>= notice`, a `Notice` within its bounds, a `WindDownDelay` in `[MIN_WIND_DOWN_DELAY, MAX_WIND_DOWN_DELAY]` | `test::upgrade::only_governance_proposes_and_cancels`, `test::upgrade::nothing_is_proposed_during_a_wind_down`, `test::setup::nothing_is_proposed_during_setup` | +| `propose_change` | `#[only_admin]` | Not winding down, setup finished, one code change at a time, an `UpgradeDelay` in `[MIN, MAX]` and `>= notice`, a `Notice` within its bounds, a `WindDownDelay` in `[MIN_WIND_DOWN_DELAY, MAX_WIND_DOWN_DELAY]`, a `Custodian` that is not the vault (`InvalidCustodian`) | `test::upgrade::only_governance_proposes_and_cancels`, `test::upgrade::nothing_is_proposed_during_a_wind_down`, `test::setup::nothing_is_proposed_during_setup`, `test::governance::a_custodian_change_to_the_vault_is_refused_when_proposed_and_when_applied` | | `cancel_change` | `#[only_admin]` | The change must be pending | `test::upgrade::only_governance_proposes_and_cancels` | -| `apply_change` | `#[only_admin]` | Eta passed, not paused, not winding down, delay `>= notice`; an `UpgradeDelay`, `Notice` or `WindDownDelay` change is checked against its bounds again | `test::upgrade::only_governance_applies`, `test::governance::a_change_shorter_than_a_raised_notice_never_lands`, `test::governance::nothing_timelocked_moves_during_a_wind_down_a_manager_revoke_included` | +| `apply_change` | `#[only_admin]` | Eta passed, not paused, not winding down, delay `>= notice`; an `UpgradeDelay`, `Notice`, `WindDownDelay` or `Custodian` change is checked against its bounds again | `test::upgrade::only_governance_applies`, `test::governance::a_change_shorter_than_a_raised_notice_never_lands`, `test::governance::nothing_timelocked_moves_during_a_wind_down_a_manager_revoke_included`, `test::governance::a_custodian_change_to_the_vault_is_refused_when_proposed_and_when_applied` | | `propose_change` (`Oracle(address)`) | `#[only_admin]` | The feed names this vault, its setup is closed, and it answers `state()` and `freshness_duration()` (`FeedNotBound`) | `test::oracle_swap::a_feed_outside_this_vault_is_never_proposed`, `test::oracle_swap::a_contract_that_is_not_a_feed_is_refused_when_proposed_and_when_applied` | | `apply_change` (`Oracle(address)`) | `#[only_admin]` | Delay run; the feed still names this vault with its setup closed, and still answers `state()` and `freshness_duration()` (`FeedNotBound`) | `test::oracle_swap::the_oracle_changes_only_through_the_timelock`, `test::oracle_swap::a_feed_that_stops_naming_this_vault_is_never_installed`, `test::oracle_swap::a_contract_that_is_not_a_feed_is_refused_when_proposed_and_when_applied` | | `pending_changes` | Open view | None | None (view) | diff --git a/docs/strata-product-and-architecture.md b/docs/strata-product-and-architecture.md index abcbc3cb..6fabcb42 100644 --- a/docs/strata-product-and-architecture.md +++ b/docs/strata-product-and-architecture.md @@ -606,7 +606,7 @@ public surface given the vault address. | Boundary | Risk | Mitigation | | ---------------------- | ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Attestation authority | Wrong or compromised reports misprice requests | Multisig reporter; the deviation cap bounds a single report in both directions, and a downward cap of 100% leaves `min_answer` as the only floor; the cooldown bounds frequency; a sustained sequence of biased reports within the cap remains possible, is bounded in speed, and is the monitoring plan's primary alert, with the guardian pause as the reactive control. Residual risk: value transfer between entry and exit cohorts | -| Governance keys | Malicious upgrade | Timelock on every code change and on the delay itself, bounded by a minimum no shorter than the standing notice and a maximum ceiling (90 days). A pause freezes the timelock clock and pushes the ETA by the duration paused, guaranteeing that investors have an unpaused exit window before any code change applies; applying while paused is refused; nothing is proposed or applied while the vault winds down. The vault's own custodian, role grants, `manager` revocations, admin handovers and lowering of the notice or the wind-down delay wait on that clock once its one-time setup closes, and it takes no deposit before then; revoking its treasury or guardian stays instant. Setup closes only once the token and the oracle run on the vault's clock with their own setup closed, the vault is the token's only minter, no shares exist yet (shares minted earlier would redeem against depositors' money), the vault passes the identity check, its address is not frozen on the token and the token is bound to its compliance contract; the kit's own compliance contract must also run on the vault's clock with its setup closed. The share token's upgrades, grants, `manager` revocations, admin handovers and compliance or verifier swaps wait on the same clock once its setup closes. So do the oracle's upgrades, grants, admin handovers and every guardrail change except a shorter freshness window; its NAV pause, `clear_latest` and revoking its attester or guardian stay instant. On the kit's compliance contract, adding or removing a module, binding or unbinding a token and handing over the admin wait on the same clock once its setup closes. | +| Governance keys | Malicious upgrade | Timelock on every code change and on the delay itself, bounded by a minimum no shorter than the standing notice and a maximum ceiling (90 days). A pause freezes the timelock clock and pushes the ETA by the duration paused, guaranteeing that investors have an unpaused exit window before any code change applies; applying while paused is refused; nothing is proposed or applied while the vault winds down. The vault's own custodian, role grants, `manager` revocations, admin handovers and lowering of the notice or the wind-down delay wait on that clock once its one-time setup closes, and it takes no deposit before then; revoking its treasury or guardian stays instant. Setup closes only once the token and the oracle run on the vault's clock with their own setup closed, the vault is the token's only minter, no shares exist yet (shares minted earlier would redeem against depositors' money), the vault passes the identity check, its address is not frozen on the token, the token is bound to its compliance contract, the asset has the token's decimals, the asset, the token, the oracle and the vault are four different addresses, the oracle answers the feed interface, and a custodian other than the vault is set; the kit's own compliance contract must also run on the vault's clock with its setup closed. The share token's upgrades, grants, `manager` revocations, admin handovers and compliance or verifier swaps wait on the same clock once its setup closes. So do the oracle's upgrades, grants, admin handovers and every guardrail change except a shorter freshness window; its NAV pause, `clear_latest` and revoking its attester or guardian stay instant. On the kit's compliance contract, adding or removing a module, binding or unbinding a token and handing over the admin wait on the same clock once its setup closes. | | Mis-wired deployment | A vault goes live without every binding wired | `health()` reports each wiring step; `finish_setup` refuses until all hold, and the vault takes no deposit before that | | Treasury keys | Reserve drained | Only free reserve is movable, only to the genesis-configured custodian, verified on-chain; outbound transfers are blocked while anything is uncovered, and escrowed subscriptions never leave the vault | | Guardian keys | Griefing via pause | Guardian can only pause the vault, which stops deposit requests, epoch pricing and upgrade application and freezes the upgrade timelock; it cannot halt redemptions, custodian deployments, or payable claims, nor move funds; governance unpauses and rotates the vault role. It holds no role on the share token: the token's transfer pause is admin-gated (governance by deploy), not a guardian lever. No contract keeps governance and guardian apart |