ICS24: Restructure provable packet keys (#1155)

* imp: note that commitments must be lexographically ordered to maintain soundness (#1153)

* restructure ibc keys

* add value instead of redundant provable store column

* chore: remove myself as codeowner (#1156)

* Fix variable name (#1162)

* fix link and put new provable keys into 04-channel spec

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Christoph Otter <chipshort@tutanota.com>

.github/CODEOWNERS

@@ -1,14 +1,14 @@
 # Default owners for repository
 # 2/n quorum required for merge
 
-*       @adityasripal @cwgoes @angbrav @colin-axner @damiannolan @sangier
+*       @adityasripal @cwgoes @angbrav @damiannolan @sangier
 
 # CODEOWNERS for the CODEOWNER file
 
 /.github/CODEOWNERS @adityasripal @cwgoes @angbrav 
 
 # CODEOWNERS for the specs
 
-/spec/app @adityasripal @cwgoes @colin-axner @damiannolan @sangier
+/spec/app @adityasripal @cwgoes @damiannolan @sangier
 
 /spec/app/ics-028-cross-chain-validation @mpoke @adityasripal @cwgoes @angbrav @insumity

spec/core/ics-023-vector-commitments/README.md

@@ -249,6 +249,8 @@ For any prefix `prefix` and any path `path` not set in the commitment `acc`, for
 Probability(verifyMembership(root, proof, applyPrefix(prefix, path), value) === true) negligible in k
 ```
 
+To ensure the commitment proofs are *sound*, the commitment must be lexographically ordered to ensure that non-existence proofs of the key `b` may be proven by showing the existence of key `a` and key `c` in addition to proving that these two keys are neightbors in the commitment. 
+
 #### Position binding
 
 Commitment proofs MUST be *position binding*: a given commitment path can only map to one value, and a commitment proof cannot prove that the same path opens to a different value except with probability negligible in k.

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

@@ -163,7 +163,7 @@ Thus, constant-size commitments to packet data fields are stored under the packe
 
 ```typescript
 function packetCommitmentPath(channelSourceId: bytes, sequence: BigEndianUint64): Path {
-    return "commitments/channels/{channelSourceId}/sequences/{sequence}"
+    return "{channelSourceId}|0x1|{bigEndianUint64Sequence}"
 }
 ```
 
@@ -173,15 +173,15 @@ Packet receipt data are stored under the `packetReceiptPath`. In the case of a s
 
 ```typescript
 function packetReceiptPath(channelDestId: bytes, sequence: BigEndianUint64): Path {
-    return "receipts/channels/{channelDestId}/sequences/{sequence}"
+    return "{channelDestId}|0x2|{bigEndianUint64Sequence}"
 }
 ```
 
 Packet acknowledgement data are stored under the `packetAcknowledgementPath`:
 
 ```typescript
 function packetAcknowledgementPath(channelSourceId: bytes, sequence: BigEndianUint64): Path {
-    return "acks/channels/{channelSourceId}/sequences/{sequence}"
+    return "{channelSourceId}|0x3|{bigEndianUint64Sequence}"
 }
 ```
 

spec/core/v2/ics-024-host-requirements/README.md

@@ -110,11 +110,11 @@ IBC/TAO implementations MUST implement the following paths for the `provableStor
 Future paths may be used in future versions of the protocol, so the entire key-space in the provable store MUST be reserved for the IBC handler.
 
 
-| Store          | Path format                                                                | Value type        | Defined in |
-| -------------- | -------------------------------------------------------------------------- | ----------------- | ---------------------- |
-| provableStore  | "commitments/channels/{identifier}/sequences/{bigEndianUint64Sequence}"    | bytes             | [ICS 4](../ics-004-packet-semantics) |
-| provableStore  | "receipts/channels/{identifier}/sequences/{bigEndianUint64Sequence}"       | bytes             | [ICS 4](../ics-004-packet-semantics) |
-| provableStore  | "acks/channels/{identifier}/sequences/{bigEndianUint64Sequence}"           | bytes             | [ICS 4](../ics-004-packet-semantics) |
+| Value                      | Path format                                       | Value type | Defined in                           |
+| -------------------------- | ------------------------------------------------- | ---------- | ------------------------------------ |
+| Packet Commitment          | {channelIdentifier}|0x1|{bigEndianUint64Sequence} | bytes      | [ICS 4](../ics-004-channel-and-packet-semantics) |
+| Packet Receipt             | {channelIdentifier}|0x2|{bigEndianUint64Sequence} | bytes      | [ICS 4](../ics-004-channel-and-packet-semantics) |
+| Acknowledgement Commitment | {channelIdentifier}|0x3|{bigEndianUint64Sequence} | bytes      | [ICS 4](../ics-004-channel-and-packet-semantics) |
 
 ### Provable Commitments
 

spec/eureka/README.md

@@ -191,17 +191,17 @@ function sendPacket(
     assert(timeoutHeight === 0 || latestClientHeight < timeoutHeight)
 
     // if the sequence doesn't already exist, this call initializes the sequence to 0
-    sequence = channelStore.get(nextSequenceSendPath(commitPort, sourceChannel))
+    sequence = channelStore.get(nextSequenceSendPath(sourcePort, sourceChannel))
     
     // store commitment to the packet data & packet timeout
     channelStore.set(
-      packetCommitmentPath(commitPort, sourceChannel, sequence),
+      packetCommitmentPath(sourcePort, sourceChannel, sequence),
       hash(hash(data), timeoutHeight, timeoutTimestamp)
     )
 
     // increment the sequence. Thus there are monotonically increasing sequences for packet flow
     // from sourcePort, sourceChannel pair
-    channelStore.set(nextSequenceSendPath(commitPort, sourceChannel), sequence+1)
+    channelStore.set(nextSequenceSendPath(sourcePort, sourceChannel), sequence+1)
 
     // log that a packet can be safely sent
     emitLogEntry("sendPacket", {