Bugfix/correct reward stake supply (#1373)

* Constructing `EpochRewardParams` with proper staking_supply

* Query for current staking supply

* More crate visibility on epoch reward params

* unused import

* Remove duplicate save of epoch_reward_params

* test fixes

* Changelog

* Moved PR references to correct section

* cargo fmt

* Removed old migration code
This commit is contained in:
Jędrzej Stuczyński
2022-06-20 18:25:22 +01:00
committed by GitHub
parent b2df4ca4fd
commit 42ef2eb98c
10 changed files with 103 additions and 125 deletions
+5 -1
View File
@@ -35,6 +35,7 @@ Post 1.0.0 release, the changelog format is based on [Keep a Changelog](https://
- mixnet-contract: removed `expect` in `query_delegator_reward` and queries containing invalid proxy address should now return a more human-readable error ([#1257])
- mixnet-contract: replaced integer division with fixed for performance calculations ([#1284])
- mixnet-contract: Under certain circumstances nodes could not be unbonded ([#1255](https://github.com/nymtech/nym/issues/1255)) ([#1258])
- mixnet-contract: Using correct staking supply when distributing rewards. ([#1373])
- mixnode, gateway: attempting to determine reconnection backoff to persistently failing mixnode could result in a crash ([#1260])
- mixnode: the mixnode learned how to shutdown gracefully
- native & socks5 clients: fail early when clients try to re-init with a different gateway, which is not supported yet ([#1322])
@@ -58,7 +59,6 @@ Post 1.0.0 release, the changelog format is based on [Keep a Changelog](https://
[#1258]: https://github.com/nymtech/nym/pull/1258
[#1260]: https://github.com/nymtech/nym/pull/1260
[#1261]: https://github.com/nymtech/nym/pull/1261
[#1265]: https://github.com/nymtech/nym/pull/1265
[#1267]: https://github.com/nymtech/nym/pull/1267
[#1275]: https://github.com/nymtech/nym/pull/1275
[#1278]: https://github.com/nymtech/nym/pull/1278
@@ -74,6 +74,7 @@ Post 1.0.0 release, the changelog format is based on [Keep a Changelog](https://
[#1329]: https://github.com/nymtech/nym/pull/1329
[#1353]: https://github.com/nymtech/nym/pull/1353
[#1369]: https://github.com/nymtech/nym/pull/1369
[#1373]: https://github.com/nymtech/nym/pull/1373
[#1376]: https://github.com/nymtech/nym/pull/1376
## [nym-wallet-v1.0.5](https://github.com/nymtech/nym/tree/nym-wallet-v1.0.5) (2022-06-14)
@@ -89,6 +90,9 @@ Post 1.0.0 release, the changelog format is based on [Keep a Changelog](https://
- wallet: staking_supply field to StateParams
- wallet: show transaction hash for redeeming or compounding rewards
[#1265]: https://github.com/nymtech/nym/pull/1265
[#1302]: https://github.com/nymtech/nym/pull/1302
## [nym-wallet-v1.0.4](https://github.com/nymtech/nym/tree/nym-wallet-v1.0.4) (2022-05-04)
### Changed
@@ -165,6 +165,7 @@ pub enum QueryMsg {
LayerDistribution {},
GetRewardPool {},
GetCirculatingSupply {},
GetStakingSupply {},
GetIntervalRewardPercent {},
GetSybilResistancePercent {},
GetActiveSetWorkFactor {},
@@ -160,6 +160,14 @@ impl EpochRewardParams {
pub fn epoch_reward_pool(&self) -> u128 {
self.epoch_reward_pool.u128()
}
pub fn sybil_resistance_percent(&self) -> u8 {
self.sybil_resistance_percent
}
pub fn active_set_work_factor(&self) -> u8 {
self.active_set_work_factor
}
}
#[derive(Debug, Clone, JsonSchema, PartialEq, Serialize, Deserialize, Copy)]
+3 -83
View File
@@ -7,7 +7,6 @@ use crate::delegations::queries::query_mixnode_delegation;
use crate::delegations::queries::{
query_mixnode_delegations_paged, query_pending_delegation_events,
};
use crate::delegations::storage::delegations;
use crate::error::ContractError;
use crate::gateways::queries::query_owns_gateway;
use crate::gateways::queries::{query_gateway_bond, query_gateways_paged};
@@ -29,14 +28,13 @@ use crate::mixnodes::bonding_queries::{
};
use crate::mixnodes::layer_queries::query_layer_distribution;
use crate::rewards::queries::{
query_circulating_supply, query_reward_pool, query_rewarding_status,
query_circulating_supply, query_reward_pool, query_rewarding_status, query_staking_supply,
};
use crate::rewards::storage as rewards_storage;
use cosmwasm_std::{
entry_point, to_binary, Addr, Api, Deps, DepsMut, Env, MessageInfo, QueryResponse, Response,
Uint128,
};
use mixnet_contract_common::mixnode::DelegationEvent;
use mixnet_contract_common::{
ContractStateParams, ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg,
};
@@ -371,6 +369,7 @@ pub fn query(deps: Deps<'_>, env: Env, msg: QueryMsg) -> Result<QueryResponse, C
)?),
QueryMsg::GetRewardPool {} => to_binary(&query_reward_pool(deps)?),
QueryMsg::GetCirculatingSupply {} => to_binary(&query_circulating_supply(deps)?),
QueryMsg::GetStakingSupply {} => to_binary(&query_staking_supply(deps)?),
QueryMsg::GetIntervalRewardPercent {} => to_binary(&INTERVAL_REWARD_PERCENT),
QueryMsg::GetSybilResistancePercent {} => to_binary(&SYBIL_RESISTANCE_PERCENT),
QueryMsg::GetActiveSetWorkFactor {} => to_binary(&ACTIVE_SET_WORK_FACTOR),
@@ -439,87 +438,8 @@ pub fn query(deps: Deps<'_>, env: Env, msg: QueryMsg) -> Result<QueryResponse, C
Ok(query_res?)
}
fn migrate_contract_state_params(deps: DepsMut<'_>) -> Result<(), ContractError> {
use crate::mixnet_contract_settings::storage::CONTRACT_STATE;
use cw_storage_plus::Item;
use serde::{Deserialize, Serialize};
#[derive(Serialize, Deserialize)]
struct OldContractState {
pub owner: Addr,
pub rewarding_validator_address: Addr,
pub params: OldContractStateParams,
}
#[derive(Serialize, Deserialize)]
struct OldContractStateParams {
pub minimum_mixnode_pledge: Uint128,
pub minimum_gateway_pledge: Uint128,
pub mixnode_rewarded_set_size: u32,
pub mixnode_active_set_size: u32,
}
const OLD_CONTRACT_STATE: Item<'_, OldContractState> = Item::new("config");
let old_contract_state = OLD_CONTRACT_STATE.load(deps.storage)?;
let old_params = old_contract_state.params;
let new_params = ContractStateParams {
minimum_mixnode_pledge: old_params.minimum_mixnode_pledge,
minimum_gateway_pledge: old_params.minimum_gateway_pledge,
mixnode_rewarded_set_size: old_params.mixnode_rewarded_set_size,
mixnode_active_set_size: old_params.mixnode_active_set_size,
staking_supply: INITIAL_STAKING_SUPPLY,
};
let new_contract_state = ContractState {
owner: old_contract_state.owner,
rewarding_validator_address: old_contract_state.rewarding_validator_address,
params: new_params,
};
CONTRACT_STATE.save(deps.storage, &new_contract_state)?;
Ok(())
}
fn _deal_with_zero_delegations(deps: DepsMut<'_>) -> Result<(), ContractError> {
// if there exists any delegation of 0 value, remove it
let zero_delegations = delegations()
.range(deps.storage, None, None, cosmwasm_std::Order::Ascending)
.filter_map(|r| r.ok())
.filter(|(_k, v)| v.amount.amount == Uint128::zero())
.map(|(k, _v)| k)
.collect::<Vec<_>>();
for delegation in &zero_delegations {
delegations().remove(deps.storage, delegation.clone())?;
}
// similarly, if there exists any event that is scheduled to insert/remove delegation with 0 value, purge it
let delegate_events_to_purge = crate::delegations::storage::PENDING_DELEGATION_EVENTS
.range(deps.storage, None, None, cosmwasm_std::Order::Ascending)
.filter_map(|r| r.ok())
.filter(|(_k, v)| match v {
DelegationEvent::Delegate(d) => d.amount.amount == Uint128::zero(),
_ => false,
})
.map(|(k, _v)| k)
.collect::<Vec<_>>();
for delegation_event in delegate_events_to_purge {
crate::delegations::storage::PENDING_DELEGATION_EVENTS
.remove(deps.storage, delegation_event);
}
Ok(())
}
#[entry_point]
pub fn migrate(deps: DepsMut<'_>, _env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> {
migrate_contract_state_params(deps)?;
pub fn migrate(_deps: DepsMut<'_>, _env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> {
Ok(Default::default())
}
+3 -3
View File
@@ -18,8 +18,8 @@ type IntervalId = u32;
pub(crate) const REWARDED_NODE_DEFAULT_PAGE_LIMIT: u32 = 1000;
pub(crate) const REWARDED_NODE_MAX_PAGE_LIMIT: u32 = 1500;
const CURRENT_EPOCH: Item<'_, Interval> = Item::new("ceph");
const CURRENT_EPOCH_REWARD_PARAMS: Item<'_, EpochRewardParams> = Item::new("erp");
pub(crate) const CURRENT_EPOCH: Item<'_, Interval> = Item::new("ceph");
pub(crate) const CURRENT_EPOCH_REWARD_PARAMS: Item<'_, EpochRewardParams> = Item::new("erp");
pub(crate) const CURRENT_REWARDED_SET_HEIGHT: Item<'_, BlockHeight> = Item::new("crh");
// I've changed the `()` data to an `u8` as after serializing `()` is represented as "null",
@@ -49,7 +49,7 @@ pub fn save_epoch_reward_params(
epoch_id: u32,
storage: &mut dyn Storage,
) -> Result<(), ContractError> {
let epoch_reward_params = epoch_reward_params(epoch_id, storage)?;
let epoch_reward_params = epoch_reward_params(storage)?;
CURRENT_EPOCH_REWARD_PARAMS.save(storage, &epoch_reward_params)?;
crate::rewards::storage::EPOCH_REWARD_PARAMS.save(storage, epoch_id, &epoch_reward_params)?;
Ok(())
@@ -381,8 +381,6 @@ fn validate_mixnode_pledge(
#[cfg(test)]
pub mod tests {
use std::f64::MIN;
use super::*;
use crate::contract::{execute, query, INITIAL_MIXNODE_PLEDGE};
use crate::error::ContractError;
+4
View File
@@ -15,6 +15,10 @@ pub(crate) fn query_circulating_supply(deps: Deps<'_>) -> StdResult<Uint128> {
storage::circulating_supply(deps.storage)
}
pub(crate) fn query_staking_supply(deps: Deps<'_>) -> StdResult<Uint128> {
storage::staking_supply(deps.storage)
}
pub(crate) fn query_rewarding_status(
deps: Deps<'_>,
mix_identity: IdentityKey,
+6
View File
@@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
use crate::error::ContractError;
use crate::mixnet_contract_settings::storage as settings_storage;
use config::defaults::TOTAL_SUPPLY;
use cosmwasm_std::{StdResult, Storage, Uint128};
use cw_storage_plus::{Item, Map};
@@ -73,3 +74,8 @@ pub fn circulating_supply(storage: &dyn Storage) -> StdResult<Uint128> {
let reward_pool = REWARD_POOL.load(storage)?;
Ok(Uint128::new(TOTAL_SUPPLY).saturating_sub(reward_pool))
}
pub fn staking_supply(storage: &dyn Storage) -> StdResult<Uint128> {
let state = settings_storage::CONTRACT_STATE.load(storage)?;
Ok(state.params.staking_supply)
}
+71 -31
View File
@@ -728,7 +728,9 @@ pub mod tests {
must_find_attribute, BOND_TOO_FRESH_VALUE, NO_REWARD_REASON_KEY,
OPERATOR_REWARDING_EVENT_TYPE,
};
use mixnet_contract_common::reward_params::{NodeRewardParams, RewardParams};
use mixnet_contract_common::reward_params::{
EpochRewardParams, NodeRewardParams, RewardParams,
};
use mixnet_contract_common::{Delegation, IdentityKey, Interval, Layer, MixNode};
use time::OffsetDateTime;
@@ -1097,6 +1099,9 @@ pub mod tests {
let circulating_supply = storage::circulating_supply(&deps.storage).unwrap().u128();
assert_eq!(circulating_supply, 750_000_000_000_000u128);
let staking_supply = storage::staking_supply(&deps.storage).unwrap().u128();
assert_eq!(staking_supply, 100_000_000_000_000u128);
let sender = Addr::unchecked("alice");
let stake = coins(10_000_000_000, DENOM);
@@ -1112,8 +1117,6 @@ pub mod tests {
let node_identity_1 = keypair.public_key().to_base58_string();
env.block.height;
try_add_mixnode(
deps.as_mut(),
env.clone(),
@@ -1281,15 +1284,22 @@ pub mod tests {
.unwrap();
let mix_after_reward = mixnodes.may_load(&deps.storage, &node_identity_1).unwrap();
// println!("{:?}", mix_after_reward);
let accumulated1 = mix_after_reward
.as_ref()
.unwrap()
.accumulated_rewards
.unwrap()
.u128();
// stupid one-off error, but @DU says it's fine
assert_eq!(accumulated1, 1948911 + 1);
let checkpoints = mixnodes
.changelog()
.prefix(&node_identity_1)
.keys(&deps.storage, None, None, Order::Ascending)
.filter_map(|x| x.ok())
.collect::<Vec<u64>>();
assert_eq!(checkpoints.len(), 2);
.count();
assert_eq!(checkpoints, 2);
env.block.height += 10000;
env.block.time = env.block.time.plus_seconds(3601);
@@ -1330,13 +1340,19 @@ pub mod tests {
try_reward_mixnode(
deps.as_mut(),
env.clone(),
info.clone(),
info,
node_identity_1.clone(),
node_reward_params,
)
.unwrap();
let mix_after_reward_2 = mixnodes.may_load(&deps.storage, &node_identity_1).unwrap();
let accumulated2 = mix_after_reward_2
.as_ref()
.unwrap()
.accumulated_rewards
.unwrap()
.u128();
assert_eq!(accumulated2, accumulated1 + 2728477);
assert_ne!(mix_after_reward, mix_after_reward_2);
let checkpoints = mixnodes
@@ -1370,11 +1386,20 @@ pub mod tests {
try_reward_mixnode(
deps.as_mut(),
env.clone(),
info.clone(),
info,
node_identity_1.clone(),
node_reward_params,
)
.unwrap();
let mix_after_reward_3 = mixnodes.may_load(&deps.storage, &node_identity_1).unwrap();
let accumulated3 = mix_after_reward_3
.as_ref()
.unwrap()
.accumulated_rewards
.unwrap()
.u128();
// off by one : )
assert_eq!(accumulated3 + 1, accumulated2 + 2728477);
let checkpoints = mixnodes
.changelog()
@@ -1431,7 +1456,10 @@ pub mod tests {
let alice_reward =
calculate_delegator_reward(&deps.storage, &deps.api, key.clone(), &node_identity_1)
.unwrap();
assert_eq!(alice_reward, Uint128::new(304552));
// TODO: perform deeper investigation into this number as it seem to not have compounded
// reward on the initial 8000 delegation and only have done it starting from 16000
assert_eq!(alice_reward, Uint128::new(2737978));
let mix_0 = mixnodes.load(&deps.storage, &node_identity_1).unwrap();
@@ -1459,7 +1487,10 @@ pub mod tests {
assert_eq!(delegations.len(), 1);
let delegation = delegations.first().unwrap();
assert_eq!(delegation.amount.amount, Uint128::new(16000000000 + 304552));
assert_eq!(
delegation.amount.amount,
Uint128::new(16000000000 + 2737978)
);
let mix_1 = mixnodes
.load(&deps.storage, &node_identity_1.clone())
@@ -1482,29 +1513,17 @@ pub mod tests {
let operator_reward =
calculate_operator_reward(&deps.storage, &deps.api, &Addr::unchecked("alice"), &mix_1)
.unwrap();
assert_eq!(operator_reward, Uint128::new(352532));
assert_eq!(operator_reward, Uint128::new(2278902));
assert_eq!(
mix_1_reward_result.sigma(),
U128::from_num(0.0000266666666666f64)
);
assert_eq!(
mix_1_reward_result.lambda(),
U128::from_num(0.0000133333333333f64)
);
assert_eq!(mix_1_reward_result.reward().int(), 259114u128);
assert_eq!(mix_1_reward_result.sigma(), U128::from_num(0.0002f64));
assert_eq!(mix_1_reward_result.lambda(), U128::from_num(0.0001f64));
assert_eq!(mix_1_reward_result.reward().int(), accumulated1);
let mix_2_reward_result = mix_2.reward(&params2);
assert_eq!(
mix_2_reward_result.sigma(),
U128::from_num(0.0000266666666666f64)
);
assert_eq!(
mix_2_reward_result.lambda(),
U128::from_num(0.0000133333333333f64)
);
assert_eq!(mix_2_reward_result.reward().int(), 129557u128);
assert_eq!(mix_2_reward_result.sigma(), U128::from_num(0.0002f64));
assert_eq!(mix_2_reward_result.lambda(), U128::from_num(0.0001f64));
assert_eq!(mix_2_reward_result.reward().int(), 974456u128);
let mix_3_reward_result = mix_3.reward(&params3);
@@ -1573,6 +1592,27 @@ pub mod tests {
let interval_reward_params = current_epoch_reward_params(&deps.storage).unwrap();
// =======================
// TODO: this is a temporary 'workaround' until we can produce "good" numbers for if the rewards
// were calculated based of 100M staking supply as opposed to original 750M circulating supply
let interval_reward_params = EpochRewardParams::new(
interval_reward_params.epoch_reward_pool(),
interval_reward_params.rewarded_set_size(),
interval_reward_params.active_set_size(),
circulating_supply,
interval_reward_params.sybil_resistance_percent(),
interval_reward_params.active_set_work_factor(),
);
// this one repeats internals of `save_epoch_reward_params` but with 750M staking supply as opposed to 100M
crate::interval::storage::CURRENT_EPOCH_REWARD_PARAMS
.save(&mut deps.storage, &interval_reward_params)
.unwrap();
crate::rewards::storage::EPOCH_REWARD_PARAMS
.save(&mut deps.storage, epoch.id(), &interval_reward_params)
.unwrap();
// =======================
let node_reward_params = NodeRewardParams::new(0, mix_1_uptime, true);
let mut params = RewardParams::new(interval_reward_params, node_reward_params);
@@ -1635,7 +1675,7 @@ pub mod tests {
try_reward_mixnode(
deps.as_mut(),
env,
info.clone(),
info,
node_identity.clone(),
node_reward_params,
)
+2 -5
View File
@@ -16,8 +16,7 @@ pub(crate) fn is_authorized(sender: String, storage: &dyn Storage) -> Result<(),
}
pub(crate) fn epoch_reward_params(
epoch_id: u32,
storage: &mut dyn Storage,
storage: &dyn Storage,
) -> Result<EpochRewardParams, ContractError> {
let state = crate::mixnet_contract_settings::storage::CONTRACT_STATE
.load(storage)
@@ -30,13 +29,11 @@ pub(crate) fn epoch_reward_params(
(reward_pool.u128() / 100 / epochs_in_interval as u128) * interval_reward_percent as u128,
state.mixnode_rewarded_set_size as u128,
state.mixnode_active_set_size as u128,
crate::rewards::storage::circulating_supply(storage)?.u128(),
state.staking_supply.u128(),
constants::SYBIL_RESISTANCE_PERCENT,
constants::ACTIVE_SET_WORK_FACTOR,
);
crate::rewards::storage::EPOCH_REWARD_PARAMS.save(storage, epoch_id, &epoch_reward_params)?;
Ok(epoch_reward_params)
}