Merge pull request #137 from osmosis-labs/f/slash-jailed-validators

F/slash jailed validators

Author: maurolacy

contracts/consumer/converter/src/contract.rs

@@ -14,7 +14,9 @@ use mesh_apis::price_feed_api;
 use mesh_apis::virtual_staking_api;
 
 use crate::error::ContractError;
-use crate::ibc::{add_validators_msg, make_ibc_packet, tombstone_validators_msg, IBC_CHANNEL};
+use crate::ibc::{
+    add_validators_msg, jail_validators_msg, make_ibc_packet, tombstone_validators_msg, IBC_CHANNEL,
+};
 use crate::msg::ConfigResponse;
 use crate::state::Config;
 
@@ -352,13 +354,15 @@ impl ConverterApi for ConverterContract<'_> {
         ctx: ExecCtx,
         additions: Vec<Validator>,
         tombstoned: Vec<String>,
+        jailed: Vec<String>,
     ) -> Result<Response, Self::Error> {
         self.ensure_authorized(&ctx.deps, &ctx.info)?;
 
         // Send over IBC to the Consumer
         let channel = IBC_CHANNEL.load(ctx.deps.storage)?;
         let add_msg = add_validators_msg(&ctx.env, &channel, &additions)?;
         let tomb_msg = tombstone_validators_msg(&ctx.env, &channel, &tombstoned)?;
+        let jail_msg = jail_validators_msg(&ctx.env, &channel, &jailed)?;
 
         let event = Event::new("valset_update").add_attribute(
             "additions",
@@ -368,11 +372,13 @@ impl ConverterApi for ConverterContract<'_> {
                 .collect::<Vec<String>>()
                 .join(","),
         );
+        let event = event.add_attribute("jailed", jailed.join(","));
         let event = event.add_attribute("tombstoned", tombstoned.join(","));
         let resp = Response::new()
             .add_event(event)
             .add_message(add_msg)
-            .add_message(tomb_msg);
+            .add_message(tomb_msg)
+            .add_message(jail_msg);
 
         Ok(resp)
     }

contracts/consumer/converter/src/ibc.rs

@@ -146,6 +146,29 @@ pub(crate) fn add_validators_msg(
     Ok(msg)
 }
 
