From f285c674e55c552138556fe3254d5d5162bc0934 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 31 Jan 2022 11:28:51 +0100 Subject: [PATCH 1/4] Use ADMIN to control ics20 access, allow updates --- Cargo.lock | 1 + contracts/cw20-ics20/Cargo.toml | 1 + contracts/cw20-ics20/src/contract.rs | 23 +++++++++++++++-------- contracts/cw20-ics20/src/error.rs | 4 ++++ contracts/cw20-ics20/src/msg.rs | 6 +++++- contracts/cw20-ics20/src/state.rs | 7 +++++-- 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 56eb22914..1c7e38bc2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -548,6 +548,7 @@ version = "0.12.0-alpha2" dependencies = [ "cosmwasm-schema", "cosmwasm-std", + "cw-controllers", "cw-storage-plus", "cw-utils", "cw2", diff --git a/contracts/cw20-ics20/Cargo.toml b/contracts/cw20-ics20/Cargo.toml index fe31dd250..ca7a1bc00 100644 --- a/contracts/cw20-ics20/Cargo.toml +++ b/contracts/cw20-ics20/Cargo.toml @@ -23,6 +23,7 @@ cw2 = { path = "../../packages/cw2", version = "0.12.0-alpha2" } cw20 = { path = "../../packages/cw20", version = "0.12.0-alpha2" } cosmwasm-std = { version = "1.0.0-beta3", features = ["stargate"] } cw-storage-plus = { path = "../../packages/storage-plus", version = "0.12.0-alpha2" } +cw-controllers = { path = "../../packages/controllers", version = "0.12.0-alpha2" } schemars = "0.8.1" serde = { version = "1.0.103", default-features = false, features = ["derive"] } thiserror = { version = "1.0.23" } diff --git a/contracts/cw20-ics20/src/contract.rs b/contracts/cw20-ics20/src/contract.rs index 82133c6a7..3182a3e62 100644 --- a/contracts/cw20-ics20/src/contract.rs +++ b/contracts/cw20-ics20/src/contract.rs @@ -1,8 +1,8 @@ #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - ensure_eq, from_binary, to_binary, Addr, Binary, Deps, DepsMut, Env, IbcMsg, IbcQuery, - MessageInfo, Order, PortIdResponse, Response, StdResult, + from_binary, to_binary, Addr, Binary, Deps, DepsMut, Env, IbcMsg, IbcQuery, MessageInfo, Order, + PortIdResponse, Response, StdResult, }; use cw2::{get_contract_version, set_contract_version}; @@ -16,7 +16,7 @@ use crate::msg::{ AllowMsg, AllowedInfo, AllowedResponse, ChannelResponse, ConfigResponse, ExecuteMsg, InitMsg, ListAllowedResponse, ListChannelsResponse, MigrateMsg, PortResponse, QueryMsg, TransferMsg, }; -use crate::state::{AllowInfo, Config, ALLOW_LIST, CHANNEL_INFO, CHANNEL_STATE, CONFIG}; +use crate::state::{AllowInfo, Config, ADMIN, ALLOW_LIST, CHANNEL_INFO, CHANNEL_STATE, CONFIG}; use cw_utils::{maybe_addr, nonpayable, one_coin}; // version info for migration info @@ -25,7 +25,7 @@ const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); #[cfg_attr(not(feature = "library"), entry_point)] pub fn instantiate( - deps: DepsMut, + mut deps: DepsMut, _env: Env, _info: MessageInfo, msg: InitMsg, @@ -33,10 +33,12 @@ pub fn instantiate( set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; let cfg = Config { default_timeout: msg.default_timeout, - gov_contract: deps.api.addr_validate(&msg.gov_contract)?, }; CONFIG.save(deps.storage, &cfg)?; + let admin = deps.api.addr_validate(&msg.gov_contract)?; + ADMIN.set(deps.branch(), Some(admin))?; + // add all allows for allowed in msg.allowlist { let contract = deps.api.addr_validate(&allowed.contract)?; @@ -62,6 +64,10 @@ pub fn execute( execute_transfer(deps, env, msg, Amount::Native(coin), info.sender) } ExecuteMsg::Allow(allow) => execute_allow(deps, env, info, allow), + ExecuteMsg::UpdateAdmin { admin } => { + let admin = deps.api.addr_validate(&admin)?; + Ok(ADMIN.execute_update_admin(deps, info, Some(admin))?) + } } } @@ -151,8 +157,7 @@ pub fn execute_allow( info: MessageInfo, allow: AllowMsg, ) -> Result { - let cfg = CONFIG.load(deps.storage)?; - ensure_eq!(info.sender, cfg.gov_contract, ContractError::Unauthorized); + ADMIN.assert_admin(deps.as_ref(), &info.sender)?; let contract = deps.api.addr_validate(&allow.contract)?; let set = AllowInfo { @@ -207,6 +212,7 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { QueryMsg::ListAllowed { start_after, limit } => { to_binary(&list_allowed(deps, start_after, limit)?) } + QueryMsg::Admin {} => to_binary(&ADMIN.query_admin(deps)?), } } @@ -251,9 +257,10 @@ pub fn query_channel(deps: Deps, id: String) -> StdResult { fn query_config(deps: Deps) -> StdResult { let cfg = CONFIG.load(deps.storage)?; + let admin = ADMIN.get(deps)?.unwrap_or_else(|| Addr::unchecked("")); let res = ConfigResponse { default_timeout: cfg.default_timeout, - gov_contract: cfg.gov_contract.into(), + gov_contract: admin.into(), }; Ok(res) } diff --git a/contracts/cw20-ics20/src/error.rs b/contracts/cw20-ics20/src/error.rs index cdd3b0574..c9a6966a6 100644 --- a/contracts/cw20-ics20/src/error.rs +++ b/contracts/cw20-ics20/src/error.rs @@ -3,6 +3,7 @@ use std::string::FromUtf8Error; use thiserror::Error; use cosmwasm_std::StdError; +use cw_controllers::AdminError; use cw_utils::PaymentError; /// Never is a placeholder to ensure we don't return any errors @@ -17,6 +18,9 @@ pub enum ContractError { #[error("{0}")] Payment(#[from] PaymentError), + #[error("{0}")] + Admin(#[from] AdminError), + #[error("Channel doesn't exist: {id}")] NoSuchChannel { id: String }, diff --git a/contracts/cw20-ics20/src/msg.rs b/contracts/cw20-ics20/src/msg.rs index 044cbf2f6..c5330dd97 100644 --- a/contracts/cw20-ics20/src/msg.rs +++ b/contracts/cw20-ics20/src/msg.rs @@ -34,6 +34,8 @@ pub enum ExecuteMsg { Transfer(TransferMsg), /// This must be called by gov_contract, will allow a new cw20 token to be sent Allow(AllowMsg), + /// Change the admin (must be called by current admin) + UpdateAdmin { admin: String }, } /// This is the message we accept via Receive @@ -59,8 +61,10 @@ pub enum QueryMsg { /// Returns the details of the name channel, error if not created. /// Return type: ChannelResponse. Channel { id: String }, - /// Show the Config. Returns ConfigResponse + /// Show the Config. Returns ConfigResponse (currently including admin as well) Config {}, + /// Return AdminResponse + Admin {}, /// Query if a given cw20 contract is allowed. Returns AllowedResponse Allowed { contract: String }, /// List all allowed cw20 contracts. Returns ListAllowedResponse diff --git a/contracts/cw20-ics20/src/state.rs b/contracts/cw20-ics20/src/state.rs index 79c127d8c..20277035a 100644 --- a/contracts/cw20-ics20/src/state.rs +++ b/contracts/cw20-ics20/src/state.rs @@ -1,10 +1,14 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::ContractError; use cosmwasm_std::{Addr, IbcEndpoint, StdResult, Storage, Uint128}; +use cw_controllers::Admin; use cw_storage_plus::{Item, Map}; +use crate::ContractError; + +pub const ADMIN: Admin = Admin::new("admin"); + pub const CONFIG: Item = Item::new("ics20_config"); // Used to pass info from the ibc_packet_receive to the reply handler @@ -28,7 +32,6 @@ pub struct ChannelState { #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] pub struct Config { pub default_timeout: u64, - pub gov_contract: Addr, } #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] From 783f0d5fecf6ef0c810812b0be28a46f5b248c77 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 31 Jan 2022 13:36:58 +0100 Subject: [PATCH 2/4] Prepare better migrate function --- Cargo.lock | 1 + contracts/cw20-ics20/Cargo.toml | 1 + contracts/cw20-ics20/src/contract.rs | 34 ++++++++++++++++++++++++---- contracts/cw20-ics20/src/error.rs | 3 +++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1c7e38bc2..e2ee01f14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -554,6 +554,7 @@ dependencies = [ "cw2", "cw20", "schemars", + "semver", "serde", "thiserror", ] diff --git a/contracts/cw20-ics20/Cargo.toml b/contracts/cw20-ics20/Cargo.toml index ca7a1bc00..765cc7035 100644 --- a/contracts/cw20-ics20/Cargo.toml +++ b/contracts/cw20-ics20/Cargo.toml @@ -25,6 +25,7 @@ cosmwasm-std = { version = "1.0.0-beta3", features = ["stargate"] } cw-storage-plus = { path = "../../packages/storage-plus", version = "0.12.0-alpha2" } cw-controllers = { path = "../../packages/controllers", version = "0.12.0-alpha2" } schemars = "0.8.1" +semver = "1" serde = { version = "1.0.103", default-features = false, features = ["derive"] } thiserror = { version = "1.0.23" } diff --git a/contracts/cw20-ics20/src/contract.rs b/contracts/cw20-ics20/src/contract.rs index 3182a3e62..dbb52f015 100644 --- a/contracts/cw20-ics20/src/contract.rs +++ b/contracts/cw20-ics20/src/contract.rs @@ -2,8 +2,9 @@ use cosmwasm_std::entry_point; use cosmwasm_std::{ from_binary, to_binary, Addr, Binary, Deps, DepsMut, Env, IbcMsg, IbcQuery, MessageInfo, Order, - PortIdResponse, Response, StdResult, + PortIdResponse, Response, StdError, StdResult, }; +use semver::Version; use cw2::{get_contract_version, set_contract_version}; use cw20::{Cw20Coin, Cw20ReceiveMsg}; @@ -192,13 +193,36 @@ pub fn execute_allow( #[cfg_attr(not(feature = "library"), entry_point)] pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { - let version = get_contract_version(deps.storage)?; - if version.contract != CONTRACT_NAME { + let version: Version = CONTRACT_VERSION.parse().map_err(from_semver)?; + let stored = get_contract_version(deps.storage)?; + let storage_version: Version = stored.version.parse().map_err(from_semver)?; + + if CONTRACT_NAME != stored.contract { return Err(ContractError::CannotMigrate { - previous_contract: version.contract, + previous_contract: stored.contract, }); } - Ok(Response::default()) + if storage_version > version { + return Err(ContractError::CannotMigrateVersion { + previous_version: stored.version, + }); + } + + // for 0.12.0-alpha1 or earlier, migrate from the config.gov_contract to ADMIN + if storage_version <= "0.12.0-alpha1".parse().map_err(from_semver)? { + // DO migration + } + + if storage_version < version { + // we don't need to save anything if migrating from the same version + set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; + } + + Ok(Response::new()) +} + +fn from_semver(err: semver::Error) -> StdError { + StdError::generic_err(format!("Semver: {}", err)) } #[cfg_attr(not(feature = "library"), entry_point)] diff --git a/contracts/cw20-ics20/src/error.rs b/contracts/cw20-ics20/src/error.rs index c9a6966a6..286ba4a62 100644 --- a/contracts/cw20-ics20/src/error.rs +++ b/contracts/cw20-ics20/src/error.rs @@ -51,6 +51,9 @@ pub enum ContractError { #[error("Cannot migrate from different contract type: {previous_contract}")] CannotMigrate { previous_contract: String }, + #[error("Cannot migrate from unsupported version: {previous_version}")] + CannotMigrateVersion { previous_version: String }, + #[error("Got a submessage reply with unknown id: {id}")] UnknownReplyId { id: u64 }, From 8d5524f8e40f650929c820f9771ffe64b7157f2f Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 31 Jan 2022 13:45:08 +0100 Subject: [PATCH 3/4] Perform proper migration logic --- contracts/cw20-ics20/src/contract.rs | 21 +++++++++++++++++++-- contracts/cw20-ics20/src/lib.rs | 1 + contracts/cw20-ics20/src/migrations.rs | 16 ++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 contracts/cw20-ics20/src/migrations.rs diff --git a/contracts/cw20-ics20/src/contract.rs b/contracts/cw20-ics20/src/contract.rs index dbb52f015..cbcd8767b 100644 --- a/contracts/cw20-ics20/src/contract.rs +++ b/contracts/cw20-ics20/src/contract.rs @@ -192,26 +192,43 @@ pub fn execute_allow( } #[cfg_attr(not(feature = "library"), entry_point)] -pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { +pub fn migrate(mut deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { let version: Version = CONTRACT_VERSION.parse().map_err(from_semver)?; let stored = get_contract_version(deps.storage)?; let storage_version: Version = stored.version.parse().map_err(from_semver)?; + // First, ensure we are working from an equal or older version of this contract + // wrong type if CONTRACT_NAME != stored.contract { return Err(ContractError::CannotMigrate { previous_contract: stored.contract, }); } + // existing one is newer if storage_version > version { return Err(ContractError::CannotMigrateVersion { previous_version: stored.version, }); } + // Then, run the proper migration + // unsupported old version + if storage_version < "0.11.1".parse().map_err(from_semver)? { + return Err(ContractError::CannotMigrateVersion { + previous_version: stored.version, + }); + } // for 0.12.0-alpha1 or earlier, migrate from the config.gov_contract to ADMIN if storage_version <= "0.12.0-alpha1".parse().map_err(from_semver)? { - // DO migration + // Do v1 migration + let old_config = crate::migrations::v1::CONFIG.load(deps.storage)?; + ADMIN.set(deps.branch(), Some(old_config.gov_contract))?; + let config = Config { + default_timeout: old_config.default_timeout, + }; + CONFIG.save(deps.storage, &config)?; } + // otherwise no migration (yet) if storage_version < version { // we don't need to save anything if migrating from the same version diff --git a/contracts/cw20-ics20/src/lib.rs b/contracts/cw20-ics20/src/lib.rs index 5e579aafa..b9ceef245 100644 --- a/contracts/cw20-ics20/src/lib.rs +++ b/contracts/cw20-ics20/src/lib.rs @@ -2,6 +2,7 @@ pub mod amount; pub mod contract; mod error; pub mod ibc; +mod migrations; pub mod msg; pub mod state; mod test_helpers; diff --git a/contracts/cw20-ics20/src/migrations.rs b/contracts/cw20-ics20/src/migrations.rs new file mode 100644 index 000000000..888df9418 --- /dev/null +++ b/contracts/cw20-ics20/src/migrations.rs @@ -0,0 +1,16 @@ +// v1 format is anything older than 0.12.0 +pub mod v1 { + use schemars::JsonSchema; + use serde::{Deserialize, Serialize}; + + use cosmwasm_std::Addr; + use cw_storage_plus::Item; + + #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] + pub struct ConfigV1 { + pub default_timeout: u64, + pub gov_contract: Addr, + } + + pub const CONFIG: Item = Item::new("ics20_config"); +} From 20c21779b082794ef30333ade457c20fe62a8f04 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 2 Feb 2022 20:53:28 +0100 Subject: [PATCH 4/4] Cleanup from pr review --- contracts/cw20-ics20/src/contract.rs | 18 ++++++++++-------- contracts/cw20-ics20/src/migrations.rs | 4 ++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/contracts/cw20-ics20/src/contract.rs b/contracts/cw20-ics20/src/contract.rs index cbcd8767b..35f946fd4 100644 --- a/contracts/cw20-ics20/src/contract.rs +++ b/contracts/cw20-ics20/src/contract.rs @@ -13,6 +13,7 @@ use cw_storage_plus::Bound; use crate::amount::Amount; use crate::error::ContractError; use crate::ibc::Ics20Packet; +use crate::migrations::v1; use crate::msg::{ AllowMsg, AllowedInfo, AllowedResponse, ChannelResponse, ConfigResponse, ExecuteMsg, InitMsg, ListAllowedResponse, ListChannelsResponse, MigrateMsg, PortResponse, QueryMsg, TransferMsg, @@ -191,6 +192,9 @@ pub fn execute_allow( Ok(res) } +const MIGRATE_MIN_VERSION: &str = "0.11.1"; +const MIGRATE_VERSION_2: &str = "0.12.0-alpha1"; + #[cfg_attr(not(feature = "library"), entry_point)] pub fn migrate(mut deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result { let version: Version = CONTRACT_VERSION.parse().map_err(from_semver)?; @@ -212,26 +216,24 @@ pub fn migrate(mut deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Resultv2 converstion if we are v1 style + if storage_version <= MIGRATE_VERSION_2.parse().map_err(from_semver)? { + let old_config = v1::CONFIG.load(deps.storage)?; ADMIN.set(deps.branch(), Some(old_config.gov_contract))?; let config = Config { default_timeout: old_config.default_timeout, }; CONFIG.save(deps.storage, &config)?; } - // otherwise no migration (yet) + // otherwise no migration (yet) - add them here + // we don't need to save anything if migrating from the same version if storage_version < version { - // we don't need to save anything if migrating from the same version set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; } diff --git a/contracts/cw20-ics20/src/migrations.rs b/contracts/cw20-ics20/src/migrations.rs index 888df9418..e874569a0 100644 --- a/contracts/cw20-ics20/src/migrations.rs +++ b/contracts/cw20-ics20/src/migrations.rs @@ -7,10 +7,10 @@ pub mod v1 { use cw_storage_plus::Item; #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] - pub struct ConfigV1 { + pub struct Config { pub default_timeout: u64, pub gov_contract: Addr, } - pub const CONFIG: Item = Item::new("ics20_config"); + pub const CONFIG: Item = Item::new("ics20_config"); }