Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add onlyInitializing and _disableInitializers #1179

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions packages/deploy/deploy/marketplace/03_deploy_asset_matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,13 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
const {deploy} = deployments;
const {deployer} = await getNamedAccounts();

// TODO: to be fetched from env?
const deployMeta = process.env.DEPLOY_META;
const exchangeContract = deployMeta ? 'ExchangeMeta' : 'Exchange';

const assetMatcher = await deploy('AssetMatcher', {
await deploy('AssetMatcher', {
from: deployer,
contract:
'@sandbox-smart-contracts/marketplace/contracts/exchange/AssetMatcher.sol:AssetMatcher',
log: true,
skipIfAlreadyDeployed: true,
});

await deployments.execute(
exchangeContract,
{from: deployer},
'setAssetMatcherContract',
assetMatcher.address
);
};
export default func;
func.tags = ['AssetMatcher', 'AssetMatcher_deploy'];
func.dependencies = ['Exchange'];
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {DeployFunction} from 'hardhat-deploy/types';
const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
const {deployments, getNamedAccounts} = hre;
const {deploy} = deployments;
const {deployer, upgradeAdmin, exchangeFeeRecipient} =
const {deployer, sandAdmin, upgradeAdmin, exchangeFeeRecipient} =
await getNamedAccounts();

let TRUSTED_FORWARDER = await deployments.getOrNull('TRUSTED_FORWARDER');
Expand All @@ -17,32 +17,28 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
}
const orderValidator = await deployments.get('OrderValidator');
const royaltiesRegistry = await deployments.get('RoyaltiesRegistry');
const assetMatcher = await deployments.get('AssetMatcher');

// TODO: Do we need oll the combinations of flags ? Can we have two deployments scripts with different tags each ?
// TODO: to be fetched from env?
const deployMeta = process.env.DEPLOY_META;
const nativeOrder = process.env.NATIVE_ORDER;
const metaNative = process.env.META_NATIVE;

const contract = deployMeta ? 'ExchangeMeta' : 'Exchange';
const newProtocolFeePrimary = 0;
const newProtocolFeeSecondary = 250;

