Question about multiple OpenTry messages

[ancazamfir]

Summary of Bug

Testing connection handshake with multiple relayers (https://github.com/informalsystems/ibc-rs/issues/2168), i see that multiple MsgConnectionOpenTry messages with same counterparty are accepted and they generate multiple connection ends in tryOpen state. Only for one of them will the handshake be succesful of course but was wondering why the ConnOpenTry handler does not check if the counterparty is already bound to an existing connection.
I remember asking this a while ago but cannot remember the answer :)

To fix this, hermes has to perform some ID "fixup". It is true that even if the ConnOpenTry handler would reject subsequent messages the relayer would need to figure out which ID to switch to (probably by querying clientConnections).

Note: the same is true for ChanOpenTry handler.

Version

master

Steps to Reproduce

hermes create client ibc-0 ibc-1
hermes create client ibc-1 ibc-0
hermes tx raw conn-init ibc-0 ibc-1 07-tendermint-0 07-tendermint-0
hermes tx raw conn-try ibc-1 ibc-0 07-tendermint-0 07-tendermint-0 -s connection-0
hermes tx raw conn-try ibc-1 ibc-0 07-tendermint-0 07-tendermint-0 -s connection-0

---

For Admin Use

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

[AdityaSripal]

Hmm, yes this seems like a thing we should be able to check on TRY. We would have to probably store some additional state, since on the TRY step, we wouldn't be able to efficiently search through our own existing connectionIDs to see if they have the same counterparty.

So we would have to store a mapping from the proposed counterparty to the ID that we have generated for a handshake in progress on TRY. This would then get deleted on CONFIRM if the first handshake fails in order to allow for a subsequent handshake against the same counterparty.

Similar logic would need to be applied on connection/channel handshakes

cc: @damiannolan @seantking

[crodriguezvega]

So we would have to store a mapping from the proposed counterparty to the ID that we have generated for a handshake in progress on TRY

@AdityaSripal question: if we have 3 chains (A, B, C) and both A and B want to create a connection to C. Both A and B could have on their side the same client ID and connection ID, right? For example 07-tendermint-0 and connection-0. So if a relayer submits 2 MsgConnectionOpenTry messages to chain C, the counterparty client ID and connection ID will be the same in both messages, so if we store a mapping of the counterparty connection ID to the connection ID generated in the TRY step on chain C that would prevent one of the two connections from being established because the logic will wrongly assume that both messages are trying to establish a connection with the same chain. Is this reasoning correct? If it is, what other information can we use to make a key that is unique enough to be used in the mapping that we should store?

[AdityaSripal]

Hi Carlos, yes that is correct. There can be two connections with the same counterparty connectionID since they may refer to different chains. However, there cannot be two connections with the same counterparty connectionID that also has the same underlying clientID.

We cannot just key on the counterpartyID, we will also have to key on our identifier for the layer below.

So let's say we have your scenario. So chainA and chainB both have client and connectionIDs to chain C (07-tendermint-0, connection-0)

Chain C has a clientID to A: 07-tendermint-1 and for the clientID to B: 07-tendermint-2.

We should key on our clientID and the counterparty connectionID.

privateStore: conRedundancyProtection/07-tendermint-1/connection-0 => []byte{1}

So any new CONN_TRY message, can check the key with its own clientID and the counterparty connectionID. If it exists, then error since the connection has already been established.

Similar logic will apply for channel handshakes, but we use our own connectionID and the counterparty channelID.

Does that make sense? cc: @colin-axner

[colin-axner]

I remember asking this a while ago but cannot remember the answer :)

Found it here! Looks like the conclusions were the same:

Aye, the query/filter must be done on (client_id, counterparty_connection_id).

[ancazamfir]

Found it here! Looks like the conclusions were the same:

great! thanks @colin-axner! feel free to close on of the two issues.

[colin-axner]

Hi @ancazamfir!

We ran into some open questions when trying to implement this issue. I would like to ask some followup questions so we can find an appropriate solution to the problem.

There's two concerns that arose when addressing this problem:

• state introduced is not prunable at the moment (requires non trivial complexity to determine when the information is no longer needed)
• accounting for existing state which may be considered "invalid" with the updated logic

In the original issue you mention:

To fix this, hermes has to perform some ID "fixup". It is true that even if the ConnOpenTry handler would reject subsequent messages the relayer would need to figure out which ID to switch to (probably by querying clientConnections).

What sort of logic does hermes need to do? Do you expect core IBC to return an identifier of the existing connection in open try?

When trying to account for existing state which may be considered "invalid", we have a few potential solutions:

• ignore existing state (not ideal since it creates partially filled in state, but I guess it can be viewed as equivalent to pruned state)
• return a list of identifiers that connect to the same counterparty endpoint
• only store a flag to indicate the counterparty endpoint already has an existing connection

The primary issue with existing state is that core IBC cannot tell which is the correct connection identifier for the relayer to use. If there's connection-100 and connection-101 representing the same counterparty connection, it might be the case that the counterparty connection has selected connection-101 while this migration is being performed. Very much an edge case scenario, but still worth considering.

Does this issue arise for hermes when two relayers attempt to complete the same handshake? If the slower relayer fails on try (currently it'd fail on ack), what is its expected behaviour? Does it stall and wait for the other relayer to finish or does it attempt to continue the handshake using the correct identifier?

I'm assuming hermes would want to ensure the handshake finishes even if it is delayed slightly after detecting a race. Currently, this is fairly straightforward, it would either complete its duplicate try step and proceed to ack (assuming the other relayer stops after try) or it fails on ack and looks up the selected identifier on the try side.

If core IBC returns an error on OpenTry for an existing connection to the counterparty endpoint, then it would be necessary to have some way of determining which connections exist relative to that counterparty endpoint. Off the top of my head, this would require using events, or iterating over existing connections. If we ignore existing state, this seems fine to return the existing connection id with the error on open try. One benefit of allowing multiple open try's is that an invalid connection state set in the open try step doesn't require starting over the handshake, but perhaps it would be easier to introduce cleanup on failed handshakes if you enforced a single connection.

It seems to me, returning an error on open try without returning only a single identifier could make things more difficult. If we fixed this issue by ignoring existing state and returning an error on open try with the identifer for new handshakes, would that be beneficial to hermes?

[ancazamfir]

What sort of logic does hermes need to do? Do you expect core IBC to return an identifier of the existing connection in open try?

there are some comments that may help here:
https://github.com/informalsystems/hermes/blob/daad02843a091bbdb3dd608e5f4ce790895c8845/crates/relayer/src/connection.rs#L465-L512

If we fixed this issue by ignoring existing state and returning an error on open try with the identifer for new handshakes, would that be beneficial to hermes?

Not sure I understand the proposal here. If the connection to counterparty exist in OpenTry state and has some ID x would the second open try return an error and mention x?

[colin-axner]

I'm going to close this issue. IBC eureka/v2 will move away from connection/channel handshakes, so I don't think this issue would be addressed before they are removed entirely (especially since we haven't addressed it up to now, sorry for that!)