From c8e42f47397b6d585e32d789fc17577a1bb96a5e Mon Sep 17 00:00:00 2001 From: Peolite001 Date: Sat, 17 Oct 2026 21:38:53 +0100 Subject: [PATCH 1/3] test(creator-keys): add integration test for re-initialization revert (#584) - Add initialize entrypoint to CreatorKeysContract for one-time protocol setup - Add AlreadyInitialized (110) error code to ContractError enum - Add reinitialization_revert.rs integration test verifying re-init reverts with AlreadyInitialized - Assert admin, fee bps, and fee recipient remain unchanged after failed re-initialization - Document AlreadyInitialized error code in docs/error-codes.md --- creator-keys/src/lib.rs | 112 ++++++++++++ creator-keys/tests/reinitialization_revert.rs | 165 ++++++++++++++++++ docs/error-codes.md | 3 +- 3 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 creator-keys/tests/reinitialization_revert.rs diff --git a/creator-keys/src/lib.rs b/creator-keys/src/lib.rs index 4a3142cf..feb15215 100644 --- a/creator-keys/src/lib.rs +++ b/creator-keys/src/lib.rs @@ -213,6 +213,12 @@ pub enum ContractError { KeyAlertLimitReached = 108, /// No active price alert with the given id belongs to the wallet. AlertNotFound = 109, + /// Smart contract protocol configuration is already initialized. + AlreadyInitialized = 110, +} + +impl ContractError { + pub const AlreadyInitialised: Self = Self::AlreadyInitialized; } /// Errors raised by the staking entrypoints @@ -10027,6 +10033,85 @@ impl CreatorKeysContract { Ok(config.protocol_bps) } + /// One-time initialization of protocol configuration (#584). + /// + /// Sets the protocol admin, protocol fee basis points, and protocol fee recipient. + /// Calling this on an already-initialized contract reverts with [`ContractError::AlreadyInitialized`]. + /// + /// # Arguments + /// * `admin` - Protocol admin address. Must authorize the call. + /// * `fee_bps` - Protocol fee basis points (0..=PROTOCOL_BPS_MAX). + pub fn initialize(env: Env, admin: Address, fee_bps: u32) -> Result<(), ContractError> { + if env + .storage() + .persistent() + .has(&constants::storage::ADMIN_ADDRESS) + || read_protocol_fee_config(&env).is_some() + { + return Err(ContractError::AlreadyInitialized); + } + + admin.require_auth(); + validate_non_zero_address(&env, &admin)?; + + let creator_bps = fee::BPS_MAX + .checked_sub(fee_bps) + .ok_or(ContractError::InvalidFeeConfig)?; + fee::assert_valid_fee_bps(creator_bps, fee_bps)?; + + env.storage() + .persistent() + .set(&constants::storage::ADMIN_ADDRESS, &admin); + extend_key_ttl_to_full_window(&env, &constants::storage::ADMIN_ADDRESS); + + let config = fee::FeeConfig { + creator_bps, + protocol_bps: fee_bps, + }; + env.storage() + .persistent() + .set(&constants::storage::FEE_CONFIG, &config); + extend_key_ttl_to_full_window(&env, &constants::storage::FEE_CONFIG); + + env.storage() + .persistent() + .set(&constants::storage::PROTOCOL_FEE_RECIPIENT, &admin); + extend_key_ttl_to_full_window(&env, &constants::storage::PROTOCOL_FEE_RECIPIENT); + + env.events().publish( + (events::CONTRACT_INITIALIZED_EVENT_NAME, admin.clone()), + events::ContractInitializedEvent { + admin: admin.clone(), + protocol_fee_bps: fee_bps, + protocol_fee_recipient: admin.clone(), + initialized_at_ledger: env.ledger().sequence(), + }, + ); + + env.events().publish( + (events::FEE_CONFIG_UPDATED_EVENT_NAME, admin), + events::FeeConfigUpdatedEvent { + old_bps: 0, + new_bps: creator_bps, + updated_at_ledger: env.ledger().sequence(), + }, + ); + + let current_version = env + .storage() + .persistent() + .get(&constants::storage::PROTOCOL_STATE_VERSION) + .unwrap_or(PROTOCOL_STATE_VERSION_INITIAL); + let new_version = current_version + .checked_add(1) + .ok_or(ContractError::Overflow)?; + env.storage() + .persistent() + .set(&constants::storage::PROTOCOL_STATE_VERSION, &new_version); + + Ok(()) + } + /// Sets the global protocol/creator fee split. Contract initialization /// entrypoint. /// @@ -20308,6 +20393,33 @@ mod tests { assert!(!policy.compression_enabled); assert_eq!(policy.batch_size, 500); } + + // --- re-initialization unit tests (#584) --- + + #[test] + fn test_initialize_sets_protocol_config_and_reinitialization_reverts() { + let env = Env::default(); + env.mock_all_auths(); + let contract_id = env.register(super::CreatorKeysContract, ()); + let client = super::CreatorKeysContractClient::new(&env, &contract_id); + let admin_a = Address::generate(&env); + let admin_b = Address::generate(&env); + + client.initialize(&admin_a, &300u32); + + assert_eq!(client.get_protocol_admin(), Some(admin_a.clone())); + assert_eq!(client.get_protocol_fee_bps(), 300u32); + assert_eq!(client.get_protocol_fee_recipient(), Some(admin_a.clone())); + assert!(client.is_protocol_config_initialized()); + + let err = client.try_initialize(&admin_b, &500u32); + assert_eq!(err, Err(Ok(super::ContractError::AlreadyInitialized))); + + assert_eq!(client.get_protocol_admin(), Some(admin_a.clone())); + assert_eq!(client.get_protocol_fee_bps(), 300u32); + assert_eq!(client.get_protocol_fee_recipient(), Some(admin_a)); + assert!(client.is_protocol_config_initialized()); + } } #[cfg(test)] diff --git a/creator-keys/tests/reinitialization_revert.rs b/creator-keys/tests/reinitialization_revert.rs new file mode 100644 index 00000000..0f0e449d --- /dev/null +++ b/creator-keys/tests/reinitialization_revert.rs @@ -0,0 +1,165 @@ +//! Integration tests for re-initialization attempt reverting without modifying contract state (#584). +//! +//! Scope: +//! - Initialize the contract with admin A and fee bps 300 +//! - Attempt to re-initialize with admin B and fee bps 500 +//! - Assert the second call reverts with the already-initialized error code +//! - Assert admin and fee bps remain as set in the first initialization +//! +//! Acceptance Criteria: +//! - Re-initialization reverts with already-initialized error code +//! - Admin unchanged after failed re-init +//! - Fee bps unchanged after failed re-init +//! - Fee recipient unchanged after failed re-init + +mod contract_test_env; + +use contract_test_env::{register_creator_keys, test_env_with_auths}; +use creator_keys::{ContractError, CreatorKeysContractClient}; +use soroban_sdk::{testutils::Address as _, Address}; + +#[test] +fn test_reinitialization_attempt_reverts_without_modifying_state() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let admin_a = Address::generate(&env); + let fee_bps_a = 300u32; + + // 1. Initialize the contract with admin A and fee bps 300 + client.initialize(&admin_a, &fee_bps_a); + + // Assert initial state is set as expected + assert_eq!( + client.get_protocol_admin(), + Some(admin_a.clone()), + "admin must be set to admin A" + ); + assert_eq!( + client.get_protocol_fee_bps(), + fee_bps_a, + "protocol fee bps must be 300" + ); + assert_eq!( + client.get_protocol_fee_recipient(), + Some(admin_a.clone()), + "fee recipient must default to admin A" + ); + assert!( + client.is_protocol_config_initialized(), + "protocol config must be marked initialized" + ); + + let config_a = client.get_fee_config().unwrap(); + assert_eq!(config_a.creator_bps, 10_000 - fee_bps_a); + assert_eq!(config_a.protocol_bps, fee_bps_a); + + // 2. Attempt to re-initialize with admin B and fee bps 500 + let admin_b = Address::generate(&env); + let fee_bps_b = 500u32; + let result = client.try_initialize(&admin_b, &fee_bps_b); + + // 3. Acceptance criterion: Re-initialization reverts with already-initialized error code + assert_eq!( + result, + Err(Ok(ContractError::AlreadyInitialized)), + "re-initialization call must revert with AlreadyInitialized error code" + ); + + // 4. Acceptance criterion: Admin unchanged after failed re-init + assert_eq!( + client.get_protocol_admin(), + Some(admin_a.clone()), + "admin must remain unchanged after failed re-init" + ); + + // 5. Acceptance criterion: Fee bps unchanged after failed re-init + assert_eq!( + client.get_protocol_fee_bps(), + fee_bps_a, + "fee bps must remain unchanged after failed re-init" + ); + + // 6. Acceptance criterion: Fee recipient unchanged after failed re-init + assert_eq!( + client.get_protocol_fee_recipient(), + Some(admin_a.clone()), + "fee recipient must remain unchanged after failed re-init" + ); + + // Confirm fee config structure is also unchanged + let config_after = client.get_fee_config().unwrap(); + assert_eq!(config_after.creator_bps, 10_000 - fee_bps_a); + assert_eq!(config_after.protocol_bps, fee_bps_a); + assert!(client.is_protocol_config_initialized()); +} + +#[test] +fn test_reinitialization_preserves_custom_fee_recipient() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let admin_a = Address::generate(&env); + let custom_recipient = Address::generate(&env); + let fee_bps_a = 300u32; + + // Initialize with admin A and fee bps 300 + client.initialize(&admin_a, &fee_bps_a); + + // Admin A updates fee recipient to a custom address + client.set_protocol_fee_recipient(&admin_a, &custom_recipient); + assert_eq!( + client.get_protocol_fee_recipient(), + Some(custom_recipient.clone()) + ); + + // Attempt re-initialization with admin B and fee bps 500 + let admin_b = Address::generate(&env); + let fee_bps_b = 500u32; + let result = client.try_initialize(&admin_b, &fee_bps_b); + + assert_eq!(result, Err(Ok(ContractError::AlreadyInitialized))); + + // Admin, fee bps, and custom fee recipient must all remain unchanged + assert_eq!( + client.get_protocol_admin(), + Some(admin_a), + "admin must remain unchanged" + ); + assert_eq!( + client.get_protocol_fee_bps(), + fee_bps_a, + "fee bps must remain unchanged" + ); + assert_eq!( + client.get_protocol_fee_recipient(), + Some(custom_recipient), + "custom fee recipient must remain unchanged after failed re-init" + ); +} + +#[test] +fn test_reinitialization_with_same_params_also_reverts() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let admin_a = Address::generate(&env); + let fee_bps_a = 300u32; + + // Initialize + client.initialize(&admin_a, &fee_bps_a); + + // Attempt re-initialization with identical parameters + let result = client.try_initialize(&admin_a, &fee_bps_a); + assert_eq!(result, Err(Ok(ContractError::AlreadyInitialized))); + + assert_eq!(client.get_protocol_admin(), Some(admin_a.clone())); + assert_eq!(client.get_protocol_fee_bps(), fee_bps_a); + assert_eq!(client.get_protocol_fee_recipient(), Some(admin_a)); +} + +#[test] +fn test_already_initialized_error_code_numeric_value() { + assert_eq!(ContractError::AlreadyInitialized as u32, 110); + assert_eq!(ContractError::AlreadyInitialised as u32, 110); +} diff --git a/docs/error-codes.md b/docs/error-codes.md index 268f8bac..a7eb2af9 100644 --- a/docs/error-codes.md +++ b/docs/error-codes.md @@ -62,8 +62,9 @@ intentionally unassigned. | `107` | `WalletAlertLimitReached` | The wallet already holds the maximum number of active price alerts | Triggered in [`price_alerts::register_alert`](../creator-keys/src/price_alerts.rs) when the wallet has `MAX_ACTIVE_ALERTS_PER_WALLET` (10) active alerts across all keys. Alerts that have fired or been cancelled do not count. | | `108` | `KeyAlertLimitReached` | The key already has the maximum number of active price alerts | Triggered in [`price_alerts::register_alert`](../creator-keys/src/price_alerts.rs) when the key has `MAX_ACTIVE_ALERTS_PER_KEY` (16) active alerts from any wallets. | | `109` | `AlertNotFound` | No active price alert with that id belongs to the wallet | Triggered in [`price_alerts::cancel_alert`](../creator-keys/src/price_alerts.rs) when the id never existed, belongs to another wallet, or the alert has already fired or been cancelled. | +| `110` | `AlreadyInitialized` | Protocol configuration has already been initialized | Triggered in [`initialize`](../creator-keys/src/lib.rs) when attempting to re-initialize an already-initialized contract. | -**Backward compatibility for 107-109**: these price-alert codes (issue #1030) +**Backward compatibility for 107-110**: these codes are appended after the previously highest code (`106`, `CapTooLow`). No existing variant was renumbered, reordered, or reused. `register_alert` reports an unregistered key with `NotRegistered` (2) and a non-positive target price with From 9367e9bec1125e0d92351208e831e9fe1678d82a Mon Sep 17 00:00:00 2001 From: Peolite001 Date: Sat, 17 Oct 2026 21:55:14 +0100 Subject: [PATCH 2/3] fix(creator-keys): allow non_upper_case_globals for AlreadyInitialised alias --- creator-keys/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/creator-keys/src/lib.rs b/creator-keys/src/lib.rs index feb15215..4d48669e 100644 --- a/creator-keys/src/lib.rs +++ b/creator-keys/src/lib.rs @@ -218,6 +218,7 @@ pub enum ContractError { } impl ContractError { + #[allow(non_upper_case_globals)] pub const AlreadyInitialised: Self = Self::AlreadyInitialized; } From 3931c4549a3e882681eb9ff1882394b41ab1903d Mon Sep 17 00:00:00 2001 From: Peolite001 Date: Sun, 18 Oct 2026 01:19:39 +0100 Subject: [PATCH 3/3] test(creator-keys): remove unused CreatorKeysContractClient import --- creator-keys/tests/reinitialization_revert.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/creator-keys/tests/reinitialization_revert.rs b/creator-keys/tests/reinitialization_revert.rs index 0f0e449d..f34ecc34 100644 --- a/creator-keys/tests/reinitialization_revert.rs +++ b/creator-keys/tests/reinitialization_revert.rs @@ -15,7 +15,7 @@ mod contract_test_env; use contract_test_env::{register_creator_keys, test_env_with_auths}; -use creator_keys::{ContractError, CreatorKeysContractClient}; +use creator_keys::ContractError; use soroban_sdk::{testutils::Address as _, Address}; #[test]