Merge pull request #201 from osmosis-labs/trinity/tombstoned-validato…

…r-unbonding

feat: auto unbond tombstoned validator

Cargo.toml

@@ -18,7 +18,6 @@ mesh-apis = { path = "./packages/apis" }
 mesh-bindings = { path = "./packages/bindings" }
 mesh-burn = { path = "./packages/burn" }
 mesh-sync = { path = "./packages/sync" }
-mesh-virtual-staking-mock = { path = "./packages/virtual-staking-mock" }
 
 mesh-vault = { path = "./contracts/provider/vault" }
 mesh-external-staking = { path = "./contracts/provider/external-staking" }

contracts/consumer/converter/Cargo.toml

@@ -41,6 +41,7 @@ thiserror = { workspace = true }
 mesh-burn = { workspace = true }
 mesh-simple-price-feed = { workspace = true, features = ["mt"] }
 mesh-virtual-staking = { workspace = true, features = ["mt"] }
+
 cw-multi-test = { workspace = true }
 test-case = { workspace = true }
 derivative = { workspace = true }

contracts/consumer/converter/src/contract.rs

@@ -73,6 +73,7 @@ impl ConverterContract<'_> {
         discount: Decimal,
         remote_denom: String,
         virtual_staking_code_id: u64,
+        tombstoned_unbond_enable: bool,
         admin: Option<String>,
         max_retrieve: u16,
     ) -> Result<custom::Response, ContractError> {
@@ -98,8 +99,10 @@ impl ConverterContract<'_> {
             ctx.deps.api.addr_validate(admin)?;
         }
 
