807 add client specific hashing on ack and packet data (#825)

* feat: hash depending on client type

* feat: add IBC prefix for ics-08 clients and bugfixes (#826)

* feat: Add revision number to light client heights

Add revision number but for now do not add support for resseting chain height

* revert to using two clients

* feat: add IBC prefix for ics-08 clients and bugfixes

* fix: hash ack value before sending to verify membership for ics8 client

---------

Co-authored-by: izyak <[email protected]>

---------

Co-authored-by: izyak <[email protected]>

contracts/javascore/ibc/src/intTest/java/ibc/core/integration/IBCIntegrationTest.java

@@ -64,7 +64,7 @@ static void setup() throws Exception {
     @Test
     @Order(0)
     void registerClient() {
-        getClientInterface(owner).registerClient(clientType, mockLightClient._address());
+        getClientInterface(owner).registerClient(clientType, mockLightClient._address(), 0);
     }
 
     @Test

contracts/javascore/ibc/src/main/java/ibc/ics02/client/IBCClient.java

@@ -3,6 +3,7 @@
 import ibc.icon.score.util.Logger;
 import ibc.icon.score.util.NullChecker;
 import ibc.ics24.host.IBCHost;
+import ibc.ics24.host.IBCStore;
 import icon.ibc.interfaces.ILightClient;
 import icon.ibc.structs.messages.MsgCreateClient;
 import icon.ibc.structs.messages.MsgUpdateClient;
@@ -16,9 +17,10 @@ public class IBCClient extends IBCHost {
 
     static Logger logger = new Logger("ibc-core");
 
-    public void registerClient(String clientType, Address lightClient) {
+    public void registerClient(String clientType, Address lightClient, int hashType) {
         Context.require(clientRegistry.get(clientType) == null, "Already registered.");
         clientRegistry.set(clientType, lightClient);
+        IBCStore.hashType.set(clientType, hashType);
     }
 
     public String _createClient(MsgCreateClient msg) {
@@ -32,6 +34,7 @@ public String _createClient(MsgCreateClient msg) {
         clientTypes.set(clientId, msg.getClientType());
         clientImplementations.set(clientId, lightClientAddr);
         btpNetworkId.set(clientId, msg.getBtpNetworkId());
+        // hashMethod.set(clientId, msg.getHashMethod());
 
         ILightClient client = getClient(clientId);
         client.createClient(clientId, msg.getClientState(), msg.getConsensusState());

contracts/javascore/ibc/src/main/java/ibc/ics03/connection/IBCConnection.java

@@ -1,17 +1,18 @@
 package ibc.ics03.connection;
 
 import java.math.BigInteger;
-import java.util.Arrays;
 import java.util.List;
 
 import icon.proto.core.client.Height;
 import icon.proto.core.connection.ConnectionEnd;
 import icon.proto.core.connection.Counterparty;
 import icon.proto.core.connection.Version;
+import ibc.core.commitment.v1.MerklePrefix;
 import ibc.icon.score.util.ByteUtil;
 import ibc.icon.score.util.Logger;
 import ibc.ics02.client.IBCClient;
 import ibc.ics24.host.IBCCommitment;
+import ibc.ics24.host.IBCHost;
 import icon.ibc.interfaces.ILightClient;
 import icon.ibc.structs.messages.MsgConnectionOpenAck;
 import icon.ibc.structs.messages.MsgConnectionOpenConfirm;
@@ -24,6 +25,12 @@ public class IBCConnection extends IBCClient {
     public static final String v1Identifier = "1";
     public static final List<String> supportedV1Features = List.of("ORDER_ORDERED", "ORDER_UNORDERED");
 
+    public static final MerklePrefix ICS08PREFIX = new MerklePrefix();
+    static {
+        ICS08PREFIX.setKeyPrefix("ibc".getBytes());
+
+    }
+
     Logger logger = new Logger("ibc-core");
 
     public String _connectionOpenInit(MsgConnectionOpenInit msg) {
@@ -67,6 +74,9 @@ public String _connectionOpenTry(MsgConnectionOpenTry msg) {
         Counterparty expectedCounterparty = new Counterparty();
         expectedCounterparty.setClientId(msg.getClientId());
         expectedCounterparty.setConnectionId("");
+        if (IBCCommitment.getHashType(msg.getClientId()) == IBCHost.HashType.ICS08.type) {
+            expectedCounterparty.setPrefix(ICS08PREFIX);
+        }
 
         ConnectionEnd expectedConnection = new ConnectionEnd();
         expectedConnection.setClientId(counterparty.getClientId());
@@ -108,6 +118,9 @@ public byte[] _connectionOpenAck(MsgConnectionOpenAck msg) {
         Counterparty expectedCounterparty = new Counterparty();
         expectedCounterparty.setClientId(connection.getClientId());
         expectedCounterparty.setConnectionId(msg.getConnectionId());
+        if (IBCCommitment.getHashType(connection.getClientId()) == IBCHost.HashType.ICS08.type) {
+            expectedCounterparty.setPrefix(ICS08PREFIX);
+        }
 
         ConnectionEnd expectedConnection = new ConnectionEnd();
         expectedConnection.setClientId(connection.getCounterparty().getClientId());
@@ -150,6 +163,9 @@ public byte[] _connectionOpenConfirm(MsgConnectionOpenConfirm msg) {
         Counterparty expectedCounterparty = new Counterparty();
         expectedCounterparty.setClientId(connection.getClientId());
         expectedCounterparty.setConnectionId(msg.getConnectionId());
+        if (IBCCommitment.getHashType(connection.getClientId()) == IBCHost.HashType.ICS08.type) {
+            expectedCounterparty.setPrefix(ICS08PREFIX);
+        }
 
         ConnectionEnd expectedConnection = new ConnectionEnd();
         expectedConnection.setClientId(connection.getCounterparty().getClientId());

contracts/javascore/ibc/src/main/java/ibc/ics04/channel/IBCPacket.java

@@ -3,6 +3,7 @@
 import ibc.icon.score.util.ByteUtil;
 import ibc.icon.score.util.Proto;
 import ibc.ics24.host.IBCCommitment;
+import ibc.ics24.host.IBCHost;
 import icon.ibc.interfaces.ILightClient;
 import icon.ibc.structs.messages.MsgRequestTimeoutPacket;
 import score.Context;
@@ -58,7 +59,7 @@ public void _sendPacket(Packet packet) {
                 packet.getSourceChannel(),
                 packet.getSequence());
 
-        byte[] packetCommitment = createPacketCommitment(packet);
+        byte[] packetCommitment = createPacketCommitment(connection.getClientId(), packet);
         commitments.set(packetCommitmentKey, packetCommitment);
         packetHeights.at(packet.getSourcePort()).at(packet.getSourceChannel()).set(packet.getSequence(),
                 Context.getBlockHeight());
@@ -99,7 +100,7 @@ public void _recvPacket(Packet packet, byte[] proof, byte[] proofHeight) {
 
         byte[] commitmentPath = IBCCommitment.packetCommitmentPath(packet.getSourcePort(),
                 packet.getSourceChannel(), packet.getSequence());
-        byte[] commitmentBytes = createPacketCommitmentBytes(packet);
+        byte[] commitmentBytes = createPacketCommitmentBytes(connection.getClientId(), packet);
 
         verifyPacketCommitment(
                 connection,
@@ -145,7 +146,7 @@ public void _writeAcknowledgement(String destinationPortId, String destinationCh
         byte[] ackCommitmentKey = IBCCommitment.packetAcknowledgementCommitmentKey(destinationPortId,
                 destinationChannel, sequence);
         Context.require(commitments.get(ackCommitmentKey) == null, "acknowledgement for packet already exists");
-        byte[] ackCommitment = IBCCommitment.keccak256(acknowledgement);
+        byte[] ackCommitment = createAcknowledgmentCommitment(connection.getClientId(), acknowledgement);
         commitments.set(ackCommitmentKey, ackCommitment);
         ackHeights.at(destinationPortId).at(destinationChannel).set(sequence,
                 Context.getBlockHeight());
@@ -174,18 +175,19 @@ public void _acknowledgePacket(Packet packet, byte[] acknowledgement, byte[] pro
                 packet.getSourceChannel(), packet.getSequence());
         byte[] packetCommitment = commitments.get(packetCommitmentKey);
         Context.require(packetCommitment != null, "packet commitment not found");
-        byte[] commitment = createPacketCommitment(packet);
+        byte[] commitment = createPacketCommitment(connection.getClientId(), packet);
 
         Context.require(Arrays.equals(packetCommitment, commitment), "commitment byte[] are not equal");
 
         byte[] packetAckPath = IBCCommitment.packetAcknowledgementCommitmentPath(packet.getDestinationPort(),
                 packet.getDestinationChannel(), packet.getSequence());
+        byte[] commitmentBytes = createAcknowledgmentCommitmentBytes(connection.getClientId(), acknowledgement);
         verifyPacketAcknowledgement(
                 connection,
                 proofHeight,
                 proof,
                 packetAckPath,
-                acknowledgement);
+                commitmentBytes);
 
         if (channel.getOrdering() == Channel.Order.ORDER_ORDERED) {
             DictDB<String, BigInteger> nextSequenceAckSourcePort = nextSequenceAcknowledgements
@@ -236,7 +238,7 @@ public void _requestTimeout(MsgRequestTimeoutPacket msg) {
 
         byte[] commitmentPath = IBCCommitment.packetCommitmentPath(packet.getSourcePort(),
                 packet.getSourceChannel(), packet.getSequence());
-        byte[] commitmentBytes = createPacketCommitmentBytes(packet);
+        byte[] commitmentBytes = createPacketCommitmentBytes(connection.getClientId(), packet);
         verifyPacketCommitment(
                 connection,
                 proofHeight,
@@ -300,7 +302,7 @@ public void _timeoutPacket(Packet packet, byte[] proofHeight, byte[] proof, BigI
                 packet.getSourceChannel(), packet.getSequence());
         byte[] packetCommitment = commitments.get(packetCommitmentKey);
         Context.require(packetCommitment != null, "packet commitment not found");
-        byte[] commitment = createPacketCommitment(packet);
+        byte[] commitment = createPacketCommitment(connection.getClientId(), packet);
 
         Context.require(Arrays.equals(packetCommitment, commitment), "commitment byte[] are not equal");
 
@@ -432,11 +434,20 @@ private BigInteger calcBlockDelay(BigInteger timeDelay) {
         return blockDelay;
     }
 
-    private byte[] createPacketCommitment(Packet packet) {
-        return IBCCommitment.keccak256(createPacketCommitmentBytes(packet));
+    private byte[] createPacketCommitment(String clientId, Packet packet) {
+        return IBCCommitment.keccak256(createPacketCommitmentBytes(clientId, packet));
+    }
+
+    private byte[] createPacketCommitmentBytes(String clientId, Packet packet) {
+        int hashType = IBCCommitment.getHashType(clientId);
+        if (hashType == IBCHost.HashType.ICS08.type) {
+           return createIBCPacketCommitmentBytes(packet);
+        }
+
+        return createWasmPacketCommitmentBytes(packet);
     }
 
-    public static byte[] createPacketCommitmentBytes(Packet packet) {
+    public static byte[] createWasmPacketCommitmentBytes(Packet packet) {
         return ByteUtil.join(
                 Proto.encodeFixed64(packet.getTimeoutTimestamp(), false),
                 Proto.encodeFixed64(packet.getTimeoutHeight().getRevisionNumber(),
@@ -446,6 +457,29 @@ public static byte[] createPacketCommitmentBytes(Packet packet) {
                 IBCCommitment.keccak256(packet.getData()));
     }
 
+    public static byte[] createIBCPacketCommitmentBytes(Packet packet) {
+        return IBCCommitment.sha256(ByteUtil.join(
+                Proto.encodeFixed64(packet.getTimeoutTimestamp(), false),
+                Proto.encodeFixed64(packet.getTimeoutHeight().getRevisionNumber(),
+                        false),
+                Proto.encodeFixed64(packet.getTimeoutHeight().getRevisionHeight(),
+                        false),
+                IBCCommitment.sha256(packet.getData())));
+    }
+
+    private byte[] createAcknowledgmentCommitment(String clientId, byte[] ack) {
+        return IBCCommitment.keccak256(createAcknowledgmentCommitmentBytes(clientId, ack));
+    }
+
+    public static byte[] createAcknowledgmentCommitmentBytes(String clientId, byte[] ack) {
+        int hashType = IBCCommitment.getHashType(clientId);
+        if (hashType == IBCHost.HashType.ICS08.type) {
+            return IBCCommitment.sha256(ack);
+        }
+
+        return ack;
+    }
+
     private boolean lt(Height h1, Height h2) {
         return h1.getRevisionNumber().compareTo(h2.getRevisionNumber()) < 0
                 || (h1.getRevisionNumber().equals(h2.getRevisionNumber())

contracts/javascore/ibc/src/main/java/ibc/ics23/commitment/types/Merkle.java

@@ -20,10 +20,10 @@ public class Merkle {
         Proof.getTendermintSpec()
     );
 
-    public static MerklePath applyPrefix(String path) {
+    public static MerklePath applyPrefix(String prefix, String path) {
         var mpath = new MerklePath();
         List<String> keyPath = new ArrayList<>();
-        keyPath.add(StringUtil.bytesToHex("wasm".getBytes()));
+        keyPath.add(prefix);
         keyPath.add(path);
         mpath.setKeyPath(keyPath);
         return mpath;

contracts/javascore/ibc/src/main/java/ibc/ics24/host/IBCCommitment.java

@@ -11,8 +11,8 @@
  * "https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements#path-space">path-space</a>
  */
 public class IBCCommitment {
-    private static final String KECCAK256 = "keccak-256";
-    private static final String SHA256 = "sha-256";
+    public static final String KECCAK256 = "keccak-256";
+    public static final String SHA256 = "sha-256";
 
     public static byte[] keccak256(byte[] msg) {
         return Context.hash(KECCAK256, msg);
@@ -22,6 +22,11 @@ public static byte[] sha256(byte[] msg) {
         return Context.hash(SHA256, msg);
     }
 
+    public static int getHashType(String clientId) {
+        String clientType = IBCStore.clientTypes.get(clientId);
+        return IBCStore.hashType.getOrDefault(clientType, IBCHost.HashType.WASM.type);
+    }
+
     public static byte[] clientStatePath(String clientId) {
         return StringUtil.encodePacked("clients/", clientId, "/clientState");
     }

contracts/javascore/ibc/src/main/java/ibc/ics24/host/IBCHost.java

@@ -11,6 +11,25 @@ public class IBCHost extends IBCStore {
     private static final String TAG = "IBCHOST: ";
     private static final Address chainScore = Address.fromString("cx0000000000000000000000000000000000000000");
 
+    public enum HashType {
+        WASM(0),
+        ICS08(1);
+
+        public final int type;
+        HashType(int type){ this.type = type; }
+
+        public int type() { return type; }
+
+        static public HashType of(int type) {
+            for(HashType t : values()) {
+                if (t.type == type) {
+                    return t;
+                }
+            }
+            throw new IllegalArgumentException();
+        }
+    }
+
     /***
      * claimCapability allows the IBC app module to claim a capability that core IBC
      * passes to it

contracts/javascore/ibc/src/main/java/ibc/ics24/host/IBCStore.java

@@ -35,6 +35,7 @@ public abstract class IBCStore extends ModuleManager implements IIBCHost {
     private static final String BTP_NETWORK_ID = "btpNetworkId";
     private static final String TIMEOUT_REQUESTS = "timeout_requests";
     private static final String ACK_HEIGHTS = "ackHeights";
+    private static final String HASH_METHOD = "hashMethod";
 
     // DB Variables
     // Commitments
@@ -81,6 +82,8 @@ public abstract class IBCStore extends ModuleManager implements IIBCHost {
     public static final DictDB<String, Integer> btpNetworkId = Context.newDictDB(BTP_NETWORK_ID, Integer.class);
     public static final DictDB<byte[], Boolean> timeoutRequests = Context.newDictDB(TIMEOUT_REQUESTS, Boolean.class);
 
+    public static final DictDB<String, Integer> hashType = Context.newDictDB(HASH_METHOD, Integer.class);
+
     @External(readonly = true)
     public byte[] getCommitment(byte[] key) {
         return commitments.get(key);

contracts/javascore/ibc/src/main/java/ibc/ics25/handler/IBCHandler.java

@@ -3,9 +3,11 @@
 import score.Address;
 import score.Context;
 import score.annotation.External;
+import score.annotation.Optional;
 
 import java.math.BigInteger;
 
+import ibc.ics24.host.IBCHost;
 import icon.ibc.structs.messages.MsgCreateClient;
 
 public class IBCHandler extends IBCHandlerPacket {
@@ -24,9 +26,10 @@ public String name() {
      * registerClient registers a new client type into the client registry
      */
     @External
-    public void registerClient(String clientType, Address client) {
+    public void registerClient(String clientType, Address client, @Optional int hashType) {
         onlyOwner();
-        super.registerClient(clientType, client);
+        IBCHost.HashType.of(hashType);
+        super.registerClient(clientType, client, hashType);
     }
 
     @External

contracts/javascore/ibc/src/test/java/ibc/ics02/client/ClientTest.java

@@ -44,13 +44,13 @@ public void setup() throws Exception {
     void registerClient_alreadyRegistered() {
         // Arrange
         String clientType = "clientType";
-        client.invoke(owner, "registerClient", clientType, lightClient.getAddress());
+        client.invoke(owner, "registerClient", clientType, lightClient.getAddress(), 0);
 
         // Act & Assert
         String expectedErrorMessage = "Already registered";
         Executable registerWithSameType = () -> {
             client.invoke(owner, "registerClient", clientType,
-                    lightClient.getAddress());
+                    lightClient.getAddress(), 0);
         };
         AssertionError e = assertThrows(AssertionError.class, registerWithSameType);
         assertTrue(e.getMessage().contains(expectedErrorMessage));
@@ -85,7 +85,7 @@ void createClient() {
         String expectedClientId = msg.getClientType() + "-0";
 
         // Act
-        client.invoke(owner, "registerClient", msg.getClientType(), lightClient.getAddress());
+        client.invoke(owner, "registerClient", msg.getClientType(), lightClient.getAddress(), 0);
         client.invoke(owner, "_createClient", msg);
 
         // Assert

contracts/javascore/ibc/src/test/java/ibc/ics04/channel/PacketTest.java

@@ -1,6 +1,6 @@
 package ibc.ics04.channel;
 
-import static ibc.ics04.channel.IBCPacket.createPacketCommitmentBytes;
+import static ibc.ics04.channel.IBCPacket.createWasmPacketCommitmentBytes;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -405,7 +405,7 @@ void recvPacket_outOfOrder_UnOrdered() {
                 basePacket.getSourceChannel(), basePacket.getSequence());
         byte[] commitmentPath2 = IBCCommitment.packetCommitmentPath(basePacket.getSourcePort(),
                 basePacket.getSourceChannel(), basePacket.getSequence().add(BigInteger.ONE));
-        byte[] commitmentBytes = createPacketCommitmentBytes(basePacket);
+        byte[] commitmentBytes = createWasmPacketCommitmentBytes(basePacket);
 
         // Act
         basePacket.setSequence(BigInteger.TWO);
@@ -450,7 +450,7 @@ void recvPacket_UnOrdered() {
 
         byte[] commitmentPath = IBCCommitment.packetCommitmentPath(basePacket.getSourcePort(),
                 basePacket.getSourceChannel(), basePacket.getSequence());
-        byte[] commitmentBytes = createPacketCommitmentBytes(basePacket);
+        byte[] commitmentBytes = createWasmPacketCommitmentBytes(basePacket);
 
         // Act
         packet.invoke(owner, "_recvPacket", basePacket, proof, proofHeight.encode());
@@ -470,7 +470,7 @@ void recvPacket_Ordered() {
 
         byte[] commitmentPath = IBCCommitment.packetCommitmentPath(basePacket.getSourcePort(),
                 basePacket.getSourceChannel(), basePacket.getSequence());
-        byte[] commitmentBytes = createPacketCommitmentBytes(basePacket);
+        byte[] commitmentBytes = createWasmPacketCommitmentBytes(basePacket);
 
         // Act
         packet.invoke(owner, "_recvPacket", basePacket, proof, proofHeight.encode());