await deploy('Exchange', {
from: deployer,
contract: `@sandbox-smart-contracts/marketplace/contracts/exchange/${contract}.sol:${contract}`,
contract: `@sandbox-smart-contracts/marketplace/contracts/exchange/Exchange.sol:Exchange`,
proxy: {
owner: upgradeAdmin,
proxyContract: 'OpenZeppelinTransparentProxy',
execute: {
methodName: '__Exchange_init',
args: [
sandAdmin,
TRUSTED_FORWARDER.address,
0,
250,
newProtocolFeePrimary,
newProtocolFeeSecondary,
exchangeFeeRecipient,
royaltiesRegistry,
royaltiesRegistry.address,
orderValidator.address,
nativeOrder,
metaNative,
assetMatcher.address,
],
},
upgradeIndex: 0,
Expand All @@ -53,4 +49,8 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
};
export default func;
func.tags = ['Exchange', 'Exchange_deploy'];
func.dependencies = ['RoyaltiesRegistry_deploy', 'OrderValidator_deploy'];
func.dependencies = [
'RoyaltiesRegistry_deploy',
'OrderValidator_deploy',
'AssetMatcher_deploy',
];
71 changes: 8 additions & 63 deletions packages/marketplace/contracts/exchange/AssetMatcher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,11 @@
pragma solidity 0.8.21;

import {IAssetMatcher, LibAsset} from "../interfaces/IAssetMatcher.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

/// @title AssetMatcher contract
/// @notice matchAssets function should calculate if Asset types match with each other
contract AssetMatcher is Ownable, IAssetMatcher {
contract AssetMatcher is IAssetMatcher {
bytes internal constant EMPTY = "";
mapping(bytes4 => address) internal matchers;

/// @notice event emitted when an AssetMacher is set
/// @param assetType represented by bytes4
/// @param matcher address of the matcher
event MatcherChange(bytes4 indexed assetType, address indexed matcher);

/// @notice set AssetMacher
/// @param assetType to be matched by the matcher contract
/// @param matcher address of the matcher
function setAssetMatcher(bytes4 assetType, address matcher) external onlyOwner {
matchers[assetType] = matcher;
emit MatcherChange(assetType, matcher);
}

/// @notice calculate if Asset types match with each other
/// @param leftAssetType to be matched with rightAssetType
Expand All @@ -31,55 +16,15 @@ contract AssetMatcher is Ownable, IAssetMatcher {
function matchAssets(
LibAsset.AssetType memory leftAssetType,
LibAsset.AssetType memory rightAssetType
) external view returns (LibAsset.AssetType memory) {
LibAsset.AssetType memory result = matchAssetOneSide(leftAssetType, rightAssetType);
if (result.assetClass == 0) {
return matchAssetOneSide(rightAssetType, leftAssetType);
} else {
return result;
}
}

function matchAssetOneSide(
LibAsset.AssetType memory leftAssetType,
LibAsset.AssetType memory rightAssetType
) private view returns (LibAsset.AssetType memory) {
bytes4 classLeft = leftAssetType.assetClass;
bytes4 classRight = rightAssetType.assetClass;
require(classLeft != LibAsset.ETH_ASSET_CLASS, "maker cannot transfer native token");
require(classRight != LibAsset.ETH_ASSET_CLASS, "taker cannot transfer native token");
if (classLeft == LibAsset.ERC20_ASSET_CLASS) {
if (classRight == LibAsset.ERC20_ASSET_CLASS) {
return simpleMatch(leftAssetType, rightAssetType);
}
return LibAsset.AssetType(0, EMPTY);
}
if (classLeft == LibAsset.ERC721_ASSET_CLASS) {
if (classRight == LibAsset.ERC721_ASSET_CLASS) {
return simpleMatch(leftAssetType, rightAssetType);
}
return LibAsset.AssetType(0, EMPTY);
}
if (classLeft == LibAsset.ERC1155_ASSET_CLASS) {
if (classRight == LibAsset.ERC1155_ASSET_CLASS) {
return simpleMatch(leftAssetType, rightAssetType);
}
return LibAsset.AssetType(0, EMPTY);
}
if (classLeft == LibAsset.BUNDLE) {
if (classRight == LibAsset.BUNDLE) {
return simpleMatch(leftAssetType, rightAssetType);
}
return LibAsset.AssetType(0, EMPTY);
}
address matcher = matchers[classLeft];
if (matcher != address(0)) {
return IAssetMatcher(matcher).matchAssets(leftAssetType, rightAssetType);
}
) external pure returns (LibAsset.AssetType memory) {
LibAsset.AssetClassType classLeft = leftAssetType.assetClass;
LibAsset.AssetClassType classRight = rightAssetType.assetClass;
require(classLeft != LibAsset.AssetClassType.INVALID_ASSET_CLASS, "not found IAssetMatcher");
require(classRight != LibAsset.AssetClassType.INVALID_ASSET_CLASS, "not found IAssetMatcher");
if (classLeft == classRight) {
return simpleMatch(leftAssetType, rightAssetType);
}
revert("not found IAssetMatcher");
return LibAsset.AssetType(LibAsset.AssetClassType.INVALID_ASSET_CLASS, EMPTY);
}

function simpleMatch(
Expand All @@ -91,6 +36,6 @@ contract AssetMatcher is Ownable, IAssetMatcher {
if (leftHash == rightHash) {
return leftAssetType;
}
return LibAsset.AssetType(0, EMPTY);
return LibAsset.AssetType(LibAsset.AssetClassType.INVALID_ASSET_CLASS, EMPTY);
}
}
6 changes: 6 additions & 0 deletions packages/marketplace/contracts/exchange/Exchange.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ contract Exchange is Initializable, AccessControlUpgradeable, ExchangeCore, Tran
/// @return hash for EXCHANGE_ADMIN_ROLE
bytes32 public constant EXCHANGE_ADMIN_ROLE = keccak256("EXCHANGE_ADMIN_ROLE");

/// @dev this protects the implementation contract from being initialized.
/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

/// @notice Exchange contract initializer
/// @param admin the admin user that can grant/revoke roles, etc.
/// @param newTrustedForwarder address for trusted forwarder that will execute meta transactions
Expand Down
47 changes: 18 additions & 29 deletions packages/marketplace/contracts/exchange/ExchangeCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
function __ExchangeCoreInitialize(
IOrderValidator newOrderValidatorAddress,
IAssetMatcher newAssetMatcher
) internal {
) internal onlyInitializing {
_setOrderValidatorContract(newOrderValidatorAddress);
_setAssetMatcherContract(newAssetMatcher);
}
Expand Down Expand Up @@ -122,20 +122,20 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
}

/// @dev function, validate orders
/// @param from the message sender
/// @param sender the message sender
/// @param orderLeft left order
/// @param signatureLeft order left signature
/// @param orderRight right order
/// @param signatureRight order right signature
function _validateOrders(
address from,
address sender,
LibOrder.Order memory orderLeft,
bytes memory signatureLeft,
LibOrder.Order memory orderRight,
bytes memory signatureRight
) internal view {
_validateFull(from, orderLeft, signatureLeft);
_validateFull(from, orderRight, signatureRight);
orderValidator.validate(orderLeft, signatureLeft, sender);
orderValidator.validate(orderRight, signatureRight, sender);
if (orderLeft.taker != address(0)) {
if (orderRight.maker != address(0)) require(orderRight.maker == orderLeft.taker, "leftOrder.taker failed");
}
Expand All @@ -145,11 +145,11 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
}

/// @notice matches valid orders and transfers their assets
/// @param from the message sender
/// @param sender the message sender
/// @param orderLeft the left order of the match
/// @param orderRight the right order of the match
function _matchAndTransfer(
address from,
address sender,
LibOrder.Order memory orderLeft,
LibOrder.Order memory orderRight
) internal {
Expand All @@ -162,7 +162,7 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
LibOrderDataGeneric.GenericOrderData memory leftOrderData,
LibOrderDataGeneric.GenericOrderData memory rightOrderData,
LibFill.FillResult memory newFill
) = _parseOrdersSetFillEmitMatch(from, orderLeft, orderRight);
) = _parseOrdersSetFillEmitMatch(sender, orderLeft, orderRight);

doTransfers(
LibDeal.DealSide({
Expand All @@ -180,14 +180,14 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
}

/// @notice parse orders with LibOrderDataGeneric parse() to get the order data, then create a new fill with setFillEmitMatch()
/// @param from the message sender
/// @param sender the message sender
/// @param orderLeft left order
/// @param orderRight right order
/// @return leftOrderData generic order data from left order
/// @return rightOrderData generic order data from right order
/// @return newFill fill result
function _parseOrdersSetFillEmitMatch(
address from,
address sender,
LibOrder.Order memory orderLeft,
LibOrder.Order memory orderRight
)
Expand All @@ -202,17 +202,17 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
bytes32 rightOrderKeyHash = LibOrder.hashKey(orderRight);

if (orderLeft.maker == address(0)) {
orderLeft.maker = from;
orderLeft.maker = sender;
}
if (orderRight.maker == address(0)) {
orderRight.maker = from;
orderRight.maker = sender;
}

leftOrderData = LibOrderDataGeneric.parse(orderLeft);
rightOrderData = LibOrderDataGeneric.parse(orderRight);

newFill = _setFillEmitMatch(
from,
sender,
orderLeft,
orderRight,
leftOrderKeyHash,
Expand All @@ -223,14 +223,14 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
}

/// @notice calculates fills for the matched orders and set them in "fills" mapping
/// @param from the message sender
/// @param sender the message sender
/// @param orderLeft left order of the match
/// @param orderRight right order of the match
/// @param leftMakeFill true if the left orders uses make-side fills, false otherwise
/// @param rightMakeFill true if the right orders uses make-side fills, false otherwise
/// @return newFill returns change in orders' fills by the match
function _setFillEmitMatch(
address from,
address sender,
LibOrder.Order memory orderLeft,
LibOrder.Order memory orderRight,
bytes32 leftOrderKeyHash,
Expand Down Expand Up @@ -261,7 +261,7 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
}

emit Match({
from: from,
from: sender,
leftHash: leftOrderKeyHash,
rightHash: rightOrderKeyHash,
newFill: newFill,
Expand Down Expand Up @@ -293,20 +293,9 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
LibOrder.Order memory orderRight
) internal view returns (LibAsset.AssetType memory makeMatch, LibAsset.AssetType memory takeMatch) {
makeMatch = assetMatcher.matchAssets(orderLeft.makeAsset.assetType, orderRight.takeAsset.assetType);
require(makeMatch.assetClass != 0, "assets don't match");
require(makeMatch.assetClass != LibAsset.AssetClassType.INVALID_ASSET_CLASS, "assets don't match");
takeMatch = assetMatcher.matchAssets(orderLeft.takeAsset.assetType, orderRight.makeAsset.assetType);
require(takeMatch.assetClass != 0, "assets don't match");
}

/// @notice full validation of an order
/// @param from the message sender
/// @param order LibOrder.Order
/// @param signature order signature
/// @dev first validate time if order start and end are within the block timestamp
/// @dev validate signature if maker is different from sender
function _validateFull(address from, LibOrder.Order memory order, bytes memory signature) internal view {
LibOrder.validateOrderTime(order);
orderValidator.validate(order, signature, from);
require(takeMatch.assetClass != LibAsset.AssetClassType.INVALID_ASSET_CLASS, "assets don't match");
}

uint256[49] private __gap;
Expand Down
16 changes: 11 additions & 5 deletions packages/marketplace/contracts/exchange/OrderValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pragma solidity 0.8.21;

import {LibOrder, LibAsset} from "../lib-order/LibOrder.sol";
import {LibOrder} from "../lib-order/LibOrder.sol";
import {IERC1271Upgradeable} from "@openzeppelin/contracts-upgradeable/interfaces/IERC1271Upgradeable.sol";
import {ECDSAUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";
import {AddressUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
Expand All @@ -18,6 +18,12 @@ contract OrderValidator is IOrderValidator, Initializable, EIP712Upgradeable, Wh

bytes4 internal constant MAGICVALUE = 0x1626ba7e;

/// @dev this protects the implementation contract from being initialized.
/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

/// @notice initializer for OrderValidator
/// @param newTsbOnly boolean to indicate that only The Sandbox tokens are accepted by the exchange contract
/// @param newPartners boolena to indicate that partner tokens are accepted by the exchange contract
Expand All @@ -39,10 +45,10 @@ contract OrderValidator is IOrderValidator, Initializable, EIP712Upgradeable, Wh
/// @param signature signature of order
/// @param sender order sender
function validate(LibOrder.Order memory order, bytes memory signature, address sender) public view {
if (order.makeAsset.assetType.assetClass != LibAsset.ETH_ASSET_CLASS) {
address makeToken = abi.decode(order.makeAsset.assetType.data, (address));
verifyWhiteList(makeToken);
}
LibOrder.validateOrderTime(order);

address makeToken = abi.decode(order.makeAsset.assetType.data, (address));
verifyWhiteList(makeToken);

if (order.salt == 0) {
if (order.maker != address(0)) {
Expand Down
6 changes: 6 additions & 0 deletions packages/marketplace/contracts/exchange/WhiteList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ contract WhiteList is IWhiteList, OwnableUpgradeable, AccessControlUpgradeable {
/// @param erc20List boolean indicating that there is a restriction for ERC20 tokens
event PermissionSetted(bool tsbOnly, bool partners, bool open, bool erc20List);

/// @dev this protects the implementation contract from being initialized.
/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

/// @notice initializer for WhiteList
/// @param newTsbOnly allows orders with The Sandbox token
/// @param newPartners allows orders with partner token
Expand Down
Loading
Loading