-        let msg =
-            to_json_binary(&mesh_virtual_staking::contract::sv::InstantiateMsg { max_retrieve })?;
+        let msg = to_json_binary(&mesh_virtual_staking::contract::sv::InstantiateMsg {
+            max_retrieve,
+            tombstoned_unbond_enable,
+        })?;
         // Instantiate virtual staking contract
         let init_msg = WasmMsg::Instantiate {
             admin,

contracts/consumer/converter/src/multitest.rs

@@ -61,6 +61,7 @@ fn setup<'a>(app: &'a App<MtApp>, args: SetupArgs<'a>) -> SetupResponse<'a> {
             discount,
             JUNO.to_owned(),
             virtual_staking_code.code_id(),
+            true,
             Some(admin.to_owned()),
             50,
         )

contracts/consumer/virtual-staking/src/contract.rs

@@ -43,7 +43,7 @@ pub struct VirtualStakingContract<'a> {
     /// This is what validators are inactive because of tombstoning, jailing or removal (unbonded).
     // `inactive` could be a Map like `bond_requests`, but the only time we use it is to read / write the entire list in bulk (in handle_epoch),
     // never accessing one element. Reading 100 elements in an Item is much cheaper than ranging over a Map with 100 entries.
-    pub inactive: Item<'a, Vec<String>>,
+    pub inactive: Item<'a, Vec<(String, bool)>>,
     /// Amount of tokens that have been burned from a validator.
     /// This is just for accounting / tracking reasons, as token "burning" is being implemented as unbonding,
     /// and there's no real need to discount the burned amount in this contract.
@@ -75,13 +75,15 @@ impl VirtualStakingContract<'_> {
         &self,
         ctx: InstantiateCtx<VirtualStakeCustomQuery>,
         max_retrieve: u16,
+        tombstoned_unbond_enable: bool,
     ) -> Result<Response<VirtualStakeCustomMsg>, ContractError> {
         nonpayable(&ctx.info)?;
         let denom = ctx.deps.querier.query_bonded_denom()?;
         let config = Config {
             denom,
             converter: ctx.info.sender,
             max_retrieve,
+            tombstoned_unbond_enable,
         };
         self.config.save(ctx.deps.storage, &config)?;
         // initialize these to no one, so no issue when reading for the first time
@@ -294,6 +296,7 @@ fn pop_target(deps: DepsMut<VirtualStakeCustomQuery>) -> StdResult<(String, bool
 fn calculate_rebalance(
     current: Vec<(String, Uint128)>,
     desired: Vec<(String, Uint128)>,
+    tombstoned_list: HashMap<String, Coin>,
     denom: &str,
 ) -> Vec<CosmosMsg<VirtualStakeCustomMsg>> {
     let mut desired: BTreeMap<_, _> = desired.into_iter().collect();
@@ -302,6 +305,13 @@ fn calculate_rebalance(
     let mut msgs = vec![];
     for (validator, prev) in current {
         let next = desired.remove(&validator).unwrap_or_else(Uint128::zero);
+        if tombstoned_list.contains_key(&validator) && !next.is_zero() {
+            let amount = tombstoned_list.get(&validator).unwrap().clone();
+            if !amount.amount.is_zero() {
+                msgs.push(VirtualStakeMsg::Unbond { validator, amount }.into());
+            }
+            continue;
+        }
         match next.cmp(&prev) {
             Ordering::Less => {
                 let unbond = prev - next;
@@ -636,8 +646,15 @@ impl VirtualStakingApi for VirtualStakingContract<'_> {
 
         // withdraw rewards
         let bonded = self.bonded.load(deps.storage)?;
-        let inactive = self.inactive.load(deps.storage)?;
-        let withdraw = withdraw_reward_msgs(deps.branch(), &bonded, &inactive);
+        let inactive_list = self.inactive.load(deps.storage)?;
+        let withdraw = withdraw_reward_msgs(
+            deps.branch(),
+            &bonded,
+            &inactive_list
+                .iter()
+                .map(|(i, _)| i.to_string())
+                .collect::<Vec<_>>(),
+        );
         let mut resp = Response::new().add_submessages(withdraw);
 
         let bond =
@@ -647,7 +664,6 @@ impl VirtualStakingApi for VirtualStakingContract<'_> {
         let config = self.config.load(deps.storage)?;
         // If 0 max cap, then we assume all tokens were force unbonded already, and just return the withdraw rewards
         // call and set bonded to empty
-        // TODO: verify this behavior with SDK module (otherwise we send unbond message)
         if max_cap.is_zero() {
             let all_delegations = TokenQuerier::new(&deps.querier)
                 .all_delegations(env.contract.address.to_string(), config.max_retrieve)?;
@@ -684,7 +700,12 @@ impl VirtualStakingApi for VirtualStakingContract<'_> {
             self.adjust_slashings(deps.branch(), &mut current, &slash)?;
             // Update inactive list. Defensive, as it should already been updated in handle_valset_update, due to removals
             self.inactive.update(deps.branch().storage, |mut old| {
-                old.extend_from_slice(&slash.iter().map(|v| v.address.clone()).collect::<Vec<_>>());
+                old.extend_from_slice(
+                    &slash
+                        .iter()
+                        .map(|v| (v.address.clone(), v.is_tombstoned))
+                        .collect::<Vec<_>>(),
+                );
                 old.dedup();
                 Ok::<_, ContractError>(old)
             })?;
@@ -709,11 +730,31 @@ impl VirtualStakingApi for VirtualStakingContract<'_> {
             }
         }
 
+        // Force the tombstoned validator to auto unbond
+        let mut tombstoned_list: HashMap<String, Coin> = HashMap::new();
+        for (val, is_tombstoned) in inactive_list.iter() {
+            if *is_tombstoned {
+                let resp = TokenQuerier::new(&deps.querier)
+                    .total_delegations(env.contract.address.to_string(), val.to_string())?;
+                tombstoned_list.insert(val.to_string(), resp.delegation);
+            }
+        }
+
+        let mut request_with_tombstoned = requests.clone();
+        for (val, amount) in request_with_tombstoned.iter_mut() {
+            if tombstoned_list.contains_key(val) {
+                *amount = Uint128::zero();
+                // Update new value for the bond requests
+                self.bond_requests.save(deps.storage, val, amount)?;
+            }
+        }
+
         // Save the future values
-        self.bonded.save(deps.branch().storage, &requests)?;
+        self.bonded
+            .save(deps.branch().storage, &request_with_tombstoned)?;
 
         // Compare these two to make bond/unbond calls as needed
-        let rebalance = calculate_rebalance(current, requests, &config.denom);
+        let rebalance = calculate_rebalance(current, requests, tombstoned_list, &config.denom);
         resp = resp.add_messages(rebalance);
 
         Ok(resp)
@@ -756,12 +797,23 @@ impl VirtualStakingApi for VirtualStakingContract<'_> {
 
         // Update inactive list.
         // We ignore `unjailed` as it's not clear they make the validator active again or not.
-        if !removals.is_empty() || !additions.is_empty() {
+        if !removals.is_empty() || !additions.is_empty() || !tombstoned.is_empty() {
             self.inactive.update(deps.storage, |mut old| {
                 // Add removals
-                old.extend_from_slice(removals);
+                old.extend_from_slice(
+                    &tombstoned
+                        .iter()
+                        .map(|t| (t.to_string(), true))
+                        .collect::<Vec<_>>(),
+                );
+                old.extend_from_slice(
+                    &removals
+                        .iter()
+                        .map(|r| (r.to_string(), false))
+                        .collect::<Vec<_>>(),
+                );
                 // Filter additions
-                old.retain(|v| !additions.iter().any(|a| a.address == *v));
+                old.retain(|v| !additions.iter().any(|a| a.address == *v.0));
                 old.dedup();
                 Ok::<_, ContractError>(old)
             })?;
@@ -810,7 +862,7 @@ mod tests {
         testing::{mock_env, mock_info, MockApi, MockQuerier, MockStorage},
         AllDelegationsResponse, Decimal,
     };
-    use mesh_bindings::{BondStatusResponse, SlashRatioResponse};
+    use mesh_bindings::{BondStatusResponse, SlashRatioResponse, TotalDelegationResponse};
 
     use super::*;
 
@@ -1191,11 +1243,14 @@ mod tests {
 
         // Val1 is being tombstoned
         contract.tombstone(deps.as_mut(), "val1", Decimal::percent(25), Uint128::new(5));
+        knobs
+            .total_delegation
+            .update_total_delegation(15u128, &denom);
         contract
             .hit_epoch(deps.as_mut())
             .assert_bond(&[]) // No bond msgs after tombstoning
-            .assert_unbond(&[]) // No unbond msgs after tombstoning
-            .assert_rewards(&["val1", "val2"]); // Last rewards msgs after tombstoning
+            .assert_unbond(&[("val1", (15u128, &denom))]) // No unbond msgs after tombstoning
+            .assert_rewards(&["val2"]); // Last rewards msgs after tombstoning
 
         // Check that the bonded amounts of val1 have been slashed for double sign (25%)
         // Val2 is unaffected.
@@ -1204,7 +1259,7 @@ mod tests {
         assert_eq!(
             bonded,
             [
-                ("val1".to_string(), Uint128::new(15)),
+                ("val1".to_string(), Uint128::new(0)),
                 ("val2".to_string(), Uint128::new(20))
             ]
         );
@@ -1233,24 +1288,31 @@ mod tests {
 
         // And it's being tombstoned at the same time
         contract.tombstone(deps.as_mut(), "val1", Decimal::percent(25), Uint128::new(2));
+        knobs
+            .total_delegation
+            .update_total_delegation(28u128, &denom);
 
         contract
             .hit_epoch(deps.as_mut())
-            .assert_bond(&[("val1", (20, &denom))]) // FIXME?: Tombstoned validators can still bond
-            .assert_unbond(&[])
-            .assert_rewards(&["val1"]); // Rewards are still being gathered
+            .assert_bond(&[]) // Tombstoned validators will be auto unbond
+            .assert_unbond(&[("val1", (28u128, &denom))])
+            .assert_rewards(&[]); // Rewards are still being gathered
 
         // Check that the previously bonded amounts of val1 have been slashed for double sign (25%)
         let bonded = contract.bonded.load(deps.as_ref().storage).unwrap();
         assert_eq!(
             bonded,
             [
-                ("val1".to_string(), Uint128::new(8 + 20)), // Due to rounding up
+                ("val1".to_string(), Uint128::new(0)), // Due to rounding up
             ]
         );
 
         // Subsequent rewards msgs are removed after validator is tombstoned
-        contract.hit_epoch(deps.as_mut()).assert_rewards(&[]);
+        contract
+            .hit_epoch(deps.as_mut())
+            .assert_bond(&[]) // Tombstoned validators can still bond
+            .assert_unbond(&[])
+            .assert_rewards(&[]);
     }
 
     #[test]
@@ -1278,7 +1340,7 @@ mod tests {
             .hit_epoch(deps.as_mut())
             .assert_bond(&[]) // No bond msgs after jailing
             .assert_unbond(&[("val1", (8u128, &denom))]) // Unbond adjusted for double sign slashing
-            .assert_rewards(&["val1"]); // Rewards are still being gathered
+            .assert_rewards(&[]); // Rewards are still being gathered
 
         // Check that bonded accounting has been adjusted
         let bonded = contract.bonded.load(deps.as_ref().storage).unwrap();
@@ -1374,12 +1436,16 @@ mod tests {
             slash_fraction_downtime: "0.1".to_string(),
             slash_fraction_double_sign: "0.25".to_string(),
         });
+        let total_delegation = MockTotalDelegation::new(TotalDelegationResponse {
+            delegation: coin(0, "DOES NOT MATTER"),
+        });
         let all_delegations = MockAllDelegations::new(AllDelegationsResponse {
             delegations: vec![],
         });
 
         let handler = {
             let bs_copy = bond_status.clone();
+            let td_copy = total_delegation.clone();
             move |msg: &_| {
                 let VirtualStakeCustomQuery::VirtualStake(msg) = msg;
                 match msg {
@@ -1393,6 +1459,11 @@ mod tests {
                             to_json_binary(&*slash_ratio.borrow()).unwrap(),
                         ))
                     }
+                    mesh_bindings::VirtualStakeQuery::TotalDelegation { .. } => {
+                        cosmwasm_std::SystemResult::Ok(cosmwasm_std::ContractResult::Ok(
+                            to_json_binary(&*td_copy.borrow()).unwrap(),
+                        ))
+                    }
                     mesh_bindings::VirtualStakeQuery::AllDelegations { .. } => {
                         cosmwasm_std::SystemResult::Ok(cosmwasm_std::ContractResult::Ok(
                             to_json_binary(&*all_delegations.borrow()).unwrap(),
@@ -1409,12 +1480,16 @@ mod tests {
                 querier: MockQuerier::new(&[]).with_custom_handler(handler),
                 custom_query_type: PhantomData,
             },
-            StakingKnobs { bond_status },
+            StakingKnobs {
+                bond_status,
+                total_delegation,
+            },
         )
     }
 
     struct StakingKnobs {
         bond_status: MockBondStatus,
+        total_delegation: MockTotalDelegation,
     }
 
     #[derive(Clone)]
@@ -1448,6 +1523,26 @@ mod tests {
         }
     }
 
+    #[derive(Clone)]
+    struct MockTotalDelegation(Rc<RefCell<TotalDelegationResponse>>);
+
+    impl MockTotalDelegation {
+        fn new(res: TotalDelegationResponse) -> Self {
+            Self(Rc::new(RefCell::new(res)))
+        }
+
+        fn borrow(&self) -> Ref<'_, TotalDelegationResponse> {
+            self.0.borrow()
+        }
+
+        fn update_total_delegation(&self, amount: impl Into<Uint128>, denom: impl Into<String>) {
+            let mut mut_obj = self.0.borrow_mut();
+            mut_obj.delegation = Coin {
+                amount: amount.into(),
+                denom: denom.into(),
+            };
+        }
+    }
     #[derive(Clone)]
     struct MockAllDelegations(Rc<RefCell<AllDelegationsResponse>>);
 
@@ -1510,6 +1605,7 @@ mod tests {
                     info: mock_info("me", &[]),
                 },
                 50,
+                true,
             )
             .unwrap();
         }
@@ -1631,6 +1727,7 @@ mod tests {
                     power: 0,
                     slash_amount,
                     slash_ratio: nominal_slash_ratio.to_string(),
+                    is_tombstoned: false,
                 }]),
             )
             .unwrap();
@@ -1683,6 +1780,7 @@ mod tests {
                     power: 0,
                     slash_amount,
                     slash_ratio: nominal_slash_ratio.to_string(),
+                    is_tombstoned: true,
                 }]),
             )
             .unwrap();

contracts/consumer/virtual-staking/src/multitest.rs

@@ -59,6 +59,7 @@ fn setup<'a>(app: &'a App, args: SetupArgs<'a>) -> SetupResponse<'a> {
             discount,
             JUNO.to_owned(),
             virtual_staking_code.code_id(),
+            true,
             Some(admin.to_owned()),
             50,
         )

contracts/consumer/virtual-staking/src/state.rs

@@ -9,7 +9,9 @@ pub struct Config {
     /// The address of the converter contract (that is authorized to bond/unbond and will receive rewards)
     pub converter: Addr,
 
-    /// Maximum
-    ///
+    /// If it enable, tombstoned validators will be unbond automatically
+    pub tombstoned_unbond_enable: bool,
+
+    /// Maximum delegations per query
     pub max_retrieve: u16,
 }