From 392155c9b8018f34a873f490549074ec98e295d7 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 20 Jan 2023 08:59:06 -0800 Subject: [PATCH 1/8] Verify AbciEvent keys match emitted events --- crates/ibc/src/core/ics02_client/events.rs | 71 +++++++++++ .../ibc/src/core/ics03_connection/events.rs | 76 +++++++++++ crates/ibc/src/core/ics04_channel/events.rs | 119 ++++++++++++++++++ .../events/channel_attributes.rs | 15 ++- 4 files changed, 273 insertions(+), 8 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/events.rs b/crates/ibc/src/core/ics02_client/events.rs index f5c15e063..3f51bb984 100644 --- a/crates/ibc/src/core/ics02_client/events.rs +++ b/crates/ibc/src/core/ics02_client/events.rs @@ -390,3 +390,74 @@ impl From for abci::Event { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::client_state::client_type as mock_client_type; + use crate::mock::header::MockHeader; + use ibc_proto::google::protobuf::Any; + use tendermint::abci::Event as AbciEvent; + + #[test] + fn ibc_to_abci_client_events() { + struct Test { + kind: IbcEventType, + event: AbciEvent, + expected_keys: Vec<&'static str>, + } + + let client_type = mock_client_type(); + let client_id = ClientId::new(client_type.clone(), 0).unwrap(); + let consensus_height = Height::new(0, 10).unwrap(); + let consensus_heights = vec![Height::new(0, 5).unwrap(), Height::new(0, 7).unwrap()]; + let header: Any = MockHeader::new(consensus_height).into(); + let expected_keys = vec![ + CLIENT_ID_ATTRIBUTE_KEY, + CLIENT_TYPE_ATTRIBUTE_KEY, + CONSENSUS_HEIGHT_ATTRIBUTE_KEY, + CONSENSUS_HEIGHTS_ATTRIBUTE_KEY, + HEADER_ATTRIBUTE_KEY, + ]; + + let tests: Vec = vec![ + Test { + kind: IbcEventType::CreateClient, + event: CreateClient::new(client_id.clone(), client_type.clone(), consensus_height) + .into(), + expected_keys: expected_keys[0..3].to_vec(), + }, + Test { + kind: IbcEventType::UpdateClient, + event: UpdateClient::new( + client_id.clone(), + client_type.clone(), + consensus_height, + consensus_heights, + header, + ) + .into(), + expected_keys: expected_keys.clone(), + }, + Test { + kind: IbcEventType::UpgradeClient, + event: UpgradeClient::new(client_id.clone(), client_type.clone(), consensus_height) + .into(), + expected_keys: expected_keys[0..3].to_vec(), + }, + Test { + kind: IbcEventType::ClientMisbehaviour, + event: ClientMisbehaviour::new(client_id, client_type).into(), + expected_keys: expected_keys[0..2].to_vec(), + }, + ]; + + for t in tests { + assert_eq!(t.event.kind, t.kind.as_str()); + assert_eq!(t.expected_keys.len(), t.event.attributes.len()); + for (i, key) in t.expected_keys.iter().enumerate() { + assert_eq!(t.event.attributes[i].key, *key); + } + } + } +} diff --git a/crates/ibc/src/core/ics03_connection/events.rs b/crates/ibc/src/core/ics03_connection/events.rs index ff26875c2..a79726d4a 100644 --- a/crates/ibc/src/core/ics03_connection/events.rs +++ b/crates/ibc/src/core/ics03_connection/events.rs @@ -281,3 +281,79 @@ impl From for abci::Event { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::client_state::client_type as mock_client_type; + use tendermint::abci::Event as AbciEvent; + + #[test] + fn ibc_to_abci_connection_events() { + struct Test { + kind: IbcEventType, + event: AbciEvent, + expected_keys: Vec<&'static str>, + } + + let conn_id_on_a = ConnectionId::default(); + let client_id_on_a = ClientId::default(); + let conn_id_on_b = ConnectionId::new(1); + let client_id_on_b = ClientId::new(mock_client_type(), 0).unwrap(); + let expected_keys = vec![ + CONN_ID_ATTRIBUTE_KEY, + CLIENT_ID_ATTRIBUTE_KEY, + COUNTERPARTY_CLIENT_ID_ATTRIBUTE_KEY, + COUNTERPARTY_CONN_ID_ATTRIBUTE_KEY, + ]; + + let tests: Vec = vec![ + Test { + kind: IbcEventType::OpenInitConnection, + event: OpenInit::new( + conn_id_on_a.clone(), + client_id_on_a.clone(), + client_id_on_b.clone(), + ) + .into(), + expected_keys: expected_keys.clone(), + }, + Test { + kind: IbcEventType::OpenTryConnection, + event: OpenTry::new( + conn_id_on_b.clone(), + client_id_on_b.clone(), + conn_id_on_a.clone(), + client_id_on_a.clone(), + ) + .into(), + expected_keys: expected_keys.clone(), + }, + Test { + kind: IbcEventType::OpenAckConnection, + event: OpenAck::new( + conn_id_on_a.clone(), + client_id_on_a.clone(), + conn_id_on_b.clone(), + client_id_on_b.clone(), + ) + .into(), + expected_keys: expected_keys.clone(), + }, + Test { + kind: IbcEventType::OpenConfirmConnection, + event: OpenConfirm::new(conn_id_on_b, client_id_on_b, conn_id_on_a, client_id_on_a) + .into(), + expected_keys: expected_keys.clone(), + }, + ]; + + for t in tests { + assert_eq!(t.kind.as_str(), t.event.kind); + assert_eq!(t.expected_keys.len(), t.event.attributes.len()); + for (i, key) in t.expected_keys.iter().enumerate() { + assert_eq!(t.event.attributes[i].key, *key); + } + } + } +} diff --git a/crates/ibc/src/core/ics04_channel/events.rs b/crates/ibc/src/core/ics04_channel/events.rs index bb214128d..355cb49bd 100644 --- a/crates/ibc/src/core/ics04_channel/events.rs +++ b/crates/ibc/src/core/ics04_channel/events.rs @@ -1058,3 +1058,122 @@ impl TryFrom for abci::Event { }) } } + +#[cfg(test)] +mod tests { + use super::*; + use channel_attributes::{ + CHANNEL_ID_ATTRIBUTE_KEY, CONNECTION_ID_ATTRIBUTE_KEY, + COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY, COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY, + PORT_ID_ATTRIBUTE_KEY, VERSION_ATTRIBUTE_KEY, + }; + use tendermint::abci::Event as AbciEvent; + + #[test] + fn ibc_to_abci_channel_events() { + struct Test { + kind: IbcEventType, + event: AbciEvent, + expected_keys: Vec<&'static str>, + } + + let port_id = PortId::default(); + let channel_id = ChannelId::default(); + let connection_id = ConnectionId::default(); + let counterparty_port_id = PortId::default(); + let counterparty_channel_id = ChannelId::new(1); + let version = Version::default(); + let expected_keys = vec![ + PORT_ID_ATTRIBUTE_KEY, + CHANNEL_ID_ATTRIBUTE_KEY, + COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY, + COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY, + CONNECTION_ID_ATTRIBUTE_KEY, + VERSION_ATTRIBUTE_KEY, + ]; + + let tests: Vec = vec![ + Test { + kind: IbcEventType::OpenInitChannel, + event: OpenInit::new( + port_id.clone(), + channel_id.clone(), + counterparty_port_id.clone(), + connection_id.clone(), + version.clone(), + ) + .into(), + expected_keys: expected_keys.clone(), + }, + Test { + kind: IbcEventType::OpenTryChannel, + event: OpenTry::new( + port_id.clone(), + channel_id.clone(), + counterparty_port_id.clone(), + counterparty_channel_id.clone(), + connection_id.clone(), + version, + ) + .into(), + expected_keys: expected_keys.clone(), + }, + Test { + kind: IbcEventType::OpenAckChannel, + event: OpenAck::new( + port_id.clone(), + channel_id.clone(), + counterparty_port_id.clone(), + counterparty_channel_id.clone(), + connection_id.clone(), + ) + .into(), + expected_keys: expected_keys[0..5].to_vec(), + }, + Test { + kind: IbcEventType::OpenConfirmChannel, + event: OpenConfirm::new( + port_id.clone(), + channel_id.clone(), + counterparty_port_id.clone(), + counterparty_channel_id.clone(), + connection_id.clone(), + ) + .into(), + expected_keys: expected_keys[0..5].to_vec(), + }, + Test { + kind: IbcEventType::CloseInitChannel, + event: CloseInit::new( + port_id.clone(), + channel_id.clone(), + counterparty_port_id.clone(), + counterparty_channel_id.clone(), + connection_id.clone(), + ) + .into(), + expected_keys: expected_keys[0..5].to_vec(), + }, + Test { + kind: IbcEventType::CloseConfirmChannel, + event: CloseConfirm::new( + port_id, + channel_id, + counterparty_port_id, + counterparty_channel_id, + connection_id, + ) + .into(), + expected_keys: expected_keys[0..5].to_vec(), + }, + ]; + + for t in tests { + assert_eq!(t.kind.as_str(), t.event.kind); + assert_eq!(t.expected_keys.len(), t.event.attributes.len()); + for (i, key) in t.expected_keys.iter().enumerate() { + assert_eq!(t.event.attributes[i].key, *key); + } + } + } +} diff --git a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs index 05f75e2ef..09c076ec6 100644 --- a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs +++ b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs @@ -8,14 +8,13 @@ use crate::core::{ ics24_host::identifier::{ChannelId, ConnectionId, PortId}, }; -const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; -const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; -const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; -/// This attribute key is public so that OpenInit can use it to convert itself -/// to an `AbciEvent` -pub const COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY: &str = "counterparty_channel_id"; -const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; -const VERSION_ATTRIBUTE_KEY: &str = "version"; +pub(super) const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; +pub(super) const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; +pub(super) const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; +pub(super) const COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY: &str = "counterparty_channel_id"; +pub(super) const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; +pub(super) const VERSION_ATTRIBUTE_KEY: &str = "version"; + #[cfg_attr( feature = "parity-scale-codec", derive( From b2839f4816ed40202e2e9ffb40de29f563803ed0 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 20 Jan 2023 09:04:01 -0800 Subject: [PATCH 2/8] Add changelog entry --- .../unreleased/improvements/163-verify-abci-event-keys.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/163-verify-abci-event-keys.md diff --git a/.changelog/unreleased/improvements/163-verify-abci-event-keys.md b/.changelog/unreleased/improvements/163-verify-abci-event-keys.md new file mode 100644 index 000000000..3a3e34a28 --- /dev/null +++ b/.changelog/unreleased/improvements/163-verify-abci-event-keys.md @@ -0,0 +1,2 @@ +- Add tests to verify `AbciEvent` keys match the expected keys of Ibc events +([#163](https://github.com/cosmos/ibc-rs/issues/163)). \ No newline at end of file From 9d0021d953e7299dcb951ae51cb2c9e020eb716f Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 20 Jan 2023 09:48:03 -0800 Subject: [PATCH 3/8] Add expected value checks --- .../163-verify-abci-event-keys.md | 2 -- .../163-verify-ibc-to-abci-event.md | 2 ++ crates/ibc/src/core/ics02_client/events.rs | 31 +++++++++++++++-- .../ibc/src/core/ics03_connection/events.rs | 30 +++++++++++++++- crates/ibc/src/core/ics04_channel/events.rs | 34 ++++++++++++++++++- 5 files changed, 93 insertions(+), 6 deletions(-) delete mode 100644 .changelog/unreleased/improvements/163-verify-abci-event-keys.md create mode 100644 .changelog/unreleased/improvements/163-verify-ibc-to-abci-event.md diff --git a/.changelog/unreleased/improvements/163-verify-abci-event-keys.md b/.changelog/unreleased/improvements/163-verify-abci-event-keys.md deleted file mode 100644 index 3a3e34a28..000000000 --- a/.changelog/unreleased/improvements/163-verify-abci-event-keys.md +++ /dev/null @@ -1,2 +0,0 @@ -- Add tests to verify `AbciEvent` keys match the expected keys of Ibc events -([#163](https://github.com/cosmos/ibc-rs/issues/163)). \ No newline at end of file diff --git a/.changelog/unreleased/improvements/163-verify-ibc-to-abci-event.md b/.changelog/unreleased/improvements/163-verify-ibc-to-abci-event.md new file mode 100644 index 000000000..34de06b1d --- /dev/null +++ b/.changelog/unreleased/improvements/163-verify-ibc-to-abci-event.md @@ -0,0 +1,2 @@ +- Add tests to verify `AbciEvent` match the expected Ibc events +([#163](https://github.com/cosmos/ibc-rs/issues/163)). \ No newline at end of file diff --git a/crates/ibc/src/core/ics02_client/events.rs b/crates/ibc/src/core/ics02_client/events.rs index 3f51bb984..a72a2932c 100644 --- a/crates/ibc/src/core/ics02_client/events.rs +++ b/crates/ibc/src/core/ics02_client/events.rs @@ -394,7 +394,7 @@ impl From for abci::Event { #[cfg(test)] mod tests { use super::*; - use crate::mock::client_state::client_type as mock_client_type; + use crate::mock::client_state::{client_type as mock_client_type, MOCK_CLIENT_TYPE}; use crate::mock::header::MockHeader; use ibc_proto::google::protobuf::Any; use tendermint::abci::Event as AbciEvent; @@ -405,6 +405,7 @@ mod tests { kind: IbcEventType, event: AbciEvent, expected_keys: Vec<&'static str>, + expected_values: Vec, } let client_type = mock_client_type(); @@ -419,6 +420,17 @@ mod tests { CONSENSUS_HEIGHTS_ATTRIBUTE_KEY, HEADER_ATTRIBUTE_KEY, ]; + let expected_values = vec![ + client_id.to_string(), + MOCK_CLIENT_TYPE.to_string(), + consensus_height.to_string(), + consensus_heights + .iter() + .map(|h| h.to_string()) + .collect::>() + .join(","), + String::from_utf8(hex::encode(header.clone().value)).unwrap(), + ]; let tests: Vec = vec![ Test { @@ -426,6 +438,7 @@ mod tests { event: CreateClient::new(client_id.clone(), client_type.clone(), consensus_height) .into(), expected_keys: expected_keys[0..3].to_vec(), + expected_values: expected_values[0..3].to_vec(), }, Test { kind: IbcEventType::UpdateClient, @@ -438,17 +451,20 @@ mod tests { ) .into(), expected_keys: expected_keys.clone(), + expected_values: expected_values.clone(), }, Test { kind: IbcEventType::UpgradeClient, event: UpgradeClient::new(client_id.clone(), client_type.clone(), consensus_height) .into(), expected_keys: expected_keys[0..3].to_vec(), + expected_values: expected_values[0..3].to_vec(), }, Test { kind: IbcEventType::ClientMisbehaviour, event: ClientMisbehaviour::new(client_id, client_type).into(), expected_keys: expected_keys[0..2].to_vec(), + expected_values: expected_values[0..2].to_vec(), }, ]; @@ -456,7 +472,18 @@ mod tests { assert_eq!(t.event.kind, t.kind.as_str()); assert_eq!(t.expected_keys.len(), t.event.attributes.len()); for (i, key) in t.expected_keys.iter().enumerate() { - assert_eq!(t.event.attributes[i].key, *key); + assert_eq!( + t.event.attributes[i].key, *key, + "key mismatch for {:?}", + t.kind + ); + } + for (i, value) in t.expected_values.iter().enumerate() { + assert_eq!( + t.event.attributes[i].value, *value, + "value mismatch for {:?}", + t.kind + ); } } } diff --git a/crates/ibc/src/core/ics03_connection/events.rs b/crates/ibc/src/core/ics03_connection/events.rs index a79726d4a..ba5e1933a 100644 --- a/crates/ibc/src/core/ics03_connection/events.rs +++ b/crates/ibc/src/core/ics03_connection/events.rs @@ -294,6 +294,7 @@ mod tests { kind: IbcEventType, event: AbciEvent, expected_keys: Vec<&'static str>, + expected_values: Vec, } let conn_id_on_a = ConnectionId::default(); @@ -306,6 +307,12 @@ mod tests { COUNTERPARTY_CLIENT_ID_ATTRIBUTE_KEY, COUNTERPARTY_CONN_ID_ATTRIBUTE_KEY, ]; + let expected_values = vec![ + conn_id_on_a.to_string(), + client_id_on_a.to_string(), + client_id_on_b.to_string(), + conn_id_on_b.to_string(), + ]; let tests: Vec = vec![ Test { @@ -317,6 +324,11 @@ mod tests { ) .into(), expected_keys: expected_keys.clone(), + expected_values: expected_values + .iter() + .enumerate() + .map(|(i, v)| if i == 3 { "".to_string() } else { v.clone() }) + .collect(), }, Test { kind: IbcEventType::OpenTryConnection, @@ -328,6 +340,7 @@ mod tests { ) .into(), expected_keys: expected_keys.clone(), + expected_values: expected_values.iter().rev().cloned().collect(), }, Test { kind: IbcEventType::OpenAckConnection, @@ -339,12 +352,14 @@ mod tests { ) .into(), expected_keys: expected_keys.clone(), + expected_values: expected_values.clone(), }, Test { kind: IbcEventType::OpenConfirmConnection, event: OpenConfirm::new(conn_id_on_b, client_id_on_b, conn_id_on_a, client_id_on_a) .into(), expected_keys: expected_keys.clone(), + expected_values: expected_values.iter().rev().cloned().collect(), }, ]; @@ -352,7 +367,20 @@ mod tests { assert_eq!(t.kind.as_str(), t.event.kind); assert_eq!(t.expected_keys.len(), t.event.attributes.len()); for (i, key) in t.expected_keys.iter().enumerate() { - assert_eq!(t.event.attributes[i].key, *key); + assert_eq!( + t.event.attributes[i].key, + *key, + "key mismatch for {:?}", + t.kind.as_str() + ); + } + for (i, value) in t.expected_values.iter().enumerate() { + assert_eq!( + t.event.attributes[i].value, + *value, + "value mismatch for {:?}", + t.kind.as_str() + ); } } } diff --git a/crates/ibc/src/core/ics04_channel/events.rs b/crates/ibc/src/core/ics04_channel/events.rs index 355cb49bd..ffd5023e3 100644 --- a/crates/ibc/src/core/ics04_channel/events.rs +++ b/crates/ibc/src/core/ics04_channel/events.rs @@ -1075,6 +1075,7 @@ mod tests { kind: IbcEventType, event: AbciEvent, expected_keys: Vec<&'static str>, + expected_values: Vec, } let port_id = PortId::default(); @@ -1091,6 +1092,14 @@ mod tests { CONNECTION_ID_ATTRIBUTE_KEY, VERSION_ATTRIBUTE_KEY, ]; + let expected_values = vec![ + port_id.to_string(), + channel_id.to_string(), + counterparty_port_id.to_string(), + counterparty_channel_id.to_string(), + connection_id.to_string(), + version.to_string(), + ]; let tests: Vec = vec![ Test { @@ -1104,6 +1113,11 @@ mod tests { ) .into(), expected_keys: expected_keys.clone(), + expected_values: expected_values + .iter() + .enumerate() + .map(|(i, v)| if i == 3 { "".to_string() } else { v.clone() }) + .collect(), }, Test { kind: IbcEventType::OpenTryChannel, @@ -1117,6 +1131,7 @@ mod tests { ) .into(), expected_keys: expected_keys.clone(), + expected_values: expected_values.clone(), }, Test { kind: IbcEventType::OpenAckChannel, @@ -1129,6 +1144,7 @@ mod tests { ) .into(), expected_keys: expected_keys[0..5].to_vec(), + expected_values: expected_values[0..5].to_vec(), }, Test { kind: IbcEventType::OpenConfirmChannel, @@ -1141,6 +1157,7 @@ mod tests { ) .into(), expected_keys: expected_keys[0..5].to_vec(), + expected_values: expected_values[0..5].to_vec(), }, Test { kind: IbcEventType::CloseInitChannel, @@ -1153,6 +1170,7 @@ mod tests { ) .into(), expected_keys: expected_keys[0..5].to_vec(), + expected_values: expected_values[0..5].to_vec(), }, Test { kind: IbcEventType::CloseConfirmChannel, @@ -1165,6 +1183,7 @@ mod tests { ) .into(), expected_keys: expected_keys[0..5].to_vec(), + expected_values: expected_values[0..5].to_vec(), }, ]; @@ -1172,7 +1191,20 @@ mod tests { assert_eq!(t.kind.as_str(), t.event.kind); assert_eq!(t.expected_keys.len(), t.event.attributes.len()); for (i, key) in t.expected_keys.iter().enumerate() { - assert_eq!(t.event.attributes[i].key, *key); + assert_eq!( + t.event.attributes[i].key, + *key, + "key mismatch for {:?}", + t.kind.as_str() + ); + } + for (i, value) in t.expected_values.iter().enumerate() { + assert_eq!( + t.event.attributes[i].value, + *value, + "value mismatch for {:?}", + t.kind.as_str() + ); } } } From ffa0bbcf972b93e074829390cb9cd3511eae23f9 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Mon, 23 Jan 2023 13:31:41 -0800 Subject: [PATCH 4/8] Hardcode attribute key constants --- crates/ibc/src/core/ics02_client/events.rs | 14 +++++++------- crates/ibc/src/core/ics03_connection/events.rs | 8 ++++---- crates/ibc/src/core/ics04_channel/events.rs | 17 ++++++----------- .../ics04_channel/events/channel_attributes.rs | 12 +++++++----- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/events.rs b/crates/ibc/src/core/ics02_client/events.rs index a72a2932c..e444e7d09 100644 --- a/crates/ibc/src/core/ics02_client/events.rs +++ b/crates/ibc/src/core/ics02_client/events.rs @@ -394,7 +394,7 @@ impl From for abci::Event { #[cfg(test)] mod tests { use super::*; - use crate::mock::client_state::{client_type as mock_client_type, MOCK_CLIENT_TYPE}; + use crate::mock::client_state::client_type as mock_client_type; use crate::mock::header::MockHeader; use ibc_proto::google::protobuf::Any; use tendermint::abci::Event as AbciEvent; @@ -414,15 +414,15 @@ mod tests { let consensus_heights = vec![Height::new(0, 5).unwrap(), Height::new(0, 7).unwrap()]; let header: Any = MockHeader::new(consensus_height).into(); let expected_keys = vec![ - CLIENT_ID_ATTRIBUTE_KEY, - CLIENT_TYPE_ATTRIBUTE_KEY, - CONSENSUS_HEIGHT_ATTRIBUTE_KEY, - CONSENSUS_HEIGHTS_ATTRIBUTE_KEY, - HEADER_ATTRIBUTE_KEY, + "client_id", + "client_type", + "consensus_height", + "consensus_heights", + "header", ]; let expected_values = vec![ client_id.to_string(), - MOCK_CLIENT_TYPE.to_string(), + "9999-mock".to_string(), consensus_height.to_string(), consensus_heights .iter() diff --git a/crates/ibc/src/core/ics03_connection/events.rs b/crates/ibc/src/core/ics03_connection/events.rs index ba5e1933a..afbfbdbb9 100644 --- a/crates/ibc/src/core/ics03_connection/events.rs +++ b/crates/ibc/src/core/ics03_connection/events.rs @@ -302,10 +302,10 @@ mod tests { let conn_id_on_b = ConnectionId::new(1); let client_id_on_b = ClientId::new(mock_client_type(), 0).unwrap(); let expected_keys = vec![ - CONN_ID_ATTRIBUTE_KEY, - CLIENT_ID_ATTRIBUTE_KEY, - COUNTERPARTY_CLIENT_ID_ATTRIBUTE_KEY, - COUNTERPARTY_CONN_ID_ATTRIBUTE_KEY, + "connection_id", + "client_id", + "counterparty_client_id", + "counterparty_connection_id", ]; let expected_values = vec![ conn_id_on_a.to_string(), diff --git a/crates/ibc/src/core/ics04_channel/events.rs b/crates/ibc/src/core/ics04_channel/events.rs index ffd5023e3..a2525c82e 100644 --- a/crates/ibc/src/core/ics04_channel/events.rs +++ b/crates/ibc/src/core/ics04_channel/events.rs @@ -1062,11 +1062,6 @@ impl TryFrom for abci::Event { #[cfg(test)] mod tests { use super::*; - use channel_attributes::{ - CHANNEL_ID_ATTRIBUTE_KEY, CONNECTION_ID_ATTRIBUTE_KEY, - COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY, COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY, - PORT_ID_ATTRIBUTE_KEY, VERSION_ATTRIBUTE_KEY, - }; use tendermint::abci::Event as AbciEvent; #[test] @@ -1085,12 +1080,12 @@ mod tests { let counterparty_channel_id = ChannelId::new(1); let version = Version::default(); let expected_keys = vec![ - PORT_ID_ATTRIBUTE_KEY, - CHANNEL_ID_ATTRIBUTE_KEY, - COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY, - COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY, - CONNECTION_ID_ATTRIBUTE_KEY, - VERSION_ATTRIBUTE_KEY, + "port_id", + "channel_id", + "counterparty_port_id", + "counterparty_channel_id", + "connection_id", + "version", ]; let expected_values = vec![ port_id.to_string(), diff --git a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs index 09c076ec6..742f7dcf6 100644 --- a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs +++ b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs @@ -8,12 +8,14 @@ use crate::core::{ ics24_host::identifier::{ChannelId, ConnectionId, PortId}, }; -pub(super) const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; -pub(super) const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; -pub(super) const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; +pub const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; +pub const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; +pub const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; +/// This attribute key is public so that OpenInit can use it to convert itself +/// to an `AbciEvent` pub(super) const COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY: &str = "counterparty_channel_id"; -pub(super) const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; -pub(super) const VERSION_ATTRIBUTE_KEY: &str = "version"; +pub const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; +pub const VERSION_ATTRIBUTE_KEY: &str = "version"; #[cfg_attr( feature = "parity-scale-codec", From 817a44035a3e35bb04a9f22d7cb86894b54cc9f7 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Mon, 23 Jan 2023 13:35:19 -0800 Subject: [PATCH 5/8] Remove pub before channel attr key consts --- .../core/ics04_channel/events/channel_attributes.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs index 742f7dcf6..e858527bb 100644 --- a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs +++ b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs @@ -8,14 +8,14 @@ use crate::core::{ ics24_host::identifier::{ChannelId, ConnectionId, PortId}, }; -pub const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; -pub const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; -pub const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; +const CONNECTION_ID_ATTRIBUTE_KEY: &str = "connection_id"; +const CHANNEL_ID_ATTRIBUTE_KEY: &str = "channel_id"; +const PORT_ID_ATTRIBUTE_KEY: &str = "port_id"; /// This attribute key is public so that OpenInit can use it to convert itself /// to an `AbciEvent` pub(super) const COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY: &str = "counterparty_channel_id"; -pub const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; -pub const VERSION_ATTRIBUTE_KEY: &str = "version"; +const COUNTERPARTY_PORT_ID_ATTRIBUTE_KEY: &str = "counterparty_port_id"; +const VERSION_ATTRIBUTE_KEY: &str = "version"; #[cfg_attr( feature = "parity-scale-codec", From dc20ce72c523360cd693a2af000146f728cbbc5c Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Mon, 23 Jan 2023 17:16:49 -0800 Subject: [PATCH 6/8] Hardcode expected values --- crates/ibc/src/core/ics02_client/events.rs | 19 ++++--------------- .../ibc/src/core/ics03_connection/events.rs | 19 ++++++++++--------- crates/ibc/src/core/ics04_channel/events.rs | 18 +++++++++--------- 3 files changed, 23 insertions(+), 33 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/events.rs b/crates/ibc/src/core/ics02_client/events.rs index e444e7d09..de3a8db42 100644 --- a/crates/ibc/src/core/ics02_client/events.rs +++ b/crates/ibc/src/core/ics02_client/events.rs @@ -394,7 +394,6 @@ impl From for abci::Event { #[cfg(test)] mod tests { use super::*; - use crate::mock::client_state::client_type as mock_client_type; use crate::mock::header::MockHeader; use ibc_proto::google::protobuf::Any; use tendermint::abci::Event as AbciEvent; @@ -405,12 +404,12 @@ mod tests { kind: IbcEventType, event: AbciEvent, expected_keys: Vec<&'static str>, - expected_values: Vec, + expected_values: Vec<&'static str>, } - let client_type = mock_client_type(); + let client_type = ClientType::new("07-tendermint".to_string()); let client_id = ClientId::new(client_type.clone(), 0).unwrap(); - let consensus_height = Height::new(0, 10).unwrap(); + let consensus_height = Height::new(0, 5).unwrap(); let consensus_heights = vec![Height::new(0, 5).unwrap(), Height::new(0, 7).unwrap()]; let header: Any = MockHeader::new(consensus_height).into(); let expected_keys = vec![ @@ -420,17 +419,7 @@ mod tests { "consensus_heights", "header", ]; - let expected_values = vec![ - client_id.to_string(), - "9999-mock".to_string(), - consensus_height.to_string(), - consensus_heights - .iter() - .map(|h| h.to_string()) - .collect::>() - .join(","), - String::from_utf8(hex::encode(header.clone().value)).unwrap(), - ]; + let expected_values = vec!["07-tendermint-0", "07-tendermint", "0-5", "0-5,0-7"]; let tests: Vec = vec![ Test { diff --git a/crates/ibc/src/core/ics03_connection/events.rs b/crates/ibc/src/core/ics03_connection/events.rs index afbfbdbb9..e9839558c 100644 --- a/crates/ibc/src/core/ics03_connection/events.rs +++ b/crates/ibc/src/core/ics03_connection/events.rs @@ -285,7 +285,7 @@ impl From for abci::Event { #[cfg(test)] mod tests { use super::*; - use crate::mock::client_state::client_type as mock_client_type; + use crate::core::ics02_client::client_type::ClientType; use tendermint::abci::Event as AbciEvent; #[test] @@ -294,13 +294,14 @@ mod tests { kind: IbcEventType, event: AbciEvent, expected_keys: Vec<&'static str>, - expected_values: Vec, + expected_values: Vec<&'static str>, } + let client_type = ClientType::new("07-tendermint".to_string()); let conn_id_on_a = ConnectionId::default(); - let client_id_on_a = ClientId::default(); + let client_id_on_a = ClientId::new(client_type.clone(), 0).unwrap(); let conn_id_on_b = ConnectionId::new(1); - let client_id_on_b = ClientId::new(mock_client_type(), 0).unwrap(); + let client_id_on_b = ClientId::new(client_type, 1).unwrap(); let expected_keys = vec![ "connection_id", "client_id", @@ -308,10 +309,10 @@ mod tests { "counterparty_connection_id", ]; let expected_values = vec![ - conn_id_on_a.to_string(), - client_id_on_a.to_string(), - client_id_on_b.to_string(), - conn_id_on_b.to_string(), + "connection-0", + "07-tendermint-0", + "07-tendermint-1", + "connection-1", ]; let tests: Vec = vec![ @@ -327,7 +328,7 @@ mod tests { expected_values: expected_values .iter() .enumerate() - .map(|(i, v)| if i == 3 { "".to_string() } else { v.clone() }) + .map(|(i, v)| if i == 3 { "" } else { v }) .collect(), }, Test { diff --git a/crates/ibc/src/core/ics04_channel/events.rs b/crates/ibc/src/core/ics04_channel/events.rs index a2525c82e..ed7522bfa 100644 --- a/crates/ibc/src/core/ics04_channel/events.rs +++ b/crates/ibc/src/core/ics04_channel/events.rs @@ -1070,7 +1070,7 @@ mod tests { kind: IbcEventType, event: AbciEvent, expected_keys: Vec<&'static str>, - expected_values: Vec, + expected_values: Vec<&'static str>, } let port_id = PortId::default(); @@ -1078,7 +1078,7 @@ mod tests { let connection_id = ConnectionId::default(); let counterparty_port_id = PortId::default(); let counterparty_channel_id = ChannelId::new(1); - let version = Version::default(); + let version = Version::new("ics20".to_string()); let expected_keys = vec![ "port_id", "channel_id", @@ -1088,12 +1088,12 @@ mod tests { "version", ]; let expected_values = vec![ - port_id.to_string(), - channel_id.to_string(), - counterparty_port_id.to_string(), - counterparty_channel_id.to_string(), - connection_id.to_string(), - version.to_string(), + "defaultPort", + "channel-0", + "defaultPort", + "channel-1", + "connection-0", + "ics20", ]; let tests: Vec = vec![ @@ -1111,7 +1111,7 @@ mod tests { expected_values: expected_values .iter() .enumerate() - .map(|(i, v)| if i == 3 { "".to_string() } else { v.clone() }) + .map(|(i, v)| if i == 3 { "" } else { v }) .collect(), }, Test { From 13b434a433a09ed8182c3ce486fab938812f9c8d Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Mon, 23 Jan 2023 20:13:23 -0800 Subject: [PATCH 7/8] Fix version const --- crates/ibc/src/core/ics04_channel/events.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/events.rs b/crates/ibc/src/core/ics04_channel/events.rs index ed7522bfa..b937c78cf 100644 --- a/crates/ibc/src/core/ics04_channel/events.rs +++ b/crates/ibc/src/core/ics04_channel/events.rs @@ -1078,7 +1078,7 @@ mod tests { let connection_id = ConnectionId::default(); let counterparty_port_id = PortId::default(); let counterparty_channel_id = ChannelId::new(1); - let version = Version::new("ics20".to_string()); + let version = Version::new("ics20-1".to_string()); let expected_keys = vec![ "port_id", "channel_id", @@ -1093,7 +1093,7 @@ mod tests { "defaultPort", "channel-1", "connection-0", - "ics20", + "ics20-1", ]; let tests: Vec = vec![ From 2354ef54857f3bdfb5636fefbf2832ec95a4532f Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 24 Jan 2023 09:09:05 -0800 Subject: [PATCH 8/8] Move away from default + Fix header issue --- crates/ibc/src/core/ics02_client/events.rs | 26 ++++++++++++------- .../ibc/src/core/ics03_connection/events.rs | 12 ++++----- crates/ibc/src/core/ics04_channel/events.rs | 25 +++++++++--------- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/events.rs b/crates/ibc/src/core/ics02_client/events.rs index de3a8db42..a3237b609 100644 --- a/crates/ibc/src/core/ics02_client/events.rs +++ b/crates/ibc/src/core/ics02_client/events.rs @@ -395,6 +395,7 @@ impl From for abci::Event { mod tests { use super::*; use crate::mock::header::MockHeader; + use crate::timestamp::Timestamp; use ibc_proto::google::protobuf::Any; use tendermint::abci::Event as AbciEvent; @@ -411,7 +412,9 @@ mod tests { let client_id = ClientId::new(client_type.clone(), 0).unwrap(); let consensus_height = Height::new(0, 5).unwrap(); let consensus_heights = vec![Height::new(0, 5).unwrap(), Height::new(0, 7).unwrap()]; - let header: Any = MockHeader::new(consensus_height).into(); + let header: Any = MockHeader::new(consensus_height) + .with_timestamp(Timestamp::none()) + .into(); let expected_keys = vec![ "client_id", "client_type", @@ -419,7 +422,14 @@ mod tests { "consensus_heights", "header", ]; - let expected_values = vec!["07-tendermint-0", "07-tendermint", "0-5", "0-5,0-7"]; + + let expected_values = vec![ + "07-tendermint-0", + "07-tendermint", + "0-5", + "0-5,0-7", + "0a021005", + ]; let tests: Vec = vec![ Test { @@ -460,16 +470,12 @@ mod tests { for t in tests { assert_eq!(t.event.kind, t.kind.as_str()); assert_eq!(t.expected_keys.len(), t.event.attributes.len()); - for (i, key) in t.expected_keys.iter().enumerate() { - assert_eq!( - t.event.attributes[i].key, *key, - "key mismatch for {:?}", - t.kind - ); + for (i, e) in t.event.attributes.iter().enumerate() { + assert_eq!(e.key, t.expected_keys[i], "key mismatch for {:?}", t.kind); } - for (i, value) in t.expected_values.iter().enumerate() { + for (i, e) in t.event.attributes.iter().enumerate() { assert_eq!( - t.event.attributes[i].value, *value, + e.value, t.expected_values[i], "value mismatch for {:?}", t.kind ); diff --git a/crates/ibc/src/core/ics03_connection/events.rs b/crates/ibc/src/core/ics03_connection/events.rs index e9839558c..d25558e43 100644 --- a/crates/ibc/src/core/ics03_connection/events.rs +++ b/crates/ibc/src/core/ics03_connection/events.rs @@ -367,18 +367,18 @@ mod tests { for t in tests { assert_eq!(t.kind.as_str(), t.event.kind); assert_eq!(t.expected_keys.len(), t.event.attributes.len()); - for (i, key) in t.expected_keys.iter().enumerate() { + for (i, e) in t.event.attributes.iter().enumerate() { assert_eq!( - t.event.attributes[i].key, - *key, + e.key, + t.expected_keys[i], "key mismatch for {:?}", t.kind.as_str() ); } - for (i, value) in t.expected_values.iter().enumerate() { + for (i, e) in t.event.attributes.iter().enumerate() { assert_eq!( - t.event.attributes[i].value, - *value, + e.value, + t.expected_values[i], "value mismatch for {:?}", t.kind.as_str() ); diff --git a/crates/ibc/src/core/ics04_channel/events.rs b/crates/ibc/src/core/ics04_channel/events.rs index b937c78cf..77e531836 100644 --- a/crates/ibc/src/core/ics04_channel/events.rs +++ b/crates/ibc/src/core/ics04_channel/events.rs @@ -1073,10 +1073,10 @@ mod tests { expected_values: Vec<&'static str>, } - let port_id = PortId::default(); - let channel_id = ChannelId::default(); - let connection_id = ConnectionId::default(); - let counterparty_port_id = PortId::default(); + let port_id = PortId::transfer(); + let channel_id = ChannelId::new(0); + let connection_id = ConnectionId::new(0); + let counterparty_port_id = PortId::transfer(); let counterparty_channel_id = ChannelId::new(1); let version = Version::new("ics20-1".to_string()); let expected_keys = vec![ @@ -1088,9 +1088,9 @@ mod tests { "version", ]; let expected_values = vec![ - "defaultPort", + "transfer", "channel-0", - "defaultPort", + "transfer", "channel-1", "connection-0", "ics20-1", @@ -1185,18 +1185,19 @@ mod tests { for t in tests { assert_eq!(t.kind.as_str(), t.event.kind); assert_eq!(t.expected_keys.len(), t.event.attributes.len()); - for (i, key) in t.expected_keys.iter().enumerate() { + for (i, e) in t.event.attributes.iter().enumerate() { assert_eq!( - t.event.attributes[i].key, - *key, + e.key, + t.expected_keys[i], "key mismatch for {:?}", t.kind.as_str() ); } - for (i, value) in t.expected_values.iter().enumerate() { + assert_eq!(t.expected_values.len(), t.event.attributes.len()); + for (i, e) in t.event.attributes.iter().enumerate() { assert_eq!( - t.event.attributes[i].value, - *value, + e.value, + t.expected_values[i], "value mismatch for {:?}", t.kind.as_str() );