Skip to content

Commit

Permalink
Merge pull request #1215 from thesandboxgame/feat/marketplace-remove-…
Browse files Browse the repository at this point in the history
…duplicated-code

feat: simplify some duplicated code in transfer manager
  • Loading branch information
adjisb authored Oct 6, 2023
2 parents 7fd7c38 + b8238b0 commit 16b7213
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 73 deletions.
2 changes: 1 addition & 1 deletion packages/marketplace/contracts/mocks/SimpleTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {LibPart} from "../lib-part/LibPart.sol";

contract SimpleTest is TransferManager, TransferExecutor {
function getRoyaltiesByAssetTest(LibAsset.AssetType memory matchNft) external returns (LibPart.Part[] memory) {
return getRoyaltiesByAssetType(matchNft);
return _getRoyaltiesByAssetType(matchNft);
}

/// @dev returning false for mock contract
Expand Down
136 changes: 64 additions & 72 deletions packages/marketplace/contracts/transfer-manager/TransferManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

pragma solidity 0.8.21;

import {ERC165Upgradeable, IERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/ERC165Upgradeable.sol";
import {ERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/ERC165Upgradeable.sol";
import {IERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/IERC165Upgradeable.sol";
import {IRoyaltiesProvider} from "../interfaces/IRoyaltiesProvider.sol";
import {BpLibrary} from "../lib-bp/BpLibrary.sol";
import {IRoyaltyUGC} from "./interfaces/IRoyaltyUGC.sol";
Expand Down Expand Up @@ -66,6 +67,23 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
_setDefaultFeeReceiver(newDefaultFeeReceiver);
}

/// @notice executes transfers for 2 matched orders
/// @param left DealSide from the left order (see LibDeal.sol)
/// @param right DealSide from the right order (see LibDeal.sol)
/// @dev this is the main entry point, when used as a separated contract this method will be external
function doTransfers(DealSide memory left, DealSide memory right, LibAsset.FeeSide feeSide) internal override {
if (feeSide == LibAsset.FeeSide.LEFT) {
_doTransfersWithFees(left, right);
_transferPayouts(right.asset.assetType, right.asset.value, right.from, left.payouts);
} else if (feeSide == LibAsset.FeeSide.RIGHT) {
_doTransfersWithFees(right, left);
_transferPayouts(left.asset.assetType, left.asset.value, left.from, right.payouts);
} else {
_transferPayouts(left.asset.assetType, left.asset.value, left.from, right.payouts);
_transferPayouts(right.asset.assetType, right.asset.value, right.from, left.payouts);
}
}

/// @notice setter for royalty registry
/// @param newRoyaltiesRegistry address of new royalties registry
function _setRoyaltiesRegistry(IRoyaltiesProvider newRoyaltiesRegistry) internal {
Expand Down Expand Up @@ -96,30 +114,14 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
emit DefaultFeeReceiverSet(newDefaultFeeReceiver);
}

/// @notice executes transfers for 2 matched orders
/// @param left DealSide from the left order (see LibDeal.sol)
/// @param right DealSide from the right order (see LibDeal.sol)
function doTransfers(DealSide memory left, DealSide memory right, LibAsset.FeeSide feeSide) internal override {
if (feeSide == LibAsset.FeeSide.LEFT) {
doTransfersWithFees(left, right);
transferPayouts(right.asset.assetType, right.asset.value, right.from, left.payouts);
} else if (feeSide == LibAsset.FeeSide.RIGHT) {
doTransfersWithFees(right, left);
transferPayouts(left.asset.assetType, left.asset.value, left.from, right.payouts);
} else {
transferPayouts(left.asset.assetType, left.asset.value, left.from, right.payouts);
transferPayouts(right.asset.assetType, right.asset.value, right.from, left.payouts);
}
}

/// @notice executes the fee-side transfers (payment + fees)
/// @param paymentSide DealSide of the fee-side order
/// @param nftSide DealSide of the nft-side order
function doTransfersWithFees(DealSide memory paymentSide, DealSide memory nftSide) internal {
function _doTransfersWithFees(DealSide memory paymentSide, DealSide memory nftSide) internal {
uint256 rest = paymentSide.asset.value;

if (_applyFees(paymentSide.from)) {
rest = transferRoyalties(
rest = _transferRoyalties(
paymentSide.asset.assetType,
nftSide.asset.assetType,
nftSide.payouts,
Expand All @@ -131,32 +133,14 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
LibPart.Part[] memory origin = new LibPart.Part[](1);
origin[0].account = payable(defaultFeeReceiver);

bool primaryMarket = false;

// check if primary or secondary market
if (
nftSide.asset.assetType.assetClass == LibAsset.AssetClassType.ERC1155_ASSET_CLASS ||
nftSide.asset.assetType.assetClass == LibAsset.AssetClassType.ERC721_ASSET_CLASS
) {
(address token, uint256 tokenId) = abi.decode(nftSide.asset.assetType.data, (address, uint));
try IERC165Upgradeable(token).supportsInterface(INTERFACE_ID_IROYALTYUGC) returns (bool result) {
if (result) {
address creator = IRoyaltyUGC(token).getCreatorAddress(tokenId);
if (nftSide.from == creator) {
primaryMarket = true;
}
}
// solhint-disable-next-line no-empty-blocks
} catch {}
}

if (primaryMarket) {
address creator = _getCreator(nftSide.asset.assetType);
if (creator != address(0) && creator == nftSide.from) {
origin[0].value = uint96(protocolFeePrimary);
} else {
origin[0].value = uint96(protocolFeeSecondary);
}

(rest, ) = transferFees(
(rest, ) = _transferFees(
paymentSide.asset.assetType,
rest,
paymentSide.asset.value,
Expand All @@ -165,7 +149,7 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
);
}

transferPayouts(paymentSide.asset.assetType, rest, paymentSide.from, nftSide.payouts);
_transferPayouts(paymentSide.asset.assetType, rest, paymentSide.from, nftSide.payouts);
}

/// @notice transfer royalties. If there is only one royalties receiver and one address in payouts and they match,
Expand All @@ -177,46 +161,36 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
/// @param amount total amount of asset that is going to be transferred
/// @param from owner of the Asset to transfer
/// @return How much left after transferring royalties
function transferRoyalties(
function _transferRoyalties(
LibAsset.AssetType memory paymentAssetType,
LibAsset.AssetType memory nftAssetType,
LibPart.Part[] memory payouts,
uint256 rest,
uint256 amount,
address from
) internal returns (uint256) {
LibPart.Part[] memory royalties = getRoyaltiesByAssetType(nftAssetType);
LibPart.Part[] memory royalties = _getRoyaltiesByAssetType(nftAssetType);

if (
nftAssetType.assetClass == LibAsset.AssetClassType.ERC1155_ASSET_CLASS ||
nftAssetType.assetClass == LibAsset.AssetClassType.ERC721_ASSET_CLASS
) {
(address token, uint256 tokenId) = abi.decode(nftAssetType.data, (address, uint));
try IERC165Upgradeable(token).supportsInterface(INTERFACE_ID_IROYALTYUGC) returns (bool resultInterface) {
if (resultInterface) {
address creator = IRoyaltyUGC(token).getCreatorAddress(tokenId);
if (payouts.length == 1 && payouts[0].account == creator) {
require(royalties[0].value <= 5000, "Royalties are too high (>50%)");
return rest;
}
}
// solhint-disable-next-line no-empty-blocks
} catch {}
}
if (royalties.length == 1 && payouts.length == 1 && royalties[0].account == payouts[0].account) {
require(royalties[0].value <= 5000, "Royalties are too high (>50%)");
return rest;
if (payouts.length == 1) {
address creator = _getCreator(nftAssetType);
if (creator != address(0) && payouts[0].account == creator) {
require(royalties[0].value <= 5000, "Royalties are too high (>50%)");
return rest;
}
if (royalties.length == 1 && royalties[0].account == payouts[0].account) {
require(royalties[0].value <= 5000, "Royalties are too high (>50%)");
return rest;
}
}

(uint256 result, uint256 totalRoyalties) = transferFees(paymentAssetType, rest, amount, royalties, from);
(uint256 result, uint256 totalRoyalties) = _transferFees(paymentAssetType, rest, amount, royalties, from);
require(totalRoyalties <= 5000, "Royalties are too high (>50%)");
return result;
}

/// @notice calculates royalties by asset type.
/// @param nftAssetType NFT Asset Type to calculate royalties for
/// @return calculated royalties (Array of LibPart.Part)
function getRoyaltiesByAssetType(LibAsset.AssetType memory nftAssetType) internal returns (LibPart.Part[] memory) {
function _getRoyaltiesByAssetType(LibAsset.AssetType memory nftAssetType) internal returns (LibPart.Part[] memory) {
if (
nftAssetType.assetClass == LibAsset.AssetClassType.ERC1155_ASSET_CLASS ||
nftAssetType.assetClass == LibAsset.AssetClassType.ERC721_ASSET_CLASS
Expand All @@ -236,7 +210,7 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
/// @param from owner of the Asset to transfer
/// @return newRest how much left after transferring fees
/// @return totalFees total number of fees in bp
function transferFees(
function _transferFees(
LibAsset.AssetType memory assetType,
uint256 rest,
uint256 amount,
Expand All @@ -248,7 +222,7 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
for (uint256 i = 0; i < fees.length; ++i) {
totalFees = totalFees + fees[i].value;
uint256 feeValue;
(newRest, feeValue) = subFeeInBp(newRest, amount, fees[i].value);
(newRest, feeValue) = _subFeeInBp(newRest, amount, fees[i].value);
if (feeValue > 0) {
transfer(LibAsset.Asset(assetType, feeValue), from, fees[i].account);
}
Expand All @@ -260,7 +234,7 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
/// @param amount Amount of the asset to transfer
/// @param from Current owner of the asset
/// @param payouts List of payouts - receivers of the Asset
function transferPayouts(
function _transferPayouts(
LibAsset.AssetType memory assetType,
uint256 amount,
address from,
Expand Down Expand Up @@ -289,20 +263,20 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
/// @param value amount left from amount after fees are discounted
/// @param total total price for asset
/// @param feeInBp fee in basepoint to be deducted
function subFeeInBp(
function _subFeeInBp(
uint256 value,
uint256 total,
uint256 feeInBp
) internal pure returns (uint256 newValue, uint256 realFee) {
return subFee(value, total.bp(feeInBp));
return _subFee(value, total.bp(feeInBp));
}

/// @notice subtract fee from value
/// @param value from which the fees will be deducted
/// @param fee to deduct from value
/// @return newValue result from deduction, 0 if value < fee
/// @return realFee fee value if value > fee, otherwise return value input
function subFee(uint256 value, uint256 fee) internal pure returns (uint256 newValue, uint256 realFee) {
function _subFee(uint256 value, uint256 fee) internal pure returns (uint256 newValue, uint256 realFee) {
if (value > fee) {
newValue = value - fee;
realFee = fee;
Expand All @@ -312,7 +286,25 @@ abstract contract TransferManager is ERC165Upgradeable, ITransferManager {
}
}

/// @dev function deciding if the fees are applied or not, to be overriden
/// @notice return the creator of the token if the token supports INTERFACE_ID_IROYALTYUGC
/// @param assetType asset type
/// @return creator address or zero if is not able to retrieve it
function _getCreator(LibAsset.AssetType memory assetType) internal view returns (address creator) {
if (
assetType.assetClass == LibAsset.AssetClassType.ERC1155_ASSET_CLASS ||
assetType.assetClass == LibAsset.AssetClassType.ERC721_ASSET_CLASS
) {
(address token, uint256 tokenId) = abi.decode(assetType.data, (address, uint));
try IERC165Upgradeable(token).supportsInterface(INTERFACE_ID_IROYALTYUGC) returns (bool result) {
if (result) {
creator = IRoyaltyUGC(token).getCreatorAddress(tokenId);
}
// solhint-disable-next-line no-empty-blocks
} catch {}
}
}

/// @notice function deciding if the fees are applied or not, to be overriden
/// @param from address to check
function _applyFees(address from) internal virtual returns (bool);

Expand Down

1 comment on commit 16b7213

@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

86.80%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
packages/marketplace/contracts/exchange
   Exchange.sol79.71%70%87.50%86.96%174, 63, 79, 89, 89, 89, 89–90, 90, 90–91, 98
   ExchangeCore.sol84.81%61.54%100%95.45%118–119, 119, 119, 122, 188, 190, 194, 215–216, 64, 94
   OrderValidator.sol89.29%84.62%100%92%41, 71–72, 72, 72, 76
   WhiteList.sol97.62%93.75%100%100%64
packages/marketplace/contracts/exchange/libraries
   LibFill.sol60.87%33.33%75%69.23%32–33, 58, 68–69, 69, 69–70
   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
packages/marketplace/contracts/interfaces
   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
   LibOrder.sol73.33%50%100%100%64, 64, 66, 66
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.sol90.40%85.29%100%90.79%166–167, 170–171, 212, 216, 247, 250, 256, 259, 276, 60
packages/marketplace/contracts/transfer-manager
   TransferExecutor.sol90.48%75%100%100%24, 30
   TransferManager.sol80.41%65.38%100%86.75%100–101, 174, 177, 180–181, 181, 181–182, 186, 194, 201–202, 226, 243, 247–249, 249, 249–251, 256–257, 280, 284–285, 293, 63
packages/marketplace/contracts/transfer-manager/interfaces
   IRoyaltyUGC.sol100%100%100%100%
   ITransferExecutor.sol100%100%100%100%
   ITransferManager.sol100%100%100%100%

Please sign in to comment.