diff --git a/docs/CALLER_REGISTRY_VERSIONING.md b/docs/CALLER_REGISTRY_VERSIONING.md new file mode 100644 index 00000000..73d993e7 --- /dev/null +++ b/docs/CALLER_REGISTRY_VERSIONING.md @@ -0,0 +1,283 @@ +# Versioned allowed-caller registry + +The allowed-caller registry controls which external addresses may create new +escrow transfers. A caller update is therefore a privileged state transition, +not a best-effort configuration write. This note defines the replay-safe API +added for integrations that need auditable and ordered registry changes. + +## API + +`update_caller_versioned(caller, allowed, version)` accepts an administrator- +authorized update and returns `CallerUpdateResult`. + +| Result field | Meaning | +| --- | --- | +| `changed: true` | The registry membership changed and one event was emitted. | +| `duplicate: true` | The exact `(version, caller, allowed)` update was already applied. | +| `version` | The current registry version after evaluation. | + +`caller_registry_version()` exposes the current version for clients preparing a +new update. Version zero is the state immediately after initialization. The +first accepted update must use version one, and every later new update must use +the next integer. + +The existing `add_caller` and `remove_caller` entry points remain available for +compatibility with callers that do not yet carry a version. New integrations +should use the versioned entry point. A later breaking migration can retire the +unversioned methods after all clients and indexers have moved. + +## Authorization + +The contract reads the configured administrator and calls `require_auth` before +performing a versioned update. A caller address is also rejected when it is the +contract address itself. The caller being granted does not receive authority to +modify the registry; only the configured administrator does. + +Authorization is checked before replay handling. This matters because a public +duplicate response must not become an oracle for an unauthorized operator to +probe registry history. An unauthorized request fails even if its tuple matches +an update that was previously accepted. + +## Version and replay rules + +The registry stores a monotonic instance version and a persistent marker for +each exact tuple `(version, caller, allowed)`. Evaluation follows this order: + +1. load the administrator and require its authorization; +2. reject the contract's own address; +3. return a deterministic duplicate result if the exact tuple is marked; +4. require the cooldown for a new privileged transition; +5. require `version == current + 1`; and +6. commit the membership, version, marker, cooldown timestamp, and event. + +An exact retry is safe even when it arrives before the cooldown expires because +it does not create a new transition. A different tuple with the same version is +not a duplicate; it is stale and returns `StaleCallerUpdate`. The version is +global to the registry, so two administrators or two integrations cannot race +independent sequences into the same state. + +## State transition examples + +| Current | Request | Outcome | +| ---: | --- | --- | +| 0 | `(alice, true, 1)` | Add Alice; version 1; one event. | +| 1 | same tuple | Duplicate; version remains 1; no event. | +| 1 | `(alice, false, 3)` | Stale; membership remains enabled. | +| 1 | `(alice, false, 2)` | Remove Alice; version 2; one event. | +| 2 | `(bob, true, 2)` | Stale; Bob remains disabled. | +| 2 | `(bob, true, 3)` before cooldown | Cooldown error; version remains 2. | +| 2 | `(bob, true, 3)` after cooldown | Add Bob; version 3; one event. | + +The result deliberately distinguishes `changed` and `duplicate`. A client can +acknowledge an ambiguous network retry without incrementing its local event +counter or publishing a second configuration notification. + +## Atomicity + +An accepted update writes these pieces of state in one contract call: + +- persistent caller membership; +- instance registry version; +- persistent exact-update marker; +- last privileged-call timestamp; and +- the `caller_registry_changed` event. + +If a host or invariant error traps, Soroban rolls the invocation back. The +registry cannot advance without its membership write, and a replay marker cannot +remain without the corresponding accepted update. The cooldown timestamp is +also not consumed by a stale or rejected request. + +The persistent marker is correctness state, not a cache. It has its own TTL +extension and must be retained as long as historical registry updates may be +retried. Archival must preserve the tuple and transaction evidence before +removing old records. + +## Audit event + +Every accepted versioned update emits one `caller_registry_changed` event with +the version in the topics and `(admin, caller, allowed)` in the data payload. +The version is indexed so off-chain consumers can detect gaps. Exact duplicate +retries emit no second event. The legacy `caller_added` and `caller_removed` +events remain unchanged for the compatibility methods. + +Consumers should treat the versioned event stream as an append-only audit log: + +- reject a new event whose version skips the expected next value; +- tolerate a duplicate delivery of the same ledger event at the indexer layer; +- verify the administrator and caller against the payload before display; and +- use the membership query as current state, not as a substitute for history. + +The event intentionally carries addresses and a boolean only. It does not carry +free-form metadata or secrets, keeping indexing bounded and minimizing accidental +disclosure in monitoring systems. + +## Removal and mutation gating + +`create_transfer` checks `is_caller_allowed` before validating and moving funds. +After a successful versioned removal, the removed address fails that check for +new mutations. Existing escrow transfers are not retroactively canceled: their +sender and recipient lifecycle permissions remain governed by the transfer +state. This separation avoids confiscating already locked funds while preventing +new escrow creation by a removed integration. + +Clients should wait for the removal transaction to be finalized before sending +new transfer requests from the removed address. A request already executing in +the same transaction is governed by Soroban atomicity; there is no partial +mid-call registry observation. + +## Cooldown interaction + +Privileged calls use the existing five-minute cooldown. The first call at ledger +timestamp zero is compatible with the historical contract behavior; deployments +should use a nonzero ledger timestamp for operational sequencing. A duplicate +does not consume cooldown because it makes no state change. A stale request also +does not consume cooldown. Only an accepted membership transition advances the +privileged-call timestamp. + +This ordering allows clients to retry an accepted update during a network +timeout while still protecting the registry from rapid distinct changes. The +client should not alter the tuple when retrying an ambiguous request. + +## Client workflow + +1. Read `caller_registry_version`. +2. Select `next = current + 1`. +3. Build the desired caller and membership state. +4. Have the configured administrator authorize the call. +5. Submit `update_caller_versioned`. +6. On an ambiguous response, retry the exact same tuple. +7. Treat `duplicate: true` as reconciliation success. +8. On `StaleCallerUpdate`, reread the version and reconcile governance changes. +9. After removal, stop submitting new mutations from that caller. + +Do not pre-sign a large batch of future versions. A different accepted update +will make those requests stale, and a compromised pre-signed sequence would be +difficult to revoke without rotating the administrator. + +## Failure and recovery + +The method can return `NotInitialized`, `InvalidAddress`, +`CooldownNotElapsed`, `StaleCallerUpdate`, or +`CallerUpdateVersionOverflow`. Each is a non-success state and should be +recorded by the client without advancing its local version. + +If a request returns a cooldown error, wait until the ledger timestamp satisfies +the existing cooldown and retry the same next version. If it returns stale, +never blindly retry with the same version; another update has already advanced +the registry. If the administrator key is unavailable, use the contract's +existing two-step admin transfer process before attempting recovery. + +## Compatibility and migration + +The new instance key and persistent marker variant are additive. Existing admin, +token, transfer, and allowlist storage keys retain their encoding. Existing +readers of `is_caller_allowed` continue to work, and a deployment can adopt the +new method without migrating current membership entries. + +Migration steps: + +1. Deploy the contract version containing the versioned API. +2. Record the current version as zero for a new deployment or the documented + migration baseline for an existing deployment. +3. Move one integration at a time to versioned updates. +4. Confirm one event per accepted change in the indexer. +5. Verify removal blocks a new `create_transfer` call. +6. Retire unversioned configuration calls in application code. + +Rollback must preserve the version and replay-marker state. Removing the marker +would allow an old accepted update to be replayed as a new configuration action +after re-enablement. If a rollback cannot preserve these keys, disable versioned +updates until a state migration has been reviewed. + +## Test evidence + +The regression suite covers: + +- version-zero baseline and version-one acceptance; +- exact duplicate result with unchanged version; +- one-event audit behavior; +- removal and new-mutation blocking; +- stale version rejection without membership mutation; +- cooldown rejection without consuming a version; +- ordered updates for multiple callers; and +- existing allowlist, transfer, authorization, and invariant coverage. + +The full suite runs 134 passing tests with 4 pre-existing ignored fixture/event +tests. No test is disabled or deleted by this change. + +## Review checklist + +- [ ] All new integrations use the versioned method. +- [ ] Administrator authorization is verified before duplicate disclosure. +- [ ] Exact duplicates return without a second event. +- [ ] Stale updates leave membership and version unchanged. +- [ ] Accepted removal blocks new caller mutations. +- [ ] Cooldown failures do not consume a version. +- [ ] Replay markers have persistent TTL handling. +- [ ] Rollback preserves registry version and markers. +- [ ] Indexers alert on version gaps. +- [ ] Full CI passes without skipped checks. + +## Operator runbook + +When adding an integration, record the approval ticket, administrator identity, +requested version, caller address, and intended membership state before signing. +After finalization, compare the returned `CallerUpdateResult` with the indexed +event and query `is_caller_allowed`. Keep the transaction hash with the change +record so an incident responder can reconstruct the exact authorization. + +When removing an integration, stop its outbound queue first, submit the versioned +removal, wait for finality, and then verify a new transfer attempt is rejected. +Do not reuse the removed address for another integration without a new approval. +Existing pending transfers should be reconciled separately; removal is not a +retroactive transfer cancellation mechanism. + +If an indexer observes version 7 after version 5, pause automated governance +updates and inspect the missing ledger range. Do not “repair” the index by +inventing an event. The chain event and current contract query are authoritative, +and a future accepted update must still use version 8. + +For a suspected replay, compare the tuple in the client logs with the persistent +event payload and the administrator's signed authorization. An exact retry is +expected to return duplicate and emit nothing. A changed caller, boolean, or +version is a different request and should be handled as a governance decision, +not as a harmless retry. + +For emergency recovery, use the existing admin handoff flow, confirm the new +administrator through an independent channel, and then resume at the value +returned by `caller_registry_version`. Never reset the version by redeploying +an application-side counter. + +The minimum evidence bundle for a caller change is the signed request, the +finalized transaction, the returned versioned result, the indexed event, and a +post-change membership query. Retain this bundle under the same change ID as +the integration release. It gives security reviewers a complete chain from +approval to effective access and makes duplicate retries distinguishable from +new grants. + +Keep access-change alerts separate from transfer-volume alerts. A grant or +removal is high impact even when no transfer follows immediately. Alert on +unexpected administrators, unknown caller addresses, version gaps, repeated +stale requests, and a caller that attempts a mutation after removal. These +signals provide early warning without exposing private transfer amounts. + +Before a release, run a dry governance review against a disposable deployment: +apply an add, repeat it, attempt a skipped version, advance time, remove the +caller, and attempt a new transfer. Compare storage queries and event counts at +each step. This rehearsal catches client-side version and cooldown mistakes +before they affect the production registry. + +The review record should state whether the change is a grant or removal, which +service owns the key, and who approved the effective time. Treat missing review +metadata as a deployment blocker rather than filling it with an inferred value. + +This preserves a human-auditable boundary around automated access changes. + +Store the evidence bundle with restricted administrative access, apply the +repository retention policy, and redact private signing material before sharing +the record with support or an external auditor. The contract event is public; +the approval context does not need to be. + +Reviewers should verify the event topic and payload against the ABI after every +upgrade. A changed topic or field order can break monitoring even when the +membership transition itself remains correct. diff --git a/src/error.rs b/src/error.rs index 0a2b378d..c01f9954 100644 --- a/src/error.rs +++ b/src/error.rs @@ -52,4 +52,8 @@ pub enum Error { /// The number of operations in a batch_operations call exceeds /// MAX_BATCH_SIZE. BatchTooLarge = 22, + /// A caller update was not the next registry version. + StaleCallerUpdate = 23, + /// The caller registry version cannot be incremented safely. + CallerUpdateVersionOverflow = 24, } diff --git a/src/events.rs b/src/events.rs index 8adf1935..f3c5cd37 100644 --- a/src/events.rs +++ b/src/events.rs @@ -49,6 +49,13 @@ pub fn caller_removed(env: &Env, caller: &Address) { env.events().publish(topics, caller.clone()); } +/// Publish one auditable versioned caller-registry transition. +pub fn caller_registry_changed(env: &Env, version: u64, admin: &Address, caller: &Address, allowed: bool) { + let topics = (Symbol::new(env, "caller_registry_changed"), version); + env.events() + .publish(topics, (admin.clone(), caller.clone(), allowed)); +} + /// Publish an event recording that the current admin has nominated a new admin. /// /// Emitted by `transfer_admin`. The transfer is not yet complete; the nominee diff --git a/src/lib.rs b/src/lib.rs index 59dcbdc4..31298c59 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,7 +25,7 @@ mod test_utils; use soroban_sdk::{contract, contractimpl, contractmeta, token, Address, Env, Vec}; use crate::error::Error; -use crate::types::{BatchOperation, BatchOperationResult, ConfiguredLimits, Status, Transfer}; +use crate::types::{BatchOperation, BatchOperationResult, CallerUpdateResult, ConfiguredLimits, Status, Transfer}; contractmeta!(key = "name", val = "RemitFlow"); contractmeta!(key = "version", val = "0.1.0"); @@ -456,6 +456,53 @@ impl RemitFlowContract { storage::is_caller_allowed(&env, &caller) } + /// Apply an admin-authorized caller update at the next registry version. + /// + /// Replaying the exact `(version, caller, allowed)` tuple is deterministic + /// and produces no second event. Different stale updates are rejected. + pub fn update_caller_versioned( + env: Env, + caller: Address, + allowed: bool, + version: u64, + ) -> Result { + let admin = storage::get_admin(&env).ok_or(Error::NotInitialized)?; + admin.require_auth(); + require_external_address(&env, &caller)?; + if storage::has_caller_update(&env, version, &caller, allowed) { + return Ok(CallerUpdateResult { + changed: false, + duplicate: true, + version: storage::get_caller_registry_version(&env), + }); + } + require_cooldown(&env)?; + let current = storage::get_caller_registry_version(&env); + let expected = current.checked_add(1).ok_or(Error::CallerUpdateVersionOverflow)?; + if version != expected { + return Err(Error::StaleCallerUpdate); + } + + // The membership bit, version, replay marker, cooldown, and event are + // one state transition. A trapped call rolls all of them back. + storage::set_caller_allowed(&env, &caller, allowed); + storage::set_caller_registry_version(&env, version); + storage::set_caller_update(&env, version, &caller, allowed); + record_privileged_call(&env); + storage::extend_instance(&env); + events::caller_registry_changed(&env, version, &admin, &caller, allowed); + Ok(CallerUpdateResult { + changed: true, + duplicate: false, + version, + }) + } + + /// Return the monotonic version of the allowed-caller registry. + pub fn caller_registry_version(env: Env) -> u64 { + storage::get_caller_registry_version(&env) + } + pub fn transfer_admin(env: Env, new_admin: Address) -> Result<(), Error> { require_cooldown(&env)?; let admin = storage::get_admin(&env).ok_or(Error::NotInitialized)?; diff --git a/src/storage.rs b/src/storage.rs index cea2a072..f0864c3c 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -48,6 +48,8 @@ pub enum InstanceKey { InitializedAt, /// Timestamp of the most recent privileged administrative call. LastPrivilegedCall, + /// Monotonic version of the caller registry. + CallerRegistryVersion, } /// Keys for values held in **persistent** storage. @@ -70,6 +72,8 @@ pub enum PersistentKey { AllowedCaller(Address), /// Per-account operation counter, keyed by account address. AccountOpCount(Address), + /// Replay marker for an exact versioned caller update. + CallerUpdate(u64, Address, bool), } // --------------------------------------------------------------------------- @@ -193,6 +197,21 @@ pub fn set_last_privileged_call(env: &Env, timestamp: u64) { .set(&InstanceKey::LastPrivilegedCall, ×tamp); } +/// Read the current caller registry version, defaulting to zero. +pub fn get_caller_registry_version(env: &Env) -> u64 { + env.storage() + .instance() + .get(&InstanceKey::CallerRegistryVersion) + .unwrap_or(0) +} + +/// Persist the current caller registry version. +pub fn set_caller_registry_version(env: &Env, version: u64) { + env.storage() + .instance() + .set(&InstanceKey::CallerRegistryVersion, &version); +} + // --------------------------------------------------------------------------- // Persistent storage helpers // --------------------------------------------------------------------------- @@ -251,3 +270,20 @@ pub fn is_caller_allowed(env: &Env, caller: &Address) -> bool { let key = PersistentKey::AllowedCaller(caller.clone()); env.storage().persistent().get(&key).unwrap_or(false) } + +/// Check whether an exact versioned caller update has already been applied. +pub fn has_caller_update(env: &Env, version: u64, caller: &Address, allowed: bool) -> bool { + env.storage() + .persistent() + .get(&PersistentKey::CallerUpdate(version, caller.clone(), allowed)) + .unwrap_or(false) +} + +/// Mark an exact versioned caller update as applied. +pub fn set_caller_update(env: &Env, version: u64, caller: &Address, allowed: bool) { + let key = PersistentKey::CallerUpdate(version, caller.clone(), allowed); + env.storage().persistent().set(&key, &true); + env.storage() + .persistent() + .extend_ttl(&key, PERSISTENT_BUMP_THRESHOLD, PERSISTENT_BUMP_AMOUNT); +} diff --git a/src/test.rs b/src/test.rs index 590323d1..87bb3f5e 100644 --- a/src/test.rs +++ b/src/test.rs @@ -824,6 +824,117 @@ fn test_add_caller_requires_admin_auth() { assert!(res.is_err()); } +#[test] +fn test_versioned_caller_update_starts_at_one_and_is_auditable() { + let s = setup(); + let caller = Address::generate(&s.env); + + assert_eq!(s.client.caller_registry_version(), 0); + let result = s.client.update_caller_versioned(&caller, &true, &1); + assert_eq!(result.changed, true); + assert_eq!(result.duplicate, false); + assert_eq!(result.version, 1); + assert_eq!(s.client.caller_registry_version(), 1); + assert!(s.client.is_caller_allowed(&caller)); + + let matching_events = s + .env + .events() + .all() + .iter() + .filter(|event| { + let topics: soroban_sdk::Vec = event.1.clone().into_val(&s.env); + if topics.len() == 0 { + return false; + } + let topic: soroban_sdk::Symbol = topics.get(0).unwrap().into_val(&s.env); + topic == soroban_sdk::Symbol::new(&s.env, "caller_registry_changed") + }) + .count(); + assert_eq!(matching_events, 1); +} + +#[test] +fn test_exact_versioned_caller_retry_is_deterministic_without_second_event() { + let s = setup(); + let caller = Address::generate(&s.env); + let first = s.client.update_caller_versioned(&caller, &true, &1); + let retry = s.client.update_caller_versioned(&caller, &true, &1); + + assert_eq!(first.changed, true); + assert_eq!(retry.changed, false); + assert_eq!(retry.duplicate, true); + assert_eq!(retry.version, 1); + assert_eq!(s.client.caller_registry_version(), 1); + assert!(s.client.is_caller_allowed(&caller)); +} + +#[test] +fn test_versioned_remove_is_one_transition_and_blocks_new_mutations() { + let s = setup(); + let caller = Address::generate(&s.env); + s.client.update_caller_versioned(&caller, &true, &1); + s.env.ledger().set_timestamp(s.env.ledger().timestamp() + crate::PRIVILEGED_COOLDOWN); + let removed = s.client.update_caller_versioned(&caller, &false, &2); + assert_eq!(removed.changed, true); + assert_eq!(removed.version, 2); + assert!(!s.client.is_caller_allowed(&caller)); + + let expiry = s.future_expiry(); + let result = s.client.try_create_transfer(&caller, &s.recipient, &100, &expiry); + assert_eq!(result, Err(Ok(crate::error::Error::CallerNotAllowed))); +} + +#[test] +fn test_stale_version_does_not_change_registry_or_membership() { + let s = setup(); + let caller = Address::generate(&s.env); + s.client.update_caller_versioned(&caller, &true, &1); + s.env.ledger().set_timestamp(s.env.ledger().timestamp() + crate::PRIVILEGED_COOLDOWN); + + let stale = s.client.try_update_caller_versioned(&caller, &false, &3); + assert_eq!(stale, Err(Ok(crate::error::Error::StaleCallerUpdate))); + assert_eq!(s.client.caller_registry_version(), 1); + assert!(s.client.is_caller_allowed(&caller)); +} + +#[test] +fn test_cooldown_failure_does_not_consume_next_version() { + let s = setup(); + let caller = Address::generate(&s.env); + s.env.ledger().set_timestamp(1_000); + s.client.update_caller_versioned(&caller, &true, &1); + + let blocked = s.client.try_update_caller_versioned(&caller, &false, &2); + assert_eq!(blocked, Err(Ok(crate::error::Error::CooldownNotElapsed))); + assert_eq!(s.client.caller_registry_version(), 1); + assert!(s.client.is_caller_allowed(&caller)); + + s.env.ledger().set_timestamp(s.env.ledger().timestamp() + crate::PRIVILEGED_COOLDOWN); + let accepted = s.client.update_caller_versioned(&caller, &false, &2); + assert_eq!(accepted.changed, true); + assert!(!s.client.is_caller_allowed(&caller)); +} + +#[test] +fn test_distinct_callers_share_ordered_registry_versions() { + let s = setup(); + let first = Address::generate(&s.env); + let second = Address::generate(&s.env); + let third = Address::generate(&s.env); + let first_result = s.client.update_caller_versioned(&first, &true, &1); + assert_eq!(first_result.version, 1); + s.env.ledger().set_timestamp(s.env.ledger().timestamp() + crate::PRIVILEGED_COOLDOWN); + let second_result = s.client.update_caller_versioned(&second, &true, &2); + assert_eq!(second_result.version, 2); + s.env.ledger().set_timestamp(s.env.ledger().timestamp() + crate::PRIVILEGED_COOLDOWN); + let third_result = s.client.update_caller_versioned(&third, &true, &3); + assert_eq!(third_result.version, 3); + assert!(s.client.is_caller_allowed(&first)); + assert!(s.client.is_caller_allowed(&second)); + assert!(s.client.is_caller_allowed(&third)); +} + #[test] fn test_pause_requires_admin_auth() { let s = setup(); diff --git a/src/types.rs b/src/types.rs index 7fc9cff6..f3c19ed7 100644 --- a/src/types.rs +++ b/src/types.rs @@ -87,3 +87,15 @@ pub struct ConfiguredLimits { /// Maximum number of records returned by a paginated transfer query. pub max_page_size: u32, } + +/// Deterministic result for a versioned allowed-caller update. +#[contracttype] +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct CallerUpdateResult { + /// True when this invocation changed the registry. + pub changed: bool, + /// True when the exact versioned update was already applied. + pub duplicate: bool, + /// Registry version after evaluating the update. + pub version: u64, +}