diff --git a/Cargo.lock b/Cargo.lock index cc35237b23..62fd3c5c80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4989,9 +4989,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.79" +version = "1.0.83" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e8d9fa5c3b304765ce1fd9c4c8a3de2c8db365a5b91be52f186efc675681d95" +checksum = "38dd04e3c8279e75b31ef29dbdceebfe5ad89f4d0937213c53f7d49d01b3d5a7" dependencies = [ "itoa 1.0.1", "ryu", diff --git a/contracts/CHANGELOG.md b/contracts/CHANGELOG.md index bed698076b..6360f1334f 100644 --- a/contracts/CHANGELOG.md +++ b/contracts/CHANGELOG.md @@ -1,3 +1,11 @@ +## Unreleased + +### Fixed + +- vesting-contract: the contract now correctly stores delegations with their timestamp as opposed to using block height ([#1544]) + +[#1544]: https://github.com/nymtech/nym/pull/1544 + ## [nym-contracts-v1.0.1](https://github.com/nymtech/nym/tree/nym-contracts-v1.0.1) (2022-06-22) ### Added diff --git a/contracts/vesting/src/storage.rs b/contracts/vesting/src/storage.rs index 400d90fc3e..c753e57c37 100644 --- a/contracts/vesting/src/storage.rs +++ b/contracts/vesting/src/storage.rs @@ -5,7 +5,7 @@ use cw_storage_plus::{Item, Map}; use mixnet_contract_common::IdentityKey; use vesting_contract_common::PledgeData; -type BlockHeight = u64; +pub(crate) type BlockTimestampSecs = u64; pub const KEY: Item<'_, u32> = Item::new("key"); const ACCOUNTS: Map<'_, String, Account> = Map::new("acc"); @@ -14,7 +14,7 @@ const BALANCES: Map<'_, u32, Uint128> = Map::new("blc"); const WITHDRAWNS: Map<'_, u32, Uint128> = Map::new("wthd"); const BOND_PLEDGES: Map<'_, u32, PledgeData> = Map::new("bnd"); const GATEWAY_PLEDGES: Map<'_, u32, PledgeData> = Map::new("gtw"); -pub const DELEGATIONS: Map<'_, (u32, IdentityKey, BlockHeight), Uint128> = Map::new("dlg"); +pub const DELEGATIONS: Map<'_, (u32, IdentityKey, BlockTimestampSecs), Uint128> = Map::new("dlg"); pub const ADMIN: Item<'_, String> = Item::new("adm"); pub const MIXNET_CONTRACT_ADDRESS: Item<'_, String> = Item::new("mix"); pub const MIX_DENOM: Item<'_, String> = Item::new("den"); @@ -35,7 +35,7 @@ pub fn update_locked_pledge_cap( } pub fn save_delegation( - key: (u32, IdentityKey, BlockHeight), + key: (u32, IdentityKey, BlockTimestampSecs), amount: Uint128, storage: &mut dyn Storage, ) -> Result<(), ContractError> { @@ -44,7 +44,7 @@ pub fn save_delegation( } pub fn remove_delegation( - key: (u32, IdentityKey, BlockHeight), + key: (u32, IdentityKey, BlockTimestampSecs), storage: &mut dyn Storage, ) -> Result<(), ContractError> { DELEGATIONS.remove(storage, key); diff --git a/contracts/vesting/src/vesting/account/delegating_account.rs b/contracts/vesting/src/vesting/account/delegating_account.rs index 48f32207ac..80f825084b 100644 --- a/contracts/vesting/src/vesting/account/delegating_account.rs +++ b/contracts/vesting/src/vesting/account/delegating_account.rs @@ -88,7 +88,7 @@ impl DelegatingAccount for Account { vec![coin.clone()], )?; self.track_delegation( - env.block.height, + env.block.time.seconds(), mix_identity, current_balance, coin, @@ -129,14 +129,14 @@ impl DelegatingAccount for Account { fn track_delegation( &self, - block_height: u64, + block_timestamp_secs: u64, mix_identity: IdentityKey, current_balance: Uint128, delegation: Coin, storage: &mut dyn Storage, ) -> Result<(), ContractError> { save_delegation( - (self.storage_key(), mix_identity, block_height), + (self.storage_key(), mix_identity, block_timestamp_secs), delegation.amount, storage, )?; diff --git a/contracts/vesting/src/vesting/account/mod.rs b/contracts/vesting/src/vesting/account/mod.rs index b5369aa410..f698037f8e 100644 --- a/contracts/vesting/src/vesting/account/mod.rs +++ b/contracts/vesting/src/vesting/account/mod.rs @@ -3,7 +3,7 @@ use crate::errors::ContractError; use crate::storage::{ load_balance, load_bond_pledge, load_gateway_pledge, load_withdrawn, remove_bond_pledge, remove_delegation, remove_gateway_pledge, save_account, save_balance, save_bond_pledge, - save_gateway_pledge, save_withdrawn, DELEGATIONS, KEY, + save_gateway_pledge, save_withdrawn, BlockTimestampSecs, DELEGATIONS, KEY, }; use cosmwasm_std::{Addr, Coin, Order, Storage, Timestamp, Uint128}; use cw_storage_plus::Bound; @@ -261,4 +261,19 @@ impl Account { .filter_map(|x| x.ok()) .fold(Uint128::zero(), |acc, (_key, val)| acc + val)) } + + pub fn total_delegations_at_timestamp( + &self, + storage: &dyn Storage, + start_time: BlockTimestampSecs, + ) -> Result { + Ok(DELEGATIONS + .sub_prefix(self.storage_key()) + .range(storage, None, None, Order::Ascending) + .filter_map(|x| x.ok()) + .filter(|((_mix, block_time), _amount)| *block_time <= start_time) + .fold(Uint128::zero(), |acc, ((_mix, _block_time), amount)| { + acc + amount + })) + } } diff --git a/contracts/vesting/src/vesting/account/vesting_account.rs b/contracts/vesting/src/vesting/account/vesting_account.rs index dc93627d1d..060ea95973 100644 --- a/contracts/vesting/src/vesting/account/vesting_account.rs +++ b/contracts/vesting/src/vesting/account/vesting_account.rs @@ -1,7 +1,7 @@ use crate::errors::ContractError; -use crate::storage::{delete_account, save_account, DELEGATIONS, MIX_DENOM}; +use crate::storage::{delete_account, save_account, MIX_DENOM}; use crate::traits::VestingAccount; -use cosmwasm_std::{Addr, Coin, Env, Order, Storage, Timestamp, Uint128}; +use cosmwasm_std::{Addr, Coin, Env, Storage, Timestamp, Uint128}; use vesting_contract_common::{OriginalVestingResponse, Period}; use super::Account; @@ -16,13 +16,6 @@ impl VestingAccount for Account { + self.get_pledged_vesting(None, env, storage)?.amount) } - fn track_reward(&self, amount: Coin, storage: &mut dyn Storage) -> Result<(), ContractError> { - let current_balance = self.load_balance(storage)?; - let new_balance = current_balance + amount.amount; - self.save_balance(new_balance, storage)?; - Ok(()) - } - fn locked_coins( &self, block_time: Option, @@ -141,14 +134,7 @@ impl VestingAccount for Account { Period::In(idx) => self.periods[idx as usize].start_time, }; - let coin = DELEGATIONS - .sub_prefix(self.storage_key()) - .range(storage, None, None, Order::Ascending) - .filter_map(|x| x.ok()) - .filter(|((_mix, block_time), _amount)| *block_time < start_time) - .fold(Uint128::zero(), |acc, ((_mix, _block_time), amount)| { - acc + amount - }); + let coin = self.total_delegations_at_timestamp(storage, start_time)?; let amount = Uint128::new(coin.u128().min(max_available.u128())); @@ -158,6 +144,7 @@ impl VestingAccount for Account { }) } + // TODO: why do we allow querying for block times in the past? - just use env.block.time all the time fn get_delegated_vesting( &self, block_time: Option, @@ -166,9 +153,18 @@ impl VestingAccount for Account { ) -> Result { let block_time = block_time.unwrap_or(env.block.time); let delegated_free = self.get_delegated_free(Some(block_time), env, storage)?; - let total_delegations = self.total_delegations(storage)?; - let amount = total_delegations - delegated_free.amount; + let period = self.get_current_vesting_period(block_time); + let start_time = match period { + Period::Before => 0, + Period::After => u64::MAX, + Period::In(idx) => self.periods[idx as usize].start_time, + }; + + let delegations_before_start_time = + self.total_delegations_at_timestamp(storage, start_time)?; + + let amount = delegations_before_start_time - delegated_free.amount; Ok(Coin { amount, @@ -261,4 +257,11 @@ impl VestingAccount for Account { save_account(self, storage)?; Ok(()) } + + fn track_reward(&self, amount: Coin, storage: &mut dyn Storage) -> Result<(), ContractError> { + let current_balance = self.load_balance(storage)?; + let new_balance = current_balance + amount.amount; + self.save_balance(new_balance, storage)?; + Ok(()) + } } diff --git a/contracts/vesting/src/vesting/mod.rs b/contracts/vesting/src/vesting/mod.rs index 9dc389864d..6fc9c56204 100644 --- a/contracts/vesting/src/vesting/mod.rs +++ b/contracts/vesting/src/vesting/mod.rs @@ -44,10 +44,11 @@ mod tests { use crate::traits::DelegatingAccount; use crate::traits::VestingAccount; use crate::traits::{GatewayBondingAccount, MixnodeBondingAccount}; + use crate::vesting::{populate_vesting_periods, Account}; use cosmwasm_std::testing::{mock_env, mock_info}; use cosmwasm_std::{coins, Addr, Coin, Timestamp, Uint128}; use mixnet_contract_common::{Gateway, MixNode}; - use vesting_contract_common::messages::ExecuteMsg; + use vesting_contract_common::messages::{ExecuteMsg, VestingSpecification}; use vesting_contract_common::Period; #[test] @@ -757,4 +758,171 @@ mod tests { .unwrap(); assert_eq!(Uint128::zero(), bonded_vesting.amount); } + + #[test] + fn delegated_free() { + let mut deps = init_contract(); + let mut env = mock_env(); + + let vesting_period_length_secs = 3600; + + let account_creation_timestamp = 1650000000; + let account_creation_blockheight = 12345; + + // this value is completely arbitrary, I just wanted to keep consistent + // (and make sure that if block timestamp increases so does the block height) + let blocks_per_period = 100; + + env.block.height = account_creation_blockheight; + env.block.time = Timestamp::from_seconds(account_creation_timestamp); + + // lets define some helper timestamps + + // our account is set to be created after 2 vesting periods already passed + let vesting_start_blockheight = account_creation_blockheight - 2 * blocks_per_period; + let vesting_start_timestamp = account_creation_timestamp - 2 * vesting_period_length_secs; + + let vesting_period2_start_blockheight = vesting_start_blockheight + blocks_per_period; + let vesting_period2_start_timestamp = vesting_start_timestamp + vesting_period_length_secs; + + // this vesting period is currently in progress! + let vesting_period3_start_blockheight = + vesting_period2_start_blockheight + blocks_per_period; + let vesting_period3_start_timestamp = + vesting_period2_start_timestamp + vesting_period_length_secs; + + // and this one is in the future! (in relation to account creation) + let vesting_period4_start_blockheight = + vesting_period3_start_blockheight + blocks_per_period; + let vesting_period4_start_timestamp = + vesting_period3_start_timestamp + vesting_period_length_secs; + + // lets create our vesting account + let periods = populate_vesting_periods( + vesting_start_timestamp, + VestingSpecification::new(None, Some(vesting_period_length_secs), None), + ); + + let vesting_account = Account::new( + Addr::unchecked("owner"), + Some(Addr::unchecked("staking")), + Coin { + amount: Uint128::new(1_000_000_000_000), + denom: TEST_COIN_DENOM.to_string(), + }, + Timestamp::from_seconds(account_creation_timestamp), + periods, + deps.as_mut().storage, + ) + .unwrap(); + + // time for some delegations + + let mix_identity = "alice".to_string(); + + let delegation = Coin { + amount: Uint128::new(90_000_000_000), + denom: TEST_COIN_DENOM.to_string(), + }; + + // delegate explicitly at the time the account was created + // (i.e. after 2 vesting periods already elapsed) + env.block.height = account_creation_blockheight; + env.block.time = Timestamp::from_seconds(account_creation_timestamp); + let ok = vesting_account.try_delegate_to_mixnode( + mix_identity.clone(), + delegation.clone(), + &env, + &mut deps.storage, + ); + assert!(ok.is_ok()); + + let vested_coins = vesting_account + .get_vested_coins(None, &env, &deps.storage) + .unwrap(); + let vesting_coins = vesting_account + .get_vesting_coins(None, &env, &deps.storage) + .unwrap(); + assert_eq!(vested_coins.amount, Uint128::new(250_000_000_000)); + assert_eq!(vesting_coins.amount, Uint128::new(750_000_000_000)); + let delegated_free = vesting_account + .get_delegated_free(None, &env, &deps.storage) + .unwrap(); + let delegated_vesting = vesting_account + .get_delegated_vesting(None, &env, &deps.storage) + .unwrap(); + + // all good so far + assert_eq!(delegated_free.amount, Uint128::new(90_000_000_000)); + assert_eq!(delegated_vesting.amount, Uint128::zero()); + + // some time passes, and we're now into the next vesting period, more of our coins got unlocked! + env.block.height = vesting_period4_start_blockheight; + env.block.time = Timestamp::from_seconds(vesting_period4_start_timestamp); + + let vested_coins = vesting_account + .get_vested_coins(None, &env, &deps.storage) + .unwrap(); + let vesting_coins = vesting_account + .get_vesting_coins(None, &env, &deps.storage) + .unwrap(); + assert_eq!(vested_coins.amount, Uint128::new(375_000_000_000)); + assert_eq!(vesting_coins.amount, Uint128::new(625_000_000_000)); + + // and nothing about our existing delegation changed + let delegated_free = vesting_account + .get_delegated_free(None, &env, &deps.storage) + .unwrap(); + let delegated_vesting = vesting_account + .get_delegated_vesting(None, &env, &deps.storage) + .unwrap(); + assert_eq!(delegated_free.amount, Uint128::new(90_000_000_000)); + assert_eq!(delegated_vesting.amount, Uint128::zero()); + + // however, create a new delegation now in this brand new vesting period + let delegation = Coin { + amount: Uint128::new(50_000_000_000), + denom: TEST_COIN_DENOM.to_string(), + }; + let ok = vesting_account.try_delegate_to_mixnode( + mix_identity.clone(), + delegation.clone(), + &env, + &mut deps.storage, + ); + assert!(ok.is_ok()); + + // we're still good here, we have delegated in total 140M from our vested tokens! + let delegated_free = vesting_account + .get_delegated_free(None, &env, &deps.storage) + .unwrap(); + let delegated_vesting = vesting_account + .get_delegated_vesting(None, &env, &deps.storage) + .unwrap(); + assert_eq!(delegated_free.amount, Uint128::new(140_000_000_000)); + assert_eq!(delegated_vesting.amount, Uint128::zero()); + + // but let's ask now a different question: + // how many vested tokens have I had delegated during vesting period3? (i.e. after account creation) + let delegated_free = vesting_account + .get_delegated_free( + Some(Timestamp::from_seconds(vesting_period3_start_timestamp)), + &env, + &deps.storage, + ) + .unwrap(); + let delegated_vesting = vesting_account + .get_delegated_vesting( + Some(Timestamp::from_seconds(vesting_period3_start_timestamp)), + &env, + &deps.storage, + ) + .unwrap(); + + // returns 90M as the 50M delegation didn't exist at this point of time + assert_eq!(delegated_free.amount, Uint128::new(90_000_000_000)); + + // the 50M delegation wasn't a thing here for VESTING tokens either + assert_eq!(delegated_vesting.amount, Uint128::zero()); + } }