Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions contracts/ERROR_CODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |

---

Expand Down Expand Up @@ -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` |

---

Expand Down
50 changes: 49 additions & 1 deletion contracts/optimistic_governance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -248,6 +254,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 status_code(status: &ProposalStatus) -> u32 {
Expand Down
6 changes: 6 additions & 0 deletions contracts/optimistic_governance/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
122 changes: 36 additions & 86 deletions contracts/optimistic_governance/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,139 +341,89 @@ fn test_expired_uncleared_proposal_still_executable() {
assert_eq!(result, 8); // 7 + 1
}

// ── Challenge-window boundary coverage (#960) ─────────────────────────
//
// Boundary semantics (document for indexers / callers):
// - Window START = propose timestamp (inclusive): dispute OK, execute FAIL
// - Window END = execution_time (inclusive for execute): dispute FAIL, execute OK
// - Before start is not reachable after propose (window opens immediately)
// - After expiry = timestamp > execution_time: dispute FAIL, execute OK if Pending
// ── Storage Migration Tests (#896) ─────────────────────────────────────

#[test]
fn test_boundary_at_exact_window_start() {
// At propose time (window start): dispute allowed, execute rejected
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 target = env.register(TargetContract, ());
let gov_id = env.register(OptimisticGovernance, ());
let client = OptimisticGovernanceClient::new(&env, &gov_id);

let challenge_window: u64 = 3 * 24 * 60 * 60;
client.initialize(&admin, &ve_yield, &challenge_window);

let args: Vec<Val> = vec![&env, 0i128.into_val(&env)];
let proposal_id = client.propose(&admin, &target, &Symbol::new(&env, "action"), &args);

// Still at start of window (timestamp unchanged after propose)
let proposal = client.get_proposal(&proposal_id).unwrap();
assert_eq!(proposal.status, ProposalStatus::Pending);
assert!(client.try_execute(&proposal_id).is_err());
client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60));

let disputer = Address::generate(&env);
client.dispute(&disputer, &proposal_id);
assert_eq!(
client.get_proposal(&proposal_id).unwrap().status,
ProposalStatus::Disputed
);
assert_eq!(client.get_storage_version(), 1);
}

#[test]
fn test_unchallenged_transitions_pending_to_executed_at_end() {
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 target = env.register(TargetContract, ());
let gov_id = env.register(OptimisticGovernance, ());
let client = OptimisticGovernanceClient::new(&env, &gov_id);

let challenge_window: u64 = 100;
client.initialize(&admin, &ve_yield, &challenge_window);

let args: Vec<Val> = vec![&env, 2i128.into_val(&env)];
let proposal_id = client.propose(&admin, &target, &Symbol::new(&env, "action"), &args);
assert_eq!(
client.get_proposal(&proposal_id).unwrap().status,
ProposalStatus::Pending
);
client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60));

// Exact end of window
env.ledger().with_mut(|li| li.timestamp = challenge_window);
let val = client.execute(&proposal_id);
let result: i128 = val.into_val(&env);
assert_eq!(result, 3);
assert_eq!(
client.get_proposal(&proposal_id).unwrap().status,
ProposalStatus::Executed
);
client.migrate_storage(&admin, &2);
assert_eq!(client.get_storage_version(), 2);
}

#[test]
fn test_challenged_stays_disputed_after_expiry() {
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 target = env.register(TargetContract, ());
let gov_id = env.register(OptimisticGovernance, ());
let client = OptimisticGovernanceClient::new(&env, &gov_id);

let challenge_window: u64 = 100;
client.initialize(&admin, &ve_yield, &challenge_window);

let args: Vec<Val> = vec![&env, 0i128.into_val(&env)];
let proposal_id = client.propose(&admin, &target, &Symbol::new(&env, "action"), &args);

let disputer = Address::generate(&env);
client.dispute(&disputer, &proposal_id);
client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60));

// After expiry — still Disputed, execute rejected
env.ledger()
.with_mut(|li| li.timestamp = challenge_window + 50);
assert!(client.try_execute(&proposal_id).is_err());
assert_eq!(
client.get_proposal(&proposal_id).unwrap().status,
ProposalStatus::Disputed
);
// Dispute also rejected after expiry
let other = Address::generate(&env);
assert!(client.try_dispute(&other, &proposal_id).is_err());
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_before_start_execute_fails_and_after_expiry_execute_ok() {
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 target = env.register(TargetContract, ());
let gov_id = env.register(OptimisticGovernance, ());
let client = OptimisticGovernanceClient::new(&env, &gov_id);

let challenge_window: u64 = 50;
client.initialize(&admin, &ve_yield, &challenge_window);
client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60));

let args: Vec<Val> = vec![&env, 9i128.into_val(&env)];
let proposal_id = client.propose(&admin, &target, &Symbol::new(&env, "action"), &args);
// 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);

// Before end (= still inside window / "before executable")
env.ledger().with_mut(|li| li.timestamp = challenge_window - 1);
assert!(client.try_execute(&proposal_id).is_err());
client.initialize(&admin, &ve_yield, &(3 * 24 * 60 * 60));

// After expiry
env.ledger()
.with_mut(|li| li.timestamp = challenge_window + 1);
let val = client.execute(&proposal_id);
let result: i128 = val.into_val(&env);
assert_eq!(result, 10);
assert_eq!(
client.get_proposal(&proposal_id).unwrap().status,
ProposalStatus::Executed
);
client.migrate_storage(&impostor, &2);
}
2 changes: 1 addition & 1 deletion contracts/yield_vault/STORAGE_HARDENING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
64 changes: 59 additions & 5 deletions contracts/yield_vault/src/admin.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{events, DataKey, VaultError, YieldVault};
use soroban_sdk::{symbol_short, xdr::ToXdr, Address, Bytes, Env};
use soroban_sdk::xdr::ToXdr;
use soroban_sdk::{symbol_short, Address, Env, IntoVal, Val, Vec};

impl YieldVault {
/// Note: `emergency_pause` and `emergency_unpause` have moved to emergency.rs.
Expand Down Expand Up @@ -196,18 +197,28 @@ impl YieldVault {
let network = env.ledger().network_id();
let contract = env.current_contract_address();

let preimage = (network, contract, operation_type, nonce, expiry_timestamp).to_xdr(env);
let op_hash: Bytes = env.crypto().sha256(&preimage).into();
let mut op_hash_input: Vec<Val> = 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(&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(())
}
Expand Down Expand Up @@ -264,4 +275,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(())
}
}
Loading
Loading