Merge pull request #121 from osmosis-labs/f/validator-tombstones

Add Consumer validator tombstoning routing / msgs

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, packet_timeout_rewards, IBC_CHANNEL};
+use crate::ibc::{
+    add_validators_msg, packet_timeout_rewards, tombstone_validators_msg, IBC_CHANNEL,
+};
 use crate::msg::ConfigResponse;
 use crate::state::Config;
 
@@ -335,6 +337,7 @@ impl ConverterApi for ConverterContract<'_> {
         &self,
         ctx: ExecCtx,
         additions: Vec<Validator>,
+        tombstones: Vec<Validator>,
     ) -> Result<Response, Self::Error> {
         let virtual_stake = self.virtual_stake.load(ctx.deps.storage)?;
         ensure_eq!(
@@ -345,7 +348,8 @@ impl ConverterApi for ConverterContract<'_> {
 
         // Send over IBC to the Consumer
         let channel = IBC_CHANNEL.load(ctx.deps.storage)?;
-        let msg = add_validators_msg(&ctx.env, channel, &additions)?;
+        let add_msg = add_validators_msg(&ctx.env, &channel, &additions)?;
+        let tomb_msg = tombstone_validators_msg(&ctx.env, &channel, &tombstones)?;
 
         let event = Event::new("valset_update").add_attribute(
             "additions",
@@ -355,7 +359,18 @@ impl ConverterApi for ConverterContract<'_> {
                 .collect::<Vec<String>>()
                 .join(","),
         );
-        let resp = Response::new().add_event(event).add_message(msg);
+        let event = event.add_attribute(
+            "tombstones",
+            tombstones
+                .iter()
+                .map(|v| v.address.clone())
+                .collect::<Vec<String>>()
+                .join(","),
+        );
+        let resp = Response::new()
+            .add_event(event)
+            .add_message(add_msg)
+            .add_message(tomb_msg);
 
         Ok(resp)
     }

contracts/consumer/converter/src/ibc.rs

@@ -114,14 +114,14 @@ pub fn ibc_channel_connect(
 
     // Send a validator sync packet to arrive with the newly established channel
     let validators = deps.querier.query_all_validators()?;
-    let msg = add_validators_msg(&env, channel, &validators)?;
+    let msg = add_validators_msg(&env, &channel, &validators)?;
 
     Ok(IbcBasicResponse::new().add_message(msg))
 }
 
 pub(crate) fn add_validators_msg(
     env: &Env,
-    channel: IbcChannel,
+    channel: &IbcChannel,
     validators: &[Validator],
 ) -> Result<IbcMsg, ContractError> {
     let updates = validators
@@ -138,7 +138,22 @@ pub(crate) fn add_validators_msg(
         .collect();
     let packet = ConsumerPacket::AddValidators(updates);
     let msg = IbcMsg::SendPacket {
-        channel_id: channel.endpoint.channel_id,
+        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,
+    validators: &[Validator],
+) -> Result<IbcMsg, ContractError> {
+    let updates = validators.iter().map(|v| v.address.clone()).collect();
+    let packet = ConsumerPacket::RemoveValidators(updates);
+    let msg = IbcMsg::SendPacket {
+        channel_id: channel.endpoint.channel_id.clone(),
         data: to_binary(&packet)?,
         timeout: packet_timeout_validator(env),
     };

contracts/consumer/converter/src/multitest.rs

@@ -223,7 +223,7 @@ fn valset_update_works() {
     );
 
     // Send a valset update
-    let new_validators = vec![
+    let add_validators = vec![
         Validator {
             address: "validator1".to_string(),
             commission: Default::default(),
@@ -237,17 +237,23 @@ fn valset_update_works() {
             max_change_rate: Default::default(),
         },
     ];
+    let rem_validators = vec![Validator {
+        address: "validator3".to_string(),
+        commission: Default::default(),
+        max_commission: Default::default(),
+        max_change_rate: Default::default(),
+    }];
 
     // Check that only the virtual staking contract can call this handler
     let res = converter
         .converter_api_proxy()
-        .valset_update(vec![])
+        .valset_update(vec![], vec![])
         .call(owner);
     assert_eq!(res.unwrap_err(), Unauthorized {});
 
     let res = converter
         .converter_api_proxy()
-        .valset_update(new_validators)
+        .valset_update(add_validators, rem_validators)
         .call(virtual_staking.contract_addr.as_ref());
 
     // This fails because of lack of IBC support in mt now.

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

@@ -144,15 +144,17 @@ impl VirtualStakingContract<'_> {
         deps: DepsMut<VirtualStakeCustomQuery>,
         additions: &[Validator],
         removals: &[Validator],
+        tombstones: &[Validator],
     ) -> Result<Response<VirtualStakeCustomMsg>, ContractError> {
         // TODO: Store/process removals (and additions) locally, so that they are filtered out from
         // the `bonded` list
         let _ = removals;
 
-        // Send additions to the converter. Removals are considered non-permanent and ignored
+        // Send additions and tombstones to the Converter. Removals are non-permanent and ignored
         let cfg = self.config.load(deps.storage)?;
         let msg = converter_api::ExecMsg::ValsetUpdate {
             additions: additions.to_vec(),
+            tombstones: tombstones.to_vec(),
         };
         let msg = WasmMsg::Execute {
             contract_addr: cfg.converter.to_string(),
@@ -357,6 +359,12 @@ pub fn sudo(
         SudoMsg::ValsetUpdate {
             additions,
             removals,
-        } => VirtualStakingContract::new().handle_valset_update(deps, &additions, &removals),
+            tombstones,
+        } => VirtualStakingContract::new().handle_valset_update(
+            deps,
+            &additions,
+            &removals,
+            &tombstones,
+        ),
     }
 }

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

@@ -153,9 +153,16 @@ fn valset_update_sudo() {
         max_commission: Decimal::percent(20),
         max_change_rate: Default::default(),
     }];
+    let tombs = vec![Validator {
+        address: "cosmosval3".to_string(),
+        commission: Decimal::percent(3),
+        max_commission: Decimal::percent(30),
+        max_change_rate: Default::default(),
+    }];
     let msg = SudoMsg::ValsetUpdate {
         additions: adds,
         removals: rems,
+        tombstones: tombs,
     };
 
     let res = app

packages/apis/src/converter_api.rs

@@ -27,8 +27,11 @@ pub trait ConverterApi {
         payments: Vec<RewardInfo>,
     ) -> Result<Response, Self::Error>;
 
-    /// Valset updates. Only additions are accepted, as removals (leaving the active validator set)
-    /// are non-permanent and ignored (CRDTs only support permanent removals).
+    /// Valset updates.
+    ///
+    /// Only additions and permanent removals are accepted, as removals (leaving the active
+    /// validator set) are non-permanent and ignored on the Provider (CRDTs only support permanent
+    /// removals).
     ///
     /// If a validator that already exists in the list is re-sent for addition, its pubkey
     /// will be updated.
@@ -38,6 +41,7 @@ pub trait ConverterApi {
         &self,
         ctx: ExecCtx,
         additions: Vec<Validator>,
+        tombstones: Vec<Validator>,
     ) -> Result<Response, Self::Error>;
 }
 

packages/apis/src/virtual_staking_api.rs

@@ -18,7 +18,7 @@ pub trait VirtualStakingApi {
 
     /// Requests to unbond tokens from a validator. This will be actually handled at the next epoch.
     /// If the virtual staking module is over the max cap, it will trigger a rebalance in addition to unbond.
-    /// If the virtual staking contract doesn't have at least amont tokens staked to the given validator, this will return an error.
+    /// If the virtual staking contract doesn't have at least amount tokens staked to the given validator, this will return an error.
     #[msg(exec)]
     fn unbond(
         &self,
@@ -37,10 +37,12 @@ pub enum SudoMsg {
     /// It should also withdraw all pending rewards here, and send them to the converter contract.
     Rebalance {},
     /// SudoMsg::ValsetUpdate{} should be called every time there's a validator set update: addition
-    /// of a new validator to the active validator set, or removal of a validator from the
-    /// active validator set.
+    /// of a new validator to the active validator set, removal of a validator from the
+    /// active validator set, or permanent removal (i.e. tombstoning) of a validator from the active
+    /// validator set.
     ValsetUpdate {
         additions: Vec<Validator>,
         removals: Vec<Validator>,
+        tombstones: Vec<Validator>,
     },
 }