Merge pull request #112 from confio/94-jailing-period-start

valset: store and expose the start of a jailing period

contracts/tgrade-valset/Cargo.toml

@@ -33,6 +33,7 @@ cw-utils = { version = "0.12.1" }
 cw-controllers = { version = "0.12.1" }
 cw-storage-plus = { version = "0.12.1" }
 schemars = "0.8"
+semver = "1"
 serde = { version = "1.0.103", default-features = false, features = ["derive"] }
 thiserror = { version = "1.0.21" }
 tg4 = { path = "../../packages/tg4", version = "0.6.2" }

contracts/tgrade-valset/src/contract.rs

@@ -9,11 +9,12 @@ use cosmwasm_std::{
     Order, Reply, StdError, StdResult, Timestamp, WasmMsg,
 };
 
-use cw2::set_contract_version;
+use cw2::{get_contract_version, set_contract_version};
 use cw_controllers::AdminError;
 use cw_storage_plus::Bound;
 use cw_utils::{maybe_addr, parse_reply_instantiate_data};
 
+use semver::Version;
 use tg4::{Member, Tg4Contract};
 use tg_bindings::{
     request_privileges, Ed25519Pubkey, Evidence, EvidenceType, Privilege, PrivilegeChangeMsg,
@@ -22,8 +23,9 @@ use tg_bindings::{
 use tg_utils::{ensure_from_older_version, JailingDuration, SlashMsg, ADMIN};
 
 use crate::error::ContractError;
+use crate::migration::migrate_jailing_period;
 use crate::msg::{
-    EpochResponse, ExecuteMsg, InstantiateMsg, InstantiateResponse, JailingPeriod,
+    EpochResponse, ExecuteMsg, InstantiateMsg, InstantiateResponse, JailingEnd, JailingPeriod,
     ListActiveValidatorsResponse, ListValidatorResponse, ListValidatorSlashingResponse, MigrateMsg,
     OperatorResponse, QueryMsg, RewardsDistribution, RewardsInstantiateMsg, ValidatorMetadata,
     ValidatorResponse,
@@ -274,9 +276,9 @@ fn execute_jail<Q: CustomQuery>(
         &expiration,
     )?;
 
-    let until_attr = match expiration {
-        JailingPeriod::Until(expires) => Timestamp::from(expires).to_string(),
-        JailingPeriod::Forever {} => "forever".to_owned(),
+    let until_attr = match expiration.end {
+        JailingEnd::Until(expires) => Timestamp::from(expires).to_string(),
+        JailingEnd::Forever {} => "forever".to_owned(),
     };
 
     let res = Response::new()
@@ -303,19 +305,17 @@ fn execute_unjail<Q: CustomQuery>(
         return Err(AdminError::NotAdmin {}.into());
     }
 
-    match JAIL.may_load(deps.storage, operator) {
-        Err(err) => return Err(err.into()),
-        // Operator is not jailed, unjailing does nothing and succeeds
-        Ok(None) => (),
-        Ok(Some(JailingPeriod::Forever {})) => {
+    // if this is `None`, the validator was not unjailed and unjailing succeeds
+    if let Some(expiration) = JAIL.may_load(deps.storage, operator)? {
+        if expiration.is_forever() {
             return Err(ContractError::UnjailFromJailForeverForbidden {});
         }
-        // Jailing period expired or called by admin - can unjail
-        Ok(Some(expiration)) if (expiration.is_expired(&env.block) || is_admin) => {
+
+        if expiration.is_expired(&env.block) || is_admin {
             JAIL.remove(deps.storage, operator);
+        } else {
+            return Err(ContractError::JailDidNotExpire {});
         }
-        // Jail not expired
-        _ => return Err(ContractError::JailDidNotExpire {}),
     }
 
     let res = Response::new()
@@ -593,9 +593,9 @@ fn list_validator_slashing<Q: CustomQuery>(
     let slashing = VALIDATOR_SLASHING
         .may_load(deps.storage, &addr)?
         .unwrap_or_default();
-    let (jailed_until, tombstoned) = match JAIL.may_load(deps.storage, &addr)? {
-        Some(JailingPeriod::Forever {}) => (None, true),
-        Some(JailingPeriod::Until(u)) => (Some(u), false),
+    let (jailed_until, tombstoned) = match JAIL.may_load(deps.storage, &addr)?.map(|j| j.end) {
+        Some(JailingEnd::Forever {}) => (None, true),
+        Some(JailingEnd::Until(u)) => (Some(u), false),
         None => (None, false),
     };
     Ok(ListValidatorSlashingResponse {
@@ -878,7 +878,7 @@ fn calculate_diff(
 
 #[cfg_attr(not(feature = "library"), entry_point)]
 pub fn migrate(
-    deps: DepsMut<TgradeQuery>,
+    mut deps: DepsMut<TgradeQuery>,
     _env: Env,
     msg: MigrateMsg,
 ) -> Result<Response, ContractError> {
@@ -894,6 +894,12 @@ pub fn migrate(
         Ok(cfg)
     })?;
 
+    let stored_version = get_contract_version(deps.storage)?;
+    // Unwrapping as version check before would fail if stored version is invalid
+    let stored_version: Version = stored_version.version.parse().unwrap();
+
+    migrate_jailing_period(deps.branch(), &stored_version)?;
+
     Ok(Response::new())
 }
 
@@ -983,7 +989,11 @@ fn begin_block<Q: CustomQuery>(
                     config.double_sign_slash_ratio,
                 )?;
 
-                JAIL.save(deps.storage, &validator, &JailingPeriod::Forever {})?;
+                JAIL.save(
+                    deps.storage,
+                    &validator,
+                    &JailingPeriod::from_duration(JailingDuration::Forever {}, &env.block),
+                )?;
 
                 response = response
                     .clone()

contracts/tgrade-valset/src/lib.rs

@@ -1,5 +1,6 @@
 pub mod contract;
 pub mod error;
+mod migration;
 pub mod msg;
 #[cfg(test)]
 mod multitest;

contracts/tgrade-valset/src/migration.rs

@@ -0,0 +1,124 @@
+use cosmwasm_std::{Addr, CustomQuery, DepsMut, Order, Timestamp};
+use cw_storage_plus::Map;
+use schemars::JsonSchema;
+use semver::Version;
+use serde::{Deserialize, Serialize};
+use tg_utils::Expiration;
+
+use crate::error::ContractError;
+use crate::msg::{JailingEnd, JailingPeriod};
+use crate::state::JAIL;
+
+/// `crate::msg::JailingPeriod` version from v0.6.2 and before
+#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
+pub enum JailingPeriodV0_6_2 {
+    Until(Expiration),
+    Forever {},
+}
+
+impl JailingPeriodV0_6_2 {
+    fn update(self) -> JailingPeriod {
+        JailingPeriod {
+            start: Timestamp::from_seconds(0),
+            end: match self {
+                JailingPeriodV0_6_2::Until(u) => JailingEnd::Until(u),
+                JailingPeriodV0_6_2::Forever {} => JailingEnd::Forever {},
+            },
+        }
+    }
+}
+
+pub fn migrate_jailing_period<Q: CustomQuery>(
+    deps: DepsMut<Q>,
+    version: &Version,
+) -> Result<(), ContractError> {
+    let jailings: Vec<_> = if *version <= "0.6.2".parse::<Version>().unwrap() {
+        let jailings: Map<&Addr, JailingPeriodV0_6_2> = Map::new("jail");
+
+        jailings
+            .range(deps.storage, None, None, Order::Ascending)
+            .map(|record| record.map(|(key, jailing_period)| (key, jailing_period.update())))
+            .collect::<Result<_, _>>()?
+    } else {
+        return Ok(());
+    };
+
+    for (addr, jailing_period) in jailings {
+        JAIL.save(deps.storage, &addr, &jailing_period)?;
+    }
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    //! These are very rudimentary tests that only -mock- old state and perform migrations on it.
+    //! It's absolutely vital to do more thorough migration testing on some actual old state.
+
+    use cosmwasm_std::{testing::mock_dependencies, StdError, Storage};
+
+    use super::*;
+
+    fn mock_v_0_6_2_jailing_periods(
+        store: &mut dyn Storage,
+        jailings: &[(&str, JailingPeriodV0_6_2)],
+    ) {
+        let jail_map: Map<&Addr, JailingPeriodV0_6_2> = Map::new("jail");
+
+        for (addr, period) in jailings.iter().cloned() {
+            jail_map
+                .update(store, &Addr::unchecked(addr), |_| -> Result<_, StdError> {
+                    Ok(period)
+                })
+                .unwrap();
+        }
+    }
+
+    #[test]
+    fn migrate_jailing_period_v_0_6_2() {
+        let mut deps = mock_dependencies();
+
+        mock_v_0_6_2_jailing_periods(
+            &mut deps.storage,
+            &[
+                (
+                    "alice",
+                    JailingPeriodV0_6_2::Until(Expiration::at_timestamp(Timestamp::from_seconds(
+                        123,
+                    ))),
+                ),
+                ("bob", JailingPeriodV0_6_2::Forever {}),
+            ],
+        );
+
+        migrate_jailing_period(deps.as_mut(), &Version::parse("0.6.2").unwrap()).unwrap();
+
+        // verify the data is what we expect
+        let jailed = JAIL
+            .range(&deps.storage, None, None, Order::Ascending)
+            .collect::<Result<Vec<_>, _>>()
+            .unwrap();
+
+        assert_eq!(
+            jailed,
+            [
+                (
+                    Addr::unchecked("alice"),
+                    JailingPeriod {
+                        start: Timestamp::from_seconds(0),
+                        end: JailingEnd::Until(Expiration::at_timestamp(Timestamp::from_seconds(
+                            123
+                        )))
+                    }
+                ),
+                (
+                    Addr::unchecked("bob"),
+                    JailingPeriod {
+                        start: Timestamp::from_seconds(0),
+                        end: JailingEnd::Forever {}
+                    }
+                )
+            ]
+        );
+    }
+}

contracts/tgrade-valset/src/msg.rs

@@ -9,7 +9,7 @@ use tg_utils::{Expiration, JailingDuration};
 
 use crate::error::ContractError;
 use crate::state::{DistributionContract, OperatorInfo, ValidatorInfo, ValidatorSlashing};
-use cosmwasm_std::{Addr, Api, BlockInfo, Coin, Decimal};
+use cosmwasm_std::{Addr, Api, BlockInfo, Coin, Decimal, Timestamp};
 
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
 pub struct InstantiateMsg {
@@ -395,25 +395,38 @@ impl OperatorResponse {
     }
 }
 
+#[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
+pub struct JailingPeriod {
+    pub start: Timestamp,
+    pub end: JailingEnd,
+}
+
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
 #[serde(rename_all = "snake_case")]
-pub enum JailingPeriod {
+pub enum JailingEnd {
     Until(Expiration),
     Forever {},
 }
 
 impl JailingPeriod {
     pub fn from_duration(duration: JailingDuration, block: &BlockInfo) -> Self {
-        match duration {
-            JailingDuration::Duration(duration) => Self::Until(duration.after(block)),
-            JailingDuration::Forever {} => Self::Forever {},
+        Self {
+            start: block.time,
+            end: match duration {
+                JailingDuration::Duration(duration) => JailingEnd::Until(duration.after(block)),
+                JailingDuration::Forever {} => JailingEnd::Forever {},
+            },
         }
     }
 
+    pub fn is_forever(&self) -> bool {
+        matches!(self.end, JailingEnd::Forever {})
+    }
+
     pub fn is_expired(&self, block: &BlockInfo) -> bool {
-        match self {
-            Self::Forever {} => false,
-            Self::Until(expires) => expires.is_expired(block),
+        match self.end {
+            JailingEnd::Forever {} => false,
+            JailingEnd::Until(expires) => expires.is_expired(block),
         }
     }
 }

contracts/tgrade-valset/src/multitest/double_sign.rs

@@ -4,7 +4,7 @@ use tg_bindings::{Ed25519Pubkey, Evidence, EvidenceType, ToAddress, Validator};
 
 use super::helpers::{addr_to_pubkey, assert_operators};
 use super::suite::SuiteBuilder;
-use crate::msg::{JailingPeriod, ValidatorMetadata};
+use crate::msg::{JailingEnd, ValidatorMetadata};
 use crate::multitest::helpers::members_init;
 use crate::test_helpers::mock_pubkey;
 
@@ -56,7 +56,7 @@ fn evidence_slash_and_jail() {
     assert_operators(
         &suite.list_validators(None, None).unwrap(),
         &[
-            (members[0].0, Some(JailingPeriod::Forever {})),
+            (members[0].0, Some(JailingEnd::Forever {})),
             (members[1].0, None),
         ],
     );
@@ -100,7 +100,7 @@ fn evidence_doesnt_affect_engagement_rewards() {
     assert_operators(
         &suite.list_validators(None, None).unwrap(),
         &[
-            (members[0].0, Some(JailingPeriod::Forever {})),
+            (members[0].0, Some(JailingEnd::Forever {})),
             (members[1].0, None),
         ],
     );
@@ -176,9 +176,9 @@ fn multiple_evidences() {
     assert_operators(
         &suite.list_validators(None, None).unwrap(),
         &[
-            (members[0].0, Some(JailingPeriod::Forever {})),
+            (members[0].0, Some(JailingEnd::Forever {})),
             (members[1].0, None),
-            (members[2].0, Some(JailingPeriod::Forever {})),
+            (members[2].0, Some(JailingEnd::Forever {})),
         ],
     );
 }
@@ -228,7 +228,7 @@ fn evidence_with_not_matching_date() {
         &suite.list_validators(None, None).unwrap(),
         &[
             (members[0].0, None),
-            (members[1].0, Some(JailingPeriod::Forever {})),
+            (members[1].0, Some(JailingEnd::Forever {})),
             (members[2].0, None),
         ],
     );

contracts/tgrade-valset/src/multitest/helpers.rs

@@ -1,7 +1,7 @@
 use cosmwasm_std::Binary;
 use tg_bindings::Pubkey;
 
-use crate::msg::{JailingPeriod, OperatorResponse};
+use crate::msg::{JailingEnd, OperatorResponse};
 use crate::state::ValidatorInfo;
 
 // Converts address to valid public key
@@ -42,11 +42,11 @@ pub fn assert_active_validators(received: &[ValidatorInfo], expected: &[(&str, u
 /// completely ignored, therefore as expected value vector of `(addr, jailed_until)` are taken.
 /// Also order of operators should not matter, so proper sorting is also handled.
 #[track_caller]
-pub fn assert_operators(received: &[OperatorResponse], expected: &[(&str, Option<JailingPeriod>)]) {
+pub fn assert_operators(received: &[OperatorResponse], expected: &[(&str, Option<JailingEnd>)]) {
     let mut received: Vec<_> = received
         .iter()
         .cloned()
-        .map(|operator| (operator.operator, operator.jailed_until))
+        .map(|operator| (operator.operator, operator.jailed_until.map(|j| j.end)))
         .collect();
 
     let mut expected: Vec<_> = expected