-
Notifications
You must be signed in to change notification settings - Fork 344
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
Add Never type to avoid ibc_packet_receive errors #1513
Conversation
177a36a
to
ce9d90a
Compare
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.
Looks good, happy for this type.
However, we no longer need this in receive as we auto-wrap errors in ibc receive as long as you use the same JSON format for errors as in ics20 and ics27.
We really should have this for ack and timeout, so the type is useful.
} | ||
} | ||
|
||
impl core::fmt::Display for Never { |
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.
Maybe add a comment that this is needed to satisfy the requirement of error type in cosmwasm results
Where/how is this happening? Did I miss something that I should be aware of? |
We changed this when upgrading to ibc-go v3. This handles errors just in reply. It was added in January. I know I had various discussions on GitHub here and I thought it was documented. |
Oh, this PR might be obsolete then. I will check that. |
Never in other areas would be useful. (And anyone that doesn't want that error format must ensure Never and manually handle) |
Yeah, it seems like we have the same issue in ibc_packet_ack, right? You don't want to fail the ack transaction from the relayer just because of some failing |
f7faa61
to
d233dee
Compare
d233dee
to
2b8c553
Compare
When refactoring some
ibc_packet_receive
code in Nois I realized I really want type safety to ensure I don't shoot myself in the foot. ANever
type makes explicit what is documented as best practive already: letibc_packet_receive
never return an error.This way we keep compatibility with the current API (ibc_packet_receive returns
Result<IbcReceiveResponse<C>, E>
withE: ToString
) but make it impossible to refactor code in a way that it returns an error (which is easy to get wrong with the ? operator).