+pub(crate) fn jail_validators_msg(
+    env: &Env,
+    channel: &IbcChannel,
+    validators: &[String],
+) -> Result<IbcMsg, ContractError> {
+    let packet = ConsumerPacket::JailValidators(
+        validators
+            .iter()
+            .map(|v| RemoveValidator {
+                valoper: v.to_string(),
+                height: env.block.height,
+                time: env.block.time.seconds(),
+            })
+            .collect(),
+    );
+    let msg = IbcMsg::SendPacket {
+        channel_id: channel.endpoint.channel_id.clone(),
+        data: to_binary(&packet)?,
+        timeout: packet_timeout_validator(env),
+    };
+    Ok(msg)
+}
+
 pub(crate) fn tombstone_validators_msg(
     env: &Env,
     channel: &IbcChannel,
@@ -156,8 +179,8 @@ pub(crate) fn tombstone_validators_msg(
             .iter()
             .map(|v| RemoveValidator {
                 valoper: v.to_string(),
-                end_height: env.block.height,
-                end_time: env.block.time.seconds(),
+                height: env.block.height,
+                time: env.block.time.seconds(),
             })
             .collect(),
     );

contracts/consumer/converter/src/multitest.rs

@@ -243,13 +243,13 @@ fn valset_update_works() {
     // Check that only the virtual staking contract can call this handler
     let res = converter
         .converter_api_proxy()
-        .valset_update(vec![], vec![])
+        .valset_update(vec![], vec![], vec![])
         .call(owner);
     assert_eq!(res.unwrap_err(), Unauthorized {});
 
     let res = converter
         .converter_api_proxy()
-        .valset_update(add_validators, rem_validators)
+        .valset_update(add_validators, rem_validators, vec![])
         .call(virtual_staking.contract_addr.as_ref());
 
     // This fails because of lack of IBC support in mt now.
@@ -304,7 +304,7 @@ fn unauthorized() {
 
     let err = converter
         .converter_api_proxy()
-        .valset_update(vec![], vec![])
+        .valset_update(vec![], vec![], vec![])
         .call("mallory")
         .unwrap_err();
 

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

@@ -156,16 +156,13 @@ impl VirtualStakingContract<'_> {
         // the `bonded` list
         let _ = (removals, updated, jailed, unjailed);
 
-        // Send additions and tombstones to the Converter. Removals are non-permanent and ignored
-        // TODO: Send jailed even when they are non-permanent, for slashing
-        // FIXME: Account for tombstoned validators `bond_requests`. Add a `slashings` field to the config,
-        // and avoid unbondings of slashed amounts.
-        // FIXME: Account for jailed validators `bond_requests`. avoid unbondings of slashed
-        // amounts. Requires computing the slashing amount, i.e. obtaining the chain's slash_ratio.
+        // Send additions and tombstones to the Converter. Removals are non-permanent and ignored.
+        // Send jailed even when they are non-permanent, for slashing.
         let cfg = self.config.load(deps.storage)?;
         let msg = converter_api::ExecMsg::ValsetUpdate {
             additions: additions.to_vec(),
             tombstoned: tombstoned.to_vec(),
+            jailed: jailed.to_vec(),
         };
         let msg = WasmMsg::Execute {
             contract_addr: cfg.converter.to_string(),

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

@@ -9,8 +9,8 @@ use cosmwasm_std::{
 use cw_storage_plus::Item;
 use mesh_apis::ibc::{
     ack_success, validate_channel_order, AckWrapper, AddValidator, AddValidatorsAck,
-    ConsumerPacket, DistributeAck, ProtocolVersion, ProviderPacket, RemoveValidator,
-    RemoveValidatorsAck,
+    ConsumerPacket, DistributeAck, JailValidatorsAck, ProtocolVersion, ProviderPacket,
+    RemoveValidator, RemoveValidatorsAck,
 };
 
 use crate::contract::ExternalStakingContract;
@@ -149,8 +149,8 @@ pub fn ibc_packet_receive(
             let mut msgs = vec![];
             for RemoveValidator {
                 valoper,
-                end_height,
-                end_time: _end_time,
+                height: end_height,
+                time: _end_time,
             } in to_remove
             {
                 // Check that the validator is active at height and slash it if that is the case
@@ -170,6 +170,32 @@ pub fn ibc_packet_receive(
             let ack = ack_success(&RemoveValidatorsAck {})?;
             IbcReceiveResponse::new().set_ack(ack).add_messages(msgs)
         }
+        ConsumerPacket::JailValidators(to_jail) => {
+            let mut msgs = vec![];
+            for RemoveValidator {
+                valoper,
+                height: end_height,
+                time: _end_time,
+            } in to_jail
+            {
+                // Check that the validator is active at height and slash it if that is the case
+                let active = contract.val_set.is_active_validator_at_height(
+                    deps.storage,
+                    &valoper,
+                    end_height,
+                )?;
+                // We don't change the validator's state here, as that's currently not supported
+                // (only Active and Tombstoned)
+                if active {
+                    // slash the validator
+                    // TODO: Slash with a different slash ratio! (downtime / offline slash ratio)
+                    let msg = contract.handle_slashing(&env, deps.storage, &valoper)?;
+                    msgs.push(msg);
+                }
+            }
+            let ack = ack_success(&JailValidatorsAck {})?;
+            IbcReceiveResponse::new().set_ack(ack).add_messages(msgs)
+        }
         ConsumerPacket::Distribute { validator, rewards } => {
             let contract = ExternalStakingContract::new();
             let evt = contract.distribute_rewards(deps, &validator, rewards)?;

packages/apis/src/converter_api.rs

@@ -42,6 +42,7 @@ pub trait ConverterApi {
         ctx: ExecCtx,
         additions: Vec<Validator>,
         tombstoned: Vec<String>,
+        jailed: Vec<String>,
     ) -> Result<Response, Self::Error>;
 }
 

packages/apis/src/ibc/packet.rs

@@ -65,6 +65,10 @@ pub enum ConsumerPacket {
     /// but when it is no longer a valid target to delegate to.
     /// It contains a list of `valoper_address` to be removed, along with the removal's height.
     RemoveValidators(Vec<RemoveValidator>),
+    /// This is sent when a validator is jailed.
+    /// It contains a list of `valoper_address` to be slashed for temporary jailing, along with the
+    /// jail event's block height.
+    JailValidators(Vec<RemoveValidator>),
     /// This is part of the rewards protocol
     Distribute {
         /// The validator whose stakers should receive these rewards
@@ -117,15 +121,15 @@ pub struct RemoveValidator {
     /// This is the validator operator (valoper) address used for delegations and rewards
     pub valoper: String,
 
-    /// This is the height the validator is being tombstoned.
-    /// It is used to detect slashing conditions, that is, avoid slashing an already tombstoned
+    /// This is the height the validator is being removed.
+    /// It is used to detect slashing conditions, that is, avoid slashing an already jailed or tombstoned
     /// validator.
-    pub end_height: u64,
+    pub height: u64,
 
-    /// This is the timestamp of the block the validator was tombstoned.
+    /// This is the timestamp of the block the validator was removed.
     /// It may be used for unbonding_period issues, maybe just for informational purposes.
     /// Stored as unix seconds.
-    pub end_time: u64,
+    pub time: u64,
 }
 
 /// Ack sent for ConsumerPacket::AddValidators
@@ -136,6 +140,10 @@ pub struct AddValidatorsAck {}
 #[cw_serde]
 pub struct RemoveValidatorsAck {}
 
+/// Ack sent for ConsumerPacket::JailValidators
+#[cw_serde]
+pub struct JailValidatorsAck {}
+
 /// Ack sent for ConsumerPacket::Distribute
 #[cw_serde]
 pub struct DistributeAck {}