-
Notifications
You must be signed in to change notification settings - Fork 383
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
Asynchronous acknowledgements #480
Conversation
.... the PDF creation issue seems to be specific to my setup, perhaps an Arch Linux upgrade issue. |
Still trying to fix PDF creation but the spec changes are ready for review. |
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.
LGTM, just left some suggestions for clarity
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
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.
A few questions, so I can update our IBC modules properly.
@@ -565,15 +571,14 @@ The IBC handler performs the following steps in order: | |||
- Checks that the packet sequence is the next sequence the channel end expects to receive (for ordered channels) | |||
- Checks that the timeout height has not yet passed | |||
- Checks the inclusion proof of packet data commitment in the outgoing chain's state | |||
- Sets the opaque acknowledgement value at a store path unique to the packet (if the acknowledgement is non-empty or the channel is unordered) | |||
- Sets a store path to indicate that the packet has been received (unordered channels only) |
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.
Don't we need to set recvPacket all the time now? As we do not rely on acknowledgements?
If a synchronous acknowledgement obviates the need for recvPacket
then this should specify that either acknowledgement or receipt must be written atomically with onRecv
.
Or maybe you can explain why I misunderstand this.
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.
recvPacket
always needs to be called, as usual, regardless of whether or not an acknowledgement will be written.
This build issue seems to be related to sergiocorreia/panflute#142; downgrading my |
Closes #478
writeAcknowledgement
functionmake
process (somehow broken)