From 9ed103f113d13fa2dd2682dc3e0853aab62be65d Mon Sep 17 00:00:00 2001 From: larry <26318510+larry0x@users.noreply.github.com> Date: Sun, 30 Jul 2023 20:41:37 +0100 Subject: [PATCH] use `cosmwasm_std::Coins` over `HashMap` --- contracts/incentives/src/contract.rs | 67 +++++++++---------- .../incentives/tests/test_claim_rewards.rs | 10 ++- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/contracts/incentives/src/contract.rs b/contracts/incentives/src/contract.rs index 78969d0f9..4d8bd08c5 100644 --- a/contracts/incentives/src/contract.rs +++ b/contracts/incentives/src/contract.rs @@ -1,10 +1,8 @@ -use std::collections::HashMap; - #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - attr, coins, to_binary, Addr, BankMsg, Binary, Coin, CosmosMsg, Decimal, Deps, DepsMut, Env, - Event, MessageInfo, Order, Response, StdError, StdResult, Uint128, + attr, to_binary, Addr, BankMsg, Binary, Coin, Coins, Decimal, Deps, DepsMut, Env, Event, + MessageInfo, Order, Response, StdError, StdResult, Uint128, }; use cw_storage_plus::Bound; use mars_owner::{OwnerInit::SetInitialOwner, OwnerUpdate}; @@ -422,11 +420,11 @@ pub fn execute_claim_rewards( let red_bank_addr = query_red_bank_address(deps.as_ref())?; let user_addr = info.sender; - let mut response = Response::new(); - let base_event = Event::new("mars/incentives/claim_rewards") - .add_attribute("action", "claim_rewards") - .add_attribute("user", user_addr.to_string()); - let mut events = vec![base_event]; + let mut response = Response::new().add_event( + Event::new("mars/incentives/claim_rewards") + .add_attribute("action", "claim_rewards") + .add_attribute("user", user_addr.to_string()), + ); let asset_incentives = state::paginate_incentive_states( deps.storage, @@ -435,7 +433,7 @@ pub fn execute_claim_rewards( limit, )?; - let mut total_unclaimed_rewards: HashMap = HashMap::new(); + let mut total_unclaimed_rewards = Coins::default(); for ((collateral_denom, incentive_denom), _) in asset_incentives { let querier = deps.querier; @@ -456,25 +454,25 @@ pub fn execute_claim_rewards( &Uint128::zero(), )?; - total_unclaimed_rewards - .entry(incentive_denom) - .and_modify(|amount| *amount += unclaimed_rewards) - .or_insert(unclaimed_rewards); + total_unclaimed_rewards.add(Coin { + denom: incentive_denom, + amount: unclaimed_rewards, + })?; } - for (denom, amount) in total_unclaimed_rewards.iter() { - response = response.add_message(CosmosMsg::Bank(BankMsg::Send { - to_address: user_addr.to_string(), - amount: coins(amount.u128(), denom), - })); - events.push( - Event::new("mars/incentives/claim_rewards/claimed_reward") - .add_attribute("denom", denom) - .add_attribute("amount", *amount), - ); + if !total_unclaimed_rewards.is_empty() { + response = response + .add_event( + Event::new("mars/incentives/claim_rewards/claimed_rewards") + .add_attribute("coins", total_unclaimed_rewards.to_string()), + ) + .add_message(BankMsg::Send { + to_address: user_addr.into(), + amount: total_unclaimed_rewards.into(), + }); } - Ok(response.add_events(events)) + Ok(response) } pub fn execute_update_config( @@ -649,7 +647,7 @@ pub fn query_user_unclaimed_rewards( limit, )?; - let mut total_unclaimed_rewards: HashMap = HashMap::new(); + let mut total_unclaimed_rewards = Coins::default(); for ((collateral_denom, incentive_denom), _) in incentive_states { let unclaimed_rewards = compute_user_unclaimed_rewards( @@ -661,19 +659,14 @@ pub fn query_user_unclaimed_rewards( &collateral_denom, &incentive_denom, )?; - total_unclaimed_rewards - .entry(incentive_denom) - .and_modify(|amount| *amount += unclaimed_rewards) - .or_insert(unclaimed_rewards); + + total_unclaimed_rewards.add(Coin { + denom: incentive_denom, + amount: unclaimed_rewards, + })?; } - Ok(total_unclaimed_rewards - .into_iter() - .map(|(denom, amount)| Coin { - denom, - amount, - }) - .collect()) + Ok(total_unclaimed_rewards.into()) } fn query_red_bank_address(deps: Deps) -> StdResult { diff --git a/contracts/incentives/tests/test_claim_rewards.rs b/contracts/incentives/tests/test_claim_rewards.rs index 66c887556..e2162dfa6 100644 --- a/contracts/incentives/tests/test_claim_rewards.rs +++ b/contracts/incentives/tests/test_claim_rewards.rs @@ -223,10 +223,14 @@ fn execute_claim_rewards() { let res = execute(deps.as_mut(), env.clone(), info, msg).unwrap(); // query after execution gives 0 rewards + // + // NOTE: the query should return an empty array, instead of a non-empty array + // with a zero-amount coin! the latter is considered an invalid coins array + // and will result in error. let rewards_query_after = query_user_unclaimed_rewards(deps.as_ref(), env, String::from("user"), None, None, None) .unwrap(); - assert_eq!(rewards_query_after[0].amount, Uint128::zero()); + assert!(rewards_query_after.is_empty()); // ASSERT @@ -240,11 +244,11 @@ fn execute_claim_rewards() { assert_eq!( res.events[0].attributes, - vec![attr("action", "claim_rewards"), attr("user", "user"),] + vec![attr("action", "claim_rewards"), attr("user", "user")] ); assert_eq!( res.events[1].attributes, - vec![attr("denom", "umars"), attr("amount", expected_accrued_rewards),] + vec![attr("coins", format!("{expected_accrued_rewards}umars"))] ); // asset and zero incentives get updated, no_user does not let asset_incentive =