diff --git a/contracts/async-vault/src/error.rs b/contracts/async-vault/src/error.rs index 5c92a6d4..8c246d06 100644 --- a/contracts/async-vault/src/error.rs +++ b/contracts/async-vault/src/error.rs @@ -52,8 +52,10 @@ pub enum VaultError { /// The epoch is priced, so the request can no longer be cancelled. The /// holder claims instead. AlreadyPriced = 6039, - /// The vault must always have an admin. Governance hands over in two steps - /// rather than stepping down into nobody. + /// The call needs the admin. `unpause` refuses any other caller. + /// `renounce_admin` is always refused, because the vault must always have + /// an admin: governance hands over in two steps rather than stepping down + /// into nobody. AdminRequired = 6040, /// The epoch is sealed and the feed can price it, so the price it will take /// is already knowable. Cancelling now would be declining a price after diff --git a/contracts/async-vault/src/guards.rs b/contracts/async-vault/src/guards.rs index 71029fda..784917d6 100644 --- a/contracts/async-vault/src/guards.rs +++ b/contracts/async-vault/src/guards.rs @@ -1,5 +1,8 @@ use soroban_sdk::{panic_with_error, Address, Env, Symbol}; -use stellar_access::{access_control::AccessControlStorageKey, role_transfer}; +use stellar_access::{ + access_control::{self, AccessControlStorageKey}, + role_transfer, +}; use stellar_tokens::{ fungible::FungibleTokenClient, rwa::{compliance::ComplianceClient as RulesClient, RWATokenClient}, @@ -94,6 +97,13 @@ pub(crate) fn no_hand_over_pending(e: &Env) { } } +/// Stands in for `#[only_admin]` where an OpenZeppelin trait fixes the caller. +pub(crate) fn caller_is_admin(e: &Env, caller: &Address) { + if access_control::get_admin(e).as_ref() != Some(caller) { + panic_with_error!(e, VaultError::AdminRequired); + } +} + pub(crate) fn wiring_complete(e: &Env) { if !governance::wiring(e).complete() { panic_with_error!(e, VaultError::SetupIncomplete); diff --git a/contracts/async-vault/src/lib.rs b/contracts/async-vault/src/lib.rs index 700a809e..11dc124a 100644 --- a/contracts/async-vault/src/lib.rs +++ b/contracts/async-vault/src/lib.rs @@ -178,8 +178,7 @@ impl AsyncVault { } #[only_admin] - pub fn set_deposit_cap(e: &Env, cap: Option, caller: Address) { - caller.require_auth(); + pub fn set_deposit_cap(e: &Env, cap: Option) { guards::non_negative_cap(e, cap); let old_cap = state::deposit_cap(e); state::set_deposit_cap(e, cap); @@ -193,8 +192,7 @@ impl AsyncVault { /// Instant during setup; afterwards the custodian changes through the /// timelock. #[only_admin] - pub fn set_custodian(e: &Env, custodian: Address, caller: Address) { - caller.require_auth(); + pub fn set_custodian(e: &Env, custodian: Address) { guards::direct_change_allowed(e); treasury::set_custodian(e, &custodian); } @@ -202,8 +200,7 @@ impl AsyncVault { /// Raising is instant. Lowering shortens the warning holders get before a /// wind-down starts, so after setup it waits for the timelock. #[only_admin] - pub fn set_wind_down_delay(e: &Env, secs: u64, caller: Address) { - caller.require_auth(); + pub fn set_wind_down_delay(e: &Env, secs: u64) { guards::wind_down_delay_in_bounds(e, secs); guards::not_lowered_after_setup(e, secs, wind_down::delay(e)); wind_down::set_delay(e, secs); @@ -218,14 +215,12 @@ impl AsyncVault { } #[only_admin] - pub fn propose_wind_down(e: &Env, caller: Address) { - caller.require_auth(); + pub fn propose_wind_down(e: &Env) { wind_down::propose(e); } #[only_admin] - pub fn cancel_wind_down_proposal(e: &Env, caller: Address) { - caller.require_auth(); + pub fn cancel_wind_down_proposal(e: &Env) { wind_down::cancel_proposal(e); } @@ -267,8 +262,7 @@ impl AsyncVault { /// Raising is instant. Lowering shortens the wait every closed epoch gives /// before it prices, so after setup it waits for the timelock. #[only_admin] - pub fn set_notice(e: &Env, secs: u64, caller: Address) { - caller.require_auth(); + pub fn set_notice(e: &Env, secs: u64) { guards::not_lowered_after_setup(e, secs, state::notice(e)); guards::notice_in_bounds(e, secs); governance::set_notice(e, secs); @@ -279,8 +273,7 @@ impl AsyncVault { /// manager, allowlisted, not frozen), no shares outstanding and no admin /// hand-over pending. Deposits open from here on, and it never reopens. #[only_admin] - pub fn finish_setup(e: &Env, caller: Address) { - caller.require_auth(); + pub fn finish_setup(e: &Env) { governance::finish_setup(e); } @@ -297,22 +290,19 @@ impl AsyncVault { /// Queues a change behind the timelock and returns its id. #[only_admin] - pub fn propose_change(e: &Env, change: VaultChange, caller: Address) -> u32 { - caller.require_auth(); + pub fn propose_change(e: &Env, change: VaultChange) -> u32 { governance::propose(e, change) } #[only_admin] - pub fn cancel_change(e: &Env, id: u32, caller: Address) { - caller.require_auth(); + pub fn cancel_change(e: &Env, id: u32) { governance::cancel(e, id); } /// Lands a change whose delay has run. Refused while paused or winding /// down. #[only_admin] - pub fn apply_change(e: &Env, id: u32, caller: Address) { - caller.require_auth(); + pub fn apply_change(e: &Env, id: u32) { governance::apply(e, id); } @@ -452,8 +442,8 @@ impl Pausable for AsyncVault { strata_timelock::pause::start(e); } - #[only_admin] fn unpause(e: &Env, caller: Address) { + guards::caller_is_admin(e, &caller); caller.require_auth(); pausable::unpause(e); strata_timelock::pause::end(e); diff --git a/contracts/async-vault/src/test/compliance.rs b/contracts/async-vault/src/test/compliance.rs index ffd539a7..ae84f4a0 100644 --- a/contracts/async-vault/src/test/compliance.rs +++ b/contracts/async-vault/src/test/compliance.rs @@ -24,7 +24,7 @@ fn an_investor_who_was_never_allowlisted_is_refused_before_any_cash_moves() { fn a_delisted_investor_cannot_request_a_deposit() { let f = setup(); let investor = f.investor(1_000); - f.identity.allow(&investor, &false, &f.compliance); + f.identity.allow(&investor, &false); refused( f.vault.try_request_deposit(&investor, &400), @@ -48,9 +48,9 @@ fn relisting_after_pricing_lets_the_claim_through() { let investor = f.investor(1_000); f.vault.request_deposit(&investor, &400); f.fulfill_epoch(wad(2)); - f.identity.allow(&investor, &false, &f.compliance); + f.identity.allow(&investor, &false); - f.identity.allow(&investor, &true, &f.compliance); + f.identity.allow(&investor, &true); assert_eq!(f.vault.claim_deposit(&investor, &1), 200); assert_eq!(f.shares(&investor), 200); @@ -65,14 +65,14 @@ fn a_holder_delisted_after_pricing_is_paid_through_a_wind_down() { f.fulfill_epoch(wad(2)); assert_eq!(f.vault.cancellable_escrow(), 0); assert_eq!(f.vault.pending_mint_shares(), 200); - f.identity.allow(&investor, &false, &f.compliance); + f.identity.allow(&investor, &false); refused( f.vault.try_cancel_deposit(&investor, &epoch), VaultError::AlreadyPriced, ); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); assert_eq!(f.vault.claim_deposit(&investor, &epoch), 0); @@ -119,7 +119,7 @@ fn a_delisted_holder_can_still_queue_an_exit() { let user = f.holder(500); let epoch = f.vault.current_epoch(); - f.identity.allow(&user, &false, &f.compliance); + f.identity.allow(&user, &false); assert_eq!(f.vault.request_redeem(&user, &200), epoch); assert_eq!(f.shares(&f.vault.address), 200); @@ -132,7 +132,7 @@ fn a_delisted_holder_can_still_claim_their_exit() { let epoch = f.vault.request_redeem(&user, &200); f.fulfill_epoch(wad(2)); - f.identity.allow(&user, &false, &f.compliance); + f.identity.allow(&user, &false); assert_eq!(f.vault.claim_redeem(&user, &epoch), 400); assert_eq!(f.balance(&user), 400); @@ -145,7 +145,7 @@ fn a_delisted_investor_can_cancel_deposit_and_receive_asset() { let epoch = f.vault.request_deposit(&investor, &400); // Investor gets delisted - f.identity.allow(&investor, &false, &f.compliance); + f.identity.allow(&investor, &false); // Delisted investor can still cancel deposit and receive settlement asset assert_eq!(f.vault.cancel_deposit(&investor, &epoch), 400); @@ -159,7 +159,7 @@ fn a_delisted_holder_cannot_cancel_redemption_back_to_shares() { let epoch = f.vault.request_redeem(&user, &200); // Delisted before pricing - f.identity.allow(&user, &false, &f.compliance); + f.identity.allow(&user, &false); refused( f.vault.try_cancel_redeem(&user, &epoch), @@ -202,11 +202,11 @@ fn a_frozen_or_delisted_holder_cannot_transfer_shares() { assert_eq!(f.shares(&recipient), 100); // Delisted sender cannot transfer - f.identity.allow(&user, &false, &f.compliance); + f.identity.allow(&user, &false); assert!(f.share.try_transfer(&user, &recipient, &100).is_err()); // Restore allowlist, then freeze sender - f.identity.allow(&user, &true, &f.compliance); + f.identity.allow(&user, &true); f.share.set_address_frozen(&user, &true, &f.compliance); assert!(f.share.try_transfer(&user, &recipient, &100).is_err()); } @@ -321,7 +321,7 @@ fn a_frozen_holder_is_not_moved_into_the_exit_queue() { let investor = f.investor(1_000); let epoch = f.vault.request_deposit(&investor, &400); f.fulfill_epoch(wad(2)); - f.identity.allow(&investor, &false, &f.compliance); + f.identity.allow(&investor, &false); f.share.set_address_frozen(&investor, &true, &f.compliance); refused( diff --git a/contracts/async-vault/src/test/controls.rs b/contracts/async-vault/src/test/controls.rs index 6799488f..44059782 100644 --- a/contracts/async-vault/src/test/controls.rs +++ b/contracts/async-vault/src/test/controls.rs @@ -44,6 +44,17 @@ fn the_guardian_pauses_but_only_governance_unpauses() { assert!(!f.vault.paused()); } +#[test] +fn a_caller_other_than_the_admin_cannot_unpause_even_when_it_signs() { + let f = setup(); + f.vault.pause(&f.guardian); + + for who in [f.guardian.clone(), Address::generate(&f.e)] { + refused(f.vault.try_unpause(&who), VaultError::AdminRequired); + } + assert!(f.vault.paused()); +} + #[test] fn a_paused_vault_still_lets_investors_claim() { let f = setup(); @@ -108,17 +119,17 @@ fn governance_can_replace_the_guardian() { let f = setup(); let next = Address::generate(&f.e); - let grant = f.vault.propose_change( - &VaultChange::GrantRole(symbol_short!("guardian"), next.clone()), - &f.admin, - ); - let revoke = f.vault.propose_change( - &VaultChange::RevokeRole(symbol_short!("guardian"), f.guardian.clone()), - &f.admin, - ); + let grant = f.vault.propose_change(&VaultChange::GrantRole( + symbol_short!("guardian"), + next.clone(), + )); + let revoke = f.vault.propose_change(&VaultChange::RevokeRole( + symbol_short!("guardian"), + f.guardian.clone(), + )); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&grant, &f.admin); - f.vault.apply_change(&revoke, &f.admin); + f.vault.apply_change(&grant); + f.vault.apply_change(&revoke); assert_eq!(f.vault.guardian(), Some(next.clone())); assert!(f.vault.try_pause(&f.guardian).is_err()); @@ -242,11 +253,9 @@ fn governance_hands_over_in_two_steps_instead() { let f = setup(); let next = Address::generate(&f.e); - let id = f - .vault - .propose_change(&VaultChange::Admin(next.clone()), &f.admin); + let id = f.vault.propose_change(&VaultChange::Admin(next.clone())); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); f.vault.accept_admin_transfer(); assert_eq!(f.vault.governance(), Some(next.clone())); diff --git a/contracts/async-vault/src/test/delisted.rs b/contracts/async-vault/src/test/delisted.rs index cead24d1..c5a25efd 100644 --- a/contracts/async-vault/src/test/delisted.rs +++ b/contracts/async-vault/src/test/delisted.rs @@ -9,7 +9,7 @@ fn delisted_with_a_priced_deposit(f: &Fixture) -> (Address, u64) { let investor = f.investor(1_000); let epoch = f.vault.request_deposit(&investor, &400); f.fulfill_epoch(wad(2)); - f.identity.allow(&investor, &false, &f.compliance); + f.identity.allow(&investor, &false); (investor, epoch) } @@ -88,7 +88,7 @@ fn a_conversion_adds_to_a_redemption_already_open_in_the_epoch() { f.fulfill_epoch(wad(2)); let open = f.vault.current_epoch(); f.vault.request_redeem(&investor, &100); - f.identity.allow(&investor, &false, &f.compliance); + f.identity.allow(&investor, &false); f.vault.claim_deposit(&investor, &epoch); @@ -107,7 +107,7 @@ fn a_conversion_adds_to_a_redemption_already_open_in_the_epoch() { fn in_a_wind_down_the_shares_are_credited_before_the_first_round() { let f = setup(); let (investor, epoch) = delisted_with_a_priced_deposit(&f); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); @@ -140,7 +140,7 @@ fn in_a_wind_down_the_shares_are_credited_before_the_first_round() { fn in_a_wind_down_the_shares_are_credited_after_the_first_round() { let f = setup(); let (investor, epoch) = delisted_with_a_priced_deposit(&f); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); assert_eq!(f.vault.finalize_wind_down_round(), 400); @@ -157,7 +157,7 @@ fn a_credit_before_the_first_round_leaves_every_holder_their_share() { let f = setup(); let other = f.holder(200); let (investor, epoch) = delisted_with_a_priced_deposit(&f); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); f.vault.claim_deposit(&investor, &epoch); @@ -193,7 +193,7 @@ fn a_conversion_caught_by_a_wind_down_is_paid_from_the_rounds() { let f = setup(); let other = f.holder(200); let (investor, epoch) = delisted_with_a_priced_deposit(&f); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.vault.claim_deposit(&investor, &epoch); let open = f.vault.current_epoch(); f.advance(MIN_WIND_DOWN_DELAY); @@ -223,7 +223,7 @@ fn a_redemption_surrendered_after_the_first_round_keeps_the_rounds_solvent() { let f = setup(); let other = f.holder(200); let (investor, epoch) = delisted_with_a_priced_deposit(&f); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.vault.claim_deposit(&investor, &epoch); let open = f.vault.current_epoch(); f.advance(MIN_WIND_DOWN_DELAY); @@ -243,8 +243,8 @@ fn a_delisted_holders_unpriced_redemption_joins_the_wind_down_when_cancelled() { let investor = f.holder(200); let other = f.holder(200); let epoch = f.vault.request_redeem(&investor, &200); - f.identity.allow(&investor, &false, &f.compliance); - f.vault.propose_wind_down(&f.admin); + f.identity.allow(&investor, &false); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); @@ -261,8 +261,8 @@ fn a_conversion_waits_while_the_vault_is_delisted() { let investor = f.investor(1_000); let epoch = f.vault.request_deposit(&investor, &1_000); f.fulfill_epoch(wad(2)); - f.identity.allow(&investor, &false, &f.compliance); - f.identity.allow(&f.vault.address, &false, &f.compliance); + f.identity.allow(&investor, &false); + f.identity.allow(&f.vault.address, &false); assert_eq!( f.vault.try_claim_deposit(&investor, &epoch), @@ -270,7 +270,7 @@ fn a_conversion_waits_while_the_vault_is_delisted() { ); assert!(f.vault.get_deposit_request(&epoch, &investor).is_some()); - f.identity.allow(&f.vault.address, &true, &f.compliance); + f.identity.allow(&f.vault.address, &true); assert_eq!(f.vault.claim_deposit(&investor, &epoch), 0); let open = f.vault.current_epoch(); assert_eq!( @@ -309,7 +309,7 @@ proptest! { for (_, i) in claims { let who = &investors[i]; if delisted[i] { - f.identity.allow(who, &false, &f.compliance); + f.identity.allow(who, &false); } let queued = || f.vault.get_redeem_request(&open, who).map_or(0, |r| r.shares); let before = queued(); diff --git a/contracts/async-vault/src/test/deposit_cap.rs b/contracts/async-vault/src/test/deposit_cap.rs index 28ac415e..e0c321d1 100644 --- a/contracts/async-vault/src/test/deposit_cap.rs +++ b/contracts/async-vault/src/test/deposit_cap.rs @@ -1,3 +1,5 @@ +use soroban_sdk::IntoVal; + use super::*; #[test] @@ -15,15 +17,15 @@ fn governance_sets_updates_and_clears_cap() { let f = setup(); // Set cap - f.vault.set_deposit_cap(&Some(1_000), &f.admin); + f.vault.set_deposit_cap(&Some(1_000)); assert_eq!(f.vault.deposit_cap(), Some(1_000)); // Update cap - f.vault.set_deposit_cap(&Some(2_000), &f.admin); + f.vault.set_deposit_cap(&Some(2_000)); assert_eq!(f.vault.deposit_cap(), Some(2_000)); // Clear cap - f.vault.set_deposit_cap(&None, &f.admin); + f.vault.set_deposit_cap(&None); assert_eq!(f.vault.deposit_cap(), None); } @@ -31,7 +33,7 @@ fn governance_sets_updates_and_clears_cap() { fn negative_cap_rejected() { let f = setup(); refused( - f.vault.try_set_deposit_cap(&Some(-1), &f.admin), + f.vault.try_set_deposit_cap(&Some(-1)), VaultError::InvalidAmount, ); } @@ -41,26 +43,17 @@ fn unauthorized_caller_rejected() { let f = setup(); let rando = Address::generate(&f.e); - f.e.set_auths(&[]); - assert!(f.vault.try_set_deposit_cap(&Some(1_000), &rando).is_err()); - assert!(f - .vault - .try_set_deposit_cap(&Some(1_000), &f.manager) - .is_err()); - assert!(f - .vault - .try_set_deposit_cap(&Some(1_000), &f.guardian) - .is_err()); - assert!(f - .vault - .try_set_deposit_cap(&Some(1_000), &f.treasury) - .is_err()); + for who in [&rando, &f.manager, &f.guardian, &f.treasury] { + f.sign_only_as(who, "set_deposit_cap", (Some(1_000_i128),).into_val(&f.e)); + assert!(f.vault.try_set_deposit_cap(&Some(1_000)).is_err()); + } + assert_eq!(f.vault.deposit_cap(), None); } #[test] fn boundary_value_enforcement() { let f = setup(); - f.vault.set_deposit_cap(&Some(1_000), &f.admin); + f.vault.set_deposit_cap(&Some(1_000)); let investor1 = f.investor(2_000); let investor2 = f.investor(2_000); @@ -85,7 +78,7 @@ fn boundary_value_enforcement() { #[test] fn cancellation_frees_capacity() { let f = setup(); - f.vault.set_deposit_cap(&Some(1_000), &f.admin); + f.vault.set_deposit_cap(&Some(1_000)); let investor1 = f.investor(1_000); let investor2 = f.investor(1_000); @@ -110,7 +103,7 @@ fn cancellation_frees_capacity() { #[test] fn deployed_capital_and_multi_epoch_accounting() { let f = setup(); - f.vault.set_deposit_cap(&Some(1_000), &f.admin); + f.vault.set_deposit_cap(&Some(1_000)); let investor1 = f.investor(1_000); let investor2 = f.investor(1_000); @@ -132,7 +125,7 @@ fn deployed_capital_and_multi_epoch_accounting() { ); // Governance raises cap to 1_500 - f.vault.set_deposit_cap(&Some(1_500), &f.admin); + f.vault.set_deposit_cap(&Some(1_500)); // Now investor2 can deposit 500 let epoch2 = f.vault.request_deposit(&investor2, &500); @@ -142,7 +135,7 @@ fn deployed_capital_and_multi_epoch_accounting() { #[test] fn redemptions_free_capacity() { let f = setup(); - f.vault.set_deposit_cap(&Some(1_000), &f.admin); + f.vault.set_deposit_cap(&Some(1_000)); let investor1 = f.investor(1_000); let investor2 = f.investor(1_000); @@ -177,7 +170,7 @@ fn redemptions_free_capacity() { #[test] fn zero_cap_blocks_all_deposits() { let f = setup(); - f.vault.set_deposit_cap(&Some(0), &f.admin); + f.vault.set_deposit_cap(&Some(0)); let investor = f.investor(100); refused( diff --git a/contracts/async-vault/src/test/governance.rs b/contracts/async-vault/src/test/governance.rs index d3713d75..f90bd1d0 100644 --- a/contracts/async-vault/src/test/governance.rs +++ b/contracts/async-vault/src/test/governance.rs @@ -1,7 +1,4 @@ -use soroban_sdk::{ - testutils::{Events as _, MockAuth, MockAuthInvoke}, - Event as _, IntoVal, -}; +use soroban_sdk::{testutils::Events as _, Event as _, IntoVal}; use stellar_access::role_transfer::RoleTransferError; use super::*; @@ -9,9 +6,9 @@ use crate::{ChangeApplied, CustodianSet, ADMIN_ACCEPT_LEDGERS}; /// Proposes a change, waits one delay and applies it. fn land(f: &Fixture, change: VaultChange) { - let id = f.vault.propose_change(&change, &f.admin); + let id = f.vault.propose_change(&change); f.advance(f.vault.upgrade_delay()); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); } #[test] @@ -19,21 +16,15 @@ fn the_custodian_changes_through_the_timelock() { let f = setup(); let next = Address::generate(&f.e); - refused( - f.vault.try_set_custodian(&next, &f.admin), - VaultError::UseTimelock, - ); + refused(f.vault.try_set_custodian(&next), VaultError::UseTimelock); let id = f .vault - .propose_change(&VaultChange::Custodian(next.clone()), &f.admin); + .propose_change(&VaultChange::Custodian(next.clone())); f.advance(MIN_UPGRADE_DELAY - 1); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::ChangeNotReady, - ); + refused(f.vault.try_apply_change(&id), VaultError::ChangeNotReady); f.advance(1); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!(f.vault.custodian(), Some(next)); } @@ -43,10 +34,10 @@ fn applying_a_custodian_change_publishes_both_events() { let f = setup(); let next = Address::generate(&f.e); let change = VaultChange::Custodian(next.clone()); - let id = f.vault.propose_change(&change, &f.admin); + let id = f.vault.propose_change(&change); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!( f.e.events().all().filter_by_contract(&f.vault.address), @@ -61,13 +52,10 @@ fn applying_a_custodian_change_publishes_both_events() { fn raising_the_notice_is_instant_and_lowering_waits() { let f = setup(); - f.vault.set_notice(&DAY_DURATION, &f.admin); + f.vault.set_notice(&DAY_DURATION); assert_eq!(f.vault.notice(), DAY_DURATION); - refused( - f.vault.try_set_notice(&3_600, &f.admin), - VaultError::UseTimelock, - ); + refused(f.vault.try_set_notice(&3_600), VaultError::UseTimelock); land(&f, VaultChange::Notice(3_600)); assert_eq!(f.vault.notice(), 3_600); @@ -79,7 +67,7 @@ fn a_notice_change_still_respects_its_bounds() { refused( f.vault - .try_propose_change(&VaultChange::Notice(MIN_UPGRADE_DELAY + 1), &f.admin), + .try_propose_change(&VaultChange::Notice(MIN_UPGRADE_DELAY + 1)), VaultError::NoticeAboveUpgradeDelay, ); } @@ -90,17 +78,17 @@ fn a_notice_change_checks_the_upgrade_delay_again_when_it_applies() { land(&f, VaultChange::UpgradeDelay(30 * DAY_DURATION)); let notice = f .vault - .propose_change(&VaultChange::Notice(20 * DAY_DURATION), &f.admin); + .propose_change(&VaultChange::Notice(20 * DAY_DURATION)); let delay = f .vault - .propose_change(&VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY), &f.admin); + .propose_change(&VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY)); f.advance(30 * DAY_DURATION); // The delay falls below the notice the first change would set. - f.vault.apply_change(&delay, &f.admin); + f.vault.apply_change(&delay); refused( - f.vault.try_apply_change(¬ice, &f.admin), + f.vault.try_apply_change(¬ice), VaultError::NoticeAboveUpgradeDelay, ); assert_eq!(f.vault.notice(), 0); @@ -110,11 +98,9 @@ fn a_notice_change_checks_the_upgrade_delay_again_when_it_applies() { fn raising_the_wind_down_delay_is_instant_and_lowering_waits() { let f = setup(); - f.vault - .set_wind_down_delay(&(2 * MIN_WIND_DOWN_DELAY), &f.admin); + f.vault.set_wind_down_delay(&(2 * MIN_WIND_DOWN_DELAY)); refused( - f.vault - .try_set_wind_down_delay(&MIN_WIND_DOWN_DELAY, &f.admin), + f.vault.try_set_wind_down_delay(&MIN_WIND_DOWN_DELAY), VaultError::UseTimelock, ); land(&f, VaultChange::WindDownDelay(MIN_WIND_DOWN_DELAY)); @@ -127,10 +113,8 @@ fn a_wind_down_delay_below_the_minimum_cannot_be_proposed() { let f = setup(); refused( - f.vault.try_propose_change( - &VaultChange::WindDownDelay(MIN_WIND_DOWN_DELAY - 1), - &f.admin, - ), + f.vault + .try_propose_change(&VaultChange::WindDownDelay(MIN_WIND_DOWN_DELAY - 1)), VaultError::WindDownDelayTooShort, ); } @@ -236,31 +220,12 @@ fn an_admin_hand_over_waits_but_cancelling_one_does_not() { fn an_applied_admin_change_starts_a_hand_over_the_next_admin_accepts() { let f = setup(); let next = Address::generate(&f.e); - let id = f - .vault - .propose_change(&VaultChange::Admin(next.clone()), &f.admin); + let id = f.vault.propose_change(&VaultChange::Admin(next.clone())); f.advance(MIN_UPGRADE_DELAY); - // The admin authorises this call and nothing else: the hand-over inside it - // asks for no further signature. The entrypoint checks the admin and the - // caller, so the same authorisation appears twice, as simulation records it. - let apply = MockAuthInvoke { - contract: &f.vault.address, - fn_name: "apply_change", - args: (id, f.admin.clone()).into_val(&f.e), - sub_invokes: &[], - }; - f.e.mock_auths(&[ - MockAuth { - address: &f.admin, - invoke: &apply, - }, - MockAuth { - address: &f.admin, - invoke: &apply, - }, - ]); - f.vault.apply_change(&id, &f.admin); + // One admin signature covers the call; the hand-over inside asks for none. + f.sign_only_as(&f.admin, "apply_change", (id,).into_val(&f.e)); + f.vault.apply_change(&id); f.e.mock_all_auths(); assert_eq!(f.vault.governance(), Some(f.admin.clone())); @@ -302,21 +267,19 @@ fn the_hand_over_offer_lasts_seven_days_of_ledgers() { fn a_change_shorter_than_a_raised_notice_never_lands() { let f = setup(); let next = Address::generate(&f.e); - let custodian = f - .vault - .propose_change(&VaultChange::Custodian(next), &f.admin); + let custodian = f.vault.propose_change(&VaultChange::Custodian(next)); let delay = f .vault - .propose_change(&VaultChange::UpgradeDelay(30 * DAY_DURATION), &f.admin); + .propose_change(&VaultChange::UpgradeDelay(30 * DAY_DURATION)); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&delay, &f.admin); + f.vault.apply_change(&delay); // The notice rises past the seven days the custodian change was given. - f.vault.set_notice(&(20 * DAY_DURATION), &f.admin); + f.vault.set_notice(&(20 * DAY_DURATION)); f.advance(30 * DAY_DURATION); refused( - f.vault.try_apply_change(&custodian, &f.admin), + f.vault.try_apply_change(&custodian), VaultError::UpgradeDelayBelowNotice, ); assert_eq!(f.vault.custodian(), Some(f.custodian.clone())); @@ -326,11 +289,11 @@ fn a_change_shorter_than_a_raised_notice_never_lands() { fn the_deposit_cap_stays_instant_both_ways() { let f = setup(); - f.vault.set_deposit_cap(&Some(1_000), &f.admin); + f.vault.set_deposit_cap(&Some(1_000)); assert_eq!(f.vault.deposit_cap(), Some(1_000)); - f.vault.set_deposit_cap(&Some(500), &f.admin); + f.vault.set_deposit_cap(&Some(500)); assert_eq!(f.vault.deposit_cap(), Some(500)); - f.vault.set_deposit_cap(&None, &f.admin); + f.vault.set_deposit_cap(&None); assert_eq!(f.vault.deposit_cap(), None); } @@ -338,18 +301,18 @@ fn the_deposit_cap_stays_instant_both_ways() { fn a_pending_role_change_lands_after_a_pause() { let f = setup(); let next = Address::generate(&f.e); - let id = f.vault.propose_change( - &VaultChange::GrantRole(symbol_short!("guardian"), next.clone()), - &f.admin, - ); + let id = f.vault.propose_change(&VaultChange::GrantRole( + symbol_short!("guardian"), + next.clone(), + )); f.vault.pause(&f.guardian); f.advance(MIN_UPGRADE_DELAY); - assert!(f.vault.try_apply_change(&id, &f.admin).is_err()); + assert!(f.vault.try_apply_change(&id).is_err()); f.vault.unpause(&f.admin); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert!(f .vault .has_role(&next, &symbol_short!("guardian")) @@ -359,23 +322,23 @@ fn a_pending_role_change_lands_after_a_pause() { #[test] fn nothing_timelocked_moves_during_a_wind_down_a_manager_revoke_included() { let f = setup(); - let revoke = f.vault.propose_change( - &VaultChange::RevokeRole(symbol_short!("manager"), f.manager.clone()), - &f.admin, - ); - f.vault.propose_wind_down(&f.admin); + let revoke = f.vault.propose_change(&VaultChange::RevokeRole( + symbol_short!("manager"), + f.manager.clone(), + )); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); refused( - f.vault.try_apply_change(&revoke, &f.admin), + f.vault.try_apply_change(&revoke), VaultError::WindDownActive, ); refused( - f.vault.try_propose_change( - &VaultChange::RevokeRole(symbol_short!("manager"), f.manager.clone()), - &f.admin, - ), + f.vault.try_propose_change(&VaultChange::RevokeRole( + symbol_short!("manager"), + f.manager.clone(), + )), VaultError::WindDownActive, ); refused( diff --git a/contracts/async-vault/src/test/guards.rs b/contracts/async-vault/src/test/guards.rs index 9cfabafa..6cd429b4 100644 --- a/contracts/async-vault/src/test/guards.rs +++ b/contracts/async-vault/src/test/guards.rs @@ -63,7 +63,7 @@ fn not_winding_down_refuses_once_the_wind_down_is_active() { let f = setup(); holds(&f, || guards::not_winding_down(&f.e)); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); holds(&f, || guards::not_winding_down(&f.e)); f.advance(MIN_WIND_DOWN_DELAY); @@ -80,7 +80,7 @@ fn winding_down_refuses_until_the_wind_down_is_active() { guards::winding_down(&f.e) }); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); refuses(&f, VaultError::WindDownNotActive, || { guards::winding_down(&f.e) }); @@ -183,6 +183,19 @@ fn no_hand_over_pending_refuses_while_an_offer_is_open() { holds(&f, || guards::no_hand_over_pending(&f.e)); } +#[test] +fn caller_is_admin_refuses_anyone_but_the_admin() { + let f = setup(); + let stranger = Address::generate(&f.e); + holds(&f, || guards::caller_is_admin(&f.e, &f.admin)); + + for who in [&f.guardian, &f.manager, &stranger] { + refuses(&f, VaultError::AdminRequired, || { + guards::caller_is_admin(&f.e, who) + }); + } +} + #[test] fn wiring_complete_refuses_a_miswired_vault() { let wired = setup_unfinished(); @@ -324,7 +337,7 @@ fn not_a_second_code_change_refuses_a_code_change_while_one_waits() { holds(&f, || guards::not_a_second_code_change(&f.e, &upgrade)); f.vault - .propose_change(&VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY), &f.admin); + .propose_change(&VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY)); refuses(&f, VaultError::UpgradeProposalExists, || { guards::not_a_second_code_change(&f.e, &upgrade) }); @@ -341,7 +354,7 @@ fn no_wind_down_proposed_refuses_a_standing_proposal() { let f = setup(); holds(&f, || guards::no_wind_down_proposed(&f.e)); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); refuses(&f, VaultError::WindDownAlreadyProposed, || { guards::no_wind_down_proposed(&f.e) }); @@ -354,7 +367,7 @@ fn wind_down_proposed_refuses_without_a_standing_proposal() { guards::wind_down_proposed(&f.e) }); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); holds(&f, || guards::wind_down_proposed(&f.e)); f.advance(MIN_WIND_DOWN_DELAY); @@ -436,7 +449,7 @@ fn within_cap_refuses_a_deposit_past_the_cap() { let f = setup(); holds(&f, || guards::within_cap(&f.e, i128::MAX)); - f.vault.set_deposit_cap(&Some(1_000), &f.admin); + f.vault.set_deposit_cap(&Some(1_000)); let investor = f.investor(400); f.vault.request_deposit(&investor, &400); holds(&f, || guards::within_cap(&f.e, 600)); @@ -647,7 +660,7 @@ fn fulfilable_refuses_an_epoch_the_vault_cannot_price_yet() { guards::fulfilable(&f.e, &behind_the_report) }); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); let within_the_notice = epoch(&f, f.close_epoch()); refuses(&f, VaultError::NoticeNotElapsed, || { guards::fulfilable(&f.e, &within_the_notice) diff --git a/contracts/async-vault/src/test/health.rs b/contracts/async-vault/src/test/health.rs index 829c4f53..afcf2589 100644 --- a/contracts/async-vault/src/test/health.rs +++ b/contracts/async-vault/src/test/health.rs @@ -134,7 +134,7 @@ fn a_notice_longer_than_a_report_lives_is_reported() { f.attest(wad(1)); let freshness = f.oracle.config().freshness_duration; - f.vault.set_notice(&(freshness + 1), &f.admin); + f.vault.set_notice(&(freshness + 1)); assert!(!f.vault.health().freshness_covers_notice); } diff --git a/contracts/async-vault/src/test/lock_policy.rs b/contracts/async-vault/src/test/lock_policy.rs index 40133a85..09a8ccfd 100644 --- a/contracts/async-vault/src/test/lock_policy.rs +++ b/contracts/async-vault/src/test/lock_policy.rs @@ -266,7 +266,7 @@ fn standing_is_delisted_for_an_address_that_was_never_allowlisted() { fn standing_is_delisted_for_a_holder_listed_then_removed() { let f = setup(); let holder = f.investor(0); - f.identity.allow(&holder, &false, &f.compliance); + f.identity.allow(&holder, &false); assert_eq!( f.e.as_contract(&f.vault.address, || lock_policy::standing(&f.e, &holder)), @@ -290,7 +290,7 @@ fn standing_is_frozen_for_a_frozen_holder_on_the_allowlist() { fn standing_is_frozen_for_a_holder_both_frozen_and_delisted() { let f = setup(); let holder = f.investor(0); - f.identity.allow(&holder, &false, &f.compliance); + f.identity.allow(&holder, &false); f.share.set_address_frozen(&holder, &true, &f.compliance); assert_eq!( diff --git a/contracts/async-vault/src/test/mod.rs b/contracts/async-vault/src/test/mod.rs index 9eb755bc..cb451efa 100644 --- a/contracts/async-vault/src/test/mod.rs +++ b/contracts/async-vault/src/test/mod.rs @@ -21,6 +21,7 @@ mod redeem; mod settlement; mod setup; mod share_returns; +mod signatures; mod supply; mod timing; mod treasury; @@ -98,9 +99,33 @@ impl Fixture<'_> { }]); } + /// The last call asked `who` alone to sign, once, on exactly this call. + fn signed_once_by( + &self, + who: &Address, + fn_name: &str, + args: soroban_sdk::Vec, + ) { + use soroban_sdk::testutils::{AuthorizedFunction, AuthorizedInvocation}; + assert_eq!( + self.e.auths(), + std::vec![( + who.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + self.vault.address.clone(), + soroban_sdk::Symbol::new(&self.e, fn_name), + args, + )), + sub_invocations: std::vec![], + }, + )] + ); + } + fn investor(&self, funded: i128) -> Address { let who = self.unverified_investor(funded); - self.identity.allow(&who, &true, &self.compliance); + self.identity.allow(&who, &true); who } @@ -176,12 +201,9 @@ impl Fixture<'_> { /// After setup, a module lands only through the compliance contract's timelock. fn add_compliance_module(&self, hook: ComplianceHook, module: &Address) { let rules = ComplianceClient::new(&self.e, &self.share.compliance()); - let id = rules.propose_change( - &ComplianceChange::AddModule(hook, module.clone()), - &self.admin, - ); + let id = rules.propose_change(&ComplianceChange::AddModule(hook, module.clone())); self.advance(MIN_UPGRADE_DELAY); - rules.apply_change(&id, &self.admin); + rules.apply_change(&id); } } @@ -367,14 +389,14 @@ fn build<'a>( if miswire == Miswire::PreMinted { // The token's admin mints during the token's own setup, then steps back. let holder = Address::generate(&e); - identity.allow(&holder, &true, &compliance_authority); + identity.allow(&holder, &true); share.grant_role(&admin, &symbol_short!("manager"), &admin); share.mint(&holder, &100, &admin); share.revoke_role(&admin, &symbol_short!("manager"), &admin); } // Converting a delisted holder's deposit mints into the vault's own escrow. if miswire != Miswire::NotAllowlisted { - identity.allow(&contract_id, &true, &compliance_authority); + identity.allow(&contract_id, &true); } if miswire == Miswire::VaultFrozen { // Until `bind_vault`, the token does not know this address is its vault. @@ -383,43 +405,43 @@ fn build<'a>( match miswire { Miswire::TokenUnbound => {} Miswire::TokenBoundElsewhere => { - share.bind_vault(®ister_with(&e, distinct_roles(&e)), &admin); - share.finish_setup(&admin); + share.bind_vault(®ister_with(&e, distinct_roles(&e))); + share.finish_setup(); } - Miswire::TokenSetupOpen => share.bind_vault(&contract_id, &admin), + Miswire::TokenSetupOpen => share.bind_vault(&contract_id), _ => { - share.bind_vault(&contract_id, &admin); - share.finish_setup(&admin); + share.bind_vault(&contract_id); + share.finish_setup(); } } match miswire { Miswire::OracleUnbound => {} Miswire::OracleBoundElsewhere => { - oracle.bind_vault(®ister_with(&e, distinct_roles(&e)), &admin); - oracle.finish_setup(&admin); + oracle.bind_vault(®ister_with(&e, distinct_roles(&e))); + oracle.finish_setup(); } - Miswire::OracleSetupOpen => oracle.bind_vault(&contract_id, &admin), + Miswire::OracleSetupOpen => oracle.bind_vault(&contract_id), _ => { - oracle.bind_vault(&contract_id, &admin); - oracle.finish_setup(&admin); + oracle.bind_vault(&contract_id); + oracle.finish_setup(); } } match miswire { Miswire::ComplianceUnbound => {} Miswire::ComplianceBoundElsewhere => { - compliance.bind_vault(®ister_with(&e, distinct_roles(&e)), &admin); - compliance.finish_setup(&admin); + compliance.bind_vault(®ister_with(&e, distinct_roles(&e))); + compliance.finish_setup(); } - Miswire::ComplianceSetupOpen => compliance.bind_vault(&contract_id, &admin), + Miswire::ComplianceSetupOpen => compliance.bind_vault(&contract_id), _ => { - compliance.bind_vault(&contract_id, &admin); - compliance.finish_setup(&admin); + compliance.bind_vault(&contract_id); + compliance.finish_setup(); } } let vault = AsyncVaultClient::new(&e, &contract_id); if finish { - vault.set_custodian(&custodian, &admin); - vault.finish_setup(&admin); + vault.set_custodian(&custodian); + vault.finish_setup(); } Fixture { diff --git a/contracts/async-vault/src/test/notice.rs b/contracts/async-vault/src/test/notice.rs index 4f33a0aa..893450e1 100644 --- a/contracts/async-vault/src/test/notice.rs +++ b/contracts/async-vault/src/test/notice.rs @@ -11,7 +11,7 @@ fn a_fresh_vault_has_no_notice() { fn governance_sets_the_notice() { let f = setup(); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); assert_eq!(f.vault.notice(), 3_600); } @@ -21,11 +21,11 @@ fn setting_the_notice_needs_governance_authorisation() { let f = setup(); f.e.set_auths(&[]); - assert!(f.vault.try_set_notice(&3_600, &f.admin).is_err()); + assert!(f.vault.try_set_notice(&3_600).is_err()); assert_eq!(f.vault.notice(), 0); f.e.mock_all_auths(); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); assert_eq!(f.vault.notice(), 3_600); } @@ -41,7 +41,7 @@ fn an_open_epoch_has_no_timestamps() { #[test] fn closing_records_the_moment_and_the_wait() { let f = setup(); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); let closed_at = f.e.ledger().timestamp(); let id = f.close_epoch(); @@ -54,11 +54,11 @@ fn closing_records_the_moment_and_the_wait() { #[test] fn a_later_notice_does_not_move_an_epoch_that_closed() { let f = setup(); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); let id = f.close_epoch(); let before = f.vault.get_epoch(&id).unwrap().priceable_at; - f.vault.set_notice(&DAY_DURATION, &f.admin); + f.vault.set_notice(&DAY_DURATION); assert_eq!(f.vault.get_epoch(&id).unwrap().priceable_at, before); } @@ -67,7 +67,7 @@ fn a_later_notice_does_not_move_an_epoch_that_closed() { fn an_epoch_cannot_be_priced_inside_its_notice() { let f = setup(); let holder = f.holder(100); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); f.vault.request_redeem(&holder, &100); let epoch = f.close_epoch(); f.attest(wad(2)); @@ -82,7 +82,7 @@ fn an_epoch_cannot_be_priced_inside_its_notice() { fn it_is_priced_once_the_notice_elapses() { let f = setup(); let holder = f.holder(100); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); f.vault.request_redeem(&holder, &100); let epoch = f.close_epoch(); @@ -110,7 +110,7 @@ fn a_notice_of_zero_prices_as_soon_as_the_valuation_lands() { fn the_notice_is_reported_before_the_valuation() { let f = setup(); let holder = f.holder(100); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); f.vault.request_redeem(&holder, &100); f.advance(60); let epoch = f.close_epoch(); @@ -127,7 +127,7 @@ fn the_notice_is_reported_before_the_valuation() { fn the_notice_does_not_reopen_the_cancellation_window() { let f = setup(); let investor = f.investor(1_000); - f.vault.set_notice(&3_600, &f.admin); + f.vault.set_notice(&3_600); f.vault.request_deposit(&investor, &1_000); let epoch = f.close_epoch(); f.attest(wad(2)); @@ -149,7 +149,7 @@ fn a_notice_beyond_the_maximum_is_refused() { let f = setup(); refused( - f.vault.try_set_notice(&(MAX_NOTICE_SECS + 1), &f.admin), + f.vault.try_set_notice(&(MAX_NOTICE_SECS + 1)), VaultError::NoticeTooLong, ); assert_eq!(f.vault.notice(), 0); @@ -160,7 +160,7 @@ fn a_notice_above_the_upgrade_delay_is_refused() { let f = setup(); refused( - f.vault.try_set_notice(&(MIN_UPGRADE_DELAY + 1), &f.admin), + f.vault.try_set_notice(&(MIN_UPGRADE_DELAY + 1)), VaultError::NoticeAboveUpgradeDelay, ); assert_eq!(f.vault.notice(), 0); @@ -173,11 +173,11 @@ fn a_notice_at_the_maximum_is_accepted() { // The delay must be raised first, since notice cannot outgrow the upgrade delay. let id = f .vault - .propose_change(&VaultChange::UpgradeDelay(MAX_NOTICE_SECS), &f.admin); + .propose_change(&VaultChange::UpgradeDelay(MAX_NOTICE_SECS)); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); - f.vault.set_notice(&MAX_NOTICE_SECS, &f.admin); + f.vault.set_notice(&MAX_NOTICE_SECS); assert_eq!(f.vault.notice(), MAX_NOTICE_SECS); } diff --git a/contracts/async-vault/src/test/oracle_pricing.rs b/contracts/async-vault/src/test/oracle_pricing.rs index 32a7773e..427e6b50 100644 --- a/contracts/async-vault/src/test/oracle_pricing.rs +++ b/contracts/async-vault/src/test/oracle_pricing.rs @@ -14,11 +14,11 @@ fn fulfill_is_rejected_while_the_nav_is_paused() { let f = setup(); f.close_epoch(); f.attest(wad(2)); - f.oracle.set_nav_paused(&true, &f.admin); + f.oracle.set_nav_paused(&true); refused(f.vault.try_fulfill_epoch(&1), VaultError::FeedNotValid); - f.oracle.set_nav_paused(&false, &f.admin); + f.oracle.set_nav_paused(&false); assert_eq!(f.vault.fulfill_epoch(&1), wad(2)); } diff --git a/contracts/async-vault/src/test/oracle_swap.rs b/contracts/async-vault/src/test/oracle_swap.rs index a383343c..2748ea0a 100644 --- a/contracts/async-vault/src/test/oracle_swap.rs +++ b/contracts/async-vault/src/test/oracle_swap.rs @@ -48,8 +48,8 @@ fn unbound_feed<'a>(f: &Fixture<'a>) -> NavOracleContractClient<'a> { fn second_feed<'a>(f: &Fixture<'a>) -> NavOracleContractClient<'a> { let feed = unbound_feed(f); - feed.bind_vault(&f.vault.address, &f.admin); - feed.finish_setup(&f.admin); + feed.bind_vault(&f.vault.address); + feed.finish_setup(); feed } @@ -71,16 +71,13 @@ fn the_oracle_changes_only_through_the_timelock() { let id = f .vault - .propose_change(&VaultChange::Oracle(next.address.clone()), &f.admin); + .propose_change(&VaultChange::Oracle(next.address.clone())); f.advance(MIN_UPGRADE_DELAY - 1); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::ChangeNotReady, - ); + refused(f.vault.try_apply_change(&id), VaultError::ChangeNotReady); assert_eq!(f.vault.oracle(), f.oracle.address); f.advance(1); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!(f.vault.oracle(), next.address); } @@ -90,9 +87,9 @@ fn the_next_epoch_prices_from_the_new_feed() { let next = second_feed(&f); let id = f .vault - .propose_change(&VaultChange::Oracle(next.address.clone()), &f.admin); + .propose_change(&VaultChange::Oracle(next.address.clone())); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); let investor = f.investor(1_000); f.vault.request_deposit(&investor, &400); let epoch = f.close_epoch(); @@ -114,9 +111,9 @@ fn a_sealed_epoch_is_priced_by_the_feed_standing_at_fulfilment() { let id = f .vault - .propose_change(&VaultChange::Oracle(next.address.clone()), &f.admin); + .propose_change(&VaultChange::Oracle(next.address.clone())); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); attest_on(&f, &next, wad(4)); f.vault.fulfill_epoch(&epoch); @@ -127,10 +124,10 @@ fn a_sealed_epoch_is_priced_by_the_feed_standing_at_fulfilment() { fn a_feed_outside_this_vault_is_never_proposed() { let f = setup(); let open = unbound_feed(&f); - open.bind_vault(&f.vault.address, &f.admin); + open.bind_vault(&f.vault.address); let elsewhere = unbound_feed(&f); - elsewhere.bind_vault(®ister_with(&f.e, distinct_roles(&f.e)), &f.admin); - elsewhere.finish_setup(&f.admin); + elsewhere.bind_vault(®ister_with(&f.e, distinct_roles(&f.e))); + elsewhere.finish_setup(); for feed in [ Address::generate(&f.e), @@ -139,8 +136,7 @@ fn a_feed_outside_this_vault_is_never_proposed() { elsewhere.address, ] { refused( - f.vault - .try_propose_change(&VaultChange::Oracle(feed), &f.admin), + f.vault.try_propose_change(&VaultChange::Oracle(feed)), VaultError::FeedNotBound, ); } @@ -154,14 +150,11 @@ fn a_feed_that_stops_naming_this_vault_is_never_installed() { stub.report(&Some(f.vault.address.clone())); let id = f .vault - .propose_change(&VaultChange::Oracle(stub.address.clone()), &f.admin); + .propose_change(&VaultChange::Oracle(stub.address.clone())); stub.report(&Some(Address::generate(&f.e))); f.advance(MIN_UPGRADE_DELAY); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::FeedNotBound, - ); + refused(f.vault.try_apply_change(&id), VaultError::FeedNotBound); assert_eq!(f.vault.oracle(), f.oracle.address); } diff --git a/contracts/async-vault/src/test/settlement.rs b/contracts/async-vault/src/test/settlement.rs index 9dfd4288..ad3d15e9 100644 --- a/contracts/async-vault/src/test/settlement.rs +++ b/contracts/async-vault/src/test/settlement.rs @@ -193,7 +193,7 @@ fn a_request_that_cannot_settle_keeps_its_epoch_open() { let epoch = f.vault.request_redeem(&a, &1); f.vault.request_redeem(&b, &3); f.fulfill_epoch(wad(1) / 2); - f.identity.allow(&a, &false, &f.compliance); + f.identity.allow(&a, &false); refused( f.vault.try_claim_redeem(&a, &epoch), diff --git a/contracts/async-vault/src/test/setup.rs b/contracts/async-vault/src/test/setup.rs index 3dad44f7..2221ab8d 100644 --- a/contracts/async-vault/src/test/setup.rs +++ b/contracts/async-vault/src/test/setup.rs @@ -1,4 +1,4 @@ -use soroban_sdk::{contract, contractimpl, testutils::Events as _, vec, Event as _, IntoVal, Vec}; +use soroban_sdk::{contract, contractimpl, testutils::Events as _, vec, Event as _, Vec}; use stellar_access::role_transfer::RoleTransferError; use super::*; @@ -20,7 +20,7 @@ fn a_deposit_before_setup_finishes_is_refused() { fn setup_finishes_once_the_wiring_is_complete() { let f = setup_unfinished(); - f.vault.finish_setup(&f.admin); + f.vault.finish_setup(); assert!(!f.vault.setup_open()); let investor = f.investor(1_000); @@ -31,7 +31,7 @@ fn setup_finishes_once_the_wiring_is_complete() { fn finishing_setup_publishes_it() { let f = setup_unfinished(); - f.vault.finish_setup(&f.admin); + f.vault.finish_setup(); assert_eq!( f.e.events().all().filter_by_contract(&f.vault.address), @@ -43,7 +43,7 @@ fn finishing_setup_publishes_it() { fn setup_never_finishes_twice() { let f = setup(); - refused(f.vault.try_finish_setup(&f.admin), VaultError::SetupClosed); + refused(f.vault.try_finish_setup(), VaultError::SetupClosed); } #[test] @@ -68,7 +68,7 @@ fn every_wiring_step_is_checked_before_setup_finishes() { let f = miswired(miswire); assert_eq!( - f.vault.try_finish_setup(&f.admin), + f.vault.try_finish_setup(), Err(Ok(VaultError::SetupIncomplete.into())), "{miswire:?}" ); @@ -160,8 +160,8 @@ fn a_token_that_cannot_answer_fails_its_checks_instead_of_trapping() { }, ), ); - oracle.bind_vault(&contract_id, &admin); - oracle.finish_setup(&admin); + oracle.bind_vault(&contract_id); + oracle.finish_setup(); let vault = AsyncVaultClient::new(&e, &contract_id); @@ -173,7 +173,7 @@ fn a_token_that_cannot_answer_fails_its_checks_instead_of_trapping() { assert!(!health.vault_unfrozen); assert!(!health.compliance_bound); - refused(vault.try_finish_setup(&admin), VaultError::SetupIncomplete); + refused(vault.try_finish_setup(), VaultError::SetupIncomplete); } #[test] @@ -183,13 +183,10 @@ fn setup_cannot_finish_while_a_hand_over_is_pending() { f.vault .transfer_admin_role(&next, &(f.e.ledger().sequence() + 100)); - refused( - f.vault.try_finish_setup(&f.admin), - VaultError::AdminTransferPending, - ); + refused(f.vault.try_finish_setup(), VaultError::AdminTransferPending); f.vault.transfer_admin_role(&next, &0); - f.vault.finish_setup(&f.admin); + f.vault.finish_setup(); assert!(f.vault.try_accept_admin_transfer().is_err()); assert_eq!(f.vault.governance(), Some(f.admin.clone())); } @@ -201,7 +198,7 @@ fn an_expired_hand_over_does_not_hold_setup_open() { .transfer_admin_role(&Address::generate(&f.e), &(f.e.ledger().sequence() + 10)); f.e.ledger().with_mut(|l| l.sequence_number += 11); - f.vault.finish_setup(&f.admin); + f.vault.finish_setup(); assert!(!f.vault.setup_open()); assert_eq!( @@ -217,8 +214,8 @@ fn only_governance_finishes_setup() { let stranger = Address::generate(&f.e); for who in [&stranger, &f.manager] { - f.sign_only_as(who, "finish_setup", (who.clone(),).into_val(&f.e)); - assert!(f.vault.try_finish_setup(who).is_err()); + f.sign_only_as(who, "finish_setup", Vec::new(&f.e)); + assert!(f.vault.try_finish_setup().is_err()); } assert!(f.vault.setup_open()); } @@ -228,12 +225,11 @@ fn admin_changes_are_instant_during_setup() { let f = setup_unfinished(); let next = Address::generate(&f.e); - f.vault.set_custodian(&f.custodian, &f.admin); - f.vault.set_notice(&3_600, &f.admin); - f.vault.set_notice(&0, &f.admin); - f.vault - .set_wind_down_delay(&(2 * MIN_WIND_DOWN_DELAY), &f.admin); - f.vault.set_wind_down_delay(&MIN_WIND_DOWN_DELAY, &f.admin); + f.vault.set_custodian(&f.custodian); + f.vault.set_notice(&3_600); + f.vault.set_notice(&0); + f.vault.set_wind_down_delay(&(2 * MIN_WIND_DOWN_DELAY)); + f.vault.set_wind_down_delay(&MIN_WIND_DOWN_DELAY); f.vault .grant_role(&next, &symbol_short!("guardian"), &f.admin); f.vault @@ -255,14 +251,12 @@ fn nothing_is_proposed_during_setup() { refused( f.vault - .try_propose_change(&VaultChange::Custodian(f.custodian.clone()), &f.admin), + .try_propose_change(&VaultChange::Custodian(f.custodian.clone())), VaultError::SetupOpen, ); refused( - f.vault.try_propose_change( - &VaultChange::WindDownDelay(MIN_WIND_DOWN_DELAY - 1), - &f.admin, - ), + f.vault + .try_propose_change(&VaultChange::WindDownDelay(MIN_WIND_DOWN_DELAY - 1)), VaultError::SetupOpen, ); assert_eq!(f.vault.pending_changes().len(), 0); diff --git a/contracts/async-vault/src/test/share_returns.rs b/contracts/async-vault/src/test/share_returns.rs index 799d38a2..bba8dbc5 100644 --- a/contracts/async-vault/src/test/share_returns.rs +++ b/contracts/async-vault/src/test/share_returns.rs @@ -26,7 +26,7 @@ fn a_delisted_vault_still_returns_escrowed_shares() { let holder = f.holder(500); let epoch = f.vault.request_redeem(&holder, &200); - f.identity.allow(&f.vault.address, &false, &f.compliance); + f.identity.allow(&f.vault.address, &false); assert!(!f.vault.health().allowlisted); assert_eq!(f.vault.cancel_redeem(&holder, &epoch), 200); @@ -38,7 +38,7 @@ fn a_delisted_holder_cannot_take_shares_back() { let f = setup(); let holder = f.holder(500); let epoch = f.vault.request_redeem(&holder, &200); - f.identity.allow(&holder, &false, &f.compliance); + f.identity.allow(&holder, &false); refused( f.vault.try_cancel_redeem(&holder, &epoch), @@ -127,7 +127,7 @@ fn a_delisted_holder_with_a_worthless_exit_keeps_the_request() { let epoch = f.vault.request_redeem(&holder, &1); // Priced at half a unit per share, one share pays nothing. f.fulfill_epoch(WAD_SCALE / 2); - f.identity.allow(&holder, &false, &f.compliance); + f.identity.allow(&holder, &false); refused( f.vault.try_claim_redeem(&holder, &epoch), diff --git a/contracts/async-vault/src/test/signatures.rs b/contracts/async-vault/src/test/signatures.rs new file mode 100644 index 00000000..48e6c8e8 --- /dev/null +++ b/contracts/async-vault/src/test/signatures.rs @@ -0,0 +1,118 @@ +//! What each privileged call asks to sign: one signature per account it needs. + +use soroban_sdk::{IntoVal, Vec}; + +use super::*; + +#[test] +fn each_setup_call_asks_the_admin_for_one_signature() { + let f = setup_unfinished(); + let holder = Address::generate(&f.e); + let treasury = symbol_short!("treasury"); + let delay = 2 * MIN_WIND_DOWN_DELAY; + let live_until = f.e.ledger().sequence() + 100; + + f.vault.set_custodian(&f.custodian); + f.signed_once_by( + &f.admin, + "set_custodian", + (f.custodian.clone(),).into_val(&f.e), + ); + f.vault.set_notice(&3_600); + f.signed_once_by(&f.admin, "set_notice", (3_600_u64,).into_val(&f.e)); + f.vault.set_wind_down_delay(&delay); + f.signed_once_by(&f.admin, "set_wind_down_delay", (delay,).into_val(&f.e)); + f.vault.set_deposit_cap(&Some(1_000)); + f.signed_once_by( + &f.admin, + "set_deposit_cap", + (Some(1_000_i128),).into_val(&f.e), + ); + f.vault.grant_role(&holder, &treasury, &f.admin); + f.signed_once_by( + &f.admin, + "grant_role", + (holder.clone(), treasury.clone(), f.admin.clone()).into_val(&f.e), + ); + f.vault.revoke_role(&holder, &treasury, &f.admin); + f.signed_once_by( + &f.admin, + "revoke_role", + (holder.clone(), treasury, f.admin.clone()).into_val(&f.e), + ); + f.vault.transfer_admin_role(&holder, &live_until); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (holder.clone(), live_until).into_val(&f.e), + ); + f.vault.transfer_admin_role(&holder, &0); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (holder, 0_u32).into_val(&f.e), + ); + f.vault.finish_setup(); + f.signed_once_by(&f.admin, "finish_setup", Vec::new(&f.e)); +} + +#[test] +fn each_admin_call_after_setup_asks_the_admin_for_one_signature() { + let f = setup(); + let next = Address::generate(&f.e); + + let change = VaultChange::Custodian(next.clone()); + let id = f.vault.propose_change(&change); + f.signed_once_by(&f.admin, "propose_change", (change,).into_val(&f.e)); + f.vault.cancel_change(&id); + f.signed_once_by(&f.admin, "cancel_change", (id,).into_val(&f.e)); + + let id = f.vault.propose_change(&VaultChange::Admin(next.clone())); + f.advance(MIN_UPGRADE_DELAY); + f.vault.apply_change(&id); + f.signed_once_by(&f.admin, "apply_change", (id,).into_val(&f.e)); + + f.vault.pause(&f.guardian); + f.vault.unpause(&f.admin); + f.signed_once_by(&f.admin, "unpause", (f.admin.clone(),).into_val(&f.e)); + + f.vault.propose_wind_down(); + f.signed_once_by(&f.admin, "propose_wind_down", Vec::new(&f.e)); + f.vault.cancel_wind_down_proposal(); + f.signed_once_by(&f.admin, "cancel_wind_down_proposal", Vec::new(&f.e)); + + f.vault.accept_admin_transfer(); + f.signed_once_by(&next, "accept_admin_transfer", Vec::new(&f.e)); +} + +#[test] +fn each_role_call_asks_its_holder_for_one_signature() { + let f = setup(); + let investor = f.investor(1_000); + f.vault.request_deposit(&investor, &400); + + let epoch = f.vault.close_epoch(&f.manager); + f.signed_once_by( + &f.manager, + "close_epoch", + (f.manager.clone(),).into_val(&f.e), + ); + f.fulfill_epoch_at(epoch, wad(2)); + let free = f.vault.free_reserve(); + f.vault.deploy_to_custodian(&f.treasury, &free); + f.signed_once_by( + &f.treasury, + "deploy_to_custodian", + (f.treasury.clone(), free).into_val(&f.e), + ); + f.vault.pause(&f.guardian); + f.signed_once_by(&f.guardian, "pause", (f.guardian.clone(),).into_val(&f.e)); + + let guardian = symbol_short!("guardian"); + f.vault.renounce_role(&guardian, &f.guardian); + f.signed_once_by( + &f.guardian, + "renounce_role", + (guardian, f.guardian.clone()).into_val(&f.e), + ); +} diff --git a/contracts/async-vault/src/test/treasury.rs b/contracts/async-vault/src/test/treasury.rs index a3823b2d..c81af58a 100644 --- a/contracts/async-vault/src/test/treasury.rs +++ b/contracts/async-vault/src/test/treasury.rs @@ -1,3 +1,5 @@ +use soroban_sdk::IntoVal; + use super::*; #[test] @@ -115,11 +117,13 @@ fn setting_the_custodian_needs_the_admin_signature() { let f = setup_unfinished(); f.e.set_auths(&[]); - assert!(f.vault.try_set_custodian(&f.custodian, &f.admin).is_err()); - assert!(f - .vault - .try_set_custodian(&f.custodian, &f.treasury) - .is_err()); + assert!(f.vault.try_set_custodian(&f.custodian).is_err()); + f.sign_only_as( + &f.treasury, + "set_custodian", + (f.custodian.clone(),).into_val(&f.e), + ); + assert!(f.vault.try_set_custodian(&f.custodian).is_err()); assert_eq!(f.vault.custodian(), None); } diff --git a/contracts/async-vault/src/test/upgrade.rs b/contracts/async-vault/src/test/upgrade.rs index 5c4ed548..5f3afeda 100644 --- a/contracts/async-vault/src/test/upgrade.rs +++ b/contracts/async-vault/src/test/upgrade.rs @@ -1,4 +1,4 @@ -use soroban_sdk::{testutils::Events as _, Event as _}; +use soroban_sdk::{testutils::Events as _, Event as _, IntoVal}; use stellar_contract_utils::pausable::PausableError; use super::*; @@ -9,14 +9,14 @@ fn wasm(f: &Fixture, byte: u8) -> BytesN<32> { } fn propose(f: &Fixture, change: VaultChange) -> u32 { - f.vault.propose_change(&change, &f.admin) + f.vault.propose_change(&change) } /// Raises the upgrade delay through the timelock, which costs one current delay. fn raise_delay(f: &Fixture, secs: u64) { let id = propose(f, VaultChange::UpgradeDelay(secs)); f.advance(f.vault.upgrade_delay()); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); } fn only_pending(f: &Fixture) -> VaultPendingChange { @@ -52,12 +52,12 @@ fn a_second_code_change_while_one_stands_is_refused() { refused( f.vault - .try_propose_change(&VaultChange::Upgrade(wasm(&f, 9)), &f.admin), + .try_propose_change(&VaultChange::Upgrade(wasm(&f, 9))), VaultError::UpgradeProposalExists, ); refused( f.vault - .try_propose_change(&VaultChange::UpgradeDelay(30 * DAY_DURATION), &f.admin), + .try_propose_change(&VaultChange::UpgradeDelay(30 * DAY_DURATION)), VaultError::UpgradeProposalExists, ); } @@ -68,7 +68,7 @@ fn a_cancelled_proposal_leaves_the_slot_free() { let hash = wasm(&f, 7); let id = propose(&f, VaultChange::Upgrade(hash.clone())); - f.vault.cancel_change(&id, &f.admin); + f.vault.cancel_change(&id); assert_eq!(f.vault.pending_changes().len(), 0); propose(&f, VaultChange::Upgrade(hash)); @@ -79,10 +79,7 @@ fn a_cancelled_proposal_leaves_the_slot_free() { fn cancelling_nothing_is_refused() { let f = setup(); - refused( - f.vault.try_cancel_change(&1, &f.admin), - VaultError::ChangeNotFound, - ); + refused(f.vault.try_cancel_change(&1), VaultError::ChangeNotFound); } #[test] @@ -91,7 +88,7 @@ fn a_delay_below_the_minimum_is_refused() { refused( f.vault - .try_propose_change(&VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY - 1), &f.admin), + .try_propose_change(&VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY - 1)), VaultError::UpgradeDelayTooShort, ); assert_eq!(f.vault.pending_changes().len(), 0); @@ -103,7 +100,7 @@ fn a_delay_above_the_maximum_is_refused() { refused( f.vault - .try_propose_change(&VaultChange::UpgradeDelay(MAX_UPGRADE_DELAY + 1), &f.admin), + .try_propose_change(&VaultChange::UpgradeDelay(MAX_UPGRADE_DELAY + 1)), VaultError::UpgradeDelayTooLong, ); assert_eq!(f.vault.pending_changes().len(), 0); @@ -114,12 +111,12 @@ fn a_delay_below_the_standing_notice_is_refused() { let f = setup(); raise_delay(&f, 30 * DAY_DURATION); - f.vault.set_notice(&(20 * DAY_DURATION), &f.admin); + f.vault.set_notice(&(20 * DAY_DURATION)); // Above the minimum, below the notice: an exit could not complete. refused( f.vault - .try_propose_change(&VaultChange::UpgradeDelay(10 * DAY_DURATION), &f.admin), + .try_propose_change(&VaultChange::UpgradeDelay(10 * DAY_DURATION)), VaultError::UpgradeDelayBelowNotice, ); @@ -133,20 +130,24 @@ fn only_governance_proposes_and_cancels() { let change = VaultChange::Upgrade(wasm(&f, 7)); let stranger = Address::generate(&f.e); - f.e.set_auths(&[]); - assert!(f.vault.try_propose_change(&change, &stranger).is_err()); - assert!(f.vault.try_propose_change(&change, &f.manager).is_err()); - assert!(f.vault.try_propose_change(&change, &f.guardian).is_err()); - assert!(f - .vault - .try_propose_change(&VaultChange::UpgradeDelay(30 * DAY_DURATION), &f.treasury) - .is_err()); + for who in [&stranger, &f.manager, &f.guardian] { + f.sign_only_as(who, "propose_change", (change.clone(),).into_val(&f.e)); + assert!(f.vault.try_propose_change(&change).is_err()); + } + let delay = VaultChange::UpgradeDelay(30 * DAY_DURATION); + f.sign_only_as( + &f.treasury, + "propose_change", + (delay.clone(),).into_val(&f.e), + ); + assert!(f.vault.try_propose_change(&delay).is_err()); f.e.mock_all_auths(); let id = propose(&f, change); - f.e.set_auths(&[]); - assert!(f.vault.try_cancel_change(&id, &stranger).is_err()); - assert!(f.vault.try_cancel_change(&id, &f.guardian).is_err()); + for who in [&stranger, &f.guardian] { + f.sign_only_as(who, "cancel_change", (id,).into_val(&f.e)); + assert!(f.vault.try_cancel_change(&id).is_err()); + } } #[test] @@ -161,19 +162,13 @@ fn applying_before_the_eta_is_refused() { let f = setup(); let id = propose(&f, VaultChange::UpgradeDelay(30 * DAY_DURATION)); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::ChangeNotReady, - ); + refused(f.vault.try_apply_change(&id), VaultError::ChangeNotReady); f.advance(MIN_UPGRADE_DELAY - 1); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::ChangeNotReady, - ); + refused(f.vault.try_apply_change(&id), VaultError::ChangeNotReady); f.advance(1); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!(f.vault.upgrade_delay(), 30 * DAY_DURATION); assert_eq!(f.vault.pending_changes().len(), 0); } @@ -199,13 +194,10 @@ fn shortening_the_delay_cannot_be_rushed() { // Back down to the minimum: still costs the full thirty days. let id = propose(&f, VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY)); f.advance(30 * DAY_DURATION - 1); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::ChangeNotReady, - ); + refused(f.vault.try_apply_change(&id), VaultError::ChangeNotReady); f.advance(1); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!(f.vault.upgrade_delay(), MIN_UPGRADE_DELAY); } @@ -215,7 +207,7 @@ fn the_notice_cannot_outgrow_the_delay() { propose(&f, VaultChange::UpgradeDelay(10 * DAY_DURATION)); refused( - f.vault.try_set_notice(&(20 * DAY_DURATION), &f.admin), + f.vault.try_set_notice(&(20 * DAY_DURATION)), VaultError::NoticeAboveUpgradeDelay, ); assert_eq!(f.vault.notice(), 0); @@ -225,10 +217,7 @@ fn the_notice_cannot_outgrow_the_delay() { fn applying_nothing_is_refused() { let f = setup(); - refused( - f.vault.try_apply_change(&1, &f.admin), - VaultError::ChangeNotFound, - ); + refused(f.vault.try_apply_change(&1), VaultError::ChangeNotFound); } #[test] @@ -241,7 +230,7 @@ fn a_pause_freezes_the_upgrade_timelock_clock() { f.vault.pause(&f.guardian); f.advance(10 * DAY_DURATION); - assert!(f.vault.try_apply_change(&id, &f.admin).is_err()); + assert!(f.vault.try_apply_change(&id).is_err()); // Unpause: the eta has moved by the ten days spent paused. f.vault.unpause(&f.admin); @@ -249,13 +238,10 @@ fn a_pause_freezes_the_upgrade_timelock_clock() { // Six unpaused days out of seven: still refused. f.advance(3 * DAY_DURATION); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::ChangeNotReady, - ); + refused(f.vault.try_apply_change(&id), VaultError::ChangeNotReady); f.advance(DAY_DURATION); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!(f.vault.pending_changes().len(), 0); assert_eq!(f.vault.upgrade_delay(), 14 * DAY_DURATION); } @@ -269,13 +255,13 @@ fn applying_while_paused_is_refused() { // At the eta, with no paused time yet, only the pause stands in the way. f.vault.pause(&f.guardian); assert_eq!( - f.vault.try_apply_change(&id, &f.admin), + f.vault.try_apply_change(&id), Err(Ok(PausableError::EnforcedPause.into())) ); assert_eq!(f.vault.upgrade_delay(), MIN_UPGRADE_DELAY); f.vault.unpause(&f.admin); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!(f.vault.upgrade_delay(), 30 * DAY_DURATION); } @@ -286,10 +272,10 @@ fn only_governance_applies() { let id = propose(&f, VaultChange::UpgradeDelay(30 * DAY_DURATION)); f.advance(MIN_UPGRADE_DELAY); - f.e.set_auths(&[]); - assert!(f.vault.try_apply_change(&id, &stranger).is_err()); - assert!(f.vault.try_apply_change(&id, &f.manager).is_err()); - assert!(f.vault.try_apply_change(&id, &f.guardian).is_err()); + for who in [&stranger, &f.manager, &f.guardian] { + f.sign_only_as(who, "apply_change", (id,).into_val(&f.e)); + assert!(f.vault.try_apply_change(&id).is_err()); + } assert_eq!(f.vault.pending_changes().len(), 1); } @@ -324,13 +310,13 @@ fn the_vault_exposes_no_immediate_upgrade() { #[test] fn nothing_is_proposed_during_a_wind_down() { let f = setup(); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); refused( f.vault - .try_propose_change(&VaultChange::Upgrade(wasm(&f, 7)), &f.admin), + .try_propose_change(&VaultChange::Upgrade(wasm(&f, 7))), VaultError::WindDownActive, ); } @@ -339,16 +325,13 @@ fn nothing_is_proposed_during_a_wind_down() { fn a_change_standing_when_the_wind_down_starts_never_applies() { let f = setup(); let id = propose(&f, VaultChange::Upgrade(wasm(&f, 7))); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); f.advance(MAX_UPGRADE_DELAY); - refused( - f.vault.try_apply_change(&id, &f.admin), - VaultError::WindDownActive, - ); - f.vault.cancel_change(&id, &f.admin); + refused(f.vault.try_apply_change(&id), VaultError::WindDownActive); + f.vault.cancel_change(&id); assert_eq!(f.vault.pending_changes().len(), 0); } @@ -359,11 +342,11 @@ fn applying_a_delay_change_checks_the_notice_again() { let id = propose(&f, VaultChange::UpgradeDelay(MIN_UPGRADE_DELAY)); // The notice rises while the shorter delay waits. - f.vault.set_notice(&(20 * DAY_DURATION), &f.admin); + f.vault.set_notice(&(20 * DAY_DURATION)); f.advance(30 * DAY_DURATION); refused( - f.vault.try_apply_change(&id, &f.admin), + f.vault.try_apply_change(&id), VaultError::UpgradeDelayBelowNotice, ); assert_eq!(f.vault.upgrade_delay(), 30 * DAY_DURATION); @@ -387,7 +370,7 @@ fn proposing_and_applying_publish_the_change() { ); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); assert_eq!( f.e.events().all().filter_by_contract(&f.vault.address), [ChangeApplied { id, change }.to_xdr(&f.e, &f.vault.address)] @@ -399,7 +382,7 @@ fn cancelling_publishes_the_change_id() { let f = setup(); let id = propose(&f, VaultChange::Upgrade(wasm(&f, 7))); - f.vault.cancel_change(&id, &f.admin); + f.vault.cancel_change(&id); assert_eq!( f.e.events().all().filter_by_contract(&f.vault.address), @@ -502,19 +485,17 @@ mod with_wasm { assert_eq!(f.vault.request_deposit(&depositor, &500), epoch); let probe_hash = f.e.deployer().upload_contract_wasm(token_probe::WASM); - let id = f - .share - .propose_change(&TokenChange::Upgrade(probe_hash), &f.admin); + let id = f.share.propose_change(&TokenChange::Upgrade(probe_hash)); f.advance(MIN_UPGRADE_DELAY); - f.share.apply_change(&id, &f.admin); + f.share.apply_change(&id); let probe = token_probe::Client::new(&f.e, &f.share.address); assert!(probe.probe()); assert_eq!(probe.schema_version(), 1); - probe.migrate(&f.admin); + probe.migrate(); assert_eq!(probe.schema_version(), 2); assert_eq!( - probe.try_migrate(&f.admin), + probe.try_migrate(), Err(Ok(token_probe::ProbeError::AlreadyMigrated.into())) ); @@ -554,11 +535,9 @@ mod with_wasm { let hash = f.e.deployer().upload_contract_wasm(WASM); assert_eq!(f.vault.schema_version(), 1); - let id = f - .vault - .propose_change(&VaultChange::Upgrade(hash), &f.admin); + let id = f.vault.propose_change(&VaultChange::Upgrade(hash)); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); let target = Client::new(&f.e, &f.vault.address); assert_eq!(target.schema_version(), 1); @@ -571,11 +550,9 @@ mod with_wasm { fn a_migration_refuses_a_second_run() { let f = setup(); let hash = f.e.deployer().upload_contract_wasm(WASM); - let id = f - .vault - .propose_change(&VaultChange::Upgrade(hash), &f.admin); + let id = f.vault.propose_change(&VaultChange::Upgrade(hash)); f.advance(MIN_UPGRADE_DELAY); - f.vault.apply_change(&id, &f.admin); + f.vault.apply_change(&id); let target = Client::new(&f.e, &f.vault.address); target.migrate(&f.admin); diff --git a/contracts/async-vault/src/test/wind_down.rs b/contracts/async-vault/src/test/wind_down.rs index d8a28c77..fee93d15 100644 --- a/contracts/async-vault/src/test/wind_down.rs +++ b/contracts/async-vault/src/test/wind_down.rs @@ -9,7 +9,7 @@ fn governance_sets_the_delay_within_the_maximum() { assert_eq!(f.vault.wind_down_delay(), MIN_WIND_DOWN_DELAY); - f.vault.set_wind_down_delay(&(14 * DAY_DURATION), &f.admin); + f.vault.set_wind_down_delay(&(14 * DAY_DURATION)); assert_eq!(f.vault.wind_down_delay(), 14 * DAY_DURATION); } @@ -19,7 +19,7 @@ fn the_delay_cannot_exceed_the_maximum() { assert!(f .vault - .try_set_wind_down_delay(&(MAX_WIND_DOWN_DELAY + 1), &f.admin) + .try_set_wind_down_delay(&(MAX_WIND_DOWN_DELAY + 1)) .is_err()); assert_eq!(f.vault.wind_down_delay(), MIN_WIND_DOWN_DELAY); } @@ -31,12 +31,8 @@ fn only_governance_sets_the_delay() { let longer = 2 * MIN_WIND_DOWN_DELAY; for who in [&stranger, &f.manager, &f.treasury, &f.guardian] { - f.sign_only_as( - who, - "set_wind_down_delay", - (longer, who.clone()).into_val(&f.e), - ); - assert!(f.vault.try_set_wind_down_delay(&longer, who).is_err()); + f.sign_only_as(who, "set_wind_down_delay", (longer,).into_val(&f.e)); + assert!(f.vault.try_set_wind_down_delay(&longer).is_err()); } assert_eq!(f.vault.wind_down_delay(), MIN_WIND_DOWN_DELAY); } @@ -54,9 +50,9 @@ const WEEK: u64 = 7 * DAY_DURATION; fn governance_proposes_and_anyone_activates_after_the_delay() { let f = setup(); let stranger = Address::generate(&f.e); - f.vault.set_wind_down_delay(&WEEK, &f.admin); + f.vault.set_wind_down_delay(&WEEK); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); let proposed = f.vault.wind_down().unwrap(); assert_eq!(proposed.status, WindDownStatus::Proposed); assert_eq!(proposed.round, 0); @@ -73,8 +69,8 @@ fn governance_proposes_and_anyone_activates_after_the_delay() { #[test] fn activation_is_open_to_a_stranger() { let f = setup(); - f.vault.set_wind_down_delay(&WEEK, &f.admin); - f.vault.propose_wind_down(&f.admin); + f.vault.set_wind_down_delay(&WEEK); + f.vault.propose_wind_down(); f.advance(WEEK); f.e.set_auths(&[]); @@ -88,22 +84,22 @@ fn only_governance_proposes() { let f = setup(); let stranger = Address::generate(&f.e); - f.e.set_auths(&[]); - assert!(f.vault.try_propose_wind_down(&stranger).is_err()); - assert!(f.vault.try_propose_wind_down(&f.manager).is_err()); - assert!(f.vault.try_propose_wind_down(&f.guardian).is_err()); + for who in [&stranger, &f.manager, &f.guardian] { + f.sign_only_as(who, "propose_wind_down", soroban_sdk::Vec::new(&f.e)); + assert!(f.vault.try_propose_wind_down().is_err()); + } assert_eq!(f.vault.wind_down(), None); } #[test] fn a_second_proposal_cannot_push_the_date_back() { let f = setup(); - f.vault.set_wind_down_delay(&WEEK, &f.admin); - f.vault.propose_wind_down(&f.admin); + f.vault.set_wind_down_delay(&WEEK); + f.vault.propose_wind_down(); let first = f.vault.wind_down().unwrap().active_at; f.advance(WEEK / 2); - assert!(f.vault.try_propose_wind_down(&f.admin).is_err()); + assert!(f.vault.try_propose_wind_down().is_err()); assert_eq!(f.vault.wind_down().unwrap().active_at, first); } @@ -111,20 +107,24 @@ fn a_second_proposal_cannot_push_the_date_back() { #[test] fn governance_cancels_a_proposal_before_activation() { let f = setup(); - f.vault.set_wind_down_delay(&WEEK, &f.admin); - f.vault.propose_wind_down(&f.admin); + f.vault.set_wind_down_delay(&WEEK); + f.vault.propose_wind_down(); let stranger = Address::generate(&f.e); - f.e.set_auths(&[]); - assert!(f.vault.try_cancel_wind_down_proposal(&stranger).is_err()); - assert!(f.vault.try_cancel_wind_down_proposal(&f.manager).is_err()); - assert!(f.vault.try_cancel_wind_down_proposal(&f.guardian).is_err()); + for who in [&stranger, &f.manager, &f.guardian] { + f.sign_only_as( + who, + "cancel_wind_down_proposal", + soroban_sdk::Vec::new(&f.e), + ); + assert!(f.vault.try_cancel_wind_down_proposal().is_err()); + } f.e.mock_all_auths(); - f.vault.cancel_wind_down_proposal(&f.admin); + f.vault.cancel_wind_down_proposal(); assert_eq!(f.vault.wind_down(), None); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); assert_eq!( f.vault.wind_down().unwrap().status, WindDownStatus::Proposed @@ -134,12 +134,12 @@ fn governance_cancels_a_proposal_before_activation() { #[test] fn an_active_wind_down_cannot_be_cancelled() { let f = setup(); - f.vault.set_wind_down_delay(&WEEK, &f.admin); - f.vault.propose_wind_down(&f.admin); + f.vault.set_wind_down_delay(&WEEK); + f.vault.propose_wind_down(); f.advance(WEEK); f.vault.activate_wind_down(); - assert!(f.vault.try_cancel_wind_down_proposal(&f.admin).is_err()); + assert!(f.vault.try_cancel_wind_down_proposal().is_err()); assert_eq!(f.vault.wind_down().unwrap().status, WindDownStatus::Active); } @@ -154,7 +154,7 @@ fn activating_without_a_proposal_is_refused() { fn an_unset_delay_still_waits_the_minimum() { let f = setup(); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY - 1); refused( f.vault.try_activate_wind_down(), @@ -170,8 +170,7 @@ fn an_unset_delay_still_waits_the_minimum() { fn a_delay_below_the_minimum_is_refused() { for f in [setup_unfinished(), setup()] { refused( - f.vault - .try_set_wind_down_delay(&(MIN_WIND_DOWN_DELAY - 1), &f.admin), + f.vault.try_set_wind_down_delay(&(MIN_WIND_DOWN_DELAY - 1)), VaultError::WindDownDelayTooShort, ); } @@ -179,7 +178,7 @@ fn a_delay_below_the_minimum_is_refused() { /// Proposes, waits and activates. Returns nothing; the vault is active after. pub(super) fn wind_down_now(f: &Fixture) { - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); } @@ -334,7 +333,7 @@ fn a_round_with_nothing_to_distribute_is_refused() { fn a_round_before_activation_is_refused() { let f = setup(); let _user = f.holder(500); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); assert!(f.vault.try_finalize_wind_down_round().is_err()); } @@ -688,11 +687,7 @@ fn propose_wind_down_reports_active_when_active() { let _user = f.holder(500); wind_down_now(&f); - let err = f - .vault - .try_propose_wind_down(&f.admin) - .unwrap_err() - .unwrap(); + let err = f.vault.try_propose_wind_down().unwrap_err().unwrap(); assert_eq!(err, VaultError::WindDownActive.into()); } @@ -705,7 +700,7 @@ fn a_later_round_needs_the_units_an_earlier_round_left_in_owed() { let a = f.holder(1); let b = f.holder(1); f.empty_reserve(); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); @@ -729,7 +724,7 @@ fn after_every_holder_claims_owed_keeps_at_most_one_unit_per_holder() { let f = setup(); let holders = [f.holder(1), f.holder(2), f.holder(4)]; f.empty_reserve(); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); f.vault.fund(&f.custodian, &5); @@ -761,7 +756,7 @@ proptest! { let total_shares: i128 = holdings.iter().sum(); let backer = f.investor(total_shares + 10_000); f.empty_reserve(); - f.vault.propose_wind_down(&f.admin); + f.vault.propose_wind_down(); f.advance(MIN_WIND_DOWN_DELAY); f.vault.activate_wind_down(); diff --git a/contracts/compliance/src/lib.rs b/contracts/compliance/src/lib.rs index bb61ec4d..34f9b4d0 100644 --- a/contracts/compliance/src/lib.rs +++ b/contracts/compliance/src/lib.rs @@ -97,7 +97,7 @@ impl Compliance { /// Names the vault whose clock this contract's timelock reads. Once, and /// only during setup. #[only_admin] - pub fn bind_vault(e: &Env, vault: Address, _caller: Address) { + pub fn bind_vault(e: &Env, vault: Address) { strata_storage::bump_instance(e); governance::bind_vault(e, &vault); } @@ -105,20 +105,20 @@ impl Compliance { /// Ends the window in which module and binding changes are instant. It /// never reopens. #[only_admin] - pub fn finish_setup(e: &Env, _caller: Address) { + pub fn finish_setup(e: &Env) { strata_storage::bump_instance(e); governance::finish_setup(e); } /// Queues a change behind the vault's timelock and returns its id. #[only_admin] - pub fn propose_change(e: &Env, change: ComplianceChange, _caller: Address) -> u32 { + pub fn propose_change(e: &Env, change: ComplianceChange) -> u32 { strata_storage::bump_instance(e); governance::propose(e, change) } #[only_admin] - pub fn cancel_change(e: &Env, id: u32, _caller: Address) { + pub fn cancel_change(e: &Env, id: u32) { strata_storage::bump_instance(e); governance::cancel(e, id); } @@ -126,7 +126,7 @@ impl Compliance { /// Lands a change whose delay has run. Refused while the vault is paused /// or winding down. #[only_admin] - pub fn apply_change(e: &Env, id: u32, _caller: Address) { + pub fn apply_change(e: &Env, id: u32) { strata_storage::bump_instance(e); governance::apply(e, id); } diff --git a/contracts/compliance/src/test/changes.rs b/contracts/compliance/src/test/changes.rs index 89734b4f..1a5ba221 100644 --- a/contracts/compliance/src/test/changes.rs +++ b/contracts/compliance/src/test/changes.rs @@ -1,7 +1,12 @@ +extern crate std; + use soroban_sdk::{ contract, contractimpl, symbol_short, - testutils::{Address as _, Events as _, Ledger as _, MockAuth, MockAuthInvoke}, - vec, Address, Env, Event as _, IntoVal, Val, Vec, + testutils::{ + Address as _, AuthorizedFunction, AuthorizedInvocation, Events as _, Ledger as _, MockAuth, + MockAuthInvoke, + }, + vec, Address, Env, Event as _, IntoVal, Symbol, Val, Vec, }; use stellar_access::role_transfer::RoleTransferError; use stellar_tokens::rwa::compliance::ComplianceHook; @@ -60,13 +65,13 @@ struct Fixture<'a> { impl Fixture<'_> { fn propose(&self, change: &ComplianceChange) -> u32 { - self.rules.propose_change(change, &self.admin) + self.rules.propose_change(change) } fn land(&self, change: &ComplianceChange) { let id = self.propose(change); self.e.ledger().with_mut(|l| l.timestamp += MIN_DELAY); - self.rules.apply_change(&id, &self.admin); + self.rules.apply_change(&id); } /// Signs the next call to the compliance contract as `who` and nothing @@ -83,6 +88,24 @@ impl Fixture<'_> { }]); } + /// The last call asked `who` alone to sign, once, on exactly this call. + fn signed_once_by(&self, who: &Address, fn_name: &str, args: Vec) { + assert_eq!( + self.e.auths(), + std::vec![( + who.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + self.rules.address.clone(), + Symbol::new(&self.e, fn_name), + args, + )), + sub_invocations: std::vec![], + }, + )] + ); + } + /// Lets the instance decay past its extend threshold, then checks the call /// extends it back to the full TTL. fn restores_the_instance(&self, call: impl FnOnce()) { @@ -116,13 +139,13 @@ fn unbound<'a>() -> Fixture<'a> { /// A compliance contract bound to a vault clock, with setup still open. fn bound<'a>() -> Fixture<'a> { let f = unbound(); - f.rules.bind_vault(&f.clock.address, &f.admin); + f.rules.bind_vault(&f.clock.address); f } fn finished<'a>() -> Fixture<'a> { let f = bound(); - f.rules.finish_setup(&f.admin); + f.rules.finish_setup(); f } @@ -148,7 +171,7 @@ fn setup_cannot_finish_without_a_vault() { let f = unbound(); refused( - f.rules.try_finish_setup(&f.admin), + f.rules.try_finish_setup(), ComplianceContractError::VaultNotBound, ); assert!(f.rules.setup_open()); @@ -158,18 +181,18 @@ fn setup_cannot_finish_without_a_vault() { fn a_second_vault_or_a_late_vault_is_refused() { let f = bound(); refused( - f.rules.try_bind_vault(&Address::generate(&f.e), &f.admin), + f.rules.try_bind_vault(&Address::generate(&f.e)), ComplianceContractError::SetupClosed, ); - f.rules.finish_setup(&f.admin); + f.rules.finish_setup(); refused( - f.rules.try_bind_vault(&Address::generate(&f.e), &f.admin), + f.rules.try_bind_vault(&Address::generate(&f.e)), ComplianceContractError::SetupClosed, ); refused( - f.rules.try_finish_setup(&f.admin), + f.rules.try_finish_setup(), ComplianceContractError::SetupClosed, ); assert_eq!(f.rules.vault(), Some(f.clock.address.clone())); @@ -179,10 +202,7 @@ fn a_second_vault_or_a_late_vault_is_refused() { fn an_address_that_reports_no_clock_cannot_be_bound() { let f = unbound(); - assert!(f - .rules - .try_bind_vault(&Address::generate(&f.e), &f.admin) - .is_err()); + assert!(f.rules.try_bind_vault(&Address::generate(&f.e)).is_err()); assert_eq!(f.rules.vault(), None); } @@ -196,8 +216,8 @@ fn the_contract_answers_the_view_the_vault_reads() { assert_eq!(view.vault(), None); assert!(view.setup_open()); - f.rules.bind_vault(&f.clock.address, &f.admin); - f.rules.finish_setup(&f.admin); + f.rules.bind_vault(&f.clock.address); + f.rules.finish_setup(); assert_eq!(view.vault(), Some(f.clock.address.clone())); assert!(!view.setup_open()); @@ -206,16 +226,19 @@ fn the_contract_answers_the_view_the_vault_reads() { #[test] fn only_the_admin_binds_and_finishes() { let f = unbound(); - f.e.set_auths(&[]); - assert!(f - .rules - .try_bind_vault(&f.clock.address, &Address::generate(&f.e)) - .is_err()); + let stranger = Address::generate(&f.e); + f.sign_only_as( + &stranger, + "bind_vault", + (f.clock.address.clone(),).into_val(&f.e), + ); + assert!(f.rules.try_bind_vault(&f.clock.address).is_err()); assert_eq!(f.rules.vault(), None); let f = bound(); - f.e.set_auths(&[]); - assert!(f.rules.try_finish_setup(&Address::generate(&f.e)).is_err()); + let stranger = Address::generate(&f.e); + f.sign_only_as(&stranger, "finish_setup", Vec::new(&f.e)); + assert!(f.rules.try_finish_setup().is_err()); assert!(f.rules.setup_open()); } @@ -249,10 +272,8 @@ fn after_setup_rule_changes_wait_for_the_timelock() { fn proposals_wait_until_setup_is_finished() { let f = bound(); refused( - f.rules.try_propose_change( - &ComplianceChange::BindToken(Address::generate(&f.e)), - &f.admin, - ), + f.rules + .try_propose_change(&ComplianceChange::BindToken(Address::generate(&f.e))), ComplianceContractError::SetupOpen, ); assert_eq!(f.rules.pending_changes().len(), 0); @@ -264,7 +285,7 @@ fn a_proposed_binding_lands_after_the_delay() { let token = Address::generate(&f.e); let change = ComplianceChange::BindToken(token.clone()); - let id = f.rules.propose_change(&change, &f.admin); + let id = f.rules.propose_change(&change); assert_eq!( f.rules.pending_changes(), @@ -278,12 +299,12 @@ fn a_proposed_binding_lands_after_the_delay() { ] ); refused( - f.rules.try_apply_change(&id, &f.admin), + f.rules.try_apply_change(&id), ComplianceContractError::ChangeNotReady, ); f.e.ledger().set_timestamp(10_000 + MIN_DELAY); - f.rules.apply_change(&id, &f.admin); + f.rules.apply_change(&id); assert_eq!(f.rules.linked_tokens(), vec![&f.e, token]); assert_eq!(f.rules.pending_changes().len(), 0); @@ -298,7 +319,7 @@ fn every_rule_change_lands_through_the_timelock() { f.rules.bind_token(&token, &f.admin); f.rules .add_module_to(&ComplianceHook::CanTransfer, &early, &f.admin); - f.rules.finish_setup(&f.admin); + f.rules.finish_setup(); f.land(&ComplianceChange::RemoveModule( ComplianceHook::CanTransfer, @@ -323,17 +344,17 @@ fn every_rule_change_lands_through_the_timelock() { fn nothing_lands_while_the_vault_is_paused_or_winding_down() { let f = finished(); let module = Address::generate(&f.e); - let id = f.rules.propose_change( - &ComplianceChange::AddModule(ComplianceHook::CanCreate, module), - &f.admin, - ); + let id = f.rules.propose_change(&ComplianceChange::AddModule( + ComplianceHook::CanCreate, + module, + )); f.e.ledger().set_timestamp(10_000 + MIN_DELAY); let mut paused = running(); paused.paused = true; f.clock.set(&paused); refused( - f.rules.try_apply_change(&id, &f.admin), + f.rules.try_apply_change(&id), ComplianceContractError::VaultPaused, ); @@ -341,14 +362,12 @@ fn nothing_lands_while_the_vault_is_paused_or_winding_down() { winding.winding_down = true; f.clock.set(&winding); refused( - f.rules.try_apply_change(&id, &f.admin), + f.rules.try_apply_change(&id), ComplianceContractError::WindDownActive, ); refused( - f.rules.try_propose_change( - &ComplianceChange::BindToken(Address::generate(&f.e)), - &f.admin, - ), + f.rules + .try_propose_change(&ComplianceChange::BindToken(Address::generate(&f.e))), ComplianceContractError::WindDownActive, ); } @@ -365,12 +384,12 @@ fn a_vault_pause_pushes_the_change_back() { assert_eq!(f.rules.pending_changes().get(0).unwrap().eta, eta); f.e.ledger().set_timestamp(eta - 1); refused( - f.rules.try_apply_change(&id, &f.admin), + f.rules.try_apply_change(&id), ComplianceContractError::ChangeNotReady, ); f.e.ledger().set_timestamp(eta); - f.rules.apply_change(&id, &f.admin); + f.rules.apply_change(&id); } #[test] @@ -384,7 +403,7 @@ fn a_change_shorter_than_a_raised_notice_never_lands() { f.e.ledger().set_timestamp(10_000 + 30 * DAY_DURATION); refused( - f.rules.try_apply_change(&id, &f.admin), + f.rules.try_apply_change(&id), ComplianceContractError::DelayBelowNotice, ); } @@ -392,15 +411,14 @@ fn a_change_shorter_than_a_raised_notice_never_lands() { #[test] fn a_cancelled_change_never_lands() { let f = finished(); - let id = f.rules.propose_change( - &ComplianceChange::BindToken(Address::generate(&f.e)), - &f.admin, - ); + let id = f + .rules + .propose_change(&ComplianceChange::BindToken(Address::generate(&f.e))); - f.rules.cancel_change(&id, &f.admin); + f.rules.cancel_change(&id); refused( - f.rules.try_apply_change(&id, &f.admin), + f.rules.try_apply_change(&id), ComplianceContractError::ChangeNotFound, ); assert_eq!(f.rules.linked_tokens().len(), 0); @@ -414,10 +432,16 @@ fn only_the_admin_proposes_cancels_and_applies() { let id = f.propose(&change); f.e.ledger().set_timestamp(10_000 + MIN_DELAY); - f.e.set_auths(&[]); - assert!(f.rules.try_propose_change(&change, &stranger).is_err()); - assert!(f.rules.try_cancel_change(&id, &stranger).is_err()); - assert!(f.rules.try_apply_change(&id, &stranger).is_err()); + f.sign_only_as( + &stranger, + "propose_change", + (change.clone(),).into_val(&f.e), + ); + assert!(f.rules.try_propose_change(&change).is_err()); + f.sign_only_as(&stranger, "cancel_change", (id,).into_val(&f.e)); + assert!(f.rules.try_cancel_change(&id).is_err()); + f.sign_only_as(&stranger, "apply_change", (id,).into_val(&f.e)); + assert!(f.rules.try_apply_change(&id).is_err()); assert_eq!(f.rules.pending_changes().len(), 1); } @@ -430,7 +454,7 @@ fn proposing_and_applying_read_the_vault_clock_once() { assert_eq!(f.clock.reads(), before + 1); f.e.ledger().set_timestamp(10_000 + MIN_DELAY); - f.rules.apply_change(&id, &f.admin); + f.rules.apply_change(&id); assert_eq!(f.clock.reads(), before + 2); } @@ -451,7 +475,7 @@ fn proposing_and_applying_publish_the_change() { ); f.e.ledger().set_timestamp(10_000 + MIN_DELAY); - f.rules.apply_change(&id, &f.admin); + f.rules.apply_change(&id); let events = f.e.events().all().filter_by_contract(&f.rules.address); assert_eq!( events.events().last(), @@ -467,13 +491,14 @@ fn setup_cannot_finish_while_a_hand_over_is_pending() { .transfer_admin_role(&next, &(f.e.ledger().sequence() + 100)); refused( - f.rules.try_finish_setup(&f.admin), + f.rules.try_finish_setup(), ComplianceContractError::AdminTransferPending, ); f.rules.accept_admin_transfer(); assert_eq!(f.rules.get_admin(), Some(next.clone())); - f.rules.finish_setup(&next); + f.rules.finish_setup(); + f.signed_once_by(&next, "finish_setup", Vec::new(&f.e)); assert!(!f.rules.setup_open()); } @@ -532,7 +557,7 @@ fn a_cancelled_hand_over_no_longer_holds_setup_open() { .transfer_admin_role(&next, &(f.e.ledger().sequence() + 100)); f.rules.transfer_admin_role(&next, &0); - f.rules.finish_setup(&f.admin); + f.rules.finish_setup(); assert_eq!(f.rules.get_admin(), Some(f.admin.clone())); } @@ -549,9 +574,9 @@ fn after_setup_the_admin_hands_over_through_the_timelock() { let id = f .rules - .propose_change(&ComplianceChange::Admin(next.clone()), &f.admin); + .propose_change(&ComplianceChange::Admin(next.clone())); f.e.ledger().set_timestamp(10_000 + MIN_DELAY); - f.rules.apply_change(&id, &f.admin); + f.rules.apply_change(&id); assert_eq!(f.rules.get_admin(), Some(f.admin.clone())); f.rules.accept_admin_transfer(); @@ -611,16 +636,129 @@ fn every_admin_call_restores_the_instance_ttl() { .transfer_admin_role(&next, &(f.e.ledger().sequence() + ADMIN_ACCEPT_LEDGERS)) }); f.restores_the_instance(|| f.rules.transfer_admin_role(&next, &0)); - f.restores_the_instance(|| f.rules.bind_vault(&f.clock.address, &f.admin)); - f.restores_the_instance(|| f.rules.finish_setup(&f.admin)); + f.restores_the_instance(|| f.rules.bind_vault(&f.clock.address)); + f.restores_the_instance(|| f.rules.finish_setup()); let change = ComplianceChange::BindToken(Address::generate(&f.e)); let mut id = 0; f.restores_the_instance(|| id = f.propose(&change)); - f.restores_the_instance(|| f.rules.cancel_change(&id, &f.admin)); + f.restores_the_instance(|| f.rules.cancel_change(&id)); let id = f.propose(&ComplianceChange::Admin(next)); f.e.ledger().with_mut(|l| l.timestamp += MIN_DELAY); - f.restores_the_instance(|| f.rules.apply_change(&id, &f.admin)); + f.restores_the_instance(|| f.rules.apply_change(&id)); f.restores_the_instance(|| f.rules.accept_admin_transfer()); } + +#[test] +fn each_admin_call_asks_the_admin_for_one_signature() { + let f = unbound(); + let token = Address::generate(&f.e); + let module = Address::generate(&f.e); + + f.rules.bind_token(&token, &f.admin); + f.signed_once_by( + &f.admin, + "bind_token", + (token.clone(), f.admin.clone()).into_val(&f.e), + ); + f.rules.unbind_token(&token, &f.admin); + f.signed_once_by( + &f.admin, + "unbind_token", + (token.clone(), f.admin.clone()).into_val(&f.e), + ); + f.rules + .add_module_to(&ComplianceHook::CanTransfer, &module, &f.admin); + f.signed_once_by( + &f.admin, + "add_module_to", + (ComplianceHook::CanTransfer, module.clone(), f.admin.clone()).into_val(&f.e), + ); + f.rules + .remove_module_from(&ComplianceHook::CanTransfer, &module, &f.admin); + f.signed_once_by( + &f.admin, + "remove_module_from", + (ComplianceHook::CanTransfer, module, f.admin.clone()).into_val(&f.e), + ); + + f.rules.bind_vault(&f.clock.address); + f.signed_once_by( + &f.admin, + "bind_vault", + (f.clock.address.clone(),).into_val(&f.e), + ); + f.rules.finish_setup(); + f.signed_once_by(&f.admin, "finish_setup", Vec::new(&f.e)); + + let change = ComplianceChange::BindToken(token); + let id = f.rules.propose_change(&change); + f.signed_once_by(&f.admin, "propose_change", (change.clone(),).into_val(&f.e)); + f.rules.cancel_change(&id); + f.signed_once_by(&f.admin, "cancel_change", (id,).into_val(&f.e)); + + let id = f.rules.propose_change(&change); + f.e.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + f.rules.apply_change(&id); + f.signed_once_by(&f.admin, "apply_change", (id,).into_val(&f.e)); + + let id = f + .rules + .propose_change(&ComplianceChange::Admin(Address::generate(&f.e))); + f.e.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + f.rules.apply_change(&id); + f.signed_once_by(&f.admin, "apply_change", (id,).into_val(&f.e)); +} + +#[test] +fn a_hand_over_asks_each_side_for_one_signature() { + let f = bound(); + let next = Address::generate(&f.e); + let live_until = f.e.ledger().sequence() + 100; + + f.rules.transfer_admin_role(&next, &live_until); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (next.clone(), live_until).into_val(&f.e), + ); + f.rules.transfer_admin_role(&next, &0); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (next.clone(), 0_u32).into_val(&f.e), + ); + + f.rules.transfer_admin_role(&next, &live_until); + f.rules.accept_admin_transfer(); + f.signed_once_by(&next, "accept_admin_transfer", Vec::new(&f.e)); +} + +#[test] +fn each_hook_asks_the_bound_token_for_one_signature() { + let f = bound(); + let token = Address::generate(&f.e); + let from = Address::generate(&f.e); + let to = Address::generate(&f.e); + f.rules.bind_token(&token, &f.admin); + + f.rules.created(&to, &10, &token); + f.signed_once_by( + &token, + "created", + (to.clone(), 10_i128, token.clone()).into_val(&f.e), + ); + f.rules.transferred(&to, &from, &4, &token); + f.signed_once_by( + &token, + "transferred", + (to.clone(), from.clone(), 4_i128, token.clone()).into_val(&f.e), + ); + f.rules.destroyed(&from, &4, &token); + f.signed_once_by( + &token, + "destroyed", + (from, 4_i128, token.clone()).into_val(&f.e), + ); +} diff --git a/contracts/compliance/src/test/guards.rs b/contracts/compliance/src/test/guards.rs index 4ab9bee9..94594eb3 100644 --- a/contracts/compliance/src/test/guards.rs +++ b/contracts/compliance/src/test/guards.rs @@ -28,13 +28,12 @@ struct Fixture<'a> { e: Env, rules: ComplianceClient<'a>, vault: Address, - admin: Address, } impl Fixture<'_> { fn finish_setup(&self) { - self.rules.bind_vault(&self.vault, &self.admin); - self.rules.finish_setup(&self.admin); + self.rules.bind_vault(&self.vault); + self.rules.finish_setup(); } } @@ -42,14 +41,9 @@ fn setup<'a>() -> Fixture<'a> { let e = Env::default(); e.mock_all_auths(); let admin = Address::generate(&e); - let rules = ComplianceClient::new(&e, &e.register(Compliance, (admin.clone(),))); + let rules = ComplianceClient::new(&e, &e.register(Compliance, (admin,))); let vault = e.register(VaultStub, ()); - Fixture { - e, - rules, - vault, - admin, - } + Fixture { e, rules, vault } } /// Runs `guard` inside the compliance contract, as an entrypoint would. diff --git a/contracts/identity-verifier/src/lib.rs b/contracts/identity-verifier/src/lib.rs index 83a677c1..bfb7af99 100644 --- a/contracts/identity-verifier/src/lib.rs +++ b/contracts/identity-verifier/src/lib.rs @@ -31,8 +31,7 @@ impl IdentityVerifier { /// Adds or removes an account from the allowlist. #[only_admin] - pub fn allow(e: &Env, account: Address, allowed: bool, caller: Address) { - caller.require_auth(); + pub fn allow(e: &Env, account: Address, allowed: bool) { strata_storage::bump_instance(e); strata_storage::set_persistent(e, &DataKey::Allowed(account), &allowed); } diff --git a/contracts/identity-verifier/src/test.rs b/contracts/identity-verifier/src/test.rs index 7b9190a1..714fae3c 100644 --- a/contracts/identity-verifier/src/test.rs +++ b/contracts/identity-verifier/src/test.rs @@ -1,13 +1,77 @@ +extern crate std; + use super::*; use soroban_sdk::{ - testutils::{storage::Instance as _, storage::Persistent as _, Address as _, Ledger as _}, - Address, Env, + testutils::{ + storage::Instance as _, storage::Persistent as _, Address as _, AuthorizedFunction, + AuthorizedInvocation, Ledger as _, MockAuth, MockAuthInvoke, + }, + Address, Env, IntoVal, Symbol, Val, Vec, }; use strata_storage::{INSTANCE_EXTEND_AMOUNT, PERSISTENT_EXTEND_AMOUNT}; const DAY_IN_LEDGERS: u32 = 17_280; use stellar_tokens::rwa::identity_verification::IdentityVerifierClient as IdClient; +/// Signs one `allow` call as `who` and nothing else. +fn sign_only_as(e: &Env, verifier: &Address, who: &Address, args: Vec) { + e.mock_auths(&[MockAuth { + address: who, + invoke: &MockAuthInvoke { + contract: verifier, + fn_name: "allow", + args, + sub_invokes: &[], + }, + }]); +} + +/// The last call asked `who` alone to sign, once, on exactly this call. +fn signed_once_by(e: &Env, verifier: &Address, who: &Address, fn_name: &str, args: Vec) { + assert_eq!( + e.auths(), + std::vec![( + who.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + verifier.clone(), + Symbol::new(e, fn_name), + args, + )), + sub_invocations: std::vec![], + }, + )] + ); +} + +#[test] +fn each_admin_call_asks_the_admin_for_one_signature() { + let e = Env::default(); + e.mock_all_auths(); + let admin = Address::generate(&e); + let investor = Address::generate(&e); + let id = e.register(IdentityVerifier, (admin.clone(),)); + + IdentityVerifierClient::new(&e, &id).allow(&investor, &true); + signed_once_by( + &e, + &id, + &admin, + "allow", + (investor.clone(), true).into_val(&e), + ); + + let topics = Address::generate(&e); + IdClient::new(&e, &id).set_claim_topics_and_issuers(&topics, &admin); + signed_once_by( + &e, + &id, + &admin, + "set_claim_topics_and_issuers", + (topics, admin.clone()).into_val(&e), + ); +} + #[test] fn only_admin_can_allow() { let e = Env::default(); @@ -22,22 +86,23 @@ fn only_admin_can_allow() { assert!(!client.is_allowed(&investor)); // Stranger cannot allow. - assert!(client.try_allow(&investor, &true, &stranger).is_err()); + sign_only_as(&e, &id, &stranger, (investor.clone(), true).into_val(&e)); + assert!(client.try_allow(&investor, &true).is_err()); assert!(!client.is_allowed(&investor)); // Admin allows. e.mock_all_auths(); - client.allow(&investor, &true, &admin); + client.allow(&investor, &true); assert!(client.is_allowed(&investor)); // Stranger cannot disallow. - e.set_auths(&[]); - assert!(client.try_allow(&investor, &false, &stranger).is_err()); + sign_only_as(&e, &id, &stranger, (investor.clone(), false).into_val(&e)); + assert!(client.try_allow(&investor, &false).is_err()); assert!(client.is_allowed(&investor)); // Admin disallows. e.mock_all_auths(); - client.allow(&investor, &false, &admin); + client.allow(&investor, &false); assert!(!client.is_allowed(&investor)); } @@ -56,7 +121,7 @@ fn verify_identity_checks_allowlist() { assert!(id_client.try_verify_identity(&investor).is_err()); // Once allowed, verification succeeds. - client.allow(&investor, &true, &admin); + client.allow(&investor, &true); id_client.verify_identity(&investor); } @@ -80,7 +145,7 @@ fn allowing_writes_the_entry_with_the_full_ttl() { let investor = Address::generate(&e); let id = e.register(IdentityVerifier, (admin.clone(),)); - IdentityVerifierClient::new(&e, &id).allow(&investor, &true, &admin); + IdentityVerifierClient::new(&e, &id).allow(&investor, &true); assert_eq!(allowed_ttl(&e, &id, &investor), PERSISTENT_EXTEND_AMOUNT); } @@ -93,7 +158,7 @@ fn checking_an_allowed_account_restores_the_full_ttl() { let investor = Address::generate(&e); let id = e.register(IdentityVerifier, (admin.clone(),)); let client = IdentityVerifierClient::new(&e, &id); - client.allow(&investor, &true, &admin); + client.allow(&investor, &true); age(&e, 2 * DAY_IN_LEDGERS); assert!(client.is_allowed(&investor)); @@ -127,7 +192,7 @@ fn allowing_and_checking_restore_the_instance_ttl() { age(&e, 2 * DAY_IN_LEDGERS); assert!(instance_ttl(&e, &id) < INSTANCE_EXTEND_AMOUNT); - client.allow(&investor, &true, &admin); + client.allow(&investor, &true); assert_eq!(instance_ttl(&e, &id), INSTANCE_EXTEND_AMOUNT); age(&e, 2 * DAY_IN_LEDGERS); diff --git a/contracts/nav-oracle/src/lib.rs b/contracts/nav-oracle/src/lib.rs index 2570e741..d7c7d4e3 100644 --- a/contracts/nav-oracle/src/lib.rs +++ b/contracts/nav-oracle/src/lib.rs @@ -250,31 +250,27 @@ impl NavOracleContract { /// Names the vault whose clock this oracle's timelock reads. Once, and only /// during setup. #[only_admin] - pub fn bind_vault(e: &Env, vault: Address, caller: Address) { - caller.require_auth(); + pub fn bind_vault(e: &Env, vault: Address) { strata_storage::bump_instance(e); governance::bind_vault(e, &vault); } /// Ends the window in which admin changes are instant. It never reopens. #[only_admin] - pub fn finish_setup(e: &Env, caller: Address) { - caller.require_auth(); + pub fn finish_setup(e: &Env) { strata_storage::bump_instance(e); governance::finish_setup(e); } /// Queues a change behind the vault's timelock and returns its id. #[only_admin] - pub fn propose_change(e: &Env, change: OracleChange, caller: Address) -> u32 { - caller.require_auth(); + pub fn propose_change(e: &Env, change: OracleChange) -> u32 { strata_storage::bump_instance(e); governance::propose(e, change) } #[only_admin] - pub fn cancel_change(e: &Env, id: u32, caller: Address) { - caller.require_auth(); + pub fn cancel_change(e: &Env, id: u32) { strata_storage::bump_instance(e); governance::cancel(e, id); } @@ -282,8 +278,7 @@ impl NavOracleContract { /// Lands a change whose delay has run. Refused while the vault is paused /// or winding down. #[only_admin] - pub fn apply_change(e: &Env, id: u32, caller: Address) { - caller.require_auth(); + pub fn apply_change(e: &Env, id: u32) { strata_storage::bump_instance(e); governance::apply(e, id); } @@ -344,8 +339,7 @@ impl NavOracleContract { } #[only_admin] - pub fn set_nav_paused(e: &Env, paused: bool, caller: Address) { - caller.require_auth(); + pub fn set_nav_paused(e: &Env, paused: bool) { state::set_nav_paused(e, paused); NavPausedSet { paused }.publish(e); } @@ -354,8 +348,7 @@ impl NavOracleContract { /// its distance from the last. Only while the NAV is paused, so /// resuming is always a deliberate second act. #[only_admin] - pub fn clear_latest(e: &Env, caller: Address) { - caller.require_auth(); + pub fn clear_latest(e: &Env) { guards::nav_paused(e); state::clear_latest(e); LatestCleared {}.publish(e); @@ -410,8 +403,7 @@ pub enum ProbeError { #[contractimpl] impl NavOracleContract { #[only_admin] - pub fn migrate(e: &Env, caller: Address) { - caller.require_auth(); + pub fn migrate(e: &Env) { strata_storage::bump_instance(e); if upgradeable::get_schema_version(e) >= 2 { panic_with_error!(e, ProbeError::AlreadyMigrated); diff --git a/contracts/nav-oracle/src/test.rs b/contracts/nav-oracle/src/test.rs index 29eb21d0..ad5e3b0d 100644 --- a/contracts/nav-oracle/src/test.rs +++ b/contracts/nav-oracle/src/test.rs @@ -2,8 +2,10 @@ extern crate std; use soroban_sdk::{ symbol_short, - testutils::{storage::Persistent as _, Address as _, Events as _, Ledger as _}, - Address, Env, Event as _, + testutils::{ + storage::Persistent as _, Address as _, Events as _, Ledger as _, MockAuth, MockAuthInvoke, + }, + Address, Env, Event as _, IntoVal, }; use crate::state::DataKey; @@ -21,6 +23,21 @@ struct Fixture<'a> { guardian: Address, } +impl Fixture<'_> { + /// Signs one call to the oracle as `who` and nothing else. + fn sign_only_as(&self, who: &Address, fn_name: &str, args: soroban_sdk::Vec) { + self.e.mock_auths(&[MockAuth { + address: who, + invoke: &MockAuthInvoke { + contract: &self.oracle.address, + fn_name, + args, + sub_invokes: &[], + }, + }]); + } +} + fn config() -> OracleConfig { OracleConfig { freshness_duration: 3600, @@ -98,11 +115,11 @@ fn the_nav_pause_holds_regardless_of_freshness() { f.oracle.attest(&r, &f.attester); assert_eq!(f.oracle.state(), OracleState::Valid); - f.oracle.set_nav_paused(&true, &f.admin); + f.oracle.set_nav_paused(&true); assert_eq!(f.oracle.state(), OracleState::Paused); assert!(f.oracle.try_ensure_consumable().is_err()); - f.oracle.set_nav_paused(&false, &f.admin); + f.oracle.set_nav_paused(&false); assert_eq!(f.oracle.state(), OracleState::Valid); } @@ -315,13 +332,14 @@ fn the_guardian_pauses_the_nav_but_unpausing_needs_governance() { f.oracle.pause_nav(&f.guardian); assert_eq!(f.oracle.state(), OracleState::Paused); + f.sign_only_as(&f.guardian, "set_nav_paused", (false,).into_val(&f.e)); + assert!(f.oracle.try_set_nav_paused(&false).is_err()); f.e.set_auths(&[]); - assert!(f.oracle.try_set_nav_paused(&false, &f.guardian).is_err()); - assert!(f.oracle.try_set_nav_paused(&false, &f.admin).is_err()); + assert!(f.oracle.try_set_nav_paused(&false).is_err()); assert_eq!(f.oracle.state(), OracleState::Paused); f.e.mock_all_auths(); - f.oracle.set_nav_paused(&false, &f.admin); + f.oracle.set_nav_paused(&false); assert_eq!(f.oracle.state(), OracleState::Valid); } @@ -422,13 +440,13 @@ fn the_record_clears_only_while_the_nav_is_paused() { f.oracle .attest(&report(&f.e, SCALE, 1, 1_000_000), &f.attester); - assert!(f.oracle.try_clear_latest(&f.admin).is_err()); + assert!(f.oracle.try_clear_latest().is_err()); f.oracle.pause_nav(&f.guardian); - f.oracle.clear_latest(&f.admin); + f.oracle.clear_latest(); // With no stored record, one in-band value lands however far it sits. - f.oracle.set_nav_paused(&false, &f.admin); + f.oracle.set_nav_paused(&false); f.e.ledger().set_timestamp(10_100); let far = report(&f.e, SCALE * 50, 2, 1_000_000); f.oracle.attest(&far, &f.attester); @@ -443,10 +461,10 @@ fn only_admin_can_clear_latest() { f.oracle.attest(&r, &f.attester); f.oracle.pause_nav(&f.guardian); - f.e.set_auths(&[]); - assert!(f.oracle.try_clear_latest(&stranger).is_err()); - assert!(f.oracle.try_clear_latest(&f.attester).is_err()); - assert!(f.oracle.try_clear_latest(&f.guardian).is_err()); + for who in [&stranger, &f.attester, &f.guardian] { + f.sign_only_as(who, "clear_latest", soroban_sdk::Vec::new(&f.e)); + assert!(f.oracle.try_clear_latest().is_err()); + } } #[test] @@ -561,7 +579,7 @@ fn attesting_is_refused_while_the_nav_is_paused() { Some(Ok(crate::OracleError::NavPaused.into())) ); - f.oracle.set_nav_paused(&false, &f.admin); + f.oracle.set_nav_paused(&false); f.oracle .attest(&report(&f.e, SCALE, 0, 1_000_000), &f.attester); assert_eq!(f.oracle.state(), OracleState::Valid); diff --git a/contracts/nav-oracle/src/test_governance.rs b/contracts/nav-oracle/src/test_governance.rs index 842fa6ee..6a64dec0 100644 --- a/contracts/nav-oracle/src/test_governance.rs +++ b/contracts/nav-oracle/src/test_governance.rs @@ -1,9 +1,12 @@ +extern crate std; + use soroban_sdk::{ contract, contractimpl, symbol_short, testutils::{ - storage::Instance as _, Address as _, Events as _, Ledger as _, MockAuth, MockAuthInvoke, + storage::Instance as _, Address as _, AuthorizedFunction, AuthorizedInvocation, + Events as _, Ledger as _, MockAuth, MockAuthInvoke, }, - Address, BytesN, Env, Event as _, IntoVal, + Address, BytesN, Env, Event as _, IntoVal, Symbol, }; use stellar_access::role_transfer::RoleTransferError; use strata_storage::INSTANCE_EXTEND_AMOUNT; @@ -73,12 +76,12 @@ impl Fixture<'_> { } fn finish(&self) { - self.oracle.bind_vault(&self.vault.address, &self.admin); - self.oracle.finish_setup(&self.admin); + self.oracle.bind_vault(&self.vault.address); + self.oracle.finish_setup(); } fn propose(&self, change: OracleChange) -> u32 { - self.oracle.propose_change(&change, &self.admin) + self.oracle.propose_change(&change) } fn attest(&self, nav_per_share: i128) { @@ -89,7 +92,7 @@ impl Fixture<'_> { fn offer_admin(&self, next: &Address) -> u32 { let id = self.propose(OracleChange::Admin(next.clone())); self.advance(MIN_DELAY); - self.oracle.apply_change(&id, &self.admin); + self.oracle.apply_change(&id); self.e.ledger().sequence() } @@ -111,7 +114,7 @@ impl Fixture<'_> { } /// Signs one call to the oracle as `who` and nothing else, so the call - /// carries the caller's signature but not the admin's. + /// carries that account's signature but not the admin's. fn sign_only_as(&self, who: &Address, fn_name: &str, args: soroban_sdk::Vec) { self.e.mock_auths(&[MockAuth { address: who, @@ -123,6 +126,29 @@ impl Fixture<'_> { }, }]); } + + /// The last call asked `who` alone to sign, once, on exactly this call. + fn signed_once_by( + &self, + who: &Address, + fn_name: &str, + args: soroban_sdk::Vec, + ) { + assert_eq!( + self.e.auths(), + std::vec![( + who.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + self.oracle.address.clone(), + Symbol::new(&self.e, fn_name), + args, + )), + sub_invocations: std::vec![], + }, + )] + ); + } } fn config() -> OracleConfig { @@ -378,12 +404,9 @@ fn a_loosened_config_lands_through_the_timelock() { let id = f.propose(OracleChange::Config(loose.clone())); f.advance(MIN_DELAY - 1); - refused( - f.oracle.try_apply_change(&id, &f.admin), - OracleError::ChangeNotReady, - ); + refused(f.oracle.try_apply_change(&id), OracleError::ChangeNotReady); f.advance(1); - f.oracle.apply_change(&id, &f.admin); + f.oracle.apply_change(&id); assert_eq!(f.oracle.config(), loose); assert_eq!(f.oracle.pending_changes().len(), 0); @@ -399,8 +422,7 @@ fn an_invalid_config_cannot_be_proposed() { }; refused( - f.oracle - .try_propose_change(&OracleChange::Config(invalid), &f.admin), + f.oracle.try_propose_change(&OracleChange::Config(invalid)), OracleError::InvalidConfig, ); } @@ -424,10 +446,7 @@ fn a_config_outside_the_bounds_is_refused_when_it_applies() { }); f.advance(MIN_DELAY); - refused( - f.oracle.try_apply_change(&id, &f.admin), - OracleError::InvalidConfig, - ); + refused(f.oracle.try_apply_change(&id), OracleError::InvalidConfig); assert_eq!(f.oracle.config(), config()); assert_eq!(f.oracle.pending_changes().len(), 1); } @@ -436,21 +455,15 @@ fn a_config_outside_the_bounds_is_refused_when_it_applies() { fn setup_is_open_until_finished_and_never_reopens() { let f = setup(); assert!(f.oracle.setup_open()); - refused( - f.oracle.try_finish_setup(&f.admin), - OracleError::VaultNotBound, - ); + refused(f.oracle.try_finish_setup(), OracleError::VaultNotBound); f.finish(); assert!(!f.oracle.setup_open()); assert_eq!(f.oracle.vault(), Some(f.vault.address.clone())); + refused(f.oracle.try_finish_setup(), OracleError::SetupClosed); refused( - f.oracle.try_finish_setup(&f.admin), - OracleError::SetupClosed, - ); - refused( - f.oracle.try_bind_vault(&Address::generate(&f.e), &f.admin), + f.oracle.try_bind_vault(&Address::generate(&f.e)), OracleError::SetupClosed, ); } @@ -458,10 +471,10 @@ fn setup_is_open_until_finished_and_never_reopens() { #[test] fn a_second_vault_cannot_be_bound() { let f = setup(); - f.oracle.bind_vault(&f.vault.address, &f.admin); + f.oracle.bind_vault(&f.vault.address); refused( - f.oracle.try_bind_vault(&Address::generate(&f.e), &f.admin), + f.oracle.try_bind_vault(&Address::generate(&f.e)), OracleError::SetupClosed, ); assert_eq!(f.oracle.vault(), Some(f.vault.address.clone())); @@ -471,10 +484,7 @@ fn a_second_vault_cannot_be_bound() { fn an_address_that_reports_no_clock_cannot_be_bound() { let f = setup(); - assert!(f - .oracle - .try_bind_vault(&Address::generate(&f.e), &f.admin) - .is_err()); + assert!(f.oracle.try_bind_vault(&Address::generate(&f.e)).is_err()); assert_eq!(f.oracle.vault(), None); } @@ -486,22 +496,15 @@ fn only_the_admin_binds_and_finishes() { f.sign_only_as( &stranger, "bind_vault", - (f.vault.address.clone(), stranger.clone()).into_val(&f.e), + (f.vault.address.clone(),).into_val(&f.e), ); - assert!(f - .oracle - .try_bind_vault(&f.vault.address, &stranger) - .is_err()); + assert!(f.oracle.try_bind_vault(&f.vault.address).is_err()); assert_eq!(f.oracle.vault(), None); f.e.mock_all_auths(); - f.oracle.bind_vault(&f.vault.address, &f.admin); - f.sign_only_as( - &stranger, - "finish_setup", - (stranger.clone(),).into_val(&f.e), - ); - assert!(f.oracle.try_finish_setup(&stranger).is_err()); + f.oracle.bind_vault(&f.vault.address); + f.sign_only_as(&stranger, "finish_setup", soroban_sdk::Vec::new(&f.e)); + assert!(f.oracle.try_finish_setup().is_err()); assert!(f.oracle.setup_open()); } @@ -525,17 +528,17 @@ fn the_oracle_answers_the_view_the_vault_reads() { fn setup_cannot_finish_while_a_hand_over_is_pending() { let f = setup(); let next = Address::generate(&f.e); - f.oracle.bind_vault(&f.vault.address, &f.admin); + f.oracle.bind_vault(&f.vault.address); f.oracle .transfer_admin_role(&next, &(f.e.ledger().sequence() + 100)); refused( - f.oracle.try_finish_setup(&f.admin), + f.oracle.try_finish_setup(), OracleError::AdminTransferPending, ); f.oracle.transfer_admin_role(&next, &0); - f.oracle.finish_setup(&f.admin); + f.oracle.finish_setup(); assert!(f.oracle.try_accept_admin_transfer().is_err()); assert_eq!(f.oracle.get_admin(), Some(f.admin.clone())); } @@ -543,12 +546,12 @@ fn setup_cannot_finish_while_a_hand_over_is_pending() { #[test] fn an_expired_hand_over_does_not_hold_setup_open() { let f = setup(); - f.oracle.bind_vault(&f.vault.address, &f.admin); + f.oracle.bind_vault(&f.vault.address); f.oracle .transfer_admin_role(&Address::generate(&f.e), &(f.e.ledger().sequence() + 10)); f.e.ledger().with_mut(|l| l.sequence_number += 11); - f.oracle.finish_setup(&f.admin); + f.oracle.finish_setup(); assert!(!f.oracle.setup_open()); assert_eq!( @@ -563,8 +566,7 @@ fn nothing_is_proposed_during_setup() { let f = setup(); refused( - f.oracle - .try_propose_change(&OracleChange::Config(config()), &f.admin), + f.oracle.try_propose_change(&OracleChange::Config(config())), OracleError::SetupOpen, ); assert_eq!(f.oracle.pending_changes().len(), 0); @@ -583,7 +585,7 @@ fn grants_wait_after_setup_and_land_through_the_timelock() { let grant = f.propose(OracleChange::GrantRole(GUARDIAN_ROLE, next.clone())); f.advance(MIN_DELAY); - f.oracle.apply_change(&grant, &f.admin); + f.oracle.apply_change(&grant); f.oracle.pause_nav(&next); assert_eq!(f.oracle.state(), OracleState::Paused); @@ -628,7 +630,7 @@ fn a_revoke_lands_through_the_timelock() { let id = f.propose(OracleChange::RevokeRole(GUARDIAN_ROLE, f.guardian.clone())); f.advance(MIN_DELAY); - f.oracle.apply_change(&id, &f.admin); + f.oracle.apply_change(&id); assert!(f.oracle.has_role(&f.guardian, &GUARDIAN_ROLE).is_none()); } @@ -657,7 +659,7 @@ fn an_admin_hand_over_waits_but_cancelling_one_does_not() { let id = f.propose(OracleChange::Admin(next.clone())); f.advance(MIN_DELAY); - f.oracle.apply_change(&id, &f.admin); + f.oracle.apply_change(&id); f.oracle.transfer_admin_role(&next, &0); assert!(f.oracle.try_accept_admin_transfer().is_err()); @@ -672,26 +674,9 @@ fn an_applied_admin_change_starts_a_hand_over_the_next_admin_accepts() { let id = f.propose(OracleChange::Admin(next.clone())); f.advance(MIN_DELAY); - // The admin authorises this call and nothing else: the hand-over inside it - // asks for no further signature. The entrypoint checks the admin and the - // caller, so the same authorisation appears twice, as simulation records it. - let apply = MockAuthInvoke { - contract: &f.oracle.address, - fn_name: "apply_change", - args: (id, f.admin.clone()).into_val(&f.e), - sub_invokes: &[], - }; - f.e.mock_auths(&[ - MockAuth { - address: &f.admin, - invoke: &apply, - }, - MockAuth { - address: &f.admin, - invoke: &apply, - }, - ]); - f.oracle.apply_change(&id, &f.admin); + // One admin signature covers the call; the hand-over inside asks for none. + f.sign_only_as(&f.admin, "apply_change", (id,).into_val(&f.e)); + f.oracle.apply_change(&id); f.e.mock_all_auths(); assert_eq!(f.oracle.get_admin(), Some(f.admin.clone())); @@ -736,9 +721,9 @@ fn the_nav_pause_and_clearing_stay_instant_after_setup() { f.finish(); f.oracle.pause_nav(&f.guardian); - f.oracle.set_nav_paused(&false, &f.admin); - f.oracle.set_nav_paused(&true, &f.admin); - f.oracle.clear_latest(&f.admin); + f.oracle.set_nav_paused(&false); + f.oracle.set_nav_paused(&true); + f.oracle.clear_latest(); assert_eq!(f.oracle.state(), OracleState::Paused); assert_eq!( @@ -763,23 +748,17 @@ fn a_vault_pause_moves_the_eta_and_holds_the_change() { f.clock(|c| c.paused_secs = 3 * DAY_DURATION); f.advance(MIN_DELAY - 4 * DAY_DURATION); - refused( - f.oracle.try_apply_change(&id, &f.admin), - OracleError::VaultPaused, - ); + refused(f.oracle.try_apply_change(&id), OracleError::VaultPaused); f.clock(|c| c.paused = false); assert_eq!( f.oracle.pending_changes().get(0).unwrap().eta, eta + 3 * DAY_DURATION ); - refused( - f.oracle.try_apply_change(&id, &f.admin), - OracleError::ChangeNotReady, - ); + refused(f.oracle.try_apply_change(&id), OracleError::ChangeNotReady); f.advance(3 * DAY_DURATION); - f.oracle.apply_change(&id, &f.admin); + f.oracle.apply_change(&id); assert_eq!(f.oracle.config().freshness_duration, 7_200); } @@ -795,15 +774,11 @@ fn nothing_is_proposed_or_applied_while_the_vault_winds_down() { f.advance(MIN_DELAY); refused( - f.oracle - .try_propose_change(&OracleChange::Config(config()), &f.admin), - OracleError::WindDownActive, - ); - refused( - f.oracle.try_apply_change(&id, &f.admin), + f.oracle.try_propose_change(&OracleChange::Config(config())), OracleError::WindDownActive, ); - f.oracle.cancel_change(&id, &f.admin); + refused(f.oracle.try_apply_change(&id), OracleError::WindDownActive); + f.oracle.cancel_change(&id); } #[test] @@ -821,7 +796,7 @@ fn a_change_shorter_than_a_raised_notice_never_lands() { f.advance(30 * DAY_DURATION); refused( - f.oracle.try_apply_change(&id, &f.admin), + f.oracle.try_apply_change(&id), OracleError::DelayBelowNotice, ); } @@ -833,10 +808,8 @@ fn one_upgrade_waits_at_a_time() { f.propose(OracleChange::Upgrade(BytesN::from_array(&f.e, &[7; 32]))); refused( - f.oracle.try_propose_change( - &OracleChange::Upgrade(BytesN::from_array(&f.e, &[9; 32])), - &f.admin, - ), + f.oracle + .try_propose_change(&OracleChange::Upgrade(BytesN::from_array(&f.e, &[9; 32]))), OracleError::UpgradeAlreadyPending, ); } @@ -853,21 +826,13 @@ fn only_the_admin_proposes_cancels_and_applies() { f.sign_only_as( &stranger, "propose_change", - (change.clone(), stranger.clone()).into_val(&f.e), - ); - assert!(f.oracle.try_propose_change(&change, &stranger).is_err()); - f.sign_only_as( - &stranger, - "cancel_change", - (id, stranger.clone()).into_val(&f.e), - ); - assert!(f.oracle.try_cancel_change(&id, &stranger).is_err()); - f.sign_only_as( - &stranger, - "apply_change", - (id, stranger.clone()).into_val(&f.e), + (change.clone(),).into_val(&f.e), ); - assert!(f.oracle.try_apply_change(&id, &stranger).is_err()); + assert!(f.oracle.try_propose_change(&change).is_err()); + f.sign_only_as(&stranger, "cancel_change", (id,).into_val(&f.e)); + assert!(f.oracle.try_cancel_change(&id).is_err()); + f.sign_only_as(&stranger, "apply_change", (id,).into_val(&f.e)); + assert!(f.oracle.try_apply_change(&id).is_err()); assert_eq!(f.oracle.pending_changes().len(), 1); } @@ -894,7 +859,7 @@ fn proposing_and_applying_publish_the_change() { ); f.advance(MIN_DELAY); - f.oracle.apply_change(&id, &f.admin); + f.oracle.apply_change(&id); assert_eq!( f.e.events().all().filter_by_contract(&f.oracle.address), [ @@ -959,17 +924,106 @@ fn setup_grants_hand_overs_and_changes_restore_the_instance_ttl() { .transfer_admin_role(&next, &(f.e.ledger().sequence() + ADMIN_ACCEPT_LEDGERS)) }); f.restores_the_instance(|| f.oracle.transfer_admin_role(&next, &0)); - f.restores_the_instance(|| f.oracle.bind_vault(&f.vault.address, &f.admin)); - f.restores_the_instance(|| f.oracle.finish_setup(&f.admin)); + f.restores_the_instance(|| f.oracle.bind_vault(&f.vault.address)); + f.restores_the_instance(|| f.oracle.finish_setup()); let change = OracleChange::Config(config()); let mut id = 0; f.restores_the_instance(|| id = f.propose(change.clone())); - f.restores_the_instance(|| f.oracle.cancel_change(&id, &f.admin)); + f.restores_the_instance(|| f.oracle.cancel_change(&id)); let id = f.propose(change); f.advance(MIN_DELAY); - f.restores_the_instance(|| f.oracle.apply_change(&id, &f.admin)); + f.restores_the_instance(|| f.oracle.apply_change(&id)); +} + +#[test] +fn each_admin_call_asks_the_admin_for_one_signature() { + let f = setup(); + let holder = Address::generate(&f.e); + let live_until = f.e.ledger().sequence() + ADMIN_ACCEPT_LEDGERS; + + f.oracle.set_config(&config()); + f.signed_once_by(&f.admin, "set_config", (config(),).into_val(&f.e)); + f.oracle.grant_role(&holder, &ATTESTER_ROLE, &f.admin); + f.signed_once_by( + &f.admin, + "grant_role", + (holder.clone(), ATTESTER_ROLE, f.admin.clone()).into_val(&f.e), + ); + f.oracle.revoke_role(&holder, &ATTESTER_ROLE, &f.admin); + f.signed_once_by( + &f.admin, + "revoke_role", + (holder.clone(), ATTESTER_ROLE, f.admin.clone()).into_val(&f.e), + ); + f.oracle.transfer_admin_role(&holder, &live_until); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (holder.clone(), live_until).into_val(&f.e), + ); + f.oracle.transfer_admin_role(&holder, &0); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (holder.clone(), 0_u32).into_val(&f.e), + ); + + f.oracle.bind_vault(&f.vault.address); + f.signed_once_by( + &f.admin, + "bind_vault", + (f.vault.address.clone(),).into_val(&f.e), + ); + f.oracle.finish_setup(); + f.signed_once_by(&f.admin, "finish_setup", soroban_sdk::Vec::new(&f.e)); + + let change = OracleChange::Config(config()); + let id = f.oracle.propose_change(&change); + f.signed_once_by(&f.admin, "propose_change", (change.clone(),).into_val(&f.e)); + f.oracle.cancel_change(&id); + f.signed_once_by(&f.admin, "cancel_change", (id,).into_val(&f.e)); + let id = f.oracle.propose_change(&change); + f.advance(MIN_DELAY); + f.oracle.apply_change(&id); + f.signed_once_by(&f.admin, "apply_change", (id,).into_val(&f.e)); + + f.oracle.pause_nav(&f.guardian); + f.oracle.clear_latest(); + f.signed_once_by(&f.admin, "clear_latest", soroban_sdk::Vec::new(&f.e)); + f.oracle.set_nav_paused(&false); + f.signed_once_by(&f.admin, "set_nav_paused", (false,).into_val(&f.e)); +} + +#[test] +fn each_role_call_asks_its_holder_for_one_signature() { + let f = setup(); + let next = Address::generate(&f.e); + + f.oracle.attest(&report(SCALE), &f.attester); + f.signed_once_by( + &f.attester, + "attest", + (report(SCALE), f.attester.clone()).into_val(&f.e), + ); + f.oracle.pause_nav(&f.guardian); + f.signed_once_by( + &f.guardian, + "pause_nav", + (f.guardian.clone(),).into_val(&f.e), + ); + f.oracle.renounce_role(&ATTESTER_ROLE, &f.attester); + f.signed_once_by( + &f.attester, + "renounce_role", + (ATTESTER_ROLE, f.attester.clone()).into_val(&f.e), + ); + + f.oracle + .transfer_admin_role(&next, &(f.e.ledger().sequence() + ADMIN_ACCEPT_LEDGERS)); + f.oracle.accept_admin_transfer(); + f.signed_once_by(&next, "accept_admin_transfer", soroban_sdk::Vec::new(&f.e)); } #[cfg(feature = "upgrade_wasm")] @@ -1041,15 +1095,15 @@ mod with_wasm { let id = f.propose(OracleChange::Upgrade(probe_hash)); f.advance(MIN_DELAY); - f.oracle.apply_change(&id, &f.admin); + f.oracle.apply_change(&id); let probe = oracle_probe::Client::new(&f.e, &f.oracle.address); assert!(probe.probe()); assert_eq!(probe.schema_version(), 1); - probe.migrate(&f.admin); + probe.migrate(); assert_eq!(probe.schema_version(), 2); assert_eq!( - probe.try_migrate(&f.admin), + probe.try_migrate(), Err(Ok(oracle_probe::ProbeError::AlreadyMigrated.into())) ); diff --git a/contracts/nav-oracle/src/test_guards.rs b/contracts/nav-oracle/src/test_guards.rs index 934ff514..63b8989f 100644 --- a/contracts/nav-oracle/src/test_guards.rs +++ b/contracts/nav-oracle/src/test_guards.rs @@ -37,15 +37,14 @@ struct Fixture<'a> { e: Env, oracle: NavOracleContractClient<'a>, vault: Address, - admin: Address, attester: Address, guardian: Address, } impl Fixture<'_> { fn finish_setup(&self) { - self.oracle.bind_vault(&self.vault, &self.admin); - self.oracle.finish_setup(&self.admin); + self.oracle.bind_vault(&self.vault); + self.oracle.finish_setup(); } fn attest(&self, nav_per_share: i128) { @@ -82,7 +81,7 @@ fn setup<'a>() -> Fixture<'a> { &e, &e.register( NavOracleContract, - (admin.clone(), attester.clone(), guardian.clone(), config()), + (admin, attester.clone(), guardian.clone(), config()), ), ); let vault = e.register(VaultStub, ()); @@ -90,7 +89,6 @@ fn setup<'a>() -> Fixture<'a> { e, oracle, vault, - admin, attester, guardian, } @@ -270,7 +268,7 @@ fn not_a_second_upgrade_refuses_an_upgrade_while_one_waits() { let grant = OracleChange::GrantRole(symbol_short!("attester"), Address::generate(&f.e)); holds(&f, || guards::not_a_second_upgrade(&f.e, &upgrade)); - f.oracle.propose_change(&upgrade, &f.admin); + f.oracle.propose_change(&upgrade); refuses(&f, OracleError::UpgradeAlreadyPending, || { guards::not_a_second_upgrade(&f.e, &upgrade) }); diff --git a/contracts/share-token/src/contract.rs b/contracts/share-token/src/contract.rs index f5a70045..28f98927 100644 --- a/contracts/share-token/src/contract.rs +++ b/contracts/share-token/src/contract.rs @@ -55,31 +55,27 @@ impl ShareToken { /// Names the vault whose clock this token's timelock reads. Once, and only /// during setup. #[only_admin] - pub fn bind_vault(e: &Env, vault: Address, caller: Address) { - caller.require_auth(); + pub fn bind_vault(e: &Env, vault: Address) { strata_storage::bump_instance(e); governance::bind_vault(e, &vault); } /// Ends the window in which admin changes are instant. It never reopens. #[only_admin] - pub fn finish_setup(e: &Env, caller: Address) { - caller.require_auth(); + pub fn finish_setup(e: &Env) { strata_storage::bump_instance(e); governance::finish_setup(e); } /// Queues a change behind the vault's timelock and returns its id. #[only_admin] - pub fn propose_change(e: &Env, change: TokenChange, caller: Address) -> u32 { - caller.require_auth(); + pub fn propose_change(e: &Env, change: TokenChange) -> u32 { strata_storage::bump_instance(e); governance::propose(e, change) } #[only_admin] - pub fn cancel_change(e: &Env, id: u32, caller: Address) { - caller.require_auth(); + pub fn cancel_change(e: &Env, id: u32) { strata_storage::bump_instance(e); governance::cancel(e, id); } @@ -87,8 +83,7 @@ impl ShareToken { /// Lands a change whose delay has run. Refused while the vault is paused /// or winding down. #[only_admin] - pub fn apply_change(e: &Env, id: u32, caller: Address) { - caller.require_auth(); + pub fn apply_change(e: &Env, id: u32) { strata_storage::bump_instance(e); governance::apply(e, id); } @@ -139,14 +134,14 @@ impl VaultBinding for ShareToken { #[contractimpl(contracttrait)] impl Pausable for ShareToken { - #[only_admin] fn pause(e: &Env, caller: Address) { + guards::caller_is_admin(e, &caller); caller.require_auth(); strata_storage::bump_instance(e); pausable::pause(e); } - #[only_admin] fn unpause(e: &Env, caller: Address) { + guards::caller_is_admin(e, &caller); caller.require_auth(); strata_storage::bump_instance(e); pausable::unpause(e); @@ -272,15 +267,15 @@ impl RWAToken for ShareToken { strata_storage::bump_instance(e); RWA::unfreeze_partial_tokens(e, &user_address, amount); } - #[only_admin] fn set_compliance(e: &Env, compliance: Address, operator: Address) { + guards::caller_is_admin(e, &operator); operator.require_auth(); guards::direct_change_allowed(e); strata_storage::bump_instance(e); RWA::set_compliance(e, &compliance); } - #[only_admin] fn set_identity_verifier(e: &Env, identity_verifier: Address, operator: Address) { + guards::caller_is_admin(e, &operator); operator.require_auth(); guards::direct_change_allowed(e); strata_storage::bump_instance(e); @@ -301,8 +296,7 @@ pub enum ProbeError { #[contractimpl] impl ShareToken { #[only_admin] - pub fn migrate(e: &Env, caller: Address) { - caller.require_auth(); + pub fn migrate(e: &Env) { strata_storage::bump_instance(e); if upgradeable::get_schema_version(e) >= 2 { panic_with_error!(e, ProbeError::AlreadyMigrated); diff --git a/contracts/share-token/src/error.rs b/contracts/share-token/src/error.rs index 31353a62..7d9779b5 100644 --- a/contracts/share-token/src/error.rs +++ b/contracts/share-token/src/error.rs @@ -6,9 +6,11 @@ use soroban_sdk::contracterror; #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[repr(u32)] pub enum ShareTokenError { - /// The token must always have an admin: `pause`, `unpause` and role - /// management need it. Hand it over instead: propose and apply an `Admin` - /// change (`transfer_admin_role` during setup), then the nominee calls + /// The call needs the admin. `pause`, `unpause`, `set_compliance` and + /// `set_identity_verifier` refuse any other caller. `renounce_admin` is + /// always refused, because the token must always have an admin: hand it + /// over instead by proposing and applying an `Admin` change + /// (`transfer_admin_role` during setup), then the nominee calls /// `accept_admin_transfer`. AdminRequired = 7000, /// No pending change has this id. diff --git a/contracts/share-token/src/guards.rs b/contracts/share-token/src/guards.rs index 4f09e2cb..fcd26a7f 100644 --- a/contracts/share-token/src/guards.rs +++ b/contracts/share-token/src/guards.rs @@ -61,6 +61,13 @@ fn upgrade_pending(e: &Env) -> bool { }) } +/// Stands in for `#[only_admin]` where an OpenZeppelin trait fixes the caller. +pub(crate) fn caller_is_admin(e: &Env, caller: &Address) { + if access_control::get_admin(e).as_ref() != Some(caller) { + panic_with_error!(e, ShareTokenError::AdminRequired); + } +} + /// The admin signs too, and is a different account from the caller. pub(crate) fn admin_co_signs(e: &Env, caller: &Address) { let admin = access_control::enforce_admin_auth(e); diff --git a/contracts/share-token/src/test.rs b/contracts/share-token/src/test.rs index 7a4a67b0..52137224 100644 --- a/contracts/share-token/src/test.rs +++ b/contracts/share-token/src/test.rs @@ -1,7 +1,12 @@ +extern crate std; + use soroban_sdk::{ symbol_short, - testutils::{storage::Instance as _, Address as _, Ledger as _, MockAuth, MockAuthInvoke}, - Address, Env, IntoVal, String, Symbol, + testutils::{ + storage::Instance as _, Address as _, AuthorizedFunction, AuthorizedInvocation, + Ledger as _, MockAuth, MockAuthInvoke, + }, + Address, Env, IntoVal, String, Symbol, Val, Vec, }; use stellar_access::access_control::{self as access_control, AccessControlError}; use stellar_tokens::rwa::RWAError; @@ -41,7 +46,7 @@ fn manager_mints_to_allowlisted_account() { token.grant_role(&manager, &symbol_short!("manager"), &admin); compliance.bind_token(&token.address, &admin); // compliance rejects ops from unbound tokens - identity.allow(&receiver, &true, &admin); + identity.allow(&receiver, &true); token.mint(&receiver, &100, &manager); @@ -49,7 +54,6 @@ fn manager_mints_to_allowlisted_account() { } struct Fixture<'a> { - #[allow(dead_code)] e: Env, token: ShareTokenClient<'a>, admin: Address, @@ -61,6 +65,25 @@ struct Fixture<'a> { identity_verifier: Address, } +impl Fixture<'_> { + /// The last call asked each of `signers`, in order, to sign it once, and nobody else. + fn signed_once_by(&self, signers: &[&Address], fn_name: &str, args: Vec) { + let call = AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + self.token.address.clone(), + Symbol::new(&self.e, fn_name), + args, + )), + sub_invocations: std::vec![], + }; + let expected: std::vec::Vec<_> = signers + .iter() + .map(|who| ((*who).clone(), call.clone())) + .collect(); + assert_eq!(self.e.auths(), expected); + } +} + fn setup_fixture(e: &Env) -> Fixture<'_> { let admin = Address::generate(e); let manager = Address::generate(e); @@ -281,7 +304,7 @@ fn the_compliance_authority_freezes_and_moves_balances() { let (f, identity) = setup_live(&e); let holder = Address::generate(&e); let target = Address::generate(&e); - identity.allow(&holder, &true, &f.admin); + identity.allow(&holder, &true); f.token.mint(&holder, &1_000, &f.manager); f.token @@ -298,7 +321,7 @@ fn the_compliance_authority_freezes_and_moves_balances() { .unfreeze_partial_tokens(&holder, &400, &f.compliance_authority); assert_eq!(f.token.get_frozen_tokens(&holder), 0); - identity.allow(&target, &true, &f.admin); + identity.allow(&target, &true); f.token.legal_transfer( &holder, &target, @@ -315,7 +338,7 @@ fn the_compliance_authority_reaches_recovery() { let (f, identity) = setup_live(&e); let lost = Address::generate(&e); let replacement = Address::generate(&e); - identity.allow(&replacement, &true, &f.admin); + identity.allow(&replacement, &true); // The role check passes; the testnet verifier names no recovery target. assert_eq!( @@ -368,7 +391,7 @@ fn the_manager_escrows_but_does_not_intervene() { let (f, identity) = setup_live(&e); let holder = Address::generate(&e); let new_holder = Address::generate(&e); - identity.allow(&holder, &true, &f.admin); + identity.allow(&holder, &true); f.token.mint(&holder, &1_000, &f.manager); assert_unauthorized(f.token.try_set_address_frozen(&holder, &true, &f.manager)); @@ -437,8 +460,8 @@ fn decayed_live_token(e: &Env) -> (Fixture<'_>, Address, Address) { let (f, identity) = setup_live(e); let holder = Address::generate(e); let other = Address::generate(e); - identity.allow(&holder, &true, &f.admin); - identity.allow(&other, &true, &f.admin); + identity.allow(&holder, &true); + identity.allow(&other, &true); f.token.mint(&holder, &100, &f.manager); age(e, TWO_DAYS_IN_LEDGERS); @@ -533,6 +556,63 @@ fn a_legal_transfer_restores_the_instance_ttl() { assert_eq!(instance_ttl(&e, &f.token.address), INSTANCE_EXTEND_AMOUNT); } +#[test] +fn each_role_call_asks_its_holder_for_one_signature() { + let e = Env::default(); + let (f, identity) = setup_live(&e); + let holder = Address::generate(&e); + let other = Address::generate(&e); + identity.allow(&holder, &true); + identity.allow(&other, &true); + + f.token.mint(&holder, &1_000, &f.manager); + f.signed_once_by( + &[&f.manager], + "mint", + (holder.clone(), 1_000_i128, f.manager.clone()).into_val(&e), + ); + f.token.burn(&holder, &100, &f.manager); + f.signed_once_by( + &[&f.manager], + "burn", + (holder.clone(), 100_i128, f.manager.clone()).into_val(&e), + ); + f.token.forced_transfer(&holder, &other, &100, &f.manager); + f.signed_once_by( + &[&f.manager], + "forced_transfer", + (holder.clone(), other, 100_i128, f.manager.clone()).into_val(&e), + ); + + let compliance = &f.compliance_authority; + f.token.freeze_partial_tokens(&holder, &50, compliance); + f.signed_once_by( + &[compliance], + "freeze_partial_tokens", + (holder.clone(), 50_i128, compliance.clone()).into_val(&e), + ); + f.token.unfreeze_partial_tokens(&holder, &50, compliance); + f.signed_once_by( + &[compliance], + "unfreeze_partial_tokens", + (holder.clone(), 50_i128, compliance.clone()).into_val(&e), + ); + f.token.set_address_frozen(&holder, &true, compliance); + f.signed_once_by( + &[compliance], + "set_address_frozen", + (holder, true, compliance.clone()).into_val(&e), + ); + + let role = Symbol::new(&e, "compliance"); + f.token.renounce_role(&role, compliance); + f.signed_once_by( + &[compliance], + "renounce_role", + (role, compliance.clone()).into_val(&e), + ); +} + mod legal_transfer { use soroban_sdk::{ contract, contractimpl, contracttype, @@ -576,11 +656,11 @@ mod legal_transfer { let destination = Address::generate(e); let vault = e.register(VaultStub, ()); for who in [&holder, &destination, &vault] { - identity.allow(who, &true, &f.admin); + identity.allow(who, &true); } f.token.mint(&holder, &1_000, &f.manager); f.token.mint(&vault, &500, &f.manager); - f.token.bind_vault(&vault, &f.admin); + f.token.bind_vault(&vault); (f, holder, destination, vault) } @@ -878,6 +958,41 @@ mod legal_transfer { assert_eq!(f.token.balance(&destination), 1_000); } + #[test] + fn a_legal_transfer_or_a_recovery_asks_each_signer_once() { + let e = Env::default(); + let (f, holder, destination, _) = live(&e); + + f.token.legal_transfer( + &holder, + &destination, + &250, + &order(&e), + &f.compliance_authority, + ); + f.signed_once_by( + &[&f.compliance_authority, &f.admin], + "legal_transfer", + ( + holder.clone(), + destination.clone(), + 250_i128, + order(&e), + f.compliance_authority.clone(), + ) + .into_val(&e), + ); + + recovering_to(&e, &f, &destination); + f.token + .recover_balance(&holder, &destination, &f.compliance_authority); + f.signed_once_by( + &[&f.compliance_authority, &f.admin], + "recover_balance", + (holder, destination, f.compliance_authority.clone()).into_val(&e), + ); + } + #[test] fn recovery_never_takes_the_vault_balance() { let e = Env::default(); diff --git a/contracts/share-token/src/test_governance.rs b/contracts/share-token/src/test_governance.rs index b0510654..4e670937 100644 --- a/contracts/share-token/src/test_governance.rs +++ b/contracts/share-token/src/test_governance.rs @@ -1,7 +1,10 @@ +extern crate std; + use soroban_sdk::{ contract, contractimpl, symbol_short, testutils::{ - storage::Instance as _, Address as _, Events as _, Ledger as _, MockAuth, MockAuthInvoke, + storage::Instance as _, Address as _, AuthorizedFunction, AuthorizedInvocation, + Events as _, Ledger as _, MockAuth, MockAuthInvoke, }, Address, BytesN, Env, Event as _, IntoVal, String, Symbol, }; @@ -66,19 +69,19 @@ impl Fixture<'_> { } fn finish(&self) { - self.token.bind_vault(&self.vault.address, &self.admin); - self.token.finish_setup(&self.admin); + self.token.bind_vault(&self.vault.address); + self.token.finish_setup(); } fn propose(&self, change: TokenChange) -> u32 { - self.token.propose_change(&change, &self.admin) + self.token.propose_change(&change) } /// Lands an `Admin` change for `next` and returns the ledger of the offer. fn offer_admin(&self, next: &Address) -> u32 { let id = self.propose(TokenChange::Admin(next.clone())); self.advance(MIN_DELAY); - self.token.apply_change(&id, &self.admin); + self.token.apply_change(&id); self.e.ledger().sequence() } @@ -100,7 +103,7 @@ impl Fixture<'_> { } /// Signs one call to the token as `who` and nothing else, so the call - /// carries the caller's signature but not the admin's. + /// carries that account's signature but not the admin's. fn sign_only_as(&self, who: &Address, fn_name: &str, args: soroban_sdk::Vec) { self.e.mock_auths(&[MockAuth { address: who, @@ -112,6 +115,29 @@ impl Fixture<'_> { }, }]); } + + /// The last call asked `who` alone to sign, once, on exactly this call. + fn signed_once_by( + &self, + who: &Address, + fn_name: &str, + args: soroban_sdk::Vec, + ) { + assert_eq!( + self.e.auths(), + std::vec![( + who.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + self.token.address.clone(), + Symbol::new(&self.e, fn_name), + args, + )), + sub_invocations: std::vec![], + }, + )] + ); + } } fn setup<'a>() -> Fixture<'a> { @@ -165,21 +191,15 @@ fn compliance(e: &Env) -> Symbol { fn setup_is_open_until_finished_and_never_reopens() { let f = setup(); assert!(f.token.setup_open()); - refused( - f.token.try_finish_setup(&f.admin), - ShareTokenError::VaultNotBound, - ); + refused(f.token.try_finish_setup(), ShareTokenError::VaultNotBound); f.finish(); assert!(!f.token.setup_open()); assert_eq!(f.token.vault(), Some(f.vault.address.clone())); + refused(f.token.try_finish_setup(), ShareTokenError::SetupClosed); refused( - f.token.try_finish_setup(&f.admin), - ShareTokenError::SetupClosed, - ); - refused( - f.token.try_bind_vault(&Address::generate(&f.e), &f.admin), + f.token.try_bind_vault(&Address::generate(&f.e)), ShareTokenError::SetupClosed, ); } @@ -187,10 +207,10 @@ fn setup_is_open_until_finished_and_never_reopens() { #[test] fn a_second_vault_cannot_be_bound() { let f = setup(); - f.token.bind_vault(&f.vault.address, &f.admin); + f.token.bind_vault(&f.vault.address); refused( - f.token.try_bind_vault(&Address::generate(&f.e), &f.admin), + f.token.try_bind_vault(&Address::generate(&f.e)), ShareTokenError::SetupClosed, ); assert_eq!(f.token.vault(), Some(f.vault.address.clone())); @@ -200,10 +220,7 @@ fn a_second_vault_cannot_be_bound() { fn an_address_that_reports_no_clock_cannot_be_bound() { let f = setup(); - assert!(f - .token - .try_bind_vault(&Address::generate(&f.e), &f.admin) - .is_err()); + assert!(f.token.try_bind_vault(&Address::generate(&f.e)).is_err()); assert_eq!(f.token.vault(), None); } @@ -215,19 +232,15 @@ fn only_the_admin_binds_and_finishes() { f.sign_only_as( &stranger, "bind_vault", - (f.vault.address.clone(), stranger.clone()).into_val(&f.e), + (f.vault.address.clone(),).into_val(&f.e), ); - assert!(f.token.try_bind_vault(&f.vault.address, &stranger).is_err()); + assert!(f.token.try_bind_vault(&f.vault.address).is_err()); assert_eq!(f.token.vault(), None); f.e.mock_all_auths(); - f.token.bind_vault(&f.vault.address, &f.admin); - f.sign_only_as( - &stranger, - "finish_setup", - (stranger.clone(),).into_val(&f.e), - ); - assert!(f.token.try_finish_setup(&stranger).is_err()); + f.token.bind_vault(&f.vault.address); + f.sign_only_as(&stranger, "finish_setup", soroban_sdk::Vec::new(&f.e)); + assert!(f.token.try_finish_setup().is_err()); assert!(f.token.setup_open()); } @@ -311,7 +324,7 @@ fn an_admin_hand_over_waits_but_cancelling_one_does_not() { let id = f.propose(TokenChange::Admin(next.clone())); f.advance(MIN_DELAY); - f.token.apply_change(&id, &f.admin); + f.token.apply_change(&id); f.token.transfer_admin_role(&next, &0); assert!(f.token.try_accept_admin_transfer().is_err()); @@ -322,17 +335,17 @@ fn an_admin_hand_over_waits_but_cancelling_one_does_not() { fn setup_cannot_finish_while_a_hand_over_is_pending() { let f = setup(); let next = Address::generate(&f.e); - f.token.bind_vault(&f.vault.address, &f.admin); + f.token.bind_vault(&f.vault.address); f.token .transfer_admin_role(&next, &(f.e.ledger().sequence() + 100)); refused( - f.token.try_finish_setup(&f.admin), + f.token.try_finish_setup(), ShareTokenError::AdminTransferPending, ); f.token.transfer_admin_role(&next, &0); - f.token.finish_setup(&f.admin); + f.token.finish_setup(); assert!(f.token.try_accept_admin_transfer().is_err()); assert_eq!(f.token.get_admin(), Some(f.admin.clone())); } @@ -340,12 +353,12 @@ fn setup_cannot_finish_while_a_hand_over_is_pending() { #[test] fn an_expired_hand_over_does_not_hold_setup_open() { let f = setup(); - f.token.bind_vault(&f.vault.address, &f.admin); + f.token.bind_vault(&f.vault.address); f.token .transfer_admin_role(&Address::generate(&f.e), &(f.e.ledger().sequence() + 10)); f.e.ledger().with_mut(|l| l.sequence_number += 11); - f.token.finish_setup(&f.admin); + f.token.finish_setup(); assert!(!f.token.setup_open()); assert_eq!( @@ -371,10 +384,8 @@ fn nothing_is_proposed_during_setup() { let f = setup(); refused( - f.token.try_propose_change( - &TokenChange::GrantRole(manager(), f.admin.clone()), - &f.admin, - ), + f.token + .try_propose_change(&TokenChange::GrantRole(manager(), f.admin.clone())), ShareTokenError::SetupOpen, ); assert_eq!(f.token.pending_changes().len(), 0); @@ -396,12 +407,12 @@ fn a_grant_lands_one_vault_delay_after_it_was_proposed() { f.advance(MIN_DELAY - 1); refused( - f.token.try_apply_change(&id, &f.admin), + f.token.try_apply_change(&id), ShareTokenError::ChangeNotReady, ); f.advance(1); - f.token.apply_change(&id, &f.admin); + f.token.apply_change(&id); assert!(f.token.has_role(&next, &manager()).is_some()); assert_eq!(f.token.pending_changes().len(), 0); } @@ -415,7 +426,7 @@ fn a_revoke_lands_through_the_timelock() { let id = f.propose(TokenChange::RevokeRole(manager(), holder.clone())); f.advance(MIN_DELAY); - f.token.apply_change(&id, &f.admin); + f.token.apply_change(&id); assert!(f.token.has_role(&holder, &manager()).is_none()); } @@ -433,10 +444,7 @@ fn a_vault_pause_moves_the_eta_and_holds_the_change() { f.clock(|c| c.paused_secs = 3 * DAY_DURATION); f.advance(MIN_DELAY); - refused( - f.token.try_apply_change(&id, &f.admin), - ShareTokenError::VaultPaused, - ); + refused(f.token.try_apply_change(&id), ShareTokenError::VaultPaused); f.clock(|c| c.paused = false); assert_eq!( @@ -444,7 +452,7 @@ fn a_vault_pause_moves_the_eta_and_holds_the_change() { eta + 3 * DAY_DURATION ); f.advance(3 * DAY_DURATION); - f.token.apply_change(&id, &f.admin); + f.token.apply_change(&id); } #[test] @@ -457,14 +465,14 @@ fn nothing_is_proposed_or_applied_while_the_vault_winds_down() { refused( f.token - .try_propose_change(&TokenChange::Compliance(Address::generate(&f.e)), &f.admin), + .try_propose_change(&TokenChange::Compliance(Address::generate(&f.e))), ShareTokenError::WindDownActive, ); refused( - f.token.try_apply_change(&id, &f.admin), + f.token.try_apply_change(&id), ShareTokenError::WindDownActive, ); - f.token.cancel_change(&id, &f.admin); + f.token.cancel_change(&id); } #[test] @@ -479,7 +487,7 @@ fn a_change_shorter_than_a_raised_notice_never_lands() { f.advance(30 * DAY_DURATION); refused( - f.token.try_apply_change(&id, &f.admin), + f.token.try_apply_change(&id), ShareTokenError::DelayBelowNotice, ); } @@ -491,10 +499,8 @@ fn one_upgrade_waits_at_a_time() { f.propose(TokenChange::Upgrade(BytesN::from_array(&f.e, &[7; 32]))); refused( - f.token.try_propose_change( - &TokenChange::Upgrade(BytesN::from_array(&f.e, &[9; 32])), - &f.admin, - ), + f.token + .try_propose_change(&TokenChange::Upgrade(BytesN::from_array(&f.e, &[9; 32]))), ShareTokenError::UpgradeAlreadyPending, ); } @@ -511,21 +517,13 @@ fn only_the_admin_proposes_cancels_and_applies() { f.sign_only_as( &stranger, "propose_change", - (change.clone(), stranger.clone()).into_val(&f.e), - ); - assert!(f.token.try_propose_change(&change, &stranger).is_err()); - f.sign_only_as( - &stranger, - "cancel_change", - (id, stranger.clone()).into_val(&f.e), - ); - assert!(f.token.try_cancel_change(&id, &stranger).is_err()); - f.sign_only_as( - &stranger, - "apply_change", - (id, stranger.clone()).into_val(&f.e), + (change.clone(),).into_val(&f.e), ); - assert!(f.token.try_apply_change(&id, &stranger).is_err()); + assert!(f.token.try_propose_change(&change).is_err()); + f.sign_only_as(&stranger, "cancel_change", (id,).into_val(&f.e)); + assert!(f.token.try_cancel_change(&id).is_err()); + f.sign_only_as(&stranger, "apply_change", (id,).into_val(&f.e)); + assert!(f.token.try_apply_change(&id).is_err()); assert_eq!(f.token.pending_changes().len(), 1); } @@ -537,26 +535,9 @@ fn an_applied_admin_change_starts_a_hand_over_the_next_admin_accepts() { let id = f.propose(TokenChange::Admin(next.clone())); f.advance(MIN_DELAY); - // The admin authorises this call and nothing else: the hand-over inside it - // asks for no further signature. The entrypoint checks the admin and the - // caller, so the same authorisation appears twice, as simulation records it. - let apply = MockAuthInvoke { - contract: &f.token.address, - fn_name: "apply_change", - args: (id, f.admin.clone()).into_val(&f.e), - sub_invokes: &[], - }; - f.e.mock_auths(&[ - MockAuth { - address: &f.admin, - invoke: &apply, - }, - MockAuth { - address: &f.admin, - invoke: &apply, - }, - ]); - f.token.apply_change(&id, &f.admin); + // One admin signature covers the call; the hand-over inside asks for none. + f.sign_only_as(&f.admin, "apply_change", (id,).into_val(&f.e)); + f.token.apply_change(&id); f.e.mock_all_auths(); assert_eq!(f.token.get_admin(), Some(f.admin.clone())); @@ -604,8 +585,8 @@ fn applied_rule_and_verifier_changes_take_effect() { let second = f.propose(TokenChange::IdentityVerifier(verifier.clone())); f.advance(MIN_DELAY); - f.token.apply_change(&first, &f.admin); - f.token.apply_change(&second, &f.admin); + f.token.apply_change(&first); + f.token.apply_change(&second); assert_eq!(f.token.compliance(), rules); assert_eq!(f.token.identity_verifier(), verifier); @@ -630,7 +611,7 @@ fn proposing_and_applying_publish_the_change() { ); f.advance(MIN_DELAY); - f.token.apply_change(&id, &f.admin); + f.token.apply_change(&id); let events = f.e.events().all().filter_by_contract(&f.token.address); assert_eq!( events.events().last(), @@ -686,15 +667,115 @@ fn every_admin_call_restores_the_instance_ttl() { .transfer_admin_role(&next, &(f.e.ledger().sequence() + ADMIN_ACCEPT_LEDGERS)) }); f.restores_the_instance(|| f.token.transfer_admin_role(&next, &0)); - f.restores_the_instance(|| f.token.bind_vault(&f.vault.address, &f.admin)); - f.restores_the_instance(|| f.token.finish_setup(&f.admin)); + f.restores_the_instance(|| f.token.bind_vault(&f.vault.address)); + f.restores_the_instance(|| f.token.finish_setup()); let rules = TokenChange::Compliance(Address::generate(&f.e)); let mut id = 0; f.restores_the_instance(|| id = f.propose(rules.clone())); - f.restores_the_instance(|| f.token.cancel_change(&id, &f.admin)); + f.restores_the_instance(|| f.token.cancel_change(&id)); let id = f.propose(rules); f.advance(MIN_DELAY); - f.restores_the_instance(|| f.token.apply_change(&id, &f.admin)); + f.restores_the_instance(|| f.token.apply_change(&id)); +} + +#[test] +fn each_admin_call_asks_the_admin_for_one_signature() { + let f = setup(); + let holder = Address::generate(&f.e); + let rules = Address::generate(&f.e); + let verifier = Address::generate(&f.e); + let live_until = f.e.ledger().sequence() + ADMIN_ACCEPT_LEDGERS; + + f.token.pause(&f.admin); + f.signed_once_by(&f.admin, "pause", (f.admin.clone(),).into_val(&f.e)); + f.token.unpause(&f.admin); + f.signed_once_by(&f.admin, "unpause", (f.admin.clone(),).into_val(&f.e)); + f.token.set_compliance(&rules, &f.admin); + f.signed_once_by( + &f.admin, + "set_compliance", + (rules.clone(), f.admin.clone()).into_val(&f.e), + ); + f.token.set_identity_verifier(&verifier, &f.admin); + f.signed_once_by( + &f.admin, + "set_identity_verifier", + (verifier, f.admin.clone()).into_val(&f.e), + ); + f.token.grant_role(&holder, &manager(), &f.admin); + f.signed_once_by( + &f.admin, + "grant_role", + (holder.clone(), manager(), f.admin.clone()).into_val(&f.e), + ); + f.token.revoke_role(&holder, &manager(), &f.admin); + f.signed_once_by( + &f.admin, + "revoke_role", + (holder.clone(), manager(), f.admin.clone()).into_val(&f.e), + ); + f.token.transfer_admin_role(&holder, &live_until); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (holder.clone(), live_until).into_val(&f.e), + ); + f.token.transfer_admin_role(&holder, &0); + f.signed_once_by( + &f.admin, + "transfer_admin_role", + (holder.clone(), 0_u32).into_val(&f.e), + ); + + f.token.bind_vault(&f.vault.address); + f.signed_once_by( + &f.admin, + "bind_vault", + (f.vault.address.clone(),).into_val(&f.e), + ); + f.token.finish_setup(); + f.signed_once_by(&f.admin, "finish_setup", soroban_sdk::Vec::new(&f.e)); + + let change = TokenChange::Compliance(rules); + let id = f.token.propose_change(&change); + f.signed_once_by(&f.admin, "propose_change", (change.clone(),).into_val(&f.e)); + f.token.cancel_change(&id); + f.signed_once_by(&f.admin, "cancel_change", (id,).into_val(&f.e)); + + let id = f.token.propose_change(&TokenChange::Admin(holder.clone())); + f.advance(MIN_DELAY); + f.token.apply_change(&id); + f.signed_once_by(&f.admin, "apply_change", (id,).into_val(&f.e)); + f.token.accept_admin_transfer(); + f.signed_once_by( + &holder, + "accept_admin_transfer", + soroban_sdk::Vec::new(&f.e), + ); +} + +#[test] +fn a_caller_other_than_the_admin_is_refused_even_when_it_signs() { + let f = setup(); + let stranger = Address::generate(&f.e); + + refused(f.token.try_pause(&stranger), ShareTokenError::AdminRequired); + f.token.pause(&f.admin); + refused( + f.token.try_unpause(&stranger), + ShareTokenError::AdminRequired, + ); + refused( + f.token + .try_set_compliance(&Address::generate(&f.e), &stranger), + ShareTokenError::AdminRequired, + ); + refused( + f.token + .try_set_identity_verifier(&Address::generate(&f.e), &stranger), + ShareTokenError::AdminRequired, + ); + assert!(f.token.paused()); } diff --git a/contracts/share-token/src/test_guards.rs b/contracts/share-token/src/test_guards.rs index 48a3f995..fa7a6208 100644 --- a/contracts/share-token/src/test_guards.rs +++ b/contracts/share-token/src/test_guards.rs @@ -42,8 +42,8 @@ struct Fixture<'a> { impl Fixture<'_> { fn finish_setup(&self) { - self.token.bind_vault(&self.vault, &self.admin); - self.token.finish_setup(&self.admin); + self.token.bind_vault(&self.vault); + self.token.finish_setup(); } } @@ -126,13 +126,26 @@ fn not_a_second_upgrade_refuses_an_upgrade_while_one_waits() { let grant = TokenChange::GrantRole(symbol_short!("manager"), Address::generate(&f.e)); holds(&f, || guards::not_a_second_upgrade(&f.e, &upgrade)); - f.token.propose_change(&upgrade, &f.admin); + f.token.propose_change(&upgrade); refuses(&f, ShareTokenError::UpgradeAlreadyPending, || { guards::not_a_second_upgrade(&f.e, &upgrade) }); holds(&f, || guards::not_a_second_upgrade(&f.e, &grant)); } +#[test] +fn caller_is_admin_refuses_anyone_but_the_admin() { + let f = setup(); + let stranger = Address::generate(&f.e); + holds(&f, || guards::caller_is_admin(&f.e, &f.admin)); + + for who in [&f.compliance_authority, &stranger] { + refuses(&f, ShareTokenError::AdminRequired, || { + guards::caller_is_admin(&f.e, who) + }); + } +} + #[test] fn admin_co_signs_refuses_one_account_signing_as_both() { let f = setup(); @@ -179,7 +192,7 @@ fn not_vault_address_refuses_the_bound_vault() { let holder = Address::generate(&f.e); holds(&f, || guards::not_vault_address(&f.e, &f.vault)); - f.token.bind_vault(&f.vault, &f.admin); + f.token.bind_vault(&f.vault); refuses(&f, ShareTokenError::VaultAddress, || { guards::not_vault_address(&f.e, &f.vault) }); @@ -190,7 +203,7 @@ fn not_vault_address_refuses_the_bound_vault() { fn not_freezing_vault_refuses_only_a_freeze_of_the_vault() { let f = setup(); let holder = Address::generate(&f.e); - f.token.bind_vault(&f.vault, &f.admin); + f.token.bind_vault(&f.vault); refuses(&f, ShareTokenError::VaultAddress, || { guards::not_freezing_vault(&f.e, &f.vault, true) @@ -204,7 +217,7 @@ fn identity_verified_refuses_an_account_the_verifier_rejects() { let f = setup(); let listed = Address::generate(&f.e); let stranger = Address::generate(&f.e); - f.identity.allow(&listed, &true, &f.admin); + f.identity.allow(&listed, &true); holds(&f, || guards::identity_verified(&f.e, &listed)); refuses(&f, RWAError::IdentityVerificationFailed, || { diff --git a/docs/AUTHORIZATION_MATRIX.md b/docs/AUTHORIZATION_MATRIX.md index 8b97446f..877f5bfa 100644 --- a/docs/AUTHORIZATION_MATRIX.md +++ b/docs/AUTHORIZATION_MATRIX.md @@ -7,7 +7,11 @@ This document provides the complete authorization and role model specification f ## 1. Architectural Principles 1. **Direct Role Membership Checks**: Role membership is checked directly via `has_role(e, &caller, &role)` or `stellar_macros::only_role` / `only_admin`, never by enumerating members. -2. **Strict Caller Authentication**: No entrypoint takes a caller argument that it does not authenticate. Every function accepting a `caller: Address` executes `caller.require_auth()`. The testnet compliance stub is the exception: its admin calls check the stored admin's signature and ignore their `caller` or `operator` argument (§5.5). +2. **One signature per account**: A privileged call asks each account it needs for exactly one signature, on that call. No production entrypoint takes a caller argument it does not check. + - An admin entrypoint the kit defines takes no caller argument. `#[only_admin]` reads the stored admin and asks it to sign. + - Where an OpenZeppelin trait fixes a `caller` or `operator` argument (`pause`, `unpause`, `set_compliance`, `set_identity_verifier`), the entrypoint refuses any account other than the stored admin with `AdminRequired`, then asks that account to sign. + - A role entrypoint checks the role of the `caller` or `operator` it names, then asks that account to sign. `legal_transfer` and `recover_balance` also ask the admin, a different account, to sign the same call. + - The testnet stubs are the exception. The compliance contract's `bind_token`, `unbind_token`, `add_module_to` and `remove_module_from`, and the identity verifier's `set_claim_topics_and_issuers`, keep the `operator` argument their trait fixes but do not read it. `#[only_admin]` asks the stored admin to sign, so each still asks for one signature. Production replaces both stubs with external contracts (§5.4, §5.5). 3. **Exit guarantee, with one hold**: A covered redemption claim pays out cash regardless of pause, staleness or allowlist status. A delisted investor exits through the cash path and cannot re-enter share circulation. A frozen investor is held: the vault refuses their requests, cancellations and claims, and pays them only after the freeze lifts. 4. **Five Separate Authorities**: - **Governance (Admin)**: Root authority; upgrades, timelock parameters, custodian settings, unpause, oracle parameters, emergency NAV pause reset. As the share token's admin, it co-signs compliance's `legal_transfer` and `recover_balance`. @@ -36,10 +40,10 @@ In `scripts/harness/deploy.ts` and automated deployment scripts, the separation - The share token's constructor grants `compliance` to the compliance key and grants `manager` to no one; the identity verifier is constructed with the compliance key as admin. - The share token role is granted to the deployed vault contract: `token.grant_role({ account: asyncVault.address, role: "manager", caller: governance })`. - After granting, the script checks `token.has_role({ account: vault, role: "manager" })` and stops if the vault does not hold the role. That no other account holds it is checked by the vault's `finish_setup` below. -- It then calls `token.bind_vault({ vault, caller: governance })` and `token.finish_setup({ caller: 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, caller: governance })` and `compliance.finish_setup({ caller: governance })`, 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, caller: governance })` and `oracle.finish_setup({ caller: governance })`, 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, caller: governance })` and `vault.finish_setup({ caller: governance })`, 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). +- 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). --- @@ -79,28 +83,29 @@ The following entrypoints are open to any caller on purpose: The vault's `admin` is governance. Until `finish_setup`, admin changes are instant and the vault takes no deposit (`SetupOpen`). Setup finishes once the wiring is complete and no admin hand-over offered during setup is pending, and it never reopens. After setup, the following wait for the timelock through `propose_change`/`apply_change`: the custodian, grants, revoking `manager`, admin hand-over, lowering the notice or the wind-down delay, the oracle, and upgrades. Raising the notice or the wind-down delay, the deposit cap, `pause`/`unpause` and revoking `treasury` or `guardian` stay instant. During an active wind-down nothing timelocked is proposed or applied, a `manager` revoke included. -Tests: [`controls.rs`](../contracts/async-vault/src/test/controls.rs), [`epochs.rs`](../contracts/async-vault/src/test/epochs.rs), [`governance.rs`](../contracts/async-vault/src/test/governance.rs), [`notice.rs`](../contracts/async-vault/src/test/notice.rs), [`setup.rs`](../contracts/async-vault/src/test/setup.rs), [`treasury.rs`](../contracts/async-vault/src/test/treasury.rs), [`upgrade.rs`](../contracts/async-vault/src/test/upgrade.rs) and [`wind_down.rs`](../contracts/async-vault/src/test/wind_down.rs) in [`contracts/async-vault/src/test/`](../contracts/async-vault/src/test/). +Tests: [`controls.rs`](../contracts/async-vault/src/test/controls.rs), [`epochs.rs`](../contracts/async-vault/src/test/epochs.rs), [`governance.rs`](../contracts/async-vault/src/test/governance.rs), [`notice.rs`](../contracts/async-vault/src/test/notice.rs), [`setup.rs`](../contracts/async-vault/src/test/setup.rs), [`treasury.rs`](../contracts/async-vault/src/test/treasury.rs), [`upgrade.rs`](../contracts/async-vault/src/test/upgrade.rs) and [`wind_down.rs`](../contracts/async-vault/src/test/wind_down.rs) in [`contracts/async-vault/src/test/`](../contracts/async-vault/src/test/). [`signatures.rs`](../contracts/async-vault/src/test/signatures.rs) checks that each privileged call below asks one account for one signature, on that call only. | Entrypoint | Access Control / Caller Auth | State & Precondition Guards | Tested Refusal (Test Name) | |---|---|---|---| | `close_epoch` | `#[only_role(caller, "manager")]` | Epoch must be open | `test::epochs::a_non_manager_cannot_close` | | `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` | `#[only_admin]`, `caller.require_auth()` | Contract must be paused | `test::controls::the_guardian_pauses_but_only_governance_unpauses` | -| `set_custodian` | `#[only_admin]`, `caller.require_auth()` | Instant during setup; `UseTimelock` after | `test::governance::the_custodian_changes_through_the_timelock`, `test::treasury::setting_the_custodian_needs_the_admin_signature` | -| `set_notice` | `#[only_admin]`, `caller.require_auth()` | `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]`, `caller.require_auth()` | `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]`, `caller.require_auth()` | Not negative. Instant both ways, also after setup | `test::governance::the_deposit_cap_stays_instant_both_ways` | -| `finish_setup` | `#[only_admin]`, `caller.require_auth()` | 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` | +| `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_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` | | `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`) | +| `renounce_role` | The role holder, `caller.require_auth()` (OpenZeppelin's default) | The caller holds the role (`RoleNotHeld`); instant for every role, `manager` included | `test::signatures::each_role_call_asks_its_holder_for_one_signature` | | `set_role_admin` | — | Always refused (`UseTimelock`) | `test::governance::no_role_ever_gets_a_role_admin` | -| `propose_wind_down` | `#[only_admin]`, `caller.require_auth()` | No proposal standing, not active | `test::wind_down::only_governance_proposes` | -| `cancel_wind_down_proposal` | `#[only_admin]`, `caller.require_auth()` | Must be proposed, not active | `test::wind_down::governance_cancels_a_proposal_before_activation` | -| `propose_change` | `#[only_admin]`, `caller.require_auth()` | 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]`, `caller.require_auth()` | The change must be pending | `test::upgrade::only_governance_proposes_and_cancels` | -| `apply_change` | `#[only_admin]`, `caller.require_auth()` | 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_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` | +| `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` | | `pending_changes` | Open view | None | None (view) | @@ -110,12 +115,12 @@ Tests: [`controls.rs`](../contracts/async-vault/src/test/controls.rs), [`epochs. The token's `admin` is governance; the guardian holds no role on the token. Until `finish_setup`, admin changes are instant, so the deploy can grant the vault the `manager` role. Setup cannot finish while an admin hand-over offered during setup is pending. After setup, the following wait for the vault's timelock through `propose_change`/`apply_change`: grants, revoking `manager`, admin hand-over, `set_compliance`, `set_identity_verifier` and upgrades. Revoking `compliance` and `pause`/`unpause` stay instant. The `manager` role is held by the vault and the `compliance` role by the compliance authority. The token's `pause` refuses SEP-41 transfers between holders only. The vault's own escrow moves (`forced_transfer`), mint and burn ignore it, so a token pause stops neither entry nor exit. -Tests: [`contracts/share-token/src/test.rs`](../contracts/share-token/src/test.rs) and [`contracts/share-token/src/test_governance.rs`](../contracts/share-token/src/test_governance.rs). +Tests: [`contracts/share-token/src/test.rs`](../contracts/share-token/src/test.rs) and [`contracts/share-token/src/test_governance.rs`](../contracts/share-token/src/test_governance.rs). What each privileged call asks to sign: `test_governance::each_admin_call_asks_the_admin_for_one_signature`, `test::each_role_call_asks_its_holder_for_one_signature` and `test::legal_transfer::a_legal_transfer_or_a_recovery_asks_each_signer_once`. | Entrypoint | Access Control / Caller Auth | State & Precondition Guards | Tested Refusal (Test Name) | |---|---|---|---| -| `pause` | `#[only_admin]`, `caller.require_auth()` | Not paused | `test::unauthorized_caller_cannot_pause_or_unpause` | -| `unpause` | `#[only_admin]`, `caller.require_auth()` | Paused | `test::unauthorized_caller_cannot_pause_or_unpause` | +| `pause` | `caller` must be the stored admin (`AdminRequired`), then `caller.require_auth()` | Not paused | `test::unauthorized_caller_cannot_pause_or_unpause`, `test_governance::a_caller_other_than_the_admin_is_refused_even_when_it_signs` | +| `unpause` | `caller` must be the stored admin (`AdminRequired`), then `caller.require_auth()` | Paused | `test::unauthorized_caller_cannot_pause_or_unpause`, `test_governance::a_caller_other_than_the_admin_is_refused_even_when_it_signs` | | `mint` | `#[only_role(operator, "manager")]` | Receiver allowlisted | `test::unauthorized_caller_cannot_mint`, `test::the_compliance_authority_cannot_mint_burn_or_swap_the_rules` | | `burn` | `#[only_role(operator, "manager")]` | Balance sufficient | `test::unauthorized_caller_cannot_burn` | | `forced_transfer` | `#[only_role(operator, "manager")]` | Balance sufficient | `test::unauthorized_caller_cannot_forced_transfer`, `test::legal_transfer::compliance_no_longer_forces_transfers` | @@ -124,17 +129,18 @@ Tests: [`contracts/share-token/src/test.rs`](../contracts/share-token/src/test.r | `freeze_partial_tokens` | `#[only_role(operator, "compliance")]` | Balance sufficient; never the bound vault's address (`VaultAddress`) | `test::unauthorized_caller_cannot_freeze_or_recover`, `test::the_manager_escrows_but_does_not_intervene`, `test::legal_transfer::the_vault_address_cannot_be_frozen` | | `unfreeze_partial_tokens`| `#[only_role(operator, "compliance")]` | Frozen amount sufficient | `test::unauthorized_caller_cannot_freeze_or_recover`, `test::the_manager_escrows_but_does_not_intervene` | | `recover_balance` | `#[only_role(operator, "compliance")]` and the admin's `require_auth()` in the same call, from two different accounts (`CoSignRequired`) | Never from or to the vault's address (`VaultAddress`); target allowlisted and named by the verifier's `recovery_target` (the testnet stub names none, so recovery always fails there) | `test::unauthorized_caller_cannot_freeze_or_recover`, `test::the_compliance_authority_reaches_recovery`, `test::legal_transfer::recovery_needs_the_admin_too`, `test::legal_transfer::recovery_never_takes_the_vault_balance`, `test::legal_transfer::recovery_never_lands_on_the_vault_address` | -| `set_compliance` | `#[only_admin]`, `operator.require_auth()` | Instant during setup; `UseTimelock` after | `test::unauthorized_caller_cannot_set_compliance_or_verifier`, `test::the_compliance_authority_cannot_mint_burn_or_swap_the_rules`, `test_governance::rule_and_verifier_swaps_wait_after_setup` | -| `set_identity_verifier` | `#[only_admin]`, `operator.require_auth()` | Instant during setup; `UseTimelock` after | `test::unauthorized_caller_cannot_set_compliance_or_verifier`, `test::the_compliance_authority_cannot_mint_burn_or_swap_the_rules`, `test_governance::rule_and_verifier_swaps_wait_after_setup` | +| `set_compliance` | `operator` must be the stored admin (`AdminRequired`), then `operator.require_auth()` | Instant during setup; `UseTimelock` after | `test::unauthorized_caller_cannot_set_compliance_or_verifier`, `test::the_compliance_authority_cannot_mint_burn_or_swap_the_rules`, `test_governance::rule_and_verifier_swaps_wait_after_setup`, `test_governance::a_caller_other_than_the_admin_is_refused_even_when_it_signs` | +| `set_identity_verifier` | `operator` must be the stored admin (`AdminRequired`), then `operator.require_auth()` | Instant during setup; `UseTimelock` after | `test::unauthorized_caller_cannot_set_compliance_or_verifier`, `test::the_compliance_authority_cannot_mint_burn_or_swap_the_rules`, `test_governance::rule_and_verifier_swaps_wait_after_setup`, `test_governance::a_caller_other_than_the_admin_is_refused_even_when_it_signs` | | `renounce_admin` | Refused always (`ShareTokenError::AdminRequired`) | None | `test::the_admin_cannot_renounce_itself_out_of_the_token`; `test::admin_handover_in_two_steps_still_works`; `test_governance::an_applied_admin_change_starts_a_hand_over_the_next_admin_accepts`. A token without an admin could never be unpaused or rotate its manager; hand over by proposing and applying an `Admin` change (`transfer_admin_role` during setup), then the nominee calls `accept_admin_transfer`. | -| `bind_vault` | `#[only_admin]`, `caller.require_auth()` | During setup, once; the vault must answer `timelock_clock` | `test_governance::a_second_vault_cannot_be_bound`, `test_governance::an_address_that_reports_no_clock_cannot_be_bound` | -| `finish_setup` | `#[only_admin]`, `caller.require_auth()` | Needs a bound vault and no pending admin hand-over; once | `test_governance::setup_is_open_until_finished_and_never_reopens`, `test_governance::setup_cannot_finish_while_a_hand_over_is_pending` | -| `propose_change` | `#[only_admin]`, `caller.require_auth()` | After setup; vault not winding down; one upgrade at a time | `test_governance::nothing_is_proposed_during_setup`, `test_governance::only_the_admin_proposes_cancels_and_applies` | -| `cancel_change` | `#[only_admin]`, `caller.require_auth()` | The change must be pending | `test_governance::only_the_admin_proposes_cancels_and_applies` | -| `apply_change` | `#[only_admin]`, `caller.require_auth()` | Delay run on the vault's clock, vault unpaused and not winding down, delay `>=` the vault's notice | `test_governance::a_vault_pause_moves_the_eta_and_holds_the_change`, `test_governance::a_change_shorter_than_a_raised_notice_never_lands` | +| `bind_vault` | `#[only_admin]` | During setup, once; the vault must answer `timelock_clock` | `test_governance::a_second_vault_cannot_be_bound`, `test_governance::an_address_that_reports_no_clock_cannot_be_bound` | +| `finish_setup` | `#[only_admin]` | Needs a bound vault and no pending admin hand-over; once | `test_governance::setup_is_open_until_finished_and_never_reopens`, `test_governance::setup_cannot_finish_while_a_hand_over_is_pending` | +| `propose_change` | `#[only_admin]` | After setup; vault not winding down; one upgrade at a time | `test_governance::nothing_is_proposed_during_setup`, `test_governance::only_the_admin_proposes_cancels_and_applies` | +| `cancel_change` | `#[only_admin]` | The change must be pending | `test_governance::only_the_admin_proposes_cancels_and_applies` | +| `apply_change` | `#[only_admin]` | Delay run on the vault's clock, vault unpaused and not winding down, delay `>=` the vault's notice | `test_governance::a_vault_pause_moves_the_eta_and_holds_the_change`, `test_governance::a_change_shorter_than_a_raised_notice_never_lands` | | `pending_changes` | Open view | None | None (view) | | `grant_role` | Admin, `caller.require_auth()` | Instant during setup; `UseTimelock` after | `test_governance::grants_and_manager_revokes_wait_after_setup` | | `revoke_role` | Admin, `caller.require_auth()` | `compliance`: instant. `manager`: instant during setup, `UseTimelock` after | `test_governance::grants_and_manager_revokes_wait_after_setup` (the instant case: `test_governance::revoking_compliance_stays_instant_after_setup`) | +| `renounce_role` | The role holder, `caller.require_auth()` (OpenZeppelin's default) | The caller holds the role (`RoleNotHeld`); instant | `test::each_role_call_asks_its_holder_for_one_signature` | | `transfer_admin_role` | Admin | Instant during setup; `UseTimelock` after. A zero `live_until_ledger` cancels at once | `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_governance::setup_cannot_finish_while_a_hand_over_is_pending` | | `set_role_admin` | Refused always (`ShareTokenError::UseTimelock`) | None | `test_governance::no_role_ever_gets_a_role_admin`. A role admin could grant its role with no delay. | @@ -144,24 +150,25 @@ Tests: [`contracts/share-token/src/test.rs`](../contracts/share-token/src/test.r The oracle's `admin` is governance; its attester and guardian are constructor parameters. Until `finish_setup`, admin changes are instant. Setup cannot finish while an admin hand-over offered during setup is pending. After setup, the following wait for the vault's timelock through `propose_change`/`apply_change`: grants, admin hand-over, every `set_config` change except a shorter `freshness_duration`, and upgrades. Revoking `attester` or `guardian`, the NAV pause (`pause_nav`, `set_nav_paused`) and `clear_latest` stay instant. -Tests: [`contracts/nav-oracle/src/test.rs`](../contracts/nav-oracle/src/test.rs) and [`contracts/nav-oracle/src/test_governance.rs`](../contracts/nav-oracle/src/test_governance.rs). +Tests: [`contracts/nav-oracle/src/test.rs`](../contracts/nav-oracle/src/test.rs) and [`contracts/nav-oracle/src/test_governance.rs`](../contracts/nav-oracle/src/test_governance.rs). What each privileged call asks to sign: `test_governance::each_admin_call_asks_the_admin_for_one_signature` and `test_governance::each_role_call_asks_its_holder_for_one_signature`. | Entrypoint | Access Control / Caller Auth | State & Precondition Guards | Tested Refusal (Test Name) | |---|---|---|---| | `attest` | `#[only_role(caller, "attester")]` | Refused while the NAV is paused; cooldown, deviation cap, bounds `[min, max]`; expiry capped at the freshness window | `test::attest_is_role_gated`, `test::attesting_is_refused_while_the_nav_is_paused` | | `pause_nav` | `#[only_role(caller, "guardian")]` | None; instant after setup too | `test::pausing_the_nav_is_limited_to_the_guardian` (the instant case: `test_governance::the_nav_pause_and_clearing_stay_instant_after_setup`) | -| `set_nav_paused` | `#[only_admin]`, `caller.require_auth()` | None; instant after setup too | `test::the_guardian_pauses_the_nav_but_unpausing_needs_governance` | -| `clear_latest` | `#[only_admin]`, `caller.require_auth()` | NAV must be paused; instant after setup too | `test::only_admin_can_clear_latest`, `test::the_record_clears_only_while_the_nav_is_paused` | -| `set_config` | `#[only_admin]` (no caller argument; the stored admin signs) | Valid bounds. Instant while setup is open, or when the change is only a shorter `freshness_duration`; `UseTimelock` otherwise. Applying a `Config` change sets every field, freshness included; to keep a cut made after the proposal, cancel the queued change and propose again | `test::only_admin_can_set_config`, `test_governance::a_tighter_cap_or_band_waits_after_setup`, `test_governance::a_config_that_tightens_one_field_and_loosens_another_waits`, `test_governance::a_shorter_freshness_window_with_a_looser_field_waits`, `test_governance::a_freshness_cut_below_the_cooldown_is_refused` | +| `set_nav_paused` | `#[only_admin]` | None; instant after setup too | `test::the_guardian_pauses_the_nav_but_unpausing_needs_governance` | +| `clear_latest` | `#[only_admin]` | NAV must be paused; instant after setup too | `test::only_admin_can_clear_latest`, `test::the_record_clears_only_while_the_nav_is_paused` | +| `set_config` | `#[only_admin]` | Valid bounds. Instant while setup is open, or when the change is only a shorter `freshness_duration`; `UseTimelock` otherwise. Applying a `Config` change sets every field, freshness included; to keep a cut made after the proposal, cancel the queued change and propose again | `test::only_admin_can_set_config`, `test_governance::a_tighter_cap_or_band_waits_after_setup`, `test_governance::a_config_that_tightens_one_field_and_loosens_another_waits`, `test_governance::a_shorter_freshness_window_with_a_looser_field_waits`, `test_governance::a_freshness_cut_below_the_cooldown_is_refused` | | `renounce_admin` | Refused always (`OracleError::AdminRequired`) | None | `test::the_oracle_admin_cannot_renounce_itself_away`; `test_governance::an_applied_admin_change_starts_a_hand_over_the_next_admin_accepts`. An oracle without an admin could never be reconfigured, clear a stuck record or unpause the NAV; hand over by proposing and applying an `Admin` change (`transfer_admin_role` during setup), then the nominee calls `accept_admin_transfer`. | -| `bind_vault` | `#[only_admin]`, `caller.require_auth()` | During setup, once; the vault must answer `timelock_clock` | `test_governance::setup_is_open_until_finished_and_never_reopens`, `test_governance::a_second_vault_cannot_be_bound`, `test_governance::an_address_that_reports_no_clock_cannot_be_bound` | -| `finish_setup` | `#[only_admin]`, `caller.require_auth()` | Needs a bound vault and no pending admin hand-over; once | `test_governance::setup_is_open_until_finished_and_never_reopens`, `test_governance::setup_cannot_finish_while_a_hand_over_is_pending` | -| `propose_change` | `#[only_admin]`, `caller.require_auth()` | After setup; vault not winding down; one upgrade at a time; a `Config` change must pass the constructor's bounds | `test_governance::nothing_is_proposed_during_setup`, `test_governance::an_invalid_config_cannot_be_proposed`, `test_governance::only_the_admin_proposes_cancels_and_applies` | -| `cancel_change` | `#[only_admin]`, `caller.require_auth()` | The change must be pending | `test_governance::only_the_admin_proposes_cancels_and_applies` | -| `apply_change` | `#[only_admin]`, `caller.require_auth()` | Delay run on the vault's clock, vault unpaused and not winding down, delay `>=` the vault's notice; a `Config` change must still pass the bounds | `test_governance::a_vault_pause_moves_the_eta_and_holds_the_change`, `test_governance::nothing_is_proposed_or_applied_while_the_vault_winds_down`, `test_governance::a_change_shorter_than_a_raised_notice_never_lands`, `test_governance::a_config_outside_the_bounds_is_refused_when_it_applies` | +| `bind_vault` | `#[only_admin]` | During setup, once; the vault must answer `timelock_clock` | `test_governance::setup_is_open_until_finished_and_never_reopens`, `test_governance::a_second_vault_cannot_be_bound`, `test_governance::an_address_that_reports_no_clock_cannot_be_bound` | +| `finish_setup` | `#[only_admin]` | Needs a bound vault and no pending admin hand-over; once | `test_governance::setup_is_open_until_finished_and_never_reopens`, `test_governance::setup_cannot_finish_while_a_hand_over_is_pending` | +| `propose_change` | `#[only_admin]` | After setup; vault not winding down; one upgrade at a time; a `Config` change must pass the constructor's bounds | `test_governance::nothing_is_proposed_during_setup`, `test_governance::an_invalid_config_cannot_be_proposed`, `test_governance::only_the_admin_proposes_cancels_and_applies` | +| `cancel_change` | `#[only_admin]` | The change must be pending | `test_governance::only_the_admin_proposes_cancels_and_applies` | +| `apply_change` | `#[only_admin]` | Delay run on the vault's clock, vault unpaused and not winding down, delay `>=` the vault's notice; a `Config` change must still pass the bounds | `test_governance::a_vault_pause_moves_the_eta_and_holds_the_change`, `test_governance::nothing_is_proposed_or_applied_while_the_vault_winds_down`, `test_governance::a_change_shorter_than_a_raised_notice_never_lands`, `test_governance::a_config_outside_the_bounds_is_refused_when_it_applies` | | `pending_changes` | Open view | None | None (view) | | `grant_role` | Admin, `caller.require_auth()` | Instant during setup; `UseTimelock` after | `test_governance::grants_wait_after_setup_and_land_through_the_timelock` | | `revoke_role` | Admin, `caller.require_auth()` | Instant: taking an attester or a guardian away moves no value | `test_governance::only_the_admin_revokes_a_role` (the instant case: `test_governance::revoking_an_attester_or_a_guardian_stays_instant_after_setup`) | +| `renounce_role` | The role holder, `caller.require_auth()` (OpenZeppelin's default) | The caller holds the role (`RoleNotHeld`); instant | `test_governance::each_role_call_asks_its_holder_for_one_signature` | | `transfer_admin_role` | Admin | Instant during setup; `UseTimelock` after. A zero `live_until_ledger` cancels at once | `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_governance::setup_cannot_finish_while_a_hand_over_is_pending` | | `set_role_admin` | Refused always (`OracleError::UseTimelock`) | None | `test_governance::no_role_ever_gets_a_role_admin`. A role admin could grant its role with no delay. | @@ -171,24 +178,25 @@ Tests: [`contracts/nav-oracle/src/test.rs`](../contracts/nav-oracle/src/test.rs) A testnet-only stub. Its admin is the compliance key and cannot be transferred: the stub exposes no access-control interface, so replacing the compliance key on it means redeploying. So the compliance key loses its power at once only on the share token, where its `compliance` role can be revoked instantly. On this stub it can still allowlist and delist addresses until the token's verifier is swapped for a new deployment; after setup that swap waits for the vault's timelock, at least seven days. -Tests: [`contracts/identity-verifier/src/test.rs`](../contracts/identity-verifier/src/test.rs). +Tests: [`contracts/identity-verifier/src/test.rs`](../contracts/identity-verifier/src/test.rs). What each admin call asks to sign: `test::each_admin_call_asks_the_admin_for_one_signature`. | Entrypoint | Access Control / Caller Auth | State & Precondition Guards | Tested Refusal (Test Name) | |---|---|---|---| -| `allow` | `#[only_admin]` (the compliance key), `caller.require_auth()` | None | `test::only_admin_can_allow` | +| `allow` | `#[only_admin]` (the compliance key) | None | `test::only_admin_can_allow` | +| `set_claim_topics_and_issuers` | `#[only_admin]` (the compliance key); keeps the `operator` the trait fixes and does not read it | None; it changes nothing | None (signature: `test::each_admin_call_asks_the_admin_for_one_signature`) | | `verify_identity` | Open check (Internal hook) | `is_allowed(account) == true` | `test::verify_identity_checks_allowlist` | ### 5.5 Compliance (`contracts/compliance`) -A testnet-only stub. Its admin is governance. Module changes, token bindings and admin hand-overs are instant during setup and wait for the vault's timelock after `finish_setup`. Each admin call checks the stored admin's signature; its `caller` or `operator` argument is not checked. +A testnet-only stub. Its admin is governance. Module changes, token bindings and admin hand-overs are instant during setup and wait for the vault's timelock after `finish_setup`. Each admin call asks the stored admin to sign. Its own entrypoints take no caller argument; its OpenZeppelin trait methods keep the `operator` argument their trait fixes and do not read it (principle 2). -Tests: [`contracts/compliance/src/test/changes.rs`](../contracts/compliance/src/test/changes.rs). +Tests: [`contracts/compliance/src/test/changes.rs`](../contracts/compliance/src/test/changes.rs). What each privileged call asks to sign: `test::changes::each_admin_call_asks_the_admin_for_one_signature`, `test::changes::a_hand_over_asks_each_side_for_one_signature` and `test::changes::each_hook_asks_the_bound_token_for_one_signature`. | Entrypoint | Access Control / Caller Auth | State & Precondition Guards | Tested Refusal (Test Name) | |---|---|---|---| | `bind_vault` | `#[only_admin]` | During setup, once; the vault must answer `timelock_clock` | `test::changes::a_second_vault_or_a_late_vault_is_refused`, `test::changes::an_address_that_reports_no_clock_cannot_be_bound`, `test::changes::only_the_admin_binds_and_finishes` | | `finish_setup` | `#[only_admin]` | Needs a bound vault and no pending admin hand-over; once | `test::changes::setup_cannot_finish_without_a_vault`, `test::changes::setup_cannot_finish_while_a_hand_over_is_pending`, `test::changes::a_second_vault_or_a_late_vault_is_refused` | -| `bind_token`, `unbind_token`, `add_module_to`, `remove_module_from` | `#[only_admin]` | Instant during setup; `UseTimelock` after | `test::changes::after_setup_rule_changes_wait_for_the_timelock` | +| `bind_token`, `unbind_token`, `add_module_to`, `remove_module_from` | `#[only_admin]`; `operator` is not read | Instant during setup; `UseTimelock` after | `test::changes::after_setup_rule_changes_wait_for_the_timelock` | | `propose_change` | `#[only_admin]` | After setup; vault not winding down | `test::changes::proposals_wait_until_setup_is_finished`, `test::changes::only_the_admin_proposes_cancels_and_applies` | | `cancel_change` | `#[only_admin]` | The change must be pending | `test::changes::a_cancelled_change_never_lands` | | `apply_change` | `#[only_admin]` | Delay run on the vault's clock, vault unpaused and not winding down, delay `>=` the vault's notice | `test::changes::a_proposed_binding_lands_after_the_delay`, `test::changes::nothing_lands_while_the_vault_is_paused_or_winding_down`, `test::changes::a_vault_pause_pushes_the_change_back`, `test::changes::a_change_shorter_than_a_raised_notice_never_lands` | diff --git a/docs/TESTING.md b/docs/TESTING.md index 334d7d35..833c17ae 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -75,15 +75,20 @@ membership is the contract's real state, not a mock. Most of these tests run with `mock_all_auths`, so every signature is granted and a refusal can only come from the role check. -**Whether a signature is required.** A refusal test clears all signatures with -`set_auths(&[])` and checks the call fails and changes nothing, which proves the -entrypoint requires the right address to sign. +**Which signatures a call needs.** A refusal test signs the call as another +account only (`mock_auths`), or clears all signatures with `set_auths(&[])`, +and checks the call fails and changes nothing, which proves the entrypoint +requires the right address to sign. A signature test reads `e.auths()` right +after a call and expects one entry per account the call needs, on that call +only. The per-entrypoint role matrix, with the refusal test for each, is in [AUTHORIZATION_MATRIX.md](./AUTHORIZATION_MATRIX.md). | Behaviour | Tests | Key test names | | --- | --- | --- | +| Every privileged call asks each account it needs for one signature: an admin call asks the admin once, a role call asks the role holder once, and `legal_transfer` and `recover_balance` ask compliance and the admin once each | [`contracts/async-vault/src/test/signatures.rs`](../contracts/async-vault/src/test/signatures.rs), [`contracts/share-token/src/test.rs`](../contracts/share-token/src/test.rs), [`contracts/share-token/src/test_governance.rs`](../contracts/share-token/src/test_governance.rs), [`contracts/nav-oracle/src/test_governance.rs`](../contracts/nav-oracle/src/test_governance.rs), [`contracts/compliance/src/test/changes.rs`](../contracts/compliance/src/test/changes.rs), [`contracts/identity-verifier/src/test.rs`](../contracts/identity-verifier/src/test.rs) | `each_setup_call_asks_the_admin_for_one_signature`, `each_admin_call_after_setup_asks_the_admin_for_one_signature`, `each_admin_call_asks_the_admin_for_one_signature`, `each_role_call_asks_its_holder_for_one_signature`, `a_legal_transfer_or_a_recovery_asks_each_signer_once`, `a_hand_over_asks_each_side_for_one_signature`, `each_hook_asks_the_bound_token_for_one_signature` | +| An account other than the admin cannot pause, unpause or swap the token's rules or verifier, or unpause the vault, even when it signs | [`contracts/async-vault/src/test/controls.rs`](../contracts/async-vault/src/test/controls.rs), [`contracts/share-token/src/test_governance.rs`](../contracts/share-token/src/test_governance.rs) | `a_caller_other_than_the_admin_cannot_unpause_even_when_it_signs`, `a_caller_other_than_the_admin_is_refused_even_when_it_signs` | | A deposit is one investor signature that covers the vault call and the asset transfer inside it | [`contracts/async-vault/src/test/deposit.rs`](../contracts/async-vault/src/test/deposit.rs) | `one_investor_signature_covers_the_request_and_the_transfer` | | Investor calls refuse without the investor's signature | [`contracts/async-vault/src/test/deposit.rs`](../contracts/async-vault/src/test/deposit.rs), [`contracts/async-vault/src/test/redeem.rs`](../contracts/async-vault/src/test/redeem.rs) | `request_deposit_needs_the_investor_authorisation`, `claiming_needs_the_investor_authorisation`, `request_redeem_needs_the_holder_authorisation` | | Role calls refuse without the role holder's signature | [`contracts/async-vault/src/test/epochs.rs`](../contracts/async-vault/src/test/epochs.rs), [`contracts/async-vault/src/test/notice.rs`](../contracts/async-vault/src/test/notice.rs), [`contracts/async-vault/src/test/treasury.rs`](../contracts/async-vault/src/test/treasury.rs), [`contracts/async-vault/src/test/deposit_cap.rs`](../contracts/async-vault/src/test/deposit_cap.rs) | `closing_needs_the_manager_authorisation`, `setting_the_notice_needs_governance_authorisation`, `setting_the_custodian_needs_the_admin_signature`, `unauthorized_caller_rejected` | diff --git a/docs/kit-spec.md b/docs/kit-spec.md index 7a974229..cdc7a37a 100644 --- a/docs/kit-spec.md +++ b/docs/kit-spec.md @@ -191,7 +191,7 @@ The `async_vault` constructor strictly verifies pairwise separation across autho ## 3. Role & Authority Matrix -Strata distributes control across five native Stellar multisig authorities and permissionless public entrypoints. Every privileged function is mapped to exactly one authority, except the share token's `legal_transfer` and `recover_balance`, which need compliance and governance in the same call. +Strata distributes control across five native Stellar multisig authorities and permissionless public entrypoints. Every privileged function is mapped to exactly one authority, except the share token's `legal_transfer` and `recover_balance`, which need compliance and governance in the same call. Each call asks each account it needs for one signature. An admin entrypoint the kit defines takes no caller argument: the stored admin signs it. ### Authority Capabilities & Anti-Capabilities @@ -220,7 +220,7 @@ Strata distributes control across five native Stellar multisig authorities and p | **`AsyncVault`** | `close_epoch` | Manager | `#[only_role(caller, "manager")]` | No | | **`AsyncVault`** | `fulfill_epoch` | Permissionless | None | Yes (halted by pause) | | **`AsyncVault`** | `pause` | Guardian | `#[only_role(caller, "guardian")]` | N/A | -| **`AsyncVault`** | `unpause` | Governance | `#[only_admin]` | 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_deposit_cap` | Governance | `#[only_admin]`; instant both ways | No | @@ -241,7 +241,7 @@ Strata distributes control across five native Stellar multisig authorities and p | **`AsyncVault`** | `grant_role`, `transfer_admin_role` | Governance | Instant during setup, then `propose_change` (a zero `live_until_ledger` cancels at once) | No | | **`AsyncVault`** | `revoke_role` | Governance | `treasury`, `guardian`: instant. `manager`: instant during setup, then `propose_change` | No | | **`AsyncVault`** | `set_role_admin` | None | Always refused (`UseTimelock`) | N/A | -| **`AsyncVault`** | `accept_admin_transfer` | Proposed Admin | `caller.require_auth()` | No | +| **`AsyncVault`** | `accept_admin_transfer` | Proposed Admin | The pending admin's `require_auth()` | No | | **`NavOracle`** | `attest` | Attester | `#[only_role(caller, "attester")]` | Refused if the NAV is paused | | **`NavOracle`** | `pause_nav` | Guardian | `#[only_role(caller, "guardian")]` | N/A | | **`NavOracle`** | `set_nav_paused` | Governance | `#[only_admin]` | N/A | @@ -256,15 +256,15 @@ Strata distributes control across five native Stellar multisig authorities and p | **`NavOracle`** | `grant_role`, `transfer_admin_role` | Governance | Instant during setup, then `propose_change` (a zero `live_until_ledger` cancels at once) | No | | **`NavOracle`** | `revoke_role` | Governance | Instant (`attester`, `guardian`) | No | | **`NavOracle`** | `set_role_admin` | None | Always refused (`UseTimelock`) | N/A | -| **`ShareToken`** | `pause` | Admin (Governance) | `#[only_admin]`, `caller.require_auth()` | N/A | -| **`ShareToken`** | `unpause` | Admin (Governance) | `#[only_admin]`, `caller.require_auth()` | N/A | +| **`ShareToken`** | `pause` | Admin (Governance) | `caller` must be the admin (`AdminRequired`), then `caller.require_auth()` | N/A | +| **`ShareToken`** | `unpause` | Admin (Governance) | `caller` must be the admin (`AdminRequired`), then `caller.require_auth()` | N/A | | **`ShareToken`** | `mint` | Manager (Vault) | `#[only_role(operator, "manager")]` | Bypasses token pause | | **`ShareToken`** | `burn` | Manager (Vault) | `#[only_role(operator, "manager")]` | Bypasses token pause | | **`ShareToken`** | `forced_transfer` | Manager (Vault) | `#[only_role(operator, "manager")]` | Bypasses token pause | | **`ShareToken`** | `legal_transfer` | Compliance, co-signed by Governance (token admin) | `#[only_role(caller, "compliance")]` and the admin's `require_auth()` in the same call, from two different accounts; never from or to the vault's address; the destination passes `verify_identity`; a reference of 1–128 bytes | Bypasses token pause | | **`ShareToken`** | `set_address_frozen`, `freeze_partial_tokens`, `unfreeze_partial_tokens` | Compliance | `#[only_role(operator, "compliance")]`; never freezes the vault's address once the token knows its vault | No | | **`ShareToken`** | `recover_balance` | Compliance, co-signed by Governance (token admin) | `#[only_role(operator, "compliance")]` and the admin's `require_auth()` in the same call, from two different accounts; never from or to the vault's address | No | -| **`ShareToken`** | `set_compliance`, `set_identity_verifier` | Governance | `#[only_admin]`; instant during setup, then `propose_change` | No | +| **`ShareToken`** | `set_compliance`, `set_identity_verifier` | Governance | `operator` must be the admin (`AdminRequired`), then `operator.require_auth()`; instant during setup, then `propose_change` | No | | **`ShareToken`** | `bind_vault` | Governance (token admin) | `#[only_admin]`; during setup, once | No | | **`ShareToken`** | `finish_setup` | Governance | `#[only_admin]`; needs a bound vault and no pending admin hand-over | No | | **`ShareToken`** | `propose_change` | Governance | `#[only_admin]`; after setup | No | @@ -275,8 +275,8 @@ Strata distributes control across five native Stellar multisig authorities and p | **`ShareToken`** | `revoke_role` | Governance | `compliance`: instant. `manager`: instant during setup, then `propose_change` | No | | **`ShareToken`** | `set_role_admin` | None | Always refused (`UseTimelock`) | N/A | | **`IdentityVerifier`** | `allow` | Admin (Compliance) | `#[only_admin]` | No | -| **`IdentityVerifier`** | `set_claim_topics_and_issuers`| Admin (Compliance) | `#[only_admin]` | No | -| **`Compliance`** | `bind_token`, `unbind_token`, `add_module_to`, `remove_module_from` | Governance | `#[only_admin]`; instant during setup, then `propose_change` | No | +| **`IdentityVerifier`** | `set_claim_topics_and_issuers`| Admin (Compliance) | `#[only_admin]`; `operator` is not read | No | +| **`Compliance`** | `bind_token`, `unbind_token`, `add_module_to`, `remove_module_from` | Governance | `#[only_admin]`, `operator` is not read; instant during setup, then `propose_change` | No | | **`Compliance`** | `bind_vault` | Governance (compliance admin) | `#[only_admin]`; during setup, once | No | | **`Compliance`** | `finish_setup` | Governance | `#[only_admin]`; needs a bound vault and no pending admin hand-over | No | | **`Compliance`** | `propose_change` | Governance | `#[only_admin]`; after setup | No | @@ -295,10 +295,10 @@ A clean deployment executes in three sequenced phases: 1. **Phase 1: Contract Deployment** - Deploy `compliance`, `identity_verifier`, `share_token`, `nav_oracle`, and `async_vault` with constructor arguments matching `environments.toml`. 2. **Phase 2: Post-Deployment Role Wiring** - - Call `ShareToken::grant_role("manager", vault_address)` to authorize lazy share minting and burning. + - Call `ShareToken::grant_role(vault_address, "manager", governance)` to authorize lazy share minting and burning. - Call `ShareToken::bind_vault(vault_address)`, then `ShareToken::finish_setup()`. The token's admin changes are instant until then and wait for the vault's timelock afterwards. - 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)` 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 `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. diff --git a/scripts/harness/deploy.ts b/scripts/harness/deploy.ts index 89154ba1..af37bc9e 100644 --- a/scripts/harness/deploy.ts +++ b/scripts/harness/deploy.ts @@ -162,8 +162,8 @@ async function grantAndVerify( console.log("vault holds share_token manager role") // Grants are instant only during setup, so the token's setup closes after them. - await (await token.bind_vault({ vault, caller })).signAndSend() - await (await token.finish_setup({ caller })).signAndSend() + await (await token.bind_vault({ vault })).signAndSend() + await (await token.finish_setup()).signAndSend() if ((await token.setup_open()).result) throw new Error("the share token's setup did not finish") console.log("share_token bound to the vault, setup finished") @@ -178,7 +178,6 @@ async function grantAndVerify( await verifier.allow({ account: vault, allowed: true, - caller: accounts.compliance.publicKey(), }) ).signAndSend() const allowed = (await verifier.is_allowed({ account: vault })).result @@ -205,8 +204,8 @@ async function grantAndVerify( // Binding the token is instant only during setup, so the compliance // contract's setup closes after it. - await (await compliance.bind_vault({ vault, caller })).signAndSend() - await (await compliance.finish_setup({ caller })).signAndSend() + await (await compliance.bind_vault({ vault })).signAndSend() + await (await compliance.finish_setup()).signAndSend() if ((await compliance.setup_open()).result) throw new Error("the compliance contract's setup did not finish") console.log("compliance bound to the vault, setup finished") @@ -217,8 +216,8 @@ async function grantAndVerify( const feed = new NavOracle( clientOptions(net, contracts.nav_oracle, governance), ) - await (await feed.bind_vault({ vault, caller })).signAndSend() - await (await feed.finish_setup({ caller })).signAndSend() + await (await feed.bind_vault({ vault })).signAndSend() + await (await feed.finish_setup()).signAndSend() if ((await feed.setup_open()).result) throw new Error("the oracle's setup did not finish") console.log("nav_oracle bound to the vault, setup finished") @@ -232,7 +231,6 @@ async function finishVaultSetup( contracts: Record, ): Promise { const governance = accounts.governance - const caller = governance.publicKey() const { Client: AsyncVault } = (await import( clientEntry("async_vault") )) as typeof AsyncVaultModule @@ -242,10 +240,9 @@ async function finishVaultSetup( await ( await vault.set_custodian({ custodian: accounts.custodian.publicKey(), - caller, }) ).signAndSend() - await (await vault.finish_setup({ caller })).signAndSend() + await (await vault.finish_setup()).signAndSend() if ((await vault.setup_open()).result) throw new Error("the vault's setup did not finish") console.log("vault setup finished") diff --git a/scripts/harness/guards.test.ts b/scripts/harness/guards.test.ts index 5cdc3ab9..3cc0639f 100644 --- a/scripts/harness/guards.test.ts +++ b/scripts/harness/guards.test.ts @@ -62,7 +62,6 @@ beforeAll(async () => { verifier.allow({ account: holder.publicKey(), allowed: true, - caller: compliance.publicKey(), }), ) } @@ -90,7 +89,6 @@ describe("an account with no role", () => { send( vault(c, outsider).propose_change({ change: { tag: "Custodian", values: [who] }, - caller: who, }), ), ).rejects.toThrow(/signatures/) @@ -125,27 +123,30 @@ describe("an account with no role", () => { it("cannot unpause the vault", async () => { await send(vault(c, guardian).pause({ caller: guardian.publicKey() })) - await expect( - send(vault(c, outsider).unpause({ caller: who })), - ).rejects.toThrow(/signatures/) + const v = vault(c, outsider) + expectRefused(await v.unpause({ caller: who }), v, "AdminRequired") await send(vault(c, governance).unpause({ caller: governance.publicKey() })) }) + it("cannot pause the share token", async () => { + const token = shareToken(c, outsider) + expectRefused(await token.pause({ caller: who }), token, "AdminRequired") + }) + it("cannot set the NAV pause", async () => { await expect( - send(oracle(c, outsider).set_nav_paused({ paused: true, caller: who })), + send(oracle(c, outsider).set_nav_paused({ paused: true })), ).rejects.toThrow(/signatures/) }) it("cannot clear the latest report", async () => { await send(oracle(c, guardian).pause_nav({ caller: guardian.publicKey() })) - await expect( - send(oracle(c, outsider).clear_latest({ caller: who })), - ).rejects.toThrow(/signatures/) + await expect(send(oracle(c, outsider).clear_latest())).rejects.toThrow( + /signatures/, + ) await send( oracle(c, governance).set_nav_paused({ paused: false, - caller: governance.publicKey(), }), ) }) @@ -167,7 +168,6 @@ describe("the compliance authority's reach", () => { identityVerifier(c, governance).allow({ account: outsider.publicKey(), allowed: true, - caller: governance.publicKey(), }), ), ).rejects.toThrow(/signatures/) @@ -209,7 +209,6 @@ describe("the compliance authority's reach", () => { tag: "IdentityVerifier", values: [c.deployment.contracts.compliance], }, - caller: compliance.publicKey(), }), ), ).rejects.toThrow(/signatures/) @@ -237,22 +236,15 @@ describe("the share token after setup", () => { const id = await send( token.propose_change({ change: { tag: "IdentityVerifier", values: [verifier()] }, - caller: caller(), }), ) - expectRefused( - await token.apply_change({ id, caller: caller() }), - token, - "ChangeNotReady", - ) - await send(token.cancel_change({ id, caller: caller() })) + expectRefused(await token.apply_change({ id }), token, "ChangeNotReady") + await send(token.cancel_change({ id })) expect((await token.pending_changes()).result).toEqual([]) }) }) describe("the oracle after setup", () => { - const caller = () => governance.publicKey() - it("governance cannot loosen the feed config at once", async () => { const config = (await feed.config()).result const o = oracle(c, governance) @@ -289,15 +281,10 @@ describe("the oracle after setup", () => { tag: "Config", values: [{ ...config, max_down_bps: config.max_down_bps - 1 }], }, - caller: caller(), }), ) - expectRefused( - await o.apply_change({ id, caller: caller() }), - o, - "ChangeNotReady", - ) - await send(o.cancel_change({ id, caller: caller() })) + expectRefused(await o.apply_change({ id }), o, "ChangeNotReady") + await send(o.cancel_change({ id })) expect((await o.pending_changes()).result).toEqual([]) }) }) @@ -324,28 +311,20 @@ describe("the compliance contract after setup", () => { const id = await send( rules.propose_change({ change: { tag: "AddModule", values: [canCreate, outsider.publicKey()] }, - caller: caller(), }), ) - expectRefused( - await rules.apply_change({ id, caller: caller() }), - rules, - "ChangeNotReady", - ) - await send(rules.cancel_change({ id, caller: caller() })) + expectRefused(await rules.apply_change({ id }), rules, "ChangeNotReady") + await send(rules.cancel_change({ id })) expect((await rules.pending_changes()).result).toEqual([]) }) }) describe("the vault after setup", () => { - const caller = () => governance.publicKey() - it("governance cannot change the custodian at once", async () => { const v = vault(c, governance) expectRefused( await v.set_custodian({ custodian: outsider.publicKey(), - caller: caller(), }), v, "UseTimelock", @@ -358,15 +337,10 @@ describe("the vault after setup", () => { const id = await send( v.propose_change({ change: { tag: "Custodian", values: [outsider.publicKey()] }, - caller: caller(), }), ) - expectRefused( - await v.apply_change({ id, caller: caller() }), - v, - "ChangeNotReady", - ) - await send(v.cancel_change({ id, caller: caller() })) + expectRefused(await v.apply_change({ id }), v, "ChangeNotReady") + await send(v.cancel_change({ id })) expect((await v.pending_changes()).result).toEqual([]) }) }) @@ -619,7 +593,6 @@ describe("a de-listed holder", () => { identityVerifier(c, compliance).allow({ account: first.publicKey(), allowed: false, - caller: compliance.publicKey(), }), ) expectRefused( @@ -649,7 +622,6 @@ describe("a de-listed holder", () => { identityVerifier(c, compliance).allow({ account: first.publicKey(), allowed: true, - caller: compliance.publicKey(), }), ) expect( @@ -751,13 +723,11 @@ describe("the notice period and valuation age rules", () => { await send( oracle(c, governance).set_nav_paused({ paused: false, - caller: governance.publicKey(), }), ) await send( vault(c, governance).set_notice({ secs: noticeSecs, - caller: governance.publicKey(), }), ) }) @@ -832,11 +802,7 @@ describe("the notice period and valuation age rules", () => { it("governance cannot lower the notice at once", async () => { const v = vault(c, governance) - expectRefused( - await v.set_notice({ secs: 0n, caller: governance.publicKey() }), - v, - "UseTimelock", - ) + expectRefused(await v.set_notice({ secs: 0n }), v, "UseTimelock") expect((await reader.notice()).result).toBe(noticeSecs) }) }) diff --git a/scripts/harness/happy-path.test.ts b/scripts/harness/happy-path.test.ts index 164cffc7..00691e7e 100644 --- a/scripts/harness/happy-path.test.ts +++ b/scripts/harness/happy-path.test.ts @@ -71,7 +71,6 @@ describe("setup", () => { verifier.allow({ account: holder.publicKey(), allowed: true, - caller: compliance.publicKey(), }), ) expect( diff --git a/scripts/harness/seed.ts b/scripts/harness/seed.ts index c8285936..e524afec 100644 --- a/scripts/harness/seed.ts +++ b/scripts/harness/seed.ts @@ -105,7 +105,6 @@ async function ensureAllowlist( verifier.allow({ account: holder.publicKey(), allowed: true, - caller: c.keys.compliance.publicKey(), }), ) console.log(` ✓ ${name} allowlisted`) diff --git a/scripts/harness/wind-down.test.ts b/scripts/harness/wind-down.test.ts index 7dd0eccb..0b354cd1 100644 --- a/scripts/harness/wind-down.test.ts +++ b/scripts/harness/wind-down.test.ts @@ -71,7 +71,6 @@ beforeAll(async () => { identityVerifier(c, c.keys.compliance).allow({ account: holder.publicKey(), allowed: true, - caller: c.keys.compliance.publicKey(), }), ) }) @@ -122,10 +121,9 @@ describe("a wind-down on a throwaway vault", () => { }) it("governance proposes the wind-down and anyone activates it", async () => { - const gov = c.keys.governance.publicKey() - expect( - await call("propose_wind_down", "governance", { caller: gov }, true), - ).toBe("accepted") + expect(await call("propose_wind_down", "governance", {}, true)).toBe( + "accepted", + ) expect(await call("activate_wind_down", "outsider", {}, true)).toBe( "accepted", ) @@ -219,45 +217,30 @@ describe("a wind-down on a throwaway vault", () => { // Simulated, never sent: each write still has to encode, reach the // contract and get the answer an active wind-down gives it. const writes: Array<[string, Signer, Record, string]> = [ - ["set_deposit_cap", "governance", { cap: 1n, caller: gov }, "accepted"], + ["set_deposit_cap", "governance", { cap: 1n }, "accepted"], [ "set_custodian", "governance", - { custodian: someone, caller: gov }, + { custodian: someone }, "refused: UseTimelock", ], - [ - "set_wind_down_delay", - "governance", - { secs: 1n, caller: gov }, - "accepted", - ], + ["set_wind_down_delay", "governance", { secs: 1n }, "accepted"], [ "cancel_wind_down_proposal", "governance", - { caller: gov }, + {}, "refused: WindDownActive", ], - ["set_notice", "governance", { secs: 1n, caller: gov }, "accepted"], - ["finish_setup", "governance", { caller: gov }, "refused: SetupClosed"], + ["set_notice", "governance", { secs: 1n }, "accepted"], + ["finish_setup", "governance", {}, "refused: SetupClosed"], [ "propose_change", "governance", - { change: { tag: "Custodian", values: [someone] }, caller: gov }, + { change: { tag: "Custodian", values: [someone] } }, "refused: WindDownActive", ], - [ - "cancel_change", - "governance", - { id: 1, caller: gov }, - "refused: ChangeNotFound", - ], - [ - "apply_change", - "governance", - { id: 1, caller: gov }, - "refused: ChangeNotFound", - ], + ["cancel_change", "governance", { id: 1 }, "refused: ChangeNotFound"], + ["apply_change", "governance", { id: 1 }, "refused: ChangeNotFound"], [ "deploy_to_custodian", "treasury",