audit: fix silent metatx merge in comp timelock

.forge-snapshots/ProposeSigComp.snap

@@ -1 +1 @@
-148841
+148980

.forge-snapshots/VoteSigComp.snap

@@ -1 +1 @@
-50322
+50452

.forge-snapshots/VoteSigCompMetadata.snap

@@ -1 +1 @@
-51865
+51995

.forge-snapshots/VoteTxComp.snap

@@ -1 +1 @@
-43892
+44022

.forge-snapshots/VoteTxCompMetadata.snap

@@ -1 +1 @@
-45476
+45606

src/execution-strategies/CompTimelockCompatibleExecutionStrategy.sol

@@ -17,6 +17,8 @@ contract CompTimelockCompatibleExecutionStrategy is SimpleQuorumExecutionStrateg
     error ProposalNotQueued();
     /// @notice Thrown if the proposal execution payload hash is already queued.
     error DuplicateExecutionPayloadHash();
+    /// @notice Thrown if the same MetaTransaction appears twice in the same payload (salt is not taken into account).
+    error DuplicateMetaTransaction();
     /// @notice Thrown if veto caller is not the veto guardian.
     error OnlyVetoGuardian();
     /// @notice Thrown if the transaction is invalid.
@@ -103,11 +105,26 @@ contract CompTimelockCompatibleExecutionStrategy is SimpleQuorumExecutionStrateg
         proposalExecutionTime[proposal.executionPayloadHash] = executionTime;
 
         MetaTransaction[] memory transactions = abi.decode(payload, (MetaTransaction[]));
+
+        // An array to check that there are no duplicates in the list of transactions.
+        // We must do this because the Compound Timelock will silently "merge" two duplicate transactions.
+        // This could be problematic for off-chain indexers and UI tools.
+        bytes32[] memory txHashes = new bytes32[](transactions.length);
+
         for (uint256 i = 0; i < transactions.length; i++) {
             // Comp Timelock does not support delegate calls.
             if (transactions[i].operation == Enum.Operation.DelegateCall) {
                 revert InvalidTransaction();
             }
+
+            // Check there are not duplicates.
+            // Do not include `executionTime` because it's a constant.
+            bytes32 txHash = keccak256(abi.encode(transactions[i].to, transactions[i].value, "", transactions[i].data));
+            for (uint256 j = 0; j < i; j++) {
+                if (txHashes[j] == txHash) revert DuplicateMetaTransaction();
+            }
+            txHashes[i] = txHash;
+
             timelock.queueTransaction(
                 transactions[i].to,
                 transactions[i].value,

test/CompTimelockExecutionStrategy.t.sol

@@ -28,6 +28,7 @@ abstract contract CompTimelockExecutionStrategyTest is SpaceTest {
     error TimelockDelayNotMet();
     error ProposalNotQueued();
     error DuplicateExecutionPayloadHash();
+    error DuplicateMetaTransaction();
     error OnlyVetoGuardian();
     error InvalidTransaction();
     event CompTimelockCompatibleExecutionStrategySetUp(
@@ -97,6 +98,24 @@ abstract contract CompTimelockExecutionStrategyTest is SpaceTest {
         space.execute(proposalId, abi.encode(transactions));
     }
 
+    function testQueueingDuplicateMetaTransaction() external {
+        MetaTransaction[] memory transactions = new MetaTransaction[](2);
+        transactions[0] = MetaTransaction(recipient, 1, "", Enum.Operation.Call, 0);
+        // Salt differs but content does not
+        transactions[1] = MetaTransaction(recipient, 1, "", Enum.Operation.Call, 1);
+        uint256 proposalId = _createProposal(
+            author,
+            proposalMetadataURI,
+            Strategy(address(timelockExecutionStrategy), abi.encode(transactions)),
+            new bytes(0)
+        );
+        _vote(author, proposalId, Choice.For, userVotingStrategies, voteMetadataURI);
+        vm.roll(block.number + space.maxVotingDuration());
+
+        vm.expectRevert(DuplicateMetaTransaction.selector);
+        space.execute(proposalId, abi.encode(transactions));
+    }
+
     function testQueueingRejectedProposal() external {
         MetaTransaction[] memory transactions = new MetaTransaction[](1);
         transactions[0] = MetaTransaction(recipient, 1, "", Enum.Operation.Call, 0);