Disallow claiming tokens twice for the same userId

contracts/GuildPin.sol

@@ -42,8 +42,11 @@ contract GuildPin is IGuildPin, Initializable, OwnableUpgradeable, UUPSUpgradeab
     /// @notice The number of tokens minted in the first version of the contract.
     uint256 internal initialTokensMinted;
 
+    mapping(uint256 userId => mapping(GuildAction action => mapping(uint256 guildId => bool claimed)))
+        internal claimerUserIds;
+
     /// @notice Empty space reserved for future updates.
-    uint256[43] private __gap;
+    uint256[42] private __gap;
 
     /// @notice Sets metadata and the associated addresses.
     /// @param name The name of the token.
@@ -75,7 +78,10 @@ contract GuildPin is IGuildPin, Initializable, OwnableUpgradeable, UUPSUpgradeab
         bytes calldata signature
     ) external payable {
         if (signedAt < block.timestamp - SIGNATURE_VALIDITY) revert ExpiredSignature();
-        if (claimedTokens[pinData.receiver][pinData.guildAction][pinData.guildId] != 0) revert AlreadyClaimed();
+        if (
+            claimedTokens[pinData.receiver][pinData.guildAction][pinData.guildId] != 0 ||
+            claimerUserIds[pinData.userId][pinData.guildAction][pinData.guildId]
+        ) revert AlreadyClaimed();
         if (!isValidSignature(pinData, signedAt, cid, signature)) revert IncorrectSignature();
 
         uint256 fee = fee[payToken];
@@ -99,6 +105,7 @@ contract GuildPin is IGuildPin, Initializable, OwnableUpgradeable, UUPSUpgradeab
             uint128(pinData.createdAt)
         );
         cids[tokenId] = cid;
+        claimerUserIds[pinData.userId][pinData.guildAction][pinData.guildId] = true;
 
         // Fee collection
         // When there is no msg.value, try transferring ERC20
@@ -112,12 +119,13 @@ contract GuildPin is IGuildPin, Initializable, OwnableUpgradeable, UUPSUpgradeab
         emit Claimed(pinData.receiver, pinData.guildAction, pinData.guildId);
     }
 
-    function burn(GuildAction guildAction, uint256 guildId) external {
+    function burn(uint256 userId, GuildAction guildAction, uint256 guildId) external {
         uint256 tokenId = claimedTokens[msg.sender][guildAction][guildId];
 
         claimedTokens[msg.sender][guildAction][guildId] = 0;
         delete claimedTokensDetails[tokenId];
         delete cids[tokenId];
+        delete claimerUserIds[userId][guildAction][guildId];
 
         _burn(tokenId);
     }
@@ -153,6 +161,14 @@ contract GuildPin is IGuildPin, Initializable, OwnableUpgradeable, UUPSUpgradeab
         return claimedTokens[account][guildAction][id] != 0;
     }
 
