Skip to content

Commit

Permalink
Remove crossing hellos from Channel (#181)
Browse files Browse the repository at this point in the history 
* variable naming in ics26

* Rework ics26 channel handler

* cleanup Ics4ChannelMsg handler

* channel callbacks now return ModuleExtras

* channel_dispatch now returns a `MsgReceipt`

* remove crossing hellos logic from chan_open_try

Also removes tests that tested that logic specifically

* deprecate `MsgChannelOpenTry::previous_channel_id` field

* ics20: refactor on_chan_open_init

* Remove `version` field of `on_chan_open_try()`

* ICS20: inline on_chan_open_init

* ics20: on_chan_open_try

* ics20: refactor rest of handshake callbacks

* fmt

* ics26_routing: Derive Hash for ModuleId (#179)

* ics26_routing: Derive Hash for ModuleId

Signed-off-by: Kevin Ji <[email protected]>

* Add changelog for #179

Signed-off-by: Kevin Ji <[email protected]>
Co-authored-by: Shoaib Ahmed <[email protected]>

* Release v0.20.0 (#186)

* release v0.20.0

* bump crate version to 0.20.0

* update CONTRIBUTING

* CONTRIBUTING change

* Update ibc-proto-rs to v0.21.0

* Update CONTRIBUTING.md

* Connection proofs refactor (#170)

* conn_open_confirm refactor

* conn_open_ack handler

* disable mbt

* ConnOpenConfirm handler

Remove connection::verify.rs

* conn_open_ack: readability

* remove ConnectionOpenTry::with_previous_connection_id

* Make previous_connection_id deprecated

* remove mbt

* Remove hold_oldest_height check

* changelog

* Output logs for handlers again

* conn_open_ack: apply naming convention

* conn_open_try apply naming convention

* conn_open_init apply naming convention

* conn_open_try naming fix

* conn_open_confirm apply naming convention

* Document naming convention

* fmt

* comments

* Update crates/ibc/src/core/ics03_connection/handler.rs

Co-authored-by: Shoaib Ahmed <[email protected]>
Signed-off-by: Philippe Laferrière <[email protected]>

* Move naming convention docstring

* fix

* update deprecated annotation

Signed-off-by: Philippe Laferrière <[email protected]>
Co-authored-by: Shoaib Ahmed <[email protected]>

* changelog

* ics20 on_chan_open_init tests

* ics20 on_chan_open_try tests

* update deprecation versions

* update comment with issue

* fmt

* use ModuleExtras::empty()

Signed-off-by: Kevin Ji <[email protected]>
Signed-off-by: Philippe Laferrière <[email protected]>
Co-authored-by: Kevin Ji <[email protected]>
Co-authored-by: Shoaib Ahmed <[email protected]>
  • Loading branch information
@plafer @kevinji @hu55a1n1
3 people authored Oct 24, 2022
1 parent 9851366 commit 7e49b26
Show file tree
Hide file tree
Showing 16 changed files with 390 additions and 354 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/breaking-changes/157-channel-crossing-hellos.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Remove crossing hellos logic from channel handshake
([#157](https://github.com/cosmos/ibc-rs/issues/157))
221 changes: 169 additions & 52 deletions crates/ibc/src/applications/transfer/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::applications::transfer::relay::on_timeout_packet::process_timeout_pac
use crate::applications::transfer::{PrefixedCoin, PrefixedDenom, VERSION};
use crate::core::ics04_channel::channel::{Counterparty, Order};
use crate::core::ics04_channel::context::{ChannelKeeper, ChannelReader};
use crate::core::ics04_channel::handler::ModuleExtras;
use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement as GenericAcknowledgement;
use crate::core::ics04_channel::packet::Packet;
use crate::core::ics04_channel::Version;
Expand Down Expand Up @@ -99,106 +100,90 @@ pub trait Ics20Context:
type AccountId: TryFrom<Signer>;
}

fn validate_transfer_channel_params(
#[allow(clippy::too_many_arguments)]
pub fn on_chan_open_init(
ctx: &mut impl Ics20Context,
order: Order,
_connection_hops: &[ConnectionId],
port_id: &PortId,
_channel_id: &ChannelId,
_counterparty: &Counterparty,
version: &Version,
) -> Result<(), Ics20Error> {
) -> Result<(ModuleExtras, Version), Ics20Error> {
if order != Order::Unordered {
return Err(Ics20Error::channel_not_unordered(order));
}

let bound_port = ctx.get_port()?;
if port_id != &bound_port {
return Err(Ics20Error::invalid_port(port_id.clone(), bound_port));
}

if version != &Version::ics20() {
if !version.is_empty() && version != &Version::ics20() {
return Err(Ics20Error::invalid_version(version.clone()));
}

Ok(())
}

fn validate_counterparty_version(counterparty_version: &Version) -> Result<(), Ics20Error> {
if counterparty_version == &Version::ics20() {
Ok(())
} else {
Err(Ics20Error::invalid_counterparty_version(
counterparty_version.clone(),
))
}
}

#[allow(clippy::too_many_arguments)]
pub fn on_chan_open_init(
ctx: &mut impl Ics20Context,
_output: &mut ModuleOutputBuilder,
order: Order,
_connection_hops: &[ConnectionId],
port_id: &PortId,
channel_id: &ChannelId,
_counterparty: &Counterparty,
version: &Version,
) -> Result<(), Ics20Error> {
validate_transfer_channel_params(ctx, order, port_id, channel_id, version)
Ok((ModuleExtras::empty(), Version::ics20()))
}

#[allow(clippy::too_many_arguments)]
pub fn on_chan_open_try(
ctx: &mut impl Ics20Context,
_output: &mut ModuleOutputBuilder,
_ctx: &mut impl Ics20Context,
order: Order,
_connection_hops: &[ConnectionId],
port_id: &PortId,
channel_id: &ChannelId,
_port_id: &PortId,
_channel_id: &ChannelId,
_counterparty: &Counterparty,
version: &Version,
counterparty_version: &Version,
) -> Result<Version, Ics20Error> {
validate_transfer_channel_params(ctx, order, port_id, channel_id, version)?;
validate_counterparty_version(counterparty_version)?;
Ok(Version::ics20())
) -> Result<(ModuleExtras, Version), Ics20Error> {
if order != Order::Unordered {
return Err(Ics20Error::channel_not_unordered(order));
}
if counterparty_version != &Version::ics20() {
return Err(Ics20Error::invalid_counterparty_version(
counterparty_version.clone(),
));
}

Ok((ModuleExtras::empty(), Version::ics20()))
}

pub fn on_chan_open_ack(
_ctx: &mut impl Ics20Context,
_output: &mut ModuleOutputBuilder,
_port_id: &PortId,
_channel_id: &ChannelId,
counterparty_version: &Version,
) -> Result<(), Ics20Error> {
validate_counterparty_version(counterparty_version)?;
Ok(())
) -> Result<ModuleExtras, Ics20Error> {
if counterparty_version != &Version::ics20() {
return Err(Ics20Error::invalid_counterparty_version(
counterparty_version.clone(),
));
}

Ok(ModuleExtras::empty())
}

pub fn on_chan_open_confirm(
_ctx: &mut impl Ics20Context,
_output: &mut ModuleOutputBuilder,
_port_id: &PortId,
_channel_id: &ChannelId,
) -> Result<(), Ics20Error> {
Ok(())
) -> Result<ModuleExtras, Ics20Error> {
Ok(ModuleExtras::empty())
}

pub fn on_chan_close_init(
_ctx: &mut impl Ics20Context,
_output: &mut ModuleOutputBuilder,
_port_id: &PortId,
_channel_id: &ChannelId,
) -> Result<(), Ics20Error> {
) -> Result<ModuleExtras, Ics20Error> {
Err(Ics20Error::cant_close_channel())
}

pub fn on_chan_close_confirm(
_ctx: &mut impl Ics20Context,
_output: &mut ModuleOutputBuilder,
_port_id: &PortId,
_channel_id: &ChannelId,
) -> Result<(), Ics20Error> {
Ok(())
) -> Result<ModuleExtras, Ics20Error> {
Ok(ModuleExtras::empty())
}

pub fn on_recv_packet<Ctx: 'static + Ics20Context>(
Expand Down Expand Up @@ -284,15 +269,20 @@ pub fn on_timeout_packet(
pub(crate) mod test {
use subtle_encoding::bech32;

use crate::applications::transfer::context::cosmos_adr028_escrow_address;
use crate::applications::transfer::context::{cosmos_adr028_escrow_address, on_chan_open_try};
use crate::applications::transfer::error::Error as Ics20Error;
use crate::applications::transfer::msgs::transfer::MsgTransfer;
use crate::applications::transfer::relay::send_transfer::send_transfer;
use crate::applications::transfer::PrefixedCoin;
use crate::core::ics04_channel::channel::{Counterparty, Order};
use crate::core::ics04_channel::error::Error;
use crate::core::ics04_channel::Version;
use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
use crate::handler::HandlerOutputBuilder;
use crate::prelude::*;
use crate::test_utils::DummyTransferModule;
use crate::test_utils::{get_dummy_transfer_module, DummyTransferModule};

use super::on_chan_open_init;

pub(crate) fn deliver(
ctx: &mut DummyTransferModule,
Expand All @@ -302,6 +292,31 @@ pub(crate) mod test {
send_transfer(ctx, output, msg).map_err(|e: Ics20Error| Error::app_module(e.to_string()))
}

fn get_defaults() -> (
DummyTransferModule,
Order,
Vec<ConnectionId>,
PortId,
ChannelId,
Counterparty,
) {
let ctx = get_dummy_transfer_module();
let order = Order::Unordered;
let connection_hops = vec![ConnectionId::new(1)];
let port_id = PortId::transfer();
let channel_id = ChannelId::new(1);
let counterparty = Counterparty::new(port_id.clone(), Some(channel_id.clone()));

(
ctx,
order,
connection_hops,
port_id,
channel_id,
counterparty,
)
}

#[test]
fn test_cosmos_escrow_address() {
fn assert_eq_escrow_address(port_id: &str, channel_id: &str, address: &str) {
Expand Down Expand Up @@ -331,4 +346,106 @@ pub(crate) mod test {
"cosmos177x69sver58mcfs74x6dg0tv6ls4s3xmmcaw53",
);
}

/// If the relayer passed "", indicating that it wants us to return the versions we support.
/// We currently only support ics20
#[test]
fn test_on_chan_open_init_empty_version() {
let (mut ctx, order, connection_hops, port_id, channel_id, counterparty) = get_defaults();

let in_version = Version::new("".to_string());

let (_, out_version) = on_chan_open_init(
&mut ctx,
order,
&connection_hops,
&port_id,
&channel_id,
&counterparty,
&in_version,
)
.unwrap();

assert_eq!(out_version, Version::ics20());
}

/// If the relayer passed in the only supported version (ics20), then return ics20
#[test]
fn test_on_chan_open_init_ics20_version() {
let (mut ctx, order, connection_hops, port_id, channel_id, counterparty) = get_defaults();

let in_version = Version::ics20();
let (_, out_version) = on_chan_open_init(
&mut ctx,
order,
&connection_hops,
&port_id,
&channel_id,
&counterparty,
&in_version,
)
.unwrap();

assert_eq!(out_version, Version::ics20());
}

/// If the relayer passed in an unsupported version, then fail
#[test]
fn test_on_chan_open_init_incorrect_version() {
let (mut ctx, order, connection_hops, port_id, channel_id, counterparty) = get_defaults();

let in_version = Version::new("some-unsupported-version".to_string());
let res = on_chan_open_init(
&mut ctx,
order,
&connection_hops,
&port_id,
&channel_id,
&counterparty,
&in_version,
);

assert!(res.is_err());
}

/// If the counterparty supports ics20, then return ics20
#[test]
fn test_on_chan_open_try_counterparty_correct_version() {
let (mut ctx, order, connection_hops, port_id, channel_id, counterparty) = get_defaults();

let counterparty_version = Version::ics20();

let (_, out_version) = on_chan_open_try(
&mut ctx,
order,
&connection_hops,
&port_id,
&channel_id,
&counterparty,
&counterparty_version,
)
.unwrap();

assert_eq!(out_version, Version::ics20());
}

/// If the counterparty doesn't support ics20, then fail
#[test]
fn test_on_chan_open_try_counterparty_incorrect_version() {
let (mut ctx, order, connection_hops, port_id, channel_id, counterparty) = get_defaults();

let counterparty_version = Version::new("some-unsupported-version".to_string());

let res = on_chan_open_try(
&mut ctx,
order,
&connection_hops,
&port_id,
&channel_id,
&counterparty,
&counterparty_version,
);

assert!(res.is_err());
}
}
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics03_connection/msgs/conn_open_try.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub struct MsgConnectionOpenTry {
pub delay_period: Duration,
pub signer: Signer,

#[deprecated(since = "0.21.0")]
#[deprecated(since = "0.22.0")]
/// Only kept here for proper conversion to/from the raw type
previous_connection_id: String,
}
Expand Down
Loading

0 comments on commit 7e49b26

Please sign in to comment.