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

Consolidate ChainEndpoint::proven_*() functions with their analogous query_*() version #2223

Closed
plafer opened this issue May 18, 2022 · 0 comments · Fixed by #2226
Closed
Assignees
Milestone

Comments

@plafer
Copy link
Contributor

plafer commented May 18, 2022

I just noticed that the ChainEndpoint::proven_*() methods each have an analogous query_*() method which performs the same query except that they don't return a proof.

I suggest we remove the proven_*() methods from ChainEndpoint and ChainHandle, and change the provable query_*() functions as follows:

// previous
fn query_channel(&self, request: QueryChannelRequest) -> Result<ChannelEnd, Error>;
fn proven_channel(&self, request: QueryChannelRequest) -> Result<(ChannelEnd, MerkleProof), Error>;

// new
fn query_channel(&self, request: QueryChannelRequest, include_proof: PerformQuery) -> Result<(ChannelEnd, Option<MerkleProof>), Error>;

enum PerformQuery {
    WithProof,
    WithoutProof,
};

I introduced PerformQuery to avoid boolean blindness.

Example calls:

chain.query_channel(request, PerformQuery::WithProof);
chain.query_channel(request, PerformQuery::WithoutProof);

An alternative name for PerformQuery I thought of:

enum IncludeProof {
    Yes,
    No,
};
chain.query_channel(request, IncludeProof::Yes);
chain.query_channel(request, IncludeProof::No);

I personally prefer merging the two functions into one, as we expose fewer methods while preserving the same functionality.

Originally posted by @plafer in #2200 (comment)

@plafer plafer self-assigned this May 18, 2022
@adizere adizere added this to the v1.0.0 milestone Jun 17, 2022
@adizere adizere moved this to Backlog in IBC-rs: the road to v1 Jun 17, 2022
@adizere adizere moved this from Backlog to Closed in IBC-rs: the road to v1 Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

2 participants