Something confusing about ICS03, ICS18

[michwqy]

As function pendingDatagrams in ICS18

function pendingDatagrams(chain: Chain, counterparty: Chain): List<Set<Datagram>> {
...
  // ICS3 : Connections
  connections = chain.getConnectionsUsingClient(counterparty)
  for (const localEnd of connections) {
    remoteEnd = counterparty.getConnection(localEnd.counterpartyIdentifier)
    if (localEnd.state === INIT && (remoteEnd === null || remoteEnd.state === INIT))
      counterpartyDatagrams.push(ConnOpenTry{...})
...
}

Relayer relays ConnOpenTry to B when localEnd.state == INIT in A and remoteEnd == null || remoteEnd.state == INIT in B. But I think it is impossible that both connectionEnd equal INIT.
As function connOpenInit in ICS03

function connOpenInit(...) {
    identifier = generateIdentifier()
...
    connection = ConnectionEnd{state, "", counterpartyPrefix,
      clientIdentifier, counterpartyClientIdentifier, versions, delayPeriodTime, delayPeriodBlocks}
...
}

There is no counterpartyConnectionIdentifier when a connectionEnd in A is "INIT". Acoording to ICS03, if a connectionEnd in A and connectionEnd in B are both "INIT", function connOpenTry should be relayed to both chain and each chain will create a new connectionEnd in TRYOPEN to complete the handshake.

function connOpenTry(...) {
    identifier = generateIdentifier()
...
    connection = ConnectionEnd{TRYOPEN, counterpartyConnectionIdentifier, counterpartyPrefix,
                               clientIdentifier, counterpartyClientIdentifier, [version], delayPeriodTime, delayPeriodBlocks}
...
}

So I don't quite understand this if (localEnd.state === INIT && (remoteEnd === null || remoteEnd.state === INIT)), is there any special use for it that I don't think of?

[crodriguezvega]

Hi @michwqy. It is possible that both chains are in state INIT if a relayer submits MsgConnectionOpenInit on both chains. Execution of this message will create a connection end with state INIT and a given connection identifier (let's say connection-0 on chain A and connection-1 on chain B). Now, if the relayer submits MsgConnectionOpenTry on both chains and the message contains the connection identifier of the counterparty (i.e. connection-0 in the message to chain B and connection-1 in the message to chain A), and assuming both messages execute successfully, then what you end up having is two new connection ends on both chains with state TRY and with new connection identifiers (connection-1 on chain A and connection-2 on chain B). So basically the outcome is that you have actually 2 connections instead of one (one connection connects ends with identifiers connection-0 and connection-2 and the other connection connects ends with identifiers connection-1 and connection-1).

The check remoteEnd.state === INIT is basically there just to prevent that a connection attempt is started if the other end is already in TRYOPEN or OPEN states.

I hope this makes sense. If it does and you don't any follow-up questions, please feel free to close the issue. :)

[michwqy]

Thanks @crodriguezvega. The question is when localEnd == INIT (like connection-0 in chain A), counterpartyconnectionIdentifier in localEnd is empty, which means remoteEnd must be null. Although both connection-0 in A and connection-1 in B are INIT, they won't handshake as a pair of localEnd and remoteEnd. This code if (localEnd.state === INIT && (remoteEnd === null || remoteEnd.state === INIT)) gives the impression that the counterparty connectionEnd of localEnd is determined. I think this is misleading, and if (localEnd.state === INIT && remoteEnd === null) can be more accurate.

[crodriguezvega]

Thanks for further explaining, @michwqy! I see your point, and I think you're right. This code was probably not modified after the support for crossing hellos was removed. I can open a PR to fix this.

Please note that ICS 18 is seriously underworked and, as you can see, there's many errors, inconsistencies. It's in our plans to improve it, but it will still take some time.