Skip to content

Interchain Accounts: consider using smaller timeoutTimestamp #540

Closed

Description

Surfaced from @informalsystems audit of ICS27 InterchainAccounts at hash b8bc1a8

In the current implementation of 27-interchain-accounts/keeper/relay.go, when tx packets are sent, the timeout is set to max uint64, thus making the packets effectively never expiring:

	// timeoutTimestamp is set to be a max number here so that we never recieve a timeout
	// ics-27-1 uses ordered channels which can close upon recieving a timeout, which is an undesired effect
	const timeoutTimestamp = ^uint64(0) >> 1 // Shift the unsigned bit to satisfy hermes relayer timestamp conversion

	packet := channeltypes.NewPacket(
		icaPacketData.GetBytes(),
		sequence,
		sourcePort,
		sourceChannel,
		destinationPort,
		destinationChannel,
		clienttypes.ZeroHeight(),
		timeoutTimestamp,
	)

The problem I see with this approach is that it gives the application control away completely: the app (IA) doesn't have a chance to react in case a packet got stuck for some reason. And there are such cases, e.g. when a LightClient is not updated, and the trusting period passes, so it can't ever be updated again (besides using a special governance proposal), as outlined e.g. in the ICS02:

If a client can no longer be updated (if, for example, the trusting period has passed), it will no longer be possible to send any packets over connections & channels associated with that client, or timeout any packets in-flight (since the height & timestamp on the destination chain can no longer be verified). Manual intervention must take place to reset the client state or migrate the connections & channels to another client. This cannot safely be done completely automatically, but chains implementing IBC could elect to allow governance mechanisms to perform these actions (perhaps even per-client/connection/channel in a multi-sig or contract).

I understand the motivation for choosing the high timeout, such that the channel never gets closed because of a timed out packet. But I believe the real architectural choice here is different. Comparing the two cases:

  1. with unclosed but unusable channel, with stuck packets
  2. received notification of expired timeout, and a closed channel

The second one I think is much better, because you can react somehow. In the first one your app is stuck completely, and you loose any control over what happens next.

CC @seantking


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Type

No type

Projects

  • Status

    Todo 🏃

Relationships

None yet

Development

No branches or pull requests

Issue actions