-
Notifications
You must be signed in to change notification settings - Fork 22
feat: Add basic support for multiple incentive denoms #199
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
Changes from all commits
9dc8745
9c23e56
913d040
a6d69d5
ab4259e
11296cd
0521c6b
87571d3
a8aa803
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
use std::cmp::{max, min}; | ||
|
||
use cosmwasm_std::{ | ||
Addr, BlockInfo, Decimal, Deps, Order, OverflowError, OverflowOperation, StdError, StdResult, | ||
Uint128, | ||
Addr, BlockInfo, Decimal, Deps, OverflowError, OverflowOperation, StdError, StdResult, Uint128, | ||
}; | ||
use mars_red_bank_types::{incentives::AssetIncentive, red_bank}; | ||
|
||
|
@@ -78,8 +77,6 @@ pub fn compute_user_accrued_rewards( | |
/// Result of querying and updating the status of the user and a give asset incentives in order to | ||
/// compute unclaimed rewards. | ||
pub struct UserAssetIncentiveStatus { | ||
/// Denom of the asset that's the incentives target | ||
pub denom: String, | ||
/// Current user index's value on the contract store (not updated by current asset index) | ||
pub user_index_current: Decimal, | ||
/// Asset incentive with values updated to the current block (not neccesarily commited | ||
|
@@ -92,64 +89,62 @@ pub fn compute_user_unclaimed_rewards( | |
block: &BlockInfo, | ||
red_bank_addr: &Addr, | ||
user_addr: &Addr, | ||
) -> StdResult<(Uint128, Vec<UserAssetIncentiveStatus>)> { | ||
let mut total_unclaimed_rewards = | ||
USER_UNCLAIMED_REWARDS.may_load(deps.storage, user_addr)?.unwrap_or_else(Uint128::zero); | ||
|
||
let result_asset_incentives: StdResult<Vec<_>> = | ||
ASSET_INCENTIVES.range(deps.storage, None, None, Order::Ascending).collect(); | ||
|
||
let mut user_asset_incentive_statuses_to_update: Vec<UserAssetIncentiveStatus> = vec![]; | ||
|
||
for (denom, mut asset_incentive) in result_asset_incentives? { | ||
// Get asset user balances and total supply | ||
let collateral: red_bank::UserCollateralResponse = deps.querier.query_wasm_smart( | ||
red_bank_addr, | ||
&red_bank::QueryMsg::UserCollateral { | ||
user: user_addr.to_string(), | ||
denom: denom.clone(), | ||
}, | ||
)?; | ||
let market: red_bank::Market = deps.querier.query_wasm_smart( | ||
red_bank_addr, | ||
&red_bank::QueryMsg::Market { | ||
denom: denom.clone(), | ||
}, | ||
)?; | ||
|
||
// If user's balance is 0 there should be no rewards to accrue, so we don't care about | ||
// updating indexes. If the user's balance changes, the indexes will be updated correctly at | ||
// that point in time. | ||
if collateral.amount_scaled.is_zero() { | ||
continue; | ||
} | ||
collateral_denom: &str, | ||
incentive_denom: &str, | ||
) -> StdResult<(Uint128, Option<UserAssetIncentiveStatus>)> { | ||
let mut unclaimed_rewards = USER_UNCLAIMED_REWARDS | ||
.may_load(deps.storage, (user_addr, collateral_denom, incentive_denom))? | ||
.unwrap_or_else(Uint128::zero); | ||
|
||
let mut asset_incentive = | ||
ASSET_INCENTIVES.load(deps.storage, (collateral_denom, incentive_denom))?; //TODO: Use may_load or handle error | ||
|
||
// Get asset user balances and total supply | ||
let collateral: red_bank::UserCollateralResponse = deps.querier.query_wasm_smart( | ||
red_bank_addr, | ||
&red_bank::QueryMsg::UserCollateral { | ||
user: user_addr.to_string(), | ||
denom: collateral_denom.to_string(), | ||
}, | ||
)?; | ||
let market: red_bank::Market = deps.querier.query_wasm_smart( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm analyzing this now and if incentives is set for market which is not set in redbank it could fail the whole tx. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Should we verify that market exists in Red Bank in ExecuteMsg::SetIncentive? It's not possible to remove a market in red bank is it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it is not possible to remove. IMO we should verify in SetIncentive that collateral exists in Red Bank. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
red_bank_addr, | ||
&red_bank::QueryMsg::Market { | ||
denom: collateral_denom.to_string(), | ||
}, | ||
)?; | ||
|
||
// If user's balance is 0 there should be no rewards to accrue, so we don't care about | ||
// updating indexes. If the user's balance changes, the indexes will be updated correctly at | ||
// that point in time. | ||
if collateral.amount_scaled.is_zero() { | ||
return Ok((unclaimed_rewards, None)); | ||
} | ||
|
||
update_asset_incentive_index( | ||
&mut asset_incentive, | ||
market.collateral_total_scaled, | ||
block.time.seconds(), | ||
update_asset_incentive_index( | ||
&mut asset_incentive, | ||
market.collateral_total_scaled, | ||
block.time.seconds(), | ||
)?; | ||
|
||
let user_asset_index = USER_ASSET_INDICES | ||
.may_load(deps.storage, (user_addr, collateral_denom, incentive_denom))? | ||
.unwrap_or_else(Decimal::zero); | ||
|
||
if user_asset_index != asset_incentive.index { | ||
// Compute user accrued rewards and update user index | ||
let asset_accrued_rewards = compute_user_accrued_rewards( | ||
collateral.amount_scaled, | ||
user_asset_index, | ||
asset_incentive.index, | ||
)?; | ||
|
||
let user_asset_index = USER_ASSET_INDICES | ||
.may_load(deps.storage, (user_addr, &denom))? | ||
.unwrap_or_else(Decimal::zero); | ||
|
||
if user_asset_index != asset_incentive.index { | ||
// Compute user accrued rewards and update user index | ||
let asset_accrued_rewards = compute_user_accrued_rewards( | ||
collateral.amount_scaled, | ||
user_asset_index, | ||
asset_incentive.index, | ||
)?; | ||
total_unclaimed_rewards += asset_accrued_rewards; | ||
} | ||
|
||
user_asset_incentive_statuses_to_update.push(UserAssetIncentiveStatus { | ||
denom, | ||
user_index_current: user_asset_index, | ||
asset_incentive_updated: asset_incentive, | ||
}); | ||
unclaimed_rewards += asset_accrued_rewards; | ||
} | ||
|
||
Ok((total_unclaimed_rewards, user_asset_incentive_statuses_to_update)) | ||
let user_asset_incentive_status_to_update = UserAssetIncentiveStatus { | ||
user_index_current: user_asset_index, | ||
asset_incentive_updated: asset_incentive, | ||
}; | ||
|
||
Ok((unclaimed_rewards, Some(user_asset_incentive_status_to_update))) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,141 @@ | ||
use cosmwasm_std::{Addr, Decimal, Uint128}; | ||
use cw_storage_plus::{Item, Map}; | ||
use cosmwasm_std::{Addr, Decimal, Order, StdResult, Storage, Uint128}; | ||
use cw_storage_plus::{Bound, Item, Map, PrefixBound}; | ||
use mars_owner::Owner; | ||
use mars_red_bank_types::incentives::{AssetIncentive, Config}; | ||
|
||
// keys (for singleton) | ||
use crate::ContractError; | ||
|
||
/// The owner of the contract | ||
pub const OWNER: Owner = Owner::new("owner"); | ||
|
||
/// The configuration of the contract | ||
pub const CONFIG: Item<Config> = Item::new("config"); | ||
|
||
// namespaces (for buckets) | ||
pub const ASSET_INCENTIVES: Map<&str, AssetIncentive> = Map::new("incentives"); | ||
pub const USER_ASSET_INDICES: Map<(&Addr, &str), Decimal> = Map::new("indices"); | ||
pub const USER_UNCLAIMED_REWARDS: Map<&Addr, Uint128> = Map::new("unclaimed_rewards"); | ||
/// A map containing a configuration of an incentive for a given collateral and incentive denom. | ||
/// The key is (collateral denom, incentive denom). | ||
pub const ASSET_INCENTIVES: Map<(&str, &str), AssetIncentive> = Map::new("incentives"); | ||
|
||
/// A map containing the incentive index for a given user, collateral denom and incentive denom. | ||
/// The key is (user address, collateral denom, incentive denom). | ||
pub const USER_ASSET_INDICES: Map<(&Addr, &str, &str), Decimal> = Map::new("indices"); | ||
|
||
/// A map containing the amount of unclaimed incentives for a given user and incentive denom. | ||
/// The key is (user address, collateral denom, incentive denom). | ||
pub const USER_UNCLAIMED_REWARDS: Map<(&Addr, &str, &str), Uint128> = Map::new("unclaimed_rewards"); | ||
|
||
/// The default limit for pagination over asset incentives | ||
pub const DEFAULT_LIMIT: u32 = 5; | ||
|
||
/// The maximum limit for pagination over asset incentives | ||
/// TODO: Remove MAX_LIMIT? What is the purpose? Surely better to have the limit be whatever is the max gas limit? | ||
pub const MAX_LIMIT: u32 = 10; | ||
|
||
/// Helper function to update unclaimed rewards for a given user, collateral denom and incentive | ||
/// denom. Adds `accrued_rewards` to the existing amount. | ||
pub fn increase_unclaimed_rewards( | ||
storage: &mut dyn Storage, | ||
user_addr: &Addr, | ||
collateral_denom: &str, | ||
incentive_denom: &str, | ||
accrued_rewards: Uint128, | ||
) -> StdResult<()> { | ||
USER_UNCLAIMED_REWARDS.update( | ||
storage, | ||
(user_addr, collateral_denom, incentive_denom), | ||
|ur: Option<Uint128>| -> StdResult<Uint128> { | ||
Ok(ur.map_or_else(|| accrued_rewards, |r| r + accrued_rewards)) | ||
}, | ||
)?; | ||
Ok(()) | ||
} | ||
|
||
/// Returns asset incentives, with optional pagination. | ||
/// Caller should make sure that if start_after_incentive_denom is supplied, then | ||
/// start_after_collateral_denom is also supplied. | ||
pub fn paginate_asset_incentives( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is quite complicated to follow.. What if we simplify an API to paginate incentives per requested collateral? This way we give control to the caller and he can prepare separate msgs to claim rewards. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess really it's up to frontend devs what they prefer. The downside of your suggestion is that they would need to know all collaterals that the user has incentives for. I could imagine that this is hard to keep track of. They can't just query Red Bank since the user can remove collateral and still have unclaimed rewards. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @piobab what do you think? Personally I think current method is better so backend doesn't need to keep track of state |
||
storage: &dyn Storage, | ||
start_after_collateral_denom: Option<String>, | ||
start_after_incentive_denom: Option<String>, | ||
limit: Option<u32>, | ||
) -> Result<Vec<((String, String), AssetIncentive)>, ContractError> { | ||
let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; | ||
Ok(match (start_after_collateral_denom.as_ref(), start_after_incentive_denom.as_ref()) { | ||
(Some(collat_denom), Some(incen_denom)) => { | ||
let start = Bound::exclusive((collat_denom.as_str(), incen_denom.as_str())); | ||
ASSET_INCENTIVES.range(storage, Some(start), None, Order::Ascending) | ||
} | ||
(Some(collat_denom), None) => { | ||
let start = PrefixBound::exclusive(collat_denom.as_str()); | ||
ASSET_INCENTIVES.prefix_range(storage, Some(start), None, Order::Ascending) | ||
} | ||
(None, Some(_)) => return Err(ContractError::InvalidPaginationParams), | ||
_ => ASSET_INCENTIVES.range(storage, None, None, Order::Ascending), | ||
} | ||
.take(limit) | ||
.collect::<StdResult<Vec<_>>>()?) | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use cosmwasm_std::testing::MockStorage; | ||
|
||
use super::*; | ||
|
||
#[test] | ||
fn paginate_asset_incentives_works() { | ||
let mut storage = MockStorage::new(); | ||
|
||
//store some incentives | ||
let asset_incentive = AssetIncentive { | ||
duration: 0, | ||
emission_per_second: Uint128::zero(), | ||
index: Decimal::zero(), | ||
last_updated: 0, | ||
start_time: 0, | ||
}; | ||
let incentives = vec![ | ||
(("collat1".to_string(), "incen1".to_string()), asset_incentive.clone()), | ||
(("collat1".to_string(), "incen2".to_string()), asset_incentive.clone()), | ||
(("collat2".to_string(), "incen1".to_string()), asset_incentive.clone()), | ||
(("collat2".to_string(), "incen2".to_string()), asset_incentive.clone()), | ||
]; | ||
for ((collat, incen), incentive) in incentives.iter() { | ||
ASSET_INCENTIVES | ||
.save(&mut storage, (collat.as_str(), incen.as_str()), &incentive) | ||
.unwrap(); | ||
} | ||
|
||
// No pagination | ||
let res = paginate_asset_incentives(&storage, None, None, None).unwrap(); | ||
assert_eq!(res, incentives); | ||
|
||
// Start after collateral denom | ||
let res = | ||
paginate_asset_incentives(&storage, Some("collat1".to_string()), None, None).unwrap(); | ||
println!("start after collat1: {:?}", res); | ||
println!("expected: {:?}", incentives[2..].to_vec()); | ||
assert_eq!(res, incentives[2..]); | ||
|
||
// Start after collateral denom and incentive denom | ||
let res = paginate_asset_incentives( | ||
&storage, | ||
Some("collat1".to_string()), | ||
Some("incen1".to_string()), | ||
None, | ||
) | ||
.unwrap(); | ||
assert_eq!(res, incentives[1..]); | ||
let res = paginate_asset_incentives( | ||
&storage, | ||
Some("collat1".to_string()), | ||
Some("incen2".to_string()), | ||
None, | ||
) | ||
.unwrap(); | ||
assert_eq!(res, incentives[2..]); | ||
|
||
// Limit | ||
let res = paginate_asset_incentives(&storage, None, None, Some(2)).unwrap(); | ||
assert_eq!(res, incentives[..2].to_vec()); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.