Closed Farhad-Shabani closed 3 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 64.06%. Comparing base (
49e907a
) to head (984f5cd
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Good catches! Additionally, I realized:
impl Protobuf<RawType> for DomainType
is not required.request.rs
only requires impl TryFrom<RawType> for DomainType
.response.rs
only requires impl From<DomainType> for RawType
.Good catches! Additionally, I realized:
impl Protobuf<RawType> for DomainType
is not required.request.rs
only requiresimpl TryFrom<RawType> for DomainType
.response.rs
only requiresimpl From<DomainType> for RawType
.
From the host's perspective, we definitely only need the conversion direction you mentioned. However, it's crucial that these types also cater to relayers, so we'll need to ensure the other direction is well-supported too.
Regarding Protobuf
, I'm still leaning towards implementing it because it offers integrators convenient access to some useful encode/decode methods.
it's crucial that these types also cater to relayers
Regarding Protobuf, I'm still leaning towards implementing it because it offers integrators convenient access to some useful encode/decode methods.
Ah. I see. In that case, I am totally fine implementing these. But I mentioned this because this requirement is not maintained for all domain<>raw types.
Shouldn't we implement impl Protobuf<RawType> for DomainType
for all?
Shouldn't we implement
impl Protobuf<RawType> for DomainType
for all?
Correct. Currently, we haven't been able to implement that for all cases. I've explained the reason in the ibc-query's README. We are in discussions with the ibc-go team to update the request protobuf definitions, incorporating the query height in relevant types. Therefore, we'll need to wait until that change is applied.
@Farhad-Shabani Thanks for your responses. Now everything is good :slightly_smiling_face:
Closes: #XXX
Description
As a follow-up to PR #1129, this PR fixes some small inconsistencies.
PR author checklist:
Reviewer checklist:
Files changed
in the GitHub PR explorer.