diff --git a/contracts/mixnet/src/helpers.rs b/contracts/mixnet/src/helpers.rs index 9e9aebf001..0b3c83fefb 100644 --- a/contracts/mixnet/src/helpers.rs +++ b/contracts/mixnet/src/helpers.rs @@ -99,6 +99,7 @@ pub struct Delegations<'a, T: Clone + Serialize + DeserializeOwned> { last_page: bool, } +#[cfg(test)] impl<'a, T: Clone + Serialize + DeserializeOwned> Delegations<'a, T> { pub fn new(delegations_bucket: ReadonlyBucket<'a, T>) -> Self { Delegations { diff --git a/contracts/mixnet/src/queries.rs b/contracts/mixnet/src/queries.rs index 0239943287..e4d283ff94 100644 --- a/contracts/mixnet/src/queries.rs +++ b/contracts/mixnet/src/queries.rs @@ -7,6 +7,7 @@ use crate::storage::{ all_mix_delegations_read, circulating_supply, config_read, gateways_owners_read, gateways_read, mix_delegations_read, mixnodes_owners_read, mixnodes_read, read_layer_distribution, read_state_params, reverse_mix_delegations_read, reward_pool_value, rewarded_mixnodes_read, + total_delegation_read, }; use config::defaults::DENOM; use cosmwasm_std::{coin, Addr, Deps, Order, StdResult, Uint128}; @@ -17,7 +18,7 @@ use mixnet_contract::{ PagedReverseMixDelegationsResponse, RawDelegationData, RewardingIntervalResponse, StateParams, }; -const BOND_PAGE_MAX_LIMIT: u32 = 100; +const BOND_PAGE_MAX_LIMIT: u32 = 75; const BOND_PAGE_DEFAULT_LIMIT: u32 = 50; // currently the maximum limit before running into memory issue is somewhere between 1150 and 1200 @@ -38,7 +39,16 @@ pub fn query_mixnodes_paged( .range(start.as_deref(), None, Order::Ascending) .take(limit) .map(|res| res.map(|item| item.1)) - .collect::>>()?; + .map(|stored_bond| { + // I really don't like this additional read per entry, but I don't see an obvious way to remove it + stored_bond.map(|stored_bond| { + let total_delegation = + total_delegation_read(deps.storage).load(stored_bond.identity().as_bytes()); + total_delegation + .map(|total_delegation| stored_bond.attach_delegation(total_delegation)) + }) + }) + .collect::>>>()??; let start_next_after = nodes.last().map(|node| node.identity().clone()); @@ -241,7 +251,7 @@ pub(crate) fn query_rewarding_status( pub(crate) mod tests { use super::*; use crate::state::State; - use crate::storage::{config, gateways, mix_delegations, mixnodes}; + use crate::storage::{config, gateways, mix_delegations}; use crate::support::tests::helpers; use crate::support::tests::helpers::{ good_gateway_bond, good_mixnode_bond, raw_delegation_fixture, @@ -261,12 +271,10 @@ pub(crate) mod tests { #[test] fn mixnodes_paged_retrieval_obeys_limits() { let mut deps = helpers::init_contract(); - let storage = deps.as_mut().storage; let limit = 2; for n in 0..10000 { let key = format!("bond{}", n); - let node = helpers::mixnode_bond_fixture(); - mixnodes(storage).save(key.as_bytes(), &node).unwrap(); + helpers::add_mixnode(&key, good_mixnode_bond(), deps.as_mut()); } let page1 = query_mixnodes_paged(deps.as_ref(), None, Option::from(limit)).unwrap(); @@ -276,11 +284,9 @@ pub(crate) mod tests { #[test] fn mixnodes_paged_retrieval_has_default_limit() { let mut deps = helpers::init_contract(); - let storage = deps.as_mut().storage; for n in 0..100 { let key = format!("bond{}", n); - let node = helpers::mixnode_bond_fixture(); - mixnodes(storage).save(key.as_bytes(), &node).unwrap(); + helpers::add_mixnode(&key, good_mixnode_bond(), deps.as_mut()); } // query without explicitly setting a limit @@ -293,11 +299,9 @@ pub(crate) mod tests { #[test] fn mixnodes_paged_retrieval_has_max_limit() { let mut deps = helpers::init_contract(); - let storage = deps.as_mut().storage; for n in 0..10000 { let key = format!("bond{}", n); - let node = helpers::mixnode_bond_fixture(); - mixnodes(storage).save(key.as_bytes(), &node).unwrap(); + helpers::add_mixnode(&key, good_mixnode_bond(), deps.as_mut()); } // query with a crazily high limit in an attempt to use too many resources @@ -305,7 +309,7 @@ pub(crate) mod tests { let page1 = query_mixnodes_paged(deps.as_ref(), None, Option::from(crazy_limit)).unwrap(); // we default to a decent sized upper bound instead - let expected_limit = 100; + let expected_limit = BOND_PAGE_MAX_LIMIT; assert_eq!(expected_limit, page1.nodes.len() as u32); } @@ -317,10 +321,7 @@ pub(crate) mod tests { let addr4 = "hal103"; let mut deps = helpers::init_contract(); - let node = helpers::mixnode_bond_fixture(); - mixnodes(&mut deps.storage) - .save(addr1.as_bytes(), &node) - .unwrap(); + let _identity1 = helpers::add_mixnode(&addr1, good_mixnode_bond(), deps.as_mut()); let per_page = 2; let page1 = query_mixnodes_paged(deps.as_ref(), None, Option::from(per_page)).unwrap(); @@ -329,24 +330,20 @@ pub(crate) mod tests { assert_eq!(1, page1.nodes.len()); // save another - mixnodes(&mut deps.storage) - .save(addr2.as_bytes(), &node) - .unwrap(); + let identity2 = helpers::add_mixnode(&addr2, good_mixnode_bond(), deps.as_mut()); // page1 should have 2 results on it let page1 = query_mixnodes_paged(deps.as_ref(), None, Option::from(per_page)).unwrap(); assert_eq!(2, page1.nodes.len()); - mixnodes(&mut deps.storage) - .save(addr3.as_bytes(), &node) - .unwrap(); + let _identity3 = helpers::add_mixnode(&addr3, good_mixnode_bond(), deps.as_mut()); // page1 still has 2 results let page1 = query_mixnodes_paged(deps.as_ref(), None, Option::from(per_page)).unwrap(); assert_eq!(2, page1.nodes.len()); // retrieving the next page should start after the last key on this page - let start_after = String::from(addr2); + let start_after = identity2.clone(); let page2 = query_mixnodes_paged( deps.as_ref(), Option::from(start_after), @@ -357,11 +354,9 @@ pub(crate) mod tests { assert_eq!(1, page2.nodes.len()); // save another one - mixnodes(&mut deps.storage) - .save(addr4.as_bytes(), &node) - .unwrap(); + helpers::add_mixnode(&addr4, good_mixnode_bond(), deps.as_mut()); - let start_after = String::from(addr2); + let start_after = identity2; let page2 = query_mixnodes_paged( deps.as_ref(), Option::from(start_after), @@ -1139,7 +1134,7 @@ pub(crate) mod tests { let current_state = config_read(deps.as_mut().storage).load().unwrap(); let rewarding_validator_address = current_state.rewarding_validator_address; - let node_identity = add_mixnode("bob", good_mixnode_bond(), &mut deps); + let node_identity = add_mixnode("bob", good_mixnode_bond(), deps.as_mut()); assert!( query_rewarding_status(deps.as_ref(), node_identity.clone(), 1) diff --git a/contracts/mixnet/src/storage.rs b/contracts/mixnet/src/storage.rs index ffd32696aa..bb5e62fa32 100644 --- a/contracts/mixnet/src/storage.rs +++ b/contracts/mixnet/src/storage.rs @@ -3,18 +3,19 @@ use crate::contract::INITIAL_REWARD_POOL; use crate::error::ContractError; use crate::state::State; -use config::defaults::TOTAL_SUPPLY; -use cosmwasm_std::{StdResult, Storage, Uint128}; +use config::defaults::{DENOM, TOTAL_SUPPLY}; +use cosmwasm_std::{Coin, StdResult, Storage, Uint128}; use cosmwasm_storage::{ bucket, bucket_read, singleton, singleton_read, Bucket, ReadonlyBucket, ReadonlySingleton, Singleton, }; use mixnet_contract::{ - Addr, GatewayBond, IdentityKey, IdentityKeyRef, Layer, LayerDistribution, MixNodeBond, + Addr, GatewayBond, IdentityKey, IdentityKeyRef, Layer, LayerDistribution, MixNode, MixNodeBond, RawDelegationData, RewardingStatus, StateParams, }; use serde::de::DeserializeOwned; -use serde::Serialize; +use serde::{Deserialize, Serialize}; +use std::fmt::{Display, Formatter}; // storage prefixes // all of them must be unique and presumably not be a prefix of a different one @@ -27,7 +28,7 @@ const LAYER_DISTRIBUTION_KEY: &[u8] = b"layers"; const REWARD_POOL_PREFIX: &[u8] = b"pool"; // buckets -pub const PREFIX_MIXNODES: &[u8] = b"mn"; +const PREFIX_MIXNODES: &[u8] = b"mn"; const PREFIX_MIXNODES_OWNERS: &[u8] = b"mo"; const PREFIX_GATEWAYS: &[u8] = b"gt"; const PREFIX_GATEWAYS_OWNERS: &[u8] = b"go"; @@ -37,6 +38,71 @@ const PREFIX_REVERSE_MIX_DELEGATION: &[u8] = b"dm"; const PREFIX_REWARDED_MIXNODES: &[u8] = b"rm"; +const PREFIX_TOTAL_DELEGATION: &[u8] = b"td"; + +#[derive(Serialize, Deserialize, Debug, PartialEq)] +pub(crate) struct StoredMixnodeBond { + pub bond_amount: Coin, + pub owner: Addr, + pub layer: Layer, + pub block_height: u64, + pub mix_node: MixNode, + pub profit_margin_percent: Option, +} + +impl StoredMixnodeBond { + pub(crate) fn new( + bond_amount: Coin, + owner: Addr, + layer: Layer, + block_height: u64, + mix_node: MixNode, + profit_margin_percent: Option, + ) -> Self { + StoredMixnodeBond { + bond_amount, + owner, + layer, + block_height, + mix_node, + profit_margin_percent, + } + } + + pub(crate) fn attach_delegation(self, total_delegation: Uint128) -> MixNodeBond { + MixNodeBond { + total_delegation: Coin { + denom: self.bond_amount.denom.clone(), + amount: total_delegation, + }, + bond_amount: self.bond_amount, + owner: self.owner, + layer: self.layer, + block_height: self.block_height, + mix_node: self.mix_node, + profit_margin_percent: self.profit_margin_percent, + } + } + + pub(crate) fn identity(&self) -> &String { + &self.mix_node.identity_key + } + + pub(crate) fn bond_amount(&self) -> Coin { + self.bond_amount.clone() + } +} + +impl Display for StoredMixnodeBond { + fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { + write!( + f, + "amount: {}, owner: {}, identity: {}", + self.bond_amount, self.owner, self.mix_node.identity_key + ) + } +} + // Contract-level stuff // TODO Unify bucket and mixnode storage functions @@ -163,11 +229,11 @@ pub fn decrement_layer_count(storage: &mut dyn Storage, layer: Layer) -> StdResu // Mixnode-related stuff -pub fn mixnodes(storage: &mut dyn Storage) -> Bucket { +pub(crate) fn mixnodes(storage: &mut dyn Storage) -> Bucket { bucket(storage, PREFIX_MIXNODES) } -pub fn mixnodes_read(storage: &dyn Storage) -> ReadonlyBucket { +pub(crate) fn mixnodes_read(storage: &dyn Storage) -> ReadonlyBucket { bucket_read(storage, PREFIX_MIXNODES) } @@ -180,6 +246,14 @@ pub fn mixnodes_owners_read(storage: &dyn Storage) -> ReadonlyBucket Bucket { + bucket(storage, PREFIX_TOTAL_DELEGATION) +} + +pub fn total_delegation_read(storage: &dyn Storage) -> ReadonlyBucket { + bucket_read(storage, PREFIX_TOTAL_DELEGATION) +} + // we want to treat this bucket as a set so we don't really care about what type of data is being stored. // I went with u8 as after serialization it takes only a single byte of space, while if a `()` was used, // it would have taken 4 bytes (representation of 'null') @@ -209,9 +283,36 @@ pub(crate) fn rewarded_mixnodes_read( ) } +// helpers +pub(crate) fn read_mixnode_bond( + storage: &dyn Storage, + mix_identity: IdentityKeyRef, +) -> StdResult> { + let stored_bond = mixnodes_read(storage).may_load(mix_identity.as_bytes())?; + match stored_bond { + None => Ok(None), + Some(stored_bond) => { + let total_delegation = + total_delegation_read(storage).may_load(mix_identity.as_bytes())?; + Ok(Some(MixNodeBond { + bond_amount: stored_bond.bond_amount, + total_delegation: Coin { + denom: DENOM.to_owned(), + amount: total_delegation.unwrap_or_default(), + }, + owner: stored_bond.owner, + layer: stored_bond.layer, + block_height: stored_bond.block_height, + mix_node: stored_bond.mix_node, + profit_margin_percent: stored_bond.profit_margin_percent, + })) + } + } +} + // currently not used outside tests #[cfg(test)] -pub(crate) fn read_mixnode_bond( +pub(crate) fn read_mixnode_bond_amount( storage: &dyn Storage, identity: &[u8], ) -> StdResult { @@ -220,17 +321,6 @@ pub(crate) fn read_mixnode_bond( Ok(node.bond_amount.amount) } -// currently not used outside tests -#[cfg(test)] -pub(crate) fn read_mixnode_delegation( - storage: &dyn Storage, - identity: &[u8], -) -> StdResult { - let bucket = mixnodes_read(storage); - let node = bucket.load(identity)?; - Ok(node.total_delegation.amount) -} - // Gateway-related stuff pub fn gateways(storage: &mut dyn Storage) -> Bucket { @@ -299,7 +389,7 @@ mod tests { use super::*; use crate::helpers::identity_and_owner_to_bytes; use crate::support::tests::helpers::{ - gateway_bond_fixture, gateway_fixture, mix_node_fixture, mixnode_bond_fixture, + gateway_bond_fixture, gateway_fixture, mix_node_fixture, stored_mixnode_bond_fixture, }; use config::defaults::DENOM; use cosmwasm_std::testing::{mock_dependencies, MockStorage}; @@ -309,8 +399,8 @@ mod tests { #[test] fn mixnode_single_read_retrieval() { let mut storage = MockStorage::new(); - let bond1 = mixnode_bond_fixture(); - let bond2 = mixnode_bond_fixture(); + let bond1 = stored_mixnode_bond_fixture(); + let bond2 = stored_mixnode_bond_fixture(); mixnodes(&mut storage).save(b"bond1", &bond1).unwrap(); mixnodes(&mut storage).save(b"bond2", &bond2).unwrap(); @@ -341,15 +431,14 @@ mod tests { let node_identity: IdentityKey = "nodeidentity".into(); // produces an error if target mixnode doesn't exist - let res = read_mixnode_bond(&storage, node_owner.as_bytes()); + let res = read_mixnode_bond_amount(&storage, node_owner.as_bytes()); assert!(res.is_err()); // returns appropriate value otherwise let bond_value = 1000; - let mixnode_bond = MixNodeBond { + let mixnode_bond = StoredMixnodeBond { bond_amount: coin(bond_value, DENOM), - total_delegation: coin(0, DENOM), owner: node_owner.clone(), layer: Layer::One, block_height: 12_345, @@ -366,7 +455,7 @@ mod tests { assert_eq!( Uint128(bond_value), - read_mixnode_bond(&storage, node_identity.as_bytes()).unwrap() + read_mixnode_bond_amount(&storage, node_identity.as_bytes()).unwrap() ); } diff --git a/contracts/mixnet/src/support/tests.rs b/contracts/mixnet/src/support/tests.rs index c49b8cb160..4c28b6e87f 100644 --- a/contracts/mixnet/src/support/tests.rs +++ b/contracts/mixnet/src/support/tests.rs @@ -5,9 +5,9 @@ pub mod helpers { use crate::contract::{ query, DEFAULT_SYBIL_RESISTANCE_PERCENT, EPOCH_REWARD_PERCENT, INITIAL_REWARD_POOL, }; + use crate::storage::StoredMixnodeBond; use crate::transactions::{try_add_gateway, try_add_mixnode}; use config::defaults::{DENOM, TOTAL_SUPPLY}; - use cosmwasm_std::from_binary; use cosmwasm_std::testing::mock_dependencies; use cosmwasm_std::testing::mock_env; use cosmwasm_std::testing::mock_info; @@ -18,6 +18,7 @@ pub mod helpers { use cosmwasm_std::Coin; use cosmwasm_std::OwnedDeps; use cosmwasm_std::{coin, Uint128}; + use cosmwasm_std::{from_binary, DepsMut}; use cosmwasm_std::{Empty, MemoryStorage}; use mixnet_contract::mixnode::NodeRewardParams; use mixnet_contract::{ @@ -25,15 +26,11 @@ pub mod helpers { PagedMixnodeResponse, QueryMsg, RawDelegationData, }; - pub fn add_mixnode( - sender: &str, - stake: Vec, - deps: &mut OwnedDeps, - ) -> String { + pub fn add_mixnode(sender: &str, stake: Vec, deps: DepsMut) -> String { let info = mock_info(sender, &stake); let key = format!("{}mixnode", sender); try_add_mixnode( - deps.as_mut(), + deps, mock_env(), info, MixNode { @@ -140,6 +137,17 @@ pub mod helpers { ) } + pub(crate) fn stored_mixnode_bond_fixture() -> StoredMixnodeBond { + StoredMixnodeBond::new( + coin(50, DENOM), + Addr::unchecked("foo"), + Layer::One, + 12_345, + mix_node_fixture(), + None, + ) + } + pub fn gateway_fixture() -> Gateway { Gateway { host: "1.1.1.1".to_string(), diff --git a/contracts/mixnet/src/transactions/mod.rs b/contracts/mixnet/src/transactions/mod.rs index f0701701e3..584a8bcba3 100644 --- a/contracts/mixnet/src/transactions/mod.rs +++ b/contracts/mixnet/src/transactions/mod.rs @@ -1,16 +1,12 @@ // Copyright 2021 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 use crate::error::ContractError; -use crate::helpers::Delegations; use crate::queries; use crate::storage::*; use config::defaults::DENOM; -use cosmwasm_std::{ - attr, coins, BankMsg, Coin, DepsMut, Env, MessageInfo, Response, StdResult, Uint128, -}; -use cosmwasm_storage::ReadonlyBucket; +use cosmwasm_std::{attr, coins, BankMsg, Coin, DepsMut, Env, MessageInfo, Response, Uint128}; use mixnet_contract::{ - Gateway, GatewayBond, IdentityKey, Layer, MixNode, MixNodeBond, RawDelegationData, StateParams, + Gateway, GatewayBond, IdentityKey, Layer, MixNode, RawDelegationData, StateParams, }; pub(crate) mod rewarding; @@ -24,14 +20,6 @@ pub(crate) const MINIMUM_BLOCK_AGE_FOR_REWARDING: u64 = 17280; // approximately 30min (assuming 5s per block) pub(crate) const MAX_REWARDING_DURATION_IN_BLOCKS: u64 = 360; -fn total_delegations(delegations_bucket: ReadonlyBucket) -> StdResult { - Ok(Coin::new( - Delegations::new(delegations_bucket) - .fold(0, |acc, x| acc + x.delegation_data.amount.u128()), - DENOM, - )) -} - fn validate_mixnode_bond(bond: &[Coin], minimum_bond: Uint128) -> Result<(), ContractError> { // check if anything was put as bond if bond.is_empty() { @@ -100,7 +88,7 @@ pub(crate) fn try_add_mixnode( let layer_distribution = queries::query_layer_distribution(deps.as_ref()); let layer = layer_distribution.choose_with_fewest(); - let mut bond = MixNodeBond::new( + let stored_bond = StoredMixnodeBond::new( info.funds[0].clone(), info.sender.clone(), layer, @@ -109,16 +97,14 @@ pub(crate) fn try_add_mixnode( None, ); - // this might potentially require more gas if a significant number of delegations was there - let delegations_bucket = mix_delegations_read(deps.storage, &bond.mix_node.identity_key); - let existing_delegation = total_delegations(delegations_bucket)?; - bond.total_delegation = existing_delegation; + let identity = stored_bond.identity(); - let identity = bond.identity(); - - mixnodes(deps.storage).save(identity.as_bytes(), &bond)?; + // technically we don't have to set the total_delegation bucket, but it makes things easier + // in different places that we can guarantee that if node exists, so does the data behind the total delegation + mixnodes(deps.storage).save(identity.as_bytes(), &stored_bond)?; mixnodes_owners(deps.storage).save(sender_bytes, identity)?; - increment_layer_count(deps.storage, bond.layer)?; + total_delegation(deps.storage).save(identity.as_bytes(), &Uint128::zero())?; + increment_layer_count(deps.storage, layer)?; let attributes = vec![attr("overwritten", was_present)]; Ok(Response { @@ -368,33 +354,41 @@ pub(crate) fn try_delegate_to_mixnode( validate_delegation_stake(&info.funds)?; // check if the target node actually exists - let mut current_bond = match mixnodes_read(deps.storage).load(mix_identity.as_bytes()) { - Ok(bond) => bond, - Err(_) => { - return Err(ContractError::MixNodeBondNotFound { - identity: mix_identity, - }); - } - }; - - let amount = info.funds[0].amount; + if mixnodes_read(deps.storage) + .load(mix_identity.as_bytes()) + .is_err() + { + return Err(ContractError::MixNodeBondNotFound { + identity: mix_identity, + }); + } // update total_delegation of this node - current_bond.total_delegation.amount += info.funds[0].amount; - mixnodes(deps.storage).save(mix_identity.as_bytes(), ¤t_bond)?; + total_delegation(deps.storage).update::<_, ContractError>( + mix_identity.as_bytes(), + |total_delegation| { + // unwrap is fine as if the mixnode itself exists, so must this entry + Ok(total_delegation.unwrap() + info.funds[0].amount) + }, + )?; - let mut delegation_bucket = mix_delegations(deps.storage, &mix_identity); - let sender_bytes = info.sender.as_bytes(); + // update delegation of this delegator + mix_delegations(deps.storage, &mix_identity).update::<_, ContractError>( + info.sender.as_bytes(), + |existing_delegation| { + let existing_delegation_amount = existing_delegation + .map(|existing_delegation| existing_delegation.amount) + .unwrap_or_default(); - // write the delegation - let new_amount = match delegation_bucket.may_load(sender_bytes)? { - Some(existing_delegation) => existing_delegation.amount + amount, - None => amount, - }; - // the block height is reset, if it existed - let new_delegation = RawDelegationData::new(new_amount, env.block.height); - delegation_bucket.save(sender_bytes, &new_delegation)?; + // the block height is reset, if it existed + Ok(RawDelegationData::new( + existing_delegation_amount + info.funds[0].amount, + env.block.height, + )) + }, + )?; + // save information about delegations of this sender reverse_mix_delegations(deps.storage, &info.sender).save(mix_identity.as_bytes(), &())?; Ok(Response::default()) @@ -407,44 +401,45 @@ pub(crate) fn try_remove_delegation_from_mixnode( ) -> Result { let mut delegation_bucket = mix_delegations(deps.storage, &mix_identity); let sender_bytes = info.sender.as_bytes(); - match delegation_bucket.may_load(sender_bytes)? { - Some(delegation) => { - // remove delegation from the buckets - delegation_bucket.remove(sender_bytes); - reverse_mix_delegations(deps.storage, &info.sender).remove(mix_identity.as_bytes()); - // send delegated funds back to the delegation owner - let messages = vec![BankMsg::Send { - to_address: info.sender.to_string(), - amount: coins(delegation.amount.u128(), DENOM), - } - .into()]; + if let Some(delegation) = delegation_bucket.may_load(sender_bytes)? { + // remove all delegation associated with this delegator + delegation_bucket.remove(sender_bytes); + reverse_mix_delegations(deps.storage, &info.sender).remove(mix_identity.as_bytes()); - // update total_delegation of this node - let mut mixnodes_bucket = mixnodes(deps.storage); - // in some rare cases the mixnode bond might no longer exist as the node unbonded - // before delegation was removed. that is fine - if let Some(mut existing_bond) = mixnodes_bucket.may_load(mix_identity.as_bytes())? { - // we should NEVER underflow here, if we do, it means we have some serious error in our logic - existing_bond.total_delegation.amount = existing_bond - .total_delegation - .amount - .checked_sub(delegation.amount) - .unwrap(); - mixnodes_bucket.save(mix_identity.as_bytes(), &existing_bond)?; - } - - Ok(Response { - submessages: Vec::new(), - messages, - attributes: Vec::new(), - data: None, - }) + // send delegated funds back to the delegation owner + let messages = vec![BankMsg::Send { + to_address: info.sender.to_string(), + amount: coins(delegation.amount.u128(), DENOM), } - None => Err(ContractError::NoMixnodeDelegationFound { + .into()]; + + // update total_delegation of this node + total_delegation(deps.storage).update::<_, ContractError>( + mix_identity.as_bytes(), + |total_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_delegation + .unwrap() + .checked_sub(delegation.amount) + .unwrap()) + }, + )?; + + Ok(Response { + submessages: Vec::new(), + messages, + attributes: Vec::new(), + data: None, + }) + } else { + Err(ContractError::NoMixnodeDelegationFound { identity: mix_identity, address: info.sender, - }), + }) } } @@ -452,11 +447,10 @@ pub(crate) fn try_remove_delegation_from_mixnode( pub mod tests { use super::*; use crate::contract::{execute, query, INITIAL_GATEWAY_BOND, INITIAL_MIXNODE_BOND}; - use crate::storage::{layer_distribution_read, mix_delegations_read}; + use crate::helpers::Delegations; + use crate::storage::layer_distribution_read; use crate::support::tests::helpers; - use crate::support::tests::helpers::{ - good_gateway_bond, good_mixnode_bond, raw_delegation_fixture, - }; + use crate::support::tests::helpers::{good_gateway_bond, good_mixnode_bond}; use cosmwasm_std::testing::{mock_env, mock_info}; use cosmwasm_std::{coin, coins, from_binary, Addr, Uint128}; use mixnet_contract::{ @@ -778,7 +772,7 @@ pub mod tests { ); // let's add a node owned by bob - helpers::add_mixnode("bob", good_mixnode_bond(), &mut deps); + helpers::add_mixnode("bob", good_mixnode_bond(), deps.as_mut()); // attempt to un-register fred's node, which doesn't exist let info = mock_info("fred", &[]); @@ -823,7 +817,7 @@ pub mod tests { attr( "mixnode_bond", format!( - "amount: {} {}, owner: fred, identity: fredsmixnode", + "amount: {}{}, owner: fred, identity: fredsmixnode", INITIAL_MIXNODE_BOND, DENOM ), ), @@ -843,7 +837,7 @@ pub mod tests { attributes: expected_attributes, data: None, }; - assert_eq!(remove_fred, expected); + assert_eq!(expected, remove_fred); // only 1 node now exists, owned by bob: let mix_node_bonds = helpers::get_mix_nodes(&mut deps); @@ -1419,7 +1413,7 @@ pub mod tests { fn succeeds_for_existing_node() { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); let delegation = coin(123, DENOM); @@ -1445,11 +1439,10 @@ pub mod tests { // node's "total_delegation" is increased assert_eq!( - delegation, - mixnodes_read(&deps.storage) + delegation.amount, + total_delegation_read(&deps.storage) .load(identity.as_bytes()) .unwrap() - .total_delegation ) } @@ -1458,7 +1451,7 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); try_remove_mixnode(deps.as_mut(), mock_info(mixnode_owner, &[])).unwrap(); @@ -1481,9 +1474,9 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); try_remove_mixnode(deps.as_mut(), mock_info(mixnode_owner, &[])).unwrap(); - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation = coin(123, DENOM); let delegation_owner = Addr::unchecked("sender"); @@ -1509,11 +1502,10 @@ pub mod tests { // node's "total_delegation" is increased assert_eq!( - delegation, - mixnodes_read(&deps.storage) + delegation.amount, + total_delegation_read(&deps.storage) .load(identity.as_bytes()) .unwrap() - .total_delegation ) } @@ -1521,7 +1513,7 @@ pub mod tests { fn is_possible_for_an_already_delegated_node() { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); let delegation1 = coin(100, DENOM); @@ -1561,11 +1553,9 @@ pub mod tests { // node's "total_delegation" is sum of both assert_eq!( delegation1.amount + delegation2.amount, - mixnodes_read(&deps.storage) + total_delegation_read(&deps.storage) .load(identity.as_bytes()) .unwrap() - .total_delegation - .amount ) } @@ -1573,7 +1563,7 @@ pub mod tests { fn block_height_is_updated_on_new_delegation() { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); let delegation = coin(100, DENOM); @@ -1619,7 +1609,7 @@ pub mod tests { fn block_height_is_not_updated_on_different_delegator() { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner1 = Addr::unchecked("sender1"); let delegation_owner2 = Addr::unchecked("sender2"); let delegation1 = coin(100, DENOM); @@ -1674,7 +1664,7 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); try_delegate_to_mixnode( @@ -1705,8 +1695,8 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner1 = "bob"; let mixnode_owner2 = "fred"; - let identity1 = add_mixnode(mixnode_owner1, good_mixnode_bond(), &mut deps); - let identity2 = add_mixnode(mixnode_owner2, good_mixnode_bond(), &mut deps); + let identity1 = add_mixnode(mixnode_owner1, good_mixnode_bond(), deps.as_mut()); + let identity2 = add_mixnode(mixnode_owner2, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); assert!(try_delegate_to_mixnode( @@ -1754,7 +1744,7 @@ pub mod tests { fn is_allowed_by_multiple_users() { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation1 = coin(123, DENOM); let delegation2 = coin(234, DENOM); @@ -1778,11 +1768,9 @@ pub mod tests { // node's "total_delegation" is sum of both assert_eq!( delegation1.amount + delegation2.amount, - mixnodes_read(&deps.storage) + total_delegation_read(&deps.storage) .load(identity.as_bytes()) .unwrap() - .total_delegation - .amount ) } @@ -1791,7 +1779,7 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); try_delegate_to_mixnode( @@ -1829,7 +1817,7 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); assert_eq!( @@ -1850,7 +1838,7 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); try_delegate_to_mixnode( @@ -1893,11 +1881,9 @@ pub mod tests { // and total delegation is cleared assert_eq!( Uint128::zero(), - mixnodes_read(&deps.storage) + total_delegation_read(&deps.storage) .load(identity.as_bytes()) .unwrap() - .total_delegation - .amount ) } @@ -1906,7 +1892,7 @@ pub mod tests { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner = Addr::unchecked("sender"); try_delegate_to_mixnode( @@ -1953,7 +1939,7 @@ pub mod tests { fn total_delegation_is_preserved_if_only_some_undelegate() { let mut deps = helpers::init_contract(); let mixnode_owner = "bob"; - let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), &mut deps); + let identity = add_mixnode(mixnode_owner, good_mixnode_bond(), deps.as_mut()); let delegation_owner1 = Addr::unchecked("sender1"); let delegation_owner2 = Addr::unchecked("sender2"); @@ -1987,11 +1973,10 @@ pub mod tests { // but total delegation should still equal to what sender2 sent // node's "total_delegation" is sum of both assert_eq!( - delegation2, - mixnodes_read(&deps.storage) + delegation2.amount, + total_delegation_read(&deps.storage) .load(identity.as_bytes()) .unwrap() - .total_delegation ) } } @@ -2015,58 +2000,6 @@ pub mod tests { ); } - #[cfg(test)] - mod finding_old_delegations { - use super::*; - - #[test] - fn when_there_werent_any() { - let deps = helpers::init_contract(); - - let node_identity: IdentityKey = "nodeidentity".into(); - - let read_bucket = mix_delegations_read(&deps.storage, &node_identity); - let old_delegations = total_delegations(read_bucket).unwrap(); - - assert_eq!(Coin::new(0, DENOM), old_delegations); - } - - #[test] - fn when_some_existed() { - let num_delegations = vec![ - 1, - 5, - OLD_DELEGATIONS_CHUNK_SIZE - 1, - OLD_DELEGATIONS_CHUNK_SIZE, - OLD_DELEGATIONS_CHUNK_SIZE + 1, - OLD_DELEGATIONS_CHUNK_SIZE * 3, - OLD_DELEGATIONS_CHUNK_SIZE * 3 + 1, - ]; - - for delegations in num_delegations { - let mut deps = helpers::init_contract(); - - let node_identity: IdentityKey = "nodeidentity".into(); - - // delegate some stake - let mut write_bucket = mix_delegations(&mut deps.storage, &node_identity); - for i in 1..=delegations { - let delegator = Addr::unchecked(format!("delegator{}", i)); - let delegation = raw_delegation_fixture(i as u128); - write_bucket - .save(delegator.as_bytes(), &delegation) - .unwrap(); - } - - let read_bucket = mix_delegations_read(&deps.storage, &node_identity); - let old_delegations = total_delegations(read_bucket).unwrap(); - - let total_delegation = (1..=delegations as u128).into_iter().sum(); - assert_eq!(Coin::new(total_delegation, DENOM), old_delegations); - } - } - } - #[test] fn choose_layer_mix_node() { let mut deps = helpers::init_contract(); diff --git a/contracts/mixnet/src/transactions/rewarding.rs b/contracts/mixnet/src/transactions/rewarding.rs index 2609f3b653..46dfadf4ce 100644 --- a/contracts/mixnet/src/transactions/rewarding.rs +++ b/contracts/mixnet/src/transactions/rewarding.rs @@ -3,8 +3,8 @@ use crate::error::ContractError; use crate::storage::{ - config, config_read, decr_reward_pool, mix_delegations, mixnodes, mixnodes_read, - rewarded_mixnodes, rewarded_mixnodes_read, + config, config_read, decr_reward_pool, mix_delegations, mixnodes, read_mixnode_bond, + rewarded_mixnodes, rewarded_mixnodes_read, total_delegation, }; use crate::transactions::{MAX_REWARDING_DURATION_IN_BLOCKS, MINIMUM_BLOCK_AGE_FOR_REWARDING}; use cosmwasm_std::{attr, DepsMut, Env, MessageInfo, Response, StdResult, Storage, Uint128}; @@ -217,13 +217,15 @@ pub(crate) fn try_reward_next_mixnode_delegators_v2( next_page_info.rewarding_params, )?; - // read current bond to update the memoised total delegation field - let mut mixnodes = mixnodes(deps.storage); - if let Some(mut current_bond) = mixnodes.may_load(mix_identity.as_bytes())? { - // if the node unbonded, we don't have to worry about it. - current_bond.total_delegation.amount += delegation_rewarding_result.total_rewarded; - mixnodes.save(mix_identity.as_bytes(), ¤t_bond)?; - } + // update the memoised total delegation field + total_delegation(deps.storage).update::<_, ContractError>( + mix_identity.as_bytes(), + |current_total| { + // unwrap is fine as if the mixnode if this mixnode's delegators are getting rewarded + // it means it MUST HAVE existed at some point in the past + Ok(current_total.unwrap() + delegation_rewarding_result.total_rewarded) + }, + )?; decr_reward_pool(delegation_rewarding_result.total_rewarded, deps.storage)?; @@ -298,9 +300,9 @@ pub(crate) fn try_reward_mixnode_v2( } // check if the bond even exists - let mut current_bond = match mixnodes_read(deps.storage).load(mix_identity.as_bytes()) { - Ok(bond) => bond, - Err(_) => { + let current_bond = match read_mixnode_bond(deps.storage, &mix_identity)? { + Some(bond) => bond, + None => { return Ok(Response { attributes: vec![attr("result", "bond not found")], ..Default::default() @@ -332,9 +334,23 @@ pub(crate) fn try_reward_mixnode_v2( let delegation_rewarding_result = reward_mix_delegators_v2(deps.storage, &mix_identity, None, delegator_params)?; - current_bond.bond_amount.amount += operator_reward; - current_bond.total_delegation.amount += delegation_rewarding_result.total_rewarded; - mixnodes(deps.storage).save(mix_identity.as_bytes(), ¤t_bond)?; + total_delegation(deps.storage).update::<_, ContractError>( + mix_identity.as_bytes(), + |current_total| { + // unwrap is fine as if the mixnode itself exists, so must this entry + Ok(current_total.unwrap() + delegation_rewarding_result.total_rewarded) + }, + )?; + mixnodes(deps.storage).update::<_, ContractError>( + mix_identity.as_bytes(), + |current_bond| { + // unwrap is fine because we just read the entry... + let mut unwrapped = current_bond.unwrap(); + unwrapped.bond_amount.amount += operator_reward; + Ok(unwrapped) + }, + )?; + decr_reward_pool( operator_reward + delegation_rewarding_result.total_rewarded, deps.storage, @@ -420,8 +436,8 @@ mod tests { use super::*; use crate::contract::DEFAULT_SYBIL_RESISTANCE_PERCENT; use crate::storage::{ - circulating_supply, mix_delegations_read, read_mixnode_bond, read_mixnode_delegation, - reward_pool_value, + circulating_supply, mix_delegations_read, read_mixnode_bond, read_mixnode_bond_amount, + reward_pool_value, total_delegation_read, StoredMixnodeBond, }; use crate::support::tests::helpers; use crate::support::tests::helpers::{ @@ -431,7 +447,7 @@ mod tests { use config::defaults::DENOM; use cosmwasm_std::testing::{mock_env, mock_info}; use cosmwasm_std::{coin, Addr, Coin, Order}; - use mixnet_contract::{Layer, MixNode, MixNodeBond, RawDelegationData}; + use mixnet_contract::{Layer, MixNode, RawDelegationData}; #[cfg(test)] mod beginning_mixnode_rewarding { @@ -934,9 +950,8 @@ mod tests { let initial_bond = 10000_000000; let initial_delegation = 20000_000000; - let mixnode_bond = MixNodeBond { + let mixnode_bond = StoredMixnodeBond { bond_amount: coin(initial_bond, DENOM), - total_delegation: coin(initial_delegation, DENOM), owner: node_owner.clone(), layer: Layer::One, block_height: env.block.height, @@ -950,6 +965,9 @@ mod tests { mixnodes(deps.as_mut().storage) .save(node_identity.as_bytes(), &mixnode_bond) .unwrap(); + total_delegation(deps.as_mut().storage) + .save(node_identity.as_bytes(), &Uint128::new(initial_delegation)) + .unwrap(); // delegation happens later, but not later enough env.block.height += MINIMUM_BLOCK_AGE_FOR_REWARDING - 1; @@ -977,13 +995,14 @@ mod tests { assert_eq!( initial_bond, - read_mixnode_bond(deps.as_ref().storage, node_identity.as_bytes()) + read_mixnode_bond_amount(deps.as_ref().storage, node_identity.as_bytes()) .unwrap() .u128() ); assert_eq!( initial_delegation, - read_mixnode_delegation(deps.as_ref().storage, node_identity.as_bytes()) + total_delegation_read(deps.as_ref().storage) + .load(node_identity.as_bytes()) .unwrap() .u128() ); @@ -1010,14 +1029,15 @@ mod tests { try_finish_mixnode_rewarding(deps.as_mut(), info, 2).unwrap(); assert!( - read_mixnode_bond(deps.as_ref().storage, node_identity.as_bytes()) + read_mixnode_bond_amount(deps.as_ref().storage, node_identity.as_bytes()) .unwrap() .u128() > initial_bond ); assert_eq!( initial_delegation, - read_mixnode_delegation(deps.as_ref().storage, node_identity.as_bytes()) + total_delegation_read(deps.as_ref().storage) + .load(node_identity.as_bytes()) .unwrap() .u128() ); @@ -1031,7 +1051,7 @@ mod tests { env.block.height += MINIMUM_BLOCK_AGE_FOR_REWARDING - 1; let bond_before_rewarding = - read_mixnode_bond(deps.as_ref().storage, node_identity.as_bytes()) + read_mixnode_bond_amount(deps.as_ref().storage, node_identity.as_bytes()) .unwrap() .u128(); @@ -1049,13 +1069,14 @@ mod tests { try_finish_mixnode_rewarding(deps.as_mut(), info, 3).unwrap(); assert!( - read_mixnode_bond(deps.as_ref().storage, node_identity.as_bytes()) + read_mixnode_bond_amount(deps.as_ref().storage, node_identity.as_bytes()) .unwrap() .u128() > bond_before_rewarding ); assert!( - read_mixnode_delegation(deps.as_ref().storage, node_identity.as_bytes()) + total_delegation_read(deps.as_ref().storage) + .load(node_identity.as_bytes()) .unwrap() .u128() > initial_delegation @@ -1130,7 +1151,7 @@ mod tests { env.block.height += 2 * MINIMUM_BLOCK_AGE_FOR_REWARDING; - let mix_1 = mixnodes_read(&deps.storage).load(b"alice").unwrap(); + let mix_1 = read_mixnode_bond(&deps.storage, "alice").unwrap().unwrap(); let mix_1_uptime = 100; let mut params = NodeRewardParams::new( @@ -1168,10 +1189,13 @@ mod tests { assert_eq!(mix1_delegator1_reward, U128::from_num(22552615)); assert_eq!(mix1_delegator2_reward, U128::from_num(5638153)); - let pre_reward_bond = read_mixnode_bond(&deps.storage, b"alice").unwrap().u128(); + let pre_reward_bond = read_mixnode_bond_amount(&deps.storage, b"alice") + .unwrap() + .u128(); assert_eq!(pre_reward_bond, 10000_000_000); - let pre_reward_delegation = read_mixnode_delegation(&deps.storage, b"alice") + let pre_reward_delegation = total_delegation_read(&deps.storage) + .load(b"alice") .unwrap() .u128(); assert_eq!(pre_reward_delegation, 10000_000_000); @@ -1179,11 +1203,14 @@ mod tests { try_reward_mixnode_v2(deps.as_mut(), env, info, "alice".to_string(), params, 1).unwrap(); assert_eq!( - read_mixnode_bond(&deps.storage, b"alice").unwrap().u128(), + read_mixnode_bond_amount(&deps.storage, b"alice") + .unwrap() + .u128(), U128::from_num(pre_reward_bond) + U128::from_num(mix1_operator_profit) ); assert_eq!( - read_mixnode_delegation(&deps.storage, b"alice") + total_delegation_read(&deps.storage) + .load(b"alice") .unwrap() .u128(), pre_reward_delegation + mix1_delegator1_reward + mix1_delegator2_reward @@ -1485,8 +1512,8 @@ mod tests { }, ) .unwrap(); - let bond = mixnodes_read(deps.as_ref().storage) - .load(node_identity.as_bytes()) + let bond = read_mixnode_bond(deps.as_ref().storage, &*node_identity) + .unwrap() .unwrap(); let base_delegation = 200_000000; @@ -1540,8 +1567,8 @@ mod tests { }, ) .unwrap(); - let bond = mixnodes_read(deps.as_ref().storage) - .load(node_identity.as_bytes()) + let bond = read_mixnode_bond(deps.as_ref().storage, &*node_identity) + .unwrap() .unwrap(); let base_delegation = 200_000000;