diff --git a/creator-keys/src/lib.rs b/creator-keys/src/lib.rs index 4a3142cf..4d48669e 100644 --- a/creator-keys/src/lib.rs +++ b/creator-keys/src/lib.rs @@ -213,6 +213,13 @@ 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 { + #[allow(non_upper_case_globals)] + pub const AlreadyInitialised: Self = Self::AlreadyInitialized; } /// Errors raised by the staking entrypoints @@ -10027,6 +10034,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 +20394,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..f34ecc34 --- /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; +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