Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
66 changes: 60 additions & 6 deletions contracts/yield_vault/src/admin.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{DataKey, VaultError, YieldVault, YieldVaultArgs, YieldVaultClient};
use soroban_sdk::{symbol_short, xdr::ToXdr, Address, Bytes, Env};
use crate::{events, DataKey, VaultError, YieldVault};
use soroban_sdk::xdr::ToXdr;
use soroban_sdk::{symbol_short, Address, Env, IntoVal, Val, Vec};

#[soroban_sdk::contractimpl]
impl YieldVault {
Expand Down Expand Up @@ -139,18 +140,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 @@ -213,4 +224,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