Add comments to handler for async handshake (#1019)

* add comments to handler interface describing how to make handshakes async

* remove writes from ICS4 to allow for async handshake

* Apply suggestions from code review

Co-authored-by: Michael FIG <michael+github@fig.org>

* indentation

* Update README.md

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Michael FIG <michael+github@fig.org>

spec/core/ics-004-channel-and-packet-semantics/README.md

@@ -455,11 +455,7 @@ function chanOpenAck(
         expected
       ))
     }
-
-    channel.state = OPEN
-    channel.version = counterpartyVersion
-    channel.counterpartyChannelIdentifier = counterpartyChannelIdentifier
-    provableStore.set(channelPath(portIdentifier, channelIdentifier), channel)
+    // write will happen in the handler defined in the ICS26 spec
 }
 ```
 
@@ -506,8 +502,7 @@ function chanOpenConfirm(
       ))
     }
 
-    channel.state = OPEN
-    provableStore.set(channelPath(portIdentifier, channelIdentifier), channel)
+    // write will happen in the handler defined in the ICS26 spec
 }
 ```
 
@@ -587,6 +582,7 @@ function chanCloseConfirm(
       ))
     }
 
+    // write may happen asynchronously in the handler defined in the ICS26 spec
     // if the channel is closing during an upgrade, 
     // then we can delete all auxiliary upgrade information
     provableStore.delete(channelUpgradePath(portIdentifier, channelIdentifier))

spec/core/ics-026-routing-module/README.md

@@ -447,6 +447,12 @@ function handleChanOpenInit(datagram: ChanOpenInit) {
     datagram.portIdentifier,
     datagram.counterpartyPortIdentifier
   )
+  // SYNCHRONOUS: the following calls happen synchronously with the call above
+  // ASYNCHRONOUS: the module callback will be called at a time later than the channel handler
+  // in this case, the channel identifier will be stored with a sentinel value in the channel path so it is not taken
+  // by a new channel handshake and the capability is reserved for the application module.
+  // When the module eventually executes its callback it must call writeChannel so that the channel
+  // can be written into an INIT state with the right version and the handshake can proceed on the counterparty.
   version, err = module.onChanOpenInit(
     datagram.order,
     datagram.connectionHops,
@@ -497,6 +503,12 @@ function handleChanOpenTry(datagram: ChanOpenTry) {
     datagram.proofInit,
     datagram.proofHeight
   )
+  // SYNCHRONOUS: the following calls happen syncrhonously with the call above
+  // ASYNCHRONOUS: the module callback will be called at a time later than the channel handler
+  // in this case, the channel identifier will be stored with a sentinel value in the channel path so it is not taken
+  // by a new channel handshake and the capability is reserved for the application module.
+  // When the module eventually executes its callback it must call writeChannel so that the channel
+  // can be written into a TRY state with the right version and the handshake can proceed on the counterparty.
   version, err = module.onChanOpenTry(
     datagram.order,
     datagram.connectionHops,
@@ -542,13 +554,28 @@ function handleChanOpenAck(datagram: ChanOpenAck) {
     datagram.proofTry,
     datagram.proofHeight
   )
+  // SYNCHRONOUS: the following calls happen syncrhonously with the call above
+  // ASYNCHRONOUS: the module callback will be called at a time later than the channel handler
+  // When the module eventually executes its callback it must call writeChannel so that the channel
+  // can be written into an OPEN state and the handshake can proceed on the counterparty.
   err = module.onChanOpenAck(
     datagram.portIdentifier,
     datagram.channelIdentifier,
     datagram.counterpartyChannelIdentifier,
     datagram.counterpartyVersion
   )
   abortTransactionUnless(err === nil)
+  channel = provableStore.get(channelPath(datagram.portIdentifier, datagram.channelIdentifier))
+  writeChannel(
+    datagram.portIdentifier,
+    datagram.channelIdentifier,
+    OPEN,
+    channel.order,
+    channel.counterparty.portIdentifier,
+    datagram.counterpartyChannelIdentifier,
+    channel.connectionHops,
+    datagram.counterpartyVersion
+  )
 }
 ```
 
@@ -570,11 +597,26 @@ function handleChanOpenConfirm(datagram: ChanOpenConfirm) {
     datagram.proofAck,
     datagram.proofHeight
   )
+  // SYNCHRONOUS: the following calls happen syncrhonously with the call above
+  // ASYNCHRONOUS: the module callback will be called at a time later than the channel handler
+  // When the module eventually executes its callback it must call writeChannel so that the channel
+  // can be written into an OPEN state and the handshake can proceed on the counterparty.
   err = module.onChanOpenConfirm(
     datagram.portIdentifier,
     datagram.channelIdentifier
   )
   abortTransactionUnless(err === nil)
+  channel = provableStore.get(channelPath(datagram.portIdentifier, datagram.channelIdentifier))
+  writeChannel(
+    datagram.portIdentifier,
+    datagram.channelIdentifier,
+    OPEN,
+    channel.order,
+    channel.counterparty.portIdentifier,
+    channel.counterparty.channelIdentifier,
+    channel.connectionHops,
+    channel.version
+  )
 }
 ```
 
@@ -610,18 +652,27 @@ interface ChanCloseConfirm {
 ```
 
 ```typescript
-function handleChanCloseConfirm(datagram: ChanCloseConfirm) {
+function handleChanCloseConfirm(datagram: ChanCloseConfirm) {  
+    handler.chanCloseConfirm(
+      datagram.portIdentifier,
+      datagram.channelIdentifier,
+      datagram.proofInit,
+      datagram.proofHeight
+    )
+    // SYNCHRONOUS: the following calls happen syncrhonously with the call above
+    // ASYNCHRONOUS: the module callback will be called at a time later than the channel handler
+    // When the module eventually executes its callback it must call writeChannel so that the channel
+    // can be written into a CLOSED state and the handshake can proceed on the counterparty.
     module = lookupModule(datagram.portIdentifier)
     err = module.onChanCloseConfirm(
       datagram.portIdentifier,
       datagram.channelIdentifier
     )
     abortTransactionUnless(err === nil)
-    handler.chanCloseConfirm(
+    writeChannel(
       datagram.portIdentifier,
       datagram.channelIdentifier,
-      datagram.proofInit,
-      datagram.proofHeight
+      CLOSED,
     )
 }
 ```