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

Question: Clarification on support for optimistic packet sending in ICS-004 #1170

Closed
bluele opened this issue Nov 24, 2024 · 2 comments · Fixed by #1173
Closed

Question: Clarification on support for optimistic packet sending in ICS-004 #1170

bluele opened this issue Nov 24, 2024 · 2 comments · Fixed by #1173
Labels

Comments

@bluele
Copy link
Contributor

bluele commented Nov 24, 2024

Hi there,

In the current IBC specifications, is the lack of support for optimistic packet sending intentional?

As described here, the channel state is required to be OPEN when sending a packet.
Additionally, I know that the ibc-go has decided to remove support for optimistic packet sending in this issue.

However, if optimistic packet sending is not supported in the ICS-004 as well, the following description about packet timeout in the document seems misleading or incorrect:

Since we allow optimistic sending of packets (i.e. sending a packet before a channel opens), we must also allow optimistic timing out of packets. With optimistic sends, the packet may be sent on a channel that eventually opens or a channel that will never open. If the channel does open after the packet has timed out, then the packet will never be received on the counterparty so we can safely timeout optimistically. If the channel never opens, then we MUST timeout optimistically so that any state changes made during the optimistic send by the application can be safely reverted.

Could you clarify whether this feature is supported and whether the current description aligns with the intended design?

@AdityaSripal
Copy link
Member

Hello, we disabled optimistic sends and so yes the description in packet timeout is outdated and inaccurate. Thank you for the catch!

Would you like to make a PR fixing this?

@bluele
Copy link
Contributor Author

bluele commented Dec 4, 2024

@AdityaSripal Thank you for confirming!
I'd be happy to create a PR to fix this. I’ll work on it shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants