From 773b8a1136d4fa0a59245f5b02b667a2e6b7d6d1 Mon Sep 17 00:00:00 2001 From: D'Angelo Rodriguez <70290504+dangelo352@users.noreply.github.com> Date: Sun, 21 Jun 2026 22:03:43 -0400 Subject: [PATCH] feat: add core upgrade migration path --- contracts/commitment_core/src/lib.rs | 94 ++++++++++++++++++++- contracts/commitment_core/src/tests.rs | 110 ++++++++++++++++++++++++- docs/UPGRADES.md | 4 +- docs/UPGRADE_PATHS.md | 6 ++ 4 files changed, 209 insertions(+), 5 deletions(-) diff --git a/contracts/commitment_core/src/lib.rs b/contracts/commitment_core/src/lib.rs index 0269de6b..33f6e411 100644 --- a/contracts/commitment_core/src/lib.rs +++ b/contracts/commitment_core/src/lib.rs @@ -23,12 +23,15 @@ use shared_utils::{ Validation, }; use soroban_sdk::{ - contract, contracterror, contractimpl, contracttype, log, symbol_short, token, Address, Env, - IntoVal, String, Symbol, Vec, + contract, contracterror, contractimpl, contracttype, log, symbol_short, token, Address, BytesN, + Env, IntoVal, String, Symbol, Vec, }; pub mod fuzzing; +/// Current storage schema version for commitment_core migrations. +pub const CURRENT_VERSION: u32 = 1; + /// Maximum page size for paginated owner-commitment queries. const MAX_PAGE_SIZE: u32 = 50; @@ -67,6 +70,12 @@ pub enum CommitmentError { ArithmeticOverflow = 24, /// Generated commitment ID already exists (counter/storage corruption guard) DuplicateCommitmentId = 25, + /// Invalid WASM hash for upgrade + InvalidWasmHash = 26, + /// Invalid storage version supplied for migration + InvalidVersion = 27, + /// Migration already applied + AlreadyMigrated = 28, } impl CommitmentError { @@ -101,6 +110,9 @@ impl CommitmentError { CommitmentError::DuplicateCommitmentId => { "Commitment ID already exists; counter or storage may be corrupted" } + CommitmentError::InvalidWasmHash => "Invalid WASM hash for upgrade", + CommitmentError::InvalidVersion => "Invalid storage version supplied for migration", + CommitmentError::AlreadyMigrated => "Migration already applied", } } } @@ -189,6 +201,8 @@ pub enum DataKey { CreationFeeBps, /// Collected fees per asset (asset -> i128) CollectedFees(Address), + /// Stored commitment_core schema version + Version, } // --- Internal Helpers --- @@ -285,6 +299,21 @@ fn set_reentrancy_guard(e: &Env, value: bool) { .set(&DataKey::ReentrancyGuard, &value); } +fn read_version(e: &Env) -> u32 { + e.storage() + .instance() + .get::<_, u32>(&DataKey::Version) + .unwrap_or(0) +} + +fn require_valid_wasm_hash(e: &Env, wasm_hash: &BytesN<32>) -> Result<(), CommitmentError> { + let zero = BytesN::from_array(e, &[0; 32]); + if *wasm_hash == zero { + return Err(CommitmentError::InvalidWasmHash); + } + Ok(()) +} + fn require_admin(e: &Env, caller: &Address) { caller.require_auth(); let admin = e @@ -464,6 +493,9 @@ impl CommitmentCoreContract { e.storage() .instance() .set(&DataKey::ReentrancyGuard, &false); + e.storage() + .instance() + .set(&DataKey::Version, &CURRENT_VERSION); e.storage().instance().set(&Pausable::PAUSED_KEY, &false); EmergencyControl::set_emergency_mode(&e, false); } @@ -736,6 +768,64 @@ impl CommitmentCoreContract { .unwrap_or_else(|| fail(&e, CommitmentError::NotInitialized, "get_admin")) } + /// Get current on-chain storage version (0 if legacy/uninitialized). + pub fn get_version(e: Env) -> u32 { + read_version(&e) + } + + /// Upgrade contract WASM (admin-only). + pub fn upgrade( + e: Env, + caller: Address, + new_wasm_hash: BytesN<32>, + ) -> Result<(), CommitmentError> { + require_admin(&e, &caller); + require_valid_wasm_hash(&e, &new_wasm_hash)?; + e.deployer().update_current_contract_wasm(new_wasm_hash); + Ok(()) + } + + /// Migrate storage from a previous version to CURRENT_VERSION (admin-only). + pub fn migrate(e: Env, caller: Address, from_version: u32) -> Result<(), CommitmentError> { + require_admin(&e, &caller); + + let stored_version = read_version(&e); + if stored_version == CURRENT_VERSION { + return Err(CommitmentError::AlreadyMigrated); + } + if from_version != stored_version || from_version > CURRENT_VERSION { + return Err(CommitmentError::InvalidVersion); + } + + if !e.storage().instance().has(&DataKey::TotalCommitments) { + e.storage() + .instance() + .set(&DataKey::TotalCommitments, &0u64); + } + if !e.storage().instance().has(&DataKey::TotalValueLocked) { + e.storage() + .instance() + .set(&DataKey::TotalValueLocked, &0i128); + } + if !e.storage().instance().has(&DataKey::AllCommitmentIds) { + e.storage() + .instance() + .set(&DataKey::AllCommitmentIds, &Vec::::new(&e)); + } + if !e.storage().instance().has(&DataKey::ReentrancyGuard) { + e.storage() + .instance() + .set(&DataKey::ReentrancyGuard, &false); + } + + e.storage() + .instance() + .set(&DataKey::Version, &CURRENT_VERSION); + e.events() + .publish((symbol_short!("Migrated"),), (from_version, CURRENT_VERSION)); + Ok(()) + } + /// Get NFT contract address pub fn get_nft_contract(e: Env) -> Address { e.storage() diff --git a/contracts/commitment_core/src/tests.rs b/contracts/commitment_core/src/tests.rs index 54e849bc..a20a2493 100644 --- a/contracts/commitment_core/src/tests.rs +++ b/contracts/commitment_core/src/tests.rs @@ -6,7 +6,7 @@ use soroban_sdk::{ contract, contractimpl, symbol_short, testutils::{Address as _, Events, Ledger}, token::{Client as TokenClient, StellarAssetClient}, - vec, Address, Env, IntoVal, String, + vec, Address, Bytes, BytesN, Env, IntoVal, String, }; #[contract] @@ -443,10 +443,16 @@ fn setup_token_contract(e: &Env) -> Address { Address::generate(e) } +fn upload_wasm(e: &Env) -> BytesN<32> { + let wasm = Bytes::new(e); + e.deployer().upload_contract_wasm(wasm) +} + #[test] fn test_initialize() { let e = Env::default(); let contract_id = e.register_contract(None, CommitmentCoreContract); + let client = CommitmentCoreContractClient::new(&e, &contract_id); let admin = Address::generate(&e); let nft_contract = Address::generate(&e); @@ -455,6 +461,108 @@ fn test_initialize() { e.as_contract(&contract_id, || { CommitmentCoreContract::initialize(e.clone(), admin.clone(), nft_contract.clone()); }); + + assert_eq!(client.get_version(), CURRENT_VERSION); +} + +#[test] +fn test_migrate_rejects_wrong_from_version_without_mutating_state() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, CommitmentCoreContract); + let client = CommitmentCoreContractClient::new(&e, &contract_id); + + let admin = Address::generate(&e); + let nft_contract = Address::generate(&e); + client.initialize(&admin, &nft_contract); + e.as_contract(&contract_id, || { + e.storage().instance().remove(&DataKey::Version); + }); + + assert_eq!( + client.try_migrate(&admin, &1), + Err(Ok(CommitmentError::InvalidVersion)) + ); + assert_eq!(client.get_version(), 0); +} + +#[test] +fn test_migrate_backfills_legacy_keys_and_is_idempotent() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, CommitmentCoreContract); + let client = CommitmentCoreContractClient::new(&e, &contract_id); + + let admin = Address::generate(&e); + let nft_contract = Address::generate(&e); + client.initialize(&admin, &nft_contract); + e.as_contract(&contract_id, || { + e.storage().instance().remove(&DataKey::Version); + e.storage().instance().remove(&DataKey::ReentrancyGuard); + e.storage() + .instance() + .set(&DataKey::TotalCommitments, &7u64); + }); + + assert_eq!(client.try_migrate(&admin, &0), Ok(Ok(()))); + assert_eq!(client.get_version(), CURRENT_VERSION); + assert_eq!(client.get_total_commitments(), 7); + + assert_eq!( + client.try_migrate(&admin, &CURRENT_VERSION), + Err(Ok(CommitmentError::AlreadyMigrated)) + ); +} + +#[test] +fn test_upgrade_invalid_hash_rejected() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, CommitmentCoreContract); + let client = CommitmentCoreContractClient::new(&e, &contract_id); + + let admin = Address::generate(&e); + let nft_contract = Address::generate(&e); + client.initialize(&admin, &nft_contract); + + let zero = BytesN::from_array(&e, &[0; 32]); + assert_eq!( + client.try_upgrade(&admin, &zero), + Err(Ok(CommitmentError::InvalidWasmHash)) + ); +} + +#[test] +#[should_panic(expected = "Unauthorized")] +fn test_upgrade_non_admin_rejected() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, CommitmentCoreContract); + let client = CommitmentCoreContractClient::new(&e, &contract_id); + + let admin = Address::generate(&e); + let attacker = Address::generate(&e); + let nft_contract = Address::generate(&e); + client.initialize(&admin, &nft_contract); + + let wasm_hash = upload_wasm(&e); + client.upgrade(&attacker, &wasm_hash); +} + +#[test] +fn test_upgrade_accepts_uploaded_wasm_hash() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, CommitmentCoreContract); + let client = CommitmentCoreContractClient::new(&e, &contract_id); + + let admin = Address::generate(&e); + let nft_contract = Address::generate(&e); + client.initialize(&admin, &nft_contract); + + let wasm_hash = upload_wasm(&e); + assert_eq!(client.try_upgrade(&admin, &wasm_hash), Ok(Ok(()))); + assert_eq!(client.get_version(), CURRENT_VERSION); } #[test] diff --git a/docs/UPGRADES.md b/docs/UPGRADES.md index f4455de3..f6e7a67b 100644 --- a/docs/UPGRADES.md +++ b/docs/UPGRADES.md @@ -19,7 +19,7 @@ This repository uses Soroban's native in-place upgrade mechanism. Each contract: ## Version History (Current) -- `commitment_core`: `CURRENT_VERSION = 1` - version tracking + upgrade entrypoints (no storage layout changes). +- `commitment_core`: `CURRENT_VERSION = 1` - version tracking, admin-only upgrade/migrate entrypoints, and legacy key backfills. - `commitment_nft`: `CURRENT_VERSION = 2` - caller-aware lifecycle ABI for `settle` / `mark_inactive`; no storage layout changes. - `attestation_engine`: `CURRENT_VERSION = 1` - version tracking + upgrade entrypoints (no storage layout changes). - `allocation_logic`: `CURRENT_VERSION = 1` - version tracking + upgrade entrypoints (no storage layout changes). @@ -27,7 +27,7 @@ This repository uses Soroban's native in-place upgrade mechanism. Each contract: ## Migration Requirements -- `commitment_core`: ensures counters/guards exist; preserves commitments and owner lists. +- `commitment_core`: ensures total counters, the all-commitment index, and the reentrancy guard exist; preserves commitments, owner lists, fee state, and settlement/early-exit semantics. - `commitment_nft`: ensures token counters and registries exist, then versions the core-only lifecycle ABI; preserves NFTs and ownership data. - `attestation_engine`: ensures analytics counters exist; preserves attestations and metrics. - `allocation_logic`: ensures pool registry exists; preserves pools and allocations. diff --git a/docs/UPGRADE_PATHS.md b/docs/UPGRADE_PATHS.md index 5d372849..2dbb1fad 100644 --- a/docs/UPGRADE_PATHS.md +++ b/docs/UPGRADE_PATHS.md @@ -13,6 +13,12 @@ For full details, see `docs/UPGRADES.md`. +## commitment_core +- `upgrade(caller, new_wasm_hash)` requires admin authorization and rejects the all-zero WASM hash. +- `migrate(caller, from_version)` accepts legacy version `0` only when the stored `Version` is also `0`, then writes `CURRENT_VERSION`. +- Migration backfills `TotalCommitments`, `TotalValueLocked`, `AllCommitmentIds`, and `ReentrancyGuard` only when those keys are missing. +- Migration preserves commitments, owner lists, fee state, asset custody, and create/settle/early-exit behavior. + ## Migration idempotency guarantees - `migrate(caller, from_version)` is admin-only and rejects non-admin callers before writing state. - `from_version` must match the stored `Version`; mismatches return `InvalidVersion` and leave `Version` unchanged.