diff --git a/.changelog/unreleased/breaking-changes/242-change-return-value.md b/.changelog/unreleased/breaking-changes/242-change-return-value.md new file mode 100644 index 000000000..f3e30b2d0 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/242-change-return-value.md @@ -0,0 +1,2 @@ +- Change `host_height`, `host_timestamp` return value to a `Result` in `ClientReader`, `ConnectionReader`, `ChannelReader` and `ValidationContext` + ([#242](https://github.com/cosmos/ibc-rs/issues/242)) \ No newline at end of file diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index f4584ac58..5280f85b5 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -403,7 +403,7 @@ impl Ics2ClientState for ClientState { untrusted_state, trusted_state, &options, - ctx.host_timestamp().into_tm_time().unwrap(), + ctx.host_timestamp()?.into_tm_time().unwrap(), ); match verdict { @@ -574,7 +574,7 @@ impl Ics2ClientState for ClientState { untrusted_state, trusted_state, &options, - ctx.host_timestamp().into_tm_time().unwrap(), + ctx.host_timestamp()?.into_tm_time().unwrap(), ); match verdict { @@ -915,8 +915,12 @@ fn verify_delay_passed( height: Height, connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { - let current_timestamp = ctx.host_timestamp(); - let current_height = ctx.host_height(); + let current_timestamp = ctx + .host_timestamp() + .map_err(|e| Ics02Error::other(e.to_string()))?; + let current_height = ctx + .host_height() + .map_err(|e| Ics02Error::other(e.to_string()))?; let client_id = connection_end.client_id(); let processed_time = ctx diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 38e34d4fa..7fead02c4 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -90,14 +90,14 @@ pub trait ValidationContext { ) -> Result>, ClientError>; /// Returns the current height of the local chain. - fn host_height(&self) -> Height; + fn host_height(&self) -> Result; /// Returns the current timestamp of the local chain. - fn host_timestamp(&self) -> Timestamp { + fn host_timestamp(&self) -> Result { let pending_consensus_state = self .pending_host_consensus_state() .expect("host must have pending consensus state"); - pending_consensus_state.timestamp() + Ok(pending_consensus_state.timestamp()) } /// Returns the pending `ConsensusState` of the host (local) chain. diff --git a/crates/ibc/src/core/ics02_client/context.rs b/crates/ibc/src/core/ics02_client/context.rs index 4ad9e9b7e..77d0aed88 100644 --- a/crates/ibc/src/core/ics02_client/context.rs +++ b/crates/ibc/src/core/ics02_client/context.rs @@ -51,14 +51,14 @@ pub trait ClientReader { ) -> Result>, Error>; /// Returns the current height of the local chain. - fn host_height(&self) -> Height; + fn host_height(&self) -> Result; /// Returns the current timestamp of the local chain. - fn host_timestamp(&self) -> Timestamp { + fn host_timestamp(&self) -> Result { let pending_consensus_state = self .pending_host_consensus_state() .expect("host must have pending consensus state"); - pending_consensus_state.timestamp() + Ok(pending_consensus_state.timestamp()) } /// Returns the `ConsensusState` of the host (local) chain at a specific height. diff --git a/crates/ibc/src/core/ics02_client/handler/create_client.rs b/crates/ibc/src/core/ics02_client/handler/create_client.rs index 49b2e50cb..900eb5dd3 100644 --- a/crates/ibc/src/core/ics02_client/handler/create_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/create_client.rs @@ -90,12 +90,12 @@ where ctx.store_update_time( client_id.clone(), client_state.latest_height(), - ctx.host_timestamp(), + ctx.host_timestamp()?, )?; ctx.store_update_height( client_id.clone(), client_state.latest_height(), - ctx.host_height(), + ctx.host_height()?, )?; ctx.emit_ibc_event(IbcEvent::CreateClient(CreateClient::new( @@ -141,8 +141,8 @@ pub fn process(ctx: &dyn ClientReader, msg: MsgCreateClient) -> HandlerResult( debug!("latest consensus state: {:?}", latest_consensus_state); - let now = ClientReader::host_timestamp(ctx); + let now = ClientReader::host_timestamp(ctx)?; let duration = now .duration_since(&latest_consensus_state.timestamp()) .ok_or_else(|| { @@ -191,8 +191,8 @@ pub fn process( client_id: client_id.clone(), client_state, consensus_state, - processed_time: ClientReader::host_timestamp(ctx), - processed_height: ctx.host_height(), + processed_time: ClientReader::host_timestamp(ctx)?, + processed_height: ctx.host_height()?, }); output.emit(IbcEvent::UpdateClient(UpdateClient::new( diff --git a/crates/ibc/src/core/ics03_connection/context.rs b/crates/ibc/src/core/ics03_connection/context.rs index 83ddb2e74..3e33775ed 100644 --- a/crates/ibc/src/core/ics03_connection/context.rs +++ b/crates/ibc/src/core/ics03_connection/context.rs @@ -28,11 +28,11 @@ pub trait ConnectionReader { fn decode_client_state(&self, client_state: Any) -> Result, Error>; /// Returns the current height of the local chain. - fn host_current_height(&self) -> Height; + fn host_current_height(&self) -> Result; #[deprecated(since = "0.20.0")] /// Returns the oldest height available on the local chain. - fn host_oldest_height(&self) -> Height; + fn host_oldest_height(&self) -> Result; /// Returns the prefix that the local chain uses in the KV store. fn commitment_prefix(&self) -> CommitmentPrefix; diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs index 7e8405dd0..ebc41ed4c 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs @@ -19,10 +19,10 @@ pub(crate) fn process( ) -> HandlerResult { let mut output = HandlerOutput::builder(); - if msg.consensus_height_of_a_on_b > ctx_a.host_current_height() { + if msg.consensus_height_of_a_on_b > ctx_a.host_current_height()? { return Err(Error::invalid_consensus_height( msg.consensus_height_of_a_on_b, - ctx_a.host_current_height(), + ctx_a.host_current_height()?, )); } diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs index 5846ba9c4..41d984567 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs @@ -24,11 +24,11 @@ pub(crate) fn process( ctx_b.validate_self_client(msg.client_state_of_b_on_a.clone())?; - if msg.consensus_height_of_b_on_a > ctx_b.host_current_height() { + if msg.consensus_height_of_b_on_a > ctx_b.host_current_height()? { // Fail if the consensus height is too advanced. return Err(Error::invalid_consensus_height( msg.consensus_height_of_b_on_a, - ctx_b.host_current_height(), + ctx_b.host_current_height()?, )); } diff --git a/crates/ibc/src/core/ics04_channel/context.rs b/crates/ibc/src/core/ics04_channel/context.rs index 89795e517..e153c5cef 100644 --- a/crates/ibc/src/core/ics04_channel/context.rs +++ b/crates/ibc/src/core/ics04_channel/context.rs @@ -113,14 +113,14 @@ pub trait ChannelReader { fn hash(&self, value: Vec) -> Vec; /// Returns the current height of the local chain. - fn host_height(&self) -> Height; + fn host_height(&self) -> Result; /// Returns the current timestamp of the local chain. - fn host_timestamp(&self) -> Timestamp { + fn host_timestamp(&self) -> Result { let pending_consensus_state = self .pending_host_consensus_state() .expect("host must have pending consensus state"); - pending_consensus_state.timestamp() + Ok(pending_consensus_state.timestamp()) } /// Returns the `ConsensusState` of the host (local) chain at a specific height. diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index c1b65cca2..bd99e5070 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -133,7 +133,7 @@ mod tests { let client_id = ClientId::new(mock_client_type(), 24).unwrap(); let conn_id = ConnectionId::new(2); let default_context = MockContext::default(); - let client_consensus_state_height = default_context.host_height(); + let client_consensus_state_height = default_context.host_height().unwrap(); let conn_end = ConnectionEnd::new( ConnectionState::Open, diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs index c62faab62..829fd2b86 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs @@ -111,7 +111,7 @@ mod tests { let context = { let default_context = MockContext::default(); - let client_consensus_state_height = default_context.host_height(); + let client_consensus_state_height = default_context.host_height().unwrap(); default_context .with_client(&client_id, client_consensus_state_height) diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index da9985951..4f7901d35 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -140,7 +140,8 @@ mod tests { let client_id = ClientId::new(mock_client_type(), 24).unwrap(); let conn_id = ConnectionId::new(2); let context = MockContext::default(); - let client_consensus_state_height = context.host_current_height().revision_height(); + let client_consensus_state_height = + context.host_current_height().unwrap().revision_height(); // The connection underlying the channel we're trying to open. let conn_end = ConnectionEnd::new( diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index 37844705e..320ec85f8 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -66,7 +66,7 @@ pub fn process( )); } - let latest_height = ChannelReader::host_height(ctx); + let latest_height = ChannelReader::host_height(ctx)?; if packet.timeout_height.has_expired(latest_height) { return Err(Error::low_packet_height( latest_height, @@ -74,7 +74,7 @@ pub fn process( )); } - let latest_timestamp = ChannelReader::host_timestamp(ctx); + let latest_timestamp = ChannelReader::host_timestamp(ctx)?; if let Expiry::Expired = latest_timestamp.check_expiry(&packet.timeout_timestamp) { return Err(Error::low_packet_timestamp()); } @@ -186,7 +186,7 @@ mod tests { let context = MockContext::default(); - let host_height = context.query_latest_height().increment(); + let host_height = context.query_latest_height().unwrap().increment(); let client_height = host_height.increment(); diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 88b845ad2..c5d0376fe 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -845,12 +845,12 @@ impl ChannelReader for MockContext { sha2::Sha256::digest(value).to_vec() } - fn host_height(&self) -> Height { - self.latest_height() + fn host_height(&self) -> Result { + Ok(self.latest_height()) } - fn host_timestamp(&self) -> Timestamp { - ClientReader::host_timestamp(self) + fn host_timestamp(&self) -> Result { + ClientReader::host_timestamp(self).map_err(|e| Ics04Error::other(e.to_string())) } fn host_consensus_state(&self, height: Height) -> Result, Ics04Error> { @@ -1103,13 +1103,13 @@ impl ConnectionReader for MockContext { ClientReader::decode_client_state(self, client_state).map_err(Ics03Error::ics02_client) } - fn host_current_height(&self) -> Height { - self.latest_height() + fn host_current_height(&self) -> Result { + Ok(self.latest_height()) } - fn host_oldest_height(&self) -> Height { + fn host_oldest_height(&self) -> Result { // history must be non-empty, so `self.history[0]` is valid - self.history[0].height() + Ok(self.history[0].height()) } fn commitment_prefix(&self) -> CommitmentPrefix { @@ -1275,17 +1275,18 @@ impl ClientReader for MockContext { Ok(None) } - fn host_height(&self) -> Height { - self.latest_height() + fn host_height(&self) -> Result { + Ok(self.latest_height()) } - fn host_timestamp(&self) -> Timestamp { - self.history + fn host_timestamp(&self) -> Result { + Ok(self + .history .last() .expect("history cannot be empty") .timestamp() .add(self.block_time) - .unwrap() + .unwrap()) } fn host_consensus_state(&self, height: Height) -> Result, Ics02Error> { @@ -1401,8 +1402,8 @@ impl ClientKeeper for MockContext { } impl RelayerContext for MockContext { - fn query_latest_height(&self) -> Height { - self.host_current_height() + fn query_latest_height(&self) -> Result { + self.host_current_height().map_err(Ics18Error::ics03) } fn query_client_full_state(&self, client_id: &ClientId) -> Option> { @@ -1411,7 +1412,7 @@ impl RelayerContext for MockContext { } fn query_latest_header(&self) -> Option> { - let block_ref = self.host_block(self.host_current_height()); + let block_ref = self.host_block(self.host_current_height().unwrap()); block_ref.cloned().map(Header::into_box) } diff --git a/crates/ibc/src/relayer/ics18_relayer/context.rs b/crates/ibc/src/relayer/ics18_relayer/context.rs index 933143f93..9b4bf375d 100644 --- a/crates/ibc/src/relayer/ics18_relayer/context.rs +++ b/crates/ibc/src/relayer/ics18_relayer/context.rs @@ -17,7 +17,7 @@ use crate::Height; /// types, light client, RPC client, etc.) pub trait RelayerContext { /// Returns the latest height of the chain. - fn query_latest_height(&self) -> Height; + fn query_latest_height(&self) -> Result; /// Returns this client state for the given `client_id` on this chain. /// Wrapper over the `/abci_query?path=..` endpoint. diff --git a/crates/ibc/src/relayer/ics18_relayer/error.rs b/crates/ibc/src/relayer/ics18_relayer/error.rs index e975de5a3..d141358ca 100644 --- a/crates/ibc/src/relayer/ics18_relayer/error.rs +++ b/crates/ibc/src/relayer/ics18_relayer/error.rs @@ -1,3 +1,4 @@ +use crate::core::ics03_connection; use crate::core::ics24_host::identifier::ClientId; use crate::core::ics26_routing::error::Error as RoutingError; use crate::Height; @@ -34,5 +35,9 @@ define_error! { TransactionFailed [ RoutingError ] | _ | { "transaction processing by modules failed" }, + + Ics03 + [ ics03_connection::error::Error ] + | _ | { "ics03 connection error" } } } diff --git a/crates/ibc/src/relayer/ics18_relayer/utils.rs b/crates/ibc/src/relayer/ics18_relayer/utils.rs index c2d0449f9..349008fc8 100644 --- a/crates/ibc/src/relayer/ics18_relayer/utils.rs +++ b/crates/ibc/src/relayer/ics18_relayer/utils.rs @@ -149,7 +149,7 @@ mod tests { .query_client_full_state(&client_on_b_for_a) .unwrap() .latest_height(); - assert_eq!(client_height_b, ctx_a.query_latest_height()); + assert_eq!(client_height_b, ctx_a.query_latest_height().unwrap()); // Update client on chain B to latest height of B. // - create the client update message with the latest header from B @@ -206,7 +206,7 @@ mod tests { .query_client_full_state(&client_on_a_for_b) .unwrap() .latest_height(); - assert_eq!(client_height_a, ctx_b.query_latest_height()); + assert_eq!(client_height_a, ctx_b.query_latest_height().unwrap()); } } }