feat: return per-item batch results, migrate tests, update_contract_i…

abis/settlement-workflow.json

@@ -14,11 +14,13 @@
     "pause",
     "resume_all",
     "transfer_admin",
-    "unpause"
+    "unpause",
+    "update_contract_ids"
   ],
   "events": [
     "admin_transfer_initiated",
     "admin_transferred",
+    "contract_ids_updated",
     "emergency_pause_completed",
     "emergency_pause_configured",
     "emergency_pause_resumed",

contracts/compliance/tests/schema_version_test.rs

@@ -143,3 +143,136 @@ fn migrate_rejects_non_admin() {
         Err(Ok(ComplianceError::Unauthorized))
     );
 }
+
+// ── #763 migrate from v1 to v2 with populated state ──────────────────────────
+
+/// Seeds v1-shaped storage with allowed, blocked and expiring addresses,
+/// runs `migrate`, and checks that every query returns the same answers.
+#[test]
+fn migrate_v1_to_v2_preserves_populated_state() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let allowed_addr = Address::generate(&env);
+    let blocked_addr = Address::generate(&env);
+    let expiring_addr = Address::generate(&env);
+    let now = env.ledger().timestamp();
+    let far_future = now + 3600;
+
+    let id = env.register_contract(None, ComplianceContract);
+    let client = ComplianceContractClient::new(&env, &id);
+    client.initialize(&admin);
+
+    // Force storage to v1 while keeping the populated v1-shaped state.
+    env.as_contract(&id, || {
+        env.storage()
+            .instance()
+            .set(&DataKey::SchemaVersion, &1u32);
+        env.storage().persistent().set(&DataKey::Allowed(allowed_addr.clone()), &true);
+        env.storage().persistent().set(&DataKey::Blocked(blocked_addr.clone()), &true);
+        env.storage()
+            .persistent()
+            .set(&DataKey::Allowed(expiring_addr.clone()), &true);
+        env.storage()
+            .persistent()
+            .set(&DataKey::AllowedUntil(expiring_addr.clone()), &far_future);
+    });
+
+    // Snapshot query answers before migration.
+    let before_allowed = client.is_allowed(&allowed_addr);
+    let before_blocked_allowed = client.is_allowed(&blocked_addr);
+    let before_expiring_allowed = client.is_allowed(&expiring_addr);
+    let before_blocked = client.is_blocked(&blocked_addr);
+    let before_expiring_blocked = client.is_blocked(&expiring_addr);
+    let before_expiring_status = client.address_status(&expiring_addr);
+
+    // Migrate from v1 to v2.
+    let version = client.migrate(&admin);
+    assert_eq!(version, CURRENT_SCHEMA_VERSION);
+    assert_eq!(client.get_schema_version(), CURRENT_SCHEMA_VERSION);
+
+    // Every query must return the same answers after migration.
+    assert_eq!(client.is_allowed(&allowed_addr), before_allowed);
+    assert_eq!(client.is_allowed(&blocked_addr), before_blocked_allowed);
+    assert_eq!(client.is_allowed(&expiring_addr), before_expiring_allowed);
+    assert_eq!(client.is_blocked(&blocked_addr), before_blocked);
+    assert_eq!(client.is_blocked(&expiring_addr), before_expiring_blocked);
+    assert_eq!(client.address_status(&expiring_addr), before_expiring_status);
+}
+
+/// `address_status` must be callable and consistent before and after migration
+/// for an address that has both an allow expiry and a block override.
+#[test]
+fn migrate_preserves_address_status_for_blocked_and_expiring() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let addr = Address::generate(&env);
+    let now = env.ledger().timestamp();
+    let far_future = now + 3600;
+
+    let id = env.register_contract(None, ComplianceContract);
+    let client = ComplianceContractClient::new(&env, &id);
+    client.initialize(&admin);
+
+    env.as_contract(&id, || {
+        env.storage()
+            .instance()
+            .set(&DataKey::SchemaVersion, &1u32);
+        env.storage()
+            .persistent()
+            .set(&DataKey::Allowed(addr.clone()), &true);
+        env.storage()
+            .persistent()
+            .set(&DataKey::Blocked(addr.clone()), &true);
+        env.storage()
+            .persistent()
+            .set(&DataKey::AllowedUntil(addr.clone()), &far_future);
+    });
+
+    let before = client.address_status(&addr);
+    client.migrate(&admin);
+    let after = client.address_status(&addr);
+    assert_eq!(before, after);
+    assert_eq!(client.get_schema_version(), CURRENT_SCHEMA_VERSION);
+}
+
+// ── #764 migrate at current schema version is a clean no-op ──────────────────
+
+/// Calling `migrate` when already at `CURRENT_SCHEMA_VERSION` must be a clean
+/// no-op: it returns the current version, emits no `schema_migrated` event, and
+/// leaves storage untouched.
+#[test]
+fn migrate_at_current_version_is_noop() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+
+    let id = env.register_contract(None, ComplianceContract);
+    let client = ComplianceContractClient::new(&env, &id);
+    client.initialize(&admin);
+
+    // Already at CURRENT_SCHEMA_VERSION (= 2) after initialize.
+    assert_eq!(client.get_schema_version(), CURRENT_SCHEMA_VERSION);
+
+    let before_events: std::vec::Vec<String> = env
+        .events()
+        .all()
+        .iter()
+        .map(|(_, topics, _)| Symbol::from_val(&env, &topics.get_unchecked(0)).to_string())
+        .collect();
+    assert!(!before_events.iter().any(|n| n == "schema_migrated"));
+
+    let result = client.migrate(&admin);
+    assert_eq!(result, CURRENT_SCHEMA_VERSION);
+    assert_eq!(client.get_schema_version(), CURRENT_SCHEMA_VERSION);
+
+    // No `schema_migrated` event should have been emitted by the no-op call.
+    let after_events: std::vec::Vec<String> = env
+        .events()
+        .all()
+        .iter()
+        .map(|(_, topics, _)| Symbol::from_val(&env, &topics.get_unchecked(0)).to_string())
+        .collect();
+    assert_eq!(before_events, after_events);
+}

