Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issues oak audit no 16 #417

Merged
merged 4 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion contracts/credit-manager/src/stake_astro_lp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use mars_types::{
};

use crate::{
error::ContractResult,
error::{ContractError, ContractResult},
state::{COIN_BALANCES, INCENTIVES},
utils::{decrement_coin_balance, increment_coin_balance},
};
Expand All @@ -27,6 +27,10 @@ pub fn stake_lp(deps: DepsMut, account_id: &str, lp_coin: ActionCoin) -> Contrac
ActionAmount::AccountBalance => coin_balance,
};

if new_amount.is_zero() {
return Err(ContractError::NoAmount);
};

let updated_coin = coin(new_amount.u128(), lp_coin.denom.as_str());

decrement_coin_balance(deps.storage, account_id, &updated_coin)?;
Expand Down
23 changes: 23 additions & 0 deletions contracts/credit-manager/tests/tests/test_stake_astro_lp.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use cosmwasm_std::{coins, Addr, Coin, Uint128};
use mars_credit_manager::error::ContractError;
use mars_testing::multitest::helpers::AccountToFund;
use mars_types::credit_manager::{Action, ActionAmount, ActionCoin};

Expand Down Expand Up @@ -28,6 +29,28 @@ fn stake_claims_rewards() {
amount: Uint128::new(200),
};

// should fail when trying to stake 0 amount
let err: ContractError = mock
.update_credit_account(
&account_id,
&user,
vec![
Action::Deposit(lp_coin.clone()),
Action::StakeAstroLp {
lp_token: ActionCoin {
denom: lp_denom.to_string(),
amount: ActionAmount::Exact(Uint128::new(0)),
},
},
],
&[lp_coin.clone()],
)
.unwrap_err()
.downcast()
.unwrap();

assert_eq!(err, ContractError::NoAmount);

// stake
mock.update_credit_account(
&account_id,
Expand Down
1 change: 1 addition & 0 deletions contracts/vault/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ cw-paginate = { workspace = true }
cw-vault-standard = { workspace = true }
mars-owner = { workspace = true }
mars-types = { workspace = true }
mars-utils = { workspace = true }
thiserror = { workspace = true }
osmosis-std = { workspace = true }

Expand Down
7 changes: 7 additions & 0 deletions contracts/vault/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use cosmwasm_std::{
};
use cw_utils::PaymentError;
use mars_owner::OwnerError;
use mars_utils::error::ValidationError;

#[derive(Debug, PartialEq, thiserror::Error)]
pub enum ContractError {
Expand Down Expand Up @@ -34,6 +35,9 @@ pub enum ContractError {
#[error(transparent)]
Payment(#[from] PaymentError),

#[error(transparent)]
Validation(#[from] ValidationError),

#[error("{0}")]
Generic(String),

Expand Down Expand Up @@ -71,6 +75,9 @@ pub enum ContractError {

#[error("Withdrawal interval not passed")]
WithdrawalIntervalNotPassed {},

#[error("Invalid cooldown period, expected value greater than 0")]
ZeroCooldownPeriod,
}

pub type ContractResult<T> = Result<T, ContractError>;
Expand Down
9 changes: 8 additions & 1 deletion contracts/vault/src/instantiate.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use cosmwasm_std::{DepsMut, Env, MessageInfo, Response};
use mars_owner::OwnerInit;
use mars_utils::helpers::validate_native_denom;

use crate::{
error::ContractResult,
error::{ContractError, ContractResult},
msg::InstantiateMsg,
performance_fee::PerformanceFeeState,
state::{
Expand Down Expand Up @@ -42,6 +43,10 @@ pub fn init(
DESCRIPTION.save(deps.storage, &desc)?;
}

if msg.cooldown_period == 0 {
return Err(ContractError::ZeroCooldownPeriod {});
}

COOLDOWN_PERIOD.save(deps.storage, &msg.cooldown_period)?;

// initialize performance fee state
Expand All @@ -53,6 +58,8 @@ pub fn init(
let vault_token =
TokenFactoryDenom::new(env.contract.address.to_string(), msg.vault_token_subdenom);
VAULT_TOKEN.save(deps.storage, &vault_token)?;

validate_native_denom(&msg.base_token)?;
BASE_TOKEN.save(deps.storage, &msg.base_token)?;

Ok(vault_token.instantiate()?)
Expand Down
37 changes: 37 additions & 0 deletions contracts/vault/tests/tests/test_instantiate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,43 @@ fn cannot_instantiate_with_invalid_performance_fee() {
);
}

#[test]
fn cannot_instantiate_with_zero_cooldown_period() {
piobab marked this conversation as resolved.
Show resolved Hide resolved
let fund_manager = Addr::unchecked("fund-manager");
let mut mock = MockEnv::new()
.fund_account(AccountToFund {
addr: fund_manager.clone(),
funds: vec![coin(1_000_000_000, "untrn")],
})
.build()
.unwrap();
let credit_manager = mock.rover.clone();

let contract_code_id = mock.app.store_code(mock_managed_vault_contract());
let res = mock.app.instantiate_contract(
contract_code_id,
fund_manager,
&InstantiateMsg {
base_token: "uusdc".to_string(),
vault_token_subdenom: "fund".to_string(),
title: None,
subtitle: None,
description: None,
credit_manager: credit_manager.to_string(),
cooldown_period: 0,
performance_fee_config: PerformanceFeeConfig {
fee_rate: Decimal::from_str("0.000046287042457350").unwrap(),
withdrawal_interval: 1563,
},
},
&[coin(10_000_000, "untrn")], // Token Factory fee for minting new denom. Configured in the Token Factory module in `mars-testing` package.
"mock-managed-vault",
None,
);

assert_vault_err(res, ContractError::ZeroCooldownPeriod {});
}

fn assert_vault_err(res: AnyResult<Addr>, err: ContractError) {
match res {
Ok(_) => panic!("Result was not an error"),
Expand Down
Loading