diff --git a/abis/settlement-workflow.json b/abis/settlement-workflow.json index 89f7274..cb0ffed 100644 --- a/abis/settlement-workflow.json +++ b/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", diff --git a/contracts/compliance/tests/schema_version_test.rs b/contracts/compliance/tests/schema_version_test.rs index 40103c0..cdf4c90 100644 --- a/contracts/compliance/tests/schema_version_test.rs +++ b/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 = 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 = env + .events() + .all() + .iter() + .map(|(_, topics, _)| Symbol::from_val(&env, &topics.get_unchecked(0)).to_string()) + .collect(); + assert_eq!(before_events, after_events); +} diff --git a/contracts/settlement-workflow/src/lib.rs b/contracts/settlement-workflow/src/lib.rs index e94fd6e..edbb085 100644 --- a/contracts/settlement-workflow/src/lib.rs +++ b/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) ─────────────────────────────────── diff --git a/contracts/settlement-workflow/tests/batch_mixed_compliance_test.rs b/contracts/settlement-workflow/tests/batch_mixed_compliance_test.rs index 10af266..3d9fa34 100644 --- a/contracts/settlement-workflow/tests/batch_mixed_compliance_test.rs +++ b/contracts/settlement-workflow/tests/batch_mixed_compliance_test.rs @@ -14,7 +14,9 @@ //! so the batch is partial — funds move only for the items that actually //! executed, and the per-item plus summary events describe exactly that. use compliance::{ComplianceContract, ComplianceContractClient}; -use settlement_workflow::{SettlementWorkflowContract, SettlementWorkflowContractClient}; +use settlement_workflow::{ + SettlementBatchResult, SettlementWorkflowContract, SettlementWorkflowContractClient, +}; use soroban_sdk::{ testutils::{Address as _, Events}, token, Address, Env, FromVal, Symbol, TryFromVal, Vec, @@ -255,7 +257,13 @@ fn batch_executes_after_the_merchant_passes_the_gate_again() { let executed = f .workflow .execute_with_compliance_batch(&batch, &f.token_id, &f.merchant); - assert_eq!(executed, Vec::from_array(&f.env, [s1, s2])); + assert_eq!( + executed, + soroban_sdk::Vec::from_array( + &f.env, + [SettlementBatchResult::Executed, SettlementBatchResult::Executed] + ) + ); assert_eq!(f.balance(&f.merchant), 5_000_000); } @@ -284,7 +292,18 @@ fn batch_skips_unsettleable_items_and_settles_the_rest() { &f.merchant, ); - assert_eq!(executed, Vec::from_array(&f.env, [first, second])); + assert_eq!( + executed, + soroban_sdk::Vec::from_array( + &f.env, + [ + SettlementBatchResult::Executed, + SettlementBatchResult::SkippedNotFound, + SettlementBatchResult::SkippedOnHold, + SettlementBatchResult::Executed, + ] + ) + ); // Only the two executable settlements moved funds. assert_eq!(f.balance(&f.merchant), 4_000_000); assert_eq!(f.status(first), SettlementStatus::Executed); @@ -372,7 +391,16 @@ fn previously_skipped_settlement_executes_in_a_later_batch() { let executed = f.workflow .execute_with_compliance_batch(&f.ids(&[first, held]), &f.token_id, &f.merchant); - assert_eq!(executed, Vec::from_array(&f.env, [first])); + assert_eq!( + executed, + soroban_sdk::Vec::from_array( + &f.env, + [ + SettlementBatchResult::Executed, + SettlementBatchResult::SkippedOnHold, + ] + ) + ); assert_eq!(f.balance(&f.merchant), 1_000_000); f.treasury.release_hold(&f.admin, &held); @@ -380,7 +408,10 @@ fn previously_skipped_settlement_executes_in_a_later_batch() { let executed = f.workflow .execute_with_compliance_batch(&f.ids(&[held]), &f.token_id, &f.merchant); - assert_eq!(executed, Vec::from_array(&f.env, [held])); + assert_eq!( + executed, + soroban_sdk::Vec::from_array(&f.env, [SettlementBatchResult::Executed]) + ); assert_eq!(f.balance(&f.merchant), 3_000_000); } @@ -403,7 +434,16 @@ fn replayed_batch_executes_nothing_and_pays_nothing_twice() { let executed = f .workflow .execute_with_compliance_batch(&batch, &f.token_id, &f.merchant); - assert_eq!(executed, Vec::from_array(&f.env, [])); + assert_eq!( + executed, + soroban_sdk::Vec::from_array( + &f.env, + [ + SettlementBatchResult::SkippedAlreadyExecuted, + SettlementBatchResult::SkippedAlreadyExecuted, + ] + ) + ); let (_, _, summary) = f .env @@ -445,7 +485,10 @@ fn allowed_merchant_is_unaffected_by_another_blocked_merchant() { let executed = f.workflow .execute_with_compliance_batch(&f.ids(&[allowed_id]), &f.token_id, &f.merchant); - assert_eq!(executed, Vec::from_array(&f.env, [allowed_id])); + assert_eq!( + executed, + soroban_sdk::Vec::from_array(&f.env, [SettlementBatchResult::Executed]) + ); assert_eq!(f.balance(&f.merchant), 1_500_000); // Blocked recipient: rejected, receives nothing. diff --git a/contracts/settlement-workflow/tests/settlement_workflow_test.rs b/contracts/settlement-workflow/tests/settlement_workflow_test.rs index d29ffaf..96b3dc4 100644 --- a/contracts/settlement-workflow/tests/settlement_workflow_test.rs +++ b/contracts/settlement-workflow/tests/settlement_workflow_test.rs @@ -196,7 +196,14 @@ fn batch_executes_multiple_settlements_and_skips_invalid_ids() { let executed = workflow.execute_with_compliance_batch(&ids, &token_id, &merchant); assert_eq!( executed, - soroban_sdk::Vec::from_array(&env, [good_1, good_2]) + soroban_sdk::Vec::from_array( + &env, + [ + SettlementBatchResult::Executed, + SettlementBatchResult::SkippedNotFound, + SettlementBatchResult::Executed, + ] + ) ); assert_eq!( token::Client::new(&env, &token_id).balance(&merchant), @@ -759,7 +766,16 @@ fn batch_summary_is_emitted_when_every_item_is_skipped() { ids.push_back(bogus); let executed = workflow.execute_with_compliance_batch(&ids, &token_id, &merchant); - assert_eq!(executed, soroban_sdk::Vec::from_array(&env, [])); + assert_eq!( + executed, + soroban_sdk::Vec::from_array( + &env, + [ + SettlementBatchResult::SkippedAlreadyExecuted, + SettlementBatchResult::SkippedNotFound, + ] + ) + ); let events = workflow_events(&env, &workflow_id); assert_eq!( @@ -776,6 +792,90 @@ fn batch_summary_is_emitted_when_every_item_is_skipped() { ); } +/// #771: `update_contract_ids` lets the admin re-point the workflow at upgraded +/// compliance or treasury contracts without redeploying the workflow. +#[test] +fn update_contract_ids_repoints_workflow() { + let ( + env, + admin, + _merchant, + _compliance, + _compliance_id, + _treasury, + _treasury_id, + workflow, + _token_id, + ) = setup(); + let new_compliance_id = Address::generate(&env); + let new_treasury_id = Address::generate(&env); + + // Admin can update the pinned contract IDs. + workflow.update_contract_ids(&admin, &new_compliance_id, &new_treasury_id); + + // A stranger cannot. + let stranger = Address::generate(&env); + let err = workflow + .try_update_contract_ids(&stranger, &new_compliance_id, &new_treasury_id) + .unwrap_err() + .unwrap(); + assert_eq!(err, TreasuryError::Unauthorized.into()); +} + +/// #771: `update_contract_ids` emits `contract_ids_updated`. +#[test] +fn update_contract_ids_emits_event() { + let ( + env, + admin, + _merchant, + _compliance, + _compliance_id, + _treasury, + _treasury_id, + workflow, + _token_id, + ) = setup(); + let workflow_id = workflow.address.clone(); + let new_compliance_id = Address::generate(&env); + let new_treasury_id = Address::generate(&env); + + workflow.update_contract_ids(&admin, &new_compliance_id, &new_treasury_id); + + let events = workflow_events(&env, &workflow_id); + assert!( + events.iter().any(|(name, ..)| name == "contract_ids_updated"), + "contract_ids_updated event must be emitted" + ); +} + +/// #771: `update_contract_ids` is rejected while paused. +#[test] +fn update_contract_ids_rejected_while_paused() { + let ( + env, + admin, + _merchant, + _compliance, + _compliance_id, + _treasury, + _treasury_id, + workflow, + _token_id, + ) = setup(); + + workflow.pause(&admin); + let err = workflow + .try_update_contract_ids( + &admin, + &Address::generate(&env), + &Address::generate(&env), + ) + .unwrap_err() + .unwrap(); + assert_eq!(err, TreasuryError::ContractPaused.into()); +} + /// The workflow's emitted event names must match `abis/settlement-workflow.json` /// exactly (#614). ///