diff --git a/contracts/domain/BosonErrors.sol b/contracts/domain/BosonErrors.sol index 4457ce6b2..9122d1a9e 100644 --- a/contracts/domain/BosonErrors.sol +++ b/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% diff --git a/contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol b/contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol index 1acc6f980..cd36403a5 100644 --- a/contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol +++ b/contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol @@ -37,7 +37,7 @@ interface IBosonSequentialCommitHandler is BosonErrors, IBosonExchangeEvents, IB * - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer) * - Received ERC20 token amount differs from the expected value * - Protocol does not receive the voucher - * - Transfer of voucher to the buyer fails for some reasong (e.g. buyer is contract that doesn't accept voucher) + * - Transfer of voucher to the buyer fails for some reason (e.g. buyer is contract that doesn't accept voucher) * - Reseller did not approve protocol to transfer exchange token in escrow * - Call to price discovery contract fails * - Protocol fee and royalties combined exceed the secondary price diff --git a/contracts/mock/BuyerContract.sol b/contracts/mock/BuyerContract.sol new file mode 100644 index 000000000..6cb352112 --- /dev/null +++ b/contracts/mock/BuyerContract.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity 0.8.22; + +import { IERC721Receiver } from "../interfaces/IERC721Receiver.sol"; + +/** + * @title BuyerContract + * + * @notice Contract that acts as a buyer for testing purposes + */ +contract BuyerContract is IERC721Receiver { + enum FailType { + None, + Revert, + ReturnWrongSelector + } + + FailType public failType; + + /** + * @dev Set fail type + */ + function setFailType(FailType _failType) external { + failType = _failType; + } + + /** + * @dev Return wrong selector to test revert + */ + function onERC721Received( + address operator, + address from, + uint256 tokenId, + bytes calldata data + ) external returns (bytes4) { + if (failType == FailType.Revert) revert("BuyerContract: revert"); + if (failType == FailType.ReturnWrongSelector) return 0x12345678; + return IERC721Receiver.onERC721Received.selector; + } +} diff --git a/contracts/mock/MockDisputeHandlerFacet.sol b/contracts/mock/MockDisputeHandlerFacet.sol new file mode 100644 index 000000000..00fc7068a --- /dev/null +++ b/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); + } +} diff --git a/contracts/mock/MockExchangeHandlerFacet.sol b/contracts/mock/MockExchangeHandlerFacet.sol index 289746124..a4e508745 100644 --- a/contracts/mock/MockExchangeHandlerFacet.sol +++ b/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); + } +} diff --git a/contracts/protocol/bases/PriceDiscoveryBase.sol b/contracts/protocol/bases/PriceDiscoveryBase.sol index 6b3fbbd02..f7b70d480 100644 --- a/contracts/protocol/bases/PriceDiscoveryBase.sol +++ b/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; } /** @@ -147,7 +140,7 @@ contract PriceDiscoveryBase is ProtocolBase { if (_bosonVoucher.ownerOf(_tokenId) != address(this)) revert VoucherNotReceived(); // Transfer voucher to buyer - _bosonVoucher.transferFrom(address(this), _buyer, _tokenId); + _bosonVoucher.safeTransferFrom(address(this), _buyer, _tokenId); } uint256 overchargedAmount = _priceDiscovery.price - actualPrice; diff --git a/contracts/protocol/facets/DisputeHandlerFacet.sol b/contracts/protocol/facets/DisputeHandlerFacet.sol index 8a4ce2c3c..f84e2a15c 100644 --- a/contracts/protocol/facets/DisputeHandlerFacet.sol +++ b/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; diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 4fa9a9f63..a1afba797 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/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; diff --git a/contracts/protocol/facets/PriceDiscoveryHandlerFacet.sol b/contracts/protocol/facets/PriceDiscoveryHandlerFacet.sol index a248e79d6..b4fc00eaa 100644 --- a/contracts/protocol/facets/PriceDiscoveryHandlerFacet.sol +++ b/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 diff --git a/contracts/protocol/facets/SellerHandlerFacet.sol b/contracts/protocol/facets/SellerHandlerFacet.sol index ce621b929..986be2b66 100644 --- a/contracts/protocol/facets/SellerHandlerFacet.sol +++ b/contracts/protocol/facets/SellerHandlerFacet.sol @@ -172,6 +172,8 @@ contract SellerHandlerFacet is SellerBase { 1; } royaltyRecipients.pop(); + + delete royaltyRecipientIndexBySellerAndRecipient[_seller.treasury]; } // Update treasury @@ -487,19 +489,21 @@ contract SellerHandlerFacet is SellerBase { RoyaltyRecipient[] storage royaltyRecipients = lookups.royaltyRecipientsBySeller[_sellerId]; for (uint256 i = 0; i < _royaltyRecipients.length; ) { + // Cache storage pointer to avoid multiple lookups + mapping(address => uint256) storage royaltyRecipientIndexByRecipient = lookups + .royaltyRecipientIndexBySellerAndRecipient[_sellerId]; + // No uniqueness check for externalIds since they are not used in the protocol if ( _royaltyRecipients[i].wallet == treasury || - lookups.royaltyRecipientIndexBySellerAndRecipient[_sellerId][_royaltyRecipients[i].wallet] != 0 + royaltyRecipientIndexByRecipient[_royaltyRecipients[i].wallet] != 0 ) revert RecipientNotUnique(); if (_royaltyRecipients[i].minRoyaltyPercentage > protocolLimits().maxRoyaltyPercentage) revert InvalidRoyaltyPercentage(); royaltyRecipients.push(_royaltyRecipients[i]); - lookups.royaltyRecipientIndexBySellerAndRecipient[_sellerId][ - _royaltyRecipients[i].wallet - ] = royaltyRecipients.length; // can be optimized to use counter instead of array length + royaltyRecipientIndexByRecipient[_royaltyRecipients[i].wallet] = royaltyRecipients.length; // can be optimized to use counter instead of array length unchecked { i++; @@ -559,18 +563,16 @@ contract SellerHandlerFacet is SellerBase { if (royaltyRecipientId == 0) { if (_royaltyRecipients[i].wallet != address(0)) revert WrongDefaultRecipient(); } else { - uint256 royaltyRecipientIndex = lookups.royaltyRecipientIndexBySellerAndRecipient[_sellerId][ - _royaltyRecipients[i].wallet - ]; + // Cache storage pointer to avoid multiple lookups + mapping(address => uint256) storage royaltyRecipientIndexByRecipient = lookups + .royaltyRecipientIndexBySellerAndRecipient[_sellerId]; + + uint256 royaltyRecipientIndex = royaltyRecipientIndexByRecipient[_royaltyRecipients[i].wallet]; if (royaltyRecipientIndex == 0) { // update index - lookups.royaltyRecipientIndexBySellerAndRecipient[_sellerId][_royaltyRecipients[i].wallet] = - royaltyRecipientId + - 1; - delete lookups.royaltyRecipientIndexBySellerAndRecipient[_sellerId][ - royaltyRecipients[royaltyRecipientId].wallet - ]; + royaltyRecipientIndexByRecipient[_royaltyRecipients[i].wallet] = royaltyRecipientId + 1; + delete royaltyRecipientIndexByRecipient[royaltyRecipients[royaltyRecipientId].wallet]; } else { if (royaltyRecipientIndex - 1 != royaltyRecipientId) revert RecipientNotUnique(); } @@ -628,15 +630,15 @@ contract SellerHandlerFacet is SellerBase { if (royaltyRecipientId >= previousId) revert RoyaltyRecipientIdsNotSorted(); // this also ensures that royaltyRecipientId will never be out of bounds if (royaltyRecipientId == 0) revert CannotRemoveDefaultRecipient(); - delete lookups.royaltyRecipientIndexBySellerAndRecipient[_sellerId][ - royaltyRecipients[royaltyRecipientId].wallet - ]; + // Cache storage pointer to avoid multiple lookups + mapping(address => uint256) storage royaltyRecipientIndexByRecipient = lookups + .royaltyRecipientIndexBySellerAndRecipient[_sellerId]; + + delete royaltyRecipientIndexByRecipient[royaltyRecipients[royaltyRecipientId].wallet]; if (royaltyRecipientId != lastRoyaltyRecipientsId) { royaltyRecipients[royaltyRecipientId] = royaltyRecipients[lastRoyaltyRecipientsId]; - lookups.royaltyRecipientIndexBySellerAndRecipient[_sellerId][ - royaltyRecipients[royaltyRecipientId].wallet - ] = royaltyRecipientId; + royaltyRecipientIndexByRecipient[royaltyRecipients[royaltyRecipientId].wallet] = royaltyRecipientId; } royaltyRecipients.pop(); lastRoyaltyRecipientsId--; // will never underflow. Even if all non-default royalty recipients are removed, default recipient will remain diff --git a/contracts/protocol/facets/SequentialCommitHandlerFacet.sol b/contracts/protocol/facets/SequentialCommitHandlerFacet.sol index c1b502389..e13b46437 100644 --- a/contracts/protocol/facets/SequentialCommitHandlerFacet.sol +++ b/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. @@ -65,7 +60,7 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis * - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer) * - Received ERC20 token amount differs from the expected value * - Protocol does not receive the voucher - * - Transfer of voucher to the buyer fails for some reasong (e.g. buyer is contract that doesn't accept voucher) + * - Transfer of voucher to the buyer fails for some reason (e.g. buyer is contract that doesn't accept voucher) * - Reseller did not approve protocol to transfer exchange token in escrow * - Call to price discovery contract fails * - Protocol fee and royalties combined exceed the secondary price @@ -139,9 +134,12 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis revert FeeAmountTooHigh(); } - // Get price paid by current buyer - uint256 len = exchangeCosts.length; - uint256 currentPrice = len == 0 ? offer.price : exchangeCosts[len - 1].price; + uint256 currentPrice; + unchecked { + // Get price paid by current buyer + uint256 len = exchangeCosts.length; + currentPrice = len == 0 ? offer.price : exchangeCosts[len - 1].price; + } // Calculate the minimal amount to be kept in the escrow escrowAmount = @@ -156,7 +154,10 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis } // Make sure enough get escrowed - payout = exchangeCost.price - escrowAmount; + // Escrow amount is guaranteed to be less than or equal to price + unchecked { + payout = exchangeCost.price - escrowAmount; + } if (_priceDiscovery.side == Side.Ask) { if (escrowAmount > 0) { @@ -179,7 +180,7 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis } if (payout > 0) { - FundsLib.transferFundsFromProtocol(exchangeToken, payable(seller), payout); // also emits FundsWithdrawn + FundsLib.transferFundsFromProtocol(exchangeToken, payable(seller), payout); } } } diff --git a/contracts/protocol/libs/FundsLib.sol b/contracts/protocol/libs/FundsLib.sol index f5fcb1a9f..9c2848fc7 100644 --- a/contracts/protocol/libs/FundsLib.sol +++ b/contracts/protocol/libs/FundsLib.sol @@ -342,7 +342,6 @@ library FundsLib { // protocolFee and sellerRoyalties can be multiplied by effectivePriceMultiplier just at the end protocolFee = (protocolFee * effectivePriceMultiplier) / 10000; - sellerRoyalties = sellerRoyalties; } /** diff --git a/scripts/config/facet-deploy.js b/scripts/config/facet-deploy.js index 7426e80fe..5efa5b8b4 100644 --- a/scripts/config/facet-deploy.js +++ b/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. diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index f8bb341a7..6975f4035 100644 --- a/scripts/config/revert-reasons.js +++ b/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", @@ -174,6 +175,8 @@ exports.RevertReasons = { SAFE_ERC20_LOW_LEVEL_CALL: "SafeERC20: low-level call failed", SAFE_ERC20_OPERATION_FAILED: "SafeERC20: ERC20 operation did not succeed", INITIALIZABLE_ALREADY_INITIALIZED: "Initializable: contract is already initialized", + ERC721_NON_RECEIVER: "ERC721: transfer to non ERC721Receiver implementer", + BUYER_CONTRACT_REVERT: "BuyerContract: revert", // Meta-Transactions related NONCE_USED_ALREADY: "NonceUsedAlready", @@ -190,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", diff --git a/test/protocol/DisputeHandlerTest.js b/test/protocol/DisputeHandlerTest.js index abfe2c01d..105ec6e2a 100644 --- a/test/protocol/DisputeHandlerTest.js +++ b/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); + }); + }); + }); + }); }); diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 613a053e6..595dbba01 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/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); + }); + }); + }); + }); }); diff --git a/test/protocol/SellerHandlerTest.js b/test/protocol/SellerHandlerTest.js index 23e3e2560..0d701e9b1 100644 --- a/test/protocol/SellerHandlerTest.js +++ b/test/protocol/SellerHandlerTest.js @@ -3885,7 +3885,72 @@ describe("SellerHandler", function () { ); expect(returnedRoyaltyRecipientList).to.deep.equal( expectedRoyaltyRecipientList, - "Default royalty recipient mismatch" + "Royalty recipient mismatch" + ); + }); + + it("correctly handle treasury during the seller update", async function () { + // Add royalty recipients + await accountHandler.connect(admin).addRoyaltyRecipients(seller.id, royaltyRecipientList.toStruct()); + + // Update the seller, so one of the recipients is removed + seller.treasury = other1.address; + await accountHandler.connect(admin).updateSeller(seller, emptyAuthToken); + + // other1 is not a recipient anymore + let returnedRoyaltyRecipientList = RoyaltyRecipientList.fromStruct( + await accountHandler.connect(rando).getRoyaltyRecipients(seller.id) + ); + + royaltyRecipientList = new RoyaltyRecipientList([ + new RoyaltyRecipient(other3.address, "300", "other3"), + new RoyaltyRecipient(other2.address, "200", "other2"), + ]); + + expectedRoyaltyRecipientList = new RoyaltyRecipientList([ + new RoyaltyRecipient(ZeroAddress, voucherInitValues.royaltyPercentage, DEFAULT_ROYALTY_RECIPIENT), + ...royaltyRecipientList.royaltyRecipients, + ]); + + expect(returnedRoyaltyRecipientList).to.deep.equal( + expectedRoyaltyRecipientList, + "Royalty recipient mismatch" + ); + + // other 1 now cannot be added as another recipient + royaltyRecipientList = new RoyaltyRecipientList([new RoyaltyRecipient(other1.address, "100", "other1")]); + + // Adding other 1 should fail + await expect( + accountHandler.connect(admin).addRoyaltyRecipients(seller.id, royaltyRecipientList.toStruct()) + ).to.revertedWithCustomError(bosonErrors, RevertReasons.RECIPIENT_NOT_UNIQUE); + + // Update the seller again, so other 1 can later be added as a recipient + seller.treasury = rando.address; + await accountHandler.connect(admin).updateSeller(seller, emptyAuthToken); + + // Now adding should succeed + await accountHandler.connect(admin).addRoyaltyRecipients(seller.id, royaltyRecipientList.toStruct()); + + // other1 is back on the list + returnedRoyaltyRecipientList = RoyaltyRecipientList.fromStruct( + await accountHandler.connect(rando).getRoyaltyRecipients(seller.id) + ); + + royaltyRecipientList = new RoyaltyRecipientList([ + new RoyaltyRecipient(other3.address, "300", "other3"), + new RoyaltyRecipient(other2.address, "200", "other2"), + new RoyaltyRecipient(other1.address, "100", "other1"), + ]); + + expectedRoyaltyRecipientList = new RoyaltyRecipientList([ + new RoyaltyRecipient(ZeroAddress, voucherInitValues.royaltyPercentage, DEFAULT_ROYALTY_RECIPIENT), + ...royaltyRecipientList.royaltyRecipients, + ]); + + expect(returnedRoyaltyRecipientList).to.deep.equal( + expectedRoyaltyRecipientList, + "Royalty recipient mismatch" ); }); diff --git a/test/protocol/SequentialCommitHandlerTest.js b/test/protocol/SequentialCommitHandlerTest.js index 3b52bba78..8a670c8b1 100644 --- a/test/protocol/SequentialCommitHandlerTest.js +++ b/test/protocol/SequentialCommitHandlerTest.js @@ -706,6 +706,55 @@ describe("IBosonSequentialCommitHandler", function () { .sequentialCommitToOffer(buyer2.address, tokenId, priceDiscovery, { value: price2 }) ).to.revertedWithCustomError(bosonErrors, RevertReasons.VOUCHER_NOT_RECEIVED); }); + + context("Buyer rejects the voucher", async function () { + it("Buyer contract does not implement the receiver", async function () { + const [buyerContract] = await deployMockTokens(["Foreign20"]); + + // Sequential commit to offer, expect revert + await expect( + sequentialCommitHandler + .connect(rando) + .sequentialCommitToOffer(await buyerContract.getAddress(), tokenId, priceDiscovery, { + value: price2, + }) + ).to.revertedWith(RevertReasons.ERC721_NON_RECEIVER); + }); + + it("Buyer contract reverts with custom error", async function () { + const buyerContractFactory = await getContractFactory("BuyerContract"); + const buyerContract = await buyerContractFactory.deploy(); + await buyerContract.waitForDeployment(); + + await buyerContract.setFailType(1); // Type 1 = revert with own error + + // Sequential commit to offer, expect revert + await expect( + sequentialCommitHandler + .connect(rando) + .sequentialCommitToOffer(await buyerContract.getAddress(), tokenId, priceDiscovery, { + value: price2, + }) + ).to.revertedWith(RevertReasons.BUYER_CONTRACT_REVERT); + }); + + it("Buyer contract returns the wrong selector", async function () { + const buyerContractFactory = await getContractFactory("BuyerContract"); + const buyerContract = await buyerContractFactory.deploy(); + await buyerContract.waitForDeployment(); + + await buyerContract.setFailType(2); // Type 2 = wrong selector + + // Sequential commit to offer, expect revert + await expect( + sequentialCommitHandler + .connect(rando) + .sequentialCommitToOffer(await buyerContract.getAddress(), tokenId, priceDiscovery, { + value: price2, + }) + ).to.revertedWith(RevertReasons.ERC721_NON_RECEIVER); + }); + }); }); });