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.

Sign up for GitHub

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

Redesign channel types for ibc_channel_open #1008

Merged
merged 7 commits into from
Jul 19, 2021
Merged

Redesign channel types for ibc_channel_open #1008

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eb26893
Move counterparty_version to IbcChannelOpenMsg
uint Jul 14, 2021
455b49a
Diff between the 6 different IBC entry points
uint Jul 15, 2021
429ca60
Update contracts
uint Jul 15, 2021
f28520d
style
uint Jul 19, 2021
71ead80
Update vm test contracts
uint Jul 19, 2021
c9d8088
Update docs re IBC channel entrypoint changes
uint Jul 19, 2021
ed2f744
Update docs
uint Jul 19, 2021
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
35 changes: 17 additions & 18 deletions contracts/ibc-reflect-send/src/ibc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub const PACKET_LIFETIME: u64 = 60 * 60;
#[entry_point]
/// enforces ordering and versioing constraints
pub fn ibc_channel_open(_deps: DepsMut, _env: Env, msg: IbcChannelOpenMsg) -> StdResult<()> {
let channel = msg.channel;
let channel = msg.channel();

if channel.order != IbcOrder::Ordered {
return Err(StdError::generic_err("Only supports ordered channels"));
Expand All @@ -29,10 +29,9 @@ pub fn ibc_channel_open(_deps: DepsMut, _env: Env, msg: IbcChannelOpenMsg) -> St
IBC_VERSION
)));
}
// TODO: do we need to check counterparty version as well?
// This flow needs to be well documented
if let Some(counter_version) = msg.counterparty_version {
if counter_version.as_str() != IBC_VERSION {

if let Some(counter_version) = msg.counterparty_version() {
webmaster128 marked this conversation as resolved.
Show resolved Hide resolved
if counter_version != IBC_VERSION {
return Err(StdError::generic_err(format!(
"Counterparty version must be `{}`",
IBC_VERSION
Expand All @@ -50,9 +49,9 @@ pub fn ibc_channel_connect(
env: Env,
msg: IbcChannelConnectMsg,
) -> StdResult<IbcBasicResponse> {
let channel = msg.channel;
let channel = msg.channel();

let channel_id = channel.endpoint.channel_id;
let channel_id = &channel.endpoint.channel_id;

// create an account holder the channel exists (not found if not registered)
let data = AccountData::default();
Expand Down Expand Up @@ -83,10 +82,10 @@ pub fn ibc_channel_close(
_env: Env,
msg: IbcChannelCloseMsg,
) -> StdResult<IbcBasicResponse> {
let channel = msg.channel;
let channel = msg.channel();

// remove the channel
let channel_id = channel.endpoint.channel_id;
let channel_id = &channel.endpoint.channel_id;
accounts(deps.storage).remove(channel_id.as_bytes());

Ok(IbcBasicResponse {
Expand Down Expand Up @@ -260,8 +259,9 @@ mod tests {
use crate::msg::{AccountResponse, ExecuteMsg, InstantiateMsg, QueryMsg};

use cosmwasm_std::testing::{
mock_dependencies, mock_env, mock_ibc_channel_connect, mock_ibc_channel_open,
mock_ibc_packet_ack, mock_info, MockApi, MockQuerier, MockStorage,
mock_dependencies, mock_env, mock_ibc_channel_connect_ack, mock_ibc_channel_open_init,
mock_ibc_channel_open_try, mock_ibc_packet_ack, mock_info, MockApi, MockQuerier,
MockStorage,
};
use cosmwasm_std::{coin, coins, BankMsg, CosmosMsg, IbcAcknowledgement, OwnedDeps};

Expand All @@ -279,15 +279,13 @@ mod tests {
// connect will run through the entire handshake to set up a proper connect and
// save the account (tested in detail in `proper_handshake_flow`)
fn connect(mut deps: DepsMut, channel_id: &str) {
// open packet has no counterparty version, connect does
let mut handshake_open = mock_ibc_channel_open(channel_id, IbcOrder::Ordered, IBC_VERSION);
handshake_open.counterparty_version = None;
let handshake_open = mock_ibc_channel_open_init(channel_id, IbcOrder::Ordered, IBC_VERSION);
webmaster128 marked this conversation as resolved.
Show resolved Hide resolved
// first we try to open with a valid handshake
ibc_channel_open(deps.branch(), mock_env(), handshake_open).unwrap();

// then we connect (with counter-party version set)
let handshake_connect =
mock_ibc_channel_connect(channel_id, IbcOrder::Ordered, IBC_VERSION);
mock_ibc_channel_connect_ack(channel_id, IbcOrder::Ordered, IBC_VERSION);
let res = ibc_channel_connect(deps.branch(), mock_env(), handshake_connect).unwrap();

// this should send a WhoAmI request, which is received some blocks later
Expand Down Expand Up @@ -316,13 +314,14 @@ mod tests {
fn enforce_version_in_handshake() {
let mut deps = setup();

let wrong_order = mock_ibc_channel_open("channel-12", IbcOrder::Unordered, IBC_VERSION);
let wrong_order = mock_ibc_channel_open_try("channel-12", IbcOrder::Unordered, IBC_VERSION);
ibc_channel_open(deps.as_mut(), mock_env(), wrong_order).unwrap_err();

let wrong_version = mock_ibc_channel_open("channel-12", IbcOrder::Ordered, "reflect");
let wrong_version = mock_ibc_channel_open_try("channel-12", IbcOrder::Ordered, "reflect");
ibc_channel_open(deps.as_mut(), mock_env(), wrong_version).unwrap_err();

let valid_handshake = mock_ibc_channel_open("channel-12", IbcOrder::Ordered, IBC_VERSION);
let valid_handshake =
mock_ibc_channel_open_try("channel-12", IbcOrder::Ordered, IBC_VERSION);
ibc_channel_open(deps.as_mut(), mock_env(), valid_handshake).unwrap();
}

Expand Down
17 changes: 10 additions & 7 deletions contracts/ibc-reflect-send/tests/integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
//! });
//! 4. Anywhere you see query(&deps, ...) you must replace it with query(&mut deps, ...)

use cosmwasm_std::testing::{mock_ibc_channel_connect, mock_ibc_channel_open, mock_ibc_packet_ack};
use cosmwasm_std::testing::{
mock_ibc_channel_connect_ack, mock_ibc_channel_open_init, mock_ibc_channel_open_try,
mock_ibc_packet_ack,
};
use cosmwasm_std::{
attr, coin, coins, BankMsg, CosmosMsg, Empty, IbcAcknowledgement, IbcBasicResponse, IbcMsg,
IbcOrder, Response,
Expand Down Expand Up @@ -53,13 +56,13 @@ fn setup() -> Instance<MockApi, MockStorage, MockQuerier> {
// save the account (tested in detail in `proper_handshake_flow`)
fn connect(deps: &mut Instance<MockApi, MockStorage, MockQuerier>, channel_id: &str) {
// open packet has no counterparty version, connect does
let mut handshake_open = mock_ibc_channel_open(channel_id, IbcOrder::Ordered, IBC_VERSION);
handshake_open.counterparty_version = None;
let handshake_open = mock_ibc_channel_open_init(channel_id, IbcOrder::Ordered, IBC_VERSION);
// first we try to open with a valid handshake
ibc_channel_open(deps, mock_env(), handshake_open).unwrap();

// then we connect (with counter-party version set)
let handshake_connect = mock_ibc_channel_connect(channel_id, IbcOrder::Ordered, IBC_VERSION);
let handshake_connect =
mock_ibc_channel_connect_ack(channel_id, IbcOrder::Ordered, IBC_VERSION);
let res: IbcBasicResponse = ibc_channel_connect(deps, mock_env(), handshake_connect).unwrap();

// this should send a WhoAmI request, which is received some blocks later
Expand Down Expand Up @@ -100,13 +103,13 @@ fn instantiate_works() {
fn enforce_version_in_handshake() {
let mut deps = setup();

let wrong_order = mock_ibc_channel_open("channel-12", IbcOrder::Unordered, IBC_VERSION);
let wrong_order = mock_ibc_channel_open_try("channel-12", IbcOrder::Unordered, IBC_VERSION);
ibc_channel_open(&mut deps, mock_env(), wrong_order).unwrap_err();

let wrong_version = mock_ibc_channel_open("channel-12", IbcOrder::Ordered, "reflect");
let wrong_version = mock_ibc_channel_open_try("channel-12", IbcOrder::Ordered, "reflect");
ibc_channel_open(&mut deps, mock_env(), wrong_version).unwrap_err();

let valid_handshake = mock_ibc_channel_open("channel-12", IbcOrder::Ordered, IBC_VERSION);
let valid_handshake = mock_ibc_channel_open_try("channel-12", IbcOrder::Ordered, IBC_VERSION);
ibc_channel_open(&mut deps, mock_env(), valid_handshake).unwrap();
}

Expand Down
42 changes: 19 additions & 23 deletions contracts/ibc-reflect/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub fn query_list_accounts(deps: Deps) -> StdResult<ListAccountsResponse> {
#[entry_point]
/// enforces ordering and versioing constraints
pub fn ibc_channel_open(_deps: DepsMut, _env: Env, msg: IbcChannelOpenMsg) -> StdResult<()> {
let channel = msg.channel;
let channel = msg.channel();

if channel.order != IbcOrder::Ordered {
return Err(StdError::generic_err("Only supports ordered channels"));
Expand All @@ -138,10 +138,9 @@ pub fn ibc_channel_open(_deps: DepsMut, _env: Env, msg: IbcChannelOpenMsg) -> St
IBC_VERSION
)));
}
// TODO: do we need to check counterparty version as well?
// This flow needs to be well documented
if let Some(counter_version) = msg.counterparty_version {
if counter_version.as_str() != IBC_VERSION {

if let Some(counter_version) = msg.counterparty_version() {
if counter_version != IBC_VERSION {
return Err(StdError::generic_err(format!(
"Counterparty version must be `{}`",
IBC_VERSION
Expand All @@ -159,9 +158,9 @@ pub fn ibc_channel_connect(
_env: Env,
msg: IbcChannelConnectMsg,
) -> StdResult<IbcBasicResponse> {
let channel = msg.channel;
let channel = msg.channel();
let cfg = config(deps.storage).load()?;
let chan_id = channel.endpoint.channel_id;
let chan_id = &channel.endpoint.channel_id;

let msg = WasmMsg::Instantiate {
admin: None,
Expand Down Expand Up @@ -190,7 +189,7 @@ pub fn ibc_channel_close(
env: Env,
msg: IbcChannelCloseMsg,
) -> StdResult<IbcBasicResponse> {
let channel = msg.channel;
let channel = msg.channel();
// get contract address and remove lookup
let channel_id = channel.endpoint.channel_id.as_str();
let reflect_addr = accounts(deps.storage).load(channel_id.as_bytes())?;
Expand Down Expand Up @@ -362,9 +361,9 @@ pub fn ibc_packet_timeout(
mod tests {
use super::*;
use cosmwasm_std::testing::{
mock_dependencies, mock_env, mock_ibc_channel_close, mock_ibc_channel_connect,
mock_ibc_channel_open, mock_ibc_packet_recv, mock_info, MockApi, MockQuerier, MockStorage,
MOCK_CONTRACT_ADDR,
mock_dependencies, mock_env, mock_ibc_channel_close_init, mock_ibc_channel_connect_ack,
mock_ibc_channel_open_init, mock_ibc_channel_open_try, mock_ibc_packet_recv, mock_info,
MockApi, MockQuerier, MockStorage, MOCK_CONTRACT_ADDR,
};
use cosmwasm_std::{coin, coins, from_slice, BankMsg, OwnedDeps, WasmMsg};

Expand Down Expand Up @@ -403,16 +402,13 @@ mod tests {
fn connect<T: Into<String>>(mut deps: DepsMut, channel_id: &str, account: T) {
let account: String = account.into();

// open packet has no counterparty versin, connect does
// TODO: validate this with alpe
let mut handshake_open = mock_ibc_channel_open(channel_id, IbcOrder::Ordered, IBC_VERSION);
handshake_open.counterparty_version = None;
let handshake_open = mock_ibc_channel_open_init(channel_id, IbcOrder::Ordered, IBC_VERSION);
// first we try to open with a valid handshake
ibc_channel_open(deps.branch(), mock_env(), handshake_open).unwrap();

// then we connect (with counter-party version set)
let handshake_connect =
mock_ibc_channel_connect(channel_id, IbcOrder::Ordered, IBC_VERSION);
mock_ibc_channel_connect_ack(channel_id, IbcOrder::Ordered, IBC_VERSION);
let res = ibc_channel_connect(deps.branch(), mock_env(), handshake_connect).unwrap();
assert_eq!(1, res.messages.len());
assert_eq!(1, res.events.len());
Expand Down Expand Up @@ -446,13 +442,14 @@ mod tests {
fn enforce_version_in_handshake() {
let mut deps = setup();

let wrong_order = mock_ibc_channel_open("channel-12", IbcOrder::Unordered, IBC_VERSION);
let wrong_order = mock_ibc_channel_open_try("channel-12", IbcOrder::Unordered, IBC_VERSION);
ibc_channel_open(deps.as_mut(), mock_env(), wrong_order).unwrap_err();

let wrong_version = mock_ibc_channel_open("channel-12", IbcOrder::Ordered, "reflect");
let wrong_version = mock_ibc_channel_open_try("channel-12", IbcOrder::Ordered, "reflect");
ibc_channel_open(deps.as_mut(), mock_env(), wrong_version).unwrap_err();

let valid_handshake = mock_ibc_channel_open("channel-12", IbcOrder::Ordered, IBC_VERSION);
let valid_handshake =
mock_ibc_channel_open_try("channel-12", IbcOrder::Ordered, IBC_VERSION);
ibc_channel_open(deps.as_mut(), mock_env(), valid_handshake).unwrap();
}

Expand All @@ -462,13 +459,12 @@ mod tests {
let channel_id = "channel-1234";

// first we try to open with a valid handshake
let mut handshake_open = mock_ibc_channel_open(channel_id, IbcOrder::Ordered, IBC_VERSION);
handshake_open.counterparty_version = None;
let handshake_open = mock_ibc_channel_open_init(channel_id, IbcOrder::Ordered, IBC_VERSION);
ibc_channel_open(deps.as_mut(), mock_env(), handshake_open).unwrap();

// then we connect (with counter-party version set)
let handshake_connect =
mock_ibc_channel_connect(channel_id, IbcOrder::Ordered, IBC_VERSION);
mock_ibc_channel_connect_ack(channel_id, IbcOrder::Ordered, IBC_VERSION);
let res = ibc_channel_connect(deps.as_mut(), mock_env(), handshake_connect).unwrap();
// and set up a reflect account
assert_eq!(1, res.messages.len());
Expand Down Expand Up @@ -628,7 +624,7 @@ mod tests {
assert_eq!(funds, balance);

// close the channel
let channel = mock_ibc_channel_close(channel_id, IbcOrder::Ordered, IBC_VERSION);
let channel = mock_ibc_channel_close_init(channel_id, IbcOrder::Ordered, IBC_VERSION);
let res = ibc_channel_close(deps.as_mut(), mock_env(), channel).unwrap();

// it pulls out all money from the reflect contract
Expand Down
21 changes: 11 additions & 10 deletions contracts/ibc-reflect/tests/integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
//! 4. Anywhere you see query(&deps, ...) you must replace it with query(&mut deps, ...)

use cosmwasm_std::testing::{
mock_ibc_channel_connect, mock_ibc_channel_open, mock_ibc_packet_recv,
mock_ibc_channel_connect_ack, mock_ibc_channel_open_init, mock_ibc_channel_open_try,
mock_ibc_packet_recv,
};
use cosmwasm_std::{
attr, coins, BankMsg, ContractResult, CosmosMsg, Event, IbcBasicResponse, IbcOrder,
Expand Down Expand Up @@ -80,12 +81,12 @@ fn connect<T: Into<String>>(
) {
let account: String = account.into();
// first we try to open with a valid handshake
let mut handshake_open = mock_ibc_channel_open(channel_id, IbcOrder::Ordered, IBC_VERSION);
handshake_open.counterparty_version = None;
let handshake_open = mock_ibc_channel_open_init(channel_id, IbcOrder::Ordered, IBC_VERSION);
ibc_channel_open(deps, mock_env(), handshake_open).unwrap();

// then we connect (with counter-party version set)
let handshake_connect = mock_ibc_channel_connect(channel_id, IbcOrder::Ordered, IBC_VERSION);
let handshake_connect =
mock_ibc_channel_connect_ack(channel_id, IbcOrder::Ordered, IBC_VERSION);
let res: IbcBasicResponse = ibc_channel_connect(deps, mock_env(), handshake_connect).unwrap();
assert_eq!(1, res.messages.len());
assert_eq!(1, res.events.len());
Expand Down Expand Up @@ -120,13 +121,13 @@ fn instantiate_works() {
fn enforce_version_in_handshake() {
let mut deps = setup();

let wrong_order = mock_ibc_channel_open("channel-1234", IbcOrder::Unordered, IBC_VERSION);
let wrong_order = mock_ibc_channel_open_try("channel-1234", IbcOrder::Unordered, IBC_VERSION);
ibc_channel_open(&mut deps, mock_env(), wrong_order).unwrap_err();

let wrong_version = mock_ibc_channel_open("channel-1234", IbcOrder::Ordered, "reflect");
let wrong_version = mock_ibc_channel_open_try("channel-1234", IbcOrder::Ordered, "reflect");
ibc_channel_open(&mut deps, mock_env(), wrong_version).unwrap_err();

let valid_handshake = mock_ibc_channel_open("channel-1234", IbcOrder::Ordered, IBC_VERSION);
let valid_handshake = mock_ibc_channel_open_try("channel-1234", IbcOrder::Ordered, IBC_VERSION);
ibc_channel_open(&mut deps, mock_env(), valid_handshake).unwrap();
}

Expand All @@ -136,12 +137,12 @@ fn proper_handshake_flow() {
let channel_id = "channel-432";

// first we try to open with a valid handshake
let mut handshake_open = mock_ibc_channel_open(channel_id, IbcOrder::Ordered, IBC_VERSION);
handshake_open.counterparty_version = None;
let handshake_open = mock_ibc_channel_open_init(channel_id, IbcOrder::Ordered, IBC_VERSION);
ibc_channel_open(&mut deps, mock_env(), handshake_open).unwrap();

// then we connect (with counter-party version set)
let handshake_connect = mock_ibc_channel_connect(channel_id, IbcOrder::Ordered, IBC_VERSION);
let handshake_connect =
mock_ibc_channel_connect_ack(channel_id, IbcOrder::Ordered, IBC_VERSION);
let res: IbcBasicResponse =
ibc_channel_connect(&mut deps, mock_env(), handshake_connect).unwrap();
// and set up a reflect account
Expand Down
Loading