From 81f923c5bad984ca933f06a9d4ba7811a8ec033b Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 13:19:49 +0100 Subject: [PATCH 01/12] Add denom check --- contracts/provider/native-staking-proxy/src/contract.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contracts/provider/native-staking-proxy/src/contract.rs b/contracts/provider/native-staking-proxy/src/contract.rs index b578cdee..80b77036 100644 --- a/contracts/provider/native-staking-proxy/src/contract.rs +++ b/contracts/provider/native-staking-proxy/src/contract.rs @@ -105,6 +105,13 @@ impl NativeStakingProxyContract<'_> { nonpayable(&ctx.info)?; + // Check denom + ensure_eq!( + amount.denom, + cfg.denom, + ContractError::InvalidDenom(amount.denom) + ); + let validators = match validator { Some(validator) => { let validator = ctx From 73847df5ed0bc4a7206a8e7b995a312ecfdf3f80 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 13:37:12 +0100 Subject: [PATCH 02/12] Add proportional burn helper / pkg --- Cargo.toml | 3 ++- packages/burn/Cargo.toml | 5 +++++ packages/burn/src/burn.rs | 40 +++++++++++++++++++++++++++++++++++++++ packages/burn/src/lib.rs | 3 +++ 4 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 packages/burn/Cargo.toml create mode 100644 packages/burn/src/burn.rs create mode 100644 packages/burn/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 48829f19..939b2382 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,8 @@ repository = "https://github.com/osmosis-labs/mesh-security" [workspace.dependencies] mesh-apis = { path = "./packages/apis" } -mesh-bindings = { path = "./packages/bindings" } +mesh-bindings = { path = "./packages/bindings" } +mesh-burn = { path = "./packages/burn" } mesh-sync = { path = "./packages/sync" } mesh-virtual-staking-mock = { path = "./packages/virtual-staking-mock" } diff --git a/packages/burn/Cargo.toml b/packages/burn/Cargo.toml new file mode 100644 index 00000000..a748f3c2 --- /dev/null +++ b/packages/burn/Cargo.toml @@ -0,0 +1,5 @@ +[package] +name = "mesh-burn" +version = { workspace = true } +edition = { workspace = true } +license = { workspace = true } diff --git a/packages/burn/src/burn.rs b/packages/burn/src/burn.rs new file mode 100644 index 00000000..5e2aa11c --- /dev/null +++ b/packages/burn/src/burn.rs @@ -0,0 +1,40 @@ +use std::cmp::min; + +/// Tries to burn `amount` evenly from `delegations`. +/// Assigns the remainder to the first validator that has enough stake. +/// `delegations` must not be empty, or this will panic. +/// +/// Returns the total amount burned, and the list of validators and amounts. +/// The total burned amount can be used to check if the user has enough stake in `delegations`. +/// +/// N.B..: This can be improved by distributing the remainder evenly across validators +pub fn distribute_burn( + delegations: &[(String, u128)], + amount: u128, +) -> (u128, Vec<(&String, u128)>) { + let mut burns = vec![]; + let mut burned = 0; + let proportional_amount = amount / delegations.len() as u128; + for (validator, delegated_amount) in delegations { + // Check validator has `proportional_amount` delegated. Adjust accordingly if not. + let burn_amount = min(*delegated_amount, proportional_amount); + if burn_amount == 0 { + continue; + } + burns.push((validator, burn_amount)); + burned += burn_amount; + } + // Adjust possible rounding issues / unfunded validators + if burned < amount { + // Look for the first validator that has enough stake, and burn it from there + let burn_amount = amount - burned; + for (validator, delegated_amount) in delegations { + if burn_amount + proportional_amount <= *delegated_amount { + burns.push((validator, burn_amount)); + burned += burn_amount; + break; + } + } + } + (burned, burns) +} diff --git a/packages/burn/src/lib.rs b/packages/burn/src/lib.rs new file mode 100644 index 00000000..ca4d28e5 --- /dev/null +++ b/packages/burn/src/lib.rs @@ -0,0 +1,3 @@ +mod burn; + +pub use burn::distribute_burn; From 0cfef30cbc00221aa5e96b6dd710a4197b56b529 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 13:40:49 +0100 Subject: [PATCH 03/12] Update lock file --- Cargo.lock | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index c96e1b1c..5d95ab5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -569,6 +569,10 @@ dependencies = [ "cosmwasm-std", ] +[[package]] +name = "mesh-burn" +version = "0.8.0-alpha.1" + [[package]] name = "mesh-converter" version = "0.8.0-alpha.1" @@ -652,6 +656,7 @@ dependencies = [ "cw2", "derivative", "mesh-apis", + "mesh-burn", "mesh-native-staking", "mesh-vault", "schemars", From baf9f801fa668d9da29c8c7d785c6bd0e5bd8142 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 13:42:57 +0100 Subject: [PATCH 04/12] Use proportional burn helper for native staking burn --- .../provider/native-staking-proxy/Cargo.toml | 1 + .../native-staking-proxy/src/contract.rs | 75 ++++++++----------- 2 files changed, 33 insertions(+), 43 deletions(-) diff --git a/contracts/provider/native-staking-proxy/Cargo.toml b/contracts/provider/native-staking-proxy/Cargo.toml index ea147acc..25e6f542 100644 --- a/contracts/provider/native-staking-proxy/Cargo.toml +++ b/contracts/provider/native-staking-proxy/Cargo.toml @@ -20,6 +20,7 @@ mt = ["library", "sylvia/mt"] [dependencies] mesh-apis = { workspace = true } +mesh-burn = { workspace = true } sylvia = { workspace = true } cosmwasm-schema = { workspace = true } diff --git a/contracts/provider/native-staking-proxy/src/contract.rs b/contracts/provider/native-staking-proxy/src/contract.rs index 80b77036..9646a883 100644 --- a/contracts/provider/native-staking-proxy/src/contract.rs +++ b/contracts/provider/native-staking-proxy/src/contract.rs @@ -1,5 +1,3 @@ -use std::cmp::min; - use cosmwasm_std::WasmMsg::Execute; use cosmwasm_std::{ coin, ensure_eq, to_binary, Coin, DistributionMsg, GovMsg, Response, StakingMsg, VoteOption, @@ -91,7 +89,7 @@ impl NativeStakingProxyContract<'_> { } /// Burn `amount` tokens from the given validator, if set. - /// If `validator` is not set, undelegate evenly from all validators the user has stake in. + /// If `validator` is not set, undelegate evenly from all validators the user has stake. /// Can only be called by the parent contract #[msg(exec)] fn burn( @@ -112,81 +110,72 @@ impl NativeStakingProxyContract<'_> { ContractError::InvalidDenom(amount.denom) ); - let validators = match validator { + let delegations = match validator { Some(validator) => { - let validator = ctx + let delegation = ctx .deps .querier .query_delegation(ctx.env.contract.address.clone(), validator)? .map(|full_delegation| { - (full_delegation.validator, full_delegation.amount.amount) + ( + full_delegation.validator, + full_delegation.amount.amount.u128(), + ) }) .unwrap(); - vec![validator] + vec![delegation] } None => { - let validators = ctx + let delegations = ctx .deps .querier .query_all_delegations(ctx.env.contract.address.clone())? .iter() - .map(|delegation| (delegation.validator.clone(), delegation.amount.amount)) + .map(|delegation| { + ( + delegation.validator.clone(), + delegation.amount.amount.u128(), + ) + }) .collect::>(); - validators + delegations } }; // Error if no validators - if validators.is_empty() { + if delegations.is_empty() { return Err(ContractError::InsufficientDelegations( ctx.env.contract.address.to_string(), amount.amount, )); } - let mut unstake_msgs = vec![]; - let mut unstaked = 0; - let proportional_amount = amount.amount.u128() / validators.len() as u128; - for (validator, delegated_amount) in &validators { - // Check validator has `proportional_amount` delegated. Adjust accordingly if not. - let unstake_amount = min(delegated_amount.u128(), proportional_amount); - let unstake_msg = StakingMsg::Undelegate { - validator: validator.to_string(), - amount: coin(unstake_amount, &cfg.denom), - }; - unstaked += unstake_amount; - unstake_msgs.push(unstake_msg); - } - // Adjust possible rounding issues - if unstaked < amount.amount.u128() { - // Look for the first validator that has enough stake, and unstake it from there - let unstake_amount = amount.amount.u128() - unstaked; - for (validator, delegated_amount) in &validators { - if delegated_amount.u128() >= unstake_amount + proportional_amount { - let unstake_msg = StakingMsg::Undelegate { - validator: validator.to_string(), - amount: coin(unstake_amount, &cfg.denom), - }; - unstaked += unstake_amount; - unstake_msgs.push(unstake_msg); - break; - } - } - } - // Bail if we still don't have enough stake - if unstaked < amount.amount.u128() { + let (burned, burns) = mesh_burn::distribute_burn(&delegations, amount.amount.u128()); + + // Bail if we don't have enough delegations + if burned < amount.amount.u128() { return Err(ContractError::InsufficientDelegations( ctx.env.contract.address.to_string(), amount.amount, )); } + // Build undelegate messages + let mut undelegate_msgs = vec![]; + for (validator, burn_amount) in burns { + let undelegate_msg = StakingMsg::Undelegate { + validator: validator.to_string(), + amount: coin(burn_amount, &cfg.denom), + }; + undelegate_msgs.push(undelegate_msg); + } + // Accounting trick to avoid burning stake self.burned.update(ctx.deps.storage, |old| { Ok::<_, ContractError>(old + amount.amount.u128()) })?; - Ok(Response::new().add_messages(unstake_msgs)) + Ok(Response::new().add_messages(undelegate_msgs)) } /// Re-stakes the given amount from the one validator to another on behalf of the calling user. From 18340e526f36ae7d78aee39943f2226135ce2d61 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 16:51:45 +0100 Subject: [PATCH 05/12] Use proportional burn helper for external staking burn --- .../provider/external-staking/Cargo.toml | 1 + .../provider/external-staking/src/contract.rs | 63 +++++++++++++------ .../provider/external-staking/src/error.rs | 3 + 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/contracts/provider/external-staking/Cargo.toml b/contracts/provider/external-staking/Cargo.toml index afb19f76..df3d6b27 100644 --- a/contracts/provider/external-staking/Cargo.toml +++ b/contracts/provider/external-staking/Cargo.toml @@ -19,6 +19,7 @@ mt = ["library", "sylvia/mt"] [dependencies] mesh-apis = { workspace = true } +mesh-burn = { workspace = true } mesh-sync = { workspace = true } sylvia = { workspace = true } diff --git a/contracts/provider/external-staking/src/contract.rs b/contracts/provider/external-staking/src/contract.rs index 4dc2248b..b03098d5 100644 --- a/contracts/provider/external-staking/src/contract.rs +++ b/contracts/provider/external-staking/src/contract.rs @@ -1272,11 +1272,16 @@ pub mod cross_staking { let owner = ctx.deps.api.addr_validate(&owner)?; - let validators: Vec<_> = match validator { + let stakes: Vec<_> = match validator { Some(validator) => { // Burn from validator // TODO: Preferentially, i.e. burn remaining amount, if any, from other validators - vec![validator] + let stake = self + .stakes + .stake + .may_load(ctx.deps.storage, (&owner, &validator))? + .unwrap_or_default(); + vec![(validator, stake.stake.high().u128())] // Burn takes precedence over any pending txs } None => { // Burn proportionally from all validators associated to the user @@ -1285,27 +1290,42 @@ pub mod cross_staking { .prefix(&owner) .range(ctx.deps.storage, None, None, Order::Ascending) .map(|item| { - let (validator, _) = item?; - Ok::<_, Self::Error>(validator) + let (validator, stake) = item?; + Ok::<_, Self::Error>((validator, stake.stake.high().u128())) + // Burn takes precedence over any pending txs }) .collect::>()? } }; - let num_validators = Uint128::new(validators.len() as u128); - // FIXME? Check for zero len validators - // TODO: Deal with rounding / unbonded validators - let proportional_amount = amount.amount / num_validators; - for validator in &validators { + + // Error if no stakes + if stakes.is_empty() { + return Err(ContractError::InsufficientDelegations( + owner.to_string(), + amount.amount, + )); + } + + let (burned, burns) = mesh_burn::distribute_burn(&stakes, amount.amount.u128()); + + // Bail if we don't have enough stake + if burned < amount.amount.u128() { + return Err(ContractError::InsufficientDelegations( + owner.to_string(), + amount.amount, + )); + } + + for (validator, burn_amount) in &burns { + let burn_amount = Uint128::new(*burn_amount); + // Perform stake subtraction let mut stake = self .stakes .stake - .may_load(ctx.deps.storage, (&owner, validator))? - .unwrap_or_default(); - - // Perform stake subtraction. + .load(ctx.deps.storage, (&owner, validator))?; // We don't check for min here, as this call can only come from the `vault` contract, which already // performed the proper check. - stake.stake.sub(proportional_amount, None)?; + stake.stake.sub(burn_amount, None)?; // Load distribution let mut distribution = self @@ -1316,8 +1336,8 @@ pub mod cross_staking { // Distribution alignment stake .points_alignment - .stake_decreased(proportional_amount, distribution.points_per_stake); - distribution.total_stake -= proportional_amount; + .stake_decreased(burn_amount, distribution.points_per_stake); + distribution.total_stake -= burn_amount; // Save stake self.stakes @@ -1331,7 +1351,7 @@ pub mod cross_staking { let channel = IBC_CHANNEL.load(ctx.deps.storage)?; let packet = ProviderPacket::Burn { - validators: validators.clone(), + validators: burns.iter().map(|v| v.0.to_string()).collect(), burn: amount.clone(), }; let msg = IbcMsg::SendPacket { @@ -1353,7 +1373,14 @@ pub mod cross_staking { resp = resp .add_attribute("action", "burn_virtual_stake") .add_attribute("owner", owner) - .add_attribute("validators", validators.join(", ")) + .add_attribute( + "validators", + stakes + .into_iter() + .map(|s| s.0) + .collect::>() + .join(", "), + ) .add_attribute("amount", amount.to_string()); Ok(resp) diff --git a/contracts/provider/external-staking/src/error.rs b/contracts/provider/external-staking/src/error.rs index 5b26d027..84e316b8 100644 --- a/contracts/provider/external-staking/src/error.rs +++ b/contracts/provider/external-staking/src/error.rs @@ -59,4 +59,7 @@ pub enum ContractError { #[error("{0}")] Range(#[from] RangeError), + + #[error("User {0} has not enough delegated funds: {1}")] + InsufficientDelegations(String, Uint128), } From 38357c8986521952d950893562ea10d88b0fa1cf Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 16:51:58 +0100 Subject: [PATCH 06/12] Update lock file --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 5d95ab5b..e49d6136 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -608,6 +608,7 @@ dependencies = [ "cw-utils", "cw2", "mesh-apis", + "mesh-burn", "mesh-native-staking", "mesh-native-staking-proxy", "mesh-sync", From 2821793712fc4bdba691cf00c9934392375128d9 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 17:08:43 +0100 Subject: [PATCH 07/12] Use proportional burn helper for virtual staking mock burn --- contracts/consumer/converter/Cargo.toml | 1 + .../src/multitest/virtual_staking_mock.rs | 67 ++++++++----------- 2 files changed, 29 insertions(+), 39 deletions(-) diff --git a/contracts/consumer/converter/Cargo.toml b/contracts/consumer/converter/Cargo.toml index 61147731..668c4509 100644 --- a/contracts/consumer/converter/Cargo.toml +++ b/contracts/consumer/converter/Cargo.toml @@ -34,6 +34,7 @@ serde = { workspace = true } thiserror = { workspace = true } [dev-dependencies] +mesh-burn = { workspace = true } mesh-simple-price-feed = { workspace = true, features = ["mt"] } cw-multi-test = { workspace = true } diff --git a/contracts/consumer/converter/src/multitest/virtual_staking_mock.rs b/contracts/consumer/converter/src/multitest/virtual_staking_mock.rs index ac4d5732..13552e60 100644 --- a/contracts/consumer/converter/src/multitest/virtual_staking_mock.rs +++ b/contracts/consumer/converter/src/multitest/virtual_staking_mock.rs @@ -1,6 +1,5 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::{ensure_eq, Addr, Coin, Response, StdError, StdResult, Uint128}; -use std::cmp::min; use cw_storage_plus::{Item, Map}; use cw_utils::{nonpayable, PaymentError}; @@ -30,9 +29,6 @@ pub enum ContractError { #[error("Wrong denom. Cannot stake {0}")] WrongDenom(String), - #[error("Empty validators list")] - NoValidators {}, - #[error("Virtual staking {0} has not enough delegated funds: {1}")] InsufficientDelegations(String, Uint128), } @@ -186,51 +182,44 @@ impl VirtualStakingApi for VirtualStakingMock<'_> { ContractError::WrongDenom(cfg.denom) ); - // Error if no validators - if validators.is_empty() { - return Err(ContractError::NoValidators {}); + let mut stakes = vec![]; + for validator in validators { + let stake = self + .stake + .may_load(ctx.deps.storage, &validator)? + .unwrap_or_default() + .u128(); + if stake != 0 { + stakes.push((validator, stake)); + } } - let mut unstaked = 0; - let proportional_amount = Uint128::new(amount.amount.u128() / validators.len() as u128); - for validator in &validators { - // Checks that validator has `proportional_amount` delegated. Adjust accordingly if not. - self.stake - .update::<_, ContractError>(ctx.deps.storage, validator, |old| { - let delegated_amount = old.unwrap_or_default(); - let unstake_amount = min(delegated_amount, proportional_amount); - unstaked += unstake_amount.u128(); - Ok(delegated_amount - unstake_amount) - })?; - } - // Adjust possible rounding issues - if unstaked < amount.amount.u128() { - // Look for the first validator that has enough stake, and unstake it from there - let unstake_amount = Uint128::new(amount.amount.u128() - unstaked); - for validator in &validators { - let delegated_amount = self - .stake - .may_load(ctx.deps.storage, validator)? - .unwrap_or_default(); - if delegated_amount >= unstake_amount { - self.stake.save( - ctx.deps.storage, - validator, - &(delegated_amount - unstake_amount), - )?; - unstaked += unstake_amount.u128(); - break; - } - } + // Error if no delegations + if stakes.is_empty() { + return Err(ContractError::InsufficientDelegations( + ctx.env.contract.address.to_string(), + amount.amount, + )); } + + let (burned, burns) = mesh_burn::distribute_burn(stakes.as_slice(), amount.amount.u128()); + // Bail if we still don't have enough stake - if unstaked < amount.amount.u128() { + if burned < amount.amount.u128() { return Err(ContractError::InsufficientDelegations( ctx.env.contract.address.to_string(), amount.amount, )); } + // Update stake + for (validator, burn_amount) in burns { + self.stake + .update::<_, ContractError>(ctx.deps.storage, validator, |old| { + Ok(old.unwrap_or_default() - Uint128::new(burn_amount)) + })?; + } + Ok(Response::new()) } } From 6d371665bd64c7d077a7e23598fac708a68ce35e Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 17:09:03 +0100 Subject: [PATCH 08/12] Update lock file --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index e49d6136..5337c773 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -587,6 +587,7 @@ dependencies = [ "cw2", "derivative", "mesh-apis", + "mesh-burn", "mesh-simple-price-feed", "schemars", "serde", From 605652900da899fe4915a6a5425066c21b91102c Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 17:30:27 +0100 Subject: [PATCH 09/12] Use proportional burn helper for virtual staking burn --- contracts/consumer/virtual-staking/Cargo.toml | 1 + .../consumer/virtual-staking/src/contract.rs | 64 ++++++++----------- 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/contracts/consumer/virtual-staking/Cargo.toml b/contracts/consumer/virtual-staking/Cargo.toml index d68cf427..588ec4f3 100644 --- a/contracts/consumer/virtual-staking/Cargo.toml +++ b/contracts/consumer/virtual-staking/Cargo.toml @@ -20,6 +20,7 @@ mt = ["library", "sylvia/mt"] [dependencies] mesh-apis = { workspace = true } +mesh-burn = { workspace = true } mesh-bindings = { workspace = true } sylvia = { workspace = true } diff --git a/contracts/consumer/virtual-staking/src/contract.rs b/contracts/consumer/virtual-staking/src/contract.rs index 7a68053e..36503c64 100644 --- a/contracts/consumer/virtual-staking/src/contract.rs +++ b/contracts/consumer/virtual-staking/src/contract.rs @@ -1,4 +1,4 @@ -use std::cmp::{min, Ordering}; +use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::str::FromStr; @@ -579,54 +579,42 @@ impl VirtualStakingApi for VirtualStakingContract<'_> { cfg.denom, ContractError::WrongDenom(cfg.denom) ); + let mut bonds = vec![]; + for validator in validators { + let stake = self + .bond_requests + .may_load(ctx.deps.storage, &validator)? + .unwrap_or_default() + .u128(); + if stake != 0 { + bonds.push((validator, stake)); + } + } - // Error if no validators - if validators.is_empty() { - return Err(ContractError::NoValidators {}); + // Error if no delegations + if bonds.is_empty() { + return Err(ContractError::InsufficientDelegations( + ctx.env.contract.address.to_string(), + amount.amount, + )); } - let mut unstaked = 0; - let proportional_amount = Uint128::new(amount.amount.u128() / validators.len() as u128); - for validator in &validators { - // Checks that validator has `proportional_amount` bonded. Adjust accordingly if not. + let (burned, burns) = mesh_burn::distribute_burn(bonds.as_slice(), amount.amount.u128()); + + for (validator, burn_amount) in burns { + // Update bond requests self.bond_requests .update::<_, ContractError>(ctx.deps.storage, validator, |old| { - let bonded_amount = old.unwrap_or_default(); - let unstake_amount = min(bonded_amount, proportional_amount); - unstaked += unstake_amount.u128(); - Ok(bonded_amount - unstake_amount) + Ok(old.unwrap_or_default() - Uint128::new(burn_amount)) })?; // Accounting trick to avoid burning stake self.burned.update(ctx.deps.storage, validator, |old| { - Ok::<_, ContractError>(old.unwrap_or_default() + proportional_amount.u128()) + Ok::<_, ContractError>(old.unwrap_or_default() + burn_amount) })?; } - // Adjust possible rounding issues - if unstaked < amount.amount.u128() { - // Look for the first validator that has enough stake, and unstake it from there - let unstake_amount = Uint128::new(amount.amount.u128() - unstaked); - for validator in &validators { - let bonded_amount = self - .bond_requests - .may_load(ctx.deps.storage, validator)? - .unwrap_or_default(); - if bonded_amount >= unstake_amount { - self.bond_requests.save( - ctx.deps.storage, - validator, - &(bonded_amount - unstake_amount), - )?; - // Accounting trick to avoid burning stake - self.burned.update(ctx.deps.storage, validator, |old| { - Ok::<_, ContractError>(old.unwrap_or_default() + unstake_amount.u128()) - })?; - unstaked += unstake_amount.u128(); - break; - } - } - } + // Bail if we still don't have enough stake - if unstaked < amount.amount.u128() { + if burned < amount.amount.u128() { return Err(ContractError::InsufficientDelegations( ctx.env.contract.address.to_string(), amount.amount, From 019ddbdbe3acbb8ede73250de267748e0045062d Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 3 Nov 2023 17:30:51 +0100 Subject: [PATCH 10/12] Update lock file --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 5337c773..3eb51171 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -743,6 +743,7 @@ dependencies = [ "itertools 0.11.0", "mesh-apis", "mesh-bindings", + "mesh-burn", "mesh-converter", "mesh-simple-price-feed", "schemars", From 423004677ce0d366088dcf3c796b87af731cfc1a Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sat, 4 Nov 2023 07:52:38 +0100 Subject: [PATCH 11/12] Add distribute burn tests --- packages/burn/src/burn.rs | 106 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/packages/burn/src/burn.rs b/packages/burn/src/burn.rs index 5e2aa11c..7d9267ec 100644 --- a/packages/burn/src/burn.rs +++ b/packages/burn/src/burn.rs @@ -38,3 +38,109 @@ pub fn distribute_burn( } (burned, burns) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn distribute_burn_works() { + let delegations = vec![ + ("validator1".to_string(), 100), + ("validator2".to_string(), 200), + ("validator3".to_string(), 300), + ]; + let (burned, burns) = distribute_burn(&delegations, 100); + assert_eq!(burned, 100); + assert_eq!(burns.len(), 4); + assert_eq!(burns[0].0, "validator1"); + assert_eq!(burns[0].1, 33); + assert_eq!(burns[1].0, "validator2"); + assert_eq!(burns[1].1, 33); + assert_eq!(burns[2].0, "validator3"); + assert_eq!(burns[2].1, 33); + // And the remainder + assert_eq!(burns[3].0, "validator1"); + assert_eq!(burns[3].1, 1); + } + + /// Panics on empty delegations + #[test] + #[should_panic] + fn distribute_burn_empty_distributions() { + let delegations = vec![]; + distribute_burn(&delegations, 100); + } + + #[test] + fn distribute_burn_one_validator() { + let delegations = vec![("validator1".to_string(), 100)]; + let (burned, burns) = distribute_burn(&delegations, 100); + assert_eq!(burned, 100); + assert_eq!(burns.len(), 1); + assert_eq!(burns[0].0, "validator1"); + assert_eq!(burns[0].1, 100); + } + + /// Some validators do not have enough funds, so the remainder is burned from the first validator + /// that has enough funds + #[test] + fn distribute_burn_unfunded_validator() { + let delegations = vec![ + ("validator1".to_string(), 100), + ("validator2".to_string(), 1), + ]; + let (burned, burns) = distribute_burn(&delegations, 101); + assert_eq!(burned, 101); + assert_eq!(burns.len(), 3); + assert_eq!(burns[0].0, "validator1"); + assert_eq!(burns[0].1, 50); + assert_eq!(burns[1].0, "validator2"); + assert_eq!(burns[1].1, 1); + assert_eq!(burns[2].0, "validator1"); + assert_eq!(burns[2].1, 50); + } + + /// There are not enough funds to burn, so the returned burned amount is less that the requested amount + #[test] + fn distribute_burn_insufficient_delegations() { + let delegations = vec![ + ("validator1".to_string(), 100), + ("validator2".to_string(), 1), + ]; + let (burned, burns) = distribute_burn(&delegations, 102); + assert_eq!(burned, 52); + assert_eq!(burns.len(), 2); + assert_eq!(burns[0].0, "validator1"); + assert_eq!(burns[0].1, 51); + assert_eq!(burns[1].0, "validator2"); + assert_eq!(burns[1].1, 1); + } + + /// There are enough funds to burn, but they are not consolidated enough in a single delegation. + /// This is a limitation of the current impl. + #[test] + fn distribute_burn_insufficient_whole_delegation() { + let delegations = vec![ + ("validator1".to_string(), 29), + ("validator2".to_string(), 30), + ("validator3".to_string(), 31), + ("validator4".to_string(), 1), + ]; + assert_eq!( + delegations.iter().map(|(_, amount)| amount).sum::(), + 91 + ); + let (burned, burns) = distribute_burn(&delegations, 91); + assert_eq!(burned, 67); + assert_eq!(burns.len(), 4); + assert_eq!(burns[0].0, "validator1"); + assert_eq!(burns[0].1, 22); + assert_eq!(burns[1].0, "validator2"); + assert_eq!(burns[1].1, 22); + assert_eq!(burns[2].0, "validator3"); + assert_eq!(burns[2].1, 22); + assert_eq!(burns[3].0, "validator4"); + assert_eq!(burns[3].1, 1); + } +} From 3510b505798433235d292f8a8c41b39735caf01f Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sat, 4 Nov 2023 13:16:06 +0100 Subject: [PATCH 12/12] Improve burns generation --- packages/burn/src/burn.rs | 85 ++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/packages/burn/src/burn.rs b/packages/burn/src/burn.rs index 7d9267ec..14d59e3e 100644 --- a/packages/burn/src/burn.rs +++ b/packages/burn/src/burn.rs @@ -1,4 +1,5 @@ use std::cmp::min; +use std::collections::HashMap; /// Tries to burn `amount` evenly from `delegations`. /// Assigns the remainder to the first validator that has enough stake. @@ -7,12 +8,12 @@ use std::cmp::min; /// Returns the total amount burned, and the list of validators and amounts. /// The total burned amount can be used to check if the user has enough stake in `delegations`. /// -/// N.B..: This can be improved by distributing the remainder evenly across validators +/// N.B..: This can be improved by distributing the remainder evenly across validators. pub fn distribute_burn( delegations: &[(String, u128)], amount: u128, ) -> (u128, Vec<(&String, u128)>) { - let mut burns = vec![]; + let mut burns = HashMap::new(); let mut burned = 0; let proportional_amount = amount / delegations.len() as u128; for (validator, delegated_amount) in delegations { @@ -21,7 +22,10 @@ pub fn distribute_burn( if burn_amount == 0 { continue; } - burns.push((validator, burn_amount)); + burns + .entry(validator) + .and_modify(|amount| *amount += burn_amount) + .or_insert(burn_amount); burned += burn_amount; } // Adjust possible rounding issues / unfunded validators @@ -29,20 +33,37 @@ pub fn distribute_burn( // Look for the first validator that has enough stake, and burn it from there let burn_amount = amount - burned; for (validator, delegated_amount) in delegations { - if burn_amount + proportional_amount <= *delegated_amount { - burns.push((validator, burn_amount)); + if burn_amount + burns.get(&validator).unwrap_or(&0) <= *delegated_amount { + burns + .entry(validator) + .and_modify(|amount| *amount += burn_amount) + .or_insert(burn_amount); burned += burn_amount; break; } } } - (burned, burns) + (burned, burns.into_iter().collect()) } #[cfg(test)] mod tests { use super::*; + #[track_caller] + fn assert_burns(burns: &[(&String, u128)], expected: &[(&str, u128)]) { + let mut burns = burns + .iter() + .map(|(validator, amount)| (validator.to_string(), *amount)) + .collect::>(); + burns.sort_by(|(v1, _), (v2, _)| v1.cmp(v2)); + let expected = expected + .iter() + .map(|(validator, amount)| (validator.to_string(), *amount)) + .collect::>(); + assert_eq!(burns, expected); + } + #[test] fn distribute_burn_works() { let delegations = vec![ @@ -52,16 +73,10 @@ mod tests { ]; let (burned, burns) = distribute_burn(&delegations, 100); assert_eq!(burned, 100); - assert_eq!(burns.len(), 4); - assert_eq!(burns[0].0, "validator1"); - assert_eq!(burns[0].1, 33); - assert_eq!(burns[1].0, "validator2"); - assert_eq!(burns[1].1, 33); - assert_eq!(burns[2].0, "validator3"); - assert_eq!(burns[2].1, 33); - // And the remainder - assert_eq!(burns[3].0, "validator1"); - assert_eq!(burns[3].1, 1); + assert_burns( + &burns, + &[("validator1", 34), ("validator2", 33), ("validator3", 33)], + ); } /// Panics on empty delegations @@ -77,9 +92,7 @@ mod tests { let delegations = vec![("validator1".to_string(), 100)]; let (burned, burns) = distribute_burn(&delegations, 100); assert_eq!(burned, 100); - assert_eq!(burns.len(), 1); - assert_eq!(burns[0].0, "validator1"); - assert_eq!(burns[0].1, 100); + assert_burns(&burns, &[("validator1", 100)]); } /// Some validators do not have enough funds, so the remainder is burned from the first validator @@ -92,13 +105,7 @@ mod tests { ]; let (burned, burns) = distribute_burn(&delegations, 101); assert_eq!(burned, 101); - assert_eq!(burns.len(), 3); - assert_eq!(burns[0].0, "validator1"); - assert_eq!(burns[0].1, 50); - assert_eq!(burns[1].0, "validator2"); - assert_eq!(burns[1].1, 1); - assert_eq!(burns[2].0, "validator1"); - assert_eq!(burns[2].1, 50); + assert_burns(&burns, &[("validator1", 100), ("validator2", 1)]); } /// There are not enough funds to burn, so the returned burned amount is less that the requested amount @@ -110,15 +117,11 @@ mod tests { ]; let (burned, burns) = distribute_burn(&delegations, 102); assert_eq!(burned, 52); - assert_eq!(burns.len(), 2); - assert_eq!(burns[0].0, "validator1"); - assert_eq!(burns[0].1, 51); - assert_eq!(burns[1].0, "validator2"); - assert_eq!(burns[1].1, 1); + assert_burns(&burns, &[("validator1", 51), ("validator2", 1)]); } /// There are enough funds to burn, but they are not consolidated enough in a single delegation. - /// This is a limitation of the current impl. + // FIXME? This is a limitation of the current impl. #[test] fn distribute_burn_insufficient_whole_delegation() { let delegations = vec![ @@ -133,14 +136,14 @@ mod tests { ); let (burned, burns) = distribute_burn(&delegations, 91); assert_eq!(burned, 67); - assert_eq!(burns.len(), 4); - assert_eq!(burns[0].0, "validator1"); - assert_eq!(burns[0].1, 22); - assert_eq!(burns[1].0, "validator2"); - assert_eq!(burns[1].1, 22); - assert_eq!(burns[2].0, "validator3"); - assert_eq!(burns[2].1, 22); - assert_eq!(burns[3].0, "validator4"); - assert_eq!(burns[3].1, 1); + assert_burns( + &burns, + &[ + ("validator1", 22), + ("validator2", 22), + ("validator3", 22), + ("validator4", 1), + ], + ); } }