Merge pull request #74 from osmosis-labs/ensure-known-validators

Ensure validators are registered before staking

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

@@ -6,10 +6,12 @@ use cw2::set_contract_version;
 use cw_storage_plus::{Bounder, Item, Map};
 use cw_utils::must_pay;
 use mesh_apis::cross_staking_api::{self, CrossStakingApi};
+use mesh_apis::ibc::AddValidator;
 use mesh_apis::local_staking_api::MaxSlashResponse;
 use mesh_apis::vault_api::VaultApiHelper;
 use mesh_sync::Lockable;
 
+use crate::crdt::CrdtState;
 // IBC sending is disabled in tests...
 #[cfg(not(test))]
 use crate::ibc::{packet_timeout, IBC_CHANNEL};
@@ -22,7 +24,6 @@ use sylvia::contract;
 use sylvia::types::{ExecCtx, InstantiateCtx, QueryCtx};
 
 use crate::error::ContractError;
-use crate::ibc::VAL_CRDT;
 use crate::msg::{
     AllTxsResponse, AuthorizedEndpointResponse, ConfigResponse, IbcChannelResponse,
     ListRemoteValidatorsResponse, PendingRewards, ReceiveVirtualStake, StakeInfo, StakesResponse,
@@ -53,6 +54,8 @@ pub struct ExternalStakingContract<'a> {
     /// Pending txs information
     pub tx_count: Item<'a, u64>,
     pub pending_txs: Map<'a, u64, Tx>,
+    /// Valset CRDT
+    pub val_set: CrdtState<'a>,
 }
 
 #[cfg_attr(not(feature = "library"), sylvia::entry_points)]
@@ -67,6 +70,7 @@ impl ExternalStakingContract<'_> {
             distribution: Map::new("distribution"),
             pending_txs: Map::new("pending_txs"),
             tx_count: Item::new("tx_count"),
+            val_set: CrdtState::new(),
         }
     }
 
@@ -242,6 +246,38 @@ impl ExternalStakingContract<'_> {
         }
     }
 
+    /// Rollbacks a pending stake.
+    /// Method used for tests only.
+    #[msg(exec)]
+    fn test_set_active_validator(
+        &self,
+        ctx: ExecCtx,
+        validator: AddValidator,
+    ) -> Result<Response, ContractError> {
+        #[cfg(test)]
+        {
+            let AddValidator {
+                valoper,
+                pub_key,
+                start_height,
+                start_time,
+            } = validator;
+            let update = crate::crdt::ValUpdate {
+                pub_key,
+                start_height,
+                start_time,
+            };
+            self.val_set
+                .add_validator(ctx.deps.storage, &valoper, update)?;
+            Ok(Response::new())
+        }
+        #[cfg(not(test))]
+        {
+            let _ = (ctx, validator);
+            Err(ContractError::Unauthorized {})
+        }
+    }
+
     /// Schedules tokens for release, adding them to the pending unbonds. After `unbonding_period`
     /// passes, funds are ready to be released with `withdraw_unbonded` call by the user
     #[msg(exec)]
@@ -629,7 +665,8 @@ impl ExternalStakingContract<'_> {
     ) -> Result<ListRemoteValidatorsResponse, ContractError> {
         let limit = limit.unwrap_or(100) as usize;
         let validators =
-            VAL_CRDT.list_active_validators(ctx.deps.storage, start_after.as_deref(), limit)?;
+            self.val_set
+                .list_active_validators(ctx.deps.storage, start_after.as_deref(), limit)?;
         Ok(ListRemoteValidatorsResponse { validators })
     }
 
