From 9117066b8774a1a4393ff534f16bd9692bb9d5fa Mon Sep 17 00:00:00 2001 From: Jagadeesh Date: Wed, 29 Jul 2026 02:34:26 +0530 Subject: [PATCH] refactor(events): extract shared cfg_set publish into a helper --- contracts/escrow/src/lib.rs | 49 +++++++++--------- contracts/escrow/src/test.rs | 96 ++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 25 deletions(-) diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index 7c284ba..d94282c 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -2,7 +2,7 @@ use soroban_sdk::{ contract, contracterror, contractimpl, contracttype, panic_with_error, symbol_short, Address, - Env, String, Symbol, Vec, + Env, IntoVal, String, Symbol, Val, Vec, }; /// Current on-chain storage schema version stamped at init. @@ -459,6 +459,23 @@ fn ensure_service_usable(env: &Env, service_id: &Symbol) { } } +/// Publish the shared `cfg_set(tag, value)` event used by every single- +/// scalar admin config setter (`set_allowlist_enabled`, +/// `set_min_requests_per_call`, `set_max_requests_per_call`, +/// `set_max_requests_per_window`, `set_rate_window_seconds`, +/// `set_require_service_registration`). Centralising the publish call keeps +/// the topic and payload shape identical at every call site — a new config +/// setter only needs to pick a `tag` and call this helper, rather than +/// duplicating the `env.events().publish(...)` block. +fn publish_cfg_event(env: &Env, tag: Symbol, value: T) +where + T: IntoVal, + (Symbol, T): IntoVal, +{ + env.events() + .publish((symbol_short!("cfg_set"),), (tag, value)); +} + // Shared access-control helpers. // // Admin-gated entrypoints and the pause gate repeat the same small blocks of @@ -1789,10 +1806,7 @@ impl Escrow { pub fn set_allowlist_enabled(env: Env, enabled: bool) { require_admin(&env); write_flag(&env, &DataKey::AllowlistEnabled, enabled); - env.events().publish( - (symbol_short!("cfg_set"),), - (symbol_short!("allowlist"), enabled), - ); + publish_cfg_event(&env, symbol_short!("allowlist"), enabled); } /// Read the master allowlist toggle. @@ -1879,10 +1893,7 @@ impl Escrow { env.storage() .persistent() .set(&DataKey::MinRequestsPerCall, &min_requests); - env.events().publish( - (symbol_short!("cfg_set"),), - (symbol_short!("min_call"), min_requests), - ); + publish_cfg_event(&env, symbol_short!("min_call"), min_requests); } /// Read the configured per-call cap, or `u32::MAX` (no limit) if @@ -1915,10 +1926,7 @@ impl Escrow { env.storage() .persistent() .set(&DataKey::MaxRequestsPerWindow, &max_requests); - env.events().publish( - (symbol_short!("cfg_set"),), - (symbol_short!("max_win"), max_requests), - ); + publish_cfg_event(&env, symbol_short!("max_win"), max_requests); } /// Read the configured rate-limit window length in seconds, or `0` @@ -1942,10 +1950,7 @@ impl Escrow { env.storage() .persistent() .set(&DataKey::WindowSeconds, &window_seconds); - env.events().publish( - (symbol_short!("cfg_set"),), - (symbol_short!("win_sec"), window_seconds), - ); + publish_cfg_event(&env, symbol_short!("win_sec"), window_seconds); } /// Admin-gated, pause-respecting entrypoint that clears the per-agent @@ -2056,10 +2061,7 @@ impl Escrow { env.storage() .persistent() .set(&DataKey::MaxRequestsPerCall, &max_requests); - env.events().publish( - (symbol_short!("cfg_set"),), - (symbol_short!("max_call"), max_requests), - ); + publish_cfg_event(&env, symbol_short!("max_call"), max_requests); } /// Admin toggles strict-registration mode. When enabled, @@ -2071,10 +2073,7 @@ impl Escrow { 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!("req_reg"), required), - ); + publish_cfg_event(&env, symbol_short!("req_reg"), required); } /// Read the strict-registration flag. diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 7812811..b0fc64f 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -7645,3 +7645,99 @@ fn test_settle_all_service_metadata_not_found_panics() { let _ = owner; client.settle_all(&intruder, &agent); } + +// ── Shared cfg_set event payload (publish_cfg_event helper) ───────────────── +// +// Six admin config setters share a single `cfg_set(tag, value)` publish call, +// now centralised in the private `publish_cfg_event` helper. None of them had +// a direct payload-correctness test before this refactor; these lock in the +// exact (tag, value) pair each setter must keep emitting. + +#[test] +fn test_cfg_set_payload_set_allowlist_enabled() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.set_allowlist_enabled(&true); + let (_, topics, data) = env.events().all().last().unwrap(); + let expected: soroban_sdk::Vec = (symbol_short!("cfg_set"),).into_val(&env); + assert_eq!(topics, expected); + let decoded: (Symbol, bool) = data.into_val(&env); + assert_eq!(decoded, (symbol_short!("allowlist"), true)); +} +#[test] +fn test_cfg_set_payload_set_min_requests_per_call() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.set_min_requests_per_call(&3u32); + let (_, topics, data) = env.events().all().last().unwrap(); + let expected: soroban_sdk::Vec = (symbol_short!("cfg_set"),).into_val(&env); + assert_eq!(topics, expected); + let decoded: (Symbol, u32) = data.into_val(&env); + assert_eq!(decoded, (symbol_short!("min_call"), 3u32)); +} +#[test] +fn test_cfg_set_payload_set_max_requests_per_call() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.set_max_requests_per_call(&500u32); + let (_, topics, data) = env.events().all().last().unwrap(); + let expected: soroban_sdk::Vec = (symbol_short!("cfg_set"),).into_val(&env); + assert_eq!(topics, expected); + let decoded: (Symbol, u32) = data.into_val(&env); + assert_eq!(decoded, (symbol_short!("max_call"), 500u32)); +} +#[test] +fn test_cfg_set_payload_set_max_requests_per_window() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.set_max_requests_per_window(&100u32); + let (_, topics, data) = env.events().all().last().unwrap(); + let expected: soroban_sdk::Vec = (symbol_short!("cfg_set"),).into_val(&env); + assert_eq!(topics, expected); + let decoded: (Symbol, u32) = data.into_val(&env); + assert_eq!(decoded, (symbol_short!("max_win"), 100u32)); +} +#[test] +fn test_cfg_set_payload_set_rate_window_seconds() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.set_rate_window_seconds(&60u64); + let (_, topics, data) = env.events().all().last().unwrap(); + let expected: soroban_sdk::Vec = (symbol_short!("cfg_set"),).into_val(&env); + assert_eq!(topics, expected); + let decoded: (Symbol, u64) = data.into_val(&env); + assert_eq!(decoded, (symbol_short!("win_sec"), 60u64)); +} +#[test] +fn test_cfg_set_payload_set_require_service_registration() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.set_require_service_registration(&true); + let (_, topics, data) = env.events().all().last().unwrap(); + let expected: soroban_sdk::Vec = (symbol_short!("cfg_set"),).into_val(&env); + assert_eq!(topics, expected); + let decoded: (Symbol, bool) = data.into_val(&env); + assert_eq!(decoded, (symbol_short!("req_reg"), true)); +} +#[test] +fn test_cfg_set_payload_tags_are_mutually_distinct() { + // The six setters share one topic; the tag in payload position 0 is + // what disambiguates them. Guards against two setters ever reusing + // the same tag, which would make them indistinguishable to indexers. + let tags = [ + symbol_short!("allowlist"), + symbol_short!("min_call"), + symbol_short!("max_call"), + symbol_short!("max_win"), + symbol_short!("win_sec"), + symbol_short!("req_reg"), + ]; + for i in 0..tags.len() { + for j in (i + 1)..tags.len() { + assert_ne!( + tags[i], tags[j], + "cfg_set tags must all be mutually distinct" + ); + } + } +}