Skip to content

Commit

Permalink
Revert custom token id creation.
Browse files Browse the repository at this point in the history
  • Loading branch information
piobab committed Aug 12, 2024
1 parent c106b90 commit 34c6454
Show file tree
Hide file tree
Showing 30 changed files with 48 additions and 573 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

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

1 change: 0 additions & 1 deletion contracts/account-nft/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,4 @@ cw-multi-test = { workspace = true }
mars-mock-credit-manager = { workspace = true }
mars-mock-rover-health = { workspace = true }
mars-owner = { workspace = true }
proptest = { workspace = true }
serde_json = { workspace = true }
3 changes: 1 addition & 2 deletions contracts/account-nft/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ pub fn execute(
match msg {
ExecuteMsg::Mint {
user,
token_id,
} => mint(deps, info, &user, token_id),
} => mint(deps, info, &user),
ExecuteMsg::UpdateConfig {
updates,
} => update_config(deps, info, updates),
Expand Down
5 changes: 0 additions & 5 deletions contracts/account-nft/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,4 @@ pub enum ContractError {

#[error("{0}")]
Version(#[from] cw2::VersionError),

#[error("Invalid token_id: {reason}")]
InvalidTokenId {
reason: String,
},
}
20 changes: 4 additions & 16 deletions contracts/account-nft/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,14 @@ use crate::{
error::ContractError::{
self, BaseError, BurnNotAllowed, CreditManagerContractNotSet, HealthContractNotSet,
},
helpers::validate_token_id,
state::{CONFIG, NEXT_ID},
};

pub fn mint(
deps: DepsMut,
info: MessageInfo,
user: &str,
token_id: Option<String>,
) -> Result<Response, ContractError> {
let next_id = if let Some(ti) = token_id {
validate_token_id(&ti)?;
ti
} else {
let next_id = NEXT_ID.load(deps.storage)?;
NEXT_ID.save(deps.storage, &(next_id + 1))?;
next_id.to_string()
};
pub fn mint(deps: DepsMut, info: MessageInfo, user: &str) -> Result<Response, ContractError> {
let next_id = NEXT_ID.load(deps.storage)?;
NEXT_ID.save(deps.storage, &(next_id + 1))?;
Parent::default()
.mint(deps, info, next_id, user.to_string(), None, Empty {})
.mint(deps, info, next_id.to_string(), user.to_string(), None, Empty {})
.map_err(Into::into)
}

Expand Down
37 changes: 0 additions & 37 deletions contracts/account-nft/src/helpers.rs

This file was deleted.

1 change: 0 additions & 1 deletion contracts/account-nft/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
pub mod contract;
pub mod error;
pub mod execute;
mod helpers;
pub mod migrations;
pub mod query;
pub mod state;
9 changes: 0 additions & 9 deletions contracts/account-nft/tests/tests/helpers/mock_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,20 +122,11 @@ impl MockEnv {
}

pub fn mint(&mut self, token_owner: &Addr) -> AnyResult<String> {
self.mint_with_custom_token_id(token_owner, None)
}

pub fn mint_with_custom_token_id(
&mut self,
token_owner: &Addr,
token_id: Option<String>,
) -> AnyResult<String> {
let res = self.app.execute_contract(
self.minter.clone(),
self.nft_contract.clone(),
&ExecuteMsg::Mint {
user: token_owner.into(),
token_id,
},
&[],
)?;
Expand Down
77 changes: 0 additions & 77 deletions contracts/account-nft/tests/tests/test_mint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use mars_types::{
account_nft::{ExecuteMsg, QueryMsg::OwnerOf},
health::AccountKind,
};
use proptest::prelude::*;

use super::helpers::{below_max_for_burn, MockEnv};

Expand Down Expand Up @@ -71,7 +70,6 @@ fn only_minter_can_mint() {
mock.nft_contract.clone(),
&ExecuteMsg::Mint {
user: bad_guy.into(),
token_id: None,
},
&[],
);
Expand Down Expand Up @@ -106,78 +104,3 @@ fn normal_base_cw721_actions_can_still_be_taken() {
.unwrap();
assert_eq!(res.owner, rover_user_b.to_string())
}

#[test]
fn invalid_custom_token_id_length() {
let mut mock = MockEnv::new().build().unwrap();
mock.assert_next_id("1");

let user = Addr::unchecked("user_abc");

let res = mock.mint_with_custom_token_id(&user, Some("abc".to_string()));
let err: ContractError = res.unwrap_err().downcast().unwrap();
assert_eq!(
err,
ContractError::InvalidTokenId {
reason: "token_id length should be between 4 and 15 chars".to_string()
}
);
mock.assert_next_id("1");

let res = mock.mint_with_custom_token_id(&user, Some("abcdefghijklmnop".to_string()));
let err: ContractError = res.unwrap_err().downcast().unwrap();
assert_eq!(
err,
ContractError::InvalidTokenId {
reason: "token_id length should be between 4 and 15 chars".to_string()
}
);
mock.assert_next_id("1");
}

#[test]
fn custom_token_id_can_not_be_same_as_automatically_generated() {
let mut mock = MockEnv::new().build().unwrap();
mock.assert_next_id("1");

let user = Addr::unchecked("user_abc");

let res = mock.mint_with_custom_token_id(&user, Some("12345".to_string()));
let err: ContractError = res.unwrap_err().downcast().unwrap();
assert_eq!(
err,
ContractError::InvalidTokenId {
reason: "token_id should contain at least one letter".to_string()
}
);
mock.assert_next_id("1");
}

proptest! {
#[test]
fn invalid_custom_token_id_characters(token_id in "[!@#$%^&*()-+]{4,15}") {
let mut mock = MockEnv::new().build().unwrap();
mock.assert_next_id("1");

let user = Addr::unchecked("user_abc");
let res = mock.mint_with_custom_token_id(&user, Some(token_id));
let err: ContractError = res.unwrap_err().downcast().unwrap();
prop_assert_eq!(err, ContractError::InvalidTokenId { reason: "token_id can contain only letters, numbers, and underscores".to_string() });
mock.assert_next_id("1");
}

/// The regex pattern ensures that the string:
/// - starts with three characters (letters, digits, or underscores),
/// - contains at least one letter,
/// - can have up to 11 additional characters (letters, digits, or underscores) to fulfill the length requirement.
#[test]
fn valid_custom_token_id(token_id in "[a-zA-Z0-9_]{3}[a-zA-Z][a-zA-Z0-9_]{0,11}") {
let mut mock = MockEnv::new().build().unwrap();
mock.assert_next_id("1");

let user = Addr::unchecked("user_abc");
let saved_token_id = mock.mint_with_custom_token_id(&user, Some(token_id.clone())).unwrap();
assert_eq!(saved_token_id, token_id);
mock.assert_next_id("1");
}
}
6 changes: 1 addition & 5 deletions contracts/credit-manager/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,8 @@ pub fn execute(
) -> ContractResult<Response> {
match msg {
ExecuteMsg::CreateCreditAccount(kind) => {
create_credit_account(&mut deps, info.sender, kind, None).map(|res| res.1)
create_credit_account(&mut deps, info.sender, kind).map(|res| res.1)
}
ExecuteMsg::CreateCreditAccountV2 {
kind,
account_id,
} => create_credit_account(&mut deps, info.sender, kind, account_id).map(|res| res.1),
ExecuteMsg::UpdateConfig {
updates,
} => update_config(deps, env, info, updates),
Expand Down
13 changes: 3 additions & 10 deletions contracts/credit-manager/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,18 @@ pub fn create_credit_account(
deps: &mut DepsMut,
user: Addr,
kind: AccountKind,
account_id: Option<String>,
) -> ContractResult<(String, Response)> {
let account_nft = ACCOUNT_NFT.load(deps.storage)?;

let next_id = if let Some(ai) = account_id.clone() {
ai
} else {
account_nft.query_next_id(&deps.querier)?
};
let next_id = account_nft.query_next_id(&deps.querier)?;

let mut msgs = vec![];

let nft_mint_msg = CosmosMsg::Wasm(WasmMsg::Execute {
contract_addr: account_nft.address().into(),
funds: vec![],
msg: to_json_binary(&NftExecuteMsg::Mint {
user: user.to_string(),
token_id: account_id,
})?,
});
msgs.push(nft_mint_msg);
Expand Down Expand Up @@ -92,8 +87,7 @@ pub fn create_credit_account(
let response = Response::new()
.add_messages(msgs)
.add_attribute("action", "create_credit_account")
.add_attribute("kind", kind.to_string())
.add_attribute("account_id", next_id.clone());
.add_attribute("kind", kind.to_string());

Ok((next_id, response))
}
Expand All @@ -118,7 +112,6 @@ pub fn dispatch_actions(
&mut deps,
info.sender.clone(),
account_kind.unwrap_or(AccountKind::Default),
None,
)?;
response = res;
acc_id
Expand Down
2 changes: 1 addition & 1 deletion contracts/credit-manager/src/update_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub fn update_config(
)?;

let (_, res) =
create_credit_account(&mut deps, rewards_collector_addr, AccountKind::Default, None)?;
create_credit_account(&mut deps, rewards_collector_addr, AccountKind::Default)?;

response = response
.add_submessages(res.messages)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
use cosmwasm_std::{coin, Addr, Empty};
use cosmwasm_std::{Addr, Empty};
use cw721::OwnerOfResponse;
use cw721_base::QueryMsg as NftQueryMsg;
use mars_types::health::AccountKind;
use test_case::test_case;

use super::helpers::MockEnv;
use crate::tests::helpers::{deploy_managed_vault, AccountToFund};

#[test]
fn create_credit_account_fails_without_nft_contract_set() {
Expand Down Expand Up @@ -69,78 +67,3 @@ fn after_create_returns_account_kind() {
assert_eq!(position_1.account_kind, AccountKind::Default);
assert_eq!(position_2.account_kind, AccountKind::HighLeveredStrategy);
}

#[test_case("Mars_default", AccountKind::Default; "create Default account")]
#[test_case("Mars_HLS", AccountKind::HighLeveredStrategy; "create HLS account")]
fn create_credit_account_v2(custom_account_id: &str, account_kind: AccountKind) {
let mut mock = MockEnv::new().build().unwrap();

// Double checking ownership by querying NFT account-nft for correct owner
let config = mock.query_config();

let user = Addr::unchecked("user_123");
let account_id = mock
.create_credit_account_v2(&user, account_kind.clone(), Some(custom_account_id.to_string()))
.unwrap();
assert_eq!(account_id, custom_account_id.to_string());

let owner_res: OwnerOfResponse = mock
.app
.wrap()
.query_wasm_smart(
config.account_nft.clone().unwrap(),
&NftQueryMsg::<Empty>::OwnerOf {
token_id: account_id.clone(),
include_expired: None,
},
)
.unwrap();

assert_eq!(user, owner_res.owner);
let position = mock.query_positions(&account_id);
assert_eq!(position.account_kind, account_kind);
}

#[test]
fn create_fund_manager_credit_account_v2() {
let custom_account_id = "Mars_Vault";
let user = Addr::unchecked("user_123");

let mut mock = MockEnv::new()
.fund_account(AccountToFund {
addr: user.clone(),
funds: vec![coin(1_000_000_000, "untrn")],
})
.build()
.unwrap();

// Double checking ownership by querying NFT account-nft for correct owner
let config = mock.query_config();

let credit_manager = mock.rover.clone();
let managed_vault_addr = deploy_managed_vault(&mut mock.app, &user, &credit_manager);

let account_kind = AccountKind::FundManager {
vault_addr: managed_vault_addr.to_string(),
};
let account_id = mock
.create_credit_account_v2(&user, account_kind.clone(), Some(custom_account_id.to_string()))
.unwrap();
assert_eq!(account_id, custom_account_id.to_string());

let owner_res: OwnerOfResponse = mock
.app
.wrap()
.query_wasm_smart(
config.account_nft.clone().unwrap(),
&NftQueryMsg::<Empty>::OwnerOf {
token_id: account_id.clone(),
include_expired: None,
},
)
.unwrap();

assert_eq!(user, owner_res.owner);
let position = mock.query_positions(&account_id);
assert_eq!(position.account_kind, account_kind);
}
Loading

0 comments on commit 34c6454

Please sign in to comment.