@@ -796,6 +833,7 @@ pub mod cross_staking {
             let config = self.config.load(ctx.deps.storage)?;
             ensure_eq!(ctx.info.sender, config.vault.0, ContractError::Unauthorized);
 
+            // sending proper denom
             ensure_eq!(
                 amount.denom,
                 config.denom,
@@ -804,7 +842,14 @@ pub mod cross_staking {
 
             let owner = ctx.deps.api.addr_validate(&owner)?;
 
+            // parse and validate message
             let msg: ReceiveVirtualStake = from_binary(&msg)?;
+            if !self
+                .val_set
+                .is_active_validator(ctx.deps.storage, &msg.validator)?
+            {
+                return Err(ContractError::ValidatorNotActive(msg.validator));
+            }
             let mut stake_lock = self
                 .stakes
                 .may_load(ctx.deps.storage, (&owner, &msg.validator))?

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

@@ -33,9 +33,12 @@ pub enum ContractError {
     #[error("Not enough tokens released, up to {0} can be claimed")]
     NotEnoughRelease(Uint128),
 
-    #[error("Validator for user missmatch, {0} expected")]
+    #[error("Validator for user mismatch, {0} expected")]
     InvalidValidator(String),
 
+    #[error("Cannot stake to {0}, not listed as an active validator on consumer")]
+    ValidatorNotActive(String),
+
     #[error("Contract already has an open IBC channel")]
     IbcChannelAlreadyOpen,
 

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

@@ -12,12 +12,10 @@ use mesh_apis::ibc::{
     ConsumerPacket, ProtocolVersion, ProviderPacket, RemoveValidatorsAck,
 };
 
-use crate::{
-    contract::ExternalStakingContract,
-    crdt::{CrdtState, ValUpdate},
-    error::ContractError,
-    msg::AuthorizedEndpoint,
-};
+use crate::contract::ExternalStakingContract;
+use crate::crdt::ValUpdate;
+use crate::error::ContractError;
+use crate::msg::AuthorizedEndpoint;
 
 /// This is the maximum version of the Mesh Security protocol that we support
 const SUPPORTED_IBC_PROTOCOL_VERSION: &str = "1.0.0";
@@ -26,12 +24,8 @@ const MIN_IBC_PROTOCOL_VERSION: &str = "1.0.0";
 
 // IBC specific state
 pub const AUTH_ENDPOINT: Item<AuthorizedEndpoint> = Item::new("auth_endpoint");
-
-// TODO: expected endpoint
 pub const IBC_CHANNEL: Item<IbcChannel> = Item::new("ibc_channel");
 
-pub const VAL_CRDT: CrdtState = CrdtState::new();
-
 // If we don't hear anything within 10 minutes, let's abort, for better UX
 // This is long enough to allow some clock drift between chains
 const DEFAULT_TIMEOUT: u64 = 10 * 60;
@@ -127,6 +121,7 @@ pub fn ibc_packet_receive(
 ) -> Result<IbcReceiveResponse, ContractError> {
     // There is only one channel, so we don't need to switch.
     // We also don't care about packet sequence as this is fully commutative.
+    let contract = ExternalStakingContract::new();
     let packet: ConsumerPacket = from_slice(&msg.packet.data)?;
     let ack = match packet {
         ConsumerPacket::AddValidators(to_add) => {
@@ -142,13 +137,15 @@ pub fn ibc_packet_receive(
                     start_height,
                     start_time,
                 };
-                VAL_CRDT.add_validator(deps.storage, &valoper, update)?;
+                contract
+                    .val_set
+                    .add_validator(deps.storage, &valoper, update)?;
             }
             ack_success(&AddValidatorsAck {})?
         }
         ConsumerPacket::RemoveValidators(to_remove) => {
             for valoper in to_remove {
-                VAL_CRDT.remove_validator(deps.storage, &valoper)?;
+                contract.val_set.remove_validator(deps.storage, &valoper)?;
             }
             ack_success(&RemoveValidatorsAck {})?
         }

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

@@ -1,6 +1,7 @@
 use anyhow::Result as AnyResult;
 
 use cosmwasm_std::{coin, coins, to_binary, Addr, Decimal};
+use mesh_apis::ibc::AddValidator;
 use mesh_native_staking::contract::multitest_utils::CodeId as NativeStakingCodeId;
 use mesh_native_staking::contract::InstantiateMsg as NativeStakingInstantiateMsg;
 use mesh_native_staking_proxy::contract::multitest_utils::CodeId as NativeStakingProxyCodeId;
@@ -102,6 +103,15 @@ fn staking() {
 
     let (vault, contract) = setup(&app, owner, 100).unwrap();
 
+    // set these validators to be active
+    for val in validators {
+        let activate = AddValidator::mock(val);
+        contract
+            .test_set_active_validator(activate)
+            .call("test")
+            .unwrap();
+    }
+
     // Bond tokens
     vault
         .bond()
@@ -115,6 +125,23 @@ fn staking() {
         .call(users[1])
         .unwrap();
 
+    /*
+    // Fail to stake on non-registered validator
+    let msg = to_binary(&ReceiveVirtualStake {
+        validator: "unknown".to_string(),
+    })
+    .unwrap();
+    println!("START");
+    // FIXME: Sylvia panics here, with this line in ExecProxy::call
+    //             .map_err(|err| err.downcast().unwrap())
+    // Note that the error didn't happen in vault, but in a SubMsg, so this should be some StdError not ContractError...
+    let res = vault
+        .stake_remote(contract.contract_addr.to_string(), coin(100, OSMO), msg)
+        .call(users[0]);
+    println!("GOT: {:?}", res);
+    assert!(res.is_err());
+    */
+
     // Perform couple stakes
     // users[0] stakes 200 on validators[0] in 2 batches
     // users[0] stakes 100 on validators[1]
@@ -311,6 +338,15 @@ fn unstaking() {
 
     let (vault, contract) = setup(&app, owner, 100).unwrap();
 
+    // set these validators to be active
+    for val in validators {
+        let activate = AddValidator::mock(val);
+        contract
+            .test_set_active_validator(activate)
+            .call("test")
+            .unwrap();
+    }
+
     // Bond and stake tokens
     //
     // users[0] stakes 200 on validators[0]
@@ -626,6 +662,15 @@ fn distribution() {
 
     let (vault, contract) = setup(&app, owner, 100).unwrap();
 
+    // set these validators to be active
+    for val in validators {
+        let activate = AddValidator::mock(val);
+        contract
+            .test_set_active_validator(activate)
+            .call("test")
+            .unwrap();
+    }
+
     // Bond and stake tokens
     //
     // users[0] stakes 200 on validators[0]

packages/apis/src/ibc/packet.rs

@@ -78,6 +78,17 @@ pub struct AddValidator {
     pub start_time: u64,
 }
 
+impl AddValidator {
+    pub fn mock(valoper: &str) -> Self {
+        Self {
+            valoper: valoper.to_string(),
+            pub_key: "mock-pubkey".to_string(),
+            start_height: 12345,
+            start_time: 1687357499,
+        }
+    }
+}
+
 /// Ack sent for ConsumerPacket::AddValidators
 #[cw_serde]
 pub struct AddValidatorsAck {}