reduce staking rewards rate #147
Conversation
| let prev_rewards_rate = fixed_point64::create_from_rational(rewards_rate_numerator as u128, rewards_rate_denominator as u128); | ||
| let aip_monthly_rewards_rate_reduction = fixed_point64::create_from_rational(25, 10000); // 25 bps | ||
| let new_rewards_rate = prev_rewards_rate.sub(aip_monthly_rewards_rate_reduction); // subtract 25 bps from the current rewards rate | ||
| let min_rewards_rate = fixed_point64::create_from_raw_value(52628560842293); |
There was a problem hiding this comment.
I'd suggest using create_from_rational so you can pass in numerator and denominator instead of magic looking values here that are harder to review https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/framework/aptos-stdlib/sources/fixed_point64.move#L129
There was a problem hiding this comment.
there's no simple way to extract the original the rewards_rate_decrease denominator or numerator
fixed64 doesn't provide this functionality either. if you know the last proposal with changed staking config, we
could use the same numerator/denominator for rewards_rate_decrease
or we could apply a bitmask to the u128 value, in my opinion create_from_raw_value for the min_rewards_rate and new_reward_rate will be less prone to errors
looking at the implementation of staking_config::reward_rate(), i agree it will be easier to simply compute the new per-epoch reward_rate, ill try to write a test to config the new rewards_rate returns the right reward_rate()
| let new_rewards_rate = prev_rewards_rate.sub(aip_monthly_rewards_rate_reduction); // subtract 25 bps from the current rewards rate | ||
| let min_rewards_rate = fixed_point64::create_from_raw_value(52628560842293); | ||
| let rewards_rate_period_sec = 31536000; // unchanged, 1 year | ||
| let rewards_rate_decrease_rate = fixed_point64::create_from_raw_value(276701161105643274); // unchanged |
There was a problem hiding this comment.
not possible here, can you confirm the raw values from the PR as the same as the on-chain values
i also have a fork with the view functions that would allow us to just view the previous value
| let aip_monthly_rewards_rate_reduction = fixed_point64::create_from_rational(25, 10000); // 25 bps | ||
| let new_rewards_rate = prev_rewards_rate.sub(aip_monthly_rewards_rate_reduction); // subtract 25 bps from the current rewards rate | ||
| let min_rewards_rate = fixed_point64::create_from_raw_value(52628560842293); | ||
| let rewards_rate_period_sec = 31536000; // unchanged, 1 year |
There was a problem hiding this comment.
For any unchanged values, I'd suggest just reading current values and passing them here to make absolutely sure things don't accidentally get changed
There was a problem hiding this comment.
there's no view function for these values
i grabbed the magic numbers from the explorer, the only data that's exposed is rewards_rate(), and rewards_rate() is the per-epoch rate, not the yearly rate which is what we need to pass here as input
| use aptos_std::fixed_point64; | ||
|
|
||
| fun main(proposal_id: u64) { | ||
| let framework_signer = aptos_governance::resolve_multi_step_proposal(proposal_id, @0000000000000000000000000000000000000000000000000000000000000001, vector::empty<u8>()); |
There was a problem hiding this comment.
nit: Use vector[] instead of vector::empty
|
@movekevin ive created a fork of aptos-core with view functions to check the results of the updating the rewards rate: https://github.com/mirage-protocol/aptos-core.git", rev = "expose-staking-rewards-config-for-testing ive added a sources folder with the script and tests. if you don't like having a script call an inline function, which is a bit sketchy, ive also copied the script into the folder above following the patterns of the other proposals there's tests to make sure that the annual rewards rate is lowered by 25bps as well as tests to make sure the other StakingRewardsConfig variables dont change |
AIP: AIP-119
Release:
Type: Tokenomics
Security Consideration
The PR should reduce rewards_rate by 25bps (0.25%), and leave all other variables unchanged.
Test Result
TODO
Ecosystem Impact
staking_config::staking_rate() will go from 6.79->6.54. All staking operators, and defi protocols will need to adjust to a new staking_rate()