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

feat: add option to deactivate a campaign #114

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
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.

9 changes: 5 additions & 4 deletions contracts/airdrop/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ edition = "2021"
homepage = "https://nibiru.fi"
repository = "https://github.com/NibiruChain/cw-nibiru"
exclude = [
# Those files are rust-optimizer artifacts. You might want to commit them for convenience but they should not be part of the source code publication.
"contract.wasm",
"hash.txt",
# Those files are rust-optimizer artifacts. You might want to commit them for convenience but they should not be part of the source code publication.
"contract.wasm",
"hash.txt",
]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
Expand Down Expand Up @@ -52,4 +52,5 @@ cw2 = { workspace = true }
semver = "1"

[dev-dependencies]
anyhow = { workspace = true }
anyhow = { workspace = true }
serde-json-wasm = "1.0.0"
81 changes: 72 additions & 9 deletions contracts/airdrop/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@
}

let campaign = Campaign {
campaign_id: msg.campaign_id,
campaign_name: msg.campaign_name,
campaign_description: msg.campaign_description,
owner: info.sender.clone(),
managers: msg.managers,
unallocated_amount: coin.amount,
is_active: true,
};
CAMPAIGN.save(deps.storage, &campaign)?;

Expand Down Expand Up @@ -92,6 +92,7 @@
}
ExecuteMsg::Claim {} => claim(deps, env, info),
ExecuteMsg::Withdraw { amount } => withdraw(deps, env, info, amount),
ExecuteMsg::DeactivateCampaign {} => deactivate(deps, env, info),

Check warning on line 95 in contracts/airdrop/src/contract.rs

View check run for this annotation

Codecov / codecov/patch

contracts/airdrop/src/contract.rs#L95

Added line #L95 was not covered by tests
}
}

