From d8ab86344f2fc4f46fbfe4cd03b01c1aa42939bd Mon Sep 17 00:00:00 2001 From: Noah Saso Date: Fri, 19 Jul 2024 17:04:56 -0400 Subject: [PATCH] added ability to update config for a registered denom --- .../schema/dao-rewards-distributor.json | 136 ++++++++++++------ .../dao-rewards-distributor/src/contract.rs | 118 +++++++++------ .../dao-rewards-distributor/src/error.rs | 5 +- .../dao-rewards-distributor/src/msg.rs | 40 ++++-- .../dao-rewards-distributor/src/state.rs | 2 +- .../src/testing/suite.rs | 96 ++++++++++--- .../src/testing/tests.rs | 37 ++++- 7 files changed, 311 insertions(+), 123 deletions(-) diff --git a/contracts/distribution/dao-rewards-distributor/schema/dao-rewards-distributor.json b/contracts/distribution/dao-rewards-distributor/schema/dao-rewards-distributor.json index e95426aac..d484f6d96 100644 --- a/contracts/distribution/dao-rewards-distributor/schema/dao-rewards-distributor.json +++ b/contracts/distribution/dao-rewards-distributor/schema/dao-rewards-distributor.json @@ -61,20 +61,66 @@ "additionalProperties": false }, { - "description": "Claims rewards for the sender.", + "description": "registers a new reward denom", "type": "object", "required": [ - "claim" + "register_denom" ], "properties": { - "claim": { + "register_denom": { + "$ref": "#/definitions/RegisterDenomMsg" + } + }, + "additionalProperties": false + }, + { + "description": "updates the config for a registered denom", + "type": "object", + "required": [ + "update_denom" + ], + "properties": { + "update_denom": { "type": "object", "required": [ "denom" ], "properties": { "denom": { + "description": "denom to update", "type": "string" + }, + "emission_rate": { + "description": "reward emission rate", + "anyOf": [ + { + "$ref": "#/definitions/RewardEmissionRate" + }, + { + "type": "null" + } + ] + }, + "hook_caller": { + "description": "address that will update the reward split when the voting power distribution changes", + "type": [ + "string", + "null" + ] + }, + "vp_contract": { + "description": "address to query the voting power", + "type": [ + "string", + "null" + ] + }, + "withdraw_destination": { + "description": "destination address for reward clawbacks. defaults to owner", + "type": [ + "string", + "null" + ] } }, "additionalProperties": false @@ -110,13 +156,13 @@ "additionalProperties": false }, { - "description": "shuts down the rewards distributor. withdraws all future staking rewards back to the treasury. members can claim whatever they earned until this point.", + "description": "Claims rewards for the sender.", "type": "object", "required": [ - "shutdown" + "claim" ], "properties": { - "shutdown": { + "claim": { "type": "object", "required": [ "denom" @@ -132,37 +178,20 @@ "additionalProperties": false }, { - "description": "registers a new reward denom", + "description": "shuts down the rewards distributor for a denom. withdraws all future staking rewards back to the treasury. members can claim whatever they earned until this point.", "type": "object", "required": [ - "register_reward_denom" - ], - "properties": { - "register_reward_denom": { - "$ref": "#/definitions/RegisterRewardDenomMsg" - } - }, - "additionalProperties": false - }, - { - "description": "updates the reward emission rate for a registered denom", - "type": "object", - "required": [ - "update_reward_emission_rate" + "shutdown" ], "properties": { - "update_reward_emission_rate": { + "shutdown": { "type": "object", "required": [ - "denom", - "emission_rate" + "denom" ], "properties": { "denom": { "type": "string" - }, - "emission_rate": { - "$ref": "#/definitions/RewardEmissionRate" } }, "additionalProperties": false @@ -449,7 +478,7 @@ } ] }, - "RegisterRewardDenomMsg": { + "RegisterDenomMsg": { "type": "object", "required": [ "denom", @@ -459,18 +488,31 @@ ], "properties": { "denom": { - "$ref": "#/definitions/UncheckedDenom" + "description": "denom to register", + "allOf": [ + { + "$ref": "#/definitions/UncheckedDenom" + } + ] }, "emission_rate": { - "$ref": "#/definitions/RewardEmissionRate" + "description": "reward emission rate", + "allOf": [ + { + "$ref": "#/definitions/RewardEmissionRate" + } + ] }, "hook_caller": { + "description": "address that will update the reward split when the voting power distribution changes", "type": "string" }, "vp_contract": { + "description": "address to query the voting power", "type": "string" }, "withdraw_destination": { + "description": "destination address for reward clawbacks. defaults to owner", "type": [ "string", "null" @@ -715,7 +757,7 @@ "active_epoch", "denom", "funded_amount", - "historic_epochs", + "historical_earned_puvp", "hook_caller", "last_update", "vp_contract", @@ -746,12 +788,13 @@ } ] }, - "historic_epochs": { - "description": "historic denom distribution epochs", - "type": "array", - "items": { - "$ref": "#/definitions/Epoch" - } + "historical_earned_puvp": { + "description": "historical rewards earned per unit voting power from past epochs due to changes in the emission rate. each time emission rate is changed, this value is increased by the `active_epoch`'s rewards earned puvp.", + "allOf": [ + { + "$ref": "#/definitions/Uint256" + } + ] }, "hook_caller": { "description": "address that will update the reward split when the voting power distribution changes", @@ -778,7 +821,7 @@ ] }, "withdraw_destination": { - "description": "optional destination address for reward clawbacks", + "description": "destination address for reward clawbacks", "allOf": [ { "$ref": "#/definitions/Addr" @@ -1283,7 +1326,7 @@ "active_epoch", "denom", "funded_amount", - "historic_epochs", + "historical_earned_puvp", "hook_caller", "last_update", "vp_contract", @@ -1314,12 +1357,13 @@ } ] }, - "historic_epochs": { - "description": "historic denom distribution epochs", - "type": "array", - "items": { - "$ref": "#/definitions/Epoch" - } + "historical_earned_puvp": { + "description": "historical rewards earned per unit voting power from past epochs due to changes in the emission rate. each time emission rate is changed, this value is increased by the `active_epoch`'s rewards earned puvp.", + "allOf": [ + { + "$ref": "#/definitions/Uint256" + } + ] }, "hook_caller": { "description": "address that will update the reward split when the voting power distribution changes", @@ -1346,7 +1390,7 @@ ] }, "withdraw_destination": { - "description": "optional destination address for reward clawbacks", + "description": "destination address for reward clawbacks", "allOf": [ { "$ref": "#/definitions/Addr" diff --git a/contracts/distribution/dao-rewards-distributor/src/contract.rs b/contracts/distribution/dao-rewards-distributor/src/contract.rs index cbf1345ab..5b7acb337 100644 --- a/contracts/distribution/dao-rewards-distributor/src/contract.rs +++ b/contracts/distribution/dao-rewards-distributor/src/contract.rs @@ -17,8 +17,8 @@ use crate::hooks::{ subscribe_denom_to_hook, }; use crate::msg::{ - ExecuteMsg, InstantiateMsg, PendingRewardsResponse, QueryMsg, ReceiveMsg, - RegisterRewardDenomMsg, RewardEmissionRate, RewardsStateResponse, + ExecuteMsg, InstantiateMsg, PendingRewardsResponse, QueryMsg, ReceiveMsg, RegisterDenomMsg, + RewardEmissionRate, RewardsStateResponse, }; use crate::rewards::{ get_accrued_rewards_since_last_user_action, get_active_total_earned_puvp, update_rewards, @@ -55,50 +55,38 @@ pub fn execute( ExecuteMsg::StakeChangeHook(msg) => execute_stake_changed(deps, env, info, msg), ExecuteMsg::NftStakeChangeHook(msg) => execute_nft_stake_changed(deps, env, info, msg), ExecuteMsg::MemberChangedHook(msg) => execute_membership_changed(deps, env, info, msg), - ExecuteMsg::Claim { denom } => execute_claim(deps, env, info, denom), - ExecuteMsg::Fund {} => execute_fund_native(deps, env, info), - ExecuteMsg::Receive(msg) => execute_receive(deps, env, info, msg), ExecuteMsg::UpdateOwnership(action) => execute_update_owner(deps, info, env, action), - ExecuteMsg::Shutdown { denom } => execute_shutdown(deps, info, env, denom), - ExecuteMsg::RegisterRewardDenom(register_msg) => { - execute_register_reward_denom(deps, info, register_msg) - } - ExecuteMsg::UpdateRewardEmissionRate { + ExecuteMsg::RegisterDenom(register_msg) => execute_register_denom(deps, info, register_msg), + ExecuteMsg::UpdateDenom { denom, emission_rate, - } => execute_update_reward_rate(deps, env, info, denom, emission_rate), + vp_contract, + hook_caller, + withdraw_destination, + } => execute_update_denom( + deps, + env, + info, + denom, + emission_rate, + vp_contract, + hook_caller, + withdraw_destination, + ), + ExecuteMsg::Fund {} => execute_fund_native(deps, env, info), + ExecuteMsg::Receive(msg) => execute_receive(deps, env, info, msg), + ExecuteMsg::Claim { denom } => execute_claim(deps, env, info, denom), + ExecuteMsg::Shutdown { denom } => execute_shutdown(deps, info, env, denom), } } -/// updates the reward emission rate for a registered denom -fn execute_update_reward_rate( - deps: DepsMut, - env: Env, - info: MessageInfo, - denom: String, - new_emission_rate: RewardEmissionRate, -) -> Result { - // only the owner can update the reward rate - cw_ownable::assert_owner(deps.storage, &info.sender)?; - - let mut reward_state = DENOM_REWARD_STATES - .load(deps.storage, denom.clone()) - .map_err(|_| ContractError::DenomNotRegistered {})?; - - // transition the epoch to the new emission rate and save - reward_state.transition_epoch(deps.as_ref(), new_emission_rate, &env.block)?; - DENOM_REWARD_STATES.save(deps.storage, denom.clone(), &reward_state)?; - - Ok(Response::new().add_attribute("action", "update_reward_rate")) -} - /// registers a new denom for rewards distribution. /// only the owner can register a new denom. /// a denom can only be registered once; update if you need to change something. -fn execute_register_reward_denom( +fn execute_register_denom( deps: DepsMut, info: MessageInfo, - msg: RegisterRewardDenomMsg, + msg: RegisterDenomMsg, ) -> Result { // only the owner can register a new denom cw_ownable::assert_owner(deps.storage, &info.sender)?; @@ -154,10 +142,52 @@ fn execute_register_reward_denom( Ok(Response::default().add_attribute("action", "register_reward_denom")) } -/// shutdown the rewards distributor contract. -/// can only be called by the admin and only during the distribution period. -/// this will clawback all (undistributed) future rewards to the admin. -/// updates the period finish expiration to the current block. +/// updates the config for a registered denom +fn execute_update_denom( + deps: DepsMut, + env: Env, + info: MessageInfo, + denom: String, + emission_rate: Option, + vp_contract: Option, + hook_caller: Option, + withdraw_destination: Option, +) -> Result { + // only the owner can update a denom config + cw_ownable::assert_owner(deps.storage, &info.sender)?; + + let mut reward_state = DENOM_REWARD_STATES + .load(deps.storage, denom.clone()) + .map_err(|_| ContractError::DenomNotRegistered {})?; + + if let Some(emission_rate) = emission_rate { + // transition the epoch to the new emission rate + reward_state.transition_epoch(deps.as_ref(), emission_rate, &env.block)?; + } + + if let Some(vp_contract) = vp_contract { + reward_state.vp_contract = validate_voting_power_contract(&deps, vp_contract)?; + } + + if let Some(hook_caller) = hook_caller { + reward_state.hook_caller = deps.api.addr_validate(&hook_caller)?; + } + + if let Some(withdraw_destination) = withdraw_destination { + reward_state.withdraw_destination = deps.api.addr_validate(&withdraw_destination)?; + } + + DENOM_REWARD_STATES.save(deps.storage, denom.clone(), &reward_state)?; + + Ok(Response::new() + .add_attribute("action", "update_denom") + .add_attribute("denom", denom)) +} + +/// shutdown the rewards distribution for a specific denom. can only be called +/// by the admin and only during the distribution period. this will clawback all +/// (undistributed) future rewards to the admin. updates the period finish +/// expiration to the current block. fn execute_shutdown( deps: DepsMut, info: MessageInfo, @@ -188,12 +218,10 @@ fn execute_shutdown( // get the fraction of what part of rewards duration is in the past // and sub from 1 to get the remaining rewards - let remaining_reward_duration_fraction = Decimal::one() - .checked_sub(Decimal::from_ratio( - passed_scalar_units_since_start, - reward_duration_scalar, - )) - .map_err(|e| ContractError::Std(e.into()))?; + let remaining_reward_duration_fraction = Decimal::one().checked_sub(Decimal::from_ratio( + passed_scalar_units_since_start, + reward_duration_scalar, + ))?; // to get the clawback msg let clawback_msg = get_transfer_msg( diff --git a/contracts/distribution/dao-rewards-distributor/src/error.rs b/contracts/distribution/dao-rewards-distributor/src/error.rs index 25ce5638f..d7aea5c06 100644 --- a/contracts/distribution/dao-rewards-distributor/src/error.rs +++ b/contracts/distribution/dao-rewards-distributor/src/error.rs @@ -1,4 +1,4 @@ -use cosmwasm_std::StdError; +use cosmwasm_std::{OverflowError, StdError}; use thiserror::Error; #[derive(Error, Debug, PartialEq)] @@ -12,6 +12,9 @@ pub enum ContractError { #[error(transparent)] Cw20Error(#[from] cw20_base::ContractError), + #[error(transparent)] + Overflow(#[from] OverflowError), + #[error("Invalid Cw20")] InvalidCw20 {}, diff --git a/contracts/distribution/dao-rewards-distributor/src/msg.rs b/contracts/distribution/dao-rewards-distributor/src/msg.rs index 4d07ae065..c9735a6b8 100644 --- a/contracts/distribution/dao-rewards-distributor/src/msg.rs +++ b/contracts/distribution/dao-rewards-distributor/src/msg.rs @@ -33,30 +33,46 @@ pub enum ExecuteMsg { NftStakeChangeHook(NftStakeChangedHookMsg), /// Called when tokens are staked or unstaked. StakeChangeHook(StakeChangedHookMsg), - /// Claims rewards for the sender. - Claim { denom: String }, + /// registers a new reward denom + RegisterDenom(RegisterDenomMsg), + /// updates the config for a registered denom + UpdateDenom { + /// denom to update + denom: String, + /// reward emission rate + emission_rate: Option, + /// address to query the voting power + vp_contract: Option, + /// address that will update the reward split when the voting power + /// distribution changes + hook_caller: Option, + /// destination address for reward clawbacks. defaults to owner + withdraw_destination: Option, + }, /// Used to fund this contract with cw20 tokens. Receive(Cw20ReceiveMsg), /// Used to fund this contract with native tokens. Fund {}, - /// shuts down the rewards distributor. withdraws all future staking rewards - /// back to the treasury. members can claim whatever they earned until this point. + /// Claims rewards for the sender. + Claim { denom: String }, + /// shuts down the rewards distributor for a denom. withdraws all future + /// staking rewards back to the treasury. members can claim whatever they + /// earned until this point. Shutdown { denom: String }, - /// registers a new reward denom - RegisterRewardDenom(RegisterRewardDenomMsg), - /// updates the reward emission rate for a registered denom - UpdateRewardEmissionRate { - denom: String, - emission_rate: RewardEmissionRate, - }, } #[cw_serde] -pub struct RegisterRewardDenomMsg { +pub struct RegisterDenomMsg { + /// denom to register pub denom: UncheckedDenom, + /// reward emission rate pub emission_rate: RewardEmissionRate, + /// address to query the voting power pub vp_contract: String, + /// address that will update the reward split when the voting power + /// distribution changes pub hook_caller: String, + /// destination address for reward clawbacks. defaults to owner pub withdraw_destination: Option, } diff --git a/contracts/distribution/dao-rewards-distributor/src/state.rs b/contracts/distribution/dao-rewards-distributor/src/state.rs index 695fb8428..0bcf4b07d 100644 --- a/contracts/distribution/dao-rewards-distributor/src/state.rs +++ b/contracts/distribution/dao-rewards-distributor/src/state.rs @@ -81,7 +81,7 @@ pub struct DenomRewardState { pub hook_caller: Addr, /// total amount of rewards funded pub funded_amount: Uint128, - /// optional destination address for reward clawbacks + /// destination address for reward clawbacks pub withdraw_destination: Addr, /// historical rewards earned per unit voting power from past epochs due to /// changes in the emission rate. each time emission rate is changed, this diff --git a/contracts/distribution/dao-rewards-distributor/src/testing/suite.rs b/contracts/distribution/dao-rewards-distributor/src/testing/suite.rs index 61d3d1aef..408c815cf 100644 --- a/contracts/distribution/dao-rewards-distributor/src/testing/suite.rs +++ b/contracts/distribution/dao-rewards-distributor/src/testing/suite.rs @@ -11,7 +11,7 @@ use cw_utils::Duration; use crate::{ msg::{ - ExecuteMsg, InstantiateMsg, PendingRewardsResponse, QueryMsg, RegisterRewardDenomMsg, + ExecuteMsg, InstantiateMsg, PendingRewardsResponse, QueryMsg, RegisterDenomMsg, RewardEmissionRate, RewardsStateResponse, }, state::DenomRewardState, @@ -383,7 +383,7 @@ impl Suite { .unwrap() } - pub fn _get_denom_reward_state(&mut self, denom: &str) -> DenomRewardState { + pub fn get_denom_reward_state(&mut self, denom: &str) -> DenomRewardState { let resp: DenomRewardState = self .app .wrap() @@ -394,9 +394,18 @@ impl Suite { }, ) .unwrap(); - // println!("[{} REWARD STATE] {:?}", denom, resp); resp } + + pub fn get_owner(&mut self) -> Addr { + let ownable_response: cw_ownable::Ownership = self + .app + .borrow_mut() + .wrap() + .query_wasm_smart(self.distribution_contract.clone(), &QueryMsg::Ownership {}) + .unwrap(); + ownable_response.owner.unwrap() + } } // SUITE ASSERTIONS @@ -438,16 +447,6 @@ impl Suite { assert_eq!(units, expected); } - pub fn get_owner(&mut self) -> Addr { - let ownable_response: cw_ownable::Ownership = self - .app - .borrow_mut() - .wrap() - .query_wasm_smart(self.distribution_contract.clone(), &QueryMsg::Ownership {}) - .unwrap(); - ownable_response.owner.unwrap() - } - pub fn assert_pending_rewards(&mut self, address: &str, _denom: &str, expected: u128) { let res: PendingRewardsResponse = self .app @@ -510,7 +509,7 @@ impl Suite { } pub fn register_reward_denom(&mut self, reward_config: RewardsConfig, hook_caller: &str) { - let register_reward_denom_msg = ExecuteMsg::RegisterRewardDenom(RegisterRewardDenomMsg { + let register_reward_denom_msg = ExecuteMsg::RegisterDenom(RegisterDenomMsg { denom: reward_config.denom.clone(), emission_rate: RewardEmissionRate { amount: Uint128::new(reward_config.amount), @@ -686,12 +685,75 @@ impl Suite { epoch_duration: Duration, epoch_rewards: u128, ) { - let msg = ExecuteMsg::UpdateRewardEmissionRate { + let msg: ExecuteMsg = ExecuteMsg::UpdateDenom { denom: denom.to_string(), - emission_rate: RewardEmissionRate { + emission_rate: Some(RewardEmissionRate { amount: Uint128::new(epoch_rewards), duration: epoch_duration, - }, + }), + vp_contract: None, + hook_caller: None, + withdraw_destination: None, + }; + + let _resp = self + .app + .execute_contract( + Addr::unchecked(OWNER), + self.distribution_contract.clone(), + &msg, + &[], + ) + .unwrap(); + } + + pub fn update_vp_contract(&mut self, denom: &str, vp_contract: &str) { + let msg: ExecuteMsg = ExecuteMsg::UpdateDenom { + denom: denom.to_string(), + emission_rate: None, + vp_contract: Some(vp_contract.to_string()), + hook_caller: None, + withdraw_destination: None, + }; + + let _resp = self + .app + .execute_contract( + Addr::unchecked(OWNER), + self.distribution_contract.clone(), + &msg, + &[], + ) + .unwrap(); + } + + pub fn update_hook_caller(&mut self, denom: &str, hook_caller: &str) { + let msg: ExecuteMsg = ExecuteMsg::UpdateDenom { + denom: denom.to_string(), + emission_rate: None, + vp_contract: None, + hook_caller: Some(hook_caller.to_string()), + withdraw_destination: None, + }; + + let _resp = self + .app + .execute_contract( + Addr::unchecked(OWNER), + self.distribution_contract.clone(), + &msg, + &[], + ) + .unwrap(); + } + + pub fn update_withdraw_destination(&mut self, denom: &str, withdraw_destination: &str) { + let msg: ExecuteMsg = ExecuteMsg::UpdateDenom { + denom: denom.to_string(), + emission_rate: None, + vp_contract: None, + hook_caller: None, + withdraw_destination: Some(withdraw_destination.to_string()), }; let _resp = self diff --git a/contracts/distribution/dao-rewards-distributor/src/testing/tests.rs b/contracts/distribution/dao-rewards-distributor/src/testing/tests.rs index 919730d28..7d49d9a10 100644 --- a/contracts/distribution/dao-rewards-distributor/src/testing/tests.rs +++ b/contracts/distribution/dao-rewards-distributor/src/testing/tests.rs @@ -7,6 +7,7 @@ use cw4::Member; use cw_multi_test::Executor; use cw_utils::Duration; +use crate::testing::native_setup::setup_native_token_test; use crate::{ msg::ExecuteMsg, testing::{ADDR1, ADDR2, ADDR3, DENOM}, @@ -59,7 +60,7 @@ fn test_native_dao_rewards_update_reward_rate() { // double the rewards rate // now there will be 10_000_000 tokens distributed over 100_000 blocks - suite.update_reward_emission_rate(DENOM, Duration::Height(10), 1000); + suite.update_reward_emission_rate(DENOM, Duration::Height(10), 1_000); // skip 1/10th of the time suite.skip_blocks(100_000); @@ -1307,3 +1308,37 @@ fn test_update_owner() { let owner = suite.get_owner().to_string(); assert_eq!(owner, new_owner); } + +#[test] +fn test_update_vp_contract() { + let mut suite = SuiteBuilder::base(super::suite::DaoType::Native).build(); + + let new_vp_contract = setup_native_token_test(suite.app.borrow_mut()); + + suite.update_vp_contract(DENOM, new_vp_contract.as_str()); + + let denom = suite.get_denom_reward_state(DENOM); + assert_eq!(denom.vp_contract, new_vp_contract); +} + +#[test] +fn test_update_hook_caller() { + let mut suite = SuiteBuilder::base(super::suite::DaoType::Native).build(); + + let new_hook_caller = "new_hook_caller"; + suite.update_hook_caller(DENOM, new_hook_caller); + + let denom = suite.get_denom_reward_state(DENOM); + assert_eq!(denom.hook_caller, new_hook_caller); +} + +#[test] +fn update_withdraw_destination() { + let mut suite = SuiteBuilder::base(super::suite::DaoType::Native).build(); + + let new_withdraw_destination = "new_withdraw_destination"; + suite.update_withdraw_destination(DENOM, new_withdraw_destination); + + let denom = suite.get_denom_reward_state(DENOM); + assert_eq!(denom.withdraw_destination, new_withdraw_destination); +}