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

Jump to bottom

Add tests to verify AbciEvent match emitted events #363

Merged
merged 8 commits into from
Jan 24, 2023
Merged

Add tests to verify AbciEvent match emitted events #363

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
392155c
Verify AbciEvent keys match emitted events
Farhad-Shabani Jan 20, 2023
b2839f4
Add changelog entry
Farhad-Shabani Jan 20, 2023
9d0021d
Add expected value checks
Farhad-Shabani Jan 20, 2023
ffa0bbc
Hardcode attribute key constants
Farhad-Shabani Jan 23, 2023
817a440
Remove pub before channel attr key consts
Farhad-Shabani Jan 23, 2023
dc20ce7
Hardcode expected values
Farhad-Shabani Jan 24, 2023
13b434a
Fix version const
Farhad-Shabani Jan 24, 2023
2354ef5
Move away from default + Fix header issue
Farhad-Shabani Jan 24, 2023
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 .changelog/unreleased/improvements/163-verify-ibc-to-abci-event.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add tests to verify `AbciEvent` match the expected Ibc events
([#163](https://github.com/cosmos/ibc-rs/issues/163)).
87 changes: 87 additions & 0 deletions crates/ibc/src/core/ics02_client/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,3 +390,90 @@ impl From<UpgradeClient> for abci::Event {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
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>,
expected_values: Vec<&'static str>,
}

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, 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![
"client_id",
"client_type",
"consensus_height",
"consensus_heights",
"header",
];
let expected_values = vec!["07-tendermint-0", "07-tendermint", "0-5", "0-5,0-7"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, we're still missing the value for header

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, weird. let me fix it

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed, this header's expected value is checked using MockHeaderhere. In order to verify it later against an emitted ibc-go event, a PR has been opened (#374)


let tests: Vec<Test> = 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(),
expected_values: expected_values[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(),
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(),
},
];

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, value) in t.expected_values.iter().enumerate() {
assert_eq!(
t.event.attributes[i].value, *value,
"value mismatch for {:?}",
t.kind
);
}
}
}
}
105 changes: 105 additions & 0 deletions crates/ibc/src/core/ics03_connection/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,3 +281,108 @@ impl From<OpenConfirm> for abci::Event {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::core::ics02_client::client_type::ClientType;
use tendermint::abci::Event as AbciEvent;

#[test]
fn ibc_to_abci_connection_events() {
struct Test {
kind: IbcEventType,
event: AbciEvent,
expected_keys: Vec<&'static str>,
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::new(client_type.clone(), 0).unwrap();
let conn_id_on_b = ConnectionId::new(1);
let client_id_on_b = ClientId::new(client_type, 1).unwrap();
let expected_keys = vec![
"connection_id",
"client_id",
"counterparty_client_id",
"counterparty_connection_id",
];
let expected_values = vec![
"connection-0",
"07-tendermint-0",
"07-tendermint-1",
"connection-1",
];

let tests: Vec<Test> = 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(),
expected_values: expected_values
.iter()
.enumerate()
.map(|(i, v)| if i == 3 { "" } else { v })
.collect(),
},
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(),
expected_values: expected_values.iter().rev().cloned().collect(),
},
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(),
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(),
},
];

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,
"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()
);
}
}
}
}
146 changes: 146 additions & 0 deletions crates/ibc/src/core/ics04_channel/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,3 +1058,149 @@ impl TryFrom<TimeoutPacket> for abci::Event {
})
}
}

#[cfg(test)]
mod tests {
use super::*;
use tendermint::abci::Event as AbciEvent;

#[test]
fn ibc_to_abci_channel_events() {
struct Test {
kind: IbcEventType,
event: AbciEvent,
expected_keys: Vec<&'static str>,
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();
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not use default() here too; we can just use new(). I don't like the use of default() in tests as they make it harder to verify correctness quickly (I need to go check the implementation for default() for each of these), for little gain.

let counterparty_channel_id = ChannelId::new(1);
let version = Version::new("ics20-1".to_string());
let expected_keys = vec![
"port_id",
"channel_id",
"counterparty_port_id",
"counterparty_channel_id",
"connection_id",
"version",
];
let expected_values = vec![
"defaultPort",
"channel-0",
"defaultPort",
"channel-1",
"connection-0",
"ics20-1",
];

let tests: Vec<Test> = 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(),
expected_values: expected_values
.iter()
.enumerate()
.map(|(i, v)| if i == 3 { "" } else { v })
.collect(),
},
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(),
expected_values: expected_values.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(),
expected_values: expected_values[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(),
expected_values: expected_values[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(),
expected_values: expected_values[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(),
expected_values: expected_values[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,
"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()
);
}
}
}
}
3 changes: 2 additions & 1 deletion crates/ibc/src/core/ics04_channel/events/channel_attributes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ 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";
pub(super) 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";

#[cfg_attr(
feature = "parity-scale-codec",
derive(
Expand Down