Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow modifying admin of Ics20 contract #641

Merged
merged 5 commits into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions contracts/cw20-ics20/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ 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"
semver = "1"
serde = { version = "1.0.103", default-features = false, features = ["derive"] }
thiserror = { version = "1.0.23" }

Expand Down
76 changes: 63 additions & 13 deletions contracts/cw20-ics20/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#[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, StdError, StdResult,
};
use semver::Version;

use cw2::{get_contract_version, set_contract_version};
use cw20::{Cw20Coin, Cw20ReceiveMsg};
Expand All @@ -12,11 +13,12 @@ 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,
};
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
Expand All @@ -25,18 +27,20 @@ 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,
) -> Result<Response, ContractError> {
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)?;
Expand All @@ -62,6 +66,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))?)
}
}
}

Expand Down Expand Up @@ -151,8 +159,7 @@ pub fn execute_allow(
info: MessageInfo,
allow: AllowMsg,
) -> Result<Response, ContractError> {
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 {
Expand Down Expand Up @@ -185,15 +192,56 @@ 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(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> {
let version = get_contract_version(deps.storage)?;
if version.contract != CONTRACT_NAME {
pub fn migrate(mut deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> {
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: version.contract,
previous_contract: stored.contract,
});
}
Ok(Response::default())
// existing one is newer
if storage_version > version {
return Err(ContractError::CannotMigrateVersion {
previous_version: stored.version,
});
}

// Then, run the proper migration
if storage_version < MIGRATE_MIN_VERSION.parse().map_err(from_semver)? {
return Err(ContractError::CannotMigrateVersion {
previous_version: stored.version,
});
}
// run the v1->v2 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) - add them here

// we don't need to save anything if migrating from the same version
if storage_version < version {
set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
}

Ok(Response::new())
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for now, but I wonder how much of this logic can be turned into a helper function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some of this is in poe-contracts/tg-utils

but matching on versions would be a good thing to abstract out (later, when we have some good examples/experience)


fn from_semver(err: semver::Error) -> StdError {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

StdError::generic_err(format!("Semver: {}", err))
}

#[cfg_attr(not(feature = "library"), entry_point)]
Expand All @@ -207,6 +255,7 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> {
QueryMsg::ListAllowed { start_after, limit } => {
to_binary(&list_allowed(deps, start_after, limit)?)
}
QueryMsg::Admin {} => to_binary(&ADMIN.query_admin(deps)?),
}
}

Expand Down Expand Up @@ -251,9 +300,10 @@ pub fn query_channel(deps: Deps, id: String) -> StdResult<ChannelResponse> {

fn query_config(deps: Deps) -> StdResult<ConfigResponse> {
let cfg = CONFIG.load(deps.storage)?;
let admin = ADMIN.get(deps)?.unwrap_or_else(|| Addr::unchecked(""));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Wouldn't "None" be better here? I think we used that in the past.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this contract, we don't allow None, so it doesn't map much to the ADMIN, I kind of force it

let res = ConfigResponse {
default_timeout: cfg.default_timeout,
gov_contract: cfg.gov_contract.into(),
gov_contract: admin.into(),
};
Ok(res)
}
Expand Down
7 changes: 7 additions & 0 deletions contracts/cw20-ics20/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 },

Expand Down Expand Up @@ -47,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 },

Expand Down
1 change: 1 addition & 0 deletions contracts/cw20-ics20/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions contracts/cw20-ics20/src/migrations.rs
Original file line number Diff line number Diff line change
@@ -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 Config {
pub default_timeout: u64,
pub gov_contract: Addr,
}

pub const CONFIG: Item<Config> = Item::new("ics20_config");
}
6 changes: 5 additions & 1 deletion contracts/cw20-ics20/src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions contracts/cw20-ics20/src/state.rs
Original file line number Diff line number Diff line change
@@ -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<Config> = Item::new("ics20_config");

// Used to pass info from the ibc_packet_receive to the reply handler
Expand All @@ -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)]
Expand Down