Skip to content

Commit

Permalink
fix: function ordering
Browse files Browse the repository at this point in the history
  • Loading branch information
capedcrusader21 committed Oct 3, 2023
1 parent e9d46e3 commit fe9d1be
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 34 deletions.
41 changes: 15 additions & 26 deletions packages/marketplace/contracts/exchange/Exchange.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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");
Expand All @@ -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() {
Expand Down Expand Up @@ -64,14 +53,14 @@ contract Exchange is
IAssetMatcher newAssetMatcher
) external initializer {
__ERC2771Handler_init(newTrustedForwarder);
__AccessControl_init();
__TransferManager_init_unchained(
newProtocolFeePrimary,
newProtocolFeeSecondary,
newDefaultFeeReceiver,
newRoyaltiesProvider
);
__ExchangeCoreInitialize(orderValidatorAddress, newAssetMatcher);
__AccessControl_init();
_grantRole(DEFAULT_ADMIN_ROLE, admin);
}

Expand Down Expand Up @@ -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()
Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion packages/marketplace/contracts/mocks/SimpleTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
10 changes: 5 additions & 5 deletions packages/marketplace/test/exchange/Exchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
);

Expand Down

1 comment on commit fe9d1be

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage for this commit

75.18%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
packages/marketplace/contracts/exchange
   AssetMatcher.sol100%100%100%100%
   Exchange.sol87.27%85%86.67%90%156, 54, 79, 79–80
   ExchangeCore.sol78.76%59.09%100%89.66%116, 139–140, 140, 140, 140, 140, 143, 143, 204–205, 207–208, 245, 247–249, 255–257, 280–281, 298, 76
   OrderValidator.sol69.81%53.85%100%81.82%38, 54, 61–62, 62, 62, 72, 82–83, 88, 90, 90, 90, 90–91, 93
   WhiteList.sol97.73%93.75%100%100%57
packages/marketplace/contracts/exchange/libraries
   LibFill.sol55%33.33%66.67%63.64%48–49, 61, 71–72, 72, 72–73
   LibOrderDataGeneric.sol22.81%22.22%40%20.59%16–23, 25–26, 28, 41, 43, 43, 43–44, 46–47, 50, 50, 50–51, 53, 56, 56, 56–57, 59, 62, 66, 68, 68, 68–70, 73, 82, 82, 82–84
packages/marketplace/contracts/interfaces
   IAssetMatcher.sol100%100%100%100%
   IOrderValidator.sol100%100%100%100%
   IRoyaltiesProvider.sol100%100%100%100%
   IWhiteList.sol100%100%100%100%
packages/marketplace/contracts/lib-asset
   LibAsset.sol100%100%100%100%
packages/marketplace/contracts/lib-bp
   BpLibrary.sol100%100%100%100%
packages/marketplace/contracts/lib-order
   LibMath.sol27.50%18.75%50%30%100–103, 17–18, 33–34, 50, 50, 50–51, 72, 72, 72–73, 75, 88, 88, 88–89, 93, 93, 93, 93, 93, 97
   LibOrder.sol66.67%50%100%72.73%105, 105, 107, 107, 41–43, 54, 66
   LibOrderData.sol100%100%100%100%
packages/marketplace/contracts/lib-part
   LibPart.sol0%100%0%0%21
packages/marketplace/contracts/royalties
   IERC2981.sol100%100%100%100%
   LibRoyalties2981.sol78.57%50%100%88.89%19–20, 23
packages/marketplace/contracts/royalties-registry
   IMultiRoyaltyRecipients.sol100%100%100%100%
   RoyaltiesRegistry.sol79.20%79.41%100%75%166–167, 170–171, 212, 216, 232–233, 236–244, 247, 247, 247–248, 250, 256, 259, 276, 60
packages/marketplace/contracts/transfer-manager
   TransferExecutor.sol75.76%64.29%100%81.25%49–51, 58–59, 63, 66–67
   TransferManager.sol82.91%72.41%100%87.50%110, 114–115, 203, 210–211, 211, 211–212, 216, 256, 273, 277–279, 279, 279–281, 286–287, 310, 314–315, 62, 82–83
packages/marketplace/contracts/transfer-manager/interfaces
   IRoyaltyUGC.sol100%100%100%100%
   ITransferExecutor.sol100%100%100%100%
   ITransferManager.sol100%100%100%100%
packages/marketplace/contracts/transfer-manager/lib
   LibDeal.sol100%100%100%100%
   LibFeeSide.sol44.44%37.50%100%44.44%21, 24, 24, 24–25, 27, 27, 27–28, 30
   LibTransfer.sol0%0%0%0%7–8, 8, 8

Please sign in to comment.