Flip the order of callback and actual channel closure on timeout

[Lockwarr]

Summary of Bug

When an ordered ics-27 channel (interchain accounts) receives a timeout, the channel gets closed. However, the current implementation invokes the callback reporting to the contract that the channel is closed before the actual channel closure. This leads to a discrepancy where the channel's state is changed to state_closed only after the callback, causing issues in subsequent operations like channel registration.

Details:

The current process for handling timeouts in interchain accounts follows this sequence:

1. Timeout event triggers.
2. Callback reporting channel closure is invoked.
3. Channel closure is processed, changing the channel's state to state_closed.

Problem:

The order of operations poses a problem for processes relying on the accurate state of the channel. Specifically, when a new channel for the same interchain account needs to be registered. Due to the current implementation, the state transition to state_closed occurs after the callback, creating a timing issue and the contract must wait for the channel to be in state_closed before proceeding with a call to open a new interchain channel for the interchain account.

Expected Behaviour

To resolve this issue, we propose changing the order of calls in the timeout handling process. Specifically, the sequence should be adjusted to:

1. Close the channel.
2. Call the callback reporting channel closure.

Tested on ibc-go v7.3.1

We have tested the proposed solution locally and confirmed that it resolves the timing issue, allowing the contract to initiate a channel registration without waiting explicitly.

Changes:

moved ChannelKeeper.TimeoutExecuted before cbs.OnTimeoutPacket in both TimeoutOnClose() and Timeout() functions in modules/core/keeper/msg_server.go

This change is expected to positively impact processes reliant on accurate channel state, particularly in scenarios where subsequent actions depend on the channel being in the state_closed state.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged/assigned

[crodriguezvega]

Thanks for the detailed issue, @Lockwarr. I think this should be fine (in fact the ICS026 specifies the order you propose), but I would like to have a confirmation from @AdityaSripal.

[crodriguezvega]

The proposed change was implemented in GHSA-j496-crgh-34mx.