contracts/settlement-workflow/src/lib.rs

@@ -68,6 +68,55 @@ pub trait PausableInterface {
     fn unpause(env: Env, admin: Address);
 }
 
+/// Per-item outcome of `execute_with_compliance_batch` (#776).
+///
+/// One entry is returned per requested settlement ID, in the same order, so
+/// callers can tell exactly which settlements executed and which were skipped
+/// (and why) without diffing events or re-querying treasury.
+#[contracttype]
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum SettlementBatchResult {
+    /// The settlement was executed and funds moved.
+    Executed,
+    /// No settlement exists with this ID.
+    SkippedNotFound,
+    /// The settlement is already executed (or otherwise terminal).
+    SkippedAlreadyExecuted,
+    /// The settlement is on hold and the hold has not expired.
+    SkippedOnHold,
+    /// The settlement's approval weight is below the threshold.
+    SkippedThresholdNotMet,
+    /// The settlement's execution deadline has passed.
+    SkippedExecutionDeadlineExceeded,
+    /// The token contract is the treasury itself (invalid).
+    SkippedInvalidTokenContract,
+    /// The token contract is not on the allowlist.
+    SkippedTokenNotAllowed,
+}
+
+/// Maps a treasury execution error to the corresponding per-item skip status.
+/// Errors that are pre-checked at batch level (threshold, token contract,
+/// allowlist) are included for completeness even though they abort the whole
+/// batch before the per-item loop.
+fn treasury_error_to_batch_result(err: TreasuryError) -> SettlementBatchResult {
+    use TreasuryError::*;
+    match err {
+        SettlementNotFound => SettlementBatchResult::SkippedNotFound,
+        AlreadyExecuted => SettlementBatchResult::SkippedAlreadyExecuted,
+        SettlementOnHold => SettlementBatchResult::SkippedOnHold,
+        ThresholdNotMet => SettlementBatchResult::SkippedThresholdNotMet,
+        ExecutionDeadlineExceeded => {
+            SettlementBatchResult::SkippedExecutionDeadlineExceeded
+        }
+        InvalidTokenContract => SettlementBatchResult::SkippedInvalidTokenContract,
+        TokenNotAllowed => SettlementBatchResult::SkippedTokenNotAllowed,
+        // Batch-level pre-check failures; should not occur per-item once the
+        // pre-checks have passed, but mapped defensively.
+        ThresholdNotConfigured => SettlementBatchResult::SkippedThresholdNotMet,
+        _ => SettlementBatchResult::SkippedAlreadyExecuted,
+    }
+}
+
 /// Largest number of contracts a single `emergency_pause_all` may fan out to.
 ///
 /// Bounded because the fan-out is a sequence of cross-contract invocations: an
@@ -272,6 +321,35 @@ impl SettlementWorkflowContract {
         Ok(())
     }
 
