From 31841312dd0986f1267fecdf9ba7ca7b32ffb412 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 24 Apr 2024 01:03:47 -0700 Subject: [PATCH 1/7] fix: use working_set for querying at rollup's current height --- .../src/client_state/execution.rs | 8 +- mocks/src/relayer/handle/rollup.rs | 50 +- mocks/src/sovereign/rollup.rs | 6 +- mocks/src/tests/client.rs | 4 +- modules/sov-ibc/src/context.rs | 10 +- modules/sov-ibc/src/rpc/helpers.rs | 32 +- modules/sov-ibc/src/rpc/methods.rs | 541 +++--------------- modules/sov-ibc/src/rpc/mod.rs | 1 + modules/sov-ibc/src/rpc/responses.rs | 475 +++++++++++++++ 9 files changed, 617 insertions(+), 510 deletions(-) create mode 100644 modules/sov-ibc/src/rpc/responses.rs diff --git a/clients/sov-celestia/src/client_state/execution.rs b/clients/sov-celestia/src/client_state/execution.rs index 25c3c572..ca6f8a9c 100644 --- a/clients/sov-celestia/src/client_state/execution.rs +++ b/clients/sov-celestia/src/client_state/execution.rs @@ -224,10 +224,10 @@ where upgraded_client_state.zero_custom_fields(); - // Creates new Sovereign client parameters. The `genesis_da_height` and - // `genesis_state_root` are considered immutable properties of the client. - // Changing them implies creating a client that could potentially be compatible - // with other rollups. + // Creates new Sovereign client parameters. The `genesis_state_root` are + // considered immutable properties of the client. Changing them implies + // creating a client that could potentially be compatible with other + // rollups. let new_sovereign_params = client_state .sovereign_params .update_on_upgrade(upgraded_client_state.sovereign_params); diff --git a/mocks/src/relayer/handle/rollup.rs b/mocks/src/relayer/handle/rollup.rs index 159b5319..72ec4c5f 100644 --- a/mocks/src/relayer/handle/rollup.rs +++ b/mocks/src/relayer/handle/rollup.rs @@ -1,8 +1,7 @@ use async_trait::async_trait; -use ibc_core::client::context::ClientValidationContext; use ibc_core::client::types::Height; use ibc_core::handler::types::events::IbcEvent; -use ibc_core::host::types::path::{ClientConsensusStatePath, Path}; +use ibc_core::host::types::path::Path; use ibc_core::host::ValidationContext; use ibc_query::core::channel::QueryPacketCommitmentRequest; use ibc_query::core::client::{QueryClientStateRequest, QueryConsensusStateRequest}; @@ -42,24 +41,35 @@ where QueryReq::HostConsensusState(height) => { QueryResp::HostConsensusState(ibc_ctx.host_consensus_state(&height).unwrap().into()) } - QueryReq::ClientState(client_id) => QueryResp::ClientState( - ibc_ctx - .get_client_validation_context() - .client_state(&client_id) - .unwrap() - .into(), - ), - QueryReq::ConsensusState(client_id, height) => QueryResp::ConsensusState( - ibc_ctx - .get_client_validation_context() - .consensus_state(&ClientConsensusStatePath::new( - client_id, - height.revision_number(), - height.revision_height(), - )) - .unwrap() - .into(), - ), + QueryReq::ClientState(client_id) => { + let resp = ibc_ctx + .ibc + .client_state( + QueryClientStateRequest { + client_id, + query_height: None, + }, + &mut ibc_ctx.working_set.borrow_mut(), + ) + .unwrap(); + + QueryResp::ClientState(resp.client_state) + } + QueryReq::ConsensusState(client_id, height) => { + let resp = ibc_ctx + .ibc + .consensus_state( + QueryConsensusStateRequest { + client_id, + consensus_height: Some(height), + query_height: None, + }, + &mut ibc_ctx.working_set.borrow_mut(), + ) + .unwrap(); + + QueryResp::ConsensusState(resp.consensus_state) + } QueryReq::Header(target_height, trusted_height) => { let sov_header = self.obtain_ibc_header(target_height, trusted_height); diff --git a/mocks/src/sovereign/rollup.rs b/mocks/src/sovereign/rollup.rs index 33c3d35a..8ccc94ee 100644 --- a/mocks/src/sovereign/rollup.rs +++ b/mocks/src/sovereign/rollup.rs @@ -1,7 +1,5 @@ //! Defines a mock rollup structure with default configurations and specs -use std::cell::RefCell; -use std::rc::Rc; use std::sync::{Arc, Mutex}; use ibc_client_tendermint::types::Header; @@ -108,9 +106,7 @@ where } pub fn ibc_ctx<'a>(&'a self, working_set: &'a mut WorkingSet) -> IbcContext<'a, S> { - let shared_working_set = Rc::new(RefCell::new(working_set)); - - IbcContext::new(&self.runtime.ibc, shared_working_set.clone()) + IbcContext::new(&self.runtime.ibc, working_set) } pub fn obtain_ibc_header(&self, target_height: Height, trusted_height: Height) -> SovTmHeader { diff --git a/mocks/src/tests/client.rs b/mocks/src/tests/client.rs index b1d5002a..50efd535 100644 --- a/mocks/src/tests/client.rs +++ b/mocks/src/tests/client.rs @@ -45,7 +45,7 @@ async fn test_create_client_on_sov() { .src_chain_ctx() .query(QueryReq::ValueWithProof( Path::ClientState(ClientStatePath(rly.dst_client_id().clone())), - Some(client_state.latest_height().increment()), // increment as the proof is available as of the next height + None, )) .await { @@ -64,7 +64,7 @@ async fn test_create_client_on_sov() { revision_number: client_state.latest_height().revision_number(), revision_height: client_state.latest_height().revision_height(), }), - Some(client_state.latest_height().increment()), // increment as the proof is available as of the next height + None, )) .await { diff --git a/modules/sov-ibc/src/context.rs b/modules/sov-ibc/src/context.rs index 7c445fde..cd40eab1 100644 --- a/modules/sov-ibc/src/context.rs +++ b/modules/sov-ibc/src/context.rs @@ -44,11 +44,11 @@ impl<'a, S> IbcContext<'a, S> where S: Spec, { - pub fn new( - ibc: &'a Ibc, - working_set: Rc>>, - ) -> IbcContext<'a, S> { - IbcContext { ibc, working_set } + pub fn new(ibc: &'a Ibc, working_set: &'a mut WorkingSet) -> IbcContext<'a, S> { + IbcContext { + ibc, + working_set: Rc::new(RefCell::new(working_set)), + } } /// Check that the context slot number matches the host height that IBC modules view. diff --git a/modules/sov-ibc/src/rpc/helpers.rs b/modules/sov-ibc/src/rpc/helpers.rs index ef5e9e40..4ea1c658 100644 --- a/modules/sov-ibc/src/rpc/helpers.rs +++ b/modules/sov-ibc/src/rpc/helpers.rs @@ -1,9 +1,5 @@ -use std::cell::RefCell; -use std::rc::Rc; - use borsh::BorshSerialize; use ibc_core::client::types::Height; -use ibc_core::host::ValidationContext; use jsonrpsee::core::RpcResult; use sov_ibc_transfer::to_jsonrpsee_error; use sov_modules_api::{Spec, StateMap, WorkingSet}; @@ -13,22 +9,24 @@ use crate::context::IbcContext; use crate::Ibc; impl Ibc { - /// Determines the query height to use for the given request. If the query - /// height is not provided, it queries the host for the current height. - pub(super) fn determine_query_height( + pub(super) fn handle_request( &self, - query_height: Option, + request_height: Option, working_set: &mut WorkingSet, - ) -> RpcResult { - match query_height { - Some(height) => Ok(height), + method: F, + ) -> Response + where + F: FnOnce(&IbcContext<'_, S>) -> Response, + { + match request_height { + Some(h) => { + let mut archival_working_set = working_set.get_archival_at(h.revision_height()); + let ibc_ctx = IbcContext::new(self, &mut archival_working_set); + method(&ibc_ctx) + } None => { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - ibc_ctx.host_height().map_err(to_jsonrpsee_error) + let ibc_ctx = IbcContext::new(self, working_set); + method(&ibc_ctx) } } } diff --git a/modules/sov-ibc/src/rpc/methods.rs b/modules/sov-ibc/src/rpc/methods.rs index f2249836..393d6804 100644 --- a/modules/sov-ibc/src/rpc/methods.rs +++ b/modules/sov-ibc/src/rpc/methods.rs @@ -1,9 +1,4 @@ //! Defines JSON RPC methods exposed by the ibc module -use std::cell::RefCell; -use std::rc::Rc; - -use ibc_core::client::context::client_state::ClientStateCommon; -use ibc_core::host::ValidationContext; use ibc_query::core::channel::{ query_channels, query_connection_channels, query_packet_acknowledgements, query_packet_commitments, query_unreceived_acks, query_unreceived_packets, @@ -21,9 +16,9 @@ use ibc_query::core::channel::{ }; use ibc_query::core::client::{ query_client_states, query_client_status, query_consensus_state_heights, - query_consensus_states, IdentifiedClientState, QueryClientStateRequest, - QueryClientStateResponse, QueryClientStatesRequest, QueryClientStatesResponse, - QueryClientStatusRequest, QueryClientStatusResponse, QueryConsensusStateHeightsRequest, + query_consensus_states, QueryClientStateRequest, QueryClientStateResponse, + QueryClientStatesRequest, QueryClientStatesResponse, QueryClientStatusRequest, + QueryClientStatusResponse, QueryConsensusStateHeightsRequest, QueryConsensusStateHeightsResponse, QueryConsensusStateRequest, QueryConsensusStateResponse, QueryConsensusStatesRequest, QueryConsensusStatesResponse, QueryUpgradedClientStateRequest, QueryUpgradedClientStateResponse, QueryUpgradedConsensusStateRequest, @@ -42,8 +37,6 @@ use sov_ibc_transfer::to_jsonrpsee_error; use sov_modules_api::macros::rpc_gen; use sov_modules_api::{Spec, WorkingSet}; -use crate::context::IbcContext; -use crate::helpers::{WithProof, WithoutProof}; use crate::Ibc; /// Structure returned by the `client_state` rpc method. @@ -55,24 +48,9 @@ impl Ibc { request: QueryClientStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (client_state, proof) = ibc_ctx.query_client_state::(&request.client_id)?; - - Ok(QueryClientStateResponse::new( - client_state - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Client state not found for client {:?}", - request.client_id - )) - })? - .into(), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.client_state_response(request) + }) } #[rpc_method(name = "clientStates")] @@ -81,12 +59,9 @@ impl Ibc { request: QueryClientStatesRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_client_states(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_client_states(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "consensusState")] @@ -95,34 +70,9 @@ impl Ibc { request: QueryConsensusStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let consensus_height = request.consensus_height.ok_or_else(|| { - to_jsonrpsee_error("Consensus height is required for querying consensus state") - })?; - - let (consensus_state, proof) = ibc_ctx.query_client_consensus_state::( - &request.client_id, - consensus_height.revision_number(), - consensus_height.revision_height(), - )?; - - let proof_height = ibc_ctx.host_height().map_err(to_jsonrpsee_error)?; - - Ok(QueryConsensusStateResponse::new( - consensus_state - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Consensus state not found for client {:?} at height {:?}", - request.client_id, consensus_height - )) - })? - .into(), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.consensus_state_response(request) + }) } #[rpc_method(name = "consensusStates")] @@ -131,12 +81,9 @@ impl Ibc { request: QueryConsensusStatesRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_consensus_states(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_consensus_states(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "consensusStateHeights")] @@ -145,12 +92,9 @@ impl Ibc { request: QueryConsensusStateHeightsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_consensus_state_heights(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_consensus_state_heights(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "clientStatus")] @@ -159,12 +103,9 @@ impl Ibc { request: QueryClientStatusRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_client_status(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_client_status(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "upgradedClientState")] @@ -173,24 +114,9 @@ impl Ibc { request: QueryUpgradedClientStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (upgraded_client_state, proof) = - ibc_ctx.query_upgraded_client_state::(proof_height.revision_height())?; - - Ok(QueryUpgradedClientStateResponse::new( - upgraded_client_state - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Upgraded client state not found at height {proof_height:?}" - )) - })? - .into(), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.upgraded_client_state_response(request) + }) } #[rpc_method(name = "upgradedConsensusState")] @@ -199,24 +125,9 @@ impl Ibc { request: QueryUpgradedConsensusStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (upgraded_consensus_state, proof) = - ibc_ctx.query_upgraded_consensus_state::(proof_height.revision_height())?; - - Ok(QueryUpgradedConsensusStateResponse::new( - upgraded_consensus_state - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Upgraded consensus state not found at height {proof_height:?}" - )) - })? - .into(), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.upgraded_consensus_state_response(request) + }) } #[rpc_method(name = "connection")] @@ -225,23 +136,9 @@ impl Ibc { request: QueryConnectionRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (connection_end, proof) = - ibc_ctx.query_connection_end::(&request.connection_id)?; - - Ok(QueryConnectionResponse::new( - connection_end.ok_or_else(|| { - to_jsonrpsee_error(format!( - "Connection not found for connection id {:?}", - request.connection_id - )) - })?, - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.connection_response(request) + }) } #[rpc_method(name = "connections")] @@ -250,12 +147,9 @@ impl Ibc { request: QueryConnectionsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_connections(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_connections(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "clientConnections")] @@ -264,23 +158,9 @@ impl Ibc { request: QueryClientConnectionsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(None, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (client_connections, proof) = - ibc_ctx.query_client_connections::(&request.client_id)?; - - Ok(QueryClientConnectionsResponse::new( - client_connections.ok_or_else(|| { - to_jsonrpsee_error(format!( - "Client connections not found for client id {:?}", - request.client_id - )) - })?, - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.client_connections_response(request) + }) } #[rpc_method(name = "connectionClientState")] @@ -289,37 +169,9 @@ impl Ibc { request: QueryConnectionClientStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let connection_end = ibc_ctx - .query_connection_end::(&request.connection_id)? - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Connection not found for connection id {:?}", - request.connection_id - )) - })?; - - let (client_state, proof) = - ibc_ctx.query_client_state::(connection_end.client_id())?; - - Ok(QueryConnectionClientStateResponse::new( - IdentifiedClientState::new( - connection_end.client_id().clone(), - client_state - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Client state not found for connection {:?}", - request.connection_id - )) - })? - .into(), - ), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.connection_client_state_response(request) + }) } #[rpc_method(name = "connectionConsensusState")] @@ -328,38 +180,9 @@ impl Ibc { request: QueryConnectionConsensusStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let connection_end = ibc_ctx - .query_connection_end::(&request.connection_id)? - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Connection not found for connection id {:?}", - request.connection_id - )) - })?; - - let (consensus_state, proof) = ibc_ctx.query_client_consensus_state::( - connection_end.client_id(), - request.height.revision_number(), - request.height.revision_height(), - )?; - - Ok(QueryConnectionConsensusStateResponse::new( - consensus_state - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Consensus state not found for connection {:?}", - request.connection_id - )) - })? - .into(), - connection_end.client_id().clone(), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.connection_consensus_state_response(request) + }) } #[rpc_method(name = "connectionParams")] @@ -368,12 +191,9 @@ impl Ibc { request: QueryConnectionParamsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_connection_params(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_connection_params(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "channel")] @@ -382,23 +202,9 @@ impl Ibc { request: QueryChannelRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (channel_end, proof) = - ibc_ctx.query_channel_end::(&request.port_id, &request.channel_id)?; - - Ok(QueryChannelResponse::new( - channel_end.ok_or_else(|| { - to_jsonrpsee_error(format!( - "Channel not found for port id {:?} and channel id {:?}", - request.port_id, request.channel_id - )) - })?, - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.channel_response(request) + }) } #[rpc_method(name = "channels")] @@ -407,12 +213,9 @@ impl Ibc { request: QueryChannelsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_channels(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_channels(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "connectionChannels")] @@ -421,12 +224,9 @@ impl Ibc { request: QueryConnectionChannelsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_connection_channels(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_connection_channels(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "channelClientState")] @@ -435,53 +235,9 @@ impl Ibc { request: QueryChannelClientStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let channel_end = ibc_ctx - .query_channel_end::(&request.port_id, &request.channel_id)? - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Channel not found for port id {:?} and channel id {:?}", - request.port_id, request.channel_id - )) - })?; - - let connection_id = channel_end.connection_hops().first().ok_or_else(|| { - to_jsonrpsee_error(format!( - "ConnectionId not found for channel {:?}", - request.channel_id - )) - })?; - - let connection_end = ibc_ctx - .query_connection_end::(connection_id)? - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "ConnectionEnd not found for channel {:?}", - request.channel_id - )) - })?; - - let (client_state, proof) = - ibc_ctx.query_client_state::(connection_end.client_id())?; - - Ok(QueryChannelClientStateResponse::new( - IdentifiedClientState::new( - connection_end.client_id().clone(), - client_state - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Client state not found for channel {:?}", - request.channel_id - )) - })? - .into(), - ), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.channel_client_state_response(request) + }) } #[rpc_method(name = "channelConsensusState")] @@ -490,63 +246,9 @@ impl Ibc { request: QueryChannelConsensusStateRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let channel_end = ibc_ctx - .query_channel_end::(&request.port_id, &request.channel_id)? - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Channel not found for port id {:?} and channel id {:?}", - request.port_id, request.channel_id - )) - })?; - - let connection_id = channel_end.connection_hops().first().ok_or_else(|| { - to_jsonrpsee_error(format!( - "ConnectionId not found for channel {:?}", - request.channel_id - )) - })?; - - let connection_end = ibc_ctx - .query_connection_end::(connection_id)? - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "ConnectionEnd not found for channel {:?}", - request.channel_id - )) - })?; - - let client_state = ibc_ctx - .query_client_state::(connection_end.client_id())? - .ok_or_else(|| { - to_jsonrpsee_error(format!( - "Client state not found for channel {:?}", - request.channel_id - )) - })?; - - let client_latest_height = client_state.latest_height(); - - let (consensus_state, proof) = ibc_ctx.query_client_consensus_state::( - connection_end.client_id(), - client_latest_height.revision_number(), - client_latest_height.revision_height(), - )?; - - Ok(QueryChannelConsensusStateResponse::new( - consensus_state - .ok_or(to_jsonrpsee_error(format!( - "Consensus state not found for channel {:?}", - request.channel_id - )))? - .into(), - connection_end.client_id().clone(), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.channel_consensus_state_response(request) + }) } #[rpc_method(name = "packetCommitment")] @@ -555,26 +257,9 @@ impl Ibc { request: QueryPacketCommitmentRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (commitment, proof) = ibc_ctx.query_packet_commitment::( - &request.port_id, - &request.channel_id, - request.sequence, - )?; - - Ok(QueryPacketCommitmentResponse::new( - commitment.ok_or_else(|| { - to_jsonrpsee_error(format!( - "Packet commitment not found for port id {:?}, channel id {:?} and sequence {:?}", - request.port_id, request.channel_id, request.sequence - )) - })?, - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.packet_commitment_response(request) + }) } #[rpc_method(name = "packetCommitments")] @@ -583,12 +268,9 @@ impl Ibc { request: QueryPacketCommitmentsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_packet_commitments(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_packet_commitments(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "packetReceipt")] @@ -597,24 +279,9 @@ impl Ibc { request: QueryPacketReceiptRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (receipt, proof) = ibc_ctx.query_packet_receipt::( - &request.port_id, - &request.channel_id, - request.sequence, - )?; - - // packet_receipt_map models a set using constant unit value. - // when the key (doesn't) exists in the map, - // the receipt is (not) present and returns a (non) membership proof - Ok(QueryPacketReceiptResponse::new( - receipt.is_some(), - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.packet_receipt_response(request) + }) } #[rpc_method(name = "packetAcknowledgement")] @@ -623,26 +290,9 @@ impl Ibc { request: QueryPacketAcknowledgementRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (acknowledgement, proof) = ibc_ctx.query_packet_acknowledgement::( - &request.port_id, - &request.channel_id, - request.sequence, - )?; - - Ok(QueryPacketAcknowledgementResponse::new( - acknowledgement.ok_or_else(|| { - to_jsonrpsee_error(format!( - "Packet acknowledgement not found for port id {:?}, channel id {:?} and sequence {:?}", - request.port_id, request.channel_id, request.sequence - )) - })?, - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.packet_acknowledgement_response(request) + }) } #[rpc_method(name = "packetAcknowledgements")] @@ -651,12 +301,9 @@ impl Ibc { request: QueryPacketAcknowledgementsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_packet_acknowledgements(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_packet_acknowledgements(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "unreceivedPackets")] @@ -665,12 +312,9 @@ impl Ibc { request: QueryUnreceivedPacketsRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_unreceived_packets(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_unreceived_packets(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "unreceivedAcks")] @@ -679,12 +323,9 @@ impl Ibc { request: QueryUnreceivedAcksRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let ibc_ctx = IbcContext { - ibc: self, - working_set: Rc::new(RefCell::new(working_set)), - }; - - query_unreceived_acks(&ibc_ctx, &request).map_err(to_jsonrpsee_error) + self.handle_request(None, working_set, |ibc_ctx| { + query_unreceived_acks(ibc_ctx, &request).map_err(to_jsonrpsee_error) + }) } #[rpc_method(name = "nextSequenceReceive")] @@ -693,22 +334,8 @@ impl Ibc { request: QueryNextSequenceReceiveRequest, working_set: &mut WorkingSet, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height, working_set)?; - let mut archival_working_set = working_set.get_archival_at(proof_height.revision_height()); - let ibc_ctx = IbcContext::new(self, Rc::new(RefCell::new(&mut archival_working_set))); - - let (sequence, proof) = - ibc_ctx.query_recv_sequence::(&request.port_id, &request.channel_id)?; - - Ok(QueryNextSequenceReceiveResponse::new( - sequence.ok_or_else(|| { - to_jsonrpsee_error(format!( - "Next sequence receive not found for port id {:?} and channel id {:?}", - request.port_id, request.channel_id - )) - })?, - proof, - proof_height, - )) + self.handle_request(request.query_height, working_set, |ibc_ctx| { + ibc_ctx.next_sequence_receive_response(request) + }) } } diff --git a/modules/sov-ibc/src/rpc/mod.rs b/modules/sov-ibc/src/rpc/mod.rs index 5d9a2bb3..5fc3392a 100644 --- a/modules/sov-ibc/src/rpc/mod.rs +++ b/modules/sov-ibc/src/rpc/mod.rs @@ -1,5 +1,6 @@ pub mod context; pub mod helpers; pub mod methods; +pub mod responses; pub use methods::*; diff --git a/modules/sov-ibc/src/rpc/responses.rs b/modules/sov-ibc/src/rpc/responses.rs new file mode 100644 index 00000000..5f7a1f1d --- /dev/null +++ b/modules/sov-ibc/src/rpc/responses.rs @@ -0,0 +1,475 @@ +use ibc_core::client::context::client_state::ClientStateCommon; +use ibc_core::client::types::Height; +use ibc_core::host::ValidationContext; +use ibc_query::core::channel::{ + QueryChannelClientStateRequest, QueryChannelClientStateResponse, + QueryChannelConsensusStateRequest, QueryChannelConsensusStateResponse, QueryChannelRequest, + QueryChannelResponse, QueryNextSequenceReceiveRequest, QueryNextSequenceReceiveResponse, + QueryPacketAcknowledgementRequest, QueryPacketAcknowledgementResponse, + QueryPacketCommitmentRequest, QueryPacketCommitmentResponse, QueryPacketReceiptRequest, + QueryPacketReceiptResponse, +}; +use ibc_query::core::client::{ + IdentifiedClientState, QueryClientStateRequest, QueryClientStateResponse, + QueryConsensusStateRequest, QueryConsensusStateResponse, QueryUpgradedClientStateRequest, + QueryUpgradedClientStateResponse, QueryUpgradedConsensusStateRequest, + QueryUpgradedConsensusStateResponse, +}; +use ibc_query::core::connection::{ + QueryClientConnectionsRequest, QueryClientConnectionsResponse, + QueryConnectionClientStateRequest, QueryConnectionClientStateResponse, + QueryConnectionConsensusStateRequest, QueryConnectionConsensusStateResponse, + QueryConnectionRequest, QueryConnectionResponse, +}; +use jsonrpsee::core::RpcResult; +use sov_ibc_transfer::to_jsonrpsee_error; +use sov_modules_api::Spec; + +use crate::context::IbcContext; +use crate::helpers::{WithProof, WithoutProof}; + +impl<'a, S: Spec> IbcContext<'a, S> { + /// Determines the query height to use for the given request. If the query + /// height is not provided, it queries the host for the current height. + pub(super) fn determine_query_height(&self, query_height: Option) -> RpcResult { + let height = match query_height { + Some(height) => height, + None => self.host_height().map_err(to_jsonrpsee_error)?, + }; + + Ok(height) + } + + pub(super) fn client_state_response( + &self, + request: QueryClientStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (client_state, proof) = self.query_client_state::(&request.client_id)?; + + Ok(QueryClientStateResponse::new( + client_state + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Client state not found for client {:?}", + request.client_id + )) + })? + .into(), + proof, + proof_height, + )) + } + + pub(super) fn consensus_state_response( + &self, + request: QueryConsensusStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let consensus_height = request.consensus_height.ok_or_else(|| { + to_jsonrpsee_error("Consensus height is required for querying consensus state") + })?; + + let (consensus_state, proof) = self.query_client_consensus_state::( + &request.client_id, + consensus_height.revision_number(), + consensus_height.revision_height(), + )?; + + Ok(QueryConsensusStateResponse::new( + consensus_state + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Consensus state not found for client {:?} at height {:?}", + request.client_id, consensus_height + )) + })? + .into(), + proof, + proof_height, + )) + } + + pub(super) fn upgraded_client_state_response( + &self, + request: QueryUpgradedClientStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (upgraded_client_state, proof) = + self.query_upgraded_client_state::(proof_height.revision_height())?; + + Ok(QueryUpgradedClientStateResponse::new( + upgraded_client_state + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Upgraded client state not found at height {proof_height:?}" + )) + })? + .into(), + proof, + proof_height, + )) + } + + pub(super) fn upgraded_consensus_state_response( + &self, + request: QueryUpgradedConsensusStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (upgraded_consensus_state, proof) = + self.query_upgraded_consensus_state::(proof_height.revision_height())?; + + Ok(QueryUpgradedConsensusStateResponse::new( + upgraded_consensus_state + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Upgraded consensus state not found at height {proof_height:?}" + )) + })? + .into(), + proof, + proof_height, + )) + } + + pub(super) fn connection_response( + &self, + request: QueryConnectionRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (connection_end, proof) = + self.query_connection_end::(&request.connection_id)?; + + Ok(QueryConnectionResponse::new( + connection_end.ok_or_else(|| { + to_jsonrpsee_error(format!( + "Connection not found for connection id {:?}", + request.connection_id + )) + })?, + proof, + proof_height, + )) + } + + pub(super) fn client_connections_response( + &self, + request: QueryClientConnectionsRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (client_connections, proof) = + self.query_client_connections::(&request.client_id)?; + + Ok(QueryClientConnectionsResponse::new( + client_connections.ok_or_else(|| { + to_jsonrpsee_error(format!( + "Client connections not found for client id {:?}", + request.client_id + )) + })?, + proof, + proof_height, + )) + } + + pub(super) fn connection_client_state_response( + &self, + request: QueryConnectionClientStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let connection_end = self + .query_connection_end::(&request.connection_id)? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Connection not found for connection id {:?}", + request.connection_id + )) + })?; + + let (client_state, proof) = + self.query_client_state::(connection_end.client_id())?; + + Ok(QueryConnectionClientStateResponse::new( + IdentifiedClientState::new( + connection_end.client_id().clone(), + client_state + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Client state not found for connection {:?}", + request.connection_id + )) + })? + .into(), + ), + proof, + proof_height, + )) + } + + pub(super) fn connection_consensus_state_response( + &self, + request: QueryConnectionConsensusStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let connection_end = self + .query_connection_end::(&request.connection_id)? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Connection not found for connection id {:?}", + request.connection_id + )) + })?; + + let (consensus_state, proof) = self.query_client_consensus_state::( + connection_end.client_id(), + request.height.revision_number(), + request.height.revision_height(), + )?; + + Ok(QueryConnectionConsensusStateResponse::new( + consensus_state + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Consensus state not found for connection {:?}", + request.connection_id + )) + })? + .into(), + connection_end.client_id().clone(), + proof, + proof_height, + )) + } + + pub(super) fn channel_response( + &self, + request: QueryChannelRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (channel_end, proof) = + self.query_channel_end::(&request.port_id, &request.channel_id)?; + + Ok(QueryChannelResponse::new( + channel_end.ok_or_else(|| { + to_jsonrpsee_error(format!( + "Channel not found for port id {:?} and channel id {:?}", + request.port_id, request.channel_id + )) + })?, + proof, + proof_height, + )) + } + + pub(super) fn channel_client_state_response( + &self, + request: QueryChannelClientStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let channel_end = self + .query_channel_end::(&request.port_id, &request.channel_id)? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Channel not found for port id {:?} and channel id {:?}", + request.port_id, request.channel_id + )) + })?; + + let connection_id = channel_end.connection_hops().first().ok_or_else(|| { + to_jsonrpsee_error(format!( + "ConnectionId not found for channel {:?}", + request.channel_id + )) + })?; + + let connection_end: ibc_core::connection::types::ConnectionEnd = self + .query_connection_end::(connection_id)? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "ConnectionEnd not found for channel {:?}", + request.channel_id + )) + })?; + + let (client_state, proof) = + self.query_client_state::(connection_end.client_id())?; + + Ok(QueryChannelClientStateResponse::new( + IdentifiedClientState::new( + connection_end.client_id().clone(), + client_state + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Client state not found for channel {:?}", + request.channel_id + )) + })? + .into(), + ), + proof, + proof_height, + )) + } + + pub(super) fn channel_consensus_state_response( + &self, + request: QueryChannelConsensusStateRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let channel_end = self + .query_channel_end::(&request.port_id, &request.channel_id)? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Channel not found for port id {:?} and channel id {:?}", + request.port_id, request.channel_id + )) + })?; + + let connection_id = channel_end.connection_hops().first().ok_or_else(|| { + to_jsonrpsee_error(format!( + "ConnectionId not found for channel {:?}", + request.channel_id + )) + })?; + + let connection_end = self + .query_connection_end::(connection_id)? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "ConnectionEnd not found for channel {:?}", + request.channel_id + )) + })?; + + let client_state = self + .query_client_state::(connection_end.client_id())? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Client state not found for channel {:?}", + request.channel_id + )) + })?; + + let client_latest_height = client_state.latest_height(); + + let (consensus_state, proof) = self.query_client_consensus_state::( + connection_end.client_id(), + client_latest_height.revision_number(), + client_latest_height.revision_height(), + )?; + + Ok(QueryChannelConsensusStateResponse::new( + consensus_state + .ok_or(to_jsonrpsee_error(format!( + "Consensus state not found for channel {:?}", + request.channel_id + )))? + .into(), + connection_end.client_id().clone(), + proof, + proof_height, + )) + } + + pub(super) fn packet_commitment_response( + &self, + request: QueryPacketCommitmentRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (commitment, proof) = self.query_packet_commitment::( + &request.port_id, + &request.channel_id, + request.sequence, + )?; + + Ok(QueryPacketCommitmentResponse::new( + commitment.ok_or_else(|| { + to_jsonrpsee_error(format!( + "Packet commitment not found for port id {:?}, channel id {:?} and sequence {:?}", + request.port_id, request.channel_id, request.sequence + )) + })?, + proof, + proof_height, + )) + } + + pub(super) fn packet_receipt_response( + &self, + request: QueryPacketReceiptRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (receipt, proof) = self.query_packet_receipt::( + &request.port_id, + &request.channel_id, + request.sequence, + )?; + + // packet_receipt_map models a set using constant unit value. + // when the key (doesn't) exists in the map, + // the receipt is (not) present and returns a (non) membership proof + Ok(QueryPacketReceiptResponse::new( + receipt.is_some(), + proof, + proof_height, + )) + } + + pub(super) fn packet_acknowledgement_response( + &self, + request: QueryPacketAcknowledgementRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (acknowledgement, proof) = self.query_packet_acknowledgement::( + &request.port_id, + &request.channel_id, + request.sequence, + )?; + + Ok(QueryPacketAcknowledgementResponse::new( + acknowledgement.ok_or_else(|| { + to_jsonrpsee_error(format!( + "Packet acknowledgement not found for port id {:?}, channel id {:?} and sequence {:?}", + request.port_id, request.channel_id, request.sequence + )) + })?, + proof, + proof_height, + )) + } + + pub(super) fn next_sequence_receive_response( + &self, + request: QueryNextSequenceReceiveRequest, + ) -> RpcResult { + let proof_height = self.determine_query_height(request.query_height)?; + + let (sequence, proof) = + self.query_recv_sequence::(&request.port_id, &request.channel_id)?; + + Ok(QueryNextSequenceReceiveResponse::new( + sequence.ok_or_else(|| { + to_jsonrpsee_error(format!( + "Next sequence receive not found for port id {:?} and channel id {:?}", + request.port_id, request.channel_id + )) + })?, + proof, + proof_height, + )) + } +} From 79162d425f55ca27dd162bae0bec181caebc8dfc Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Wed, 24 Apr 2024 11:58:58 +0200 Subject: [PATCH 2/7] update comment --- clients/sov-celestia/src/client_state/execution.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/clients/sov-celestia/src/client_state/execution.rs b/clients/sov-celestia/src/client_state/execution.rs index ca6f8a9c..4a0a7b32 100644 --- a/clients/sov-celestia/src/client_state/execution.rs +++ b/clients/sov-celestia/src/client_state/execution.rs @@ -224,10 +224,11 @@ where upgraded_client_state.zero_custom_fields(); - // Creates new Sovereign client parameters. The `genesis_state_root` are - // considered immutable properties of the client. Changing them implies - // creating a client that could potentially be compatible with other - // rollups. + // Creates new Sovereign client parameters. + // + // The `genesis_state_root` are considered the unique and immutable + // properties of the client. Changing them implies creating a client + // that could potentially be compatible with other rollups. let new_sovereign_params = client_state .sovereign_params .update_on_upgrade(upgraded_client_state.sovereign_params); From 7a66b2a78b03e2377b0e1e9da0773b57524c5420 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Wed, 24 Apr 2024 12:10:01 +0200 Subject: [PATCH 3/7] code refactor --- mocks/src/relayer/handle/rollup.rs | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/mocks/src/relayer/handle/rollup.rs b/mocks/src/relayer/handle/rollup.rs index 72ec4c5f..3d55d4c7 100644 --- a/mocks/src/relayer/handle/rollup.rs +++ b/mocks/src/relayer/handle/rollup.rs @@ -42,30 +42,28 @@ where QueryResp::HostConsensusState(ibc_ctx.host_consensus_state(&height).unwrap().into()) } QueryReq::ClientState(client_id) => { + let req = QueryClientStateRequest { + client_id, + query_height: None, + }; + let resp = ibc_ctx .ibc - .client_state( - QueryClientStateRequest { - client_id, - query_height: None, - }, - &mut ibc_ctx.working_set.borrow_mut(), - ) + .client_state(req, &mut ibc_ctx.working_set.borrow_mut()) .unwrap(); QueryResp::ClientState(resp.client_state) } QueryReq::ConsensusState(client_id, height) => { + let req = QueryConsensusStateRequest { + client_id, + consensus_height: Some(height), + query_height: None, + }; + let resp = ibc_ctx .ibc - .consensus_state( - QueryConsensusStateRequest { - client_id, - consensus_height: Some(height), - query_height: None, - }, - &mut ibc_ctx.working_set.borrow_mut(), - ) + .consensus_state(req, &mut ibc_ctx.working_set.borrow_mut()) .unwrap(); QueryResp::ConsensusState(resp.consensus_state) From 1b6113debefe9e30cfaa6ca76a5299bc68def5fe Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Wed, 24 Apr 2024 12:12:33 +0200 Subject: [PATCH 4/7] use ok_or_else --- modules/sov-ibc/src/rpc/responses.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/modules/sov-ibc/src/rpc/responses.rs b/modules/sov-ibc/src/rpc/responses.rs index 5f7a1f1d..2075cf2f 100644 --- a/modules/sov-ibc/src/rpc/responses.rs +++ b/modules/sov-ibc/src/rpc/responses.rs @@ -371,10 +371,12 @@ impl<'a, S: Spec> IbcContext<'a, S> { Ok(QueryChannelConsensusStateResponse::new( consensus_state - .ok_or(to_jsonrpsee_error(format!( - "Consensus state not found for channel {:?}", - request.channel_id - )))? + .ok_or_else(|| { + to_jsonrpsee_error(format!( + "Consensus state not found for channel {:?}", + request.channel_id + )) + })? .into(), connection_end.client_id().clone(), proof, From ac64484a20c2633e3343c35d7bbffc912a067e77 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Fri, 26 Apr 2024 14:35:09 +0200 Subject: [PATCH 5/7] code opt --- modules/sov-ibc/src/rpc/responses.rs | 46 ++++++++++------------------ 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/modules/sov-ibc/src/rpc/responses.rs b/modules/sov-ibc/src/rpc/responses.rs index 2075cf2f..184f8a14 100644 --- a/modules/sov-ibc/src/rpc/responses.rs +++ b/modules/sov-ibc/src/rpc/responses.rs @@ -1,5 +1,4 @@ use ibc_core::client::context::client_state::ClientStateCommon; -use ibc_core::client::types::Height; use ibc_core::host::ValidationContext; use ibc_query::core::channel::{ QueryChannelClientStateRequest, QueryChannelClientStateResponse, @@ -29,22 +28,11 @@ use crate::context::IbcContext; use crate::helpers::{WithProof, WithoutProof}; impl<'a, S: Spec> IbcContext<'a, S> { - /// Determines the query height to use for the given request. If the query - /// height is not provided, it queries the host for the current height. - pub(super) fn determine_query_height(&self, query_height: Option) -> RpcResult { - let height = match query_height { - Some(height) => height, - None => self.host_height().map_err(to_jsonrpsee_error)?, - }; - - Ok(height) - } - pub(super) fn client_state_response( &self, request: QueryClientStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (client_state, proof) = self.query_client_state::(&request.client_id)?; @@ -66,7 +54,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryConsensusStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let consensus_height = request.consensus_height.ok_or_else(|| { to_jsonrpsee_error("Consensus height is required for querying consensus state") @@ -94,9 +82,9 @@ impl<'a, S: Spec> IbcContext<'a, S> { pub(super) fn upgraded_client_state_response( &self, - request: QueryUpgradedClientStateRequest, + _request: QueryUpgradedClientStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (upgraded_client_state, proof) = self.query_upgraded_client_state::(proof_height.revision_height())?; @@ -116,9 +104,9 @@ impl<'a, S: Spec> IbcContext<'a, S> { pub(super) fn upgraded_consensus_state_response( &self, - request: QueryUpgradedConsensusStateRequest, + _request: QueryUpgradedConsensusStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (upgraded_consensus_state, proof) = self.query_upgraded_consensus_state::(proof_height.revision_height())?; @@ -140,7 +128,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryConnectionRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (connection_end, proof) = self.query_connection_end::(&request.connection_id)?; @@ -161,7 +149,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryClientConnectionsRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (client_connections, proof) = self.query_client_connections::(&request.client_id)?; @@ -182,7 +170,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryConnectionClientStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let connection_end = self .query_connection_end::(&request.connection_id)? @@ -217,7 +205,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryConnectionConsensusStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let connection_end = self .query_connection_end::(&request.connection_id)? @@ -253,7 +241,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryChannelRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (channel_end, proof) = self.query_channel_end::(&request.port_id, &request.channel_id)?; @@ -274,7 +262,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryChannelClientStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let channel_end = self .query_channel_end::(&request.port_id, &request.channel_id)? @@ -325,7 +313,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryChannelConsensusStateRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let channel_end = self .query_channel_end::(&request.port_id, &request.channel_id)? @@ -388,7 +376,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryPacketCommitmentRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (commitment, proof) = self.query_packet_commitment::( &request.port_id, @@ -412,7 +400,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryPacketReceiptRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (receipt, proof) = self.query_packet_receipt::( &request.port_id, @@ -434,7 +422,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryPacketAcknowledgementRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (acknowledgement, proof) = self.query_packet_acknowledgement::( &request.port_id, @@ -458,7 +446,7 @@ impl<'a, S: Spec> IbcContext<'a, S> { &self, request: QueryNextSequenceReceiveRequest, ) -> RpcResult { - let proof_height = self.determine_query_height(request.query_height)?; + let proof_height = self.host_height().map_err(to_jsonrpsee_error)?; let (sequence, proof) = self.query_recv_sequence::(&request.port_id, &request.channel_id)?; From 740be4543689e9cd07a9b113aedaa23d3724575f Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Fri, 26 Apr 2024 14:39:33 +0200 Subject: [PATCH 6/7] add doc comments --- modules/sov-ibc/src/rpc/responses.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/sov-ibc/src/rpc/responses.rs b/modules/sov-ibc/src/rpc/responses.rs index 184f8a14..943da818 100644 --- a/modules/sov-ibc/src/rpc/responses.rs +++ b/modules/sov-ibc/src/rpc/responses.rs @@ -27,6 +27,13 @@ use sov_modules_api::Spec; use crate::context::IbcContext; use crate::helpers::{WithProof, WithoutProof}; +/// The implementation of the IBC RPC methods. +/// +/// The context is already created with: +/// - The working set at the query height, if provided. +/// - The latest working set. +/// +/// So we can ignore the `query_height` parameter. impl<'a, S: Spec> IbcContext<'a, S> { pub(super) fn client_state_response( &self, From 45cebd68a5039d66cdddb0022c7351766c08cece Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Fri, 26 Apr 2024 14:48:43 +0200 Subject: [PATCH 7/7] add comment --- modules/sov-ibc/src/rpc/helpers.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/sov-ibc/src/rpc/helpers.rs b/modules/sov-ibc/src/rpc/helpers.rs index 4ea1c658..4b2e855c 100644 --- a/modules/sov-ibc/src/rpc/helpers.rs +++ b/modules/sov-ibc/src/rpc/helpers.rs @@ -20,6 +20,9 @@ impl Ibc { { match request_height { Some(h) => { + // note: IbcContext doesn't take ownership of the archival_working_set. + // So it can't be returned from the this method. Instead, a closure is + // passed to the method and the result is returned from the closure. let mut archival_working_set = working_set.get_archival_at(h.revision_height()); let ibc_ctx = IbcContext::new(self, &mut archival_working_set); method(&ibc_ctx)