refactor: address PR comments and optimise code

contracts/domain/BosonErrors.sol

@@ -17,6 +17,8 @@ interface BosonErrors {
     error InvalidState();
     // Two or more array parameters with different lengths
     error ArrayLengthMismatch();
+    // Array elements that are not in ascending order (i.e arr[i-1] > arr[i])
+    error NonAscendingOrder();
 
     // Reentrancy guard
     // Reentrancy guard is active and second call to protocol is made

contracts/interfaces/events/IBosonConfigEvents.sol

@@ -30,5 +30,5 @@ interface IBosonConfigEvents {
     event MinDisputePeriodChanged(uint256 minDisputePeriod, address indexed executedBy);
     event MaxPremintedVouchersChanged(uint256 maxPremintedVouchers, address indexed executedBy);
     event AccessControllerAddressChanged(address indexed accessControllerAddress, address indexed executedBy);
-    event FeeTableUpdated(address indexed token, uint256[] priceRanges, uint256[] feePercentages);
+    event FeeTableUpdated(address indexed token, uint256[] priceRanges, uint256[] feePercentages, address indexed executedBy);
 }

contracts/interfaces/handlers/IBosonConfigHandler.sol

@@ -169,7 +169,7 @@ interface IBosonConfigHandler is IBosonConfigEvents, BosonErrors {
      *
      * @return The protocol fee amount based on the token and the price.
      */
-    function getProtocolFeePercentage(address _exchangeToken, uint256 _price) external view returns (uint256);
+    function getProtocolFee(address _exchangeToken, uint256 _price) external view returns (uint256);
 
     /**
      * @notice Sets the flat protocol fee for exchanges in $BOSON.

contracts/protocol/bases/OfferBase.sol

@@ -235,7 +235,7 @@ contract OfferBase is ProtocolBase, IBosonOfferEvents {
                 if (_offer.buyerCancelPenalty > offerPrice) revert InvalidOfferPenalty();
                 if (_offer.priceType == PriceType.Static) {
                     // Calculate and set the protocol fee
-                    uint256 protocolFee = getProtocolFee(_offer.exchangeToken, offerPrice);
+                    uint256 protocolFee = _getProtocolFee(_offer.exchangeToken, offerPrice);
 
                     // Calculate the agent fee amount
                     uint256 agentFeeAmount = (agent.feePercentage * offerPrice) / HUNDRED_PERCENT;

contracts/protocol/bases/ProtocolBase.sol

@@ -9,6 +9,7 @@ import { EIP712Lib } from "../libs/EIP712Lib.sol";
 import { BosonTypes } from "../../domain/BosonTypes.sol";
 import { PausableBase } from "./PausableBase.sol";
 import { ReentrancyGuardBase } from "./ReentrancyGuardBase.sol";
+import { FundsLib } from "../libs/FundsLib.sol";
 
 /**
  * @title ProtocolBase
@@ -693,32 +694,34 @@ abstract contract ProtocolBase is PausableBase, ReentrancyGuardBase, BosonErrors
      * @param _price - the price of the exchange
      * @return protocolFee - the protocol fee
      */
-    function getProtocolFee(address _exchangeToken, uint256 _price) internal view returns (uint256 protocolFee) {
+    function _getProtocolFee(address _exchangeToken, uint256 _price) internal view returns (uint256 protocolFee) {
+        ProtocolLib.ProtocolFees storage fees = protocolFees();
         // Check if the exchange token is the Boson token
         if (_exchangeToken == protocolAddresses().token) {
             // Apply the flatBoson fee if the exchange token is the Boson token
-            return protocolFees().flatBoson;
+            return fees.flatBoson;
         }
 
-        uint256[] storage priceRanges = protocolFees().tokenPriceRanges[_exchangeToken];
-        uint256[] storage feePercentages = protocolFees().tokenFeePercentages[_exchangeToken];
+        uint256[] storage priceRanges = fees.tokenPriceRanges[_exchangeToken];
+        uint256[] storage feePercentages = fees.tokenFeePercentages[_exchangeToken];
 
         // If the token has a custom fee table, calculate based on the price ranges
-        if (priceRanges.length > 0 && feePercentages.length > 0) {
-            for (uint256 i = 0; i < priceRanges.length; i++) {
+        uint256 priceRangesLength = priceRanges.length;
+        if (priceRangesLength > 0) {
+            for (uint256 i; i < priceRangesLength; ++i) {
                 if (_price <= priceRanges[i]) {
                     // Apply the fee percentage for the matching price range
-                    uint256 feePercentage = feePercentages[i];
-                    return (feePercentage * _price) / HUNDRED_PERCENT;
+                    return FundsLib.applyPercent(_price,feePercentages[i]);
+
                 }
             }
             // If price exceeds all ranges, use the highest fee percentage
-            uint256 highestFeePercentage = feePercentages[priceRanges.length - 1];
-            return (highestFeePercentage * _price) / HUNDRED_PERCENT;
+            return FundsLib.applyPercent(_price,feePercentages[priceRangesLength - 1]);
         }
 
         // If no custom fee table exists, fallback to using the default protocol percentage
-        return (protocolFees().percentage * _price) / HUNDRED_PERCENT;
+        return FundsLib.applyPercent(_price,fees.percentage);
+
     }
 
     /**

contracts/protocol/facets/ConfigHandlerFacet.sol

@@ -247,7 +247,6 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
         uint256[] calldata _priceRanges,
         uint256[] calldata _feePercentages
     ) external override onlyRole(ADMIN) nonReentrant {
-        checkNonZeroAddress(_tokenAddress);
         if (_priceRanges.length != _feePercentages.length) revert ArrayLengthMismatch();
         // Clear existing price ranges and percentage tiers
         delete protocolFees().tokenPriceRanges[_tokenAddress];
@@ -257,7 +256,7 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
             setTokenPriceRanges(_tokenAddress, _priceRanges);
             setTokenFeePercentages(_tokenAddress, _feePercentages);
         }
-        emit FeeTableUpdated(_tokenAddress, _priceRanges, _feePercentages);
+        emit FeeTableUpdated(_tokenAddress, _priceRanges, _feePercentages, msgSender());
     }
 
     /**
@@ -283,8 +282,8 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
      *
      * @return The protocol fee amount based on the token and the price.
      */
-    function getProtocolFeePercentage(address _exchangeToken, uint256 _price) external view override returns (uint256) {
-        return getProtocolFee(_exchangeToken, _price);
+    function getProtocolFee(address _exchangeToken, uint256 _price) external view override returns (uint256) {
+        return _getProtocolFee(_exchangeToken, _price);
     }
 
     /**
@@ -618,10 +617,10 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
      * @param _priceRanges - array of price ranges for the token
      */
     function setTokenPriceRanges(address _tokenAddress, uint256[] calldata _priceRanges) internal {
-        // Set new price ranges
-        for (uint256 i = 0; i < _priceRanges.length; i++) {
-            protocolFees().tokenPriceRanges[_tokenAddress].push(_priceRanges[i]);
+        for (uint256 i = 1; i < _priceRanges.length; ++i) {
+            if(_priceRanges[i] < _priceRanges[i - 1]) revert NonAscendingOrder();
         }
+        protocolFees().tokenPriceRanges[_tokenAddress] = _priceRanges;
     }
 
     /**
@@ -632,10 +631,10 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase {
      */
     function setTokenFeePercentages(address _tokenAddress, uint256[] calldata _feePercentages) internal {
         // Set the fee percentages for the token
-        for (uint256 i = 0; i < _feePercentages.length; i++) {
+        for (uint256 i; i < _feePercentages.length; ++i) {
             checkMaxPercententage(_feePercentages[i]);
-            protocolFees().tokenFeePercentages[_tokenAddress].push(_feePercentages[i]);
         }
+        protocolFees().tokenFeePercentages[_tokenAddress] = _feePercentages;
     }
 
     /**

contracts/protocol/facets/PriceDiscoveryHandlerFacet.sol

@@ -108,7 +108,7 @@ contract PriceDiscoveryHandlerFacet is IBosonPriceDiscoveryHandler, PriceDiscove
 
         // Calculate fees
         address exchangeToken = offer.exchangeToken;
-        uint256 protocolFeeAmount = getProtocolFee(exchangeToken, actualPrice);
+        uint256 protocolFeeAmount = _getProtocolFee(exchangeToken, actualPrice);
 
         {
             // Calculate royalties

contracts/protocol/facets/SequentialCommitHandlerFacet.sol

@@ -116,7 +116,7 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis
 
             {
                 // Calculate fees
-                thisExchangeCost.protocolFeeAmount = getProtocolFee(exchangeToken, thisExchangeCost.price);
+                thisExchangeCost.protocolFeeAmount = _getProtocolFee(exchangeToken, thisExchangeCost.price);
 
                 // Calculate royalties
                 {

scripts/config/revert-reasons.js

@@ -17,6 +17,7 @@ exports.RevertReasons = {
   INVALID_STATE: "InvalidState",
   ARRAY_LENGTH_MISMATCH: "ArrayLengthMismatch",
   REENTRANCY_GUARD: "ReentrancyGuard",
+  NON_ASCENDING_ORDER: "NonAscendingOrder",
 
   // Facet initializer related
   ALREADY_INITIALIZED: "AlreadyInitialized",

test/example/SnapshotGateTest.js

@@ -148,11 +148,9 @@ describe("SnapshotGate", function () {
         maxPremintedVouchers: 10000,
       },
       // Protocol fees
-      {
-        percentage: protocolFeePercentage,
-        flatBoson: protocolFeeFlatBoson,
-        buyerEscalationDepositPercentage,
-      },
+      protocolFeePercentage,
+      protocolFeeFlatBoson,
+      buyerEscalationDepositPercentage,
     ];
 
     const facetNames = [

test/protocol/ConfigHandlerTest.js

@@ -979,7 +979,7 @@ describe("IBosonConfigHandler", function () {
           // Set new protocol fee precentage address, testing for the event
           await expect(configHandler.connect(deployer).setProtocolFeeTable(usdcAddress, feePriceRanges, feePercentages))
             .to.emit(configHandler, "FeeTableUpdated")
-            .withArgs(usdcAddress, feePriceRanges, feePercentages);
+            .withArgs(usdcAddress, feePriceRanges, feePercentages, await deployer.getAddress());
         });
 
         it("should update state and return fee amount based on configured fee table", async function () {
@@ -988,26 +988,20 @@ describe("IBosonConfigHandler", function () {
             exchangeAmount = feePriceRanges[i];
             feeTier = feePercentages[i];
             expectedFeeAmount = applyPercentage(exchangeAmount, feeTier);
-            expect(await configHandler.getProtocolFeePercentage(usdcAddress, exchangeAmount)).to.equal(
-              expectedFeeAmount
-            );
+            expect(await configHandler.getProtocolFee(usdcAddress, exchangeAmount)).to.equal(expectedFeeAmount);
           }
 
           // check for a way bigger value
           feeTier = feePercentages[feePercentages.length - 1];
           exchangeAmount = BigInt(feePriceRanges[feePriceRanges.length - 1]) * BigInt(2);
           expectedFeeAmount = applyPercentage(exchangeAmount, feeTier);
-          console.log(`Fee Tier ${feeTier}`);
-          console.log(`Exchange Amt ${expectedFeeAmount}`);
-          expect(await configHandler.getProtocolFeePercentage(usdcAddress, exchangeAmount)).to.equal(expectedFeeAmount);
+          expect(await configHandler.getProtocolFee(usdcAddress, exchangeAmount)).to.equal(expectedFeeAmount);
         });
 
         it("should update state and return boson flat fee if boson token used as exchange token", async function () {
           await configHandler.connect(deployer).setProtocolFeeTable(usdcAddress, feePriceRanges, feePercentages);
           exchangeAmount = feePriceRanges[0];
-          expect(await configHandler.getProtocolFeePercentage(bosonToken, exchangeAmount)).to.equal(
-            protocolFeeFlatBoson
-          );
+          expect(await configHandler.getProtocolFee(bosonToken, exchangeAmount)).to.equal(protocolFeeFlatBoson);
         });
 
         context("💔 Revert Reasons", async function () {
@@ -1017,12 +1011,6 @@ describe("IBosonConfigHandler", function () {
               configHandler.connect(rando).setProtocolFeeTable(usdcAddress, feePriceRanges, feePercentages)
             ).to.revertedWithCustomError(bosonErrors, RevertReasons.ACCESS_DENIED);
           });
-
-          it("exchange token is zero address", async function () {
-            await expect(
-              configHandler.connect(deployer).setProtocolFeeTable(ZeroAddress, feePriceRanges, feePercentages)
-            ).to.revertedWithCustomError(bosonErrors, RevertReasons.INVALID_ADDRESS);
-          });
           it("price ranges count different from fee percents", async function () {
             const newPriceRanges = [...feePriceRanges, parseUnits("10", "ether").toString()];
             await expect(
@@ -1037,6 +1025,17 @@ describe("IBosonConfigHandler", function () {
               configHandler.connect(deployer).setProtocolFeeTable(usdcAddress, feePriceRanges, newFeePercentages)
             ).to.revertedWithCustomError(bosonErrors, RevertReasons.FEE_PERCENTAGE_INVALID);
           });
+          it("price ranges are not in ascending order", async function () {
+            const newPriceRanges = [
+              parseUnits("1", "ether").toString(),
+              parseUnits("3", "ether").toString(),
+              parseUnits("2", "ether").toString(),
+            ];
+
+            await expect(
+              configHandler.connect(deployer).setProtocolFeeTable(usdcAddress, newPriceRanges, feePercentages)
+            ).to.revertedWithCustomError(bosonErrors, RevertReasons.NON_ASCENDING_ORDER);
+          });
         });
       });
     });