Merge pull request #214 from SocketDotTech/fix/message-id

Update message id to include sibling plug
SocketDotTech · May 16, 2023 · f0c6085 · f0c6085
2 parents 85e0f5d + 3fb8d93
commit f0c6085
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 40 deletions.
diff --git a/contracts/interfaces/ISocket.sol b/contracts/interfaces/ISocket.sol
@@ -125,12 +125,10 @@ interface ISocket {
     /**
      * @notice executes a message
      * @param packetId packet id
-     * @param localPlug local plug address
      * @param messageDetails_ the details needed for message verification
      */
     function execute(
         bytes32 packetId,
-        address localPlug,
         ISocket.MessageDetails calldata messageDetails_,
         bytes memory signature
     ) external;

diff --git a/contracts/socket/SocketBase.sol b/contracts/socket/SocketBase.sol
@@ -15,7 +15,7 @@ abstract contract SocketBase is SocketConfig {
 
     uint32 public immutable chainSlug;
     // incrementing nonce, should be handled in next socket version.
-    uint224 public messageCount;
+    uint64 public messageCount;
 
     bytes32 public immutable version;
 

diff --git a/contracts/socket/SocketBatcher.sol b/contracts/socket/SocketBatcher.sol
@@ -70,7 +70,6 @@ contract SocketBatcher is AccessControl {
      */
     struct ExecuteRequest {
         bytes32 packetId;
-        address localPlug;
         ISocket.MessageDetails messageDetails;
         bytes signature;
     }
@@ -326,7 +325,6 @@ contract SocketBatcher is AccessControl {
         for (uint256 index = 0; index < executeRequestslength; ) {
             ISocket(socketAddress_).execute(
                 executeRequests_[index].packetId,
-                executeRequests_[index].localPlug,
                 executeRequests_[index].messageDetails,
                 executeRequests_[index].signature
             );

diff --git a/contracts/socket/SocketDst.sol b/contracts/socket/SocketDst.sol
@@ -102,12 +102,10 @@ abstract contract SocketDst is SocketBase {
     /**
      * @notice executes a message, fees will go to recovered executor address
      * @param packetId_ packet id
-     * @param localPlug_ remote plug address
      * @param messageDetails_ the details needed for message verification
      */
     function execute(
         bytes32 packetId_,
-        address localPlug_,
         ISocket.MessageDetails calldata messageDetails_,
         bytes memory signature_
     ) external override {
@@ -116,14 +114,15 @@ abstract contract SocketDst is SocketBase {
         messageExecuted[messageDetails_.msgId] = true;
 
         uint32 remoteSlug = _decodeSlug(messageDetails_.msgId);
+        address localPlug = _decodePlug(messageDetails_.msgId);
 
-        PlugConfig storage plugConfig = _plugConfigs[localPlug_][remoteSlug];
+        PlugConfig storage plugConfig = _plugConfigs[localPlug][remoteSlug];
 
         bytes32 packedMessage = hasher__.packMessage(
             remoteSlug,
             plugConfig.siblingPlug,
             chainSlug,
-            localPlug_,
+            localPlug,
             messageDetails_.msgId,
             messageDetails_.msgGasLimit,
             messageDetails_.executionFee,
@@ -144,7 +143,7 @@ abstract contract SocketDst is SocketBase {
         _execute(
             executor,
             messageDetails_.executionFee,
-            localPlug_,
+            localPlug,
             remoteSlug,
             messageDetails_.msgGasLimit,
             messageDetails_.msgId,
@@ -224,9 +223,18 @@ abstract contract SocketDst is SocketBase {
     }
 
     /**
-     * @dev Decodes the chain ID from a given packet ID.
-     * @param id_ The ID of the packet to decode the chain ID from.
-     * @return chainSlug_ The chain ID decoded from the packet ID.
+     * @dev Decodes the plug address from a given message id.
+     * @param id_ The ID of the msg to decode the plug from.
+     * @return plug_ The address of sibling plug decoded from the message ID.
+     */
+    function _decodePlug(bytes32 id_) internal pure returns (address plug_) {
+        plug_ = address(uint160(uint256(id_) >> 64));
+    }
+
+    /**
+     * @dev Decodes the chain ID from a given packet/message ID.
+     * @param id_ The ID of the packet/msg to decode the chain slug from.
+     * @return chainSlug_ The chain slug decoded from the packet/message ID.
      */
     function _decodeSlug(
         bytes32 id_

diff --git a/contracts/socket/SocketSrc.sol b/contracts/socket/SocketSrc.sol
@@ -42,7 +42,7 @@ abstract contract SocketSrc is SocketBase {
         ];
         uint32 localChainSlug = chainSlug;
 
-        msgId = _encodeMsgId(localChainSlug);
+        msgId = _encodeMsgId(localChainSlug, plugConfig.siblingPlug);
 
         ISocket.Fees memory fees = _validateAndGetFees(
             msgGasLimit_,
@@ -226,9 +226,17 @@ abstract contract SocketSrc is SocketBase {
 
     // Packs the local plug, local chain slug, remote chain slug and nonce
     // messageCount++ will take care of msg id overflow as well
-    // msgId(256) = localChainSlug(32) | nonce(224)
-    function _encodeMsgId(uint32 slug_) internal returns (bytes32) {
-        return bytes32((uint256(slug_) << 224) | messageCount++);
+    // msgId(256) = localChainSlug(32) | siblingPlug_(160) | nonce(64)
+    function _encodeMsgId(
+        uint32 slug_,
+        address siblingPlug_
+    ) internal returns (bytes32) {
+        return
+            bytes32(
+                (uint256(slug_) << 224) |
+                    (uint256(uint160(siblingPlug_)) << 64) |
+                    messageCount++
+            );
     }
 
     function _encodePacketId(

diff --git a/test/IntegrationTest.t.sol b/test/IntegrationTest.t.sol
@@ -131,13 +131,14 @@ contract HappyTest is Setup {
         }
 
         vm.expectEmit(true, false, false, false);
-        emit ExecutionSuccess(_packMessageId(_a.chainSlug, 0));
+        emit ExecutionSuccess(
+            _packMessageId(_a.chainSlug, address(dstCounter__), 0)
+        );
         _executePayloadOnDst(
             _b,
             _a.chainSlug,
-            address(dstCounter__),
             packetId,
-            _packMessageId(_a.chainSlug, 0),
+            _packMessageId(_a.chainSlug, address(dstCounter__), 0),
             _msgGasLimit,
             executionFee,
             root,
@@ -148,16 +149,17 @@ contract HappyTest is Setup {
         assertEq(dstCounter__.counter(), amount);
         assertEq(srcCounter__.counter(), 0);
         assertTrue(
-            _b.socket__.messageExecuted(_packMessageId(_a.chainSlug, 0))
+            _b.socket__.messageExecuted(
+                _packMessageId(_a.chainSlug, address(dstCounter__), 0)
+            )
         );
 
         vm.expectRevert(SocketDst.MessageAlreadyExecuted.selector);
         _executePayloadOnDst(
             _b,
             _a.chainSlug,
-            address(dstCounter__),
             packetId,
-            _packMessageId(_a.chainSlug, 0),
+            _packMessageId(_a.chainSlug, address(dstCounter__), 0),
             _msgGasLimit,
             executionFee,
             root,
@@ -204,9 +206,8 @@ contract HappyTest is Setup {
         _executePayloadOnDst(
             _a,
             _b.chainSlug,
-            address(srcCounter__),
             packetId,
-            _packMessageId(_b.chainSlug, 0),
+            _packMessageId(_b.chainSlug, address(srcCounter__), 0),
             _msgGasLimit,
             0,
             root,
@@ -235,7 +236,7 @@ contract HappyTest is Setup {
             msgGasLimit
         );
 
-        msgId = _packMessageId(_a.chainSlug, count);
+        msgId = _packMessageId(_a.chainSlug, address(dstCounter__), count);
         root = _a.hasher__.packMessage(
             _a.chainSlug,
             address(srcCounter__),
@@ -320,7 +321,6 @@ contract HappyTest is Setup {
         _executePayloadOnDst(
             _b,
             _a.chainSlug,
-            address(dstCounter__),
             packetId,
             msgId1,
             _msgGasLimit,
@@ -340,7 +340,6 @@ contract HappyTest is Setup {
         _executePayloadOnDst(
             _b,
             _a.chainSlug,
-            address(dstCounter__),
             packetId,
             msgId2,
             _msgGasLimit,

diff --git a/test/Setup.t.sol b/test/Setup.t.sol
@@ -491,7 +491,6 @@ contract Setup is Test {
 
     function _executePayloadOnDstWithExecutor(
         ChainContext storage dst_,
-        address remotePlug_,
         bytes32 packetId_,
         bytes32 msgId_,
         uint256 msgGasLimit_,
@@ -513,13 +512,12 @@ contract Setup is Test {
             packedMessage_,
             executorPrivateKey_
         );
-        dst_.socket__.execute(packetId_, remotePlug_, msgDetails, sig);
+        dst_.socket__.execute(packetId_, msgDetails, sig);
     }
 
     function _executePayloadOnDst(
         ChainContext storage dst_,
         uint256,
-        address remotePlug_,
         bytes32 packetId_,
         bytes32 msgId_,
         uint256 msgGasLimit_,
@@ -530,7 +528,6 @@ contract Setup is Test {
     ) internal {
         _executePayloadOnDstWithExecutor(
             dst_,
-            remotePlug_,
             packetId_,
             msgId_,
             msgGasLimit_,
@@ -543,10 +540,16 @@ contract Setup is Test {
     }
 
     function _packMessageId(
-        uint32 srcChainSlug,
-        uint256 nonce
+        uint32 srcChainSlug_,
+        address siblingPlug_,
+        uint256 messageCount_
     ) internal pure returns (bytes32) {
-        return bytes32((uint256(srcChainSlug) << 224) | nonce);
+        return
+            bytes32(
+                (uint256(srcChainSlug_) << 224) |
+                    (uint256(uint160(siblingPlug_)) << 64) |
+                    messageCount_
+            );
     }
 
     function _getPackedId(

diff --git a/test/socket/SocketDst.t.sol b/test/socket/SocketDst.t.sol
@@ -249,7 +249,7 @@ contract SocketDstTest is Setup {
             );
         }
 
-        bytes32 msgId = _packMessageId(_a.chainSlug, 0);
+        bytes32 msgId = _packMessageId(_a.chainSlug, address(dstCounter__), 0);
         (bytes32 packetId, bytes32 root) = sealAndPropose(capacitor);
         _attestOnDst(
             address(_b.configs__[index].switchboard__),
@@ -263,7 +263,6 @@ contract SocketDstTest is Setup {
         _executePayloadOnDst(
             _b,
             _a.chainSlug,
-            address(dstCounter__),
             packetId,
             msgId,
             _msgGasLimit,
@@ -281,7 +280,6 @@ contract SocketDstTest is Setup {
         _executePayloadOnDst(
             _b,
             _a.chainSlug,
-            address(dstCounter__),
             packetId,
             msgId,
             _msgGasLimit,
@@ -332,7 +330,7 @@ contract SocketDstTest is Setup {
             );
         }
 
-        bytes32 msgId = _packMessageId(_a.chainSlug, 0);
+        bytes32 msgId = _packMessageId(_a.chainSlug, address(dstCounter__), 0);
         (bytes32 packetId, bytes32 root) = sealAndPropose(capacitor);
         _attestOnDst(
             address(_b.configs__[index].switchboard__),
@@ -343,7 +341,6 @@ contract SocketDstTest is Setup {
         vm.expectRevert(NotExecutor.selector);
         _executePayloadOnDstWithExecutor(
             _b,
-            address(dstCounter__),
             packetId,
             msgId,
             _msgGasLimit,