Merge pull request #329 from Phoenix-Protocol-Group/328-phoam-006-any…

…one-can-re-initialize-trader-contract-to-steal-balance

Trader: add checker for double initializing

CHANGELOG.md

@@ -19,6 +19,7 @@ and this project adheres to
 - Trader: Adds nominated trader contract to the DEX ([#288])
 - Multihop: Allows multihop swaps for stable pool ([#303])
 - Stake rewards: Create a new contract and prepare first testcases ([#306])
+- Trader: Adds a check if the contract has been already initialized ([#329])
 
 [#288]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/288
 [#299]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/299
@@ -28,6 +29,7 @@ and this project adheres to
 [#307]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/307
 [#308]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/308
 [#322]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/322
+[#329]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/329
 
 ## [1.0.0] - 2024-05-08
 

contracts/trader/src/contract.rs

@@ -6,14 +6,12 @@ use crate::{
     error::ContractError,
     lp_contract,
     storage::{
-        get_admin, get_name, get_output_token, get_pair, save_admin, save_name, save_output_token,
-        save_pair, Asset, BalanceInfo, OutputTokenInfo,
+        get_admin, get_name, get_output_token, get_pair, is_initialized, save_admin, save_name,
+        save_output_token, save_pair, set_initialized, Asset, BalanceInfo, OutputTokenInfo,
     },
     token_contract,
 };
 
-use phoenix::ensure_not_expired;
-
 contractmeta!(
     key = "Description",
     val = "Phoenix Protocol Designated Trader Contract"
@@ -72,6 +70,11 @@ impl TraderTrait for Trader {
     ) {
         admin.require_auth();
 
+        if is_initialized(&env) {
+            log!(&env, "Trader: Initialize: Cannot initialize trader twice!");
+            panic_with_error!(env, ContractError::AlreadyInitialized)
+        }
+
         save_admin(&env, &admin);
 
         save_name(&env, &contract_name);
@@ -80,6 +83,8 @@ impl TraderTrait for Trader {
 
         save_output_token(&env, &output_token);
 
+        set_initialized(&env);
+
         env.events()
             .publish(("Trader: Initialize", "admin: "), &admin);
         env.events()
@@ -101,10 +106,6 @@ impl TraderTrait for Trader {
     ) {
         sender.require_auth();
 
-        if let Some(deadline) = deadline {
-            ensure_not_expired!(env, deadline)
-        }
-
         if sender != get_admin(&env) {
             log!(&env, "Trader: Trade_token: Unauthorized trade");
             panic_with_error!(env, ContractError::Unauthorized);
@@ -147,6 +148,7 @@ impl TraderTrait for Trader {
             &amount,
             &None,
             &max_spread_bps,
+            &deadline,
         );
 
         env.events()

contracts/trader/src/error.rs

@@ -12,4 +12,6 @@ pub enum ContractError {
     Unauthorized = 6,
     SwapTokenNotInPair = 7,
     InvalidMaxSpreadBps = 8,
+    InitValueNotFound = 9,
+    AlreadyInitialized = 10,
 }

contracts/trader/src/storage.rs

@@ -12,6 +12,7 @@ pub enum DataKey {
     Pair,
     Token,
     MaxSpread,
+    IsInitialized,
 }
 
 #[contracttype]
@@ -105,3 +106,16 @@ pub fn get_output_token(env: &Env) -> Address {
             panic_with_error!(env, ContractError::OutputTokenNotFound)
         })
 }
+
+pub fn set_initialized(env: &Env) {
+    env.storage()
+        .persistent()
+        .set(&DataKey::IsInitialized, &true);
+}
+
+pub fn is_initialized(env: &Env) -> bool {
+    env.storage()
+        .persistent()
+        .get(&DataKey::IsInitialized)
+        .unwrap_or_default()
+}

contracts/trader/src/tests/msgs.rs

@@ -57,6 +57,61 @@ fn initialize() {
     );
 }
 
+#[test]
+#[should_panic(expected = "Trader: Initialize: Cannot initialize trader twice!")]
+fn initialize_twice_should_panic() {
+    let env = Env::default();
+    env.mock_all_auths();
+    env.budget().reset_unlimited();
+
+    let admin = Address::generate(&env);
+    let contract_name = String::from_str(&env, "XLM/USDC");
+    let xlm_token = deploy_token_contract(
+        &env,
+        &admin,
+        &6,
+        &String::from_str(&env, "Stellar"),
+        &String::from_str(&env, "XLM"),
+    );
+    let usdc_token = deploy_token_contract(
+        &env,
+        &admin,
+        &6,
+        &String::from_str(&env, "USD Coin"),
+        &String::from_str(&env, "USDC"),
+    );
+    let pho_token = deploy_token_contract(
+        &env,
+        &admin,
+        &6,
+        &String::from_str(&env, "Phoenix"),
+        &String::from_str(&env, "PHO"),
+    );
+
+    let trader_client = deploy_trader_client(&env);
+    trader_client.initialize(
+        &admin,
+        &contract_name,
+        &(xlm_token.address.clone(), usdc_token.address.clone()),
+        &pho_token.address,
+    );
+
+    assert_eq!(trader_client.query_admin_address(), admin);
+    assert_eq!(trader_client.query_contract_name(), contract_name);
+    assert_eq!(
+        trader_client.query_trading_pairs(),
+        (xlm_token.address.clone(), usdc_token.address.clone())
+    );
+
+    // second time should fail
+    trader_client.initialize(
+        &admin,
+        &contract_name,
+        &(xlm_token.address.clone(), usdc_token.address.clone()),
+        &pho_token.address,
+    );
+}
+
 #[test]
 fn simple_trade_token_and_transfer_token() {
     let env = Env::default();
@@ -142,6 +197,7 @@ fn simple_trade_token_and_transfer_token() {
         &xlm_pho_client.address,
         &Some(1_000),
         &None::<i64>,
+        &None,
     );
 
     assert_eq!(
@@ -274,6 +330,7 @@ fn extended_trade_and_transfer_token() {
         &xlm_pho_client.address,
         &Some(1_000),
         &None::<i64>,
+        &None,
     );
 
     assert_eq!(
@@ -303,6 +360,7 @@ fn extended_trade_and_transfer_token() {
         &xlm_pho_client.address,
         &Some(1_000),
         &None::<i64>,
+        &None,
     );
 
     assert_eq!(
@@ -333,6 +391,7 @@ fn extended_trade_and_transfer_token() {
         &usdc_pho_client.address,
         &Some(1_500),
         &None::<i64>,
+        &None,
     );
 
     // 1899 + 450 = 2_349
@@ -363,6 +422,7 @@ fn extended_trade_and_transfer_token() {
         &usdc_pho_client.address,
         &Some(1_500),
         &None::<i64>,
+        &None,
     );
 
     // 2_349 + 450 = 2_799
@@ -451,6 +511,7 @@ fn trade_token_should_fail_when_unauthorized() {
         &xlm_pho_client.address,
         &Some(1_000),
         &None::<i64>,
+        &None,
     );
 }
 
@@ -515,6 +576,7 @@ fn trade_token_should_fail_when_offered_token_not_in_pair() {
         &xlm_pho_client.address,
         &Some(1_000),
         &None::<i64>,
+        &None,
     );
 }
 
@@ -580,6 +642,7 @@ fn transfer_should_fail_when_unauthorized() {
         &xlm_pho_client.address,
         &Some(1_000),
         &None::<i64>,
+        &None,
     );
 
     trader_client.transfer(&Address::generate(&env), &rcpt, &1_000, &None);
@@ -647,5 +710,6 @@ fn transfer_should_fail_with_invalid_spread_bps(max_spread_bps: i64) {
         &xlm_pho_client.address,
         &Some(1_000),
         &Some(max_spread_bps),
+        &None,
     );
 }

contracts/trader/src/tests/setup.rs

@@ -104,6 +104,7 @@ pub fn deploy_and_init_lp_client(
         &6,
         &String::from_str(env, "staked Phoenix"),
         &String::from_str(env, "sPHO"),
+        &100i64,
     );
 
     lp_client.provide_liquidity(
@@ -113,6 +114,7 @@ pub fn deploy_and_init_lp_client(
         &Some(token_b_amount),
         &None::<i128>,
         &None::<i64>,
+        &None,
     );
     lp_client
 }