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

Support TimeoutPacket feature #21

Open
bluele opened this issue Jan 13, 2022 · 2 comments · May be fixed by #22
Open

Support TimeoutPacket feature #21

bluele opened this issue Jan 13, 2022 · 2 comments · May be fixed by #22

Comments

@bluele
Copy link
Member

bluele commented Jan 13, 2022

We should support the packet timeout in the proxy.

@bluele
Copy link
Member Author

bluele commented Jan 20, 2022

I considered several methods but chose to fork ibc-go and apply a patch for now. // -> Update: #21 (comment)

The reasons why the patch is needed are as follows:

  • When executing SendPacket and TimeoutPacket on downstream(src), the timestamp of upstream(dst) cannot be referenced.
  • more specifically, it cannot support GetTimestsampAtHeight and clientState.GetLatestHeight.

This patch skips the validation for packet timeout when executing SendPacket/TimeoutPacket on the downstream chain. A brief analysis of this is as follows:

  • SendPacket
    SendPacket's timeout validation prevents sending a packet that will never be received by the counterparty chain (upstream). If this validation is removed, there will be no inconsistency in sending and receiving packets between the two chains.
    Edit: this change may be harmful in the ordered channel

  • TimeoutPacket
    TimeoutPacket's timeout validation is to verify that the block time corresponding to the height at which the counterparty chain is trying to prove that it has not received a packet is past timeout. This is not necessary because the downstream will validate the Proxy commitment indicating that the proxy has validated the packet timeout on the upstream. There is no additional trust assumption due to this.

@bluele bluele linked a pull request Jan 20, 2022 that will close this issue
@bluele
Copy link
Member Author

bluele commented Jan 27, 2022

After further consideration, I have made a new experimental version at #23. Together with this, #24, and changes(cosmos/ibc-go#284 (comment)) that will be pushed into the core in the future, we may be able to avoid forking the core.

However, there are still some concerns as follows:

  1. required to changes the semantics of ClientState.GetLatestHeight.
    Currently, GetLatestHeight is expected to return a height of the chain(i.e., proxy) to be tracked, but we will modify it to return a height of the upstream instead. This change will break some existing utility functions and relayer implementations.

  2. The Proxy Client is required to support an additional function VerifyBlockTime to verify the block time of the upstream. However, I think there is a way to support this without modifying the existing implementation.

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

Successfully merging a pull request may close this issue.

1 participant