Expand All @@ -103,18 +104,20 @@
) -> Result<Response, StdError> {
let mut res = vec![];

let mut campaign = CAMPAIGN.load(deps.storage).map_err(|_| {
StdError::generic_err("Failed to load campaign data")
})?;
let mut campaign = CAMPAIGN
.load(deps.storage)
.map_err(|_| StdError::generic_err("Failed to load campaign data"))?;

if campaign.owner != info.sender
&& !campaign.managers.contains(&info.sender)
if campaign.owner != info.sender && !campaign.managers.contains(&info.sender)
{
return Err(StdError::generic_err("Unauthorized"));
}

for req in requests {
if !campaign.is_active {
return Err(StdError::generic_err("Campaign is not active"));
}

for req in requests {
if campaign.unallocated_amount < req.amount {
return Err(StdError::generic_err(
"Not enough funds in the campaign",
Expand Down Expand Up @@ -160,10 +163,15 @@
) -> Result<Response, StdError> {
let bond_denom = deps.querier.query_bonded_denom()?;

let campaign = CAMPAIGN.load(deps.storage)?;

if !campaign.is_active {
return Err(StdError::generic_err("Campaign is not active"));
}

match USER_REWARDS.may_load(deps.storage, info.sender.clone())? {
Some(user_reward) => {
USER_REWARDS
.remove(deps.storage, info.sender.clone());
USER_REWARDS.remove(deps.storage, info.sender.clone());

Ok(Response::new()
.add_attribute("method", "claim")
Expand All @@ -179,6 +187,37 @@
}
}

pub fn deactivate(
deps: DepsMut,
env: Env,
info: MessageInfo,
) -> Result<Response, StdError> {
let mut campaign = CAMPAIGN
.load(deps.storage)
.map_err(|_| StdError::generic_err("Failed to load campaign data"))?;

if campaign.owner != info.sender && !campaign.managers.contains(&info.sender)
{
return Err(StdError::generic_err("Unauthorized"));
}

if !campaign.is_active {
return Err(StdError::generic_err("Campaign is not active"));

Check warning on line 205 in contracts/airdrop/src/contract.rs

View check run for this annotation

Codecov / codecov/patch

contracts/airdrop/src/contract.rs#L205

Added line #L205 was not covered by tests
}
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous comment from the bot suggests that the deactivate function should not return an error if the campaign is already inactive. This change has not been addressed in the provided code. The function should be idempotent, meaning calling it on an already inactive campaign should not be an error but rather a no-op or return a success response.

- if !campaign.is_active {
-     return Err(StdError::generic_err("Campaign is not active"));
- }

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if !campaign.is_active {
return Err(StdError::generic_err("Campaign is not active"));
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of like the idempotency suggestion here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@k-yang, I'm glad to hear you're considering the idempotency suggestion. Here's how you can adjust the deactivate function to be idempotent:

pub fn deactivate(
    deps: DepsMut,
    env: Env,
    info: MessageInfo,
) -> Result<Response, StdError> {
    let mut campaign = CAMPAIGN
        .load(deps.storage)
        .map_err(|_| StdError::generic_err("Failed to load campaign data"))?;

    if campaign.owner != info.sender && !campaign.managers.contains(&info.sender) {
        return Err(StdError::generic_err("Unauthorized"));
    }

    // If the campaign is already inactive, we simply return a success response.
    if !campaign.is_active {
        return Ok(Response::new().add_attribute("method", "deactivate"));
    }

    campaign.is_active = false;
    CAMPAIGN.save(deps.storage, &campaign)?;

    let bond_denom = deps.querier.query_bonded_denom()?;
    let own_balance: Uint128 = deps
        .querier
        .query_balance(&env.contract.address, bond_denom.clone())
        .map_err(|_| StdError::generic_err("Failed to query contract balance"))?
        .amount;

    return withdraw(deps, env, info, own_balance);
}

This modification ensures that if the deactivate function is called on an already inactive campaign, it will not perform any state changes and will return a success response instead of an error.


campaign.is_active = false;
CAMPAIGN.save(deps.storage, &campaign)?;

let bond_denom = deps.querier.query_bonded_denom()?;
let own_balance: Uint128 = deps
.querier
.query_balance(&env.contract.address, bond_denom.clone())
.map_err(|_| StdError::generic_err("Failed to query contract balance"))?
.amount;

return withdraw(deps, env, info, own_balance);
}

pub fn withdraw(
deps: DepsMut,
env: Env,
Expand Down Expand Up @@ -213,6 +252,25 @@
}],
}));

// Update campaign unallocated amount
if amount > campaign.unallocated_amount {
CAMPAIGN.update(
deps.storage,
|mut campaign| -> StdResult<Campaign> {
campaign.unallocated_amount = Uint128::zero();
Ok(campaign)
},
)?;

Check warning on line 263 in contracts/airdrop/src/contract.rs

View check run for this annotation

Codecov / codecov/patch

contracts/airdrop/src/contract.rs#L257-L263

Added lines #L257 - L263 were not covered by tests
} else {
CAMPAIGN.update(
deps.storage,
|mut campaign| -> StdResult<Campaign> {
campaign.unallocated_amount -= amount;
Ok(campaign)
},
)?;
}

return Ok(res);
}

Expand Down Expand Up @@ -240,6 +298,11 @@
_env: Env,
user_address: Addr,
) -> StdResult<Binary> {
let campaign = CAMPAIGN.load(deps.storage)?;
if !campaign.is_active {
return Err(StdError::generic_err("Campaign is not active"));
}

match USER_REWARDS.load(deps.storage, user_address) {
Ok(user_reward) => return to_json_binary(&user_reward),
Err(_) => {
Expand Down
14 changes: 5 additions & 9 deletions contracts/airdrop/src/msg.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use cosmwasm_std::{Uint128, Addr};
use cosmwasm_std::{Addr, Uint128};
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
Expand All @@ -25,19 +25,15 @@ pub struct RewardUserResponse {
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
#[serde(rename_all = "snake_case")]
pub enum ExecuteMsg {
RewardUsers {
requests: Vec<RewardUserRequest>
},
RewardUsers { requests: Vec<RewardUserRequest> },
Claim {},
Withdraw {
amount: Uint128,
},
Withdraw { amount: Uint128 },
DeactivateCampaign {},
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
#[serde(rename_all = "snake_case")]
pub enum QueryMsg {
Campaign { },
Campaign {},
GetUserReward { user_address: Addr },
}

5 changes: 3 additions & 2 deletions contracts/airdrop/src/state.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
use cosmwasm_std::{Addr, Uint128};
use cw_storage_plus::{Map, Item};
use cw_storage_plus::{Item, Map};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
pub struct Campaign {
pub campaign_id: String,
pub campaign_name: String,
pub campaign_description: String,

pub unallocated_amount: Uint128,
pub owner: Addr,
pub managers: Vec<Addr>,

pub is_active: bool,
}

pub const CAMPAIGN: Item<Campaign> = Item::new("campaign");
Expand Down
104 changes: 90 additions & 14 deletions contracts/airdrop/src/tests/execute/reward_users.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use crate::contract::{instantiate, reward_users};
use crate::contract::{
claim, deactivate, instantiate, query_user_reward, reward_users,
};
use crate::msg::{InstantiateMsg, RewardUserRequest, RewardUserResponse};
use crate::state::{Campaign, CAMPAIGN, USER_REWARDS};
use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info};
use cosmwasm_std::{coins, from_json, Addr, Uint128, StdError};
use cosmwasm_std::{coins, from_json, Addr, StdError, Uint128};
use std::vec;

#[test]
Expand All @@ -18,7 +20,10 @@ fn test_reward_users_fully_allocated() {
campaign_id: "campaign_id".to_string(),
campaign_name: "campaign_name".to_string(),
campaign_description: "campaign_description".to_string(),
managers: vec![Addr::unchecked("manager1"), Addr::unchecked("manager2")],
managers: vec![
Addr::unchecked("manager1"),
Addr::unchecked("manager2"),
],
},
)
.unwrap();
Expand Down Expand Up @@ -66,10 +71,13 @@ fn test_reward_users_fully_allocated() {
Campaign {
owner: Addr::unchecked("owner"),
unallocated_amount: Uint128::zero(),
campaign_id: "campaign_id".to_string(),
campaign_name: "campaign_name".to_string(),
campaign_description: "campaign_description".to_string(),
managers: vec![Addr::unchecked("manager1"), Addr::unchecked("manager2")],
managers: vec![
Addr::unchecked("manager1"),
Addr::unchecked("manager2")
],
is_active: true,
}
);

Expand All @@ -88,7 +96,6 @@ fn test_reward_users_fully_allocated() {
);
}


#[test]
fn test_reward_users_as_manager() {
let mut deps = mock_dependencies();
Expand All @@ -102,7 +109,10 @@ fn test_reward_users_as_manager() {
campaign_id: "campaign_id".to_string(),
campaign_name: "campaign_name".to_string(),
campaign_description: "campaign_description".to_string(),
managers: vec![Addr::unchecked("manager1"), Addr::unchecked("manager2")],
managers: vec![
Addr::unchecked("manager1"),
Addr::unchecked("manager2"),
],
},
)
.unwrap();
Expand Down Expand Up @@ -150,10 +160,13 @@ fn test_reward_users_as_manager() {
Campaign {
owner: Addr::unchecked("owner"),
unallocated_amount: Uint128::zero(),
campaign_id: "campaign_id".to_string(),
campaign_name: "campaign_name".to_string(),
campaign_description: "campaign_description".to_string(),
managers: vec![Addr::unchecked("manager1"), Addr::unchecked("manager2")],
managers: vec![
Addr::unchecked("manager1"),
Addr::unchecked("manager2")
],
is_active: true,
}
);

Expand Down Expand Up @@ -185,7 +198,10 @@ fn test_fails_when_we_try_to_allocate_more_than_available() {
campaign_id: "campaign_id".to_string(),
campaign_name: "campaign_name".to_string(),
campaign_description: "campaign_description".to_string(),
managers: vec![Addr::unchecked("manager1"), Addr::unchecked("manager2")],
managers: vec![
Addr::unchecked("manager1"),
Addr::unchecked("manager2"),
],
},
)
.unwrap();
Expand All @@ -210,7 +226,67 @@ fn test_fails_when_we_try_to_allocate_more_than_available() {
],
);

assert_eq!(resp, Err(StdError::generic_err(
"Not enough funds in the campaign",
)));
}
assert_eq!(
resp,
Err(StdError::generic_err("Not enough funds in the campaign",))
);
}

#[test]
fn test_fails_we_allocate_inactive() {
let mut deps = mock_dependencies();
let env = mock_env();

instantiate(
deps.as_mut(),
env.clone(),
mock_info("owner", &coins(1000, "")),
InstantiateMsg {
campaign_id: "campaign_id".to_string(),
campaign_name: "campaign_name".to_string(),
campaign_description: "campaign_description".to_string(),
managers: vec![
Addr::unchecked("manager1"),
Addr::unchecked("manager2"),
],
},
)
.unwrap();

reward_users(
deps.as_mut(),
env.clone(),
mock_info("manager1", &[]),
vec![RewardUserRequest {
user_address: Addr::unchecked("user1"),
amount: Uint128::new(1),
}],
)
.unwrap();

// deactivate campaign -- fail because not owner
let resp = deactivate(deps.as_mut(), env.clone(), mock_info("user1", &[]));
assert_eq!(resp, Err(StdError::generic_err("Unauthorized")));

deactivate(deps.as_mut(), env.clone(), mock_info("owner", &[])).unwrap();

let resp = reward_users(
deps.as_mut(),
env.clone(),
mock_info("manager1", &[]),
vec![RewardUserRequest {
user_address: Addr::unchecked("user2"),
amount: Uint128::new(1),
}],
);
assert_eq!(resp, Err(StdError::generic_err("Campaign is not active",)));

// user1 should not be able to claim anymore
let resp = claim(deps.as_mut(), env.clone(), mock_info("user1", &[]));
assert_eq!(resp, Err(StdError::generic_err("Campaign is not active")));

// user1 query reward says campaign is not active
let resp =
query_user_reward(deps.as_ref(), env.clone(), Addr::unchecked("user1"));
assert_eq!(resp, Err(StdError::generic_err("Campaign is not active")));
}
Loading