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

Fix host_height, host_timestamp return value #243

Merged
merged 11 commits into from
Nov 22, 2022
Original file line number Diff line number Diff line change
@@ -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))
12 changes: 8 additions & 4 deletions crates/ibc/src/clients/ics07_tendermint/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions crates/ibc/src/core/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ pub trait ValidationContext {
) -> Result<Option<Box<dyn ConsensusState>>, ClientError>;

/// Returns the current height of the local chain.
fn host_height(&self) -> Height;
fn host_height(&self) -> Result<Height, ClientError>;

/// Returns the current timestamp of the local chain.
fn host_timestamp(&self) -> Timestamp {
fn host_timestamp(&self) -> Result<Timestamp, ClientError> {
Copy link
Contributor

Choose a reason for hiding this comment

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

ClientError is not appropriate here, but we don't have a good solution for that at the moment. We would really need some HostError. We can fix this with #164.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would also like to use this way to solve, it is not very clear that the current no_std support is not very perfect

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.
Expand Down
6 changes: 3 additions & 3 deletions crates/ibc/src/core/ics02_client/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ pub trait ClientReader {
) -> Result<Option<Box<dyn ConsensusState>>, Error>;

/// Returns the current height of the local chain.
fn host_height(&self) -> Height;
fn host_height(&self) -> Result<Height, Error>;

/// Returns the current timestamp of the local chain.
fn host_timestamp(&self) -> Timestamp {
fn host_timestamp(&self) -> Result<Timestamp, Error> {
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.
Expand Down
8 changes: 4 additions & 4 deletions crates/ibc/src/core/ics02_client/handler/create_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -141,8 +141,8 @@ pub fn process(ctx: &dyn ClientReader, msg: MsgCreateClient) -> HandlerResult<Cl
client_type: client_type.clone(),
client_state,
consensus_state,
processed_time: ctx.host_timestamp(),
processed_height: ctx.host_height(),
processed_time: ctx.host_timestamp()?,
processed_height: ctx.host_height()?,
});

output.emit(IbcEvent::CreateClient(CreateClient::new(
Expand Down
12 changes: 6 additions & 6 deletions crates/ibc/src/core/ics02_client/handler/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ where

debug!("latest consensus state: {:?}", latest_consensus_state);

let now = ctx.host_timestamp();
let now = ctx.host_timestamp()?;
let duration = now
.duration_since(&latest_consensus_state.timestamp())
.ok_or_else(|| {
Expand Down Expand Up @@ -109,12 +109,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()?,
)?;

{
Expand Down Expand Up @@ -160,7 +160,7 @@ pub fn process<Ctx: ClientReader>(

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(|| {
Expand Down Expand Up @@ -191,8 +191,8 @@ pub fn process<Ctx: ClientReader>(
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(
Expand Down
4 changes: 2 additions & 2 deletions crates/ibc/src/core/ics03_connection/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ pub trait ConnectionReader {
fn decode_client_state(&self, client_state: Any) -> Result<Box<dyn ClientState>, Error>;

/// Returns the current height of the local chain.
fn host_current_height(&self) -> Height;
fn host_current_height(&self) -> Result<Height, Error>;

#[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<Height, Error>;

/// Returns the prefix that the local chain uses in the KV store.
fn commitment_prefix(&self) -> CommitmentPrefix;
Expand Down
4 changes: 2 additions & 2 deletions crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ pub(crate) fn process(
) -> HandlerResult<ConnectionResult, Error> {
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()?,
));
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?,
));
}

Expand Down
6 changes: 3 additions & 3 deletions crates/ibc/src/core/ics04_channel/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,14 @@ pub trait ChannelReader {
fn hash(&self, value: Vec<u8>) -> Vec<u8>;

/// Returns the current height of the local chain.
fn host_height(&self) -> Height;
fn host_height(&self) -> Result<Height, Error>;

/// Returns the current timestamp of the local chain.
fn host_timestamp(&self) -> Timestamp {
fn host_timestamp(&self) -> Result<Timestamp, Error> {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions crates/ibc/src/core/ics04_channel/handler/recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ pub fn process<Ctx: ChannelReader>(
));
}

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,
packet.timeout_height,
));
}

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());
}
Expand Down Expand Up @@ -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();

Expand Down
33 changes: 17 additions & 16 deletions crates/ibc/src/mock/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Height, Ics04Error> {
Ok(self.latest_height())
}

fn host_timestamp(&self) -> Timestamp {
ClientReader::host_timestamp(self)
fn host_timestamp(&self) -> Result<Timestamp, Ics04Error> {
ClientReader::host_timestamp(self).map_err(|e| Ics04Error::other(e.to_string()))
}

fn host_consensus_state(&self, height: Height) -> Result<Box<dyn ConsensusState>, Ics04Error> {
Expand Down Expand Up @@ -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<Height, Ics03Error> {
Ok(self.latest_height())
}

fn host_oldest_height(&self) -> Height {
fn host_oldest_height(&self) -> Result<Height, Ics03Error> {
// 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 {
Expand Down Expand Up @@ -1275,17 +1275,18 @@ impl ClientReader for MockContext {
Ok(None)
}

fn host_height(&self) -> Height {
self.latest_height()
fn host_height(&self) -> Result<Height, Ics02Error> {
Ok(self.latest_height())
}

fn host_timestamp(&self) -> Timestamp {
self.history
fn host_timestamp(&self) -> Result<Timestamp, Ics02Error> {
Ok(self
.history
.last()
.expect("history cannot be empty")
.timestamp()
.add(self.block_time)
.unwrap()
.unwrap())
}

fn host_consensus_state(&self, height: Height) -> Result<Box<dyn ConsensusState>, Ics02Error> {
Expand Down Expand Up @@ -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<Height, Ics18Error> {
self.host_current_height().map_err(Ics18Error::ics03)
}

fn query_client_full_state(&self, client_id: &ClientId) -> Option<Box<dyn ClientState>> {
Expand All @@ -1411,7 +1412,7 @@ impl RelayerContext for MockContext {
}

fn query_latest_header(&self) -> Option<Box<dyn Header>> {
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)
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ibc/src/relayer/ics18_relayer/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Height, Error>;

/// Returns this client state for the given `client_id` on this chain.
/// Wrapper over the `/abci_query?path=..` endpoint.
Expand Down
5 changes: 5 additions & 0 deletions crates/ibc/src/relayer/ics18_relayer/error.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -34,5 +35,9 @@ define_error! {
TransactionFailed
[ RoutingError ]
| _ | { "transaction processing by modules failed" },

Ics03
[ ics03_connection::error::Error ]
| _ | { "ics03 connection error" }
}
}
4 changes: 2 additions & 2 deletions crates/ibc/src/relayer/ics18_relayer/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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());
}
}
}