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

Remove height information from IbcEvent #2542

Merged
merged 32 commits into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d8342ae
Introduce IbcEventWithHeight
plafer Aug 9, 2022
64fc4c2
Remove IbcEvent height() and set_height()
plafer Aug 9, 2022
2c01045
before AbciEvent -> Attributes refactor
plafer Aug 9, 2022
2e85d49
stop using try_from_tx in favor of IbcEvent::try_from
plafer Aug 10, 2022
d7348b3
Client TryFrom AbciEvent done
plafer Aug 10, 2022
de5cc19
TryFrom AbciEvent for connection
plafer Aug 10, 2022
753e8e6
AbciEvent -> IbcEvent scaffolding done
plafer Aug 10, 2022
f0a770c
Client Abci conversion done
plafer Aug 10, 2022
1eb93d2
connection abci -> IbcEvent done
plafer Aug 10, 2022
9bb944b
channel abci conversion done
plafer Aug 10, 2022
28c3162
Before TxSyncResult use IbcEventWithHeight
plafer Aug 10, 2022
ea5d134
Remove client height
plafer Aug 11, 2022
24d7fc2
Remove height from connection
plafer Aug 11, 2022
7b49fdc
channel heights removed
plafer Aug 11, 2022
cdb5e51
Move tests to modules
plafer Aug 11, 2022
bb1335e
move connection tests to modules
plafer Aug 11, 2022
900721b
move channel tests to modules
plafer Aug 11, 2022
d1449bf
fix bug
plafer Aug 11, 2022
0006402
Merge remote-tracking branch 'origin/master' into plafer/2539-remove-…
plafer Aug 11, 2022
b103150
changelog
plafer Aug 11, 2022
95ff193
Update .changelog/unreleased/improvements/2542-remove-ibcevent-height
plafer Aug 11, 2022
a60e524
fix potential regression
plafer Aug 11, 2022
ab9c8d2
Remove `event()` from IbcEventWithHeight
plafer Aug 11, 2022
083c2ad
Remove `height()` from IbcEventWithHeight
plafer Aug 11, 2022
714b03c
Fix PrettyEvents
plafer Aug 11, 2022
377b60b
fix regression - try 2
plafer Aug 11, 2022
08d2373
cargo doc fix
plafer Aug 11, 2022
785cbad
move AbciEvent -> IbcEvent to relayer
plafer Aug 11, 2022
1bdf27f
Move abci_event -> ibc_event logic to relayer #2
plafer Aug 11, 2022
12484d9
Fix get_all_events()
plafer Aug 11, 2022
5a68865
Merge remote-tracking branch 'origin/master' into plafer/2539-remove-…
plafer Aug 12, 2022
a68d91c
fix e2e tests
plafer Aug 12, 2022
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Remove height attribute from `IbcEvent` and its variants ([#2542](https://github.com/informalsystems/ibc-rs/issues/2542))
51 changes: 6 additions & 45 deletions modules/src/core/ics02_client/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ use crate::core::ics24_host::identifier::ClientId;
use crate::events::{IbcEvent, IbcEventType};
use crate::prelude::*;

/// The content of the `key` field for the attribute containing the height.
pub const HEIGHT_ATTRIBUTE_KEY: &str = "height";

/// The content of the `key` field for the attribute containing the client identifier.
pub const CLIENT_ID_ATTRIBUTE_KEY: &str = "client_id";

Expand Down Expand Up @@ -53,7 +50,6 @@ impl From<NewBlock> for IbcEvent {

#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Attributes {
pub height: Height,
pub client_id: ClientId,
pub client_type: ClientType,
pub consensus_height: Height,
Expand All @@ -62,7 +58,6 @@ pub struct Attributes {
impl Default for Attributes {
fn default() -> Self {
Attributes {
height: Height::new(0, 1).unwrap(),
client_id: Default::default(),
client_type: ClientType::Tendermint,
consensus_height: Height::new(0, 1).unwrap(),
Expand All @@ -79,34 +74,26 @@ impl Default for Attributes {
/// Once tendermint-rs improves the API of the `Key` and `Value` types,
/// we will be able to remove the `.parse().unwrap()` calls.
impl From<Attributes> for Vec<Tag> {
fn from(a: Attributes) -> Self {
let height = Tag {
key: HEIGHT_ATTRIBUTE_KEY.parse().unwrap(),
value: a.height.to_string().parse().unwrap(),
};
fn from(attrs: Attributes) -> Self {
let client_id = Tag {
key: CLIENT_ID_ATTRIBUTE_KEY.parse().unwrap(),
value: a.client_id.to_string().parse().unwrap(),
value: attrs.client_id.to_string().parse().unwrap(),
};
let client_type = Tag {
key: CLIENT_TYPE_ATTRIBUTE_KEY.parse().unwrap(),
value: a.client_type.as_str().parse().unwrap(),
value: attrs.client_type.as_str().parse().unwrap(),
};
let consensus_height = Tag {
key: CONSENSUS_HEIGHT_ATTRIBUTE_KEY.parse().unwrap(),
value: a.height.to_string().parse().unwrap(),
value: attrs.consensus_height.to_string().parse().unwrap(),
};
vec![height, client_id, client_type, consensus_height]
vec![client_id, client_type, consensus_height]
}
}

impl core::fmt::Display for Attributes {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
write!(
f,
"h: {}, cs_h: {}({})",
self.height, self.client_id, self.consensus_height
)
write!(f, "cs_h: {}({})", self.client_id, self.consensus_height)
}
}

Expand All @@ -118,12 +105,6 @@ impl CreateClient {
pub fn client_id(&self) -> &ClientId {
&self.0.client_id
}
pub fn height(&self) -> Height {
self.0.height
}
pub fn set_height(&mut self, height: Height) {
self.0.height = height;
}
}

impl From<Attributes> for CreateClient {
Expand Down Expand Up @@ -170,14 +151,6 @@ impl UpdateClient {
self.common.client_type
}

pub fn height(&self) -> Height {
self.common.height
}

pub fn set_height(&mut self, height: Height) {
self.common.height = height;
}

pub fn consensus_height(&self) -> Height {
self.common.consensus_height
}
Expand Down Expand Up @@ -236,12 +209,6 @@ impl ClientMisbehaviour {
pub fn client_id(&self) -> &ClientId {
&self.0.client_id
}
pub fn height(&self) -> Height {
self.0.height
}
pub fn set_height(&mut self, height: Height) {
self.0.height = height;
}
}

impl From<Attributes> for ClientMisbehaviour {
Expand Down Expand Up @@ -271,12 +238,6 @@ impl From<ClientMisbehaviour> for AbciEvent {
pub struct UpgradeClient(pub Attributes);

impl UpgradeClient {
pub fn set_height(&mut self, height: Height) {
self.0.height = height;
}
pub fn height(&self) -> Height {
self.0.height
}
pub fn client_id(&self) -> &ClientId {
&self.0.client_id
}
Expand Down
5 changes: 0 additions & 5 deletions modules/src/core/ics02_client/handler/create_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ pub fn process(

let event_attributes = Attributes {
client_id,
height: ctx.host_height(),
..Default::default()
};
output.emit(IbcEvent::CreateClient(event_attributes.into()));
Expand All @@ -78,7 +77,6 @@ mod tests {
use crate::core::ics02_client::client_consensus::AnyConsensusState;
use crate::core::ics02_client::client_state::ClientState;
use crate::core::ics02_client::client_type::ClientType;
use crate::core::ics02_client::context::ClientReader;
use crate::core::ics02_client::handler::{dispatch, ClientResult};
use crate::core::ics02_client::msgs::create_client::MsgCreateAnyClient;
use crate::core::ics02_client::msgs::ClientMsg;
Expand Down Expand Up @@ -118,7 +116,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::CreateClient(ref e) if e.client_id() == &expected_client_id)
);
assert_eq!(event.height(), ctx.host_height());
match result {
ClientResult::Create(create_result) => {
assert_eq!(create_result.client_type, ClientType::Mock);
Expand Down Expand Up @@ -187,7 +184,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::CreateClient(ref e) if e.client_id() == &expected_client_id)
);
assert_eq!(event.height(), ctx.host_height());
match result {
ClientResult::Create(create_res) => {
assert_eq!(create_res.client_type, msg.client_state.client_type());
Expand Down Expand Up @@ -251,7 +247,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::CreateClient(ref e) if e.client_id() == &expected_client_id)
);
assert_eq!(event.height(), ctx.host_height());
match result {
ClientResult::Create(create_res) => {
assert_eq!(create_res.client_type, ClientType::Tendermint);
Expand Down
7 changes: 0 additions & 7 deletions modules/src/core/ics02_client/handler/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ pub fn process(

let event_attributes = Attributes {
client_id,
height: ctx.host_height(),
..Default::default()
};
output.emit(IbcEvent::UpdateClient(event_attributes.into()));
Expand All @@ -109,7 +108,6 @@ mod tests {
use crate::core::ics02_client::client_consensus::AnyConsensusState;
use crate::core::ics02_client::client_state::{AnyClientState, ClientState};
use crate::core::ics02_client::client_type::ClientType;
use crate::core::ics02_client::context::ClientReader;
use crate::core::ics02_client::error::{Error, ErrorDetail};
use crate::core::ics02_client::handler::dispatch;
use crate::core::ics02_client::handler::ClientResult::Update;
Expand Down Expand Up @@ -157,7 +155,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
);
assert_eq!(event.height(), ctx.host_height());
assert!(log.is_empty());
// Check the result
match result {
Expand Down Expand Up @@ -241,7 +238,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
);
assert_eq!(event.height(), ctx.host_height());
assert!(log.is_empty());
}
Err(err) => {
Expand Down Expand Up @@ -309,7 +305,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
);
assert_eq!(event.height(), ctx.host_height());
assert!(log.is_empty());
// Check the result
match result {
Expand Down Expand Up @@ -387,7 +382,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
);
assert_eq!(event.height(), ctx.host_height());
assert!(log.is_empty());
// Check the result
match result {
Expand Down Expand Up @@ -466,7 +460,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::UpdateClient(ref e) if e.client_id() == &msg.client_id)
);
assert_eq!(event.height(), ctx.host_height());
assert!(log.is_empty());
// Check the result
match result {
Expand Down
3 changes: 0 additions & 3 deletions modules/src/core/ics02_client/handler/upgrade_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ pub fn process(
});
let event_attributes = Attributes {
client_id,
height: ctx.host_height(),
..Default::default()
};

Expand All @@ -80,7 +79,6 @@ mod tests {

use core::str::FromStr;

use crate::core::ics02_client::context::ClientReader;
use crate::core::ics02_client::error::{Error, ErrorDetail};
use crate::core::ics02_client::handler::dispatch;
use crate::core::ics02_client::handler::ClientResult::Upgrade;
Expand Down Expand Up @@ -125,7 +123,6 @@ mod tests {
assert!(
matches!(event, IbcEvent::UpgradeClient(ref e) if e.client_id() == &msg.client_id)
);
assert_eq!(event.height(), ctx.host_height());
assert!(log.is_empty());
// Check the result
match result {
Expand Down
54 changes: 5 additions & 49 deletions modules/src/core/ics03_connection/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,24 @@ use serde_derive::{Deserialize, Serialize};
use tendermint::abci::tag::Tag;
use tendermint::abci::Event as AbciEvent;

use crate::core::ics02_client::height::Height;
use crate::core::ics24_host::identifier::{ClientId, ConnectionId};
use crate::events::{IbcEvent, IbcEventType};
use crate::prelude::*;

/// The content of the `key` field for the attribute containing the connection identifier.
pub const HEIGHT_ATTRIBUTE_KEY: &str = "height";
pub const CONN_ID_ATTRIBUTE_KEY: &str = "connection_id";
pub const CLIENT_ID_ATTRIBUTE_KEY: &str = "client_id";
pub const COUNTERPARTY_CONN_ID_ATTRIBUTE_KEY: &str = "counterparty_connection_id";
pub const COUNTERPARTY_CLIENT_ID_ATTRIBUTE_KEY: &str = "counterparty_client_id";

#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Default, Deserialize, Serialize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

pub struct Attributes {
pub height: Height,
pub connection_id: Option<ConnectionId>,
pub client_id: ClientId,
pub counterparty_connection_id: Option<ConnectionId>,
pub counterparty_client_id: ClientId,
}

impl Default for Attributes {
fn default() -> Self {
Self {
height: Height::new(0, 1).unwrap(),
connection_id: Default::default(),
client_id: Default::default(),
counterparty_connection_id: Default::default(),
counterparty_client_id: Default::default(),
}
}
}

/// Convert attributes to Tendermint ABCI tags
///
/// # Note
Expand All @@ -48,11 +33,6 @@ impl Default for Attributes {
impl From<Attributes> for Vec<Tag> {
fn from(a: Attributes) -> Self {
let mut attributes = vec![];
let height = Tag {
key: HEIGHT_ATTRIBUTE_KEY.parse().unwrap(),
value: a.height.to_string().parse().unwrap(),
};
attributes.push(height);
if let Some(conn_id) = a.connection_id {
let conn_id = Tag {
key: CONN_ID_ATTRIBUTE_KEY.parse().unwrap(),
Expand Down Expand Up @@ -82,7 +62,7 @@ impl From<Attributes> for Vec<Tag> {
}

#[derive(Debug, Serialize, Clone, PartialEq, Eq)]
pub struct OpenInit(Attributes);
pub struct OpenInit(pub Attributes);

impl OpenInit {
pub fn attributes(&self) -> &Attributes {
Expand All @@ -91,12 +71,6 @@ impl OpenInit {
pub fn connection_id(&self) -> Option<&ConnectionId> {
self.0.connection_id.as_ref()
}
pub fn height(&self) -> Height {
self.0.height
}
pub fn set_height(&mut self, height: Height) {
self.0.height = height;
}
}

impl From<Attributes> for OpenInit {
Expand All @@ -122,7 +96,7 @@ impl From<OpenInit> for AbciEvent {
}

#[derive(Debug, Serialize, Clone, PartialEq, Eq)]
pub struct OpenTry(Attributes);
pub struct OpenTry(pub Attributes);

impl OpenTry {
pub fn attributes(&self) -> &Attributes {
Expand All @@ -131,12 +105,6 @@ impl OpenTry {
pub fn connection_id(&self) -> Option<&ConnectionId> {
self.0.connection_id.as_ref()
}
pub fn height(&self) -> Height {
self.0.height
}
pub fn set_height(&mut self, height: Height) {
self.0.height = height;
}
}

impl From<Attributes> for OpenTry {
Expand All @@ -162,7 +130,7 @@ impl From<OpenTry> for AbciEvent {
}

#[derive(Debug, Serialize, Clone, PartialEq, Eq)]
pub struct OpenAck(Attributes);
pub struct OpenAck(pub Attributes);

impl OpenAck {
pub fn attributes(&self) -> &Attributes {
Expand All @@ -171,12 +139,6 @@ impl OpenAck {
pub fn connection_id(&self) -> Option<&ConnectionId> {
self.0.connection_id.as_ref()
}
pub fn height(&self) -> Height {
self.0.height
}
pub fn set_height(&mut self, height: Height) {
self.0.height = height;
}
}

impl From<Attributes> for OpenAck {
Expand All @@ -202,7 +164,7 @@ impl From<OpenAck> for AbciEvent {
}

#[derive(Debug, Serialize, Clone, PartialEq, Eq)]
pub struct OpenConfirm(Attributes);
pub struct OpenConfirm(pub Attributes);

impl OpenConfirm {
pub fn attributes(&self) -> &Attributes {
Expand All @@ -211,12 +173,6 @@ impl OpenConfirm {
pub fn connection_id(&self) -> Option<&ConnectionId> {
self.0.connection_id.as_ref()
}
pub fn height(&self) -> Height {
self.0.height
}
pub fn set_height(&mut self, height: Height) {
self.0.height = height;
}
}

impl From<Attributes> for OpenConfirm {
Expand Down
Loading