Merge branch 'main' into audit_v2_4_0_sch_01m

contracts/domain/BosonErrors.sol

@@ -214,6 +214,8 @@ interface BosonErrors {
     error ExchangeAlreadyExists();
     // Range length is 0, is more than quantity available or it would cause an overflow
     error InvalidRangeLength();
+    // Exchange is being finalized into an invalid state
+    error InvalidTargeExchangeState();
 
     // Twin related
     // Twin does not exist
@@ -300,6 +302,8 @@ interface BosonErrors {
     error InvalidDisputeTimeout();
     // Absolute zero offers cannot be escalated
     error EscalationNotAllowed();
+    // Dispute is being finalized into an invalid state
+    error InvalidTargeDisputeState();
 
     // Config related
     // Percentage exceeds 100%

contracts/mock/MockDisputeHandlerFacet.sol

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity 0.8.22;
+
+import { DisputeHandlerFacet } from "../protocol/facets/DisputeHandlerFacet.sol";
+
+/**
+ * @title TestDisputeHandlerFacet
+ *
+ * @notice Extended DisputeHandlerFacet with additional external functions for testing
+ */
+contract TestDisputeHandlerFacet is DisputeHandlerFacet {
+    /**
+     * @notice Test function to test invalid final dispute state
+     *
+     * @param _exchangeId  - the id of the associated exchange
+     * @param _targetState - target final state
+     */
+    function finalizeDispute(uint256 _exchangeId, DisputeState _targetState) external {
+        (, Exchange storage exchange) = fetchExchange(_exchangeId);
+        (, Dispute storage dispute, DisputeDates storage disputeDates) = fetchDispute(_exchangeId);
+        finalizeDispute(_exchangeId, exchange, dispute, disputeDates, _targetState, 1000);
+    }
+}

contracts/mock/MockExchangeHandlerFacet.sol

@@ -7,6 +7,7 @@ import { DisputeBase } from "../protocol/bases/DisputeBase.sol";
 import { FundsLib } from "../protocol/libs/FundsLib.sol";
 import "../domain/BosonConstants.sol";
 import "@openzeppelin/contracts/utils/Address.sol";
+import { ExchangeHandlerFacet } from "../protocol/facets/ExchangeHandlerFacet.sol";
 
 /**
  * @title MockExchangeHandlerFacet
@@ -537,3 +538,24 @@ contract MockExchangeHandlerFacetWithDefect is MockExchangeHandlerFacet {
         emit VoucherCanceled2(exchange.offerId, _exchangeId, msgSender());
     }
 }
+
+/**
+ * @title TestExchangeHandlerFacet
+ *
+ * @notice Extended ExchangeHandlerFacet with additional external functions for testing
+ */
+contract TestExchangeHandlerFacet is ExchangeHandlerFacet {
+    //solhint-disable-next-line
+    constructor(uint256 _firstExchangeId2_2_0) ExchangeHandlerFacet(_firstExchangeId2_2_0) {}
+
+    /**
+     * @notice Test function to test invalid final exchange state
+     *
+     * @param _exchangeId - the id of the exchange to finalize
+     * @param _targetState - the target state to which the exchange should be transitioned
+     */
+    function finalizeExchange(uint256 _exchangeId, ExchangeState _targetState) external {
+        (, Exchange storage exchange) = fetchExchange(_exchangeId);
+        finalizeExchange(exchange, _targetState);
+    }
+}

contracts/protocol/bases/PriceDiscoveryBase.sol

@@ -21,24 +21,17 @@ contract PriceDiscoveryBase is ProtocolBase {
     using SafeERC20 for IERC20;
 
     IWrappedNative internal immutable wNative;
-    uint256 private immutable EXCHANGE_ID_2_2_0; // solhint-disable-line
 
     /**
      * @notice
      * For offers with native exchange token, it is expected the the price discovery contracts will
      * operate with wrapped native token. Set the address of the wrapped native token in the constructor.
      *
-     * After v2.2.0, token ids are derived from offerId and exchangeId.
-     * EXCHANGE_ID_2_2_0 is the first exchange id to use for 2.2.0.
-     * Set EXCHANGE_ID_2_2_0 in the constructor.
-     *
      * @param _wNative - the address of the wrapped native token
-     * @param _firstExchangeId2_2_0 - the first exchange id to use for 2.2.0
      */
     //solhint-disable-next-line
-    constructor(address _wNative, uint256 _firstExchangeId2_2_0) {
+    constructor(address _wNative) {
         wNative = IWrappedNative(_wNative);
-        EXCHANGE_ID_2_2_0 = _firstExchangeId2_2_0;
     }
 
     /**

contracts/protocol/facets/DisputeHandlerFacet.sol

@@ -439,6 +439,9 @@ contract DisputeHandlerFacet is DisputeBase, IBosonDisputeHandler {
         DisputeState _targetState,
         uint256 _buyerPercent
     ) internal disputesNotPaused {
+        if (_targetState == DisputeState.Escalated || _targetState == DisputeState.Resolving)
+            revert InvalidTargeDisputeState();
+
         // update dispute and exchange
         _disputeDates.finalized = block.timestamp;
         _dispute.state = _targetState;

contracts/protocol/facets/ExchangeHandlerFacet.sol

@@ -947,11 +947,11 @@ contract ExchangeHandlerFacet is DisputeBase, BuyerBase, IBosonExchangeHandler,
      */
     function finalizeExchange(Exchange storage _exchange, ExchangeState _targetState) internal {
         // Make sure target state is a final state
-        require(
-            _targetState == ExchangeState.Completed ||
-                _targetState == ExchangeState.Revoked ||
-                _targetState == ExchangeState.Canceled
-        );
+        if (
+            _targetState != ExchangeState.Completed &&
+            _targetState != ExchangeState.Revoked &&
+            _targetState != ExchangeState.Canceled
+        ) revert InvalidTargeExchangeState();
 
         // Set the exchange state to the target state
         _exchange.state = _targetState;

contracts/protocol/facets/PriceDiscoveryHandlerFacet.sol

@@ -29,15 +29,10 @@ contract PriceDiscoveryHandlerFacet is IBosonPriceDiscoveryHandler, PriceDiscove
      * For offers with native exchange token, it is expected the the price discovery contracts will
      * operate with wrapped native token. Set the address of the wrapped native token in the constructor.
      *
-     * After v2.2.0, token ids are derived from offerId and exchangeId.
-     * EXCHANGE_ID_2_2_0 is the first exchange id to use for 2.2.0.
-     * Set EXCHANGE_ID_2_2_0 in the constructor.
-     *
      * @param _wNative - the address of the wrapped native token
-     * @param _firstExchangeId2_2_0 - the first exchange id to use for 2.2.0
      */
     //solhint-disable-next-line
-    constructor(address _wNative, uint256 _firstExchangeId2_2_0) PriceDiscoveryBase(_wNative, _firstExchangeId2_2_0) {}
+    constructor(address _wNative) PriceDiscoveryBase(_wNative) {}
 
     /**
      * @notice Facet Initializer

contracts/protocol/facets/SequentialCommitHandlerFacet.sol

@@ -24,15 +24,10 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis
      * For offers with native exchange token, it is expected the the price discovery contracts will
      * operate with wrapped native token. Set the address of the wrapped native token in the constructor.
      *
-     * After v2.2.0, token ids are derived from offerId and exchangeId.
-     * EXCHANGE_ID_2_2_0 is the first exchange id to use for 2.2.0.
-     * Set EXCHANGE_ID_2_2_0 in the constructor.
-     *
      * @param _wNative - the address of the wrapped native token
-     * @param _firstExchangeId2_2_0 - the first exchange id to use for 2.2.0
      */
     //solhint-disable-next-line
-    constructor(address _wNative, uint256 _firstExchangeId2_2_0) PriceDiscoveryBase(_wNative, _firstExchangeId2_2_0) {}
+    constructor(address _wNative) PriceDiscoveryBase(_wNative) {}
 
     /**
      * @notice Initializes facet.

scripts/config/facet-deploy.js

@@ -80,11 +80,11 @@ async function getFacets(config) {
   facetArgs["ExchangeHandlerFacet"] = { init: [], constructorArgs: [protocolConfig.EXCHANGE_ID_2_2_0[network]] };
   facetArgs["SequentialCommitHandlerFacet"] = {
     init: [],
-    constructorArgs: [protocolConfig.WrappedNative[network], protocolConfig.EXCHANGE_ID_2_2_0[network]],
+    constructorArgs: [protocolConfig.WrappedNative[network]],
   };
   facetArgs["PriceDiscoveryHandlerFacet"] = {
     init: [],
-    constructorArgs: [protocolConfig.WrappedNative[network], protocolConfig.EXCHANGE_ID_2_2_0[network]],
+    constructorArgs: [protocolConfig.WrappedNative[network]],
   };
 
   // metaTransactionsHandlerFacet initializer arguments.

scripts/config/revert-reasons.js

@@ -134,6 +134,7 @@ exports.RevertReasons = {
   EXCHANGE_IS_NOT_IN_A_FINAL_STATE: "ExchangeIsNotInAFinalState",
   INVALID_RANGE_LENGTH: "InvalidRangeLength",
   EXCHANGE_ALREADY_EXISTS: "ExchangeAlreadyExists",
+  INVALID_TARGET_EXCHANGE_STATE: "InvalidTargeExchangeState",
 
   // Voucher related
   EXCHANGE_ID_IN_RESERVED_RANGE: "ExchangeIdInReservedRange",
@@ -192,6 +193,7 @@ exports.RevertReasons = {
   DISPUTE_STILL_VALID: "DisputeStillValid",
   INVALID_DISPUTE_TIMEOUT: "InvalidDisputeTimeout",
   ESCALATION_NOT_ALLOWED: "EscalationNotAllowed",
+  INVALID_TARGET_DISPUTE_STATE: "InvalidTargeDisputeState",
 
   // Config related
   FEE_PERCENTAGE_INVALID: "InvalidFeePercentage",

test/protocol/DisputeHandlerTest.js

@@ -1,5 +1,5 @@
 const { ethers } = require("hardhat");
-const { ZeroAddress, provider, zeroPadBytes, MaxUint256, parseUnits, getContractAt } = ethers;
+const { ZeroAddress, provider, zeroPadBytes, MaxUint256, parseUnits, getContractAt, id } = ethers;
 const { expect, assert } = require("chai");
 const Exchange = require("../../scripts/domain/Exchange");
 const Dispute = require("../../scripts/domain/Dispute");
@@ -29,6 +29,7 @@ const {
   mockBuyer,
   accountId,
 } = require("../util/mock");
+const { FacetCutAction } = require("../../scripts/util/diamond-utils.js");
 
 /**
  *  Test the Boson Dispute Handler interface
@@ -2580,4 +2581,46 @@ describe("IBosonDisputeHandler", function () {
       });
     });
   });
+
+  // Internal functions, tested with TestDisputeHandlerFacet
+  context("📋 Internal Dispute Handler Methods", async function () {
+    let testDisputeHandler;
+    beforeEach(async function () {
+      // Deploy test facet and cut the test functions
+      const TestDisputeHandlerFacet = await ethers.getContractFactory("TestDisputeHandlerFacet");
+      const testDisputeHandlerFacet = await TestDisputeHandlerFacet.deploy();
+      await testDisputeHandlerFacet.waitForDeployment();
+
+      const protocolDiamondAddress = await disputeHandler.getAddress();
+      const cutFacetViaDiamond = await getContractAt("DiamondCutFacet", protocolDiamondAddress);
+
+      // Define the facet cut
+      const facetCuts = [
+        {
+          facetAddress: await testDisputeHandlerFacet.getAddress(),
+          action: FacetCutAction.Add,
+          functionSelectors: [id("finalizeDispute(uint256,uint8)").slice(0, 10)],
+        },
+      ];
+
+      // Send the DiamondCut transaction
+      await cutFacetViaDiamond.diamondCut(facetCuts, ZeroAddress, "0x");
+
+      testDisputeHandler = await getContractAt("TestDisputeHandlerFacet", protocolDiamondAddress);
+    });
+
+    context("👉 finalizeDispute()", async function () {
+      const invalidFinalStates = ["Resolving", "Escalated"];
+
+      invalidFinalStates.forEach((finalState) => {
+        it(`final state is ${finalState}`, async function () {
+          const exchangeId = 1;
+
+          await expect(
+            testDisputeHandler.finalizeDispute(exchangeId, DisputeState[finalState])
+          ).to.revertedWithCustomError(bosonErrors, RevertReasons.INVALID_TARGET_DISPUTE_STATE);
+        });
+      });
+    });
+  });
 });

test/protocol/ExchangeHandlerTest.js

@@ -8132,4 +8132,45 @@ describe("IBosonExchangeHandler", function () {
       });
     });
   });
+
+  // Internal functions, tested with TestExchangeHandlerFacet
+  context("📋 Internal Exchange Handler Methods", async function () {
+    let testExchangeHandler;
+    beforeEach(async function () {
+      // Deploy test facet and cut the test functions
+      const TestExchangeHandlerFacet = await ethers.getContractFactory("TestExchangeHandlerFacet");
+      const testExchangeHandlerFacet = await TestExchangeHandlerFacet.deploy(0);
+      await testExchangeHandlerFacet.waitForDeployment();
+
+      const cutFacetViaDiamond = await getContractAt("DiamondCutFacet", protocolDiamondAddress);
+
+      // Define the facet cut
+      const facetCuts = [
+        {
+          facetAddress: await testExchangeHandlerFacet.getAddress(),
+          action: FacetCutAction.Add,
+          functionSelectors: [id("finalizeExchange(uint256,uint8)").slice(0, 10)],
+        },
+      ];
+
+      // Send the DiamondCut transaction
+      await cutFacetViaDiamond.connect(deployer).diamondCut(facetCuts, ZeroAddress, "0x");
+
+      testExchangeHandler = await getContractAt("TestExchangeHandlerFacet", protocolDiamondAddress);
+    });
+
+    context("👉 finalizeExchange()", async function () {
+      const invalidFinalStates = ["Committed", "Redeemed", "Disputed"];
+
+      invalidFinalStates.forEach((finalState) => {
+        it(`final state is ${finalState}`, async function () {
+          const exchangeId = 1;
+
+          await expect(
+            testExchangeHandler.finalizeExchange(exchangeId, ExchangeState[finalState])
+          ).to.revertedWithCustomError(bosonErrors, RevertReasons.INVALID_TARGET_EXCHANGE_STATE);
+        });
+      });
+    });
+  });
 });