Fix token gating

contracts/domain/BosonConstants.sol

@@ -105,7 +105,7 @@ string constant GROUP_HAS_CONDITION = "Offer belongs to a group with a condition
 string constant MAX_COMMITS_ADDRESS_REACHED = "Max commits per address reached";
 string constant MAX_COMMITS_TOKEN_REACHED = "Max commits per token id reached";
 string constant TOKEN_ID_NOT_IN_CONDITION_RANGE = "Token id not in condition range";
-string constant INVALID_TOKEN_ID = "ERC1155 requires non-zero tokenId; ERC721 and ERC20 require zero tokenId.";
+string constant INVALID_TOKEN_ID = "ERC721 and ERC20 require zero tokenId";
 
 // Revert Reasons: Exchange related
 string constant NO_SUCH_EXCHANGE = "No such exchange";

contracts/protocol/bases/GroupBase.sol

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.18;
+
 import "./../../domain/BosonConstants.sol";
 import { IBosonGroupEvents } from "../../interfaces/events/IBosonGroupEvents.sol";
 import { ProtocolBase } from "./../bases/ProtocolBase.sol";
@@ -120,7 +121,8 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents {
      * @return valid - validity of condition
      *
      */
-    function validateCondition(Condition memory _condition) internal pure returns (bool valid) {
+    function validateCondition(Condition memory _condition) internal pure returns (bool) {
+        bool valid = true;
         if (_condition.method == EvaluationMethod.None) {
             valid = (_condition.tokenAddress == address(0) &&
                 _condition.tokenId == 0 &&
@@ -129,40 +131,25 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents {
                 _condition.length == 0);
         } else {
             if (_condition.tokenId != 0) {
-                if (_condition.length == 0) {
-                    return false;
-                }
-
-                // Create local copy so we can use assembly to check for overflow
-                uint256 tokenId = _condition.tokenId;
-                uint256 length = _condition.length;
-                uint256 sum;
-                assembly {
-                    // Adding and checking for overflow in Assembly
-                    let tmp := add(tokenId, sub(length, 1))
-                    if iszero(lt(tmp, tokenId)) {
-                        sum := tmp
-                    }
-                }
-
-                if (sum == 0) {
-                    return false;
-                }
+                valid = _condition.length != 0;
+                valid = valid && type(uint256).max - _condition.length >= _condition.tokenId;
             }
 
             if (_condition.method == EvaluationMethod.Threshold) {
-                valid = (_condition.tokenAddress != address(0) &&
-                    _condition.maxCommits > 0 &&
-                    _condition.threshold > 0);
+                valid =
+                    valid &&
+                    (_condition.tokenAddress != address(0) && _condition.maxCommits > 0 && _condition.threshold > 0);
 
                 if (_condition.tokenType != TokenType.MultiToken) {
                     // NonFungibleToken and FungibleToken should not have length and tokenId
                     valid = valid && _condition.length == 0 && _condition.tokenId == 0;
                 }
             } else {
-                valid = (_condition.tokenAddress != address(0) &&
-                    _condition.maxCommits > 0 &&
-                    _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken
+                valid =
+                    valid &&
+                    (_condition.tokenAddress != address(0) &&
+                        _condition.maxCommits > 0 &&
+                        _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken
 
                 // SpecificToken with NonFungibleToken should not have threshold
                 if (_condition.tokenType == TokenType.NonFungibleToken) {
@@ -173,6 +160,8 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents {
                 }
             }
         }
+
+        return valid;
     }
 
     /**

contracts/protocol/facets/ExchangeHandlerFacet.sol

@@ -145,8 +145,8 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
 
         require(condition.method != EvaluationMethod.None, GROUP_HAS_NO_CONDITION);
 
-        if (condition.length > 1) {
-            // If condition has length > 1, check that the token id is in range
+        if (condition.length >= 1) {
+            // If condition has length >= 1, check that the token id is in range
             require(
                 _tokenId >= condition.tokenId && _tokenId < condition.tokenId + condition.length,
                 TOKEN_ID_NOT_IN_CONDITION_RANGE
@@ -208,15 +208,20 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
             // Get the condition
             Condition storage condition = fetchCondition(groupId);
 
+            uint256 tokenId = 0;
+
             // If is a per-token condition or a per-address condition gated with a 1155 token, make sure the condition is not a range since caller (Boson Voucher) cannot specify the token id
             if (
                 condition.method == EvaluationMethod.SpecificToken ||
                 (condition.method == EvaluationMethod.Threshold && condition.tokenType == TokenType.MultiToken)
             ) {
                 require(condition.length == 1, CANNOT_COMMIT);
+
+                // Uses token id from the condition
+                tokenId = condition.tokenId;
             }
 
-            bool allow = authorizeCommit(_buyer, condition, groupId, 0);
+            bool allow = authorizeCommit(_buyer, condition, groupId, tokenId);
             require(allow, CANNOT_COMMIT);
         }
 
@@ -987,7 +992,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
 
             require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED);
 
-            allow = holdsSpecificToken(_buyer, _condition, _condition.length == 1 ? _condition.tokenId : _tokenId);
+            allow = holdsSpecificToken(_buyer, _condition, _tokenId);
 
             if (allow) {
                 // Increment number of commits to the group for this token id if they are allowed to commit
@@ -999,7 +1004,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
 
             require(commitCount < _condition.maxCommits, MAX_COMMITS_ADDRESS_REACHED);
 
-            allow = holdsThreshold(_buyer, _condition, _condition.length == 1 ? _condition.tokenId : _tokenId);
+            allow = holdsThreshold(_buyer, _condition, _tokenId);
 
             if (allow) {
                 // Increment number of commits to the group for this address if they are allowed to commit

scripts/config/revert-reasons.js

@@ -62,7 +62,7 @@ exports.RevertReasons = {
   GROUP_HAS_NO_CONDITION: "Offer belongs to a group without a condition. Use commitToOffer instead",
   GROUP_HAS_CONDITION: "Offer belongs to a group with a condition. Use commitToConditionalOffer instead",
   TOKEN_ID_NOT_IN_CONDITION_RANGE: "Token id not in condition range",
-  INVALID_TOKEN_ID: "ERC1155 requires non-zero tokenId; ERC721 and ERC20 require zero tokenId.",
+  INVALID_TOKEN_ID: "ERC721 and ERC20 require zero tokenId",
 
   // Account-related
   MUST_BE_ACTIVE: "Account must be active",

test/example/SnapshotGateTest.js

@@ -896,7 +896,7 @@ describe("SnapshotGate", function () {
           // Check that holder cannot commit directly to the offer on the protocol itself
           await expect(
             exchangeHandler.connect(holder).commitToConditionalOffer(holder.address, offerId, entry.tokenId)
-          ).to.revertedWith("Caller cannot commit");
+          ).to.revertedWith(RevertReasons.CANNOT_COMMIT);
         });
       });
     });

test/protocol/ExchangeHandlerTest.js

@@ -915,7 +915,7 @@ describe("IBosonExchangeHandler", function () {
         ).to.emit(exchangeHandler, "BuyerCommitted");
       });
 
-      context("Offer is part a group", async function () {
+      context("Offer is part of a group", async function () {
         let groupId;
         let offerIds;
 
@@ -970,7 +970,7 @@ describe("IBosonExchangeHandler", function () {
           );
         });
 
-        it("Offer is part of a group that enfoces per-address conditions and utilizes ERC1155 tokens with range length == 1", async function () {
+        it("Offer is part of a group that enforces per-address conditions and utilizes ERC1155 tokens with range length == 1", async function () {
           condition = mockCondition({
             tokenAddress: foreign1155.address,
             threshold: "2",
@@ -1021,17 +1021,18 @@ describe("IBosonExchangeHandler", function () {
           );
         });
 
-        it("Offer is part of a group that has a per-wallet ERC721 condition", async function () {
+        it("Offer is part of a group that enforces per-wallet conditions and range length == 1", async function () {
           // Required constructor params for Group
           groupId = "1";
           offerIds = [offerId];
 
           condition = mockCondition({
             tokenAddress: foreign721.address,
-            threshold: "1",
+            threshold: "0",
             maxCommits: "3",
             tokenType: TokenType.NonFungibleToken,
-            method: EvaluationMethod.Threshold,
+            method: EvaluationMethod.SpecificToken,
+            length: "1",
           });
 
           expect(condition.isValid()).to.be.true;
@@ -1163,7 +1164,7 @@ describe("IBosonExchangeHandler", function () {
           ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT);
         });
 
-        it("Offer is part of a group that has a per-address condition and token is ERC1155", async function () {
+        it("Offer is part of a group that has a per-address condition and token is ERC1155 but accets any token", async function () {
           // Required constructor params for Group
           groupId = "1";
           offerIds = [offerId];
@@ -1244,7 +1245,7 @@ describe("IBosonExchangeHandler", function () {
           ).to.revertedWith(RevertReasons.CANNOT_COMMIT);
         });
 
-        it("Offer is part a group with a per-token condition with length > 1", async function () {
+        it("Offer is part of a group with a per-token condition with length > 1", async function () {
           // Required constructor params for Group
           groupId = "1";
           offerIds = [offerId];
@@ -1255,7 +1256,7 @@ describe("IBosonExchangeHandler", function () {
             maxCommits: "3",
             tokenType: TokenType.NonFungibleToken, // ERC721
             tokenId: "0",
-            method: EvaluationMethod.SpecificToken, // per-wallet
+            method: EvaluationMethod.SpecificToken, // per-token
             length: "0",
           });
 
@@ -1565,11 +1566,7 @@ describe("IBosonExchangeHandler", function () {
           condition.tokenId = "0";
           condition.length = "0";
 
-          // remove offer from old group
-          await groupHandler.connect(assistant).removeOffersFromGroup(groupId, offerIds);
-
-          // create new group with new condition
-          await groupHandler.connect(assistant).createGroup(group, condition);
+          await groupHandler.connect(assistant).setGroupCondition(group.id, condition);
 
           // mint any token for buyer
           tokenId = "123";
@@ -1676,11 +1673,7 @@ describe("IBosonExchangeHandler", function () {
           condition.tokenId = "0";
           condition.length = "0";
 
-          // remove offer from old group
-          await groupHandler.connect(assistant).removeOffersFromGroup(groupId, offerIds);
-
-          // create new group with new condition
-          await groupHandler.connect(assistant).createGroup(group, condition);
+          await groupHandler.connect(assistant).setGroupCondition(group.id, condition);
 
           // mint any token for buyer
           tokenId = "123";

test/protocol/GroupHandlerTest.js

@@ -470,7 +470,7 @@ describe("IBosonGroupHandler", function () {
 
           it("Condition 'Threshold' with MultiToken has tokenId + length - 1 > max uint256", async function () {
             condition.tokenType = TokenType.MultiToken;
-            condition.tokenId = 2;
+            condition.tokenId = 1;
             condition.length = ethers.constants.MaxUint256;
 
             // Attempt to create the group, expecting revert