Merge pull request #152 from osmosis-labs/124-improve-cross-bond-unbond

Immediately release stake when possible

contracts/provider/external-staking/src/contract.rs

@@ -17,7 +17,7 @@ use mesh_apis::ibc::{AddValidator, ProviderPacket};
 use mesh_apis::vault_api::{SlashInfo, VaultApiHelper};
 use mesh_sync::{Tx, ValueRange};
 
-use crate::crdt::CrdtState;
+use crate::crdt::{CrdtState, State};
 use crate::error::ContractError;
 use crate::ibc::{packet_timeout, IBC_CHANNEL};
 use crate::msg::{
@@ -370,9 +370,18 @@ impl ExternalStakingContract<'_> {
         let amount = min(tx_amount, stake.stake.high());
         stake.stake.commit_sub(amount);
 
+        let immediate_release = matches!(
+            self.val_set.validator_state(deps.storage, &tx_validator)?,
+            State::Unbonded {} | State::Tombstoned {}
+        );
+
         // FIXME? Release period being computed after successful IBC tx
         // (Note: this is good for now, but can be revisited in v1 design)
-        let release_at = env.block.time.plus_seconds(config.unbonding_period);
+        let release_at = if immediate_release {
+            env.block.time
+        } else {
+            env.block.time.plus_seconds(config.unbonding_period)
+        };
         let unbond = PendingUnbond { amount, release_at };
         stake.pending_unbonds.push(unbond);
 

contracts/provider/external-staking/src/crdt.rs

@@ -28,7 +28,7 @@ impl ValidatorState {
         if self.is_empty() {
             State::Unknown {}
         } else {
-            self.0[0].state.clone()
+            self.0[0].state
         }
     }
 
@@ -66,6 +66,7 @@ pub struct ValState {
 }
 
 #[cw_serde]
+#[derive(Copy)]
 pub enum State {
     /// Validator is part of the validator set.
     Active {},
@@ -393,6 +394,14 @@ impl<'a> CrdtState<'a> {
         self.validators.save(storage, valoper, &validator_state)?;
         Ok(())
     }
+
+    pub fn validator_state(&self, storage: &dyn Storage, valoper: &str) -> StdResult<State> {
+        Ok(self
+            .validators
+            .may_load(storage, valoper)?
+            .map(|state| state.get_state())
+            .unwrap_or(State::Unknown {}))
+    }
 }
 
 #[cfg(test)]

contracts/provider/external-staking/src/multitest.rs

@@ -453,6 +453,82 @@ fn unstaking() {
     assert_eq!(claim.amount.val().unwrap().u128(), 240);
 }
 
+#[test]
+fn immediate_unstake_if_unbonded_validator() {
+    let user = "user1";
+
+    let app = App::new_with_balances(&[(user, &coins(200, OSMO))]);
+
+    let owner = "owner";
+
+    let (vault, contract) = setup(&app, owner, 100).unwrap();
+
+    let validators = contract.activate_validators(["validator1"]);
+
+    vault
+        .bond()
+        .with_funds(&coins(200, OSMO))
+        .call(user)
+        .unwrap();
+    vault.stake(&contract, user, validators[0], coin(200, OSMO));
+
+    contract.remove_validator(validators[0]);
+
+    contract
+        .unstake(validators[0].to_string(), coin(200, OSMO))
+        .call(user)
+        .unwrap();
+    contract
+        .test_methods_proxy()
+        .test_commit_unstake(get_last_external_staking_pending_tx_id(&contract).unwrap())
+        .call("test")
+        .unwrap();
+    contract.withdraw_unbonded().call(user).unwrap();
+
+    let claim = vault
+        .claim(user.to_string(), contract.contract_addr.to_string())
+        .unwrap();
+    assert_eq!(claim.amount.val().unwrap().u128(), 0);
+}
+
+#[test]
+fn immediate_unstake_if_tombstoned_validator() {
+    let user = "user1";
+
+    let app = App::new_with_balances(&[(user, &coins(200, OSMO))]);
+
+    let owner = "owner";
+
+    let (vault, contract) = setup(&app, owner, 100).unwrap();
+
+    let validators = contract.activate_validators(["validator1"]);
+
+    vault
+        .bond()
+        .with_funds(&coins(200, OSMO))
+        .call(user)
+        .unwrap();
+    vault.stake(&contract, user, validators[0], coin(200, OSMO));
+
+    contract.tombstone_validator(validators[0]);
+
+    contract
+        .unstake(validators[0].to_string(), coin(200, OSMO))
+        .call(user)
+        .unwrap();
+    contract
+        .test_methods_proxy()
+        .test_commit_unstake(get_last_external_staking_pending_tx_id(&contract).unwrap())
+        .call("test")
+        .unwrap();
+    contract.withdraw_unbonded().call(user).unwrap();
+
+    let claim = vault
+        .claim(user.to_string(), contract.contract_addr.to_string())
+        .unwrap();
+    assert_eq!(claim.amount.val().unwrap().u128(), 0);
+}
+
 #[test]
 fn distribution() {
     let owner = "owner";

contracts/provider/external-staking/src/multitest/utils.rs

@@ -64,6 +64,9 @@ pub(crate) trait ContractExt {
         validators: [&'static str; N],
     ) -> [&'static str; N];
 
+    fn remove_validator(&self, validator: &'static str);
+    fn tombstone_validator(&self, validator: &'static str);
+
     fn distribute_batch(
         &self,
         caller: impl AsRef<str>,
@@ -89,6 +92,22 @@ impl ContractExt for Contract<'_> {
         validators
     }
 
+    #[track_caller]
+    fn remove_validator(&self, validator: &'static str) {
+        self.test_methods_proxy()
+            .test_remove_validator(validator.to_string(), 101, 1234)
+            .call("test")
+            .unwrap();
+    }
+
+    #[track_caller]
+    fn tombstone_validator(&self, validator: &'static str) {
+        self.test_methods_proxy()
+            .test_tombstone_validator(validator.to_string(), 101, 1234)
+            .call("test")
+            .unwrap();
+    }
+
     #[track_caller]
     fn distribute_batch(
         &self,

contracts/provider/external-staking/src/test_methods.rs

@@ -28,6 +28,25 @@ pub trait TestMethods {
         time: u64,
     ) -> Result<Response, Self::Error>;
 
+    /// Sets validator as `unbonded`.
+    #[msg(exec)]
+    fn test_remove_validator(
+        &self,
+        ctx: ExecCtx,
+        valoper: String,
+        height: u64,
+        time: u64,
+    ) -> Result<Response, Self::Error>;
+
+    #[msg(exec)]
+    fn test_tombstone_validator(
+        &self,
+        ctx: ExecCtx,
+        valoper: String,
+        height: u64,
+        time: u64,
+    ) -> Result<Response, Self::Error>;
+
     /// Commits a pending unstake.
     #[msg(exec)]
     fn test_commit_unstake(&self, ctx: ExecCtx, tx_id: u64) -> Result<Response, Self::Error>;

contracts/provider/external-staking/src/test_methods_impl.rs

@@ -67,6 +67,50 @@ impl TestMethods for ExternalStakingContract<'_> {
         }
     }
 
+    /// Sets validator as `unbonded`.
+    #[msg(exec)]
+    fn test_remove_validator(
+        &self,
+        ctx: ExecCtx,
+        valoper: String,
+        height: u64,
+        time: u64,
+    ) -> Result<Response, ContractError> {
+        #[cfg(any(feature = "mt", test))]
+        {
+            self.val_set
+                .remove_validator(ctx.deps.storage, &valoper, height, time)?;
+            Ok(Response::new())
+        }
+        #[cfg(not(any(feature = "mt", test)))]
+        {
+            let _ = (ctx, valoper, height, time);
+            Err(ContractError::Unauthorized {})
+        }
+    }
+
+    /// Sets validator as `unbonded`.
+    #[msg(exec)]
+    fn test_tombstone_validator(
+        &self,
+        ctx: ExecCtx,
+        valoper: String,
+        height: u64,
+        time: u64,
+    ) -> Result<Response, ContractError> {
+        #[cfg(any(feature = "mt", test))]
+        {
+            self.val_set
+                .tombstone_validator(ctx.deps.storage, &valoper, height, time)?;
+            Ok(Response::new())
+        }
+        #[cfg(not(any(feature = "mt", test)))]
+        {
+            let _ = (ctx, valoper, height, time);
+            Err(ContractError::Unauthorized {})
+        }
+    }
+
     /// Commits a pending unstake.
     #[msg(exec)]
     fn test_commit_unstake(&self, ctx: ExecCtx, tx_id: u64) -> Result<Response, ContractError> {