Merge branch 'main' into audit_v2_3_0_parenthesis

README.md

@@ -43,7 +43,7 @@ Done or in progress are:
 
 We welcome contributions! Until now, Boson Protocol has been largely worked on by a small dedicated team. However, the ultimate goal is for all of the Boson Protocol repositories to be fully owned by the community and contributors. Issues, pull requests, suggestions, and any sort of involvement are more than welcome.
 
-If you have noticed a bug, please report it via our [Bug Bounty program](https://immunefi.com/bounty/bosonprotocol/).
+If you have noticed a bug, please report it via our [Bug Bounty program](https://github.com/bosonprotocol/community/blob/main/BugBountyProgram.md).
 
 Questions and feedback are always welcome, we will use them to improve our offering.
 

contracts/interfaces/handlers/IBosonPauseHandler.sol

@@ -19,7 +19,6 @@ interface IBosonPauseHandler is IBosonPauseEvents {
      *
      * Reverts if:
      * - Caller does not have PAUSER role
-     * - A region is specified more than once
      *
      * @param _regions - an array of regions to pause. See: {BosonTypes.PausableRegion}
      */
@@ -33,7 +32,6 @@ interface IBosonPauseHandler is IBosonPauseEvents {
      * Reverts if:
      * - Caller does not have PAUSER role
      * - Protocol is not currently paused
-     * - A region is specified more than once
      *
      * @param _regions - an array of regions to pause. See: {BosonTypes.PausableRegion}
      */

contracts/protocol/clients/voucher/BosonVoucher.sol

@@ -73,7 +73,12 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable
         address _newOwner,
         VoucherInitValues calldata voucherInitValues
     ) public initializer {
-        string memory sellerId = string.concat(Strings.toString(_sellerId), "_", Strings.toString(_collectionIndex));
+        string memory sellerId = string.concat(
+            "S",
+            Strings.toString(_sellerId),
+            "_C",
+            Strings.toString(_collectionIndex)
+        );
         string memory voucherName = string.concat(VOUCHER_NAME, " ", sellerId);
         string memory voucherSymbol = string.concat(VOUCHER_SYMBOL, "_", sellerId);
 

contracts/protocol/facets/ConfigHandlerFacet.sol

@@ -43,8 +43,8 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
         setBuyerEscalationDepositPercentage(_fees.buyerEscalationDepositPercentage);
         setMaxTotalOfferFeePercentage(_limits.maxTotalOfferFeePercentage);
         setMaxRoyaltyPecentage(_limits.maxRoyaltyPecentage);
-        setMinResolutionPeriod(_limits.minResolutionPeriod);
         setMaxResolutionPeriod(_limits.maxResolutionPeriod);
+        setMinResolutionPeriod(_limits.minResolutionPeriod);
         setMinDisputePeriod(_limits.minDisputePeriod);
 
         // Initialize protocol counters
@@ -428,7 +428,13 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
         // Make sure _maxResolutionPeriod is greater than 0
         checkNonZero(_minResolutionPeriod);
 
-        protocolLimits().minResolutionPeriod = _minResolutionPeriod;
+        // cache protocol limits
+        ProtocolLib.ProtocolLimits storage limits = protocolLimits();
+
+        // Make sure _minResolutionPeriod is less than _maxResolutionPeriod
+        require(_minResolutionPeriod <= limits.maxResolutionPeriod, INVALID_RESOLUTION_PERIOD);
+
+        limits.minResolutionPeriod = _minResolutionPeriod;
         emit MinResolutionPeriodChanged(_minResolutionPeriod, msgSender());
     }
 
@@ -456,7 +462,13 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
         // Make sure _maxResolutionPeriod is greater than 0
         checkNonZero(_maxResolutionPeriod);
 
-        protocolLimits().maxResolutionPeriod = _maxResolutionPeriod;
+        // cache protocol limits
+        ProtocolLib.ProtocolLimits storage limits = protocolLimits();
+
+        // Make sure _maxResolutionPeriod is greater than _minResolutionPeriod
+        require(_maxResolutionPeriod >= limits.minResolutionPeriod, INVALID_RESOLUTION_PERIOD);
+
+        limits.maxResolutionPeriod = _maxResolutionPeriod;
         emit MaxResolutionPeriodChanged(_maxResolutionPeriod, msgSender());
     }
 

contracts/protocol/facets/ExchangeHandlerFacet.sol

@@ -989,28 +989,38 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
         ProtocolLib.ProtocolLookups storage lookups = protocolLookups();
 
         if (_condition.method == EvaluationMethod.SpecificToken) {
+            // Cache conditionalCommitsByTokenId mapping for reference
+            mapping(uint256 => uint256) storage conditionalCommitsByTokenId = lookups.conditionalCommitsByTokenId[
+                _tokenId
+            ];
+
             // How many times has this token id been used to commit to offers in the group?
-            uint256 commitCount = lookups.conditionalCommitsByTokenId[_tokenId][_groupId];
+            uint256 commitCount = conditionalCommitsByTokenId[_groupId];
 
             require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED);
 
             allow = holdsSpecificToken(_buyer, _condition, _tokenId);
 
             if (allow) {
                 // Increment number of commits to the group for this token id if they are allowed to commit
-                lookups.conditionalCommitsByTokenId[_tokenId][_groupId] = ++commitCount;
+                conditionalCommitsByTokenId[_groupId] = ++commitCount;
             }
         } else if (_condition.method == EvaluationMethod.Threshold) {
+            // Cache conditionalCommitsByAddress mapping for reference
+            mapping(uint256 => uint256) storage conditionalCommitsByAddress = lookups.conditionalCommitsByAddress[
+                _buyer
+            ];
+
             // How many times has this address committed to offers in the group?
-            uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][_groupId];
+            uint256 commitCount = conditionalCommitsByAddress[_groupId];
 
             require(commitCount < _condition.maxCommits, MAX_COMMITS_ADDRESS_REACHED);
 
             allow = holdsThreshold(_buyer, _condition, _tokenId);
 
             if (allow) {
                 // Increment number of commits to the group for this address if they are allowed to commit
-                lookups.conditionalCommitsByAddress[_buyer][_groupId] = ++commitCount;
+                conditionalCommitsByAddress[_groupId] = ++commitCount;
             }
         } else {
             allow = true;

contracts/protocol/facets/PauseHandlerFacet.sol

@@ -30,7 +30,6 @@ contract PauseHandlerFacet is ProtocolBase, IBosonPauseHandler {
      *
      * Reverts if:
      * - Caller does not have PAUSER role
-     * - A region is specified more than once
      *
      * @param _regions - an array of regions to pause. See: {BosonTypes.PausableRegion}
      */
@@ -49,7 +48,6 @@ contract PauseHandlerFacet is ProtocolBase, IBosonPauseHandler {
      * Reverts if:
      * - Caller does not have PAUSER role
      * - Protocol is not currently paused
-     * - A region is specified more than once
      */
     function unpause(BosonTypes.PausableRegion[] calldata _regions) external onlyRole(PAUSER) nonReentrant {
         // Cache protocol status for reference
@@ -105,9 +103,6 @@ contract PauseHandlerFacet is ProtocolBase, IBosonPauseHandler {
      *
      * Toggle all regions if none are specified.
      *
-     * Reverts if:
-     * - A region is specified more than once
-     *
      * @param _regions - an array of regions to pause/unpause. See: {BosonTypes.PausableRegion}
      * @param _pause - a boolean indicating whether to pause (true) or unpause (false)
      */

contracts/protocol/facets/ProtocolInitializationHandlerFacet.sol

@@ -48,7 +48,7 @@ contract ProtocolInitializationHandlerFacet is IBosonProtocolInitializationHandl
      * - For upgrade to v2.2.0:
      *   - If versions is set already
      *   - If _initializationData cannot be decoded to uin256
-     *   - If _initializationData is represents value 0
+     *   - If _initializationData is represents value
      *
      * @param _version - version of the protocol
      * @param _addresses - array of facet addresses to call initialize methods
@@ -143,10 +143,11 @@ contract ProtocolInitializationHandlerFacet is IBosonProtocolInitializationHandl
      * Reverts if:
      *  - Current version is not 2.2.1
      *  - There are already twins. This version adds a new mapping for twins which make it incompatible with previous versions.
-     *  - minResolutionPeriond is not present in _initializationData parameter
+     *  - minResolutionPeriod is not present in _initializationData parameter
      *  - length of seller creators does not match the length of seller ids
      *  - if some of seller creators is zero address
      *  - if some of seller ids does not bellong to a seller
+     *  - if minResolutionPeriod is greater than maxResolutionPeriod
      *
      * @param _initializationData - data representing uint256 _minResolutionPeriod, uint256[] memory sellerIds, address[] memory sellerCreators
      */
@@ -161,9 +162,15 @@ contract ProtocolInitializationHandlerFacet is IBosonProtocolInitializationHandl
             (uint256, uint256[], address[])
         );
 
+        // cache protocol limits
+        ProtocolLib.ProtocolLimits storage limits = protocolLimits();
+
+        // make sure _minResolutionPeriod is less than maxResolutionPeriod
+        require(limits.maxResolutionPeriod >= _minResolutionPeriod, INVALID_RESOLUTION_PERIOD);
+
         // Initialize limits.maxPremintedVouchers (configHandlerFacet initializer)
         require(_minResolutionPeriod != 0, VALUE_ZERO_NOT_ALLOWED);
-        protocolLimits().minResolutionPeriod = _minResolutionPeriod;
+        limits.minResolutionPeriod = _minResolutionPeriod;
         emit MinResolutionPeriodChanged(_minResolutionPeriod, msgSender());
 
         // Initialize sellerCreators

contracts/protocol/facets/SellerHandlerFacet.sol

@@ -274,9 +274,9 @@ contract SellerHandlerFacet is SellerBase {
                 IBosonVoucher(lookups.cloneAddress[_sellerId]).transferOwnership(sender); // default voucher contract
                 Collection[] storage sellersAdditionalCollections = lookups.additionalCollections[_sellerId];
                 uint256 collectionCount = sellersAdditionalCollections.length;
-                for (i = 0; i < collectionCount; i++) {
+                for (uint256 j = 0; j < collectionCount; j++) {
                     // Additional collections (if they exist)
-                    IBosonVoucher(sellersAdditionalCollections[i].collectionAddress).transferOwnership(sender);
+                    IBosonVoucher(sellersAdditionalCollections[j].collectionAddress).transferOwnership(sender);
                 }
 
                 // Store new seller id by assistant mapping

test/protocol/ConfigHandlerTest.js

@@ -1,7 +1,6 @@
 const { ethers } = require("hardhat");
 const { getSigners, getContractAt, ZeroAddress, parseUnits } = ethers;
 const { expect } = require("chai");
-
 const Role = require("../../scripts/domain/Role");
 const { getInterfaceIds } = require("../../scripts/config/supported-interfaces.js");
 const { RevertReasons } = require("../../scripts/config/revert-reasons.js");
@@ -718,6 +717,55 @@ describe("IBosonConfigHandler", function () {
         });
       });
 
+      context("👉 setMinResolutionPeriod()", async function () {
+        let minResolutionPeriod;
+        beforeEach(async function () {
+          // set new value
+          minResolutionPeriod = oneWeek;
+        });
+
+        it("should emit a MinResolutionPeriodChanged event", async function () {
+          // Set new resolution period
+          await expect(configHandler.connect(deployer).setMinResolutionPeriod(minResolutionPeriod))
+            .to.emit(configHandler, "MinResolutionPeriodChanged")
+            .withArgs(minResolutionPeriod, await deployer.getAddress());
+        });
+
+        it("should update state", async function () {
+          // Set new resolution period
+          await configHandler.connect(deployer).setMinResolutionPeriod(minResolutionPeriod);
+
+          // Verify that new value is stored
+          expect(await configHandler.connect(rando).getMinResolutionPeriod()).to.equal(minResolutionPeriod);
+        });
+
+        context("💔 Revert Reasons", async function () {
+          it("caller is not the admin", async function () {
+            // Attempt to set new value, expecting revert
+            await expect(configHandler.connect(rando).setMinResolutionPeriod(minResolutionPeriod)).to.revertedWith(
+              RevertReasons.ACCESS_DENIED
+            );
+          });
+
+          it("minResolutionPeriod is zero", async function () {
+            minResolutionPeriod = 0;
+            await expect(configHandler.connect(deployer).setMinResolutionPeriod(minResolutionPeriod)).to.revertedWith(
+              RevertReasons.VALUE_ZERO_NOT_ALLOWED
+            );
+          });
+
+          it("minResolutionPeriod is greater than maxResolutionPeriod", async function () {
+            const maxResolutionPeriod = oneMonth;
+            await configHandler.connect(deployer).setMaxResolutionPeriod(maxResolutionPeriod);
+
+            minResolutionPeriod = maxResolutionPeriod + 1n;
+            await expect(configHandler.connect(deployer).setMinResolutionPeriod(minResolutionPeriod)).to.revertedWith(
+              RevertReasons.INVALID_RESOLUTION_PERIOD
+            );
+          });
+        });
+      });
+
       context("👉 setMaxResolutionPeriod()", async function () {
         let maxResolutionPeriod;
         beforeEach(async function () {
@@ -754,6 +802,16 @@ describe("IBosonConfigHandler", function () {
               RevertReasons.VALUE_ZERO_NOT_ALLOWED
             );
           });
+
+          it("maxResolutionPeriod is less than minResolutionPeriod", async function () {
+            const minResolutionPeriod = oneWeek;
+            await configHandler.connect(deployer).setMinResolutionPeriod(minResolutionPeriod);
+
+            const maxResolutionPeriod = minResolutionPeriod - 1n;
+            await expect(configHandler.connect(deployer).setMaxResolutionPeriod(maxResolutionPeriod)).to.revertedWith(
+              RevertReasons.INVALID_RESOLUTION_PERIOD
+            );
+          });
         });
       });
 

test/protocol/OfferHandlerTest.js

@@ -737,6 +737,7 @@ describe("IBosonOfferHandler", function () {
         });
 
         it("Resolution period is greater than protocol max resolution period", async function () {
+          await configHandler.setMinResolutionPeriod(oneDay - 1n);
           // Set max resolution period to 1 day
           await configHandler.setMaxResolutionPeriod(oneDay); // 24 hours