Host chain doesn't account for multiple controller chains using overlapping namespace

[colin-axner]

Summary

The host chain currently assumes that the controller port ID is unique amongst all its connected chains. This is immediately evident when viewing its setting of the interchain account. It uses only the controller port as the key. Thus two controller chains using the same format to generate the port address will collide. This seems highly probable as chains will likely reuse the same ICA auth modules.

Previously the connection sequences were included in the portID thus making this assumption okay since a chains binded ports must be unique. Thus, there would never be two ownerA ports on connection 0. But there may be ownerA on connection 0 and ownerA on connection 1.

Problem Definition

The interchain account mapping on the host chain uses the controller port as the key. Controller ports are a prefix and account owner address. There is no uniqueness properties in the controller ports anymore.

Proposal

We should use either the connection sequence or channel sequence in the key for the interchain account address mapping. Since we still have the active channel mapping, I think it makes sense to use the connection sequence, otherwise you'd need to migrate the mapping when opening a new active channel. You'd also need to account for the channelID in ICA address generation. It is possible a channel is created on the controller chain which will never become an active channel (but the current controller handshake code might create an interchain account anyways). I don't think we need the controller chain connection sequence?

Action items (separate pr's):

• use the host chain connection sequence in the ica address mapping (key is now connectionSeq/controllerPort) + test cases
• use host chain connection sequence when generating ica address + test cases

---

For Admin Use

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

[AdityaSripal]

Nice catch, until we use ORDERED channels that don't close on timeout, I think we'll need to use the connection sequence as you mentioned.

[seantking]

Yeah good catch, makes sense to me. Agree with the proposal.

[colin-axner]

We need this for the active channel mapping (on the host chain)? Since the controller port is only unique from the perspective of the controller chain

[damiannolan]

Do we need this for the active channel mapping on the controller chain also? In the scenario where one owner account wants to control interchain accounts on multiple different hosts chains. If we key active channel ID by the port ID then we risk overwriting, right?

[colin-axner]

That's an interesting point. Thoughts @AdityaSripal?

So we could allow multiple active channels connected to a single port on the controller channel (so long as they use different connections)

Or we could force ica auth modules to create a new owner address taking into account the connection sequence

[damiannolan]

Or we could force ica auth modules to create a new owner address taking into account the connection sequence

Maybe this is the best approach. After taking a second look, controllers cannot reuse the same owner account as it will fail when checking if the port is bound.

This makes my last point re. active channel overwriting void. But it does raise the question on whether or not an account owner should, or shouldn't be allowed to reuse the same account address for different interchain accounts on separate hosts.

[seantking]

Maybe this is the best approach. After taking a second look, controllers cannot reuse the same owner account as it will fail when checking if the port is bound

Damn, this was a pretty big oversight by us. In hindsight, I do remember now that I put the connection seq in the portID for this very reason.

I think we need to be very careful here with regards to dev UX on the ica-auth side of things.

[seantking]

But it does raise the question on whether or not an account owner should, or shouldn't be allowed to reuse the same account address for different interchain accounts on separate hosts

This loops back around to the owner-account model not being entirely clear in the first place. But in terms of what we set out to build with ics27 so far, an owner-account should absolutely be able to own separate ICA accounts across different host chains. In practice, there's nothing stopping this from still being the case as at the end of the day it's just an arbitrary string, but we'll need to document/specify this better if we expect devs to workout around this limitation.

IMO it's partly a documentation/spec issue & also a conversation around how much we expect an ica-auth dev to handle themselves vs how much we bake into ICA out of the box with regards to owning accounts.

[AdityaSripal]

I think we definitely want a single controller address to control host addresses on multiple host chains. We should not change that ability.

Or we could force ica auth modules to create a new owner address taking into account the connection sequence

If we do this, this is basically the old architecture but forcing ica-auth devs to handle it instead of handling it internally. I'd rather abstract that into ICA if it is going to be necessary.

What does seem weird is that we are creating a unique port for each (owner address, connection sequence) tuple. This is unnecessary. Perhaps we can just create a unique port for each owner address, and then only bind port once. But once it is bound for that controller, we can reuse that port to open channels on different connections without a problem.
Note: This will require a new check in the controller code that we only open a single channel for an owner address on the same connection since that will no longer be automatically checked. I believe this will also affect the reopening logic since we need to ensure that reopening on a port is only allowed if the active channel for that connection is CLOSED.

I think the above is a viable option. But I think this issue underscores how much workaround is necessary given that timeouts close ORDERED channels. How much resistance is there to getting ORDERED_NO_TIMEOUT channels in and just removing all this complexity?

[damiannolan]

Closed by #807 #790 #791