From fe9d1be4f6f6f7cc28bb8ab6cfc56bd4b189d012 Mon Sep 17 00:00:00 2001 From: Ayush Tiwari Date: Tue, 3 Oct 2023 16:26:16 +0530 Subject: [PATCH] fix: function ordering --- .../contracts/exchange/Exchange.sol | 41 +++++++------------ .../contracts/mocks/SimpleTest.sol | 2 +- .../transfer-manager/TransferManager.sol | 4 +- .../test/exchange/Exchange.test.ts | 10 ++--- 4 files changed, 23 insertions(+), 34 deletions(-) diff --git a/packages/marketplace/contracts/exchange/Exchange.sol b/packages/marketplace/contracts/exchange/Exchange.sol index ce9a63598c..ae2df77fa7 100644 --- a/packages/marketplace/contracts/exchange/Exchange.sol +++ b/packages/marketplace/contracts/exchange/Exchange.sol @@ -2,10 +2,10 @@ pragma solidity 0.8.21; -import {ERC165Upgradeable, IERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/ERC165Upgradeable.sol"; import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {ContextUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol"; +import {ERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/ERC165Upgradeable.sol"; import {ERC2771HandlerUpgradeable} from "@sandbox-smart-contracts/dependency-metatx/contracts/ERC2771HandlerUpgradeable.sol"; import {IOrderValidator} from "../interfaces/IOrderValidator.sol"; import {IAssetMatcher} from "../interfaces/IAssetMatcher.sol"; @@ -17,14 +17,7 @@ import {ExchangeCore} from "./ExchangeCore.sol"; /// @notice Used to exchange assets, that is, tokens. /// @dev Main functions are in ExchangeCore /// @dev TransferManager is used to execute token transfers -contract Exchange is - Initializable, - ERC165Upgradeable, - AccessControlUpgradeable, - ExchangeCore, - TransferManager, - ERC2771HandlerUpgradeable -{ +contract Exchange is Initializable, AccessControlUpgradeable, ExchangeCore, TransferManager, ERC2771HandlerUpgradeable { /// @notice role erc1776 trusted meta transaction contracts (Sand for example). /// @return hash for ERC1776_OPERATOR_ROLE bytes32 public constant ERC1776_OPERATOR_ROLE = keccak256("ERC1776_OPERATOR_ROLE"); @@ -33,10 +26,6 @@ contract Exchange is /// @return hash for EXCHANGE_ADMIN_ROLE bytes32 public constant EXCHANGE_ADMIN_ROLE = keccak256("EXCHANGE_ADMIN_ROLE"); - /// @notice role to identify the sandbox accounts - /// @return hash for SKIP_FEES_ROLE - bytes32 public constant SKIP_FEES_ROLE = keccak256("SKIP_FEES_ROLE"); - /// @dev this protects the implementation contract from being initialized. /// @custom:oz-upgrades-unsafe-allow constructor constructor() { @@ -64,6 +53,7 @@ contract Exchange is IAssetMatcher newAssetMatcher ) external initializer { __ERC2771Handler_init(newTrustedForwarder); + __AccessControl_init(); __TransferManager_init_unchained( newProtocolFeePrimary, newProtocolFeeSecondary, @@ -71,7 +61,6 @@ contract Exchange is newRoyaltiesProvider ); __ExchangeCoreInitialize(orderValidatorAddress, newAssetMatcher); - __AccessControl_init(); _grantRole(DEFAULT_ADMIN_ROLE, admin); } @@ -139,8 +128,18 @@ contract Exchange is _setDefaultFeeReceiver(newDefaultFeeReceiver); } - function _skipFees(address from) internal view override returns (bool) { - return !hasRole(SKIP_FEES_ROLE, from); + /** + * @dev See {IERC165-supportsInterface}. + */ + function supportsInterface( + bytes4 interfaceId + ) public view virtual override(ERC165Upgradeable, AccessControlUpgradeable) returns (bool) { + return + ERC165Upgradeable.supportsInterface(interfaceId) || AccessControlUpgradeable.supportsInterface(interfaceId); + } + + function _applyFees(address from) internal view override returns (bool) { + return !hasRole(EXCHANGE_ADMIN_ROLE, from); } function _msgSender() @@ -156,14 +155,4 @@ contract Exchange is function _msgData() internal view override(ContextUpgradeable, ERC2771HandlerUpgradeable) returns (bytes calldata) { return ERC2771HandlerUpgradeable._msgData(); } - - /** - * @dev See {IERC165-supportsInterface}. - */ - function supportsInterface( - bytes4 interfaceId - ) public view virtual override(ERC165Upgradeable, AccessControlUpgradeable) returns (bool) { - return - ERC165Upgradeable.supportsInterface(interfaceId) || AccessControlUpgradeable.supportsInterface(interfaceId); - } } diff --git a/packages/marketplace/contracts/mocks/SimpleTest.sol b/packages/marketplace/contracts/mocks/SimpleTest.sol index 7a51dbf6a1..bcce885051 100644 --- a/packages/marketplace/contracts/mocks/SimpleTest.sol +++ b/packages/marketplace/contracts/mocks/SimpleTest.sol @@ -11,5 +11,5 @@ contract SimpleTest is TransferManager, TransferExecutor { return getRoyaltiesByAssetType(matchNft); } - function _skipFees(address from) internal virtual override returns (bool) {} + function _applyFees(address from) internal virtual override returns (bool) {} } diff --git a/packages/marketplace/contracts/transfer-manager/TransferManager.sol b/packages/marketplace/contracts/transfer-manager/TransferManager.sol index ba2b19707b..7beeb26800 100644 --- a/packages/marketplace/contracts/transfer-manager/TransferManager.sol +++ b/packages/marketplace/contracts/transfer-manager/TransferManager.sol @@ -122,7 +122,7 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager { function doTransfersWithFees(LibDeal.DealSide memory paymentSide, LibDeal.DealSide memory nftSide) internal { uint256 rest = paymentSide.asset.value; - if (_skipFees(paymentSide.from)) { + if (_applyFees(paymentSide.from)) { rest = transferRoyalties( paymentSide.asset.assetType, nftSide.asset.assetType, @@ -316,7 +316,7 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager { } } - function _skipFees(address from) internal virtual returns (bool); + function _applyFees(address from) internal virtual returns (bool); uint256[46] private __gap; } diff --git a/packages/marketplace/test/exchange/Exchange.test.ts b/packages/marketplace/test/exchange/Exchange.test.ts index 58776bb4d6..da9a6f5d13 100644 --- a/packages/marketplace/test/exchange/Exchange.test.ts +++ b/packages/marketplace/test/exchange/Exchange.test.ts @@ -840,7 +840,7 @@ describe('Exchange.sol', function () { ); }); - it('should execute a complete match order without fee and royalties for TSB seller', async function () { + it('should execute a complete match order without fee and royalties for privileged seller', async function () { const { ExchangeContractAsUser, ExchangeContractAsDeployer, @@ -866,11 +866,11 @@ describe('Exchange.sol', function () { // set up receiver await ERC721WithRoyaltyV2981.setRoyaltiesReceiver(1, deployer.address); - // grant Skip Fees role to seller - const SKIP_FEES_ROLE = - '0x9c14d84aa0a4264a5c33560cb08e43e3ed227d0565fa3d19078d0f01304516eb'; // keccak256("SKIP_FEES_ROLE") + // grant exchange admin role to seller + const EXCHANGE_ADMIN_ROLE = + '0x541943c4a49765b7940b4b1392c4b1f8ede6efd4e23572572987ae02e569a786'; // keccak256("EXCHANGE_ADMIN_ROLE") await ExchangeContractAsDeployer.connect(admin).grantRole( - SKIP_FEES_ROLE, + EXCHANGE_ADMIN_ROLE, taker.address );