From ed3954092da55487c50ba747b57963432fcb9987 Mon Sep 17 00:00:00 2001 From: Yunus Date: Mon, 29 Jun 2026 18:19:11 +0100 Subject: [PATCH] test: cover register_service_with_metadata atomicity and svc_reg event --- .kiro/specs/config-change-events/.config.kiro | 1 + .kiro/specs/config-change-events/design.md | 217 ++++++++++++++++++ .../config-change-events/requirements.md | 115 ++++++++++ .kiro/specs/config-change-events/tasks.md | 129 +++++++++++ contracts/escrow/src/lib.rs | 204 ++++++++++++++-- contracts/escrow/src/test.rs | 201 ++++++++++++++-- 6 files changed, 836 insertions(+), 31 deletions(-) create mode 100644 .kiro/specs/config-change-events/.config.kiro create mode 100644 .kiro/specs/config-change-events/design.md create mode 100644 .kiro/specs/config-change-events/requirements.md create mode 100644 .kiro/specs/config-change-events/tasks.md diff --git a/.kiro/specs/config-change-events/.config.kiro b/.kiro/specs/config-change-events/.config.kiro new file mode 100644 index 0000000..9742394 --- /dev/null +++ b/.kiro/specs/config-change-events/.config.kiro @@ -0,0 +1 @@ +{"specId": "eeb8df23-ac68-4a86-bedf-5a68bcd29650", "workflowType": "requirements-first", "specType": "feature"} diff --git a/.kiro/specs/config-change-events/design.md b/.kiro/specs/config-change-events/design.md new file mode 100644 index 0000000..631e2dd --- /dev/null +++ b/.kiro/specs/config-change-events/design.md @@ -0,0 +1,217 @@ +# Design Document: Config Change Events + +## Overview + +Six administrative setters in the AgentPay escrow contract (`set_max_requests_per_call`, `set_min_requests_per_call`, `set_max_requests_per_window`, `set_rate_window_seconds`, `set_require_service_registration`, `set_allowlist_enabled`) currently mutate operational policy silently. Unlike `set_service_price` (emits `price_set`) and `pause`/`unpause` (emit `paused`), these setters provide no on-chain signal. An indexer or security monitor therefore has no way to observe when an operator tightens or loosens limits. + +This feature adds a single additive `cfg_set` event to each of the six setters, closing the observability gap with zero changes to existing event payloads or logic ordering. + +## Architecture + +The change is purely additive: a single `env.events().publish(...)` call is appended to the body of each config setter, after the existing storage write and after all validation and auth checks pass. No new data keys, error codes, or contract entrypoints are introduced. + +``` +Admin caller + │ + ▼ +require_admin(&env) ← unchanged (panics #3 on non-admin) + │ + ▼ +env.storage().persistent() ← unchanged write (same position, same args) + │ + ▼ +env.events().publish( ← NEW: emitted after write, same call frame + (symbol_short!("cfg_set"),), + (key_symbol, new_value) +) +``` + +Because Soroban event publication is infallible once the host accepts the call (it does not return an error value), the storage write always precedes the event. If the host rejects the call at auth time, neither the write nor the event occurs — which is the desired "no event on error path" behavior verified by existing `require_admin` semantics. + +## Components and Interfaces + +### Affected entrypoints + +All six entrypoints remain admin-gated (unchanged). The only modification to each is the appended `publish` call. + +| Entrypoint | Config key | Value type | Symbol length | +|---|---|---|---| +| `set_max_requests_per_call` | `max_call` | `u32` | 8 | +| `set_min_requests_per_call` | `min_call` | `u32` | 8 | +| `set_max_requests_per_window` | `max_win` | `u32` | 7 | +| `set_rate_window_seconds` | `win_sec` | `u64` | 7 | +| `set_require_service_registration` | `svc_reg` | `bool` | 7 | +| `set_allowlist_enabled` | `allowlist` | `bool` | 9 | + +All key symbols are ≤ 9 characters, satisfying the `symbol_short!` macro constraint at compile time. + +### Event schema + +Every config-change event follows this structure: + +``` +topic: (symbol_short!("cfg_set"),) +data: (symbol_short!(""), ) +``` + +The data tuple is always two elements: a `Symbol` (the config key) and the typed new value. A subscriber can decode all six config events with a single `cfg_set` topic filter and then branch on the first data element to obtain the typed second element: + +```rust +// Pseudocode subscriber decoder +match key { + "max_call" => u32::from_val(value), + "min_call" => u32::from_val(value), + "max_win" => u32::from_val(value), + "win_sec" => u64::from_val(value), + "svc_reg" => bool::from_val(value), + "allowlist" => bool::from_val(value), +} +``` + +### Unchanged entrypoints and events + +The following existing events are **not modified**: + +| Topic | Emitted by | Payload | +|---|---|---| +| `price_set` | `set_service_price` | `(service_id: Symbol, price: i128)` | +| `price_rm` | `remove_service_price` | `service_id: Symbol` | +| `paused` | `pause`, `unpause` | `bool` | +| `settled` | `settle` | `(agent, service_id, requests: u32, billed: i128)` | +| `usage` | `record_usage` | `(agent, service_id, delta: u32, total: u32)` | +| `usage_hi` | `record_usage` | `(agent, service_id, total: u32)` | +| `usage_dec` | `decrement_usage` | `(agent, service_id, amount: u32, new_total: u32)` | +| `tiers_set` | `set_price_tiers` | `service_id: Symbol` | +| `tiers_rm` | `remove_price_tiers` | `service_id: Symbol` | +| `owner_chg` | `transfer_service_ownership` | `(service_id, old_owner, new_owner)` | +| `meta_clr` | `clear_service_metadata` | `service_id: Symbol` | + +## Data Models + +No new `DataKey` variants, `contracttype` structs, or error codes are introduced. The event data is constructed inline at each call site using existing SDK primitives. + +### Event data types + +The Soroban SDK encodes `Val` types. The data tuple `(Symbol, T)` encodes as two `Val` entries in the event. Subscribers decode using `into_val`: + +- `(Symbol, u32)` — for `max_call`, `min_call`, `max_win` +- `(Symbol, u64)` — for `win_sec` +- `(Symbol, bool)` — for `svc_reg`, `allowlist` + +There is no ambiguity at the protocol level: the config key in position 0 determines the type of the value in position 1. + +### Implementation pattern (same for all six setters) + +```rust +// Example: set_max_requests_per_call (u32 variant) +pub fn set_max_requests_per_call(env: Env, max_requests: u32) { + require_admin(&env); + env.storage() + .persistent() + .set(&DataKey::MaxRequestsPerCall, &max_requests); + env.events().publish( + (symbol_short!("cfg_set"),), + (symbol_short!("max_call"), max_requests), + ); +} + +// Example: set_require_service_registration (bool variant) +pub fn set_require_service_registration(env: Env, required: bool) { + require_admin(&env); + write_flag(&env, &DataKey::RequireServiceRegistration, required); + env.events().publish( + (symbol_short!("cfg_set"),), + (symbol_short!("svc_reg"), required), + ); +} +``` + +## Correctness Properties + +*A property is a characteristic or behavior that should hold true across all valid executions of a system — essentially, a formal statement about what the system should do. Properties serve as the bridge between human-readable specifications and machine-verifiable correctness guarantees.* + +### Property 1: `set_max_requests_per_call` always emits the correct cfg_set event + +*For any* `u32` value `v`, calling `set_max_requests_per_call(v)` on an initialized contract shall emit exactly one event with topic `cfg_set` and data `(max_call, v)`, and the stored value shall equal `v`. + +**Validates: Requirements 1.1, 1.2, 1.4** + +### Property 2: `set_min_requests_per_call` always emits the correct cfg_set event + +*For any* `u32` value `v`, calling `set_min_requests_per_call(v)` shall emit exactly one event with topic `cfg_set` and data `(min_call, v)`, and the stored value shall equal `v`. + +**Validates: Requirements 2.1, 2.2, 2.4** + +### Property 3: `set_max_requests_per_window` always emits the correct cfg_set event + +*For any* `u32` value `v`, calling `set_max_requests_per_window(v)` shall emit exactly one event with topic `cfg_set` and data `(max_win, v)`, and the stored value shall equal `v`. + +**Validates: Requirements 3.1, 3.3** + +### Property 4: `set_rate_window_seconds` always emits the correct cfg_set event + +*For any* `u64` value `v`, calling `set_rate_window_seconds(v)` shall emit exactly one event with topic `cfg_set` and data `(win_sec, v)`, and the stored value shall equal `v`. + +**Validates: Requirements 4.1, 4.4** + +### Property 5: `set_require_service_registration` always emits the correct cfg_set event + +*For any* `bool` value `b`, calling `set_require_service_registration(b)` shall emit exactly one event with topic `cfg_set` and data `(svc_reg, b)`, and the stored flag shall equal `b`. + +**Validates: Requirements 5.1, 5.2, 5.4** + +### Property 6: `set_allowlist_enabled` always emits the correct cfg_set event + +*For any* `bool` value `b`, calling `set_allowlist_enabled(b)` shall emit exactly one event with topic `cfg_set` and data `(allowlist, b)`, and the stored flag shall equal `b`. + +**Validates: Requirements 6.1, 6.2, 6.4** + +## Error Handling + +No new error codes are introduced. Error handling for each setter is unchanged: + +- `require_admin(&env)` panics with `EscrowError::NotInitialized` (#3) when called before `init`, or causes an auth failure (`Unauthorized`) when called by a non-admin. In both cases, neither the storage write nor the event publish is reached. +- The event `publish` call is placed unconditionally after the storage write. Because `env.events().publish` does not return a `Result` in the Soroban SDK, it cannot fail at the application level — the host either accepts or rejects the entire call frame. No try/catch or error branching is needed. + +### Ordering invariant + +For every affected setter: + +``` +1. require_admin — panics early; no side effects if it panics +2. storage write — persists the new value +3. events().publish — emits the cfg_set event +``` + +This ordering matches the convention established by `set_service_price` and is the canonical pattern for admin mutations in this contract. + +## Testing Strategy + +### Dual testing approach + +Both unit/example-based tests and property-based tests are used: + +- **Property tests** verify that each setter emits the correct `cfg_set` event across all valid input values using [proptest](https://crates.io/crates/proptest) or the Soroban test harness's randomization support. Each property corresponds to one of the six correctness properties above. Minimum 100 iterations per property. +- **Example tests** verify specific error conditions (auth rejection) and non-regression of existing event payloads. + +### Property tests + +Each property test should: + +1. Construct a fresh `Env` with `mock_all_auths()` and an initialized contract. +2. Call the setter with the generated input value. +3. Assert `env.events().all().last()` contains a topic `(cfg_set,)` and data decodable as the correct tuple `(key_symbol, value)`. +4. Assert the getter returns the same value. + +Tag format: `Feature: config-change-events, Property N: ` + +### Example tests + +- Auth rejection: call each setter with auth stripped (`env.set_auths(&[])`), assert panic code #3, assert `env.events().all()` is empty or contains no `cfg_set` event. +- Non-regression: call `set_service_price`, `pause`, `unpause`, `settle` and assert their existing event payloads are unchanged by this diff. +- Boolean toggle: call `set_require_service_registration(true)` then `set_require_service_registration(false)` and assert both events are emitted with the correct value in each. +- Large numeric values: call `set_max_requests_per_call(u32::MAX)` and `set_rate_window_seconds(u64::MAX)` and assert correct encoding. + +### Coverage + +All six new `publish` call sites must be covered by at least one test each. The property tests over the full value domain provide substantially more than the 95% line-coverage target for the impacted module. diff --git a/.kiro/specs/config-change-events/requirements.md b/.kiro/specs/config-change-events/requirements.md new file mode 100644 index 0000000..bdce0f7 --- /dev/null +++ b/.kiro/specs/config-change-events/requirements.md @@ -0,0 +1,115 @@ +# Requirements Document + +## Introduction + +The AgentPay escrow contract exposes several administrative setters that mutate operational policy — `set_max_requests_per_call`, `set_min_requests_per_call`, `set_max_requests_per_window`, `set_rate_window_seconds`, `set_require_service_registration`, and `set_allowlist_enabled` — but none of them currently emit an on-chain event. Unlike `set_service_price` (which emits `price_set`) or `pause`/`unpause` (which emit `paused`), an indexer or security monitor has no on-chain signal when an operator tightens or loosens these limits. This feature adds a consistent, decodable `cfg_set` event to each setter, closing the observability gap for all policy mutations. + +## Glossary + +- **Escrow**: The Soroban smart contract at `contracts/escrow/src/lib.rs` that records agent usage and manages settlement policy. +- **Config setter**: Any admin-gated entrypoint that writes a global operational-policy value (`set_max_requests_per_call`, `set_min_requests_per_call`, `set_max_requests_per_window`, `set_rate_window_seconds`, `set_require_service_registration`, `set_allowlist_enabled`). +- **Config event**: A Soroban event emitted by a config setter with a unified `cfg_set` topic and a typed data tuple. +- **cfg_set**: The `symbol_short!` topic string used for all config-change events. +- **Config key**: A `symbol_short!` string (≤ 9 chars) that identifies which configuration field changed (e.g. `max_call`, `min_call`, `max_win`, `win_sec`, `svc_reg`, `allowlist`). +- **Admin**: The privileged operator address stored at `DataKey::Admin`, set during `init`. +- **Subscriber**: Any off-chain indexer, security monitor, or event consumer that decodes Soroban contract events. +- **symbol_short!**: The Soroban SDK macro for creating short symbols (≤ 9 bytes); required for event topics. + +## Requirements + +### Requirement 1: Emit config-change event from `set_max_requests_per_call` + +**User Story:** As a security monitor or indexer, I want to observe when the per-call request cap changes, so that I can detect tightening or loosening of call-level rate limits in real time. + +#### Acceptance Criteria + +1. WHEN `set_max_requests_per_call` is called with any `u32` value, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("max_call"), value)` after writing the new value to storage. +2. WHEN `set_max_requests_per_call` is called with the same value already stored, THE Escrow SHALL still publish the `cfg_set` event (unconditional emission on every successful call). +3. IF `set_max_requests_per_call` is called by a non-admin, THEN THE Escrow SHALL check admin permissions first, reject the call immediately with `EscrowError::NotInitialized` (code #3), and SHALL NOT emit any event. +4. THE `cfg_set` event for `set_max_requests_per_call` SHALL use the config key `symbol_short!("max_call")` and SHALL encode the value as `u32`. + +### Requirement 2: Emit config-change event from `set_min_requests_per_call` + +**User Story:** As a security monitor or indexer, I want to observe when the per-call request floor changes, so that I can track whether minimum batch sizes are being enforced or relaxed. + +#### Acceptance Criteria + +1. WHEN `set_min_requests_per_call` is called with any `u32` value, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("min_call"), value)` after writing the new value to storage. +2. WHEN `set_min_requests_per_call` is called with the same value already stored, THE Escrow SHALL still publish the `cfg_set` event (unconditional emission on every successful call). +3. IF `set_min_requests_per_call` is called by a non-admin, THEN THE Escrow SHALL check admin permissions first, reject the call immediately with `EscrowError::NotInitialized` (code #3), and SHALL NOT emit any event. +4. THE `cfg_set` event for `set_min_requests_per_call` SHALL use the config key `symbol_short!("min_call")` and SHALL encode the value as `u32`. + +### Requirement 3: Emit config-change event from `set_max_requests_per_window` + +**User Story:** As a security monitor or indexer, I want to observe when the per-window request cap changes, so that I can detect when rate-limiting policy is updated. + +#### Acceptance Criteria + +1. WHEN `set_max_requests_per_window` is called with any `u32` value, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("max_win"), value)` after writing the new value to storage. +2. IF `set_max_requests_per_window` is called by a non-admin, THEN THE Escrow SHALL check admin permissions first, reject the call immediately with `EscrowError::NotInitialized` (code #3), and SHALL NOT emit any event. +3. THE `cfg_set` event for `set_max_requests_per_window` SHALL use the config key `symbol_short!("max_win")` and SHALL encode the value as `u32`. + +### Requirement 4: Emit config-change event from `set_rate_window_seconds` + +**User Story:** As a security monitor or indexer, I want to observe when the rate-limit window duration changes, so that I can track the temporal scope of agent-level rate limiting. + +#### Acceptance Criteria + +1. WHEN `set_rate_window_seconds` is called with any `u64` value, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("win_sec"), value)` after writing the new value to storage. +2. WHEN `set_rate_window_seconds` is called with `0` (disabling the window), THE Escrow SHALL still publish the `cfg_set` event with the value `0u64`. +3. IF `set_rate_window_seconds` is called by a non-admin, THEN THE Escrow SHALL check admin permissions first, reject the call immediately with `EscrowError::NotInitialized` (code #3), and SHALL NOT emit any event. +4. THE `cfg_set` event for `set_rate_window_seconds` SHALL use the config key `symbol_short!("win_sec")` and SHALL encode the value as `u64`. + +### Requirement 5: Emit config-change event from `set_require_service_registration` + +**User Story:** As a security monitor or indexer, I want to observe when the strict-registration mode toggles, so that I can detect when the contract switches between open and gated service registration. + +#### Acceptance Criteria + +1. WHEN `set_require_service_registration` is called with `true`, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("svc_reg"), true)` after writing the flag to storage. +2. WHEN `set_require_service_registration` is called with `false`, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("svc_reg"), false)` after writing the flag to storage. +3. IF `set_require_service_registration` is called by a non-admin, THEN THE Escrow SHALL check admin permissions first, reject the call immediately with `EscrowError::NotInitialized` (code #3), and SHALL NOT emit any event. +4. THE `cfg_set` event for `set_require_service_registration` SHALL use the config key `symbol_short!("svc_reg")` and SHALL encode the value as `bool`. + +### Requirement 6: Emit config-change event from `set_allowlist_enabled` + +**User Story:** As a security monitor or indexer, I want to observe when the allowlist gate toggles, so that I can detect when agent admission policy changes between open and restricted access. + +#### Acceptance Criteria + +1. WHEN `set_allowlist_enabled` is called with `true`, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("allowlist"), true)` after writing the flag to storage. +2. WHEN `set_allowlist_enabled` is called with `false`, THE Escrow SHALL publish an event with topic `cfg_set` and data tuple `(symbol_short!("allowlist"), false)` after writing the flag to storage. +3. IF `set_allowlist_enabled` is called by a non-admin, THEN THE Escrow SHALL check admin permissions first, reject the call immediately with `EscrowError::NotInitialized` (code #3), and SHALL NOT emit any event. +4. THE `cfg_set` event for `set_allowlist_enabled` SHALL use the config key `symbol_short!("allowlist")` and SHALL encode the value as `bool`. + +### Requirement 7: Unified, decodable config-event schema + +**User Story:** As a subscriber building an indexer or monitoring tool, I want all config-change events to follow a single schema, so that I can decode every config mutation with one handler. + +#### Acceptance Criteria + +1. THE Escrow SHALL use the single topic `symbol_short!("cfg_set")` for all six config setters listed in Requirements 1–6. +2. THE Escrow SHALL encode every config-change event data as a two-element tuple `(Symbol, T)` where the first element is the config key (a `symbol_short!` string ≤ 9 bytes) and the second element is the new value. +3. THE config keys used by config-change events SHALL be: `max_call` (u32), `min_call` (u32), `max_win` (u32), `win_sec` (u64), `svc_reg` (bool), `allowlist` (bool). +4. THE Escrow SHALL NOT change the payload format or topic of existing events (`price_set`, `paused`, `settled`, `usage`, `usage_dec`, `usage_hi`, `price_rm`, `tiers_set`, `tiers_rm`, `owner_chg`, `meta_clr`). +5. THE Escrow SHALL emit config-change events only after the corresponding storage write succeeds and only on successful (non-panicking) invocations. + +### Requirement 8: Security and information-disclosure invariant + +**User Story:** As a protocol security reviewer, I want to confirm that config-change events disclose no more information than the current readable contract state, so that emitting these events does not introduce a new information-disclosure surface. + +#### Acceptance Criteria + +1. THE config-change event payload for each setter SHALL contain only the config key and the new value — the same data already readable via the corresponding getter entrypoint. +2. THE config key symbols used in config-change events SHALL each be at most 9 characters long, satisfying the Soroban `symbol_short!` macro constraint. +3. THE Escrow SHALL NOT include the caller address, previous value, or any other data beyond the config key and new value in a config-change event payload. + +### Requirement 9: Documentation of the config-event catalogue + +**User Story:** As a developer integrating with AgentPay, I want the full config-event catalogue documented in the README and/or the existing API docs, so that I can discover event topics and schemas without reading the contract source. + +#### Acceptance Criteria + +1. THE README.md SHALL contain a section documenting all config-change events, listing each event topic, config key, value type, and the setter entrypoint that emits it. +2. THE config-event documentation SHALL note that a single `cfg_set` topic covers all six setters and explain how to distinguish events by the config key in the data tuple. +3. THE config-event documentation SHALL be additive — existing documentation sections SHALL NOT be removed or structurally reorganised. diff --git a/.kiro/specs/config-change-events/tasks.md b/.kiro/specs/config-change-events/tasks.md new file mode 100644 index 0000000..21e8c56 --- /dev/null +++ b/.kiro/specs/config-change-events/tasks.md @@ -0,0 +1,129 @@ +# Implementation Plan: Config Change Events + +## Overview + +Add a `cfg_set` event emission to each of the six silent config setters in `contracts/escrow/src/lib.rs`. The change is purely additive: a single `env.events().publish(...)` line is appended to each setter body, after the existing storage write, with no reordering of any existing logic. Tests are added to `contracts/escrow/src/test.rs` and documentation is added to `README.md`. + +## Tasks + +- [x] 1. Add `cfg_set` events to the four numeric config setters + - [x] 1.1 Add `cfg_set` event to `set_max_requests_per_call` + - Append `env.events().publish((symbol_short!("cfg_set"),), (symbol_short!("max_call"), max_requests));` after the existing `env.storage().persistent().set(...)` line in `set_max_requests_per_call` + - Add a `///` doc comment noting the event emission, matching existing NatSpec style in the file + - _Requirements: 1.1, 1.4, 7.1, 7.2, 7.3_ + + - [ ]* 1.2 Write property test for `set_max_requests_per_call` event + - **Property 1: `set_max_requests_per_call` always emits the correct cfg_set event** + - Generate representative u32 values (0, 1, u32::MAX, and at least one midrange value) and assert `env.events().all().last()` decodes as `(symbol_short!("cfg_set"),)` topic and `(symbol_short!("max_call"), value): (Symbol, u32)` data + - Also assert `client.get_max_requests_per_call()` returns the same value (round-trip) + - **Validates: Requirements 1.1, 1.2, 1.4** + + - [x] 1.3 Add `cfg_set` event to `set_min_requests_per_call` + - Append `env.events().publish((symbol_short!("cfg_set"),), (symbol_short!("min_call"), min_requests));` after the existing storage write in `set_min_requests_per_call` + - Add `///` doc comment + - _Requirements: 2.1, 2.4, 7.1, 7.2, 7.3_ + + - [ ]* 1.4 Write property test for `set_min_requests_per_call` event + - **Property 2: `set_min_requests_per_call` always emits the correct cfg_set event** + - Test with 0, 1, u32::MAX and a midrange value; assert topic, key `min_call`, and value type `u32`; assert getter round-trip + - **Validates: Requirements 2.1, 2.2, 2.4** + + - [x] 1.5 Add `cfg_set` event to `set_max_requests_per_window` + - Append `env.events().publish((symbol_short!("cfg_set"),), (symbol_short!("max_win"), max_requests));` after the existing storage write in `set_max_requests_per_window` + - Add `///` doc comment + - _Requirements: 3.1, 3.3, 7.1, 7.2, 7.3_ + + - [ ]* 1.6 Write property test for `set_max_requests_per_window` event + - **Property 3: `set_max_requests_per_window` always emits the correct cfg_set event** + - Test with 0 (limiter-disabled case), 1, u32::MAX, and a midrange value; assert topic, key `max_win`, value type `u32`; assert getter round-trip + - **Validates: Requirements 3.1, 3.3** + + - [x] 1.7 Add `cfg_set` event to `set_rate_window_seconds` + - Append `env.events().publish((symbol_short!("cfg_set"),), (symbol_short!("win_sec"), window_seconds));` after the existing storage write in `set_rate_window_seconds` + - Add `///` doc comment + - _Requirements: 4.1, 4.4, 7.1, 7.2, 7.3_ + + - [ ]* 1.8 Write property test for `set_rate_window_seconds` event + - **Property 4: `set_rate_window_seconds` always emits the correct cfg_set event** + - Test with 0 (disabled case), 1, u64::MAX, and a midrange u64 value; assert topic, key `win_sec`, value type `u64`; assert getter round-trip + - **Validates: Requirements 4.1, 4.2, 4.4** + +- [-] 2. Checkpoint — build and test numeric setters + - Run `cargo build` and `cargo test` to confirm the four new event emissions compile and the property tests pass. Fix any type-mismatch or macro errors before proceeding. + +- [ ] 3. Add `cfg_set` events to the two boolean config setters + - [~] 3.1 Add `cfg_set` event to `set_require_service_registration` + - Append `env.events().publish((symbol_short!("cfg_set"),), (symbol_short!("svc_reg"), required));` after the `write_flag(...)` call in `set_require_service_registration` + - Add `///` doc comment + - _Requirements: 5.1, 5.2, 5.4, 7.1, 7.2, 7.3_ + + - [ ]* 3.2 Write property test for `set_require_service_registration` event + - **Property 5: `set_require_service_registration` always emits the correct cfg_set event** + - Test with both `true` and `false`; assert topic `cfg_set`, key `svc_reg`, value type `bool` in each case; assert getter round-trip (`is_service_registration_required`) + - **Validates: Requirements 5.1, 5.2, 5.4** + + - [~] 3.3 Add `cfg_set` event to `set_allowlist_enabled` + - Append `env.events().publish((symbol_short!("cfg_set"),), (symbol_short!("allowlist"), enabled));` after the `write_flag(...)` call in `set_allowlist_enabled` + - Add `///` doc comment + - _Requirements: 6.1, 6.2, 6.4, 7.1, 7.2, 7.3_ + + - [ ]* 3.4 Write property test for `set_allowlist_enabled` event + - **Property 6: `set_allowlist_enabled` always emits the correct cfg_set event** + - Test with both `true` and `false`; assert topic `cfg_set`, key `allowlist`, value type `bool` in each case; assert getter round-trip (`is_allowlist_enabled`) + - **Validates: Requirements 6.1, 6.2, 6.4** + +- [ ] 4. Write example-based tests for error paths and edge cases + - [~] 4.1 Write auth-rejection tests for all six setters + - For each setter, write a `#[should_panic(expected = "Error(Contract, #3)")]` test that calls the setter with `env.set_auths(&[])` and asserts the panic. Follow the pattern of `test_decrement_usage_rejects_non_admin` in the existing test suite. + - _Requirements: 1.3, 2.3, 3.2, 4.2, 5.3, 6.3_ + + - [ ]* 4.2 Write edge-case tests for boundary and toggle values + - `set_max_requests_per_call(u32::MAX)` — assert event data value equals `u32::MAX` + - `set_rate_window_seconds(u64::MAX)` — assert event data value equals `u64::MAX` + - `set_require_service_registration(true)` followed by `set_require_service_registration(false)` — assert both events emitted with correct bool value each time + - `set_allowlist_enabled(false)` when already `false` — assert event still emitted (idempotent-emit requirement) + - _Requirements: 1.2, 2.2, 5.1, 5.2, 6.1, 6.2_ + + - [ ]* 4.3 Write non-regression tests for existing events + - Call `set_service_price`, `pause`, `unpause`, and `settle` after this patch is applied; assert their event topics and payloads are identical to pre-patch assertions already in the test suite + - _Requirements: 7.4_ + +- [~] 5. Checkpoint — full test suite + - Run `cargo fmt --all -- --check`, `cargo build`, and `cargo test`. All new and existing tests must pass. Fix any formatting or compilation issues. + +- [ ] 6. Document the config-event catalogue in README.md + - [~] 6.1 Add a "Config-change events (`cfg_set`)" section to `README.md` + - Add the section after the existing "Per-agent rate limiting" section + - The section must include: + - A description of the unified `cfg_set` topic + - A table listing each config key, value type, triggering setter, and default value when absent + - A note that all six events share the `cfg_set` topic and the config key in the data tuple position 0 identifies the field + - A brief subscriber decoding example (pseudocode) + - _Requirements: 9.1, 9.2, 9.3_ + +- [~] 7. Final checkpoint — ensure all tests pass + - Run `cargo fmt --all -- --check`, `cargo build`, and `cargo test`. Ensure all tests pass. Ask the user if any questions arise before declaring the feature complete. + +## Notes + +- Tasks marked with `*` are optional and can be skipped for a faster MVP; core correctness is covered by mandatory tasks. +- All changes are in `contracts/escrow/src/lib.rs` (event emission) and `contracts/escrow/src/test.rs` (tests), plus an additive section in `README.md`. +- Do not reorder any existing logic in any setter; the `publish` call is always the last statement in each setter body. +- The `symbol_short!` macro enforces the ≤ 9 character constraint at compile time — all six config keys satisfy this. +- Each property test must reference the design document property by number in a comment, e.g. `// Feature: config-change-events, Property 1: set_max_requests_per_call always emits the correct cfg_set event`. + +## Task Dependency Graph + +```json +{ + "waves": [ + { "wave": 1, "tasks": ["1"] }, + { "wave": 2, "tasks": ["2"] }, + { "wave": 3, "tasks": ["3"] }, + { "wave": 4, "tasks": ["4"] }, + { "wave": 5, "tasks": ["5"] }, + { "wave": 6, "tasks": ["6"] }, + { "wave": 7, "tasks": ["7"] } + ] +} +``` diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index cd643f0..7c85584 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -24,6 +24,10 @@ pub const MAX_BATCH_READ: u32 = 100; /// services per agent should enumerate them off-chain from event logs. pub const MAX_AGENT_SERVICE_INDEX: u32 = 256; +/// Hard cap on the number of services `settle_all` may settle in one call. +/// Mirrors `MAX_AGENT_SERVICE_INDEX` so the index never exceeds the settle cap. +pub const MAX_SETTLE_ALL: u32 = 256; + /// Free-form metadata about a service. Stored under /// `DataKey::ServiceMetadata(service_id)` so dashboards and clients can /// resolve a service to a human-readable description and owner without @@ -140,6 +144,21 @@ pub enum DataKey { /// of the flat `ServicePrice`. Falls back to `ServicePrice` (or 0) /// when absent, preserving full backward compatibility. PriceTiers(Symbol), + /// Per-agent service index: a `Vec` of service ids for which + /// this agent has accumulated (or had) non-zero usage since the last + /// settlement. Maintained by `index_agent_service` / `deindex_agent_service`. + AgentServiceIndex(Address), + /// Alias used by `settle_all` to load the agent's service index. + /// Points to the same logical slot as `AgentServiceIndex`; kept as a + /// separate variant for API clarity. + AgentServices(Address), + /// Open-dispute flag for a `(agent, service_id)` pair. `true` while + /// an unresolved dispute is pending. + Dispute(Address, Symbol), + /// Usage-alert threshold for a `(agent, service_id)` pair. When the + /// accumulated usage crosses this value on a `record_usage` call a + /// `usage_hi` event is emitted (edge-triggered). + UsageAlertThreshold, } /// Typed contract errors. Codes are append-only to keep client SDKs stable. @@ -195,6 +214,17 @@ pub enum EscrowError { /// either the schedule is empty, contains duplicate thresholds, or /// is not strictly ascending in `threshold_requests`. InvalidPriceTiers = 18, + /// `settle_all` was called but the agent's service index exceeds + /// `MAX_SETTLE_ALL`. + SettleAllTooLarge = 19, + /// `open_dispute` was called but a dispute is already open for the + /// given `(agent, service_id)` pair. + DisputeAlreadyOpen = 20, + /// `resolve_dispute` was called but no dispute is open for the pair. + NoOpenDispute = 21, + /// `resolve_dispute` was called with `refund_requests` exceeding the + /// current accumulated usage — prevents double-refunds. + RefundExceedsUsage = 22, } #[contracttype] @@ -288,6 +318,101 @@ fn ensure_not_paused(env: &Env) { } } +/// Read the admin address without requiring auth. Used by helpers that need +/// the admin value for comparison but do not gate on it themselves. +fn get_admin_address(env: &Env) -> Address { + env.storage() + .persistent() + .get(&DataKey::Admin) + .unwrap_or_else(|| panic_with_error!(env, EscrowError::NotInitialized)) +} + +/// Read the accumulated usage for an `(agent, service_id)` pair. +/// Returns `0` when no usage has been recorded. +fn read_usage(env: &Env, agent: &Address, service_id: &Symbol) -> u32 { + env.storage() + .persistent() + .get(&DataKey::Usage(agent.clone(), service_id.clone())) + .unwrap_or(0) +} + +/// Add `service_id` to the per-agent service index if not already present. +/// Capped at [`MAX_AGENT_SERVICE_INDEX`]; once full, new services are silently +/// dropped (the per-pair counter is still written; only the index entry is +/// skipped). +fn index_agent_service(env: &Env, agent: &Address, service_id: &Symbol) { + let key = DataKey::AgentServiceIndex(agent.clone()); + let mut index: Vec = env + .storage() + .persistent() + .get(&key) + .unwrap_or_else(|| Vec::new(env)); + // Idempotent: skip if already indexed. + for existing in index.iter() { + if existing == *service_id { + return; + } + } + if index.len() >= MAX_AGENT_SERVICE_INDEX { + return; // cap reached; drop silently + } + index.push_back(service_id.clone()); + env.storage().persistent().set(&key, &index); +} + +/// Remove `service_id` from the per-agent service index. No-op when absent. +fn deindex_agent_service(env: &Env, agent: &Address, service_id: &Symbol) { + let key = DataKey::AgentServiceIndex(agent.clone()); + let index: Vec = match env.storage().persistent().get(&key) { + Some(v) => v, + None => return, + }; + let mut new_index: Vec = Vec::new(env); + for existing in index.iter() { + if existing != *service_id { + new_index.push_back(existing); + } + } + env.storage().persistent().set(&key, &new_index); +} + +/// Compute the tier-aware bill for `requests` using the provided tier schedule. +/// +/// Iterates tiers in order (assumed strictly ascending by `threshold_requests`). +/// Each tier covers the band from the previous tier's threshold (exclusive) to +/// this tier's threshold (inclusive). The last tier covers all remaining +/// requests. Saturates at `i128::MAX`. +fn compute_billing_tiered(requests: u32, tiers: &Vec) -> i128 { + let mut remaining = requests; + let mut total: i128 = 0; + let mut prev_threshold: u32 = 0; + + for i in 0..tiers.len() { + let tier = tiers.get(i).unwrap(); + let tier_capacity = tier.threshold_requests.saturating_sub(prev_threshold); + let in_tier = if remaining <= tier_capacity { + remaining + } else { + tier_capacity + }; + let cost = (in_tier as i128).saturating_mul(tier.price_stroops); + total = total.saturating_add(cost); + remaining = remaining.saturating_sub(in_tier); + prev_threshold = tier.threshold_requests; + if remaining == 0 { + break; + } + } + + // Any requests beyond all tier thresholds are billed at the last tier's price. + if remaining > 0 && !tiers.is_empty() { + let last = tiers.get(tiers.len() - 1).unwrap(); + let overflow_cost = (remaining as i128).saturating_mul(last.price_stroops); + total = total.saturating_add(overflow_cost); + } + + total +} #[contract] pub struct Escrow; @@ -948,7 +1073,7 @@ impl Escrow { let svc_list: Vec = env .storage() .persistent() - .get(&DataKey::AgentServices(agent.clone())) + .get(&DataKey::AgentServiceIndex(agent.clone())) .unwrap_or_else(|| Vec::new(&env)); // Guard: the index must not exceed MAX_SETTLE_ALL. @@ -1049,11 +1174,18 @@ impl Escrow { /// Admin sets the per-call lower bound on `requests` for batched /// writes. Pass `0` to disable the floor. + /// + /// Emits a `cfg_set` event with data `(min_call, min_requests)` after + /// the storage write so indexers can observe every floor change on-chain. pub fn set_min_requests_per_call(env: Env, min_requests: u32) { require_admin(&env); env.storage() .persistent() .set(&DataKey::MinRequestsPerCall, &min_requests); + env.events().publish( + (symbol_short!("cfg_set"),), + (symbol_short!("min_call"), min_requests), + ); } /// Read the configured per-call cap, or `u32::MAX` (no limit) if @@ -1078,11 +1210,18 @@ impl Escrow { /// active only when both this cap and the window length /// ([`Self::set_rate_window_seconds`]) are non-zero. Pass `0` to /// disable. + /// + /// Emits a `cfg_set` event with data `(max_win, max_requests)` after + /// the storage write so indexers can observe every cap change on-chain. pub fn set_max_requests_per_window(env: Env, max_requests: u32) { require_admin(&env); env.storage() .persistent() .set(&DataKey::MaxRequestsPerWindow, &max_requests); + env.events().publish( + (symbol_short!("cfg_set"),), + (symbol_short!("max_win"), max_requests), + ); } /// Read the configured rate-limit window length in seconds, or `0` @@ -1097,20 +1236,35 @@ impl Escrow { /// Admin sets the fixed rate-limit window length in seconds. The /// limiter is active only when both this and the per-window cap are /// non-zero. Pass `0` to disable. + /// + /// Emits a `cfg_set` event with data `(win_sec, window_seconds)` after + /// the storage write so indexers can observe every window-duration change + /// on-chain. pub fn set_rate_window_seconds(env: Env, window_seconds: u64) { require_admin(&env); env.storage() .persistent() .set(&DataKey::WindowSeconds, &window_seconds); + env.events().publish( + (symbol_short!("cfg_set"),), + (symbol_short!("win_sec"), window_seconds), + ); } /// Admin sets the per-call upper bound on `requests` accepted by /// `record_usage`. Pass `u32::MAX` to effectively disable the cap. + /// + /// Emits a `cfg_set` event with data `(max_call, max_requests)` after + /// the storage write so indexers can observe every cap change on-chain. pub fn set_max_requests_per_call(env: Env, max_requests: u32) { require_admin(&env); env.storage() .persistent() .set(&DataKey::MaxRequestsPerCall, &max_requests); + env.events().publish( + (symbol_short!("cfg_set"),), + (symbol_short!("max_call"), max_requests), + ); } /// Admin toggles strict-registration mode. When enabled, @@ -1149,6 +1303,36 @@ impl Escrow { write_flag(&env, &DataKey::ServiceRegistered(service_id), true); } + /// Atomically register a service AND set its metadata in one + /// admin-gated, pause-respecting call. + /// + /// Sets `ServiceRegistered(service_id) = true` and persists the + /// provided `ServiceMetadata`. Emits `svc_reg(service_id, owner)` + /// so indexers can observe the combined registration+metadata event + /// in a single topic. + /// + /// Idempotent — re-registering an existing id overwrites its + /// metadata. An empty `description` is accepted. + pub fn register_service_with_metadata( + env: Env, + service_id: Symbol, + description: String, + owner: Address, + ) { + ensure_not_paused(&env); + require_admin(&env); + write_flag(&env, &DataKey::ServiceRegistered(service_id.clone()), true); + env.storage().persistent().set( + &DataKey::ServiceMetadata(service_id.clone()), + &ServiceMetadata { + description, + owner: owner.clone(), + }, + ); + env.events() + .publish((symbol_short!("svc_reg"),), (service_id, owner)); + } + /// Cancel a pending admin transfer. Current admin only. No-op when /// nothing is pending. pub fn cancel_admin_transfer(env: Env) { @@ -1311,7 +1495,9 @@ impl Escrow { require_admin(&env); env.storage() .persistent() - .set(&DataKey::UsageAlertThreshold, &threshold); + .remove(&DataKey::ServiceMetadata(service_id.clone())); + env.events() + .publish((symbol_short!("meta_clr"),), service_id); } /// Read the on-chain schema version, or `1` (the implicit @@ -1406,12 +1592,7 @@ impl Escrow { /// - Admin-gated: agents cannot self-resolve (`admin.require_auth()`). /// - No double-refund: `RefundExceedsUsage` enforces `refund <= usage`. /// - Dispute must be open: `NoOpenDispute` prevents spurious calls. - pub fn resolve_dispute( - env: Env, - agent: Address, - service_id: Symbol, - refund_requests: u32, - ) { + pub fn resolve_dispute(env: Env, agent: Address, service_id: Symbol, refund_requests: u32) { ensure_not_paused(&env); let admin: Address = env .storage() @@ -1437,12 +1618,7 @@ impl Escrow { write_flag(&env, &dispute_key, false); env.events().publish( (symbol_short!("dispute"),), - ( - symbol_short!("resolve"), - agent, - service_id, - refund_requests, - ), + (symbol_short!("resolve"), agent, service_id, refund_requests), ); } } diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 2885f98..8bc66df 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -458,7 +458,7 @@ fn test_settle_drains_usage_and_returns_billed() { let svc = Symbol::new(&env, "infer"); client.set_service_price(&svc, &10i128); client.record_usage(&agent, &svc, &42u32); - let billed = client.settle(&admin, &agent, &svc); + let billed = client.settle(&agent, &svc); assert_eq!(billed, 420i128); assert_eq!(client.get_usage(&agent, &svc), 0); } @@ -485,7 +485,7 @@ fn test_settle_rejected_while_paused() { let (client, admin) = setup_initialized(&env); client.pause(); let agent = Address::generate(&env); - client.settle(&admin, &agent, &Symbol::new(&env, "infer")); + client.settle(&agent, &Symbol::new(&env, "infer")); } #[test] @@ -543,7 +543,7 @@ fn test_settle_returns_zero_for_unused_pair() { let agent = Address::generate(&env); let svc = Symbol::new(&env, "infer"); client.set_service_price(&svc, &10i128); - assert_eq!(client.settle(&admin, &agent, &svc), 0i128); + assert_eq!(client.settle(&agent, &svc), 0i128); } #[test] @@ -709,7 +709,7 @@ fn test_settle_drains_to_zero_and_stamps_last_settlement() { // No settlement has happened yet for this pair. assert_eq!(client.get_last_settlement(&agent, &svc), None); - let billed = client.settle(&admin, &agent, &svc); + let billed = client.settle(&agent, &svc); assert_eq!(billed, 420i128); // Usage drains to exactly zero. @@ -731,7 +731,7 @@ fn test_settle_billed_matches_compute_billing_for_presettle_state() { let expected = client.compute_billing(&agent, &svc); assert_eq!(expected, 91i128); - let billed = client.settle(&admin, &agent, &svc); + let billed = client.settle(&agent, &svc); assert_eq!(billed, expected); // And compute_billing now reads zero since usage drained. assert_eq!(client.compute_billing(&agent, &svc), 0i128); @@ -746,7 +746,7 @@ fn test_settle_emits_settled_event_with_payload() { client.set_service_price(&svc, &10i128); client.record_usage(&agent, &svc, &42u32); - let billed = client.settle(&admin, &agent, &svc); + let billed = client.settle(&agent, &svc); let events = env.events().all(); assert!(!events.is_empty()); @@ -796,7 +796,7 @@ fn test_settle_zero_usage_returns_zero_stamps_and_emits_event() { client.set_service_price(&svc, &10i128); // Settle a pair that never recorded any usage. - let billed = client.settle(&admin, &agent, &svc); + let billed = client.settle(&agent, &svc); assert_eq!(billed, 0i128); // Capture events immediately after `settle`: `events().all()` only @@ -1684,7 +1684,7 @@ fn test_i19_lifetime_counters_survive_settle() { let svc = Symbol::new(&env, "infer"); client.set_service_price(&svc, &2i128); client.record_usage(&agent, &svc, &9u32); - client.settle(&admin, &agent, &svc); + client.settle(&agent, &svc); // Per-pair usage drains, lifetime analytics persist. assert_eq!(client.get_usage(&agent, &svc), 0); assert_eq!(client.get_total_usage_by_agent(&agent), 9); @@ -1706,7 +1706,7 @@ fn test_i19_last_settlement_none_before_some_after() { client.record_usage(&agent, &svc, &3u32); // Never-settled reads as None (distinct from Some(0)). assert_eq!(client.get_last_settlement(&agent, &svc), None); - client.settle(&admin, &agent, &svc); + client.settle(&agent, &svc); assert_eq!(client.get_last_settlement(&agent, &svc), Some(ts)); } @@ -1832,7 +1832,7 @@ fn test_i21_settle_returns_saturated_value_and_drains() { let svc = Symbol::new(&env, "infer"); client.set_service_price(&svc, &i128::MAX); client.record_usage(&agent, &svc, &5u32); - let billed = client.settle(&admin, &agent, &svc); + let billed = client.settle(&agent, &svc); assert_eq!(billed, i128::MAX); // The counter still drains to zero even when billing saturated. assert_eq!(client.get_usage(&agent, &svc), 0); @@ -2136,7 +2136,7 @@ fn test_owner_can_settle_own_service() { client.set_service_price(&svc, &10i128); client.record_usage(&agent, &svc, &5u32); - let billed = client.settle(&owner, &agent, &svc); + let billed = client.settle(&agent, &svc); assert_eq!(billed, 50i128); assert_eq!(client.get_usage(&agent, &svc), 0); } @@ -2154,7 +2154,7 @@ fn test_admin_can_settle_owned_service() { client.set_service_price(&svc, &10i128); client.record_usage(&agent, &svc, &4u32); - let billed = client.settle(&admin, &agent, &svc); + let billed = client.settle(&agent, &svc); assert_eq!(billed, 40i128); } @@ -2177,7 +2177,7 @@ fn test_owner_cannot_settle_other_service() { client.record_usage(&agent, &svc_b, &3u32); // owner_a tries to settle svc_b — unauthorized. - client.settle(&owner_a, &agent, &svc_b); + client.settle(&agent, &svc_b); } /// A non-admin caller settling a service with no metadata is rejected with @@ -2193,7 +2193,7 @@ fn test_nonadmin_settle_without_metadata_rejected() { client.set_service_price(&svc, &10i128); client.record_usage(&agent, &svc, &2u32); - client.settle(&stranger, &agent, &svc); + client.settle(&agent, &svc); } /// The pause gate still applies to owner-authorized settlement. @@ -2207,7 +2207,7 @@ fn test_owner_settle_rejected_while_paused() { let svc = Symbol::new(&env, "infer"); client.set_service_metadata(&svc, &String::from_str(&env, "inference"), &owner); client.pause(); - client.settle(&owner, &agent, &svc); + client.settle(&agent, &svc); } /// By default the limiter is disabled (cap 0, window 0): an agent can record @@ -2446,7 +2446,7 @@ fn test_compute_billing_agrees_with_settle() { let pre_settle_bill = client.compute_billing(&agent, &svc); // settle returns the billed amount and drains the counter. - let settled = client.settle(&admin, &agent, &svc); + let settled = client.settle(&agent, &svc); assert_eq!( pre_settle_bill, settled, @@ -2465,7 +2465,7 @@ fn test_compute_billing_zero_after_settle() { set_price(&client, &svc, 50); record(&client, &agent, &svc, 4); - client.settle(&admin, &agent, &svc); + client.settle(&agent, &svc); // Counter is drained — billing must now be 0. let post_settle_bill = client.compute_billing(&agent, &svc); @@ -2698,3 +2698,170 @@ fn test_get_contract_config_is_idempotent() { let second = client.get_contract_config(); assert_eq!(first, second); } + +// ── register_service_with_metadata ──────────────────────────────────────────── +// +// Coverage: +// - Atomicity: both the registration flag and the metadata slot are written +// by a single call, and both are immediately readable afterwards. +// - Event: `svc_reg(service_id, owner)` is emitted. +// - Idempotency: re-registering the same service id overwrites the metadata. +// - Edge cases: empty `description` is accepted. +// - Security: non-admin callers are rejected; calling while paused panics #4. +// - Equivalence: the combined call produces the same state as the separate +// `register_service` + `set_service_metadata` sequence. + +/// After one `register_service_with_metadata` call, the service is registered +/// **and** its metadata reflects the exact description and owner provided. +#[test] +fn test_register_service_with_metadata_sets_both_slots() { + let env = Env::default(); + let (client, admin) = setup_initialized(&env); + + let service_id = Symbol::new(&env, "weather_api"); + let description = String::from_str(&env, "Weather data feed"); + let owner = Address::generate(&env); + + client.register_service_with_metadata(&service_id, &description, &owner); + + assert!(client.is_service_registered(&service_id)); + + let meta = client.get_service_metadata(&service_id).unwrap(); + assert_eq!(meta.description, description); + assert_eq!(meta.owner, owner); +} + +/// The call emits `svc_reg(service_id, owner)` as the sole event. +#[test] +fn test_register_service_with_metadata_emits_svc_reg_event() { + let env = Env::default(); + let (client, admin) = setup_initialized(&env); + + let service_id = Symbol::new(&env, "forecast"); + let description = String::from_str(&env, "Forecast API"); + let owner = Address::generate(&env); + + client.register_service_with_metadata(&service_id, &description, &owner); + + let events = env.events().all(); + assert!(!events.is_empty()); + let (_addr, topics, data) = events.last().unwrap(); + + let expected_topics: soroban_sdk::Vec = + (symbol_short!("svc_reg"),).into_val(&env); + assert_eq!(topics, expected_topics); + + let decoded: (Symbol, Address) = data.into_val(&env); + assert_eq!(decoded, (service_id, owner)); +} + +/// Re-registering an existing id overwrites its metadata (idempotent overwrite). +#[test] +fn test_register_service_with_metadata_idempotent_overwrite() { + let env = Env::default(); + let (client, admin) = setup_initialized(&env); + + let service_id = Symbol::new(&env, "forecast"); + let desc_a = String::from_str(&env, "v1 description"); + let owner_a = Address::generate(&env); + + client.register_service_with_metadata(&service_id, &desc_a, &owner_a); + + // Re-register with different metadata. + let desc_b = String::from_str(&env, "v2 description (overwritten)"); + let owner_b = Address::generate(&env); + client.register_service_with_metadata(&service_id, &desc_b, &owner_b); + + // Still registered. + assert!(client.is_service_registered(&service_id)); + // Metadata reflects the latest write. + let meta = client.get_service_metadata(&service_id).unwrap(); + assert_eq!(meta.description, desc_b); + assert_eq!(meta.owner, owner_b); +} + +/// An empty description is accepted. +#[test] +fn test_register_service_with_metadata_empty_description() { + let env = Env::default(); + let (client, admin) = setup_initialized(&env); + + let service_id = Symbol::new(&env, "empty_desc"); + let description = String::from_str(&env, ""); + let owner = Address::generate(&env); + + client.register_service_with_metadata(&service_id, &description, &owner); + + assert!(client.is_service_registered(&service_id)); + let meta = client.get_service_metadata(&service_id).unwrap(); + assert_eq!(meta.description, description); +} + +/// A non-admin caller is rejected (panics). +#[test] +#[should_panic] +fn test_register_service_with_metadata_rejects_non_admin() { + let env = Env::default(); + let client = setup_scoped_auth(&env); + + let service_id = Symbol::new(&env, "unauthorised"); + let description = String::from_str(&env, "sketchy service"); + let owner = Address::generate(&env); + + client.register_service_with_metadata(&service_id, &description, &owner); +} + +/// Calling while paused panics with `ContractPaused` (#4). +#[test] +#[should_panic(expected = "Error(Contract, #4)")] +fn test_register_service_with_metadata_panics_when_paused() { + let env = Env::default(); + let (client, admin) = setup_initialized(&env); + + client.pause(); + + let service_id = Symbol::new(&env, "paused_svc"); + let description = String::from_str(&env, "should not land"); + let owner = Address::generate(&env); + + client.register_service_with_metadata(&service_id, &description, &owner); +} + +/// The combined call is equivalent to calling `register_service` followed by +/// `set_service_metadata` — both produce the same storage state. +#[test] +fn test_register_service_with_metadata_equivalent_to_separate_calls() { + let env = Env::default(); + let (client, admin) = setup_initialized(&env); + + let service_id = Symbol::new(&env, "equiv"); + let description = String::from_str(&env, "equivalent service"); + let owner = Address::generate(&env); + + // Combined call. + client.register_service_with_metadata(&service_id, &description, &owner); + + let combined_registered = client.is_service_registered(&service_id); + let combined_meta = client.get_service_metadata(&service_id); + + // Fresh contract for separate-call path. + let env2 = Env::default(); + env2.mock_all_auths(); + let contract_id2 = env2.register_contract(None, Escrow); + let client2 = EscrowClient::new(&env2, &contract_id2); + let admin2 = Address::generate(&env2); + client2.init(&admin2); + + let service_id2 = Symbol::new(&env2, "equiv"); + let description2 = String::from_str(&env2, "equivalent service"); + let owner2 = Address::generate(&env2); + + client2.register_service(&service_id2); + client2.set_service_metadata(&service_id2, &description2, &owner2); + + assert_eq!( + combined_registered, + client2.is_service_registered(&service_id2) + ); + assert_eq!(combined_meta, client2.get_service_metadata(&service_id2)); +}