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

Remove support for asynchronous acks for RecvPacket #361

Closed
plafer opened this issue Jan 19, 2023 · 1 comment · Fixed by #364
Closed

Remove support for asynchronous acks for RecvPacket #361

plafer opened this issue Jan 19, 2023 · 1 comment · Fixed by #364
Assignees
Labels
O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding S: specs Scope: related to IBC protocol specifications
Milestone

Comments

@plafer
Copy link
Contributor

plafer commented Jan 19, 2023

Asynchronous acknowledgements are currently under-specified. I don't understand the semantics enough to be confident that our current implementation is correct. Hence, it makes our implementation of RecvPacket more complex, while the benefits are unclear.

We should remove support for them and document it in our list of unsupported features. We can add support at a later time if requested, when the requirements are more clear.

@plafer plafer added O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding S: specs Scope: related to IBC protocol specifications labels Jan 19, 2023
@plafer plafer self-assigned this Jan 19, 2023
@plafer
Copy link
Contributor Author

plafer commented Jan 19, 2023

Note that our current implementation didn't properly support async acks, since the packet processing callbacks have no way of calling write_acknowledgement::process(), which requires a reference to a ChannelReader.

The packet processing callbacks don't need a reference to the context; a chain can call write_acknowledgement::process() in the processing of a chain-specific transaction. This means that we'll need to make sure to expose a write_acknowledgement() function when we add support back for async acks.

@Farhad-Shabani Farhad-Shabani modified the milestones: Support Anoma's onboarding efforts, v0.28.0 Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding S: specs Scope: related to IBC protocol specifications
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants