diff --git a/UPGRADE.md b/UPGRADE.md index a205e460..c7ce7cbf 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -120,7 +120,7 @@ pub fn upgrade(env: Env, caller: Address, new_wasm_hash: BytesN<32>) **Version Function Signature:** ```rust -pub fn version(env: Env) -> Option> +pub fn get_version(env: Env) -> Option> ``` **Behavior note (pause semantics):** @@ -135,6 +135,7 @@ pub fn version(env: Env) -> Option> |-----|------|-------------| | `admin` | `Address` | Admin address; may call `distribute` and `set_admin` | | `usdc` | `Address` | USDC token contract address | +| `ContractVersion` | `BytesN<32>` | WASM hash set by `upgrade` function | #### Upgradeability @@ -151,7 +152,7 @@ soroban contract invoke --contract-id -- upgrade \ --caller --new_wasm_hash <32-byte-hex> ``` -The `version()` view returns the stored WASM hash; the contract emits an `upgraded` +The `get_version()` view returns the stored WASM hash; the contract emits an `upgraded` event with the admin as a topic and the new version as data. **Init signature** (`lib.rs:28-39`): @@ -168,6 +169,25 @@ pub fn init(env: Env, admin: Address, usdc_token: Address) | `vault` | `Address` | Registered vault address | | `developer_balances` | `Map` | Per-developer balance tracking | | `global_pool` | `GlobalPool` | Total balance and last updated timestamp | +| `ContractVersion` | `BytesN<32>` | WASM hash set by `upgrade` function | + +#### Upgradeability + +The settlement contract supports in-place upgrades via an admin-gated `upgrade` function. +This method calls the host deployer to update the contract WASM code while +preserving existing instance storage. + +```bash +# Build new WASM +cargo build --target wasm32-unknown-unknown --release -p callora-settlement + +# Compute WASM hash and call upgrade via RPC or tooling +soroban contract invoke --contract-id -- upgrade \ + --caller --new_wasm_hash <32-byte-hex> +``` + +The `get_version()` view returns the stored WASM hash (as an `Option>`); the contract emits an `upgraded` +event with the admin as a topic and the new version as data. **GlobalPool structure** (`lib.rs:16-19`): @@ -236,7 +256,7 @@ The vault now supports admin-gated in-place upgrades that preserve all existing 4. **Verify upgrade** ```bash # Check version marker - soroban contract invoke --contract-id -- version + soroban contract invoke --contract-id -- get_version # Should return # Verify state preserved @@ -246,11 +266,15 @@ The vault now supports admin-gated in-place upgrades that preserve all existing 5. **Run post-upgrade migration** (if needed) ```bash - # If the new WASM includes a migrate function for schema changes + // If the new WASM includes a migrate function for schema changes soroban contract invoke --contract-id -- migrate \ --caller ``` +**Migration of `request_id` Deduplication (v1.2+):** +The vault now uses `persistent` storage for `ProcessedRequest` idempotency markers rather than `temporary` storage. +During the upgrade, no explicit state migration script is needed. The `require_not_duplicate` checks have been updated to query *both* temporary (legacy) and persistent (new) storage. Temporary markers will naturally expire over the next ~30 days. Going forward, the owner should periodically invoke `prune_processed_requests` to garbage-collect old persistent markers and recover storage deposits. + 6. **Monitor and verify** - Test a small transaction - Verify all view functions return expected values @@ -264,7 +288,7 @@ The `upgrade` function emits an `upgraded` event with: - Data: New WASM hash (BytesN<32>) **Version Tracking:** -- Call `version()` to retrieve the current WASM hash +- Call `get_version()` to retrieve the current WASM hash - Returns `None` for contracts deployed before upgrade functionality - Returns `Some(BytesN<32>)` after first upgrade diff --git a/contracts/revenue_pool/src/lib.rs b/contracts/revenue_pool/src/lib.rs index a833c858..26a7a394 100644 --- a/contracts/revenue_pool/src/lib.rs +++ b/contracts/revenue_pool/src/lib.rs @@ -582,12 +582,11 @@ impl RevenuePool { /// Read the stored contract version (WASM hash) as last set by `upgrade`. /// - /// Panics if no version has been stored yet. - pub fn version(env: Env) -> BytesN<32> { + /// Returns `None` if no version has been stored yet. + pub fn get_version(env: Env) -> Option> { env.storage() .instance() .get(&Symbol::new(&env, VERSION_KEY)) - .expect("version not set") } } diff --git a/contracts/revenue_pool/src/test.rs b/contracts/revenue_pool/src/test.rs index 005e3bf0..b329c684 100644 --- a/contracts/revenue_pool/src/test.rs +++ b/contracts/revenue_pool/src/test.rs @@ -1883,8 +1883,8 @@ fn upgrade_sets_version_and_emits_event() { client.upgrade(&admin, &new_hash); // version() should return stored value - let readback: BytesN<32> = client.version(); - assert_eq!(readback, new_hash); + let readback: Option> = client.get_version(); + assert_eq!(readback, Some(new_hash)); // An `upgraded` event should have been emitted let events = env.events().all(); diff --git a/contracts/settlement/src/lib.rs b/contracts/settlement/src/lib.rs index 9376db40..45f4b314 100644 --- a/contracts/settlement/src/lib.rs +++ b/contracts/settlement/src/lib.rs @@ -1,6 +1,6 @@ #![no_std] -use soroban_sdk::{contract, contracterror, contractimpl, contracttype, token, Address, Env, Symbol, Vec}; +use soroban_sdk::{contract, contracterror, contractimpl, contracttype, token, Address, BytesN, Env, Symbol, Vec}; /// Maximum number of items allowed in a single `batch_receive_payment` call. pub const MAX_BATCH_SIZE: u32 = 50; @@ -58,6 +58,7 @@ pub enum StorageKey { DeveloperBalance(Address), GlobalPool, Usdc, + ContractVersion, } /// Developer balance record in settlement contract @@ -791,6 +792,40 @@ impl CalloraSettlement { env.panic_with_error(SettlementError::Unauthorized); } } + + /// Admin-gated contract upgrade. + /// + /// Only the current admin may call. This will instruct the host to update + /// the current contract WASM to `new_wasm_hash` and persist the version marker. + /// Emits an `upgraded` event with the admin as topic and the new version as data. + pub fn upgrade(env: Env, caller: Address, new_wasm_hash: BytesN<32>) { + caller.require_auth(); + let admin = Self::get_admin(env.clone()); + if caller != admin { + env.panic_with_error(SettlementError::Unauthorized); + } + + // Perform the on-chain upgrade via the deployer interface. + env.deployer().update_current_contract_wasm(new_wasm_hash.clone()); + + // Persist the version marker for on-chain queries. + env.storage() + .instance() + .set(&StorageKey::ContractVersion, &new_wasm_hash); + + // Emit an event for indexers / audit logs. + env.events() + .publish((Symbol::new(&env, "upgraded"), admin), new_wasm_hash); + } + + /// Read the stored contract version (WASM hash) as last set by `upgrade`. + /// + /// Returns `None` if no upgrade has been performed yet (initial deployment). + pub fn get_version(env: Env) -> Option> { + env.storage() + .instance() + .get(&StorageKey::ContractVersion) + } } #[cfg(test)] diff --git a/contracts/settlement/src/test.rs b/contracts/settlement/src/test.rs index 51661258..ee1dbdd8 100644 --- a/contracts/settlement/src/test.rs +++ b/contracts/settlement/src/test.rs @@ -1679,5 +1679,22 @@ mod settlement_tests { pool.total_balance + sum_dev_balances, "Conservation invariant violated: total credits must equal pool + developer balances" ); + #[test] + fn test_upgrade_and_get_version() { + let (env, addr, admin, _vault, _third_party) = setup_contract(); + let client = CalloraSettlementClient::new(&env, &addr); + + assert_eq!(client.get_version(), None); + + let new_hash = BytesN::from_array(&env, &[1u8; 32]); + client.upgrade(&admin, &new_hash); + + assert_eq!(client.get_version(), Some(new_hash.clone())); + + // An `upgraded` event should have been emitted + let events = env.events().all(); + let ev = events.last().unwrap(); + let name = soroban_sdk::Symbol::try_from_val(&env, &ev.1.get(0).unwrap()).unwrap(); + assert_eq!(name, soroban_sdk::Symbol::new(&env, "upgraded")); } } diff --git a/contracts/vault/STORAGE.md b/contracts/vault/STORAGE.md index 918e0cce..74afa35c 100644 --- a/contracts/vault/STORAGE.md +++ b/contracts/vault/STORAGE.md @@ -71,7 +71,7 @@ pub enum StorageKey { PendingAdmin, // Address DepositorList, // Vec
ContractVersion, // BytesN<32> - ProcessedRequest(Symbol), // bool — temporary storage, idempotency marker + ProcessedRequest(Symbol), // bool — persistent storage, idempotency marker } ``` @@ -91,7 +91,7 @@ pub enum StorageKey { | `PendingAdmin` | Instance | `Address` | Two-step admin transfer nominee | `set_admin()`, `accept_admin()` | | `DepositorList` | Instance | `Vec
` | Allowed depositor addresses | `set_allowed_depositor()`, `get_allowed_depositors()` | | `ContractVersion` | Instance | `BytesN<32>` | WASM hash set by `upgrade()` | `upgrade()`, `version()` | -| `ProcessedRequest(Symbol)` | **Temporary** | `bool` | Idempotency marker for a processed deduct `request_id` | Written by `deduct()` / `batch_deduct()`; read by `is_request_processed()` | +| `ProcessedRequest(Symbol)` | **Persistent** | `bool` | Idempotency marker for a processed deduct `request_id` | Written by `deduct()` / `batch_deduct()`; read by `is_request_processed()` | ## Data Structures @@ -342,7 +342,7 @@ Monitor storage-related events: |---------|--------| | 1.0 | Initial `StorageKey` enum with `Meta`, `AllowedDepositors`, `Admin`, `UsdcToken`, `Settlement`, `RevenuePool`, `MaxDeduct`, `Metadata(String)` | | 1.1 | Renamed `StorageKey` → `DataKey`; added doc comments to all variants; removed stale `// Replaced by StorageKey enum variants` comment; updated STORAGE.md | -| 1.2 | Added `StorageKey::ProcessedRequest(Symbol)` in **temporary storage** for `request_id` idempotency in `deduct` and `batch_deduct`. Added `VaultError::DuplicateRequestId` (code 28). Added `is_request_processed(request_id)` view. TTL: threshold ~7 days, bump to ~30 days. | +| 1.2 | Added `StorageKey::ProcessedRequest(Symbol)` in **persistent storage** for `request_id` idempotency in `deduct` and `batch_deduct`. Added `VaultError::DuplicateRequestId` (code 28). Added `is_request_processed(request_id)` view. TTL: threshold ~7 days, bump to ~30 days. | ## Canonical Storage Keys @@ -364,10 +364,10 @@ All storage is accessed via `StorageKey` enum. | `PendingOwner` | Instance | Ownership transfer nominee | | `PendingAdmin` | Instance | Admin transfer nominee | | `ContractVersion` | Instance | WASM hash (set by `upgrade()`) | -| `ProcessedRequest(Symbol)` | **Temporary** | Idempotency marker; auto-expires after ~30 days | +| `ProcessedRequest(Symbol)` | **Persistent** | Idempotency marker; manually pruned | ### Migration - Removes deprecated `AllowedDepositors` - Ensures Admin fallback from Meta.owner -- `ProcessedRequest` uses temporary storage — no manual cleanup required; markers expire automatically \ No newline at end of file +- `ProcessedRequest` uses persistent storage — markers must be explicitly pruned using `prune_processed_requests` to avoid state bloat \ No newline at end of file diff --git a/contracts/vault/src/lib.rs b/contracts/vault/src/lib.rs index 43d2363d..84cec543 100644 --- a/contracts/vault/src/lib.rs +++ b/contracts/vault/src/lib.rs @@ -25,11 +25,11 @@ /// treated as a fire-and-forget deduction with no idempotency guarantee. /// /// ### Retention / TTL -/// Processed-request markers live in temporary storage and are bumped to -/// `REQUEST_ID_BUMP_AMOUNT` ledgers on every successful deduct. The threshold -/// for triggering a bump is `REQUEST_ID_BUMP_THRESHOLD`. After the TTL expires -/// the marker is archived and a previously-seen `request_id` can be reused — -/// callers must not rely on deduplication beyond the retention window. +/// Processed-request markers live in persistent storage and are bumped to +/// `REQUEST_ID_BUMP_AMOUNT` ledgers on every successful deduct. The threshold +/// for triggering a bump is `REQUEST_ID_BUMP_THRESHOLD`. Because they are now +/// persistent, they do not silently archive. To prevent state bloat, an owner +/// can explicitly prune old markers using `prune_processed_requests`. use soroban_sdk::{ contract, contractclient, contracterror, contractimpl, contracttype, token, Address, BytesN, Env, String, Symbol, Vec, @@ -147,9 +147,8 @@ pub enum StorageKey { ContractVersion, /// Idempotency marker for a processed deduct request. /// - /// Stored in **temporary storage** so it expires automatically after - /// `REQUEST_ID_BUMP_AMOUNT` ledgers. The value is `true` (a `bool`); - /// presence of the key is the authoritative signal. + /// Stored in **persistent storage**. The value is `true` (a `bool`); + /// presence of the key is the authoritative signal. Must be pruned explicitly. ProcessedRequest(Symbol), } @@ -177,9 +176,9 @@ pub const MAX_OFFERING_ID_LEN: u32 = 64; pub const INSTANCE_BUMP_THRESHOLD: u32 = 17_280 * 30; // ~30 days pub const INSTANCE_BUMP_AMOUNT: u32 = 17_280 * 60; // ~60 days -// Processed-request idempotency markers live in temporary storage. +// Processed-request idempotency markers live in persistent storage. // Bump when fewer than 7 days remain; extend to 30 days. -// After the TTL expires the marker is archived and the request_id can be reused. +// Must be pruned via prune_processed_requests when they are no longer needed. pub const REQUEST_ID_BUMP_THRESHOLD: u32 = 17_280 * 7; // ~7 days pub const REQUEST_ID_BUMP_AMOUNT: u32 = 17_280 * 30; // ~30 days @@ -613,7 +612,7 @@ impl CalloraVault { /// When `request_id` is `Some(id)`, the contract checks whether `id` has /// already been processed. If so, `VaultError::DuplicateRequestId` is /// returned immediately — no funds are moved. On first success the marker - /// is persisted in temporary storage for `REQUEST_ID_BUMP_AMOUNT` ledgers. + /// is persisted in persistent storage for `REQUEST_ID_BUMP_AMOUNT` ledgers. /// /// When `request_id` is `None`, no deduplication is performed. /// @@ -1148,12 +1147,31 @@ impl CalloraVault { /// Read the stored contract version (WASM hash) as last set by `upgrade`. /// /// Returns `None` if no upgrade has been performed yet (initial deployment). - pub fn version(env: Env) -> Option> { + pub fn get_version(env: Env) -> Option> { env.storage() .instance() .get(&StorageKey::ContractVersion) } + /// Garbage-collect processed request markers from persistent storage. + /// Only the owner can call this. + /// Emits a `request_id_pruned` event for each removed ID. + pub fn prune_processed_requests(env: Env, caller: Address, ids: Vec) -> Result<(), VaultError> { + caller.require_auth(); + Self::require_owner(env.clone(), caller.clone())?; + + for id in ids.iter() { + let key = StorageKey::ProcessedRequest(id.clone()); + if env.storage().persistent().has(&key) { + env.storage().persistent().remove(&key); + env.events() + .publish((Symbol::new(&env, "request_id_pruned"), caller.clone()), id.clone()); + } + } + + Ok(()) + } + // ----------------------------------------------------------------------- // Private helpers // ----------------------------------------------------------------------- @@ -1171,32 +1189,28 @@ impl CalloraVault { } /// Return `true` if `request_id` has already been processed (marker present - /// in temporary storage and not yet expired). + /// in persistent storage, or temporary storage for legacy markers). pub fn is_request_processed(env: Env, request_id: Symbol) -> bool { - env.storage() - .temporary() - .has(&StorageKey::ProcessedRequest(request_id)) + let key = StorageKey::ProcessedRequest(request_id); + env.storage().persistent().has(&key) || env.storage().temporary().has(&key) } /// Check that `request_id` has NOT been processed yet. /// Returns `VaultError::DuplicateRequestId` if the marker exists. fn require_not_duplicate(env: &Env, request_id: &Symbol) -> Result<(), VaultError> { - if env - .storage() - .temporary() - .has(&StorageKey::ProcessedRequest(request_id.clone())) - { + let key = StorageKey::ProcessedRequest(request_id.clone()); + if env.storage().persistent().has(&key) || env.storage().temporary().has(&key) { return Err(VaultError::DuplicateRequestId); } Ok(()) } - /// Persist a processed-request marker in temporary storage and set its TTL. + /// Persist a processed-request marker in persistent storage and set its TTL. fn mark_request_processed(env: &Env, request_id: &Symbol) { let key = StorageKey::ProcessedRequest(request_id.clone()); - env.storage().temporary().set(&key, &true); + env.storage().persistent().set(&key, &true); env.storage() - .temporary() + .persistent() .extend_ttl(&key, REQUEST_ID_BUMP_THRESHOLD, REQUEST_ID_BUMP_AMOUNT); } diff --git a/contracts/vault/src/test.rs b/contracts/vault/src/test.rs index 57c88236..c5e7afbd 100644 --- a/contracts/vault/src/test.rs +++ b/contracts/vault/src/test.rs @@ -5880,14 +5880,14 @@ fn upgrade_sets_version_and_emits_event() { client.init(&owner, &usdc, &Some(100), &None, &None, &None, &None); // Version should be None before any upgrade - assert_eq!(client.version(), None); + assert_eq!(client.get_version(), None); let new_hash = BytesN::from_array(&env, &[2u8; 32]); client.upgrade(&owner, &new_hash); // version() should return stored value - let readback = client.version(); + let readback = client.get_version(); assert_eq!(readback, Some(new_hash.clone())); // An `upgraded` event should have been emitted @@ -5927,7 +5927,7 @@ fn upgrade_non_owner_admin_succeeds() { // new_admin should be able to upgrade client.upgrade(&new_admin, &new_hash); - let readback = client.version(); + let readback = client.get_version(); assert_eq!(readback, Some(new_hash)); } @@ -5966,7 +5966,7 @@ fn version_returns_none_before_first_upgrade() { fund_vault(&usdc_admin, &vault_address, 100); client.init(&owner, &usdc, &Some(100), &None, &None, &None, &None); - assert_eq!(client.version(), None); + assert_eq!(client.get_version(), None); } #[test] @@ -5982,15 +5982,15 @@ fn upgrade_multiple_times_updates_version() { let hash1 = BytesN::from_array(&env, &[5u8; 32]); client.upgrade(&owner, &hash1); - assert_eq!(client.version(), Some(hash1.clone())); + assert_eq!(client.get_version(), Some(hash1.clone())); let hash2 = BytesN::from_array(&env, &[6u8; 32]); client.upgrade(&owner, &hash2); - assert_eq!(client.version(), Some(hash2.clone())); + assert_eq!(client.get_version(), Some(hash2.clone())); let hash3 = BytesN::from_array(&env, &[7u8; 32]); client.upgrade(&owner, &hash3); - assert_eq!(client.version(), Some(hash3)); + assert_eq!(client.get_version(), Some(hash3)); } // --------------------------------------------------------------------------- diff --git a/contracts/vault/src/test_idempotency.rs b/contracts/vault/src/test_idempotency.rs index 09eb5d2d..880a1d6b 100644 --- a/contracts/vault/src/test_idempotency.rs +++ b/contracts/vault/src/test_idempotency.rs @@ -423,3 +423,99 @@ fn batch_deduct_mixed_ids_marks_only_some_ids() { // None deducts still go through. assert_eq!(client.deduct(&owner, &10, &None), 765); } + +#[test] +fn replay_across_long_window_rejected() { + let env = Env::default(); + let (_, client, _, owner) = setup_vault(&env, 1_000); + + let rid = Symbol::new(&env, "req_long_win"); + + // First call succeeds + client.deduct(&owner, &100, &Some(rid.clone())); + + // Fast-forward ledger 6 months (approx 6 * 30 days) + let new_timestamp = env.ledger().timestamp() + 180 * 24 * 60 * 60; + env.ledger().set(soroban_sdk::testutils::LedgerInfo { + timestamp: new_timestamp, + protocol_version: 20, + sequence_number: env.ledger().sequence() + 180 * 17_280, + network_id: env.ledger().network_id(), + base_reserve: env.ledger().base_reserve(), + max_entry_expiration: env.ledger().max_entry_expiration(), + min_temp_entry_expiration: env.ledger().min_temp_entry_expiration(), + min_persistent_entry_expiration: env.ledger().min_persistent_entry_expiration(), + }); + + // Retry should still be rejected because it's persistent and hasn't been explicitly pruned. + let res = client.try_deduct(&owner, &100, &Some(rid.clone())); + assert!(res.is_err(), "should still reject after multi-month window"); +} + +#[test] +fn gc_entrypoint_prunes_and_emits_event() { + let env = Env::default(); + let (_, client, _, owner) = setup_vault(&env, 1_000); + + let rid1 = Symbol::new(&env, "req_gc_1"); + let rid2 = Symbol::new(&env, "req_gc_2"); + + client.deduct(&owner, &100, &Some(rid1.clone())); + client.deduct(&owner, &100, &Some(rid2.clone())); + + let mut ids_to_prune = soroban_sdk::Vec::new(&env); + ids_to_prune.push_back(rid1.clone()); + + client.prune_processed_requests(&owner, &ids_to_prune).unwrap(); + + assert_eq!(client.is_request_processed(&rid1), false); + assert_eq!(client.is_request_processed(&rid2), true); + + let events = env.events().all(); + let mut has_event = false; + for ev in events.iter() { + if let Ok(topic) = soroban_sdk::Symbol::try_from_val(&env, &ev.1.get(0).unwrap()) { + if topic == Symbol::new(&env, "request_id_pruned") { + has_event = true; + break; + } + } + } + assert!(has_event, "Should emit request_id_pruned event"); + + // Should now be able to replay rid1 + client.deduct(&owner, &100, &Some(rid1)); +} + +#[test] +fn gc_ignores_unknown_ids() { + let env = Env::default(); + let (_, client, _, owner) = setup_vault(&env, 1_000); + + let rid_unknown = Symbol::new(&env, "req_unknown"); + + let mut ids_to_prune = soroban_sdk::Vec::new(&env); + ids_to_prune.push_back(rid_unknown.clone()); + + // Shouldn't fail, just skips + client.prune_processed_requests(&owner, &ids_to_prune).unwrap(); +} + +#[test] +fn gc_allowed_during_pause() { + let env = Env::default(); + let (_, client, _, owner) = setup_vault(&env, 1_000); + + let rid1 = Symbol::new(&env, "req_gc_pause"); + client.deduct(&owner, &100, &Some(rid1.clone())); + + client.pause(&owner); + assert!(client.is_paused()); + + let mut ids_to_prune = soroban_sdk::Vec::new(&env); + ids_to_prune.push_back(rid1.clone()); + + // Prune should succeed even when paused + client.prune_processed_requests(&owner, &ids_to_prune).unwrap(); + assert_eq!(client.is_request_processed(&rid1), false); +}