From 671d59d35fff3ae8b9b1125471fb931fe5b6b571 Mon Sep 17 00:00:00 2001 From: harystyleseze Date: Sun, 26 Jul 2026 07:26:54 -0700 Subject: [PATCH] feat: governance-gated storage migrations for yield_vault and optimistic_governance Add a versioned storage schema (StorageVersion) and an admin-gated migrate_storage entrypoint to both contracts. Migrations are strictly sequential (to_version must equal current + 1), rejecting repeated and out-of-order/skipped calls with InvalidMigrationVersion before any state mutation. Each successful migration emits a versioned migrate event with (old_version, new_version, executor) for indexer audit trails. Also fixes pre-existing compile errors in yield_vault (admin.rs verify_admin_operation, oracle.rs TWAP confidence, events.rs emit_versioned_event) surfaced by the locked soroban-sdk 22.0.11 that were blocking cargo test/build for the whole crate; these functions were unrelated to and untouched in their logic by this change. Closes #896 --- contracts/ERROR_CODES.md | 5 ++ contracts/optimistic_governance/src/lib.rs | 50 ++++++++++- .../optimistic_governance/src/storage.rs | 6 ++ contracts/optimistic_governance/src/test.rs | 87 ++++++++++++++++++ contracts/yield_vault/STORAGE_HARDENING.md | 2 +- contracts/yield_vault/src/admin.rs | 71 ++++++++++++--- contracts/yield_vault/src/events.rs | 8 +- contracts/yield_vault/src/lib.rs | 90 +++++++++++++++++++ contracts/yield_vault/src/oracle.rs | 9 +- 9 files changed, 309 insertions(+), 19 deletions(-) diff --git a/contracts/ERROR_CODES.md b/contracts/ERROR_CODES.md index 8f2894bb2..5c330aaf1 100644 --- a/contracts/ERROR_CODES.md +++ b/contracts/ERROR_CODES.md @@ -40,6 +40,10 @@ Source: `contracts/yield_vault/src/lib.rs` | 11 | `StorageKeyNotFound` | Required storage key is missing | Ensure contract is properly initialized and configured | | 2001 | `InvalidDonationBps` | Donation basis points outside 0–10 000 | Pass a value between 0 and 10 000 | | 2002 | `CharityNotWhitelisted` | Charity address not on protocol whitelist | Use `set_charity_whitelist` to add the address | +| 2003 | `OperationExpired` | Admin operation intent has expired | Re-submit with a fresh nonce and expiry | +| 2004 | `OperationReplayed` | Admin operation was already executed | No action needed; operation already applied | +| 2005 | `UnauthorizedContract` | Caller is not the allowlisted contract | Use the registered contract for that role | +| 2006 | `InvalidMigrationVersion` | `migrate_storage` target isn't current version + 1 | Pass `get_storage_version() + 1` as `to_version` | --- @@ -112,6 +116,7 @@ Source: `contracts/optimistic_governance/src/lib.rs` | 7 | `ProposalAlreadyExecuted` | Proposal was already executed | No action needed | | 8 | `InsufficientVotingPower` | Caller does not have enough voting power | Acquire more governance tokens | | 9 | `ChallengeWindowExpired` | Challenge window has passed | Cannot challenge after expiry | +| 10 | `InvalidMigrationVersion` | `migrate_storage` target isn't current version + 1 | Pass `get_storage_version() + 1` as `to_version` | --- diff --git a/contracts/optimistic_governance/src/lib.rs b/contracts/optimistic_governance/src/lib.rs index 05eb08881..5a07dae75 100644 --- a/contracts/optimistic_governance/src/lib.rs +++ b/contracts/optimistic_governance/src/lib.rs @@ -9,7 +9,7 @@ mod storage; #[cfg(test)] mod test; -use storage::{DataKey, Proposal, ProposalStatus}; +use storage::{DataKey, Proposal, ProposalStatus, INITIAL_STORAGE_VERSION}; #[contracterror] #[derive(Copy, Clone, Debug, Eq, PartialEq)] @@ -24,6 +24,9 @@ pub enum Error { ProposalAlreadyExecuted = 7, InsufficientVotingPower = 8, ChallengeWindowExpired = 9, + /// Migration target version is not exactly current + 1 — rejects repeated, + /// skipped, or out-of-order migrations before any state mutation. + InvalidMigrationVersion = 10, } // Interface for ve_tokenomics (veYIELD) @@ -62,6 +65,9 @@ impl OptimisticGovernance { .set(&DataKey::ChallengeWindow, &challenge_window); env.storage().instance().set(&DataKey::ProposalCount, &0u64); env.storage().instance().set(&DataKey::IsInitialized, &true); + env.storage() + .instance() + .set(&DataKey::StorageVersion, &INITIAL_STORAGE_VERSION); Ok(()) } @@ -225,6 +231,48 @@ impl OptimisticGovernance { .unwrap_or(0) } + // ── Storage Migrations (governance-gated, #896) ───────────────── + + /// Returns the current storage schema version. Instances initialized + /// before this feature existed default to [`INITIAL_STORAGE_VERSION`]. + pub fn get_storage_version(env: Env) -> u32 { + env.storage() + .instance() + .get(&DataKey::StorageVersion) + .unwrap_or(INITIAL_STORAGE_VERSION) + } + + /// Advance the storage schema version by exactly one step. Admin-gated + /// (governance authority). Strictly sequential — rejects repeated, + /// skipped, or out-of-order migrations before mutating any state, and + /// emits a `migrate` event carrying the old version, new version, and + /// executor for indexer audit trails. + pub fn migrate_storage(env: Env, admin: Address, to_version: u32) -> Result<(), Error> { + Self::require_init(&env)?; + admin.require_auth(); + + let stored_admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); + if admin != stored_admin { + return Err(Error::Unauthorized); + } + + let current_version = Self::get_storage_version(env.clone()); + if to_version != current_version + 1 { + return Err(Error::InvalidMigrationVersion); + } + + env.storage() + .instance() + .set(&DataKey::StorageVersion, &to_version); + + env.events().publish( + (symbol_short!("migrate"), admin.clone()), + (current_version, to_version, admin), + ); + + Ok(()) + } + // ── Internal Helpers ────────────────────────────────────────── fn require_init(env: &Env) -> Result<(), Error> { diff --git a/contracts/optimistic_governance/src/storage.rs b/contracts/optimistic_governance/src/storage.rs index d0940104f..893fc4ad0 100644 --- a/contracts/optimistic_governance/src/storage.rs +++ b/contracts/optimistic_governance/src/storage.rs @@ -9,8 +9,14 @@ pub enum DataKey { Proposal(u64), ProposalCount, IsInitialized, + // Governance-gated storage schema version (#896) + StorageVersion, } +/// Storage schema version assigned on initialization, and the implicit +/// version of any instance deployed before migrations existed. +pub const INITIAL_STORAGE_VERSION: u32 = 1; + #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub enum ProposalStatus { diff --git a/contracts/optimistic_governance/src/test.rs b/contracts/optimistic_governance/src/test.rs index 8b568d1b8..b126619a1 100644 --- a/contracts/optimistic_governance/src/test.rs +++ b/contracts/optimistic_governance/src/test.rs @@ -340,3 +340,90 @@ fn test_expired_uncleared_proposal_still_executable() { let result: i128 = val.into_val(&env); assert_eq!(result, 8); // 7 + 1 } + +// ── Storage Migration Tests (#896) ───────────────────────────────────── + +#[test] +fn test_initial_storage_version() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let ve_yield = env.register(MockVeYield, ()); + let gov_id = env.register(OptimisticGovernance, ()); + let client = OptimisticGovernanceClient::new(&env, &gov_id); + + client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60)); + + assert_eq!(client.get_storage_version(), 1); +} + +#[test] +fn test_migrate_storage_advances_version() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let ve_yield = env.register(MockVeYield, ()); + let gov_id = env.register(OptimisticGovernance, ()); + let client = OptimisticGovernanceClient::new(&env, &gov_id); + + client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60)); + + client.migrate_storage(&admin, &2); + assert_eq!(client.get_storage_version(), 2); +} + +#[test] +fn test_migrate_storage_repeated_version_fails() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let ve_yield = env.register(MockVeYield, ()); + let gov_id = env.register(OptimisticGovernance, ()); + let client = OptimisticGovernanceClient::new(&env, &gov_id); + + client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60)); + + client.migrate_storage(&admin, &2); + // Repeating the same target version must be rejected, not silently re-applied. + let result = client.try_migrate_storage(&admin, &2); + assert!(result.is_err(), "repeated migration must be rejected"); +} + +#[test] +fn test_migrate_storage_out_of_order_fails_before_mutation() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let ve_yield = env.register(MockVeYield, ()); + let gov_id = env.register(OptimisticGovernance, ()); + let client = OptimisticGovernanceClient::new(&env, &gov_id); + + client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60)); + + // Skipping straight from version 1 to version 3 must fail... + let result = client.try_migrate_storage(&admin, &3); + assert!(result.is_err(), "out-of-order migration must be rejected"); + // ...and must not have mutated the stored version. + assert_eq!(client.get_storage_version(), 1); +} + +#[test] +#[should_panic(expected = "HostError: Error(Contract, #3)")] // Unauthorized +fn test_migrate_storage_by_non_admin_panics() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let impostor = Address::generate(&env); + let ve_yield = env.register(MockVeYield, ()); + let gov_id = env.register(OptimisticGovernance, ()); + let client = OptimisticGovernanceClient::new(&env, &gov_id); + + client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60)); + + client.migrate_storage(&impostor, &2); +} diff --git a/contracts/yield_vault/STORAGE_HARDENING.md b/contracts/yield_vault/STORAGE_HARDENING.md index 9aa35f3fe..c85ecbb1c 100644 --- a/contracts/yield_vault/STORAGE_HARDENING.md +++ b/contracts/yield_vault/STORAGE_HARDENING.md @@ -167,7 +167,7 @@ Build completes successfully with no errors (only one unused import warning). 1. Consider adding more specific error variants for different storage failure types 2. Add telemetry/logging for storage access failures 3. Implement storage recovery mechanisms for specific failure scenarios -4. Add contract upgrade paths that handle storage schema changes +4. ~~Add contract upgrade paths that handle storage schema changes~~ — done via `migrate_storage`/`get_storage_version` (governance-gated storage migrations, #896) ## Conclusion diff --git a/contracts/yield_vault/src/admin.rs b/contracts/yield_vault/src/admin.rs index 0b69333c3..876b824d1 100644 --- a/contracts/yield_vault/src/admin.rs +++ b/contracts/yield_vault/src/admin.rs @@ -1,5 +1,6 @@ use crate::{events, DataKey, VaultError, YieldVault}; -use soroban_sdk::{symbol_short, Address, Bytes, Env, Vec}; +use soroban_sdk::xdr::ToXdr; +use soroban_sdk::{symbol_short, Address, Env, IntoVal, Val, Vec}; impl YieldVault { /// Immediately pause all vault operations (deposit, withdraw, rebalance). @@ -164,27 +165,28 @@ impl YieldVault { let network = env.ledger().network_id(); let contract = env.current_contract_address(); - let mut op_hash_input = Vec::new(env); - op_hash_input.push_back(network); - op_hash_input.push_back(contract.into()); - op_hash_input.push_back(operation_type.into()); - op_hash_input.push_back(nonce.into()); - op_hash_input.push_back(expiry_timestamp.into()); + let mut op_hash_input: Vec = Vec::new(env); + op_hash_input.push_back(network.into_val(env)); + op_hash_input.push_back(contract.into_val(env)); + op_hash_input.push_back(operation_type.into_val(env)); + op_hash_input.push_back(nonce.into_val(env)); + op_hash_input.push_back(expiry_timestamp.into_val(env)); - let op_hash = env.crypto().sha256(&Bytes::from_slice( - env, - &op_hash_input.try_into_val(env).unwrap_or_default().to_xdr(env).as_slice(), - )); + let op_hash = env.crypto().sha256(&op_hash_input.to_xdr(env)); // Check if already executed - if env.storage().instance().has(&DataKey::ExecutedAdminOp(op_hash.clone())) { + if env + .storage() + .instance() + .has(&DataKey::ExecutedAdminOp(op_hash.clone().into())) + { return Err(VaultError::OperationReplayed); } // Mark as executed env.storage() .instance() - .set(&DataKey::ExecutedAdminOp(op_hash), &true); + .set(&DataKey::ExecutedAdminOp(op_hash.into()), &true); Ok(()) } @@ -241,4 +243,47 @@ impl YieldVault { _ => Err(VaultError::UnauthorizedContract), } } + + // ── Governance-Gated Storage Migrations (#896) ─────────────────── + + /// Returns the current storage schema version, defaulting to + /// [`crate::INITIAL_STORAGE_VERSION`] for vaults initialized before + /// this feature existed. + pub fn get_storage_version_impl(env: &Env) -> u32 { + env.storage() + .instance() + .get(&DataKey::StorageVersion) + .unwrap_or(crate::INITIAL_STORAGE_VERSION) + } + + /// Migrate storage to `to_version`. Admin-gated (governance authority). + /// Strictly sequential — rejects repeated, skipped, or out-of-order + /// migrations before mutating any state, and emits a versioned + /// `migrate` event carrying the old version, new version, and executor + /// for indexer audit trails. + pub fn migrate_storage_impl( + env: Env, + admin: Address, + to_version: u32, + ) -> Result<(), VaultError> { + Self::require_admin(&env, &admin)?; + + let current_version = Self::get_storage_version_impl(&env); + if to_version != current_version + 1 { + return Err(VaultError::InvalidMigrationVersion); + } + + env.storage() + .instance() + .set(&DataKey::StorageVersion, &to_version); + + events::emit_versioned_event( + &env, + symbol_short!("migrate"), + events::MIGRATION_EVENT_V1.schema_version, + (current_version, to_version, admin), + ); + + Ok(()) + } } diff --git a/contracts/yield_vault/src/events.rs b/contracts/yield_vault/src/events.rs index 964275287..ead5cfcf2 100644 --- a/contracts/yield_vault/src/events.rs +++ b/contracts/yield_vault/src/events.rs @@ -36,6 +36,11 @@ pub const HARVEST_EVENT_V1: EventVersion = EventVersion { schema_version: 1, }; +pub const MIGRATION_EVENT_V1: EventVersion = EventVersion { + event_type: "migration", + schema_version: 1, +}; + /// Emit a versioned event with schema version information. /// /// This helper ensures all events include version metadata for future @@ -47,7 +52,7 @@ pub const HARVEST_EVENT_V1: EventVersion = EventVersion { /// * `topic` - The event topic (e.g., "deposit") /// * `version` - The schema version for this event /// * `data` - The event data payload -pub fn emit_versioned_event>( +pub fn emit_versioned_event>( env: &Env, topic: Symbol, version: u32, @@ -86,6 +91,7 @@ pub fn check_event_version( ("withdrawal", 1) => EventDecodeStatus::Recognized, ("admin_action", 1) => EventDecodeStatus::Recognized, ("harvest", 1) => EventDecodeStatus::Recognized, + ("migration", 1) => EventDecodeStatus::Recognized, // Future versions go to dead-letter (_, future) if future > VAULT_EVENT_SCHEMA_VERSION => EventDecodeStatus::Unknown, _ => EventDecodeStatus::Invalid, diff --git a/contracts/yield_vault/src/lib.rs b/contracts/yield_vault/src/lib.rs index a12b5f695..02302034e 100644 --- a/contracts/yield_vault/src/lib.rs +++ b/contracts/yield_vault/src/lib.rs @@ -41,8 +41,14 @@ enum DataKey { ExecutedAdminOp(Bytes), // Hash of (network, contract, operation, nonce) // Cross-contract allowlist with network binding (#905) AllowedContractRole(Symbol), // Role -> Address allowlist (e.g., "zap" -> ZapContract) + // Governance-gated storage schema version (#896) + StorageVersion, } +/// Storage schema version assigned to newly initialized vaults, and the +/// implicit version of any vault deployed before migrations existed. +pub const INITIAL_STORAGE_VERSION: u32 = 1; + mod admin; mod donations; mod emergency; @@ -83,6 +89,9 @@ pub enum VaultError { OperationReplayed = 2004, /// Cross-contract call from unauthorized or wrong-network contract (maps to error code 2005). UnauthorizedContract = 2005, + /// Migration target version is not exactly current + 1 — rejects repeated, + /// skipped, or out-of-order migrations before any state mutation (maps to error code 2006). + InvalidMigrationVersion = 2006, } // ── Contract ──────────────────────────────────────────────────────────── @@ -113,6 +122,9 @@ impl YieldVault { env.storage().instance().set(&DataKey::TotalShares, &0i128); env.storage().instance().set(&DataKey::TotalAssets, &0i128); env.storage().instance().set(&DataKey::Initialized, &true); + env.storage() + .instance() + .set(&DataKey::StorageVersion, &INITIAL_STORAGE_VERSION); env.events() .publish((symbol_short!("init"),), (admin.clone(), token.clone())); @@ -512,6 +524,30 @@ impl YieldVault { Self::get_storage_required(&env, &DataKey::Token) } + // ── Storage Migrations (governance-gated, #896) ────────────────── + + /// Returns the vault's current storage schema version. Vaults + /// initialized before this feature existed default to + /// [`INITIAL_STORAGE_VERSION`]. + pub fn get_storage_version(env: Env) -> u32 { + YieldVault::get_storage_version_impl(&env) + } + + /// Advance the vault's storage schema version by exactly one step. + /// + /// Migrations are strictly sequential: `to_version` must equal the + /// currently stored version + 1. Repeated calls (`to_version` at or + /// below the current version) and out-of-order or skipped calls + /// (`to_version` more than one ahead) are rejected with + /// [`VaultError::InvalidMigrationVersion`] before any state is mutated. + /// + /// # Arguments + /// * `admin` — Must be the vault admin (governance authority). + /// * `to_version` — The storage version to migrate to. + pub fn migrate_storage(env: Env, admin: Address, to_version: u32) -> Result<(), VaultError> { + YieldVault::migrate_storage_impl(env, admin, to_version) + } + // ── Strategy: Harvest & Auto-Compound ─────────────────────────── /// Configure the strategy parameters. Admin-only. @@ -1156,6 +1192,60 @@ mod tests { assert_eq!(client.get_shares(&unknown), 0); } + // ── Storage Migration Tests (#896) ────────────────────────────── + + #[test] + fn test_initial_storage_version() { + let (_, client, _, _, _) = setup_env(); + assert_eq!(client.get_storage_version(), INITIAL_STORAGE_VERSION); + } + + #[test] + fn test_migrate_storage_advances_version() { + let (_, client, admin, _, _) = setup_env(); + + client.migrate_storage(&admin, &2); + assert_eq!(client.get_storage_version(), 2); + } + + #[test] + fn test_migrate_storage_sequential_steps() { + let (_, client, admin, _, _) = setup_env(); + + client.migrate_storage(&admin, &2); + client.migrate_storage(&admin, &3); + assert_eq!(client.get_storage_version(), 3); + } + + #[test] + #[should_panic(expected = "Error(Contract, #2006)")] + fn test_migrate_storage_repeated_version_panics() { + let (_, client, admin, _, _) = setup_env(); + + client.migrate_storage(&admin, &2); + // Repeating the same target version must be rejected, not silently re-applied. + client.migrate_storage(&admin, &2); + } + + #[test] + fn test_migrate_storage_out_of_order_fails_before_mutation() { + let (_, client, admin, _, _) = setup_env(); + + // Skipping straight from version 1 to version 3 must fail... + let result = client.try_migrate_storage(&admin, &3); + assert!(result.is_err()); + // ...and must not have mutated the stored version. + assert_eq!(client.get_storage_version(), INITIAL_STORAGE_VERSION); + } + + #[test] + #[should_panic(expected = "Error(Contract, #5)")] + fn test_migrate_storage_by_non_admin_panics() { + let (env, client, _, _, _) = setup_env(); + let impostor = Address::generate(&env); + client.migrate_storage(&impostor, &2); + } + // ── Referral Tests ─────────────────────────────────────────────── #[test] diff --git a/contracts/yield_vault/src/oracle.rs b/contracts/yield_vault/src/oracle.rs index 62e0f5f16..ea85ef37d 100644 --- a/contracts/yield_vault/src/oracle.rs +++ b/contracts/yield_vault/src/oracle.rs @@ -127,7 +127,7 @@ impl YieldVault { } // Base confidence from sample count (min 3 samples for decent confidence) - let sample_confidence = if sample_count >= 10 { + let sample_confidence: u32 = if sample_count >= 10 { 80 } else if sample_count >= 5 { 60 @@ -138,7 +138,10 @@ impl YieldVault { }; // Calculate price volatility penalty - let prices: Vec = history.iter().map(|p| p.price).collect(); + let mut prices: Vec = Vec::new(env); + for p in history.iter() { + prices.push_back(p.price); + } let volatility_penalty = Self::calculate_volatility_penalty(&prices); sample_confidence.saturating_sub(volatility_penalty) @@ -154,7 +157,7 @@ impl YieldVault { let avg = sum / prices.len() as i128; let mut max_deviation = 0i128; - for &price in prices.iter() { + for price in prices.iter() { let deviation = if price > avg { price - avg } else {