From b09db50bbaba7c0350c00f1c5860701cb350d5fe Mon Sep 17 00:00:00 2001 From: Drazen Urch Date: Fri, 29 Apr 2022 13:09:00 +0200 Subject: [PATCH] Fix checkpointing (#1230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix checkpoint strategy and order, test * Debug pending delegation events * Debug print * Switch print to log * Add debug print * More printing * Remove problematic checkpoint * checkpoint mixnodes in separate block * more debugs * Removing old migration just in case * Printing all checkpoint heights at migration * Purging old checkpoint from storage * Attempting to load raw storage value * More printing... * Removed expect * deserialization changes * error handling * cleanup * clippy * dead code * Reduce minimum age for rewarding to 1 epoch * Add checkpoint query * Get checkpoint at height * Fix delegation compounding, fix undelegate accumulate_rewards * Fix potential overflow, add debug logging * Moar logging * Fix total_delegations + rewards * Better error for horrible overflow * fmt * Fixed unit test assertions in bech32 validation Co-authored-by: Jędrzej Stuczyński --- .../mixnet-contract/src/error.rs | 5 + .../mixnet-contract/src/mixnode.rs | 28 +- .../mixnet-contract/src/msg.rs | 7 + .../mixnet-contract/src/reward_params.rs | 15 +- .../crypto/src/bech32_address_validation.rs | 11 +- contracts/Cargo.lock | 4 +- contracts/mixnet/src/constants.rs | 5 +- contracts/mixnet/src/contract.rs | 28 +- .../mixnet/src/delegations/transactions.rs | 139 ++++--- contracts/mixnet/src/error.rs | 6 + .../mixnet/src/mixnodes/bonding_queries.rs | 22 +- contracts/mixnet/src/mixnodes/storage.rs | 2 +- contracts/mixnet/src/rewards/transactions.rs | 385 ++++++++++++++++-- validator-api/src/nymd_client.rs | 5 +- 14 files changed, 528 insertions(+), 134 deletions(-) diff --git a/common/cosmwasm-smart-contracts/mixnet-contract/src/error.rs b/common/cosmwasm-smart-contracts/mixnet-contract/src/error.rs index f2ae46e01e..008306ec87 100644 --- a/common/cosmwasm-smart-contracts/mixnet-contract/src/error.rs +++ b/common/cosmwasm-smart-contracts/mixnet-contract/src/error.rs @@ -13,4 +13,9 @@ pub enum MixnetContractError { }, #[error("Error casting from U128")] CastError, + #[error("{source}")] + StdErr { + #[from] + source: cosmwasm_std::StdError, + }, } diff --git a/common/cosmwasm-smart-contracts/mixnet-contract/src/mixnode.rs b/common/cosmwasm-smart-contracts/mixnet-contract/src/mixnode.rs index a55a5a3392..b70f774e41 100644 --- a/common/cosmwasm-smart-contracts/mixnet-contract/src/mixnode.rs +++ b/common/cosmwasm-smart-contracts/mixnet-contract/src/mixnode.rs @@ -250,8 +250,14 @@ impl DelegatorRewardParams { #[derive(Debug, Clone, JsonSchema, PartialEq, Serialize, Deserialize, Copy)] pub struct StoredNodeRewardResult { reward: Uint128, - lambda: Uint128, - sigma: Uint128, + + #[schemars(with = "String")] + #[serde(with = "fixed_U128_as_string")] + lambda: U128, + + #[schemars(with = "String")] + #[serde(with = "fixed_U128_as_string")] + sigma: U128, } impl StoredNodeRewardResult { @@ -259,11 +265,11 @@ impl StoredNodeRewardResult { self.reward } - pub fn lambda(&self) -> Uint128 { + pub fn lambda(&self) -> U128 { self.lambda } - pub fn sigma(&self) -> Uint128 { + pub fn sigma(&self) -> U128 { self.sigma } } @@ -279,18 +285,8 @@ impl TryFrom for StoredNodeRewardResult { .checked_cast() .ok_or(MixnetContractError::CastError)?, ), - lambda: Uint128::new( - node_reward_result - .lambda() - .checked_cast() - .ok_or(MixnetContractError::CastError)?, - ), - sigma: Uint128::new( - node_reward_result - .sigma() - .checked_cast() - .ok_or(MixnetContractError::CastError)?, - ), + lambda: node_reward_result.lambda(), + sigma: node_reward_result.sigma(), }) } } diff --git a/common/cosmwasm-smart-contracts/mixnet-contract/src/msg.rs b/common/cosmwasm-smart-contracts/mixnet-contract/src/msg.rs index 9cc651030c..8728df83d1 100644 --- a/common/cosmwasm-smart-contracts/mixnet-contract/src/msg.rs +++ b/common/cosmwasm-smart-contracts/mixnet-contract/src/msg.rs @@ -177,6 +177,13 @@ pub enum QueryMsg { owner_address: String, proxy_address: Option, }, + GetCheckpointsForMixnode { + mix_identity: IdentityKey, + }, + GetMixnodeAtHeight { + mix_identity: IdentityKey, + height: u64, + }, } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] diff --git a/common/cosmwasm-smart-contracts/mixnet-contract/src/reward_params.rs b/common/cosmwasm-smart-contracts/mixnet-contract/src/reward_params.rs index 019de73223..96d618c9a0 100644 --- a/common/cosmwasm-smart-contracts/mixnet-contract/src/reward_params.rs +++ b/common/cosmwasm-smart-contracts/mixnet-contract/src/reward_params.rs @@ -25,11 +25,11 @@ impl NodeEpochRewards { self.epoch_id } - pub fn sigma(&self) -> Uint128 { + pub fn sigma(&self) -> U128 { self.result.sigma() } - pub fn lambda(&self) -> Uint128 { + pub fn lambda(&self) -> U128 { self.result.lambda() } @@ -57,10 +57,8 @@ impl NodeEpochRewards { pub fn operator_reward(&self, profit_margin: U128) -> Result { let reward = self.node_profit(); let operator_base_reward = reward.min(self.operator_cost()); - let operator_reward = (profit_margin - + (ONE - profit_margin) * U128::from_num(self.lambda().u128()) - / U128::from_num(self.sigma().u128())) - * reward; + let operator_reward = + (profit_margin + (ONE - profit_margin) * self.lambda() / self.sigma()) * reward; let reward = (operator_reward + operator_base_reward).max(U128::from_num(0u128)); @@ -82,9 +80,8 @@ impl NodeEpochRewards { let circulating_supply = U128::from_num(epoch_reward_params.circulating_supply()); let scaled_delegation_amount = delegation_amount / circulating_supply; - let delegator_reward = (ONE - profit_margin) * scaled_delegation_amount - / U128::from_num(self.sigma().u128()) - * self.node_profit(); + let delegator_reward = + (ONE - profit_margin) * scaled_delegation_amount / self.sigma() * self.node_profit(); let reward = delegator_reward.max(U128::ZERO); if let Some(int_reward) = reward.checked_cast() { diff --git a/common/crypto/src/bech32_address_validation.rs b/common/crypto/src/bech32_address_validation.rs index 2d9eda5935..2589537f0e 100644 --- a/common/crypto/src/bech32_address_validation.rs +++ b/common/crypto/src/bech32_address_validation.rs @@ -69,9 +69,10 @@ mod tests { #[test] fn wrong_prefix_fails() { assert_eq!( - Err(Bech32Error::WrongPrefix( - "your bech32 address prefix should be nymt, not punk".to_string() - )), + Err(Bech32Error::WrongPrefix(format!( + "your bech32 address prefix should be {}, not punk", + DEFAULT_NETWORK.bech32_prefix() + ))), validate_bech32_prefix("punk1h3w4nj7kny5dfyjw2le4vm74z03v9vd4dstpu0") ) } @@ -80,7 +81,9 @@ mod tests { fn correct_prefix_works() { assert_eq!( Ok(()), - validate_bech32_prefix("nymt1z9egw0knv47nmur0p8vk4rcx59h9gg4zuxrrr9") + validate_bech32_prefix( + "n14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9sjyvg3g" + ) ) } } diff --git a/contracts/Cargo.lock b/contracts/Cargo.lock index cfb4b11862..0e277788b7 100644 --- a/contracts/Cargo.lock +++ b/contracts/Cargo.lock @@ -980,9 +980,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.14" +version = "0.4.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51b9bbe6c47d51fc3e1a9b945965946b4c44142ab8792c50835a980d362c2710" +checksum = "6389c490849ff5bc16be905ae24bc913a9c8892e19b2341dbc175e14c341c2b8" dependencies = [ "cfg-if", ] diff --git a/contracts/mixnet/src/constants.rs b/contracts/mixnet/src/constants.rs index 37ca8a2b90..cb4457c680 100644 --- a/contracts/mixnet/src/constants.rs +++ b/contracts/mixnet/src/constants.rs @@ -1,9 +1,8 @@ // Copyright 2022 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -// approximately 1 week (assuming 5s per block) -// i.e. approximately quarter of the interval (there are 3600 * 60 * 7 = 604800 seconds in a week, i.e. ~604800 / 5 = 120960 blocks) -pub const MINIMUM_BLOCK_AGE_FOR_REWARDING: u64 = 120960; +// approximately 1 epoch (assuming 5s per block) +pub const MINIMUM_BLOCK_AGE_FOR_REWARDING: u64 = 720; pub const INTERVAL_REWARD_PERCENT: u8 = 2; // Used to calculate interval reward pool pub const SYBIL_RESISTANCE_PERCENT: u8 = 30; diff --git a/contracts/mixnet/src/contract.rs b/contracts/mixnet/src/contract.rs index adc1a19357..85a86bcfc3 100644 --- a/contracts/mixnet/src/contract.rs +++ b/contracts/mixnet/src/contract.rs @@ -24,14 +24,17 @@ use crate::mixnet_contract_settings::queries::{ use crate::mixnet_contract_settings::storage as mixnet_params_storage; use crate::mixnet_contract_settings::transactions::try_update_rewarding_validator_address; use crate::mixnodes::bonding_queries as mixnode_queries; -use crate::mixnodes::bonding_queries::query_mixnodes_paged; +use crate::mixnodes::bonding_queries::{ + query_checkpoints_for_mixnode, query_mixnode_at_height, query_mixnodes_paged, +}; use crate::mixnodes::layer_queries::query_layer_distribution; use crate::rewards::queries::{ query_circulating_supply, query_reward_pool, query_rewarding_status, }; use crate::rewards::storage as rewards_storage; use cosmwasm_std::{ - entry_point, to_binary, Addr, Deps, DepsMut, Env, MessageInfo, QueryResponse, Response, Uint128, + entry_point, to_binary, Addr, Api, Deps, DepsMut, Env, MessageInfo, QueryResponse, Response, + Uint128, }; use mixnet_contract_common::mixnode::DelegationEvent; use mixnet_contract_common::{ @@ -54,6 +57,10 @@ pub const INITIAL_ACTIVE_SET_WORK_FACTOR: u8 = 10; pub const DEFAULT_FIRST_INTERVAL_START: OffsetDateTime = time::macros::datetime!(2022-01-01 12:00 UTC); +pub fn debug_with_visibility>(api: &dyn Api, msg: S) { + api.debug(&*format!("\n\n\n=========================================\n{}\n=========================================\n\n\n", msg.into())); +} + fn default_initial_state(owner: Addr, rewarding_validator_address: Addr) -> ContractState { ContractState { owner, @@ -387,13 +394,19 @@ pub fn query(deps: Deps<'_>, env: Env, msg: QueryMsg) -> Result to_binary( &crate::delegations::queries::debug_query_all_delegation_values(deps.storage)?, ), + QueryMsg::GetCheckpointsForMixnode { mix_identity } => { + to_binary(&query_checkpoints_for_mixnode(deps, mix_identity)?) + } + QueryMsg::GetMixnodeAtHeight { + mix_identity, + height, + } => to_binary(&query_mixnode_at_height(deps, mix_identity, height)?), }; Ok(query_res?) } -#[entry_point] -pub fn migrate(deps: DepsMut<'_>, _env: Env, _msg: MigrateMsg) -> Result { +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) @@ -422,6 +435,13 @@ pub fn migrate(deps: DepsMut<'_>, _env: Env, _msg: MigrateMsg) -> Result, _env: Env, _msg: MigrateMsg) -> Result { + deal_with_zero_delegations(deps)?; + Ok(Default::default()) } diff --git a/contracts/mixnet/src/delegations/transactions.rs b/contracts/mixnet/src/delegations/transactions.rs index 940389d4db..6c95ebbce6 100644 --- a/contracts/mixnet/src/delegations/transactions.rs +++ b/contracts/mixnet/src/delegations/transactions.rs @@ -1,6 +1,7 @@ // Copyright 2021 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 use super::storage::{self, PENDING_DELEGATION_EVENTS}; +// use crate::contract::debug_with_visibility; use crate::error::ContractError; use crate::mixnet_contract_settings::storage as mixnet_params_storage; use crate::mixnodes::storage as mixnodes_storage; @@ -28,12 +29,13 @@ pub fn try_reconcile_all_delegation_events( return Err(ContractError::Unauthorized); } - _try_reconcile_all_delegation_events(deps.storage) + _try_reconcile_all_delegation_events(deps.storage, deps.api) } // TODO: Error handling? pub(crate) fn _try_reconcile_all_delegation_events( storage: &mut dyn Storage, + api: &dyn Api, ) -> Result { let pending_delegation_events = PENDING_DELEGATION_EVENTS .range(storage, None, None, Order::Ascending) @@ -53,7 +55,8 @@ pub(crate) fn _try_reconcile_all_delegation_events( response = response.add_event(event); } DelegationEvent::Undelegate(pending_undelegate) => { - let undelegate_response = try_reconcile_undelegation(storage, &pending_undelegate)?; + let undelegate_response = + try_reconcile_undelegation(storage, api, &pending_undelegate)?; response = response.add_event(undelegate_response.event); if let Some(msg) = undelegate_response.bank_msg { response = response.add_message(msg); @@ -101,8 +104,7 @@ pub(crate) fn try_delegate_to_mixnode( let amount = validate_delegation_stake(info.funds)?; _try_delegate_to_mixnode( - deps.storage, - deps.api, + deps, env.block.height, &mix_identity, info.sender.as_str(), @@ -122,8 +124,7 @@ pub(crate) fn try_delegate_to_mixnode_on_behalf( let amount = validate_delegation_stake(info.funds)?; _try_delegate_to_mixnode( - deps.storage, - deps.api, + deps, env.block.height, &mix_identity, &delegate, @@ -173,19 +174,18 @@ pub(crate) fn try_reconcile_delegation( } pub(crate) fn _try_delegate_to_mixnode( - storage: &mut dyn Storage, - api: &dyn Api, + deps: DepsMut<'_>, block_height: u64, mix_identity: &str, delegate: &str, amount: Coin, proxy: Option, ) -> Result { - let delegate = api.addr_validate(delegate)?; + let delegate = deps.api.addr_validate(delegate)?; // check if the target node actually exists if mixnodes_storage::mixnodes() - .may_load(storage, mix_identity)? + .may_load(deps.storage, mix_identity)? .is_none() { return Err(ContractError::MixNodeBondNotFound { @@ -202,7 +202,7 @@ pub(crate) fn _try_delegate_to_mixnode( ); if storage::PENDING_DELEGATION_EVENTS - .may_load(storage, delegation.event_storage_key())? + .may_load(deps.storage, delegation.event_storage_key())? .is_some() { return Err(ContractError::DelegationEventAlreadyPending { @@ -213,7 +213,7 @@ pub(crate) fn _try_delegate_to_mixnode( } storage::PENDING_DELEGATION_EVENTS.save( - storage, + deps.storage, delegation.event_storage_key(), &DelegationEvent::Delegate(delegation), )?; @@ -253,10 +253,13 @@ pub struct ReconcileUndelegateResponse { pub(crate) fn try_reconcile_undelegation( storage: &mut dyn Storage, + _api: &dyn Api, pending_undelegate: &PendingUndelegate, ) -> Result { let delegation_map = storage::delegations(); + // debug_with_visibility(api, "Reconciling undelegations"); + let any_delegations = delegation_map .prefix(pending_undelegate.storage_key()) .keys(storage, None, None, cosmwasm_std::Order::Ascending) @@ -284,6 +287,8 @@ pub(crate) fn try_reconcile_undelegation( &pending_undelegate.mix_identity(), )?; + // debug_with_visibility(api, format!("Delegator reward: {}", reward)); + // Might want to introduce paging here let delegation_heights = delegation_map .prefix(pending_undelegate.storage_key()) @@ -305,7 +310,26 @@ pub(crate) fn try_reconcile_undelegation( }); } - let mut total_delegation = reward; + let mut total_delegation = Uint128::zero(); + + // debug_with_visibility(api, "Reducing accumulated rewards"); + + { + if let Some(mut bond) = crate::mixnodes::storage::mixnodes() + .may_load(storage, &pending_undelegate.mix_identity())? + { + let remaining = bond.accumulated_rewards().saturating_sub(reward); + // debug_with_visibility(api, format!("Remaining accumulated rewards: {}", remaining)); + bond.accumulated_rewards = Some(remaining); + + crate::mixnodes::storage::mixnodes().save( + storage, + &pending_undelegate.mix_identity(), + &bond, + pending_undelegate.block_height(), + )?; + } + } for h in delegation_heights { let delegation = @@ -319,6 +343,40 @@ pub(crate) fn try_reconcile_undelegation( )?; } + mixnodes_storage::TOTAL_DELEGATION.update::<_, ContractError>( + storage, + &pending_undelegate.mix_identity(), + |total_node_delegation| { + // debug_with_visibility(api, "Setting total delegation"); + let remaining = match total_node_delegation.unwrap().checked_sub(total_delegation) { + Ok(remaining) => remaining, + Err(_) => { + // debug_with_visibility( + // api, + // format!( + // "Overflowed delegation subsctraction, {} - {}", + // total_node_delegation.unwrap(), + // total_delegation + // ), + // ); + return Err(ContractError::TotalDelegationSubOverflow { + mix_identity: pending_undelegate.mix_identity(), + total_node_delegation: total_node_delegation.unwrap().u128(), + to_subtract: total_delegation.u128(), + }); + } + }; + // debug_with_visibility(api, format!("Remaining total delegation: {}", remaining)); + // the first unwrap is fine because the delegation information MUST exist, otherwise we would + // have never gotten here in the first place + // the second unwrap is also fine because we should NEVER underflow here, + // if we do, it means we have some serious error in our logic + Ok(remaining) + }, + )?; + + let total_funds = total_delegation + reward; + // don't add a bank message if it would have resulted in attempting to send 0 tokens let bank_msg = if total_delegation != Uint128::zero() { Some(BankMsg::Send { @@ -327,34 +385,19 @@ pub(crate) fn try_reconcile_undelegation( .as_ref() .unwrap_or(&pending_undelegate.delegate()) .to_string(), - amount: coins(total_delegation.u128(), DENOM), + amount: coins(total_funds.u128(), DENOM), }) } else { None }; - mixnodes_storage::TOTAL_DELEGATION.update::<_, ContractError>( - storage, - &pending_undelegate.mix_identity(), - |total_node_delegation| { - // the first unwrap is fine because the delegation information MUST exist, otherwise we would - // have never gotten here in the first place - // the second unwrap is also fine because we should NEVER underflow here, - // if we do, it means we have some serious error in our logic - Ok(total_node_delegation - .unwrap() - .checked_sub(total_delegation) - .unwrap()) - }, - )?; - let mut wasm_msg = None; if let Some(proxy) = &pending_undelegate.proxy() { let msg = Some(VestingContractExecuteMsg::TrackUndelegation { owner: pending_undelegate.delegate().as_str().to_string(), mix_identity: pending_undelegate.mix_identity(), - amount: Coin::new(total_delegation.u128(), DENOM), + amount: Coin::new(total_funds.u128(), DENOM), }); wasm_msg = Some(wasm_execute(proxy, &msg, vec![one_ucoin()])?); @@ -364,9 +407,11 @@ pub(crate) fn try_reconcile_undelegation( &pending_undelegate.delegate(), &pending_undelegate.proxy(), &pending_undelegate.mix_identity(), - total_delegation, + total_funds, ); + // debug_with_visibility(api, "Done"); + Ok(ReconcileUndelegateResponse { bank_msg, wasm_msg, @@ -519,7 +564,7 @@ mod tests { ) .is_ok()); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); let expected = Delegation::new( delegation_owner.clone(), @@ -598,7 +643,7 @@ mod tests { ) .is_ok()); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); let expected = Delegation::new( delegation_owner.clone(), @@ -661,7 +706,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); // let expected = Delegation::new( // delegation_owner.clone(), @@ -716,7 +761,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); assert_eq!( initial_height, @@ -737,7 +782,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); let delegations = crate::delegations::queries::query_mixnode_delegation( &deps.storage, @@ -782,7 +827,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); assert_eq!( initial_height, @@ -803,7 +848,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); assert_eq!( initial_height, @@ -891,7 +936,7 @@ mod tests { ) .is_ok()); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); let expected1 = Delegation::new( delegation_owner.clone(), @@ -956,7 +1001,7 @@ mod tests { identity.clone(), ) .is_ok()); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); // node's "total_delegation" is sum of both assert_eq!( @@ -986,7 +1031,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); try_remove_mixnode(mock_env(), deps.as_mut(), mock_info(mixnode_owner, &[])).unwrap(); @@ -1074,7 +1119,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); let _delegation = query_mixnode_delegation( &deps.storage, @@ -1144,7 +1189,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); let delegation = query_mixnode_delegation( &deps.storage, @@ -1179,7 +1224,7 @@ mod tests { ) ); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); assert!(test_helpers::read_delegation( &deps.storage, @@ -1212,7 +1257,7 @@ mod tests { ) .is_ok()); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); assert!(try_delegate_to_mixnode( deps.as_mut(), @@ -1222,7 +1267,7 @@ mod tests { ) .is_ok()); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); // sender1 undelegates try_remove_delegation_from_mixnode( @@ -1233,7 +1278,7 @@ mod tests { ) .unwrap(); - _try_reconcile_all_delegation_events(&mut deps.storage).unwrap(); + _try_reconcile_all_delegation_events(&mut deps.storage, &deps.api).unwrap(); // but total delegation should still equal to what sender2 sent // node's "total_delegation" is sum of both assert_eq!( diff --git a/contracts/mixnet/src/error.rs b/contracts/mixnet/src/error.rs index dee40261f2..3941d701c7 100644 --- a/contracts/mixnet/src/error.rs +++ b/contracts/mixnet/src/error.rs @@ -168,4 +168,10 @@ pub enum ContractError { identity: String, kind: String, // delegation | undelegation }, + #[error("Attempted to subsctract more then the total delegation, this MUST never happen! mix: {mix_identity}, total_node_delegation {total_node_delegation}, to_subtract {to_subtract}")] + TotalDelegationSubOverflow { + mix_identity: String, + total_node_delegation: u128, + to_subtract: u128, + }, } diff --git a/contracts/mixnet/src/mixnodes/bonding_queries.rs b/contracts/mixnet/src/mixnodes/bonding_queries.rs index 876b9e36b3..0424d4e8ce 100644 --- a/contracts/mixnet/src/mixnodes/bonding_queries.rs +++ b/contracts/mixnet/src/mixnodes/bonding_queries.rs @@ -1,13 +1,33 @@ // Copyright 2021 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -use super::storage; +use super::storage::{self, StoredMixnodeBond}; use cosmwasm_std::{Deps, Order, StdResult}; use cw_storage_plus::Bound; use mixnet_contract_common::{ IdentityKey, MixNodeBond, MixOwnershipResponse, PagedMixnodeResponse, }; +pub fn query_mixnode_at_height( + deps: Deps<'_>, + mix_identity: String, + height: u64, +) -> StdResult> { + storage::mixnodes().may_load_at_height(deps.storage, &mix_identity, height) +} + +pub fn query_checkpoints_for_mixnode( + deps: Deps<'_>, + mix_identity: IdentityKey, +) -> StdResult> { + Ok(storage::mixnodes() + .changelog() + .prefix(&mix_identity) + .keys(deps.storage, None, None, Order::Ascending) + .filter_map(|x| x.ok()) + .collect()) +} + pub fn query_mixnodes_paged( deps: Deps<'_>, start_after: Option, diff --git a/contracts/mixnet/src/mixnodes/storage.rs b/contracts/mixnet/src/mixnodes/storage.rs index 51abff6dbb..7163d78459 100644 --- a/contracts/mixnet/src/mixnodes/storage.rs +++ b/contracts/mixnet/src/mixnodes/storage.rs @@ -55,7 +55,7 @@ pub(crate) fn mixnodes<'a>( MIXNODES_PK_NAMESPACE, MIXNODES_PK_CHECKPOINTS, MIXNODES_PK_CHANGELOG, - Strategy::Never, + Strategy::Selected, indexes, ) } diff --git a/contracts/mixnet/src/rewards/transactions.rs b/contracts/mixnet/src/rewards/transactions.rs index 68fb85a02c..76a68fef59 100644 --- a/contracts/mixnet/src/rewards/transactions.rs +++ b/contracts/mixnet/src/rewards/transactions.rs @@ -14,7 +14,7 @@ use crate::mixnodes::storage::{self as mixnodes_storage, StoredMixnodeBond}; use crate::rewards::helpers; use crate::support::helpers::is_authorized; use config::defaults::DENOM; -use cosmwasm_std::{Addr, Api, Coin, DepsMut, Env, MessageInfo, Order, Response, Storage, Uint128}; +use cosmwasm_std::{Addr, Coin, DepsMut, Env, MessageInfo, Order, Response, Storage, Uint128}; use mixnet_contract_common::events::{ new_compound_delegator_reward_event, new_compound_operator_reward_event, new_mix_operator_rewarding_event, new_not_found_mix_operator_rewarding_event, @@ -114,7 +114,7 @@ pub fn calculate_operator_reward( if let Some(bond) = mixnodes().may_load_at_height(storage, bond.identity().as_str(), height)? { - if let Some(epoch_rewards) = bond.epoch_rewards { + if let Some(ref epoch_rewards) = bond.epoch_rewards { let epoch_reward_params = epoch_reward_params_for_id(storage, epoch_rewards.epoch_id())?; // Compound rewards from previous heights @@ -150,8 +150,7 @@ pub fn try_compound_delegator_reward_on_behalf( let owner = deps.api.addr_validate(&owner)?; let reward = _try_compound_delegator_reward( env.block.height, - deps.api, - deps.storage, + deps, owner.as_str(), &mix_identity, Some(proxy.clone()), @@ -176,8 +175,7 @@ pub fn try_compound_delegator_reward( let owner = deps.api.addr_validate(info.sender.as_str())?; let reward = _try_compound_delegator_reward( env.block.height, - deps.api, - deps.storage, + deps, owner.as_str(), &mix_identity, None, @@ -195,8 +193,7 @@ pub fn try_compound_delegator_reward( pub fn _try_compound_delegator_reward( block_height: u64, - api: &dyn Api, - storage: &mut dyn Storage, + mut deps: DepsMut<'_>, owner_address: &str, mix_identity: &str, proxy: Option, @@ -204,25 +201,25 @@ pub fn _try_compound_delegator_reward( let delegation_map = crate::delegations::storage::delegations(); let key = mixnet_contract_common::delegation::generate_storage_key( - &api.addr_validate(owner_address)?, + &deps.api.addr_validate(owner_address)?, proxy.as_ref(), ); - let reward = calculate_delegator_reward(storage, key.clone(), mix_identity)?; + let reward = calculate_delegator_reward(deps.storage, key.clone(), mix_identity)?; let mut compounded_delegation = reward; // Might want to introduce paging here let delegation_heights = delegation_map .prefix((mix_identity.to_string(), key.clone())) - .keys(storage, None, None, cosmwasm_std::Order::Ascending) + .keys(deps.storage, None, None, cosmwasm_std::Order::Ascending) .filter_map(|v| v.ok()) .collect::>(); for h in delegation_heights { let delegation = - delegation_map.load(storage, (mix_identity.to_string(), key.clone(), h))?; + delegation_map.load(deps.storage, (mix_identity.to_string(), key.clone(), h))?; compounded_delegation += delegation.amount.amount; delegation_map.replace( - storage, + deps.storage, (mix_identity.to_string(), key.clone(), h), None, Some(&delegation), @@ -231,13 +228,12 @@ pub fn _try_compound_delegator_reward( if compounded_delegation != Uint128::zero() { _try_delegate_to_mixnode( - storage, - api, + deps.branch(), block_height, mix_identity, owner_address, Coin { - amount: reward, + amount: compounded_delegation, denom: DENOM.to_string(), }, proxy, @@ -245,15 +241,14 @@ pub fn _try_compound_delegator_reward( } { - //TODO: Node exists all is well, life goes on, if it does not exist we'll just return the reward to the caller as there is nothing to do on the bond - if let Some(mut bond) = mixnodes().may_load(storage, mix_identity)? { + if let Some(mut bond) = mixnodes().may_load(deps.storage, mix_identity)? { bond.accumulated_rewards = Some(bond.accumulated_rewards() - reward); - mixnodes().save(storage, mix_identity, &bond, block_height)?; + mixnodes().save(deps.storage, mix_identity, &bond, block_height)?; } } DELEGATOR_REWARD_CLAIMED_HEIGHT.save( - storage, + deps.storage, (key, mix_identity.to_string()), &block_height, )?; @@ -289,6 +284,7 @@ pub fn calculate_delegator_reward( .prefix(mix_identity) .keys(storage, None, None, Order::Ascending) .filter_map(|height| height.ok()) + // Get all checkpoints greater then last claimed delegation height .filter(|height| last_claimed_height <= *height) .fold( Ok(Uint128::zero()), @@ -296,23 +292,28 @@ pub fn calculate_delegator_reward( let accumulated_reward = acc?; let delegation_at_height = delegations .iter() - .filter(|d| height <= d.block_height) + .filter(|d| d.block_height <= height) .fold(Uint128::zero(), |total, delegation| { total + delegation.amount.amount }); if delegation_at_height != Uint128::zero() { - if let Some(bond) = - mixnodes().may_load_at_height(storage, mix_identity, height)? + if let Some(bond) = mixnodes() + .may_load_at_height(storage, mix_identity, height) + .ok() + .flatten() { - if let Some(epoch_rewards) = bond.epoch_rewards { + if let Some(ref epoch_rewards) = bond.epoch_rewards { // Compound rewards from previous heights let epoch_reward_params = epoch_reward_params_for_id(storage, epoch_rewards.epoch_id())?; - let reward_at_height = epoch_rewards.delegation_reward( + let reward_at_height = match epoch_rewards.delegation_reward( delegation_at_height + accumulated_reward, bond.profit_margin(), epoch_reward_params, - )?; + ) { + Ok(reward) => reward, + Err(_err) => Uint128::zero(), + }; return Ok(accumulated_reward + reward_at_height); } } @@ -455,7 +456,9 @@ pub(crate) fn try_reward_mixnode( pub mod tests { use super::*; use crate::constants::EPOCHS_IN_INTERVAL; - use crate::delegations::transactions::try_delegate_to_mixnode; + use crate::delegations::transactions::{ + _try_remove_delegation_from_mixnode, try_delegate_to_mixnode, + }; use crate::error::ContractError; use crate::interval::storage::{ current_epoch_reward_params, save_epoch, save_epoch_reward_params, @@ -472,7 +475,7 @@ pub mod tests { use az::CheckedCast; use config::defaults::DENOM; use cosmwasm_std::testing::{mock_env, mock_info}; - use cosmwasm_std::{coin, coins, Addr, Timestamp, Uint128}; + use cosmwasm_std::{coin, coins, Addr, StdError, Timestamp, Uint128}; use mixnet_contract_common::events::{ must_find_attribute, BOND_TOO_FRESH_VALUE, NO_REWARD_REASON_KEY, OPERATOR_REWARDING_EVENT_TYPE, @@ -808,9 +811,13 @@ pub mod tests { } #[test] - fn test_reward_additivity() { + fn test_reward_additivity_and_snapshots() { use crate::constants::INTERVAL_REWARD_PERCENT; use crate::contract::INITIAL_REWARD_POOL; + use crate::mixnodes::transactions::try_add_mixnode; + use rand::thread_rng; + + let mixnodes = crate::mixnodes::storage::mixnodes(); type U128 = fixed::types::U75F53; @@ -820,14 +827,81 @@ pub mod tests { .load(deps.as_ref().storage) .unwrap(); let rewarding_validator_address = current_state.rewarding_validator_address; + + let info = mock_info(rewarding_validator_address.as_str(), &[]); + + crate::mixnodes::transactions::try_checkpoint_mixnodes( + &mut deps.storage, + env.block.height, + info.clone(), + ) + .unwrap(); + let checkpoints = mixnodes + .changelog() + .keys(&deps.storage, None, None, Order::Ascending) + .filter_map(|x| x.ok()) + .collect::>(); + assert_eq!(0, checkpoints.len()); + let period_reward_pool = (INITIAL_REWARD_POOL / 100 / EPOCHS_IN_INTERVAL as u128) * INTERVAL_REWARD_PERCENT as u128; assert_eq!(period_reward_pool, 6_944_444_444); let circulating_supply = storage::circulating_supply(&deps.storage).unwrap().u128(); assert_eq!(circulating_supply, 750_000_000_000_000u128); - let node_owner: Addr = Addr::unchecked("alice"); - let node_identity = test_helpers::add_mixnode( + let sender = Addr::unchecked("alice"); + let stake = coins(10_000_000_000, DENOM); + + let keypair = crypto::asymmetric::identity::KeyPair::new(&mut thread_rng()); + let owner_signature = keypair + .private_key() + .sign(sender.as_bytes()) + .to_base58_string(); + + let legit_sphinx_key = crypto::asymmetric::encryption::KeyPair::new(&mut thread_rng()); + + let info = mock_info(sender.as_str(), &stake); + + let node_identity_1 = keypair.public_key().to_base58_string(); + + env.block.height; + + try_add_mixnode( + deps.as_mut(), + env.clone(), + info.clone(), + MixNode { + identity_key: node_identity_1.clone(), + sphinx_key: legit_sphinx_key.public_key().to_base58_string(), + ..tests::fixtures::mix_node_fixture() + }, + owner_signature, + ) + .unwrap(); + + // tick + + env.block.height += 1; + + let info = mock_info(rewarding_validator_address.as_str(), &[]); + crate::mixnodes::transactions::try_checkpoint_mixnodes( + &mut deps.storage, + env.block.height, + info.clone(), + ) + .unwrap(); + mixnodes + .assert_checkpointed(&deps.storage, env.block.height) + .unwrap(); + let checkpoints = mixnodes + .changelog() + .keys(&deps.storage, None, None, Order::Ascending) + .filter_map(|x| x.ok()) + .collect::>(); + assert_eq!(checkpoints.len(), 1); + + let node_owner: Addr = Addr::unchecked("johnny"); + let node_identity_2 = test_helpers::add_mixnode( node_owner.as_str(), coins(10_000_000_000, DENOM), deps.as_mut(), @@ -837,7 +911,7 @@ pub mod tests { deps.as_mut(), mock_env(), mock_info("alice_d1", &[coin(8000_000000, DENOM)]), - node_identity.clone(), + node_identity_1.clone(), ) .unwrap(); @@ -845,17 +919,10 @@ pub mod tests { deps.as_mut(), mock_env(), mock_info("alice_d2", &[coin(2000_000000, DENOM)]), - node_identity.clone(), + node_identity_1.clone(), ) .unwrap(); - let node_owner: Addr = Addr::unchecked("bob"); - let node_identity_2 = test_helpers::add_mixnode( - node_owner.as_str(), - coins(10_000_000_000, DENOM), - deps.as_mut(), - ); - try_delegate_to_mixnode( deps.as_mut(), mock_env(), @@ -895,13 +962,16 @@ pub mod tests { ) .unwrap(); - crate::delegations::transactions::_try_reconcile_all_delegation_events(&mut deps.storage) - .unwrap(); + crate::delegations::transactions::_try_reconcile_all_delegation_events( + &mut deps.storage, + &deps.api, + ) + .unwrap(); let info = mock_info(rewarding_validator_address.as_ref(), &[]); env.block.height += 2 * constants::MINIMUM_BLOCK_AGE_FOR_REWARDING; - let mix_1 = mixnodes_storage::read_full_mixnode_bond(&deps.storage, &node_identity) + let mix_1 = mixnodes_storage::read_full_mixnode_bond(&deps.storage, &node_identity_1) .unwrap() .unwrap(); let mix_1_uptime = 100; @@ -942,6 +1012,228 @@ pub mod tests { let mix_1_reward_result = mix_1.reward(¶ms); + let info = mock_info(rewarding_validator_address.as_str(), &[]); + crate::mixnodes::transactions::try_checkpoint_mixnodes( + &mut deps.storage, + env.block.height, + info.clone(), + ) + .unwrap(); + mixnodes + .assert_checkpointed(&deps.storage, env.block.height) + .unwrap(); + + try_reward_mixnode( + deps.as_mut(), + env.clone(), + info.clone(), + node_identity_1.clone(), + node_reward_params, + ) + .unwrap(); + + let mix_after_reward = mixnodes.may_load(&deps.storage, &node_identity_1).unwrap(); + // println!("{:?}", mix_after_reward); + + let checkpoints = mixnodes + .changelog() + .prefix(&node_identity_1) + .keys(&deps.storage, None, None, Order::Ascending) + .filter_map(|x| x.ok()) + .collect::>(); + assert_eq!(checkpoints.len(), 2); + + env.block.height += 10000; + env.block.time = env.block.time.plus_seconds(3601); + + try_advance_epoch( + env.clone(), + &mut deps.storage, + rewarding_validator_address.to_string(), + ) + .unwrap(); + + // After two snapshots we should see an increase in delegation + try_delegate_to_mixnode( + deps.as_mut(), + env.clone(), + mock_info("alice_d1", &[coin(8000_000000, DENOM)]), + node_identity_1.clone(), + ) + .unwrap(); + + crate::delegations::transactions::_try_reconcile_all_delegation_events( + &mut deps.storage, + &deps.api, + ) + .unwrap(); + + let info = mock_info(rewarding_validator_address.as_str(), &[]); + crate::mixnodes::transactions::try_checkpoint_mixnodes( + &mut deps.storage, + env.block.height, + info.clone(), + ) + .unwrap(); + mixnodes + .assert_checkpointed(&deps.storage, env.block.height) + .unwrap(); + + try_reward_mixnode( + deps.as_mut(), + env.clone(), + info.clone(), + node_identity_1.clone(), + node_reward_params, + ) + .unwrap(); + let mix_after_reward_2 = mixnodes.may_load(&deps.storage, &node_identity_1).unwrap(); + + assert_ne!(mix_after_reward, mix_after_reward_2); + + let checkpoints = mixnodes + .changelog() + .prefix(&node_identity_1) + .keys(&deps.storage, None, None, Order::Ascending) + .collect::>>(); + assert_eq!(checkpoints.len(), 3); + + env.block.height += 10000; + env.block.time = env.block.time.plus_seconds(3601); + + try_advance_epoch( + env.clone(), + &mut deps.storage, + rewarding_validator_address.to_string(), + ) + .unwrap(); + + let info = mock_info(rewarding_validator_address.as_str(), &[]); + crate::mixnodes::transactions::try_checkpoint_mixnodes( + &mut deps.storage, + env.block.height, + info.clone(), + ) + .unwrap(); + mixnodes + .assert_checkpointed(&deps.storage, env.block.height) + .unwrap(); + + try_reward_mixnode( + deps.as_mut(), + env.clone(), + info.clone(), + node_identity_1.clone(), + node_reward_params, + ) + .unwrap(); + + let checkpoints = mixnodes + .changelog() + .prefix(&node_identity_1) + .keys(&deps.storage, None, None, Order::Ascending) + .filter_map(|x| x.ok()) + .collect::>(); + assert_eq!(checkpoints.len(), 4); + + let delegation_map = crate::delegations::storage::delegations(); + let key = "alice_d1".as_bytes().to_vec(); + + let last_claimed_height = storage::DELEGATOR_REWARD_CLAIMED_HEIGHT + .load(&deps.storage, (key.clone(), node_identity_1.to_string())) + .unwrap_or(0); + + assert_eq!(last_claimed_height, 0); + + let viable_delegations = delegation_map + .prefix((node_identity_1.to_string(), key.clone())) + .range(&deps.storage, None, None, Order::Descending) + .filter_map(|record| record.ok()) + .filter(|(height, _)| last_claimed_height <= *height) + .map(|(_, delegation)| delegation) + .collect::>(); + + assert_eq!(viable_delegations.len(), 2); + + let viable_heights = mixnodes + .changelog() + .prefix(&node_identity_1) + .keys(&deps.storage, None, None, Order::Ascending) + .filter_map(|height| height.ok()) + .filter(|height| last_claimed_height <= *height) + .collect::>(); + + // Should be equal to the number of checkpoints + assert_eq!(viable_heights.len(), 4); + + for (i, h) in viable_heights.into_iter().enumerate() { + let delegation_at_height = viable_delegations + .iter() + .filter(|d| d.block_height <= h) + .fold(Uint128::zero(), |total, delegation| { + total + delegation.amount.amount + }); + if i < 2 { + assert_eq!(delegation_at_height, Uint128::new(8000000000)); + } else { + assert_eq!(delegation_at_height, Uint128::new(16000000000)); + } + } + + let alice_reward = + calculate_delegator_reward(&deps.storage, key.clone(), &node_identity_1).unwrap(); + assert_eq!(alice_reward, Uint128::new(304552)); + + let mix_0 = mixnodes.load(&deps.storage, &node_identity_1).unwrap(); + + _try_compound_delegator_reward( + env.block.height, + deps.as_mut(), + "alice_d1", + &node_identity_1, + None, + ) + .unwrap(); + + crate::delegations::transactions::_try_reconcile_all_delegation_events( + &mut deps.storage, + &deps.api, + ) + .unwrap(); + + let delegations = crate::delegations::storage::delegations() + .prefix((node_identity_1.to_string(), key.clone())) + .range(&deps.storage, None, None, Order::Ascending) + .filter_map(|x| x.ok()) + .map(|(_, delegation)| delegation) + .collect::>(); + assert_eq!(delegations.len(), 1); + + let delegation = delegations.first().unwrap(); + assert_eq!(delegation.amount.amount, Uint128::new(16000000000 + 304552)); + + let mix_1 = mixnodes + .load(&deps.storage, &node_identity_1.clone()) + .unwrap(); + + _try_remove_delegation_from_mixnode(deps.as_mut(), env, node_identity_1, "alice_d1", None) + .unwrap(); + + crate::delegations::transactions::_try_reconcile_all_delegation_events( + &mut deps.storage, + &deps.api, + ) + .unwrap(); + + assert_eq!( + mix_0.accumulated_rewards(), + mix_1.accumulated_rewards() + alice_reward + ); + + let operator_reward = + calculate_operator_reward(&deps.storage, &Addr::unchecked("alice"), &mix_1).unwrap(); + assert_eq!(operator_reward, Uint128::new(190345)); + assert_eq!( mix_1_reward_result.sigma(), U128::from_num(0.0000266666666666f64) @@ -1011,8 +1303,11 @@ pub mod tests { ) .unwrap(); - crate::delegations::transactions::_try_reconcile_all_delegation_events(&mut deps.storage) - .unwrap(); + crate::delegations::transactions::_try_reconcile_all_delegation_events( + &mut deps.storage, + &deps.api, + ) + .unwrap(); let info = mock_info(rewarding_validator_address.as_ref(), &[]); env.block.height += 2 * constants::MINIMUM_BLOCK_AGE_FOR_REWARDING; diff --git a/validator-api/src/nymd_client.rs b/validator-api/src/nymd_client.rs index 9c4ab77faf..8ece9ec6aa 100644 --- a/validator-api/src/nymd_client.rs +++ b/validator-api/src/nymd_client.rs @@ -321,11 +321,12 @@ impl Client { where C: SigningCosmWasmClient + Sync, { - let mut msgs = reward_msgs; + // // First we create the checkpoint, all subsequent changes to a node will be made to the checkpoint + let mut msgs = vec![(ExecuteMsg::CheckpointMixnodes {}, vec![])]; + msgs.extend(reward_msgs); let epoch_msgs = vec![ (ExecuteMsg::ReconcileDelegations {}, vec![]), - (ExecuteMsg::CheckpointMixnodes {}, vec![]), (ExecuteMsg::AdvanceCurrentEpoch {}, vec![]), ( ExecuteMsg::WriteRewardedSet {