cosmos / ibc

Interchain Standards (ICS) for the Cosmos network & interchain ecosystem.
Other
919 stars 382 forks source link

fix: set upgrade timeout after `startFlushUpgradeHandshake` in `chanUpgradeAck` #1040

Closed crodriguezvega closed 9 months ago

crodriguezvega commented 9 months ago

Found this small bug when run the happy path quint test.

cc @AdityaSripal @sangier

crodriguezvega commented 9 months ago

Actually this is really important. Since we are overriding the upgrade at line 708 in chanUpgradeAck, without the proposed change we are violating the invariant that the upgrade.timeout MUST be set when we move to FLUSHING or directly to FLUSHING_COMPLETE. Indeed we would be storing the upgrade during the startFlushUpgradeHandshake (which contains the timeout value) and then override this value with an upgrade instance which contains the final agreed version but that does not contain the timeout at line 708.

Really good catch! Thank you!

Yes, correct! I checked ibc-go and we are good there. :)