Add Staking Tier Discount on Fees #36
Conversation
| @@ -74,7 +74,7 @@ fn liquidatee_must_have_the_request_vault_position() { | |||
| assert_err( | |||
| res, | |||
| ContractError::Std(NotFound { | |||
| kind: "type: mars_types::adapters::vault::amount::VaultPositionAmount; key: [00, 0F, 76, 61, 75, 6C, 74, 5F, 70, 6F, 73, 69, 74, 69, 6F, 6E, 73, 00, 01, 32, 63, 6F, 6E, 74, 72, 61, 63, 74, 31, 34]".to_string(), | |||
| kind: "type: mars_types::adapters::vault::amount::VaultPositionAmount; key: [00, 0F, 76, 61, 75, 6C, 74, 5F, 70, 6F, 73, 69, 74, 69, 6F, 6E, 73, 00, 01, 32, 63, 6F, 6E, 74, 72, 61, 63, 74, 31, 35]".to_string(), | |||
There was a problem hiding this comment.
contract becomes 15 from 14 (due to dao staking contract in address provider)
There was a problem hiding this comment.
Nice work! That was a much heavier lift than we thought but in general looks really good.
I think we need to add some more test coverage around modifying fees while a user is in a position, and the position modification in general, so that we have more confidence that there are no side effects from the changes.
| @@ -67,3 +68,9 @@ pub const VAULTS: Map<&str, Addr> = Map::new("vaults"); | |||
| pub const PERPS_LB_RATIO: Item<Decimal> = Item::new("perps_lb_ratio"); | |||
|
|
|||
| pub const SWAP_FEE: Item<Decimal> = Item::new("swap_fee"); | |||
|
|
|||
| // Fee tier discount configuration | |||
| pub const FEE_TIER_CONFIG: Item<FeeTierConfig> = Item::new("fee_tier_config"); | |||
There was a problem hiding this comment.
Just a note that we need to remember to add the migration for these new state variables otherwise contract will panic
| let base_opening_fee = | ||
| perps.query_opening_fee(&deps.querier, denom, order_size, None)?; | ||
| let opening_fee = | ||
| perps.query_opening_fee(&deps.querier, denom, order_size, Some(discount_pct))?; |
There was a problem hiding this comment.
Is there a way to only sent one smart contract query here, and apply the discount pct in this function? Not sure if it makes sense, but my thinking is that maybe it would be more efficient if we were not querying twice
| ) -> ContractResult<Response> { | ||
| let pnl = position.unrealized_pnl.to_coins(&position.base_denom).pnl; | ||
| let pnl_string = position.unrealized_pnl.pnl.to_string(); | ||
| let (funds, response) = update_state_based_on_pnl(&mut deps, account_id, pnl, None, response)?; | ||
| let funds = funds.map_or_else(Vec::new, |c| vec![c]); | ||
|
|
||
| let msg = perps.execute_perp_order(account_id, denom, order_size, reduce_only, funds)?; | ||
| // Get base and effective fees for logging | ||
| let base_opening_fee = perps.query_opening_fee(&deps.querier, denom, order_size, None)?; |
There was a problem hiding this comment.
same here - maybe can we use just one query to reduce gas costs?
| /// Returns the tier with the highest min_voting_power that the user qualifies for | ||
| pub fn find_applicable_tier(&self, voting_power: Uint128) -> StdResult<&FeeTier> { | ||
| // Ensure tiers are sorted in descending order of min_voting_power | ||
| if self.config.tiers.is_empty() { |
There was a problem hiding this comment.
We usually follow a pattern of using 'assert_xyz', which the auditors have often requested. Think might be best to extract to helper in utils.rs
|
|
||
| // Validate discount percentages are reasonable (0-100%) | ||
| for tier in &self.config.tiers { | ||
| if tier.discount_pct >= Decimal::one() { |
There was a problem hiding this comment.
minor detail - but maybe we should allow 100%? Comment above seems to apply that
| @@ -501,7 +501,7 @@ fn unlock_and_withdraw_if_zero_withdrawal_balance() { | |||
|
|
|||
| // open a position | |||
| let size = Int128::from_str("50").unwrap(); | |||
| let atom_opening_fee = mock.query_opening_fee("uatom", size).fee; | |||
| let atom_opening_fee = mock.query_opening_fee("uatom", size, None).fee; | |||
There was a problem hiding this comment.
If I add a value here (e.g 20% the test fails in the next line from invalid funds sent for the fee (9vs12) - is this just a test config issue or something else?
There was a problem hiding this comment.
Think it would be good to test here using different opening fees also, similar to my suggestion above
| @@ -1,5 +1,6 @@ | |||
| use cosmwasm_std::Empty; | |||
| use cw_multi_test::{Contract, ContractWrapper}; | |||
| use mars_mock_dao_staking as _; // ensure dependency is linked | |||
There was a problem hiding this comment.
I fee like "Governance" is a better name :)
| #[cw_serde] | ||
| pub struct FeeTier { | ||
| pub id: String, | ||
| pub min_voting_power: String, // Uint128 as string |
There was a problem hiding this comment.
Not sure about using a string here? I think it would be better to use Uint128 if possible
Overview
Implements staking-based fee discounts for Mars Credit Manager users based on their MARS token staking tier.
Key Features
8-tier system: 0% to 80% fee discounts based on MARS staked (10k to 1.5M+ tokens)
Applies to: Swap fees and perpetual trading fees
Integration: Queries DAO staking contract for user voting power
Implementation
New staking.rs module with StakingTierManager
Updated swap.rs and perp.rs to apply discounts
New query endpoints for tier information
Comprehensive test coverage (4 new test files)
Benefits
Incentivizes long-term MARS staking
Configurable tier thresholds and discounts
No breaking changes to existing functionality