From 282bd6707514797f4e4ef309239bf6c9319d0e29 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 14 Feb 2024 21:21:33 -0800 Subject: [PATCH 1/3] feat: implement convenient path methods + add iterateConsensusState key --- .../1090-add-iterateConsensusState-key.md | 2 + .../1089-implement-convenient-path-methods.md | 3 + .../ics07-tendermint/src/client_state.rs | 8 +- .../src/handler/conn_open_ack.rs | 2 +- .../src/handler/conn_open_init.rs | 2 +- .../src/handler/conn_open_try.rs | 4 +- ibc-core/ics24-host/types/src/path.rs | 303 +++++++++++++++--- ibc-query/src/core/channel/query.rs | 2 +- ibc-query/src/core/client/query.rs | 2 +- ibc-query/src/core/connection/query.rs | 4 +- .../testapp/ibc/clients/mock/client_state.rs | 17 +- 11 files changed, 284 insertions(+), 65 deletions(-) create mode 100644 .changelog/unreleased/feature/1090-add-iterateConsensusState-key.md create mode 100644 .changelog/unreleased/improvements/1089-implement-convenient-path-methods.md diff --git a/.changelog/unreleased/feature/1090-add-iterateConsensusState-key.md b/.changelog/unreleased/feature/1090-add-iterateConsensusState-key.md new file mode 100644 index 000000000..e3f8222ca --- /dev/null +++ b/.changelog/unreleased/feature/1090-add-iterateConsensusState-key.md @@ -0,0 +1,2 @@ +- [ibc-core-host] Add `iterateConsensusState` key + ([\#1090](https://github.com/cosmos/ibc-rs/issues/1090)) diff --git a/.changelog/unreleased/improvements/1089-implement-convenient-path-methods.md b/.changelog/unreleased/improvements/1089-implement-convenient-path-methods.md new file mode 100644 index 000000000..b93a64caf --- /dev/null +++ b/.changelog/unreleased/improvements/1089-implement-convenient-path-methods.md @@ -0,0 +1,3 @@ +- [ibc-core-host] Improve path segment access by exposing path prefixes and + implementing some convenient parent/leaf methods + ([\#1089](https://github.com/cosmos/ibc-rs/issues/1089)) diff --git a/ibc-clients/ics07-tendermint/src/client_state.rs b/ibc-clients/ics07-tendermint/src/client_state.rs index 3c2c2ab5f..6bc90e506 100644 --- a/ibc-clients/ics07-tendermint/src/client_state.rs +++ b/ibc-clients/ics07-tendermint/src/client_state.rs @@ -328,7 +328,7 @@ where let tm_consensus_state = TmConsensusState::try_from(consensus_state)?; - ctx.store_client_state(ClientStatePath::new(client_id), self.clone().into())?; + ctx.store_client_state(ClientStatePath::new(client_id.clone()), self.clone().into())?; ctx.store_consensus_state( ClientConsensusStatePath::new( client_id.clone(), @@ -389,7 +389,7 @@ where TmConsensusState::from(new_consensus_state).into(), )?; ctx.store_client_state( - ClientStatePath::new(client_id), + ClientStatePath::new(client_id.clone()), ClientState::from(new_client_state).into(), )?; ctx.store_update_meta( @@ -419,7 +419,7 @@ where let wrapped_frozen_client_state = ClientState::from(frozen_client_state); ctx.store_client_state( - ClientStatePath::new(client_id), + ClientStatePath::new(client_id.clone()), wrapped_frozen_client_state.into(), )?; @@ -480,7 +480,7 @@ where let host_height = CommonContext::host_height(ctx)?; ctx.store_client_state( - ClientStatePath::new(client_id), + ClientStatePath::new(client_id.clone()), ClientState::from(new_client_state).into(), )?; ctx.store_consensus_state( diff --git a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs index 865bd78ff..82b447201 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs @@ -100,7 +100,7 @@ where prefix_on_b, &msg.proof_client_state_of_a_on_b, consensus_state_of_b_on_a.root(), - Path::ClientState(ClientStatePath::new(vars.client_id_on_b())), + Path::ClientState(ClientStatePath::new(vars.client_id_on_b().clone())), msg.client_state_of_a_on_b.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { diff --git a/ibc-core/ics03-connection/src/handler/conn_open_init.rs b/ibc-core/ics03-connection/src/handler/conn_open_init.rs index 2f4a4506d..090d1b8b3 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_init.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_init.rs @@ -74,7 +74,7 @@ where ctx_a.increase_connection_counter()?; ctx_a.store_connection_to_client( - &ClientConnectionPath::new(&msg.client_id_on_a), + &ClientConnectionPath::new(msg.client_id_on_a), conn_id_on_a.clone(), )?; ctx_a.store_connection(&ConnectionPath::new(&conn_id_on_a), conn_end_on_a)?; diff --git a/ibc-core/ics03-connection/src/handler/conn_open_try.rs b/ibc-core/ics03-connection/src/handler/conn_open_try.rs index 8ddd3c274..f19299818 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_try.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_try.rs @@ -95,7 +95,7 @@ where prefix_on_a, &msg.proof_client_state_of_b_on_a, consensus_state_of_a_on_b.root(), - Path::ClientState(ClientStatePath::new(client_id_on_a)), + Path::ClientState(ClientStatePath::new(client_id_on_a.clone())), msg.client_state_of_b_on_a.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { @@ -162,7 +162,7 @@ where ctx_b.increase_connection_counter()?; ctx_b.store_connection_to_client( - &ClientConnectionPath::new(&msg.client_id_on_b), + &ClientConnectionPath::new(msg.client_id_on_b), vars.conn_id_on_b.clone(), )?; ctx_b.store_connection(&ConnectionPath::new(&vars.conn_id_on_b), vars.conn_end_on_b)?; diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index ac31922b3..0502a1a20 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -11,13 +11,36 @@ use ibc_primitives::prelude::*; use crate::identifiers::{ChannelId, ClientId, ConnectionId, PortId, Sequence}; +pub const NEXT_CLIENT_SEQUENCE: &str = "nextClientSequence"; +pub const NEXT_CONNECTION_SEQUENCE: &str = "nextConnectionSequence"; +pub const NEXT_CHANNEL_SEQUENCE: &str = "nextChannelSequence"; + +pub const CLIENT_PREFIX: &str = "clients"; +pub const CLIENT_STATE: &str = "clientState"; +pub const CONSENSUS_STATE_PREFIX: &str = "consensusStates"; +pub const CONNECTION_PREFIX: &str = "connections"; +pub const CHANNEL_PREFIX: &str = "channels"; +pub const CHANNEL_END_PREFIX: &str = "channelEnds"; +pub const PORT_PREFIX: &str = "ports"; +pub const SEQUENCE_PREFIX: &str = "sequences"; +pub const NEXT_SEQ_SEND_PREFIX: &str = "nextSequenceSend"; +pub const NEXT_SEQ_RECV_PREFIX: &str = "nextSequenceRecv"; +pub const NEXT_SEQ_ACK_PREFIX: &str = "nextSequenceAck"; +pub const PACKET_COMMITMENT_PREFIX: &str = "commitments"; +pub const PACKET_ACK_PREFIX: &str = "acks"; +pub const PACKET_RECEIPT_PREFIX: &str = "receipts"; + +pub const ITERATE_CONSENSUS_STATE_PREFIX: &str = "iterateConsensusStates"; +pub const PROCESSED_TIME: &str = "processedTime"; +pub const PROCESSED_HEIGHT: &str = "processedHeight"; + /// ABCI client upgrade keys /// - The key identifying the upgraded IBC state within the upgrade sub-store -const UPGRADED_IBC_STATE: &str = "upgradedIBCState"; -///- The key identifying the upgraded client state -const UPGRADED_CLIENT_STATE: &str = "upgradedClient"; +pub const UPGRADED_IBC_STATE: &str = "upgradedIBCState"; +/// - The key identifying the upgraded client state +pub const UPGRADED_CLIENT_STATE: &str = "upgradedClient"; /// - The key identifying the upgraded consensus state -const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState"; +pub const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState"; /// The Path enum abstracts out the different sub-paths. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, From, Display)] @@ -56,7 +79,7 @@ pub enum Path { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "nextClientSequence")] +#[display(fmt = "{NEXT_CLIENT_SEQUENCE}")] pub struct NextClientSequencePath; #[cfg_attr( @@ -73,7 +96,7 @@ pub struct NextClientSequencePath; )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "nextConnectionSequence")] +#[display(fmt = "{NEXT_CONNECTION_SEQUENCE}")] pub struct NextConnectionSequencePath; #[cfg_attr( @@ -90,7 +113,7 @@ pub struct NextConnectionSequencePath; )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "nextChannelSequence")] +#[display(fmt = "{NEXT_CHANNEL_SEQUENCE}")] pub struct NextChannelSequencePath; #[cfg_attr( @@ -106,13 +129,25 @@ pub struct NextChannelSequencePath; derive(borsh::BorshSerialize, borsh::BorshDeserialize) )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "clients/{_0}/clientState")] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display, From)] +#[display(fmt = "{CLIENT_PREFIX}/{_0}/{CLIENT_STATE}")] pub struct ClientStatePath(pub ClientId); impl ClientStatePath { - pub fn new(client_id: &ClientId) -> ClientStatePath { - ClientStatePath(client_id.clone()) + pub fn new(client_id: ClientId) -> ClientStatePath { + ClientStatePath(client_id) + } + + /// Returns the client store prefix under which all the client states are + /// stored: "clients". + pub fn prefix() -> String { + CLIENT_PREFIX.to_string() + } + + /// Returns the final part (leaf) of the path under which an individual + /// client state is stored: "clientState". + pub fn leaf() -> String { + CLIENT_STATE.to_string() } } @@ -130,14 +165,23 @@ impl ClientStatePath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "clients/{client_id}/consensusStates/{revision_number}-{revision_height}")] +#[display( + fmt = "{CLIENT_PREFIX}/{client_id}/{CONSENSUS_STATE_PREFIX}/{revision_number}-{revision_height}" +)] pub struct ClientConsensusStatePath { pub client_id: ClientId, pub revision_number: u64, pub revision_height: u64, } +// Returns the full consensus state path of specific client in the format: +// "clients/{client_id}/consensusStates" as a string. +pub fn full_consensus_state_path(client_id: &ClientId) -> String { + format!("{CLIENT_PREFIX}/{client_id}/{CONSENSUS_STATE_PREFIX}") +} + impl ClientConsensusStatePath { + /// Constructs a new `ClientConsensusStatePath`. pub fn new( client_id: ClientId, revision_number: u64, @@ -149,6 +193,22 @@ impl ClientConsensusStatePath { revision_height, } } + + /// Returns the path representing the parent group under which all consensus + /// states are stored: "clients/{client_id}/consensusStates". + pub fn parent(&self) -> String { + full_consensus_state_path(&self.client_id) + } + + /// Returns the final part (leaf) of the path under which an individual + /// consensus state is stored: + /// "consensusStates/{revision_number}-{revision_height}". + pub fn leaf(&self) -> String { + format!( + "{CONSENSUS_STATE_PREFIX}/{}-{}", + self.revision_number, self.revision_height + ) + } } #[cfg_attr( @@ -166,7 +226,7 @@ impl ClientConsensusStatePath { #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] #[display( - fmt = "clients/{client_id}/consensusStates/{revision_number}-{revision_height}/processedTime" + fmt = "{CLIENT_PREFIX}/{client_id}/{CONSENSUS_STATE_PREFIX}/{revision_number}-{revision_height}/{PROCESSED_TIME}" )] pub struct ClientUpdateTimePath { pub client_id: ClientId, @@ -175,6 +235,7 @@ pub struct ClientUpdateTimePath { } impl ClientUpdateTimePath { + /// Constructs a new `ClientUpdateTimePath`. pub fn new(client_id: ClientId, revision_number: u64, revision_height: u64) -> Self { Self { client_id, @@ -182,6 +243,22 @@ impl ClientUpdateTimePath { revision_height, } } + + /// Returns the path representing the parent group under which all the + /// processed times are stored: "clients/{client_id}/consensusStates". + pub fn parent(&self) -> String { + full_consensus_state_path(&self.client_id) + } + + /// Returns the final part (leaf) of the path under which an individual + /// processed time is stored: + /// "consensusStates/{revision_number}-{revision_height}/processedTime". + pub fn leaf(&self) -> String { + format!( + "{CONSENSUS_STATE_PREFIX}/{}-{}/{PROCESSED_TIME}", + self.revision_number, self.revision_height + ) + } } #[cfg_attr( @@ -199,7 +276,7 @@ impl ClientUpdateTimePath { #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] #[display( - fmt = "clients/{client_id}/consensusStates/{revision_number}-{revision_height}/processedHeight" + fmt = "{CLIENT_PREFIX}/{client_id}/{CONSENSUS_STATE_PREFIX}/{revision_number}-{revision_height}/{PROCESSED_HEIGHT}" )] pub struct ClientUpdateHeightPath { pub client_id: ClientId, @@ -215,6 +292,70 @@ impl ClientUpdateHeightPath { revision_height, } } + + /// Returns the path representing the parent group under which all the + /// processed heights are stored: "clients/{client_id}/consensusStates". + pub fn parent(&self) -> String { + full_consensus_state_path(&self.client_id) + } + + /// Returns the final part (leaf) of the path under which an individual + /// processed height is stored: + /// "consensusStates/{revision_number}-{revision_height}/processedHeight". + pub fn leaf(&self) -> String { + format!( + "{CONSENSUS_STATE_PREFIX}/{}-{}/{PROCESSED_HEIGHT}", + self.revision_number, self.revision_height + ) + } +} + +/// This iteration key is namely used by the `ibc-go` implementation as an +/// efficient approach for iterating over consensus states. This is specifically +/// incorporated to facilitate cross-compatibility with `ibc-go` when developing +/// CosmWasm-driven light clients with `ibc-rs`. +/// +/// See `ibc-go` +/// [documentation](https://github.com/cosmos/ibc-go/blob/016a6095b577ecb9323edad508cff19d017636a1/modules/light-clients/07-tendermint/store.go#L19-L34) +/// for more details. +#[cfg_attr( + feature = "parity-scale-codec", + derive( + parity_scale_codec::Encode, + parity_scale_codec::Decode, + scale_info::TypeInfo + ) +)] +#[cfg_attr( + feature = "borsh", + derive(borsh::BorshSerialize, borsh::BorshDeserialize) +)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct IterateConsensusStates { + pub revision_number: u64, + pub revision_height: u64, +} + +impl IterateConsensusStates { + pub fn new(revision_number: u64, revision_height: u64) -> Self { + Self { + revision_number, + revision_height, + } + } + + /// The key is formatted like so: + /// `iterateConsensusStates{BigEndianRevisionBytes}{BigEndianHeightBytes}` + /// to ensures that the lexicographic order of iteration keys match the + /// height order of the consensus states. + pub fn key(&self) -> Vec { + let mut path = Vec::new(); + path.extend_from_slice(ITERATE_CONSENSUS_STATE_PREFIX.as_bytes()); + path.extend(&self.revision_number.to_be_bytes()); + path.extend(&self.revision_height.to_be_bytes()); + path + } } #[cfg_attr( @@ -231,12 +372,12 @@ impl ClientUpdateHeightPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "clients/{_0}/connections")] +#[display(fmt = "{CLIENT_PREFIX}/{_0}/{CONNECTION_PREFIX}")] pub struct ClientConnectionPath(pub ClientId); impl ClientConnectionPath { - pub fn new(client_id: &ClientId) -> ClientConnectionPath { - ClientConnectionPath(client_id.clone()) + pub fn new(client_id: ClientId) -> ClientConnectionPath { + ClientConnectionPath(client_id) } } @@ -254,13 +395,19 @@ impl ClientConnectionPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "connections/{_0}")] +#[display(fmt = "{CONNECTION_PREFIX}/{_0}")] pub struct ConnectionPath(pub ConnectionId); impl ConnectionPath { pub fn new(connection_id: &ConnectionId) -> ConnectionPath { ConnectionPath(connection_id.clone()) } + + /// Returns the connection store prefix under which all the connection are + /// stored: "connections". + pub fn prefix() -> String { + CONNECTION_PREFIX.to_string() + } } #[cfg_attr( @@ -277,7 +424,7 @@ impl ConnectionPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "ports/{_0}")] +#[display(fmt = "{PORT_PREFIX}/{_0}")] pub struct PortPath(pub PortId); #[cfg_attr( @@ -294,13 +441,47 @@ pub struct PortPath(pub PortId); )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "channelEnds/ports/{_0}/channels/{_1}")] +#[display(fmt = "{CHANNEL_END_PREFIX}/{PORT_PREFIX}/{_0}/{CHANNEL_PREFIX}/{_1}")] pub struct ChannelEndPath(pub PortId, pub ChannelId); impl ChannelEndPath { pub fn new(port_id: &PortId, channel_id: &ChannelId) -> ChannelEndPath { ChannelEndPath(port_id.clone(), channel_id.clone()) } + + /// Returns the channel end store prefix under which all the channel ends + /// are stored: "channelEnds". + pub fn prefix() -> String { + CHANNEL_END_PREFIX.to_string() + } + + /// Returns the parent group path under which all the sequences of a channel are + /// stored with the format: + /// "{prefix}/ports/{port_id}/channels/{channel_id}/sequences" + fn full_sequences_path(&self, prefix: &str) -> String { + format!( + "{prefix}/{PORT_PREFIX}/{}/{CHANNEL_PREFIX}/{}/{SEQUENCE_PREFIX}", + self.0, self.1, + ) + } + + /// Returns the parent group path under which all the commitment packets of + /// a channel are stored. + pub fn commitments_path(&self) -> String { + self.full_sequences_path(PACKET_COMMITMENT_PREFIX) + } + + /// Returns the parent group path under which all the ack packets of a + /// channel are stored. + pub fn acks_path(&self) -> String { + self.full_sequences_path(PACKET_ACK_PREFIX) + } + + /// Returns the parent group path under which all the receipt packets of a + /// channel are stored. + pub fn receipts_path(&self) -> String { + self.full_sequences_path(PACKET_RECEIPT_PREFIX) + } } #[cfg_attr( @@ -317,7 +498,7 @@ impl ChannelEndPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "nextSequenceSend/ports/{_0}/channels/{_1}")] +#[display(fmt = "{NEXT_SEQ_SEND_PREFIX}/{PORT_PREFIX}/{_0}/{CHANNEL_PREFIX}/{_1}")] pub struct SeqSendPath(pub PortId, pub ChannelId); impl SeqSendPath { @@ -340,7 +521,7 @@ impl SeqSendPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "nextSequenceRecv/ports/{_0}/channels/{_1}")] +#[display(fmt = "{NEXT_SEQ_RECV_PREFIX}/{PORT_PREFIX}/{_0}/{CHANNEL_PREFIX}/{_1}")] pub struct SeqRecvPath(pub PortId, pub ChannelId); impl SeqRecvPath { @@ -363,7 +544,7 @@ impl SeqRecvPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "nextSequenceAck/ports/{_0}/channels/{_1}")] +#[display(fmt = "{NEXT_SEQ_ACK_PREFIX}/{PORT_PREFIX}/{_0}/{CHANNEL_PREFIX}/{_1}")] pub struct SeqAckPath(pub PortId, pub ChannelId); impl SeqAckPath { @@ -386,7 +567,9 @@ impl SeqAckPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "commitments/ports/{port_id}/channels/{channel_id}/sequences/{sequence}")] +#[display( + fmt = "{PACKET_COMMITMENT_PREFIX}/{PORT_PREFIX}/{port_id}/{CHANNEL_PREFIX}/{channel_id}/{SEQUENCE_PREFIX}/{sequence}" +)] pub struct CommitmentPath { pub port_id: PortId, pub channel_id: ChannelId, @@ -401,6 +584,12 @@ impl CommitmentPath { sequence, } } + + /// Returns the commitment store prefix under which all the packet + /// commitments are stored: "commitments" + pub fn prefix() -> String { + PACKET_COMMITMENT_PREFIX.to_string() + } } #[cfg_attr( @@ -417,7 +606,9 @@ impl CommitmentPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "acks/ports/{port_id}/channels/{channel_id}/sequences/{sequence}")] +#[display( + fmt = "{PACKET_ACK_PREFIX}/{PORT_PREFIX}/{port_id}/{CHANNEL_PREFIX}/{channel_id}/{SEQUENCE_PREFIX}/{sequence}" +)] pub struct AckPath { pub port_id: PortId, pub channel_id: ChannelId, @@ -432,6 +623,12 @@ impl AckPath { sequence, } } + + /// Returns the ack store prefix under which all the packet acks are stored: + /// "acks" + pub fn prefix() -> String { + PACKET_ACK_PREFIX.to_string() + } } #[cfg_attr( @@ -448,7 +645,9 @@ impl AckPath { )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)] -#[display(fmt = "receipts/ports/{port_id}/channels/{channel_id}/sequences/{sequence}")] +#[display( + fmt = "{PACKET_RECEIPT_PREFIX}/{PORT_PREFIX}/{port_id}/{CHANNEL_PREFIX}/{channel_id}/{SEQUENCE_PREFIX}/{sequence}" +)] pub struct ReceiptPath { pub port_id: PortId, pub channel_id: ChannelId, @@ -463,6 +662,12 @@ impl ReceiptPath { sequence, } } + + /// Returns the receipt store prefix under which all the packet receipts are + /// stored: "receipts" + pub fn prefix() -> String { + PACKET_RECEIPT_PREFIX.to_string() + } } #[cfg_attr( @@ -558,9 +763,9 @@ fn parse_next_sequence(components: &[&str]) -> Option { } match *components.first()? { - "nextClientSequence" => Some(NextClientSequencePath.into()), - "nextConnectionSequence" => Some(NextConnectionSequencePath.into()), - "nextChannelSequence" => Some(NextChannelSequencePath.into()), + NEXT_CLIENT_SEQUENCE => Some(NextClientSequencePath.into()), + NEXT_CONNECTION_SEQUENCE => Some(NextConnectionSequencePath.into()), + NEXT_CHANNEL_SEQUENCE => Some(NextChannelSequencePath.into()), _ => None, } } @@ -571,7 +776,7 @@ fn parse_client_paths(components: &[&str]) -> Option { None => return None, }; - if first != "clients" { + if first != CLIENT_PREFIX { return None; } @@ -582,13 +787,13 @@ fn parse_client_paths(components: &[&str]) -> Option { if components.len() == 3 { match components[2] { - "clientState" => Some(ClientStatePath(client_id).into()), - "connections" => Some(ClientConnectionPath(client_id).into()), + CLIENT_STATE => Some(ClientStatePath(client_id).into()), + CONNECTION_PREFIX => Some(ClientConnectionPath(client_id).into()), _ => None, } } else if components.len() == 4 || components.len() == 5 { match components[2] { - "consensusStates" => {} + CONSENSUS_STATE_PREFIX => {} _ => return None, } @@ -621,7 +826,7 @@ fn parse_client_paths(components: &[&str]) -> Option { .into(), ), 5 => match components[4] { - "processedTime" => Some( + PROCESSED_TIME => Some( ClientUpdateTimePath { client_id, revision_number, @@ -629,7 +834,7 @@ fn parse_client_paths(components: &[&str]) -> Option { } .into(), ), - "processedHeight" => Some( + PROCESSED_HEIGHT => Some( ClientUpdateHeightPath { client_id, revision_number, @@ -656,7 +861,7 @@ fn parse_connections(components: &[&str]) -> Option { None => return None, }; - if first != "connections" { + if first != CONNECTION_PREFIX { return None; } @@ -683,7 +888,7 @@ fn parse_ports(components: &[&str]) -> Option { None => return None, }; - if first != "ports" { + if first != PORT_PREFIX { return None; } @@ -710,7 +915,7 @@ fn parse_channels(components: &[&str]) -> Option { None => return None, }; - if first != "channels" { + if first != CHANNEL_PREFIX { return None; } @@ -737,7 +942,7 @@ fn parse_sequences(components: &[&str]) -> Option { None => return None, }; - if first != "sequences" { + if first != SEQUENCE_PREFIX { return None; } @@ -762,7 +967,7 @@ fn parse_channel_ends(components: &[&str]) -> Option { None => return None, }; - if first != "channelEnds" { + if first != CHANNEL_END_PREFIX { return None; } @@ -810,9 +1015,9 @@ fn parse_seqs(components: &[&str]) -> Option { }; match first { - "nextSequenceSend" => Some(SeqSendPath(port_id, channel_id).into()), - "nextSequenceRecv" => Some(SeqRecvPath(port_id, channel_id).into()), - "nextSequenceAck" => Some(SeqAckPath(port_id, channel_id).into()), + NEXT_SEQ_SEND_PREFIX => Some(SeqSendPath(port_id, channel_id).into()), + NEXT_SEQ_RECV_PREFIX => Some(SeqRecvPath(port_id, channel_id).into()), + NEXT_SEQ_ACK_PREFIX => Some(SeqAckPath(port_id, channel_id).into()), _ => None, } } @@ -827,7 +1032,7 @@ fn parse_commitments(components: &[&str]) -> Option { None => return None, }; - if first != "commitments" { + if first != PACKET_COMMITMENT_PREFIX { return None; } @@ -873,7 +1078,7 @@ fn parse_acks(components: &[&str]) -> Option { None => return None, }; - if first != "acks" { + if first != PACKET_ACK_PREFIX { return None; } @@ -919,7 +1124,7 @@ fn parse_receipts(components: &[&str]) -> Option { None => return None, }; - if first != "receipts" { + if first != PACKET_RECEIPT_PREFIX { return None; } @@ -995,13 +1200,13 @@ mod tests { use super::*; #[rstest::rstest] - #[case("nextClientSequence", Path::NextClientSequence(NextClientSequencePath))] + #[case(NEXT_CLIENT_SEQUENCE, Path::NextClientSequence(NextClientSequencePath))] #[case( - "nextConnectionSequence", + NEXT_CONNECTION_SEQUENCE, Path::NextConnectionSequence(NextConnectionSequencePath) )] #[case( - "nextChannelSequence", + NEXT_CHANNEL_SEQUENCE, Path::NextChannelSequence(NextChannelSequencePath) )] #[case( diff --git a/ibc-query/src/core/channel/query.rs b/ibc-query/src/core/channel/query.rs index d14a5621c..56d9bb5be 100644 --- a/ibc-query/src/core/channel/query.rs +++ b/ibc-query/src/core/channel/query.rs @@ -146,7 +146,7 @@ where let proof = ibc_ctx .get_proof( current_height, - &Path::ClientState(ClientStatePath::new(connection_end.client_id())), + &Path::ClientState(ClientStatePath::new(connection_end.client_id().clone())), ) .ok_or(QueryError::ProofNotFound { description: format!( diff --git a/ibc-query/src/core/client/query.rs b/ibc-query/src/core/client/query.rs index 93383b6b7..5162c5de3 100644 --- a/ibc-query/src/core/client/query.rs +++ b/ibc-query/src/core/client/query.rs @@ -44,7 +44,7 @@ where let proof = ibc_ctx .get_proof( current_height, - &Path::ClientState(ClientStatePath::new(&client_id)), + &Path::ClientState(ClientStatePath::new(client_id.clone())), ) .ok_or(QueryError::ProofNotFound { description: format!("Proof not found for client state path: {client_id:?}"), diff --git a/ibc-query/src/core/connection/query.rs b/ibc-query/src/core/connection/query.rs index e7eafc92f..c75bd36b6 100644 --- a/ibc-query/src/core/connection/query.rs +++ b/ibc-query/src/core/connection/query.rs @@ -89,7 +89,7 @@ where let proof: Vec = ibc_ctx .get_proof( current_height, - &Path::ClientConnection(ClientConnectionPath::new(&client_id)), + &Path::ClientConnection(ClientConnectionPath::new(client_id.clone())), ) .ok_or(QueryError::ProofNotFound { description: format!("Proof not found for client connection path: {client_id:?}"), @@ -122,7 +122,7 @@ where let proof = ibc_ctx .get_proof( current_height, - &Path::ClientState(ClientStatePath::new(connection_end.client_id())), + &Path::ClientState(ClientStatePath::new(connection_end.client_id().clone())), ) .ok_or(QueryError::ProofNotFound { description: format!( diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs index 0c1ea1a8b..ab1cf2d73 100644 --- a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs +++ b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs @@ -309,7 +309,7 @@ where ) -> Result<(), ClientError> { let mock_consensus_state = MockConsensusState::try_from(consensus_state)?; - ctx.store_client_state(ClientStatePath::new(client_id), (*self).into())?; + ctx.store_client_state(ClientStatePath::new(client_id.clone()), (*self).into())?; ctx.store_consensus_state( ClientConsensusStatePath::new( client_id.clone(), @@ -342,7 +342,10 @@ where ), new_consensus_state.into(), )?; - ctx.store_client_state(ClientStatePath::new(client_id), new_client_state.into())?; + ctx.store_client_state( + ClientStatePath::new(client_id.clone()), + new_client_state.into(), + )?; ctx.store_update_meta( client_id.clone(), header_height, @@ -361,7 +364,10 @@ where ) -> Result<(), ClientError> { let frozen_client_state = self.with_frozen_height(Height::min(0)); - ctx.store_client_state(ClientStatePath::new(client_id), frozen_client_state.into())?; + ctx.store_client_state( + ClientStatePath::new(client_id.clone()), + frozen_client_state.into(), + )?; Ok(()) } @@ -386,7 +392,10 @@ where ), new_consensus_state.into(), )?; - ctx.store_client_state(ClientStatePath::new(client_id), new_client_state.into())?; + ctx.store_client_state( + ClientStatePath::new(client_id.clone()), + new_client_state.into(), + )?; let host_timestamp = ctx.host_timestamp()?; let host_height = ctx.host_height()?; From 2d626103fb66441556ae2c086b157bd72113792a Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 15 Feb 2024 06:10:33 -0800 Subject: [PATCH 2/3] imp: just keeping iteration key func --- .../1090-add-iterate-consensus-states-key.md | 3 ++ .../1090-add-iterateConsensusState-key.md | 2 - ibc-core/ics24-host/types/src/path.rs | 49 +++++-------------- 3 files changed, 14 insertions(+), 40 deletions(-) create mode 100644 .changelog/unreleased/feature/1090-add-iterate-consensus-states-key.md delete mode 100644 .changelog/unreleased/feature/1090-add-iterateConsensusState-key.md diff --git a/.changelog/unreleased/feature/1090-add-iterate-consensus-states-key.md b/.changelog/unreleased/feature/1090-add-iterate-consensus-states-key.md new file mode 100644 index 000000000..9fef7afd9 --- /dev/null +++ b/.changelog/unreleased/feature/1090-add-iterate-consensus-states-key.md @@ -0,0 +1,3 @@ +- [ibc-core-host] Add iteration key for cross-compatibility with `ibc-go` used + for iterating over consensus states + ([\#1090](https://github.com/cosmos/ibc-rs/issues/1090)) diff --git a/.changelog/unreleased/feature/1090-add-iterateConsensusState-key.md b/.changelog/unreleased/feature/1090-add-iterateConsensusState-key.md deleted file mode 100644 index e3f8222ca..000000000 --- a/.changelog/unreleased/feature/1090-add-iterateConsensusState-key.md +++ /dev/null @@ -1,2 +0,0 @@ -- [ibc-core-host] Add `iterateConsensusState` key - ([\#1090](https://github.com/cosmos/ibc-rs/issues/1090)) diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index 0502a1a20..717a597b3 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -315,47 +315,20 @@ impl ClientUpdateHeightPath { /// incorporated to facilitate cross-compatibility with `ibc-go` when developing /// CosmWasm-driven light clients with `ibc-rs`. /// +/// The key is formatted like so: +/// `iterateConsensusStates{BigEndianRevisionBytes}{BigEndianHeightBytes}` +/// to ensures that the lexicographic order of iteration keys match the +/// height order of the consensus states. +/// /// See `ibc-go` /// [documentation](https://github.com/cosmos/ibc-go/blob/016a6095b577ecb9323edad508cff19d017636a1/modules/light-clients/07-tendermint/store.go#L19-L34) /// for more details. -#[cfg_attr( - feature = "parity-scale-codec", - derive( - parity_scale_codec::Encode, - parity_scale_codec::Decode, - scale_info::TypeInfo - ) -)] -#[cfg_attr( - feature = "borsh", - derive(borsh::BorshSerialize, borsh::BorshDeserialize) -)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct IterateConsensusStates { - pub revision_number: u64, - pub revision_height: u64, -} - -impl IterateConsensusStates { - pub fn new(revision_number: u64, revision_height: u64) -> Self { - Self { - revision_number, - revision_height, - } - } - - /// The key is formatted like so: - /// `iterateConsensusStates{BigEndianRevisionBytes}{BigEndianHeightBytes}` - /// to ensures that the lexicographic order of iteration keys match the - /// height order of the consensus states. - pub fn key(&self) -> Vec { - let mut path = Vec::new(); - path.extend_from_slice(ITERATE_CONSENSUS_STATE_PREFIX.as_bytes()); - path.extend(&self.revision_number.to_be_bytes()); - path.extend(&self.revision_height.to_be_bytes()); - path - } +pub fn iteration_key(revision_number: u64, revision_height: u64) -> Vec { + let mut path = Vec::new(); + path.extend_from_slice(ITERATE_CONSENSUS_STATE_PREFIX.as_bytes()); + path.extend(revision_number.to_be_bytes()); + path.extend(revision_height.to_be_bytes()); + path } #[cfg_attr( From a5f053678b082b723dbe9d72b9c3d269b1d5b3c0 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 15 Feb 2024 06:59:35 -0800 Subject: [PATCH 3/3] nitpicking --- ibc-core/ics24-host/types/src/path.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index 717a597b3..16b23a5ac 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -317,7 +317,7 @@ impl ClientUpdateHeightPath { /// /// The key is formatted like so: /// `iterateConsensusStates{BigEndianRevisionBytes}{BigEndianHeightBytes}` -/// to ensures that the lexicographic order of iteration keys match the +/// to ensure that the lexicographic order of iteration keys match the /// height order of the consensus states. /// /// See `ibc-go` @@ -376,7 +376,7 @@ impl ConnectionPath { ConnectionPath(connection_id.clone()) } - /// Returns the connection store prefix under which all the connection are + /// Returns the connection store prefix under which all the connections are /// stored: "connections". pub fn prefix() -> String { CONNECTION_PREFIX.to_string()