+    /// Updates the pinned compliance and treasury contract instances (#771).
+    ///
+    /// Lets the admin point the workflow at upgraded compliance or treasury
+    /// contracts without redeploying the workflow itself. `ComplianceId` and
+    /// `TreasuryId` are fixed at initialization, so redeploying compliance
+    /// previously meant redeploying the workflow too.
+    ///
+    /// Admin-only: `admin` must authorize and be the stored administrator.
+    /// Emits: `contract_ids_updated`.
+    pub fn update_contract_ids(
+        env: Env,
+        admin: Address,
+        compliance_id: Address,
+        treasury_id: Address,
+    ) -> Result<(), TreasuryError> {
+        Self::require_admin(&env, &admin)?;
+        env.storage()
+            .instance()
+            .set(&DataKey::ComplianceId, &compliance_id);
+        env.storage()
+            .instance()
+            .set(&DataKey::TreasuryId, &treasury_id);
+        env.events().publish(
+            (Symbol::new(&env, "contract_ids_updated"),),
+            (compliance_id.clone(), treasury_id.clone()),
+        );
+        Ok(())
+    }
+
     /// Returns the pinned compliance contract instance.
     fn compliance_id(env: &Env) -> Address {
         env.storage()
@@ -439,10 +517,11 @@ impl SettlementWorkflowContract {
     /// gate for `merchant` once, then executes each settlement ID through the pinned
     /// treasury. Settlement IDs that don't exist, are already executed, or otherwise
     /// fail treasury execution are silently skipped (per treasury's batch precedent,
-    /// #38) rather than aborting the whole batch; only successfully executed IDs are
-    /// returned and emitted. If the shared compliance gate fails, the whole batch is
-    /// rejected with `ComplianceCheckFailed`. Traps with `ContractPaused` while the
-    /// workflow is paused (#616).
+    /// #38) rather than aborting the whole batch; a per-item status is returned for
+    /// every requested ID so callers can tell which settled and which were skipped.
+    /// If the shared compliance gate fails, the whole batch is rejected with
+    /// `ComplianceCheckFailed`. Traps with `ContractPaused` while the workflow is
+    /// paused (#616).
     ///
     /// Emits `settlement_workflow_executed` for each settlement actually executed,
     /// each carrying `(merchant, token_contract, amount)`, followed by exactly one
@@ -461,34 +540,41 @@ impl SettlementWorkflowContract {
         compliance.require_allowed_for_treasury(&merchant)?;
         let treasury = TreasuryOnlyClient::new(&env, &Self::treasury_id(&env));
         let requested = settlement_ids.len();
-        let mut executed = Vec::new(&env);
+        let mut results = Vec::new(&env);
+        let mut executed_count: u32 = 0;
         for id in settlement_ids.iter() {
             let result = treasury.try_execute_settlement(
                 &env.current_contract_address(),
                 &id,
                 &token_contract,
             );
-            if result.is_ok() {
-                // Read after execution: a skipped (non-existent) ID is never read,
-                // so `get_settlement`'s `SettlementNotFound` panic can't turn a
-                // skippable ID into a batch-wide abort.
-                let amount = treasury.get_settlement(&id).amount;
-                executed.push_back(id);
-                env.events().publish(
-                    (Symbol::new(&env, "settlement_workflow_executed"), id),
-                    (merchant.clone(), token_contract.clone(), amount),
-                );
+            match result {
+                Ok(()) => {
+                    // Read after execution: a skipped (non-existent) ID is never read,
+                    // so `get_settlement`'s `SettlementNotFound` panic can't turn a
+                    // skippable ID into a batch-wide abort.
+                    let amount = treasury.get_settlement(&id).amount;
+                    results.push_back(SettlementBatchResult::Executed);
+                    executed_count += 1;
+                    env.events().publish(
+                        (Symbol::new(&env, "settlement_workflow_executed"), id),
+                        (merchant.clone(), token_contract.clone(), amount),
+                    );
+                }
+                Err(err) => {
+                    results.push_back(treasury_error_to_batch_result(err));
+                    // Invalid / already-executed / threshold-failed IDs are silently skipped.
+                }
             }
-            // Invalid / already-executed / threshold-failed IDs are silently skipped.
         }
         // `workflow_batch_completed` (not `settlement_workflow_batch_completed`):
         // Soroban symbols are capped at 32 characters and the longer name is
         // rejected by the host at runtime with `InvalidInput`.
         env.events().publish(
             (Symbol::new(&env, "workflow_batch_completed"),),
-            (requested, executed.len()),
+            (requested, executed_count),
         );
-        Ok(executed)
+        Ok(results)
     }
 
     // ── emergency pause coordination (#73) ───────────────────────────────────