+    function hasTheUserIdClaimed(
+        uint256 userId,
+        GuildAction guildAction,
+        uint256 id
+    ) external view returns (bool claimed) {
+        return claimerUserIds[userId][guildAction][id];
+    }
+
     function tokenURI(uint256 tokenId) public view override returns (string memory) {
         if (!_exists(tokenId)) revert NonExistentToken(tokenId);
 

contracts/interfaces/IGuildPin.sol

@@ -49,6 +49,18 @@ interface IGuildPin {
     /// @return claimed Whether the address has claimed their token.
     function hasClaimed(address account, GuildAction guildAction, uint256 id) external view returns (bool claimed);
 
+    /// @notice Whether a userId has minted a specific pin.
+    /// @dev Used to prevent double mints in the same block.
+    /// @param userId The id of the user on Guild.
+    /// @param guildAction The action the pin was minted for.
+    /// @param id The id of the guild or role the token was minted for.
+    /// @return claimed Whether the userId has claimed the pin for the given action/guildId combination.
+    function hasTheUserIdClaimed(
+        uint256 userId,
+        GuildAction guildAction,
+        uint256 id
+    ) external view returns (bool claimed);
+
     /// @notice The time interval while a signature is valid.
     /// @return validity The time interval in seconds.
     // solhint-disable func-name-mixedcase
@@ -73,9 +85,10 @@ interface IGuildPin {
     ) external payable;
 
     /// @notice Burns a token from the sender.
+    /// @param userId The id of the user on Guild.
     /// @param guildAction The action to which the token belongs to.
     /// @param guildId The id of the guild where the token belongs to.
-    function burn(GuildAction guildAction, uint256 guildId) external;
+    function burn(uint256 userId, GuildAction guildAction, uint256 guildId) external;
 
     /// @notice Updates a minted token's cid.
     /// @dev Only callable by the owner of the token.

docs/contracts/GuildPin.md

@@ -76,6 +76,12 @@ uint256 initialTokensMinted
 
 The number of tokens minted in the first version of the contract.
 
+### claimerUserIds
+
+```solidity
+mapping(uint256 => mapping(enum IGuildPin.GuildAction => mapping(uint256 => bool))) claimerUserIds
+```
+
 ## Functions
 
 ### initialize
@@ -130,6 +136,7 @@ The contract needs to be approved if ERC20 tokens are used.
 
 ```solidity
 function burn(
+    uint256 userId,
     enum IGuildPin.GuildAction guildAction,
     uint256 guildId
 ) external
@@ -141,6 +148,7 @@ Burns a token from the sender.
 
 | Name | Type | Description |
 | :--- | :--- | :---------- |
+| `userId` | uint256 | The id of the user on Guild. |
 | `guildAction` | enum IGuildPin.GuildAction | The action to which the token belongs to. |
 | `guildId` | uint256 | The id of the guild where the token belongs to. |
 
@@ -227,6 +235,33 @@ Returns true if the address has already claimed their token.
 | Name | Type | Description |
 | :--- | :--- | :---------- |
 | `claimed` | bool | Whether the address has claimed their token. |
+### hasTheUserIdClaimed
+
+```solidity
+function hasTheUserIdClaimed(
+    uint256 userId,
+    enum IGuildPin.GuildAction guildAction,
+    uint256 id
+) external returns (bool claimed)
+```
+
+Whether a userId has minted a specific pin.
+
+Used to prevent double mints in the same block.
+
+#### Parameters
+
+| Name | Type | Description |
+| :--- | :--- | :---------- |
+| `userId` | uint256 | The id of the user on Guild. |
+| `guildAction` | enum IGuildPin.GuildAction | The action the pin was minted for. |
+| `id` | uint256 | The id of the guild or role the token was minted for. |
+
+#### Return Values
+
+| Name | Type | Description |
+| :--- | :--- | :---------- |
+| `claimed` | bool | Whether the userId has claimed the pin for the given action/guildId combination. |
 ### tokenURI
 
 ```solidity

docs/contracts/interfaces/IGuildPin.md

@@ -29,6 +29,33 @@ Returns true if the address has already claimed their token.
 | Name | Type | Description |
 | :--- | :--- | :---------- |
 | `claimed` | bool | Whether the address has claimed their token. |
+### hasTheUserIdClaimed
+
+```solidity
+function hasTheUserIdClaimed(
+    uint256 userId,
+    enum IGuildPin.GuildAction guildAction,
+    uint256 id
+) external returns (bool claimed)
+```
+
+Whether a userId has minted a specific pin.
+
+Used to prevent double mints in the same block.
+
+#### Parameters
+
+| Name | Type | Description |
+| :--- | :--- | :---------- |
+| `userId` | uint256 | The id of the user on Guild. |
+| `guildAction` | enum IGuildPin.GuildAction | The action the pin was minted for. |
+| `id` | uint256 | The id of the guild or role the token was minted for. |
+
+#### Return Values
+
+| Name | Type | Description |
+| :--- | :--- | :---------- |
+| `claimed` | bool | Whether the userId has claimed the pin for the given action/guildId combination. |
 ### SIGNATURE_VALIDITY
 
 ```solidity
@@ -83,6 +110,7 @@ The contract needs to be approved if ERC20 tokens are used.
 
 ```solidity
 function burn(
+    uint256 userId,
     enum IGuildPin.GuildAction guildAction,
     uint256 guildId
 ) external
@@ -94,6 +122,7 @@ Burns a token from the sender.
 
 | Name | Type | Description |
 | :--- | :--- | :---------- |
+| `userId` | uint256 | The id of the user on Guild. |
 | `guildAction` | enum IGuildPin.GuildAction | The action to which the token belongs to. |
 | `guildId` | uint256 | The id of the guild where the token belongs to. |
 

test/GuildPin.spec.ts

@@ -254,6 +254,43 @@ describe("GuildPin", () => {
         ).to.be.revertedWithCustomError(pin, "AlreadyClaimed");
       });
 
+      it("should revert if the userId has already claimed", async () => {
+        await pin.claim(ethers.ZeroAddress, samplePinData, timestamp, cids[0], sampleSignature, {
+          value: fee
+        });
+
+        const signature = await createSignature(
+          signer,
+          randomWallet.address,
+          GuildAction.JOINED_GUILD,
+          sampleUserId,
+          sampleGuildId,
+          sampleGuildName,
+          sampleJoinDate,
+          timestamp,
+          cids[0],
+          chainId,
+          await pin.getAddress()
+        );
+        const tx = pin.claim(
+          ethers.ZeroAddress,
+          {
+            receiver: randomWallet.address,
+            guildAction: GuildAction.JOINED_GUILD,
+            userId: sampleUserId,
+            guildId: sampleGuildId,
+            guildName: sampleGuildName,
+            createdAt: sampleJoinDate
+          },
+          timestamp,
+          cids[0],
+          signature,
+          { value: fee }
+        );
+
+        await expect(tx).to.be.revertedWithCustomError(pin, "AlreadyClaimed");
+      });
+
       it("should revert if the signature is incorrect", async () => {
         await expect(
           pin.claim(ethers.ZeroAddress, samplePinData, timestamp, cids[0], ethers.ZeroHash, {
@@ -307,10 +344,31 @@ describe("GuildPin", () => {
       });
 
       it("should set the address's claim status", async () => {
+        const hasClaimed0 = await pin.hasClaimed(wallet0.address, GuildAction.JOINED_GUILD, sampleGuildId);
         await pin.claim(ethers.ZeroAddress, samplePinData, timestamp, cids[0], sampleSignature, {
           value: fee
         });
-        expect(await pin.hasClaimed(wallet0.address, GuildAction.JOINED_GUILD, sampleGuildId)).to.eq(true);
+        const hasClaimed1 = await pin.hasClaimed(wallet0.address, GuildAction.JOINED_GUILD, sampleGuildId);
+        expect(hasClaimed0).to.eq(false);
+        expect(hasClaimed1).to.eq(true);
+      });
+
+      it("should set the userId's claim status", async () => {
+        const hasTheUserIdClaimed0 = await pin.hasTheUserIdClaimed(
+          sampleUserId,
+          GuildAction.JOINED_GUILD,
+          sampleGuildId
+        );
+        await pin.claim(ethers.ZeroAddress, samplePinData, timestamp, cids[0], sampleSignature, {
+          value: fee
+        });
+        const hasTheUserIdClaimed1 = await pin.hasTheUserIdClaimed(
+          sampleUserId,
+          GuildAction.JOINED_GUILD,
+          sampleGuildId
+        );
+        expect(hasTheUserIdClaimed0).to.eq(false);
+        expect(hasTheUserIdClaimed1).to.eq(true);
       });
 
       it("should be able to mint tokens for the same reason to different addresses", async () => {
@@ -324,7 +382,7 @@ describe("GuildPin", () => {
           signer,
           randomWallet.address,
           GuildAction.JOINED_GUILD,
-          sampleUserId,
+          sampleUserId + 1,
           sampleGuildId,
           sampleGuildName,
           sampleJoinDate,
@@ -338,7 +396,7 @@ describe("GuildPin", () => {
           {
             receiver: randomWallet.address,
             guildAction: GuildAction.JOINED_GUILD,
-            userId: sampleUserId,
+            userId: sampleUserId + 1,
             guildId: sampleGuildId,
             guildName: sampleGuildName,
             createdAt: sampleJoinDate
@@ -495,21 +553,41 @@ describe("GuildPin", () => {
 
       it("should reset hasClaimed to false", async () => {
         const hasClaimed0 = await pin.hasClaimed(wallet0.address, GuildAction.JOINED_GUILD, sampleGuildId);
-        await pin.burn(GuildAction.JOINED_GUILD, sampleGuildId);
+        await pin.burn(sampleUserId, GuildAction.JOINED_GUILD, sampleGuildId);
         const hasClaimed1 = await pin.hasClaimed(wallet0.address, GuildAction.JOINED_GUILD, sampleGuildId);
         expect(hasClaimed0).to.eq(true);
         expect(hasClaimed1).to.eq(false);
       });
 
+      it("should reset hasTheUserIdClaimed to false", async () => {
+        const hasTheUserIdClaimed0 = await pin.hasTheUserIdClaimed(
+          sampleUserId,
+          GuildAction.JOINED_GUILD,
+          sampleGuildId
+        );
+        await pin.burn(sampleUserId, GuildAction.JOINED_GUILD, sampleGuildId);
+        const hasTheUserIdClaimed1 = await pin.hasTheUserIdClaimed(
+          sampleUserId,
+          GuildAction.JOINED_GUILD,
+          sampleGuildId
+        );
+        expect(hasTheUserIdClaimed0).to.eq(true);
+        expect(hasTheUserIdClaimed1).to.eq(false);
+      });
+
       it("should decrement the total supply", async () => {
         const totalSupply0 = await pin.totalSupply();
-        await pin.burn(GuildAction.JOINED_GUILD, sampleGuildId);
+        await pin.burn(sampleUserId, GuildAction.JOINED_GUILD, sampleGuildId);
         const totalSupply1 = await pin.totalSupply();
         expect(totalSupply1).to.eq(totalSupply0 - 1n);
       });
 
       it("should burn the token", async () => {
-        await expect(pin.burn(GuildAction.JOINED_GUILD, sampleGuildId)).to.changeTokenBalance(pin, wallet0, -1);
+        await expect(pin.burn(sampleUserId, GuildAction.JOINED_GUILD, sampleGuildId)).to.changeTokenBalance(
+          pin,
+          wallet0,
+          -1
+        );
       });
     });
   });