Fix UpdateMember encoding & decoding (#32)

centrifuge · Jan 30, 2023 · e2832fe · e2832fe
1 parent f744dd2
commit e2832fe
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 35 deletions.
diff --git a/src/Connector.sol b/src/Connector.sol
@@ -141,7 +141,7 @@ contract CentrifugeConnector {
         uint64 poolId,
         bytes16 trancheId,
         address user,
-        uint256 validUntil
+        uint64 validUntil
     ) public onlyRouter {
         Tranche storage tranche = tranches[poolId][trancheId];
         require(tranche.latestPrice > 0, "CentrifugeConnector/invalid-pool-or-tranche");

diff --git a/src/Messages.sol b/src/Messages.sol
@@ -50,7 +50,7 @@ library ConnectorMessages {
     function formatAddTranche(uint64 poolId, bytes16 trancheId, string memory tokenName, string memory tokenSymbol) internal pure returns (bytes memory) {
         // TODO(nuno): Now, we encode `tokenName` as a 128-bytearray by first encoding `tokenName`
         // to bytes32 and then we encode three empty bytes32's, which sum up to a total of 128 bytes.
-        // Add support to actually encode `tokennName` fully as a 128 bytes string.
+        // Add support to actually encode `tokenName` fully as a 128 bytes string.
         return abi.encodePacked(uint8(Call.AddTranche), poolId, trancheId, stringToBytes32(tokenName), bytes32(""), bytes32(""), bytes32(""), stringToBytes32(tokenSymbol));
     }
 
@@ -96,25 +96,27 @@ library ConnectorMessages {
      * 0: call type (uint8 = 1 byte)
      * 1-8: poolId (uint64 = 8 bytes)
      * 9-25: trancheId (16 bytes)
-     * 26-46: user (Ethereum address, 20 bytes)
-     * 47-78: validUntil (uint256 = 32 bytes)
+     * 25-45: user (Ethereum address, 20 bytes - Skip 12 bytes from 32-byte addresses)
+     * 57-65: validUntil (uint64 = 8 bytes)
      * 
      * TODO: use bytes32 for user (for non-EVM compatibility)
      */
-    function formatUpdateMember(uint64 poolId, bytes16 trancheId, address user, uint256 validUntil) internal pure returns (bytes memory) {
-        return abi.encodePacked(uint8(Call.UpdateMember), poolId, trancheId, user, validUntil);
+    function formatUpdateMember(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) internal pure returns (bytes memory) {
+        // NOTE: Since parseUpdateMember parses the first 20 bytes of `user` and skips the following 12
+        // here we need to append 12 zeros to make it right. Drop once we support 32-byte addresses.
+        return abi.encodePacked(uint8(Call.UpdateMember), poolId, trancheId, user, bytes(hex"000000000000000000000000"), validUntil);
     }
 
     function isUpdateMember(bytes29 _msg) internal pure returns (bool) {
         return messageType(_msg) == Call.UpdateMember;
     }
 
-    function parseUpdateMember(bytes29 _msg) internal pure returns (uint64 poolId, bytes16 trancheId, address user, uint256 validUntil) {
+
+    function parseUpdateMember(bytes29 _msg) internal pure returns (uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) {
         poolId = uint64(_msg.indexUint(1, 8));
         trancheId = bytes16(_msg.index(9, 16));
         user = address(bytes20(_msg.index(25, 20)));
-        // TODO: skip 12 padded zeroes from address
-        validUntil = uint256(_msg.index(45, 32));
+        validUntil = uint64(_msg.indexUint(57, 8));
     }
 
     /**

diff --git a/src/routers/xcm/Router.sol b/src/routers/xcm/Router.sol
@@ -8,7 +8,7 @@ import {ConnectorMessages} from "../../Messages.sol";
 interface ConnectorLike {
   function addPool(uint64 poolId) external;
   function addTranche(uint64 poolId, bytes16 trancheId, string memory tokenName, string memory tokenSymbol) external;
-  function updateMember(uint64 poolId, bytes16 trancheId, address user, uint256 validUntil) external;
+  function updateMember(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) external;
   function updateTokenPrice(uint64 poolId, bytes16 trancheId, uint256 price) external;
   function handleTransfer(uint64 poolId, bytes16 trancheId, address user, uint256 amount) external;
 }
@@ -50,7 +50,7 @@ contract ConnectorXCMRouter {
             (uint64 poolId, bytes16 trancheId, string memory tokenName, string memory tokenSymbol) = ConnectorMessages.parseAddTranche(_msg);
             connector.addTranche(poolId, trancheId, tokenName, tokenSymbol);
         } else if (ConnectorMessages.isUpdateMember(_msg)) {
-            (uint64 poolId, bytes16 trancheId, address user, uint256 validUntil) = ConnectorMessages.parseUpdateMember(_msg);
+            (uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) = ConnectorMessages.parseUpdateMember(_msg);
             connector.updateMember(poolId, trancheId, user, validUntil);
         } else if (ConnectorMessages.isUpdateTokenPrice(_msg)) {
             (uint64 poolId, bytes16 trancheId, uint256 price) = ConnectorMessages.parseUpdateTokenPrice(_msg);

diff --git a/test/Connector.t.sol b/test/Connector.t.sol
@@ -92,9 +92,8 @@ contract ConnectorTest is Test {
         homeConnector.addTranche(poolId, trancheId, tokenName, tokenSymbol);
     }
 
-    function testUpdatingMemberWorks(uint64 poolId, bytes16 trancheId, address user, uint128 fuzzed_uint128) public {
-        vm.assume(fuzzed_uint128 > 0);
-        uint256 validUntil = safeAdd(fuzzed_uint128, safeAdd(block.timestamp, minimumDelay));
+    function testUpdatingMemberWorks(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) public {
+        vm.assume(validUntil >= safeAdd(block.timestamp, new Memberlist().minimumDelay()));
         vm.assume(user != address(0));
 
         homeConnector.addPool(poolId);
@@ -110,7 +109,7 @@ contract ConnectorTest is Test {
         assertEq(memberlist.members(user), validUntil);
     }
 
-    function testUpdatingMemberBeforeMinimumDelayFails(uint64 poolId, bytes16 trancheId, address user, uint256 validUntil) public {
+    function testUpdatingMemberBeforeMinimumDelayFails(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) public {
         vm.assume(validUntil <= safeAdd(block.timestamp, new Memberlist().minimumDelay()));
         vm.assume(user != address(0));
 
@@ -121,24 +120,23 @@ contract ConnectorTest is Test {
         homeConnector.updateMember(poolId, trancheId, user, validUntil);
     }
 
-    function testUpdatingMemberAsNonRouterFails(uint64 poolId, bytes16 trancheId, address user, uint128 fuzzed_uint128) public {
-        vm.assume(fuzzed_uint128 > 0);
-        uint256 validUntil = safeAdd(fuzzed_uint128, safeAdd(block.timestamp, new Memberlist().minimumDelay()));
+    function testUpdatingMemberAsNonRouterFails(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) public {
+        vm.assume(validUntil <= safeAdd(block.timestamp, new Memberlist().minimumDelay()));
         vm.assume(user != address(0));
 
         vm.expectRevert(bytes("CentrifugeConnector/not-the-router"));
         bridgedConnector.updateMember(poolId, trancheId, user, validUntil);
     }
 
-    function testUpdatingMemberForNonExistentPoolFails(uint64 poolId, bytes16 trancheId, address user, uint256 validUntil) public {
+    function testUpdatingMemberForNonExistentPoolFails(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) public {
         vm.assume(validUntil > block.timestamp);
         bridgedConnector.file("router", address(this));
         vm.expectRevert(bytes("CentrifugeConnector/invalid-pool-or-tranche"));
         bridgedConnector.updateMember(poolId, trancheId, user, validUntil);
     }
 
 
-    function testUpdatingMemberForNonExistentTrancheFails(uint64 poolId, bytes16 trancheId, address user, uint256 validUntil) public {
+    function testUpdatingMemberForNonExistentTrancheFails(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) public {
         vm.assume(validUntil > block.timestamp);
         bridgedConnector.file("router", address(this));
         bridgedConnector.addPool(poolId);

diff --git a/test/Messages.t.sol b/test/Messages.t.sol
@@ -91,41 +91,41 @@ contract MessagesTest is Test {
 
     function testUpdateMemberEncoding() public {
         assertEq(
-            ConnectorMessages.formatUpdateMember(5, toBytes16(fromHex("010000000000000003")), 0x225ef95fa90f4F7938A5b34234d14768cB4263dd, 1657870537), 
-            fromHex("04000000000000000500000000000000000000000000000009225ef95fa90f4f7938a5b34234d14768cb4263dd0000000000000000000000000000000000000000000000000000000062d118c9")
-            );
+            ConnectorMessages.formatUpdateMember(2, bytes16(hex"811acd5b3f17c06841c7e41e9e04cb1b"), 0x1231231231231231231231231231231231231231, 1706260138),
+            hex"040000000000000002811acd5b3f17c06841c7e41e9e04cb1b12312312312312312312312312312312312312310000000000000000000000000000000065b376aa"
+        );
     }
 
     function testUpdateMemberDecoding() public {
-        (uint64 decodedPoolId, bytes16 decodedTrancheId, address decodedUser, uint256 decodedValidUntil) = ConnectorMessages.parseUpdateMember(fromHex("04000000000000000500000000000000000000000000000009225ef95fa90f4f7938a5b34234d14768cb4263dd0000000000000000000000000000000000000000000000000000000062d118c9").ref(0));
-        assertEq(uint(decodedPoolId), uint(5));
-        assertEq(decodedTrancheId, toBytes16(fromHex("010000000000000003")));
-        assertEq(decodedUser, 0x225ef95fa90f4F7938A5b34234d14768cB4263dd);
-        assertEq(decodedValidUntil, uint(1657870537));
+        (uint64 decodedPoolId, bytes16 decodedTrancheId, address decodedUser, uint64 decodedValidUntil) = ConnectorMessages.parseUpdateMember(fromHex("040000000000000002811acd5b3f17c06841c7e41e9e04cb1b12312312312312312312312312312312312312312312312312312312312312310000000065B376AA").ref(0));
+        assertEq(uint(decodedPoolId), uint(2));
+        assertEq(decodedTrancheId, hex"811acd5b3f17c06841c7e41e9e04cb1b");
+        assertEq(decodedUser, 0x1231231231231231231231231231231231231231);
+        assertEq(decodedValidUntil, uint(1706260138));
     }
 
     function testUpdateMemberEquivalence(
         uint64 poolId,
         bytes16 trancheId,
         address user,
-        uint256 amount
+        uint64 validUntil
     ) public {
         bytes memory _message = ConnectorMessages.formatUpdateMember(
             poolId,
             trancheId,
             user,
-            amount
+            validUntil
         );
         (
             uint64 decodedPoolId,
             bytes16 decodedTrancheId,
             address decodedUser,
-            uint256 decodedAmount
+            uint64 decodedValidUntil
         ) = ConnectorMessages.parseUpdateMember(_message.ref(0));
-        assertEq(uint256(decodedPoolId), uint256(poolId));
+        assertEq(uint(decodedPoolId), uint(poolId));
         assertEq(decodedTrancheId, trancheId);
         assertEq(decodedUser, user);
-        assertEq(decodedAmount, amount);
+        assertEq(uint(decodedValidUntil), uint(validUntil));
     }
 
     function testUpdateTokenPriceEncoding() public {

diff --git a/test/mock/MockHomeConnector.sol b/test/mock/MockHomeConnector.sol
@@ -43,8 +43,8 @@ contract MockHomeConnector is Test {
     }
 
 
-    function updateMember(uint64 poolId, bytes16 trancheId, address user, uint256 amount) public {
-        bytes memory _message = ConnectorMessages.formatUpdateMember(poolId, trancheId, user, amount);
+    function updateMember(uint64 poolId, bytes16 trancheId, address user, uint64 validUntil) public {
+        bytes memory _message = ConnectorMessages.formatUpdateMember(poolId, trancheId, user, validUntil);
         router.handle(_message);
     }