ICS28: VSCPackets should have timeout on provider (#858)

* update CreateConsumerClient for sovereign chains

* update InitGenesis for sovereign chains

* establish CCV on ACK and handle preCCV completion

* update Channel Uniqueness Correctness Reasoning

* add pre-CCV module interface with staking

* add clarifications

* add overview TODO

* fix pendingChanges bug

* refactor proposal names

* add VSC timeout

* refactor proposal names (#855)

* Update spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

Co-authored-by: Daniel T <30197399+danwt@users.noreply.github.com>

* enable optimistic opening handshakes

* remove ConsumerAdditionProposal.initialHeight

* handle proposals in BeginBlock

* pass consumer unbonding period via gov proposal

* update channel init overview - wip

* update overview

* update modified date

* ICS28: Channel initialization should have a timeout (#860)

* add init timeout

* fix typo

* add note on timed out channel init

* update vsc timeout as per implementation

Co-authored-by: Daniel T <30197399+danwt@users.noreply.github.com>

spec/app/ics-028-cross-chain-validation/README.md

@@ -59,6 +59,8 @@ Aug 29, 2022 - Notify Staking module of matured unbondings in `EndBlock()`
 
 Dec 2, 2022 - Enable existing chains to become consumer chains
 
+Dec 7, 2022 - Add provider-based timeouts 
+
 ## Copyright
 
 All content herein is licensed under [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0).

spec/app/ics-028-cross-chain-validation/data_structures.md

@@ -191,6 +191,9 @@ This section describes the internal state of the CCV module. For simplicity, the
 [↑ Back to Outline](#outline)
 
 - `ProviderPortId = "provider"` is the port ID the provider CCV module is expected to bind to.
+- `initTimeout: uint64` is the maximum time duration the Channel Initialization subprotocol may execute, 
+  i.e., for any consumer chain, if the CCV channel is not established within `initTimeout` since the consumer chain was registered, then the consumer chain is removed. 
+- `vscTimeout: uint64` is the maximum time duration between sending any `VSCPacket` to any consumer chain and receiving the corresponding `VSCMaturedPacket`, without timing out the consumer chain and consequently removing it.
 - `pendingConsumerAdditionProposals: [ConsumerAdditionProposal]` is a list of pending governance proposals to add new consumer chains. 
 - `pendingConsumerRemovalProposals: [ConsumerRemovalProposal]` is a list of pending governance proposals to remove existing consumer chains. 
   Both lists of pending governance proposals expose the following interface: 
@@ -208,6 +211,7 @@ This section describes the internal state of the CCV module. For simplicity, the
 - `chainToConnection: Map<string, Identifier>` is a mapping from consumer chain IDs to the associated connection IDs.
 - `chainToChannel: Map<string, Identifier>` is a mapping from consumer chain IDs to the CCV channel IDs.
 - `channelToChain: Map<Identifier, string>` is a mapping from CCV channel IDs to consumer chain IDs.
+- `initTimeoutTimestamps: Map<string, uint64>` is a mapping from consumer chain IDs to init timeout timestamps, see `initTimeout`.
 - `pendingVSCPackets: Map<string, [VSCPacketData]>` is a mapping from consumer chain IDs to a list of pending `VSCPacketData`s that must be sent to the consumer chain once the CCV channel is established. The map exposes the following interface: 
   ```typescript
   interface Map<string, [VSCPacketData]> {
@@ -221,6 +225,7 @@ This section describes the internal state of the CCV module. For simplicity, the
   }
 - `vscId: uint64` is a monotonic strictly increasing and positive ID that is used to uniquely identify the VSCs sent to the consumer chains. 
   Note that `0` is used as a special ID for the mapping from consumer heights to provider heights.
+- `vscSendTimestamps: Map<(string, uint64), uint64>` is a mapping from `(chainId, vscId)` tuples to the timestamps of sending `VSCPacket`s.
 - `initialHeights: Map<string, Height>` is a mapping from consumer chain IDs to the heights on the provider chain. 
   For every consumer chain, the mapping stores the height when the CCV channel to that consumer chain is established. 
   Note that the provider validator set at this height matches the validator set at the height when the first VSC is provided to that consumer chain.

spec/app/ics-028-cross-chain-validation/methods.md

@@ -55,6 +55,7 @@ function BeginBlock() {
 // implements the AppModule interface
 function EndBlock(): [ValidatorUpdate] {
   EndBlockCIS()
+  EndBlockCCR()
   EndBlockVSU()
 
   // do not return anything to the consensus engine
@@ -69,6 +70,7 @@ function EndBlock(): [ValidatorUpdate] {
   - True. 
 - **Postcondition**
   - `EndBlockCIS()` is invoked (see [[CCV-PCF-EBLOCK-CIS.1]](#ccv-pcf-eblock-cis1), i.e., it contains the `EndBlock()` logic needed for the Consumer Initiated Slashing sub-protocol).
+  - `EndBlockCCR()` is invoked (see [[CCV-PCF-EBLOCK-CCR.1]](#ccv-pcf-eblock-ccr1), i.e., it contains the `EndBlock()` logic needed for the Consumer Chain Removal sub-protocol).
   - `EndBlockVSU()` is invoked (see [[CCV-PCF-EBLOCK-VSU.1]](#ccv-pcf-eblock-vsu1), i.e., it contains the `EndBlock()` logic needed for the Validator Set Update sub-protocol).
 - **Error Condition**
   - None.
@@ -485,6 +487,9 @@ function CreateConsumerClient(p: ConsumerAdditionProposal) {
 
   // store lockUnbondingOnTimeout flag
   lockUnbondingOnTimeout[p.chainId] = p.lockUnbondingOnTimeout
+
+  // add init timeout timestamp for this consumer chain
+  initTimeoutTimestamps[p.chainId] = currentTimestamp().Add(initTimeout)
 }
 ```
 - **Caller**
@@ -511,6 +516,7 @@ function CreateConsumerClient(p: ConsumerAdditionProposal) {
         - a client of the consumer chain is created and the client ID is stored;
         - a `ConsumerGenesisState` is created and stored;
     - `lockUnbondingOnTimeout[p.chainId]` is set to `p.lockUnbondingOnTimeout`.
+    - The init timeout timestamp is computed and stored in `initTimeoutTimestamps[p.chainId]`.
 - **Error Condition**
   - None.
 
@@ -519,7 +525,11 @@ function CreateConsumerClient(p: ConsumerAdditionProposal) {
 > The provider chain uses the fact that the validator set of the consumer chain is the same as its own validator set. 
 
 > **Note:** Bootstrapping the consumer CCV module requires a `ConsumerGenesisState` (see the [CCV Data Structures](./data_structures.md#ccv-data-structures) section). The provider CCV module creates such a `ConsumerGenesisState` when handling a governance proposal `ConsumerAdditionProposal`.
-  
+
+> **Note:** If the channel initialization for a consumer chain exceeds the `initTimeout` period, then the provider chain removes that consumer. 
+> As a result, all further attempts on the consumer side to established the CCV channel will fail. 
+> This means that the consumer chain requires some sort of social consensus to either restart the process of becoming a consumer chain or transitioning back to a sovereign chain. 
+ 
 <!-- omit in toc -->
 #### **[CCV-PCF-COINIT.1]**
 ```typescript
@@ -670,6 +680,9 @@ function onChanOpenConfirm(
     channelToChain[channelIdentifier] = clientState.chainId
     // set initialHeights for this consumer chain
     initialHeights[chainId] = getCurrentHeight()
+
+   // remove init timeout timestamp
+   initTimeoutTimestamps.Remove(clientState.chainId)
 }
 ```
 - **Caller**
@@ -686,6 +699,7 @@ function onChanOpenConfirm(
   - The connection mapping is set, i.e., `chainToConnection`.
   - The channel mappings are set, i.e., `chainToChannel` and `channelToChain`.
   - `initialHeights[chainId]` is set to the current height.
+  - The init timeout timestamp for the consumer chain with ID `clientState.chainId` is removed.
 - **Error Condition**
   - None.
 
@@ -1108,6 +1122,8 @@ function StopConsumerChain(chainId: string, lockUnbonding: Bool) {
   pendingVSCPackets.Remove(chainId)
   initialHeights.Remove(chainId)
   downtimeSlashRequests.Remove(chainId)
+  initTimeoutTimestamps.Remove(chainId)
+  vscSendTimestamps.Remove((chainId, *))
 
   if !lockUnbonding {
     // remove chainId form all outstanding unbonding operations
@@ -1129,9 +1145,13 @@ function StopConsumerChain(chainId: string, lockUnbonding: Bool) {
 - **Caller**
   - `HandleConsumerRemovalProposal` (see [CCV-PCF-HCRPROP.1](#ccv-pcf-hcrprop1)) 
     or `BeginBlockCCR()` (see [CCV-PCF-BBLOCK-CCR.1](#ccv-pcf-bblock-ccr1)) 
-    or `onTimeoutVSCPacket()` (see [CCV-PCF-TOVSC.1](#ccv-pcf-tovsc1)).
+    or `onTimeoutVSCPacket()` (see [CCV-PCF-TOVSC.1](#ccv-pcf-tovsc1))
+    or `EndBlockCCR()` (see [CCV-PCF-EBLOCK-CCR.1](#ccv-pcf-eblock-ccr1)).
 - **Trigger Event**
-  - Either a governance proposal to stop the consumer chain with `chainId` has passed (i.e., it got the necessary votes) or a packet sent on the CCV channel to the consumer chain with `chainId` has timed out.
+  - One of the following events:
+    - a governance proposal to stop the consumer chain with `chainId` has passed (i.e., it got the necessary votes);
+    - a `VSCPacket` sent on the CCV channel to the consumer chain with `chainId` has timed out;
+    - the channel initialization has timed out. 
 - **Precondition**
   - True.
 - **Postcondition**
@@ -1165,6 +1185,51 @@ function StopConsumerChain(chainId: string, lockUnbonding: Bool) {
 > - The second scenario (i.e., a timeout) is only possible if the *Correct Relayer* assumption is violated (see the [Assumptions](./system_model_and_properties.md#assumptions) section), 
 > which is necessary to guarantee both the *Bond-Based Consumer Voting Power* and *Slashable Consumer Misbehavior* properties (see the [Assumptions](./system_model_and_properties.md#correctness-reasoning) section).
 
+
+<!-- omit in toc -->
+#### **[CCV-PCF-EBLOCK-CCR.1]**
+```typescript
+// PCF: Provider Chain Function
+function EndBlockCCR() {
+  // iterate over vscSendTimestamps
+  for (chainId, vscId) IN vscSendTimestamps.Keys() {
+    // check get first timestamp, i.e., the smallest
+    if currentTimestamp() > vscSendTimestamps[(chainId, vscId)] + vscTimeout {
+      // vscTimeout expired: 
+      // stop the consumer chain and use lockUnbondingOnTimeout 
+      // to decide whether to lock the unbonding
+      StopConsumerChain(chainId, lockUnbondingOnTimeout[chainId])
+    }
+  }
+
+  // iterate over initTimeoutTimestamps
+  for chainId IN initTimeoutTimestamps.Keys() {
+    if currentTimestamp() > initTimeoutTimestamps[chainId] {
+      // initTimeout expired:
+      // stop the consumer chain and unlock the unbonding 
+      StopConsumerChain(chainId, false)
+    }
+  }
+}
+```
+- **Caller**
+  - The `EndBlock()` method.
+- **Trigger Event**
+  - An `EndBlock` message is received from the consensus engine; `EndBlock` messages are sent once per block.
+- **Precondition**
+  - True. 
+- **Postcondition**
+  - For each consumer chain ID `chainId` in `vscSendTimestamps.Keys()`,
+    - if `vscSendTimestamps[(chainId, vscId)] + vscTimeout` is smaller than the current timestamp, then the consumer chain with ID `chainId` is stopped.
+  - For each consumer chain ID `chainId` in `initTimeoutTimestamps.Keys()`,
+    - if the timestamp in `initTimeoutTimestamps[chainId]` is smaller than the current timestamp, then the consumer chain with ID `chainId` is stopped.
+- **Error Condition**
+  - None.
+
+> **Note**: To avoid false positives where a consumer chain is unnecessarily removed, 
+> `vscTimeout` MUST be larger than `consumerUnbondingPeriod` and 
+> SHOULD account for the time needed to relay the `VSCPacket` to the consumer and the corresponding `VSCMaturedPacket` back to the provider.
+
 <!-- omit in toc -->
 #### **[CCV-PCF-CCINIT.1]**
 ```typescript
@@ -1349,6 +1414,8 @@ function EndBlockVSU() {
           ccvTimeoutTimestamp,
           data
         )
+        // add VSC send timestamp to vscSendTimestamps
+        vscSendTimestamps[(vscId, chainId)] = currentTimestamp()
       }
 
       // remove pending VSCPackets
@@ -1377,6 +1444,7 @@ function EndBlockVSU() {
     - If there is an established CCV channel for the the consumer chain with `chainId`, then
       - for each `VSCPacketData` in the list of pending VSCPackets associated to `chainId`
         - a packet with the `VSCPacketData` is sent on the channel associated with the consumer chain with `chainId`;
+        - `vscSendTimestamps[(vscId, chainId)]` is set to the current timestamp;
       - all the pending VSCPackets associated to `chainId` are removed.
   - `vscId` is incremented.
 - **Error Condition**
@@ -1459,6 +1527,9 @@ function onRecvVSCMaturedPacket(packet: Packet): bytes {
   // clean up vscToUnbondingOps mapping
   vscToUnbondingOps.Remove((chainId, vscId))
 
+  // clean up vscSendTimestamps mapping
+  vscSendTimestamps.Remove((chainId, vscId))
+
   return VSCMaturedPacketSuccess
 }
 ```
@@ -1477,6 +1548,7 @@ function onRecvVSCMaturedPacket(packet: Packet): bytes {
       - `op.id` is added to `maturedUnbondingOps`;
       - `op.id` is removed from `unbondingOps`.
   - `(chainId, vscId)` is removed from `vscToUnbondingOps`.
+  - `(chainId, vscId)` is removed from `vscSendTimestamps`.
   - A successful acknowledgment is returned.
 - **Error Condition**
   - None.

spec/app/ics-028-cross-chain-validation/system_model_and_properties.md

@@ -38,11 +38,12 @@ furthermore, the *Correct Relayer* assumption relies on both *Safe Blockchain* a
 - ***Live Blockchain***: Both the provider and the consumer chains are *live*. This means that, for every chain, the underlying consensus engine satisfies liveness (i.e., new blocks are eventually added to the chain).
   > **Note**: Both *Safe Blockchain* and *Live Blockchain* assumptions require the consensus engine's assumptions to hold, e.g., less than a third of the voting power is Byzantine. For an example, take a look at the [Tendermint Paper](https://arxiv.org/pdf/1807.04938.pdf).
 
-- ***Correct Relayer***: There is at least one *correct*, *live* relayer between the provider and consumer chains. This assumption has two implications.
-  - First, every packet sent on the CCV channel is relayed to the receiving end before the packet timeout elapses.
-  - Second, a correct relayer will eventually relay packets on the token transfer channel.   
+- ***Correct Relayer***: There is at least one *correct*, *live* relayer between the provider and consumer chains. This assumption has the following implications.
+  - The opening handshake messages on the CCV channel are relayed before the Channel Initialization subprotocol times out (see `initTimeout`).
+  - Every packet sent on the CCV channel is relayed to the receiving end before the packet timeout elapses (see both `vscTimeout` and `ccvTimeoutTimestamp`).
+  - A correct relayer will eventually relay packets on the token transfer channel.   
 
-  Clearly, the CCV protocol is responsible of setting the timeouts (i.e., `timeoutHeight` and `timeoutTimestamp`), for the packets sent on the CCV channel, such that the *Correct Relayer* assumption is feasible.
+  Clearly, the CCV protocol is responsible of setting the timeouts (see `ccvTimeoutTimestamp`, `vscTimeout`, `initTimeout` in the [CCV State](data_structures.md#ccv-state)), such that the *Correct Relayer* assumption is feasible.
   > **Discussion**: IBC relies on timeouts to signal that a sent packet is not going to be received on the other end. 
   > Once an ordered IBC channel timeouts, the channel is closed (see [ICS 4](../../core/ics-004-channel-and-packet-semantics)). 
   > The *Correct Relayer* assumption is necessary to ensure that the CCV channel **cannot** ever timeout and, as a result, cannot transit to the closed state.