diff --git a/Cargo.lock b/Cargo.lock index ec4e336c..8562a2cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1726,6 +1726,7 @@ dependencies = [ "cw2", "derivative", "schemars", + "semver", "serde", "tg-bindings", "tg-bindings-test", diff --git a/contracts/tgrade-valset/Cargo.toml b/contracts/tgrade-valset/Cargo.toml index d0a4c5ba..5da0d731 100644 --- a/contracts/tgrade-valset/Cargo.toml +++ b/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" } diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index c0583b62..8afacb01 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/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( &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( 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( 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, + mut deps: DepsMut, _env: Env, msg: MigrateMsg, ) -> Result { @@ -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( 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() diff --git a/contracts/tgrade-valset/src/lib.rs b/contracts/tgrade-valset/src/lib.rs index c6b4d747..f56dc8ff 100644 --- a/contracts/tgrade-valset/src/lib.rs +++ b/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; diff --git a/contracts/tgrade-valset/src/migration.rs b/contracts/tgrade-valset/src/migration.rs new file mode 100644 index 00000000..faa69414 --- /dev/null +++ b/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( + deps: DepsMut, + version: &Version, +) -> Result<(), ContractError> { + let jailings: Vec<_> = if *version <= "0.6.2".parse::().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::>()? + } 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::, _>>() + .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 {} + } + ) + ] + ); + } +} diff --git a/contracts/tgrade-valset/src/msg.rs b/contracts/tgrade-valset/src/msg.rs index 6543c2bc..7cbe34df 100644 --- a/contracts/tgrade-valset/src/msg.rs +++ b/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), } } } diff --git a/contracts/tgrade-valset/src/multitest/double_sign.rs b/contracts/tgrade-valset/src/multitest/double_sign.rs index e8c6205b..c2a81ea5 100644 --- a/contracts/tgrade-valset/src/multitest/double_sign.rs +++ b/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), ], ); diff --git a/contracts/tgrade-valset/src/multitest/helpers.rs b/contracts/tgrade-valset/src/multitest/helpers.rs index e5fb6ef4..c859fa98 100644 --- a/contracts/tgrade-valset/src/multitest/helpers.rs +++ b/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)]) { +pub fn assert_operators(received: &[OperatorResponse], expected: &[(&str, Option)]) { 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 diff --git a/contracts/tgrade-valset/src/multitest/jailing.rs b/contracts/tgrade-valset/src/multitest/jailing.rs index 14d8dc87..4aa22420 100644 --- a/contracts/tgrade-valset/src/multitest/jailing.rs +++ b/contracts/tgrade-valset/src/multitest/jailing.rs @@ -1,8 +1,9 @@ use crate::error::ContractError; -use crate::msg::JailingPeriod; +use crate::msg::{JailingEnd, ValidatorResponse}; use super::helpers::{assert_active_validators, assert_operators, members_init}; use super::suite::SuiteBuilder; +use cosmwasm_std::{StdResult, Timestamp}; use cw_controllers::AdminError; use tg_utils::{Duration, Expiration, JailingDuration}; @@ -37,7 +38,7 @@ fn only_admin_can_jail() { )) ); - let jailed_until = JailingPeriod::Until(Duration::new(3600).after(&suite.app().block_info())); + let jailed_until = JailingEnd::Until(Duration::new(3600).after(&suite.app().block_info())); // Non-admin cannot jail forever let err = suite @@ -67,7 +68,7 @@ fn only_admin_can_jail() { &suite.list_validators(None, None).unwrap(), &[ (members[0], None), - (members[1], Some(JailingPeriod::Forever {})), + (members[1], Some(JailingEnd::Forever {})), (members[2], Some(jailed_until)), (members[3], None), ], @@ -109,7 +110,7 @@ fn admin_can_unjail_almost_anyone() { &suite.list_validators(None, None).unwrap(), &[ (members[0], None), - (members[1], Some(JailingPeriod::Forever {})), + (members[1], Some(JailingEnd::Forever {})), (members[2], None), (members[3], None), ], @@ -130,7 +131,7 @@ fn anyone_can_unjail_self_after_period() { suite.jail(&admin, members[1], Duration::new(3600)).unwrap(); suite.jail(&admin, members[2], Duration::new(3600)).unwrap(); - let jailed_until = JailingPeriod::Until(Duration::new(3600).after(&suite.app().block_info())); + let jailed_until = JailingEnd::Until(Duration::new(3600).after(&suite.app().block_info())); // Move a little bit forward, so some time passed, but not eough for any jailing to // expire @@ -240,7 +241,7 @@ fn auto_unjail() { let admin = suite.admin().to_owned(); - let jailed_until = JailingPeriod::Until(Duration::new(3600).after(&suite.app().block_info())); + let jailed_until = JailingEnd::Until(Duration::new(3600).after(&suite.app().block_info())); // Jailing some operators to begin with suite.jail(&admin, members[0], Duration::new(3600)).unwrap(); @@ -256,7 +257,7 @@ fn auto_unjail() { &suite.list_validators(None, None).unwrap(), &[ (members[0], Some(jailed_until)), - (members[1], Some(JailingPeriod::Forever {})), + (members[1], Some(JailingEnd::Forever {})), (members[2], None), (members[3], None), ], @@ -276,7 +277,7 @@ fn auto_unjail() { &suite.list_validators(None, None).unwrap(), &[ (members[0], None), - (members[1], Some(JailingPeriod::Forever {})), + (members[1], Some(JailingEnd::Forever {})), (members[2], None), (members[3], None), ], @@ -410,3 +411,28 @@ fn list_jailed_validators_with_pagination() { assert_eq!(operators[0].operator, members[3]); assert_eq!(operators[1].operator, members[4]); } + +#[test] +fn jailing_duration_start_is_provided() { + let members = vec!["member1", "member2"]; + let mut suite = SuiteBuilder::new() + .with_engagement(&members_init(&members, &[2, 3, 5, 8, 10])) + .with_operators(&members) + .build(); + let admin = suite.admin().to_owned(); + + fn jail_start(val: StdResult) -> Timestamp { + val.unwrap().validator.unwrap().jailed_until.unwrap().start + } + + let time1 = suite.app().block_info().time; + suite.jail(&admin, members[0], Duration::new(3600)).unwrap(); + assert_eq!(time1, jail_start(suite.validator(members[0]))); + + suite.advance_seconds(100).unwrap(); + let time2 = suite.app().block_info().time; + suite.jail(&admin, members[1], Duration::new(3600)).unwrap(); + assert_eq!(time2, time1.plus_seconds(100)); + assert_eq!(time1, jail_start(suite.validator(members[0]))); + assert_eq!(time2, jail_start(suite.validator(members[1]))); +}