From 1e517deac28db93b0757e81ada43c692a211ee41 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Mon, 19 Sep 2022 12:39:48 +0200 Subject: [PATCH 1/4] Add support for updating the distribution contracts list during migration --- contracts/tgrade-valset/src/contract.rs | 3 +++ contracts/tgrade-valset/src/msg.rs | 1 + contracts/tgrade-valset/src/multitest/migration.rs | 11 +++++++++++ 3 files changed, 15 insertions(+) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 36f7cb34..70523d76 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -997,6 +997,9 @@ pub fn migrate( if let Some(max_validators) = msg.max_validators { cfg.max_validators = max_validators; } + if let Some(distribution_contracts) = msg.distribution_contracts { + cfg.distribution_contracts = distribution_contracts; + } if let Some(verify_validators) = msg.verify_validators { cfg.verify_validators = verify_validators; } diff --git a/contracts/tgrade-valset/src/msg.rs b/contracts/tgrade-valset/src/msg.rs index fabb058e..748e4dc0 100644 --- a/contracts/tgrade-valset/src/msg.rs +++ b/contracts/tgrade-valset/src/msg.rs @@ -547,6 +547,7 @@ pub struct InstantiateResponse { pub struct MigrateMsg { pub min_points: Option, pub max_validators: Option, + pub distribution_contracts: Option>, pub verify_validators: Option, } diff --git a/contracts/tgrade-valset/src/multitest/migration.rs b/contracts/tgrade-valset/src/multitest/migration.rs index ce6d0d37..fe233d5e 100644 --- a/contracts/tgrade-valset/src/multitest/migration.rs +++ b/contracts/tgrade-valset/src/multitest/migration.rs @@ -1,5 +1,7 @@ use super::suite::SuiteBuilder; use crate::msg::MigrateMsg; +use crate::state::DistributionContract; +use cosmwasm_std::{Addr, Decimal}; #[test] fn migration_can_alter_cfg() { @@ -19,6 +21,10 @@ fn migration_can_alter_cfg() { &MigrateMsg { min_points: Some(5), max_validators: Some(10), + distribution_contracts: Some(vec![DistributionContract { + contract: Addr::unchecked("engagement1".to_string()), + ratio: Decimal::percent(50), + }]), verify_validators: Some(true), }, ) @@ -27,4 +33,9 @@ fn migration_can_alter_cfg() { let cfg = suite.config().unwrap(); assert_eq!(cfg.max_validators, 10); assert_eq!(cfg.min_points, 5); + assert!(cfg.verify_validators); + assert_eq!(cfg.distribution_contracts, vec![DistributionContract { + contract: Addr::unchecked("engagement1".to_string()), + ratio: Decimal::percent(50), + }]); } From c6dc85c243d3533ed067355a7a6f1a19359a6e43 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Mon, 19 Sep 2022 12:47:34 +0200 Subject: [PATCH 2/4] Don't enable verify validators during migration by default --- contracts/tgrade-valset/src/contract.rs | 4 +--- contracts/tgrade-valset/src/migration.rs | 17 +---------------- .../tgrade-valset/src/multitest/migration.rs | 11 +++++++---- 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 70523d76..e39a9d51 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -23,7 +23,7 @@ use tg_bindings::{ use tg_utils::{Duration, JailingDuration, SlashMsg, ADMIN}; use crate::error::ContractError; -use crate::migration::{migrate_jailing_period, migrate_verify_validators}; +use crate::migration::migrate_jailing_period; use crate::msg::{ EpochResponse, ExecuteMsg, InstantiateMsg, InstantiateResponse, JailingEnd, JailingPeriod, ListActiveValidatorsResponse, ListValidatorResponse, ListValidatorSlashingResponse, MigrateMsg, @@ -1008,8 +1008,6 @@ pub fn migrate( migrate_jailing_period(deps.branch(), &original_version)?; - migrate_verify_validators(deps.branch(), &original_version)?; - Ok(Response::new()) } diff --git a/contracts/tgrade-valset/src/migration.rs b/contracts/tgrade-valset/src/migration.rs index e78a1cbf..faa69414 100644 --- a/contracts/tgrade-valset/src/migration.rs +++ b/contracts/tgrade-valset/src/migration.rs @@ -7,7 +7,7 @@ use tg_utils::Expiration; use crate::error::ContractError; use crate::msg::{JailingEnd, JailingPeriod}; -use crate::state::{CONFIG, JAIL}; +use crate::state::JAIL; /// `crate::msg::JailingPeriod` version from v0.6.2 and before #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] @@ -50,21 +50,6 @@ pub fn migrate_jailing_period( Ok(()) } -pub fn migrate_verify_validators( - deps: DepsMut, - version: &Version, -) -> Result<(), ContractError> { - let mut config = if *version <= "0.14.0".parse::().unwrap() { - CONFIG.load(deps.storage)? - } else { - return Ok(()); - }; - config.verify_validators = true; - CONFIG.save(deps.storage, &config)?; - - Ok(()) -} - #[cfg(test)] mod tests { //! These are very rudimentary tests that only -mock- old state and perform migrations on it. diff --git a/contracts/tgrade-valset/src/multitest/migration.rs b/contracts/tgrade-valset/src/multitest/migration.rs index fe233d5e..00388174 100644 --- a/contracts/tgrade-valset/src/multitest/migration.rs +++ b/contracts/tgrade-valset/src/multitest/migration.rs @@ -34,8 +34,11 @@ fn migration_can_alter_cfg() { assert_eq!(cfg.max_validators, 10); assert_eq!(cfg.min_points, 5); assert!(cfg.verify_validators); - assert_eq!(cfg.distribution_contracts, vec![DistributionContract { - contract: Addr::unchecked("engagement1".to_string()), - ratio: Decimal::percent(50), - }]); + assert_eq!( + cfg.distribution_contracts, + vec![DistributionContract { + contract: Addr::unchecked("engagement1".to_string()), + ratio: Decimal::percent(50), + }] + ); } From 3bed84272296f53884cc22a011354f547977cf65 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Mon, 19 Sep 2022 12:51:01 +0200 Subject: [PATCH 3/4] Remove outdated migration code in passing --- contracts/tgrade-valset/src/contract.rs | 8 +- contracts/tgrade-valset/src/lib.rs | 1 - contracts/tgrade-valset/src/migration.rs | 124 ----------------------- 3 files changed, 2 insertions(+), 131 deletions(-) delete mode 100644 contracts/tgrade-valset/src/migration.rs diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index e39a9d51..610fa8af 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -23,7 +23,6 @@ use tg_bindings::{ use tg_utils::{Duration, JailingDuration, SlashMsg, ADMIN}; use crate::error::ContractError; -use crate::migration::migrate_jailing_period; use crate::msg::{ EpochResponse, ExecuteMsg, InstantiateMsg, InstantiateResponse, JailingEnd, JailingPeriod, ListActiveValidatorsResponse, ListValidatorResponse, ListValidatorSlashingResponse, MigrateMsg, @@ -983,12 +982,11 @@ fn calculate_diff( #[cfg_attr(not(feature = "library"), entry_point)] pub fn migrate( - mut deps: DepsMut, + deps: DepsMut, _env: Env, msg: MigrateMsg, ) -> Result { - let original_version = - ensure_from_older_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; + let _ = ensure_from_older_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; CONFIG.update::<_, StdError>(deps.storage, |mut cfg| { if let Some(min_points) = msg.min_points { @@ -1006,8 +1004,6 @@ pub fn migrate( Ok(cfg) })?; - migrate_jailing_period(deps.branch(), &original_version)?; - Ok(Response::new()) } diff --git a/contracts/tgrade-valset/src/lib.rs b/contracts/tgrade-valset/src/lib.rs index f56dc8ff..c6b4d747 100644 --- a/contracts/tgrade-valset/src/lib.rs +++ b/contracts/tgrade-valset/src/lib.rs @@ -1,6 +1,5 @@ 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 deleted file mode 100644 index faa69414..00000000 --- a/contracts/tgrade-valset/src/migration.rs +++ /dev/null @@ -1,124 +0,0 @@ -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 {} - } - ) - ] - ); - } -} From 6c091ec779ad2414f4ec1a477ac815666b3a59b7 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Mon, 19 Sep 2022 19:17:54 +0200 Subject: [PATCH 4/4] Improve syntax Co-authored-by: Jakub Bogucki --- contracts/tgrade-valset/src/contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 610fa8af..7d19a1b6 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -986,7 +986,7 @@ pub fn migrate( _env: Env, msg: MigrateMsg, ) -> Result { - let _ = ensure_from_older_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; + ensure_from_older_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; CONFIG.update::<_, StdError>(deps.storage, |mut cfg| { if let Some(min_points) = msg.min_points {