Skip to content

Commit

Permalink
Merge pull request #194 from Phoenix-Protocol-Group/181-audit-factory…
Browse files Browse the repository at this point in the history 
…-can-be-made-to-deploy-malicious-pools

Factory, Multihop, Phoenix: removes lp_wasm_hash from the LiquidityPoolInitInfo struct and adds as a parameter
  • Loading branch information
@ueco-jb
ueco-jb authored Jan 25, 2024
2 parents dcc3bd8 + 4060833 commit 0d8fa94
Showing 14 changed files with 178 additions and 226 deletions.
62 changes: 21 additions & 41 deletions contracts/factory/src/contract.rs
View file
Original file line number Diff line number Diff line change
@@ -23,6 +23,9 @@ pub trait FactoryTrait {
env: Env,
admin: Address,
multihop_wasm_hash: BytesN<32>,
lp_wasm_hash: BytesN<32>,
stake_wasm_hash: BytesN<32>,
token_wasm_hash: BytesN<32>,
whitelisted_accounts: Vec<Address>,
);

@@ -51,6 +54,9 @@ impl FactoryTrait for Factory {
env: Env,
admin: Address,
multihop_wasm_hash: BytesN<32>,
lp_wasm_hash: BytesN<32>,
stake_wasm_hash: BytesN<32>,
token_wasm_hash: BytesN<32>,
whitelisted_accounts: Vec<Address>,
) {
if is_initialized(&env) {
@@ -71,6 +77,9 @@ impl FactoryTrait for Factory {
Config {
admin: admin.clone(),
multihop_address,
lp_wasm_hash,
stake_wasm_hash,
token_wasm_hash,
whitelisted_accounts,
},
);
@@ -99,26 +108,21 @@ impl FactoryTrait for Factory {
&lp_init_info.stake_init_info,
);

let config = get_config(&env);
let lp_wasm_hash = config.lp_wasm_hash;
let stake_wasm_hash = config.stake_wasm_hash;
let token_wasm_hash = config.token_wasm_hash;

let lp_contract_address = deploy_lp_contract(
&env,
lp_init_info.lp_wasm_hash,
lp_wasm_hash,
&lp_init_info.token_init_info.token_a,
&lp_init_info.token_init_info.token_b,
);

let init_fn: Symbol = Symbol::new(&env, "initialize");
let init_fn_args: Vec<Val> = (
lp_init_info.admin,
lp_init_info.share_token_decimals,
lp_init_info.swap_fee_bps,
lp_init_info.fee_recipient,
lp_init_info.max_allowed_slippage_bps,
lp_init_info.max_allowed_spread_bps,
lp_init_info.max_referral_bps,
lp_init_info.token_init_info.clone(),
lp_init_info.stake_init_info,
)
.into_val(&env);
let init_fn_args: Vec<Val> =
(stake_wasm_hash, token_wasm_hash, lp_init_info.clone()).into_val(&env);

env.invoke_contract::<Val>(&lp_contract_address, &init_fn, init_fn_args);

@@ -227,7 +231,7 @@ fn validate_token_info(
#[cfg(test)]
mod tests {
use super::*;
use soroban_sdk::{testutils::Address as _, Address, BytesN, String};
use soroban_sdk::{testutils::Address as _, Address, String};

#[test]
#[should_panic(
@@ -236,9 +240,6 @@ mod tests {
fn validate_token_info_should_fail_on_token_a_less_than_token_b() {
let env = Env::default();

let token_wasm_hash = BytesN::from_array(&env, &[8u8; 0x20]);
let stake_wasm_hash = BytesN::from_array(&env, &[15u8; 0x20]);

let token_a = Address::from_string(&String::from_str(
&env,
"CBGJMPOZ573XUTIRRFWGWTGSIAOGKJRVMIKBTFYEWTEIU7AEDWKDYMUX",
@@ -248,17 +249,12 @@ mod tests {
"CAOUDQCLN3BYHH4L7GSH3OSQJFVELHKOEVKOPBENVIGZ6WZ5ZRHFC5LN",
));

let token_init_info = TokenInitInfo {
token_a,
token_b,
token_wasm_hash,
};
let token_init_info = TokenInitInfo { token_a, token_b };

let stake_init_info = StakeInitInfo {
max_distributions: 10,
min_bond: 10,
min_reward: 10,
stake_wasm_hash,
};
validate_token_info(&env, &token_init_info, &stake_init_info);
}
@@ -270,23 +266,15 @@ mod tests {
fn validate_token_info_should_fail_on_min_bond_less_than_zero() {
let env = Env::default();

let token_wasm_hash = BytesN::from_array(&env, &[8u8; 0x20]);
let stake_wasm_hash = BytesN::from_array(&env, &[15u8; 0x20]);

let token_a = Address::generate(&env);
let token_b = Address::generate(&env);

let token_init_info = TokenInitInfo {
token_a,
token_b,
token_wasm_hash,
};
let token_init_info = TokenInitInfo { token_a, token_b };

let stake_init_info = StakeInitInfo {
max_distributions: 10,
min_bond: 0,
min_reward: 10,
stake_wasm_hash,
};

validate_token_info(&env, &token_init_info, &stake_init_info);
@@ -297,23 +285,15 @@ mod tests {
fn validate_token_info_should_fail_on_min_reward_less_than_zero() {
let env = Env::default();

let token_wasm_hash = BytesN::from_array(&env, &[8u8; 0x20]);
let stake_wasm_hash = BytesN::from_array(&env, &[15u8; 0x20]);

let token_a = Address::generate(&env);
let token_b = Address::generate(&env);

let token_init_info = TokenInitInfo {
token_a,
token_b,
token_wasm_hash,
};
let token_init_info = TokenInitInfo { token_a, token_b };

let stake_init_info = StakeInitInfo {
max_distributions: 10,
min_bond: 10,
min_reward: 0,
stake_wasm_hash,
};
validate_token_info(&env, &token_init_info, &stake_init_info);
}
6 changes: 4 additions & 2 deletions contracts/factory/src/storage.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use soroban_sdk::{contracttype, Address, ConversionError, Env, TryFromVal, Val, Vec};
use soroban_sdk::{contracttype, Address, BytesN, ConversionError, Env, TryFromVal, Val, Vec};

#[derive(Clone, Copy)]
#[repr(u32)]
@@ -28,7 +28,9 @@ impl TryFromVal<Env, DataKey> for Val {
pub struct Config {
pub admin: Address,
pub multihop_address: Address,
/// A vec of whitelisted addresses that can create liquidity pools
pub lp_wasm_hash: BytesN<32>,
pub stake_wasm_hash: BytesN<32>,
pub token_wasm_hash: BytesN<32>,
pub whitelisted_accounts: Vec<Address>,
}

30 changes: 25 additions & 5 deletions contracts/factory/src/tests.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::contract::{Factory, FactoryClient};
use soroban_sdk::{testutils::Address as _, vec, Address, Env};

use self::setup::install_multihop_wasm;
use self::setup::{
deploy_factory_contract, install_lp_contract, install_multihop_wasm, install_stake_wasm,
install_token_wasm,
};

mod config;
mod setup;
@@ -17,9 +19,27 @@ fn test_deploy_factory_twice_should_fail() {
let admin = Address::generate(&env);

let auth_user = Address::generate(&env);
let multihop = FactoryClient::new(&env, &env.register_contract(None, Factory {}));
let multihop_wasm_hash = install_multihop_wasm(&env);
let lp_wasm_hash = install_lp_contract(&env);
let stake_wasm_hash = install_stake_wasm(&env);
let token_wasm_hash = install_token_wasm(&env);

multihop.initialize(&admin, &multihop_wasm_hash, &vec![&env, auth_user.clone()]);
multihop.initialize(&admin, &multihop_wasm_hash, &vec![&env, auth_user]);
let factory = deploy_factory_contract(&env, admin.clone());

factory.initialize(
&admin,
&multihop_wasm_hash,
&lp_wasm_hash,
&stake_wasm_hash,
&token_wasm_hash,
&vec![&env, auth_user.clone()],
);
factory.initialize(
&admin,
&multihop_wasm_hash,
&lp_wasm_hash,
&stake_wasm_hash,
&token_wasm_hash,
&vec![&env, auth_user.clone()],
);
}
15 changes: 1 addition & 14 deletions contracts/factory/src/tests/config.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use super::setup::{
deploy_factory_contract, install_lp_contract, install_stake_wasm, install_token_wasm,
lp_contract,
};
use super::setup::{deploy_factory_contract, lp_contract};
use phoenix::utils::{LiquidityPoolInitInfo, StakeInitInfo, TokenInitInfo};

use soroban_sdk::{
@@ -59,23 +56,18 @@ fn factory_successfully_inits_lp() {
assert_eq!(factory.get_admin(), admin);

let token_init_info = TokenInitInfo {
token_wasm_hash: install_token_wasm(&env),
token_a: token1,
token_b: token2,
};
let stake_init_info = StakeInitInfo {
stake_wasm_hash: install_stake_wasm(&env),
min_bond: 10i128,
max_distributions: 10u32,
min_reward: 5i128,
};

let lp_wasm_hash = install_lp_contract(&env);

let lp_init_info = LiquidityPoolInitInfo {
admin: admin.clone(),
fee_recipient: user.clone(),
lp_wasm_hash,
max_allowed_slippage_bps: 5_000,
max_allowed_spread_bps: 500,
share_token_decimals: 7,
@@ -135,23 +127,18 @@ fn factory_fails_to_init_lp_when_authorized_address_not_present() {
assert_eq!(factory.get_admin(), admin);

let token_init_info = TokenInitInfo {
token_wasm_hash: install_token_wasm(&env),
token_a: token1,
token_b: token2,
};
let stake_init_info = StakeInitInfo {
stake_wasm_hash: install_stake_wasm(&env),
min_bond: 10i128,
max_distributions: 10u32,
min_reward: 5i128,
};

let lp_wasm_hash = install_lp_contract(&env);

let lp_init_info = LiquidityPoolInitInfo {
admin,
fee_recipient: user.clone(),
lp_wasm_hash,
max_allowed_slippage_bps: 5_000,
max_allowed_spread_bps: 500,
share_token_decimals: 7,
Loading

0 comments on commit 0d8fa94

Please sign in to comment.