-
Notifications
You must be signed in to change notification settings - Fork 326
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
Consolidate ChainEndpoint/Handle proven queries #2226
Conversation
…date-chain-query-proven
…date-chain-query-proven
where appropriate
…date-chain-query-proven
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preemptively approved, thanks a lot for going through this @plafer!
It's a bit unfortunate that we now cannot statically rely on the proof being present in the returned type when IncludeProof
is Yes
but we can perhaps find a way to recover this statically in a follow-up PR, perhaps with something like:
trait WithProof<Proof> {
type Proof;
fn include_proof(&self) -> bool;
fn build_proof<E>(&self, build: impl FnOnce() -> Result<Proof, E>) -> Result<Self::Proof, E>;
}
struct IncludeProof;
struct NoProof;
impl<Proof> WithProof<Proof> for IncludeProof {
type Proof = Proof;
fn include_proof(&self) -> bool { true }
fn build_proof<E>(&self, build: impl FnOnce() -> Result<Proof, E>) -> Result<Self::Proof, E> {
build()
}
}
impl<Proof> WithProof<Proof> for NoProof {
type Proof = ();
fn include_proof(&self) -> bool { false }
fn build_proof<E>(&self, _build: impl FnOnce() -> Result<Proof, E>) -> Result<Self::Proof, E> {
Ok(())
}
}
impl ChainEndpoint for CosmosSdkChain {
fn query_channel<P: WithProof<MerkleProof>>(
&self,
request: QueryChannelRequest,
with_proof: P,
) -> Result<(ChannelEnd, P::Proof), Error> {
// ...
let res = self.query(
ChannelEndsPath(request.port_id, request.channel_id),
request.height,
with_proof.include_proof(),
)?;
let channel_end = ChannelEnd::decode_vec(&res.value).map_err(Error::decode)?;
let proof = with_proof.build_proof(|| res.proof.ok_or_else(Error::empty_response_proof))?;
Ok(channel_end, proof)
}
}
Not clear how that would interact with the chain runtime, but we might be able to solve this in one go once we move the queries out of the runtime and expose them directly from the ChainHandle
.
@plafer @soareschen What do you think?
…date-chain-query-proven
…om:informalsystems/ibc-rs into plafer/2223-consolidate-chain-query-proven
* Remove TODO * query_client_state consolidation * query_connection consolidation * query_channel consolidation * query_packet_commitment function * query_packet_acknowledgement * query_packet_receipt * query_next_sequence_receive * use request instead of build_packet_proofs where appropriate * build_packet_proofs now only returns proofs * build_packet_proofs no longer uses proven_packet * proven_packet removed * query_consensus_state * remove proven_client_consensus * cleanup * docstrings * changelog * fix compilation errors * mock consensus_state query Co-authored-by: Romain Ruetschi <romain@informal.systems>
Closes: #2223
Remaining:
proven_client_consensus()
PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.