Add counterpartyChannelID param to IBCModule.OnChanOpenAck()

[catShaark]

ibc v3 now set the counterpartyChannelID to channel store after calling IBCModule.OnChanOpenAck. This break any ibc modules that use counterpartyChannelID in their OnChanOpenAck logic since there's no way to fetch counterpartyChannelID value as the value is not yet set in store and is not passed into IBCModule.OnChanOpenAck. One example is wasmd ibc contracts

I think we should pass counterpartyChannelID to IBCModule.OnChanOpenAck

[crodriguezvega]

Thank you for opening this issue (and for opening already a PR for it as well!).

So if i understand it correctly the issue you're facing is that the channel state (which includes the counterpartyChannelID) is stored after the OnChanOpenAck logic is executed and therefore you don't have access to the counterpartyChannelId inside of it. This is correct, right?

I am interested also in understanding better why you need the counterpartyChannelID in the OnChanOpenAck logic. Can you please elaborate a bit more what use the counterpartyChannelID has in the OnChanOpenAck in the context of wasm IBC contracts?

This changes the onChanOpenAck callback signature and for this kind of changes we usually like to see that the spec is updated before we adopt the changes in ibc-go. I think it would be good if you also open an issue in the spec repo and explain your reasoning in it.

This change is also Go API breaking so we need to release this in a new major release. We're about to release v3 (next week) so we might be a bit short of time to include it in the release. Next planned major release (v4) will not come for at least another 2 or 3 months. How pressing is this change needed for your development?

[catShaark]

So if i understand it correctly the issue you're facing is that the channel state (which includes the counterpartyChannelID) is stored after the OnChanOpenAck logic is executed and therefore you don't have access to the counterpartyChannelId inside of it. This is correct, right?

Yes you are correct

I am interested also in understanding better why you need the counterpartyChannelID in the OnChanOpenAck logic. Can you please elaborate a bit more what use the counterpartyChannelID has in the OnChanOpenAck in the context of wasm IBC contracts?

So in wasmd IBCHandler.OnChanOpenAck, they send a wasmvmtypes.IBCChannelConnectMsg to the smart contract with the corresponding portID, that wasmvmtypes.IBCChannelConnectMsg basically contains all the info related to the channel, including its counterparty, the contract can then use that info for its logic. An ibc contract for example, would want to use the counterpartyChannelID value to calculate the ibc denom of its token on the remote chain. Another usecase I can think of is creating address using hash(counterpartyChannelID, connectionID), a 2-way ica module for example, I'm not sure.

This changes the onChanOpenAck callback signature and for this kind of changes we usually like to see that the spec is updated before we adopt the changes in ibc-go. I think it would be good if you also open an issue in the spec repo and explain your reasoning in it.

Yes If you think this make sense

This change is also Go API breaking so we need to release this in a new major release. We're about to release v3 (next week) so we might be a bit short of time to include it in the release. Next planned major release (v4) will not come for at least another 2 or 3 months. How pressing is this change needed for your development?

Not so much, we're trying to upgrade wasmd to sdk0.46 and we just noticed ibc v3 not setting counterpartyChannelID before calling IBCModule.OnChanOpenAck will make some of the ibc contracts more inconvenient.

[colin-axner]

This should be updated in the spec (and in the implementation). I think it makes sense to add this argument to the function arguments. We could adjust the order of setting the channel, but then it will disagree with the design of OnChanOpenInit and OnChanOpenTry (channel written last is necessary). The spec doesn't allow the counterparty channel ID to be accessible either since the handler.chanOpenAck is expected to write the updated channel state

function handleChanOpenAck(datagram: ChanOpenAck) {
    err = module.onChanOpenAck(
      datagram.portIdentifier,
      datagram.channelIdentifier,
      datagram.version
    )
    abortTransactionUnless(err === nil)
    handler.chanOpenAck(
      datagram.portIdentifier,
      datagram.channelIdentifier,
      datagram.version,
      datagram.proofTry,
      datagram.proofHeight
    )
}