feat: make sure ID is the same cross network & fix down func

consensus-shipyard · Oct 28, 2024 · 0d896a7 · 0d896a7
1 parent 51d3ab5
commit 0d896a7
Show file tree

Hide file tree

Showing 9 changed files with 69 additions and 40 deletions.
diff --git a/contracts/contracts/lib/CrossMsgHelper.sol b/contracts/contracts/lib/CrossMsgHelper.sol
@@ -67,7 +67,7 @@ library CrossMsgHelper {
         OutcomeType outcome,
         bytes memory ret
     ) public pure returns (IpcEnvelope memory) {
-        ResultMsg memory message = ResultMsg({id: toHash(crossMsg), outcome: outcome, ret: ret});
+        ResultMsg memory message = ResultMsg({id: toDeterministicHash(crossMsg), outcome: outcome, ret: ret});
 
         uint256 value = crossMsg.value;
         if (outcome == OutcomeType.Ok) {
@@ -135,6 +135,21 @@ library CrossMsgHelper {
         return keccak256(abi.encode(crossMsg));
     }
 
+    /// @notice Returns a deterministic hash for the given cross message. The hash remains the same accross different networks
+    /// because it doesn't include the network-specific nonce.
+    function toDeterministicHash(IpcEnvelope memory crossMsg) internal pure returns (bytes32) {
+        return keccak256(
+            abi.encode(
+                crossMsg.kind,
+                crossMsg.to,
+                crossMsg.from,
+                crossMsg.value,
+                crossMsg.message
+            )
+        );
+    }
+
+
     function toHash(IpcEnvelope[] memory crossMsgs) public pure returns (bytes32) {
         return keccak256(abi.encode(crossMsgs));
     }

diff --git a/contracts/contracts/lib/LibGateway.sol b/contracts/contracts/lib/LibGateway.sol
@@ -252,7 +252,7 @@ library LibGateway {
         subnet.topDownNonce = topDownNonce + 1;
         subnet.circSupply += crossMessage.value;
 
-        emit NewTopDownMessage({subnet: subnet.id.getAddress(), message: crossMessage, id: crossMessage.toHash()});
+        emit NewTopDownMessage({subnet: subnet.id.getAddress(), message: crossMessage, id: crossMessage.toDeterministicHash()});
     }
 
     /// @notice Commits a new cross-net message to a message batch for execution
@@ -272,7 +272,7 @@ library LibGateway {
             batch.blockHeight = epoch;
             // we need to use push here to initialize the array.
             batch.msgs.push(crossMessage);
-            emit QueuedBottomUpMessage({id: crossMessage.toHash()});
+            emit QueuedBottomUpMessage({id: crossMessage.toDeterministicHash()});
             return;
         }
 
@@ -310,7 +310,7 @@ library LibGateway {
             batch.msgs.push(crossMessage);
         }
 
-        emit QueuedBottomUpMessage({id: crossMessage.toHash()});
+        emit QueuedBottomUpMessage({id: crossMessage.toDeterministicHash()});
     }
 
     /// @notice returns the subnet created by a validator
@@ -454,7 +454,7 @@ library LibGateway {
             s.postboxKeys.add(cid);
             s.postbox[cid] = crossMsg;
 
-            emit MessageStoredInPostbox({id: cid});
+            emit MessageStoredInPostbox({id: crossMsg.toDeterministicHash()});
             return;
         }
 
@@ -525,7 +525,8 @@ library LibGateway {
         // because we're the LCA, commit a top-down message.
         if (applyType == IPCMsgType.TopDown || isLCA) {
             ++s.appliedTopDownNonce;
-            (, Subnet storage subnet) = getSubnet(to.down(s.networkName));
+            (, SubnetID memory subnetId) = to.down(s.networkName);
+            (, Subnet storage subnet) = getSubnet(subnetId);
             LibGateway.commitTopDownMsg(subnet, crossMessage);
             return (shouldBurn = false);
         }
@@ -581,7 +582,12 @@ library LibGateway {
         // If the directionality is top-down, or if we're inverting the direction
         // else we need to check if the common parent exists.
         if (applyType == IPCMsgType.TopDown || isLCA) {
-            (bool foundSubnet,) = LibGateway.getSubnet(toSubnetId.down(s.networkName));
+            (bool foundChildSubnetId, SubnetID memory childSubnetId) = toSubnetId.down(s.networkName);
+            if (!foundChildSubnetId) {
+                return CrossMessageValidationOutcome.InvalidDstSubnet;
+            }
+
+            (bool foundSubnet,) = LibGateway.getSubnet(childSubnetId);
             if (!foundSubnet) {
                 return CrossMessageValidationOutcome.InvalidDstSubnet;
             }
@@ -630,6 +636,7 @@ library LibGateway {
 
         // Cache value before deletion to avoid re-entrancy
         uint256 v = crossMsg.value;
+        bytes32 deterministicId = crossMsg.toDeterministicHash();
 
         // Remove the message to prevent re-entrancy and clean up state
         delete s.postbox[msgCid];
@@ -638,7 +645,7 @@ library LibGateway {
         // Execute side effects
         LibGateway.crossMsgSideEffects({v: v, shouldBurn: shouldBurn});
 
-        emit MessagePropagatedFromPostbox({id: msgCid});
+        emit MessagePropagatedFromPostbox({id: deterministicId});
     }
 
 }
diff --git a/contracts/contracts/lib/SubnetIDHelper.sol b/contracts/contracts/lib/SubnetIDHelper.sol
@@ -129,12 +129,12 @@ library SubnetIDHelper {
     /// subnet2 needs to be a prefix of the subnet1.
     /// If subnet1 is /a/b/c/d and subnet2 is /a/b, then the returned ID should be /a/b/c.
     /// @dev Returns an empty SubnetID if subnet2 is not a prefix of subnet1 or if the roots are different.
-    function down(SubnetID calldata subnet1, SubnetID calldata subnet2) public pure returns (SubnetID memory) {
+    function down(SubnetID calldata subnet1, SubnetID calldata subnet2) public pure returns (bool, SubnetID memory) {
         if (subnet1.root != subnet2.root) {
-            return SubnetID({root: 0, route: new address[](0)});
+            return (false, SubnetID({root: 0, route: new address[](0)}));
         }
         if (subnet1.route.length <= subnet2.route.length) {
-            return SubnetID({root: 0, route: new address[](0)});
+            return (false, SubnetID({root: 0, route: new address[](0)}));
         }
 
         uint256 i;
@@ -156,7 +156,7 @@ library SubnetIDHelper {
             }
         }
 
-        return SubnetID({root: subnet1.root, route: route});
+        return (true, SubnetID({root: subnet1.root, route: route}));
     }
 
     function isEmpty(SubnetID calldata subnetId) public pure returns (bool) {

diff --git a/contracts/test/helpers/TestUtils.sol b/contracts/test/helpers/TestUtils.sol
@@ -228,7 +228,6 @@ contract MockIpcContractPayable is IIpcHandler {
 }
 
 contract MockFallbackContract {
-    // receive() external payable {}
     fallback() external payable {}
 }
 

diff --git a/contracts/test/integration/GatewayDiamondToken.t.sol b/contracts/test/integration/GatewayDiamondToken.t.sol
@@ -110,7 +110,7 @@ contract GatewayDiamondTokenTest is Test, IntegrationTestBase {
             10
         );
         vm.expectEmit(true, true, true, true, address(gatewayDiamond));
-        emit LibGateway.NewTopDownMessage(address(saDiamond), expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(address(saDiamond), expected, expected.toDeterministicHash());
         gatewayDiamond.manager().fundWithToken(subnet.id, FvmAddressHelper.from(caller), 10);
 
         // Assert post-conditions.

diff --git a/contracts/test/integration/L2PlusSubnet.t.sol b/contracts/test/integration/L2PlusSubnet.t.sol
@@ -497,7 +497,7 @@ contract L2PlusSubnetTest is Test, IntegrationTestBase {
         emit LibGateway.NewTopDownMessage({
             subnet: params.subnet.subnetActorAddr,
             message: crossMessage,
-            id: crossMessage.toHash()
+            id: crossMessage.toDeterministicHash()
         });
 
         crossMessage.nonce = 0;
@@ -683,7 +683,7 @@ contract L2PlusSubnetTest is Test, IntegrationTestBase {
         emit LibGateway.NewTopDownMessage({
             subnet: expectedSubnetAddr,
             message: expectedMessage,
-            id: expectedMessage.toHash()
+            id: expectedMessage.toDeterministicHash()
         });
         XnetMessagingFacet xnetMessenger = gw.xnetMessenger();
         xnetMessenger.applyCrossMessages(msgs);
@@ -807,7 +807,7 @@ contract L2PlusSubnetTest is Test, IntegrationTestBase {
         emit LibGateway.NewTopDownMessage({
             subnet: expectedSubnetAddr,
             message: expectedMessage,
-            id: expectedMessage.toHash()
+            id: expectedMessage.toDeterministicHash()
         });
         checkpointer.submitCheckpoint(checkpoint, parentValidators, parentSignatures);
         vm.stopPrank();

diff --git a/contracts/test/integration/MultiSubnet.t.sol b/contracts/test/integration/MultiSubnet.t.sol
@@ -155,7 +155,7 @@ contract MultiSubnetTest is Test, IntegrationTestBase {
 
         vm.prank(caller);
         vm.expectEmit(true, true, true, true, rootSubnet.gatewayAddr);
-        emit LibGateway.NewTopDownMessage(nativeSubnet.subnetActorAddr, expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(nativeSubnet.subnetActorAddr, expected, expected.toDeterministicHash());
         rootSubnet.gateway.manager().fund{value: amount}(nativeSubnet.id, FvmAddressHelper.from(address(recipient)));
 
         IpcEnvelope[] memory msgs = new IpcEnvelope[](1);
@@ -284,7 +284,7 @@ contract MultiSubnetTest is Test, IntegrationTestBase {
 
         vm.prank(caller);
         vm.expectEmit(true, true, true, true, rootSubnet.gatewayAddr);
-        emit LibGateway.NewTopDownMessage(nativeSubnet.subnetActorAddr, expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(nativeSubnet.subnetActorAddr, expected, expected.toDeterministicHash());
         rootSubnet.gateway.manager().fund{value: amount}(nativeSubnet.id, FvmAddressHelper.from(address(recipient)));
 
         IpcEnvelope[] memory msgs = new IpcEnvelope[](1);
@@ -321,7 +321,7 @@ contract MultiSubnetTest is Test, IntegrationTestBase {
 
         vm.prank(caller);
         vm.expectEmit(true, true, true, true, rootSubnet.gatewayAddr);
-        emit LibGateway.NewTopDownMessage(tokenSubnet.subnetActorAddr, expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(tokenSubnet.subnetActorAddr, expected, expected.toDeterministicHash());
         rootSubnet.gateway.manager().fundWithToken(tokenSubnet.id, FvmAddressHelper.from(address(recipient)), amount);
 
         IpcEnvelope[] memory msgs = new IpcEnvelope[](1);
@@ -568,7 +568,7 @@ contract MultiSubnetTest is Test, IntegrationTestBase {
 
         vm.prank(caller);
         vm.expectEmit(true, true, true, true, rootSubnet.gatewayAddr);
-        emit LibGateway.NewTopDownMessage(tokenSubnet.subnetActorAddr, expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(tokenSubnet.subnetActorAddr, expected, expected.toDeterministicHash());
         rootSubnet.gateway.manager().fundWithToken(tokenSubnet.id, FvmAddressHelper.from(address(recipient)), amount);
 
         IpcEnvelope[] memory msgs = new IpcEnvelope[](1);
@@ -1130,7 +1130,7 @@ contract MultiSubnetTest is Test, IntegrationTestBase {
         emit LibGateway.NewTopDownMessage({
             subnet: nativeSubnet.subnetActorAddr,
             message: committedEvent,
-            id: committedEvent.toHash()
+            id: committedEvent.toDeterministicHash()
         });
         rootSubnet.gateway.messenger().sendContractXnetMessage{value: amount}(xnetCallMsg);
 
@@ -1282,7 +1282,7 @@ contract MultiSubnetTest is Test, IntegrationTestBase {
         emit LibGateway.NewTopDownMessage({
             subnet: tokenSubnet.subnetActorAddr,
             message: committedEvent,
-            id: committedEvent.toHash()
+            id: committedEvent.toDeterministicHash()
         });
         rootSubnet.gateway.messenger().sendContractXnetMessage{value: amount}(xnetCallMsg);
 

diff --git a/contracts/test/unit/LibGateway.t.sol b/contracts/test/unit/LibGateway.t.sol
@@ -103,7 +103,7 @@ contract LibGatewayTest is Test {
 
         ResultMsg memory message = ResultMsg({
             outcome: OutcomeType.Ok,
-            id: crossMsg.toHash(),
+            id: crossMsg.toDeterministicHash(),
             ret: abi.encode(EMPTY_BYTES)
         });
         IpcEnvelope memory expected = IpcEnvelope({
@@ -116,7 +116,7 @@ contract LibGatewayTest is Test {
         });
 
         vm.expectEmit(address(t));
-        emit LibGateway.NewTopDownMessage(childSubnetActor, expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(childSubnetActor, expected, expected.toDeterministicHash());
 
         t.applyMsg(childSubnet, crossMsg);
     }
@@ -157,7 +157,7 @@ contract LibGatewayTest is Test {
 
         ResultMsg memory message = ResultMsg({
             outcome: OutcomeType.Ok,
-            id: crossMsg.toHash(),
+            id: crossMsg.toDeterministicHash(),
             ret: abi.encode(EMPTY_BYTES)
         });
         IpcEnvelope memory expected = IpcEnvelope({
@@ -212,7 +212,7 @@ contract LibGatewayTest is Test {
 
         ResultMsg memory message = ResultMsg({
             outcome: OutcomeType.SystemErr,
-            id: crossMsg.toHash(),
+            id: crossMsg.toDeterministicHash(),
             ret: abi.encodeWithSelector(InvalidXnetMessage.selector, InvalidXnetMessageReason.Nonce)
         });
         IpcEnvelope memory expected = IpcEnvelope({
@@ -266,7 +266,7 @@ contract LibGatewayTest is Test {
         });
         crossMsg.nonce = 0;
 
-        ResultMsg memory message = ResultMsg({outcome: OutcomeType.ActorErr, id: crossMsg.toHash(), ret: new bytes(0)});
+        ResultMsg memory message = ResultMsg({outcome: OutcomeType.ActorErr, id: crossMsg.toDeterministicHash(), ret: new bytes(0)});
         IpcEnvelope memory expected = IpcEnvelope({
             kind: IpcMsgKind.Result,
             from: crossMsg.to,
@@ -321,7 +321,7 @@ contract LibGatewayTest is Test {
 
         ResultMsg memory message = ResultMsg({
             outcome: OutcomeType.SystemErr,
-            id: crossMsg.toHash(),
+            id: crossMsg.toDeterministicHash(),
             ret: abi.encodeWithSelector(InvalidXnetMessage.selector, InvalidXnetMessageReason.Nonce)
         });
         IpcEnvelope memory expected = IpcEnvelope({
@@ -334,7 +334,7 @@ contract LibGatewayTest is Test {
         });
 
         vm.expectEmit(address(t));
-        emit LibGateway.NewTopDownMessage(childSubnetActor, expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(childSubnetActor, expected, expected.toDeterministicHash());
 
         t.applyMsg(childSubnet, crossMsg);
     }
@@ -373,7 +373,7 @@ contract LibGatewayTest is Test {
         });
         crossMsg.nonce = 0;
 
-        ResultMsg memory message = ResultMsg({outcome: OutcomeType.ActorErr, id: crossMsg.toHash(), ret: new bytes(0)});
+        ResultMsg memory message = ResultMsg({outcome: OutcomeType.ActorErr, id: crossMsg.toDeterministicHash(), ret: new bytes(0)});
         IpcEnvelope memory expected = IpcEnvelope({
             kind: IpcMsgKind.Result,
             from: crossMsg.to,
@@ -385,7 +385,7 @@ contract LibGatewayTest is Test {
 
         vm.deal(address(t), 1 ether);
         vm.expectEmit(address(t));
-        emit LibGateway.NewTopDownMessage(childSubnetActor, expected, expected.toHash());
+        emit LibGateway.NewTopDownMessage(childSubnetActor, expected, expected.toDeterministicHash());
 
         t.applyMsg(childSubnet, crossMsg);
     }

diff --git a/contracts/test/unit/SubnetIDHelper.t.sol b/contracts/test/unit/SubnetIDHelper.t.sol
@@ -121,8 +121,11 @@ contract SubnetIDHelperTest is Test {
         route3[1] = SUBNET_THREE_ADDRESS;
         SubnetID memory subnetId3 = SubnetID(ROOTNET_CHAINID, route3);
 
-        require(subnetId2.down(subnetId1).equals(subnetId2));
-        require(subnetId3.down(subnetId1).equals(subnetId2));
+        (, SubnetID memory down1) = subnetId2.down(subnetId1);
+        require(down1.equals(subnetId2));
+
+        (, SubnetID memory down2) = subnetId3.down(subnetId1);
+        require(down2.equals(subnetId2));
     }
 
     function test_Down_Works_Subnet2RouteLengthLargerThanSubnet1() public {
@@ -133,7 +136,9 @@ contract SubnetIDHelperTest is Test {
         SubnetID memory subnetId1 = SubnetID(ROOTNET_CHAINID, route1);
         SubnetID memory subnetId2 = SubnetID(ROOTNET_CHAINID, route2);
 
-        assertTrue(subnetId1.down(subnetId2).isEmpty());
+        (bool foundDown, SubnetID memory down) = subnetId1.down(subnetId2);
+        assertFalse(foundDown);
+        assertTrue(down.isEmpty());
     }
 
     function test_Down_Works_Subnet2RouteLenghtEqualToSubnet1() public {
@@ -146,15 +151,18 @@ contract SubnetIDHelperTest is Test {
         SubnetID memory subnetId1 = SubnetID(ROOTNET_CHAINID, route1);
         SubnetID memory subnetId2 = SubnetID(ROOTNET_CHAINID, route2);
 
-        assertTrue(subnetId1.down(subnetId2).isEmpty());
+        (bool foundDown, SubnetID memory down) = subnetId1.down(subnetId2);
+        assertFalse(foundDown);
+        assertTrue(down.isEmpty());
     }
 
     function test_Down_Works_WrongRoot() public {
         SubnetID memory subnetId1 = SubnetID(1, new address[](0));
         SubnetID memory subnetId2 = SubnetID(2, new address[](0));
 
-        assertTrue(subnetId1.down(subnetId2).isEmpty());
-        subnetId1.down(subnetId2);
+        (bool foundDown, SubnetID memory down) = subnetId1.down(subnetId2);
+        assertFalse(foundDown);
+        assertTrue(down.isEmpty());
     }
 
     function test_Down_Works_CommonRootParent() public view {
@@ -168,7 +176,7 @@ contract SubnetIDHelperTest is Test {
         SubnetID memory subnetId1 = SubnetID(ROOTNET_CHAINID, subnetRoute1);
         SubnetID memory subnetId2 = SubnetID(ROOTNET_CHAINID, subnetRoute2);
 
-        SubnetID memory subnetId = subnetId1.down(subnetId2);
+        (, SubnetID memory subnetId) = subnetId1.down(subnetId2);
 
         require(subnetId.toHash() == ROOT_SUBNET_ID.createSubnetId(subnetRoute1[0]).toHash());
     }
@@ -187,7 +195,7 @@ contract SubnetIDHelperTest is Test {
         SubnetID memory subnetId1 = SubnetID(ROOTNET_CHAINID, subnetRoute1);
         SubnetID memory subnetId2 = SubnetID(ROOTNET_CHAINID, subnetRoute2);
 
-        SubnetID memory subnetId = subnetId1.down(subnetId2);
+        (, SubnetID memory subnetId) = subnetId1.down(subnetId2);
 
         address[] memory expectedRoute = new address[](3);
         expectedRoute[0] = address(100);
-Original file line number
+Diff line change
@@ Expand Up / @@ -228,7 +228,6 @@ contract MockIpcContractPayable is IIpcHandler { @@
     }
     contract MockFallbackContract {
-        // receive() external payable {}
         fallback() external payable {}
     }
@@ Expand Down @@