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

N-06 Gas Optimizations #1601

8 changes: 4 additions & 4 deletions packages/marketplace/contracts/ExchangeCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ abstract contract ExchangeCore is Initializable, ITransferManager {
uint256 len = matchedOrders.length;
require(len > 0, "ExchangeMatch cannot be empty");
require(len <= matchOrdersLimit, "too many ExchangeMatch");
for (uint256 i; i < len; i++) {
for (uint256 i; i < len; ++i) {
ExchangeMatch calldata m = matchedOrders[i];
_validateOrders(sender, m.orderLeft, m.signatureLeft, m.orderRight, m.signatureRight);
_matchAndTransfer(sender, m.orderLeft, m.orderRight);
Expand All @@ -141,15 +141,15 @@ abstract contract ExchangeCore is Initializable, ITransferManager {
LibOrder.Order memory orderRight,
bytes memory signatureRight
) internal view {
// validate must force order.maker != address(0)
orderValidator.validate(orderLeft, signatureLeft, sender);
orderValidator.validate(orderRight, signatureRight, sender);
if (orderLeft.taker != address(0)) {
require(orderRight.maker == orderLeft.taker, "leftOrder.taker failed");
}
if (orderRight.taker != address(0)) {
require(orderRight.taker == orderLeft.maker, "rightOrder.taker failed");
}
// validate must force order.maker != address(0)
orderValidator.validate(orderLeft, signatureLeft, sender);
orderValidator.validate(orderRight, signatureRight, sender);
}

/// @notice Matches valid orders and transfers the associated assets.
Expand Down
7 changes: 5 additions & 2 deletions packages/marketplace/contracts/OrderValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,14 @@ contract OrderValidator is IOrderValidator, Initializable, EIP712Upgradeable, ER
address makeToken;
if (asset.assetType.assetClass == LibAsset.AssetClass.BUNDLE) {
LibAsset.Bundle memory bundle = LibAsset.decodeBundle(asset.assetType);
for (uint256 i; i < bundle.bundledERC721.length; i++) {
uint256 bundledERC721Length = bundle.bundledERC721.length;
for (uint256 i; i < bundledERC721Length; ++i) {
makeToken = bundle.bundledERC721[i].erc721Address;
_verifyWhitelistsRoles(makeToken);
}
for (uint256 i; i < bundle.bundledERC1155.length; i++) {

uint256 bundledERC1155Length = bundle.bundledERC1155.length;
for (uint256 i; i < bundledERC1155Length; ++i) {
makeToken = bundle.bundledERC1155[i].erc1155Address;
_verifyWhitelistsRoles(makeToken);
}
Expand Down
5 changes: 3 additions & 2 deletions packages/marketplace/contracts/RoyaltiesRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ contract RoyaltiesRegistry is OwnableUpgradeable, IRoyaltiesProvider, ERC165Upgr
_setRoyaltiesType(token, RoyaltiesType.BY_TOKEN, address(0));
uint256 sumRoyalties = 0;
delete royaltiesByToken[token];
for (uint256 i = 0; i < royalties.length; ++i) {
uint256 royaltiesLength = royalties.length;
for (uint256 i = 0; i < royaltiesLength; ++i) {
require(royalties[i].account != address(0x0), "recipient should be present");
require(royalties[i].basisPoints != 0, "basisPoints should be > 0");
royaltiesByToken[token].royalties.push(royalties[i]);
Expand Down Expand Up @@ -253,7 +254,7 @@ contract RoyaltiesRegistry is OwnableUpgradeable, IRoyaltiesProvider, ERC165Upgr
uint256 multiRecipientsLength = multiRecipients.length;
Part[] memory royalties = new Part[](multiRecipientsLength);
uint256 sum = 0;
for (uint256 i; i < multiRecipientsLength; i++) {
for (uint256 i; i < multiRecipientsLength; ++i) {
Recipient memory splitRecipient = multiRecipients[i];
royalties[i].account = splitRecipient.recipient;
uint256 splitAmount = (splitRecipient.bps * royaltyAmount) / WEIGHT_VALUE;
Expand Down
33 changes: 17 additions & 16 deletions packages/marketplace/contracts/TransferManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ abstract contract TransferManager is Initializable, ITransferManager {
// Transfer NFT or left side if FeeSide.NONE
// NFT transfer when exchanging more than one bundle of ERC1155s
if (nftSide.asset.assetType.assetClass == LibAsset.AssetClass.BUNDLE && nftSide.asset.value > 1) {
for (uint256 i = 0; i < nftSide.asset.value; i++) {
for (uint256 i = 0; i < nftSide.asset.value; ++i) {
_transfer(nftSide.asset, nftSide.account, paymentSideRecipient);
}
} else {
Expand Down Expand Up @@ -299,10 +299,11 @@ abstract contract TransferManager is Initializable, ITransferManager {
uint256 feeSecondary,
LibAsset.Bundle memory bundle
) internal returns (uint256) {
for (uint256 i; i < bundle.bundledERC721.length; i++) {
uint256 bundledERC721Length = bundle.bundledERC721.length;
for (uint256 i; i < bundledERC721Length; ++i) {
address token = bundle.bundledERC721[i].erc721Address;
uint256 idLength = bundle.bundledERC721[i].ids.length;
for (uint256 j; j < idLength; j++) {
for (uint256 j; j < idLength; ++j) {
remainder = _processSingleAsset(
paymentSide,
nftSide,
Expand All @@ -328,13 +329,13 @@ abstract contract TransferManager is Initializable, ITransferManager {
uint256 feeSecondary,
LibAsset.Bundle memory bundle
) internal returns (uint256) {
for (uint256 i; i < bundle.bundledERC1155.length; i++) {
for (uint256 i; i < bundle.bundledERC1155.length; ++i) {
address token = bundle.bundledERC1155[i].erc1155Address;
uint256 idLength = bundle.bundledERC1155[i].ids.length;
require(idLength == bundle.bundledERC1155[i].supplies.length, "ERC1155 array error");

for (uint256 j; j < idLength; j++) {
for (uint256 k = 0; k < nftSide.asset.value; k++) {
for (uint256 j; j < idLength; ++j) {
for (uint256 k = 0; k < nftSide.asset.value; ++k) {
remainder = _processSingleAsset(
paymentSide,
nftSide,
Expand All @@ -360,7 +361,7 @@ abstract contract TransferManager is Initializable, ITransferManager {
LibAsset.Bundle memory bundle
) internal returns (uint256) {
uint256 quadSize = bundle.quads.xs.length;
for (uint256 i = 0; i < quadSize; i++) {
for (uint256 i = 0; i < quadSize; ++i) {
uint256 size = bundle.quads.sizes[i];
uint256 x = bundle.quads.xs[i];
uint256 y = bundle.quads.ys[i];
Expand Down Expand Up @@ -500,8 +501,8 @@ abstract contract TransferManager is Initializable, ITransferManager {
address recipient
) internal returns (uint256) {
uint256 totalRoyalties;
uint256 len = royalties.length;
for (uint256 i; i < len; i++) {
uint256 royaltiesLength = royalties.length;
for (uint256 i; i < royaltiesLength; ++i) {
IRoyaltiesProvider.Part memory r = royalties[i];
totalRoyalties += r.basisPoints;
if (r.account == recipient) {
Expand Down Expand Up @@ -581,22 +582,22 @@ abstract contract TransferManager is Initializable, ITransferManager {
_transferERC1155(token, from, to, tokenId, asset.value);
} else if (asset.assetType.assetClass == LibAsset.AssetClass.BUNDLE) {
LibAsset.Bundle memory bundle = LibAsset.decodeBundle(asset.assetType);
uint256 erc721Length = bundle.bundledERC721.length;
uint256 erc1155Length = bundle.bundledERC1155.length;
uint256 bundledERC721Length = bundle.bundledERC721.length;
uint256 bundledERC1155Length = bundle.bundledERC1155.length;
uint256 quadsLength = bundle.quads.xs.length;
if (erc721Length > 0 || quadsLength > 0) require(asset.value == 1, "bundle value error");
for (uint256 i; i < erc721Length; i++) {
if (bundledERC721Length > 0 || quadsLength > 0) require(asset.value == 1, "bundle value error");
for (uint256 i; i < bundledERC721Length; ++i) {
address token = bundle.bundledERC721[i].erc721Address;
uint256 idLength = bundle.bundledERC721[i].ids.length;
for (uint256 j; j < idLength; j++) {
for (uint256 j; j < idLength; ++j) {
_transferERC721(token, from, to, bundle.bundledERC721[i].ids[j]);
}
}
for (uint256 i; i < erc1155Length; i++) {
for (uint256 i; i < bundledERC1155Length; ++i) {
address token = bundle.bundledERC1155[i].erc1155Address;
uint256 idLength = bundle.bundledERC1155[i].ids.length;
require(idLength == bundle.bundledERC1155[i].supplies.length, "ERC1155 array error");
for (uint256 j; j < idLength; j++) {
for (uint256 j; j < idLength; ++j) {
_transferERC1155(
token,
from,
Expand Down
5 changes: 3 additions & 2 deletions packages/marketplace/contracts/Whitelist.sol
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,9 @@ contract Whitelist is IWhitelist, Initializable, AccessControlEnumerableUpgradea
/// @param roles List of role identifiers.
/// @param permissions List of desired status for each role.
function _setRolesEnabled(bytes32[] memory roles, bool[] memory permissions) internal {
require(roles.length == permissions.length, "Mismatched input lengths");
for (uint256 i = 0; i < roles.length; ++i) {
uint256 rolesLength = roles.length;
require(rolesLength == permissions.length, "Mismatched input lengths");
for (uint256 i = 0; i < rolesLength; ++i) {
if (isRoleEnabled(roles[i]) != permissions[i]) {
if (permissions[i]) {
_enableRole(roles[i]);
Expand Down
15 changes: 10 additions & 5 deletions packages/marketplace/contracts/libraries/LibAsset.sol
Original file line number Diff line number Diff line change
Expand Up @@ -165,20 +165,25 @@ library LibAsset {
uint256 collectiveBundlePrice = 0;

// total price of all bundled ERC721 assets
for (uint256 i = 0; i < priceDistribution.erc721Prices.length; i++) {
for (uint256 j = 0; j < priceDistribution.erc721Prices[i].length; j++)
uint256 erc721PricesLength = priceDistribution.erc721Prices.length;
for (uint256 i = 0; i < erc721PricesLength; ++i) {
uint256 erc721PricesInnerLength = priceDistribution.erc721Prices[i].length;
for (uint256 j = 0; j < erc721PricesInnerLength; ++j)
collectiveBundlePrice += priceDistribution.erc721Prices[i][j];
}

// total price of all bundled ERC1155 assets
for (uint256 i = 0; i < priceDistribution.erc1155Prices.length; i++) {
for (uint256 j = 0; j < priceDistribution.erc1155Prices[i].length; j++) {
uint256 erc1155PricesLength = priceDistribution.erc1155Prices.length;
for (uint256 i = 0; i < erc1155PricesLength; ++i) {
uint256 erc1155PricesInnerLength = priceDistribution.erc1155Prices[i].length;
for (uint256 j = 0; j < erc1155PricesInnerLength; ++j) {
collectiveBundlePrice += priceDistribution.erc1155Prices[i][j];
}
}

// total price of all bundled Quad assets
for (uint256 i = 0; i < priceDistribution.quadPrices.length; i++) {
uint256 quadPricesLength = priceDistribution.quadPrices.length;
for (uint256 i = 0; i < quadPricesLength; ++i) {
collectiveBundlePrice += priceDistribution.quadPrices[i];
}

Expand Down
109 changes: 103 additions & 6 deletions packages/marketplace/test/exchange/Bundle.behavior.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,47 @@ import {
import {hashKey, OrderDefault, signOrder, Order} from '../utils/order.ts';
import {ZeroAddress, Contract, Signer} from 'ethers';

function calculateFinalPrice(
priceDistribution: PriceDistribution,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
protocolFees: any
): number {
let finalPrice = 0;

// ERC721 assets
for (let i = 0; i < priceDistribution.erc721Prices.length; i++) {
for (let j = 0; j < priceDistribution.erc721Prices[i].length; j++) {
const assetPrice = priceDistribution.erc721Prices[i][j];
const protocolFee = protocolFees.erc721ProtocolFees[i][j];

const deductedProtocolFee = (assetPrice * protocolFee) / 10000;
finalPrice += assetPrice - deductedProtocolFee;
}
}

// ERC1155 assets
for (let i = 0; i < priceDistribution.erc1155Prices.length; i++) {
for (let j = 0; j < priceDistribution.erc1155Prices[i].length; j++) {
const assetPrice = priceDistribution.erc1155Prices[i][j];
const protocolFee = protocolFees.erc1155ProtocolFees[i][j];

const deductedProtocolFee = (assetPrice * protocolFee) / 10000;
finalPrice += assetPrice - deductedProtocolFee;
}
}

// Quad assets
for (let i = 0; i < priceDistribution.quadPrices.length; i++) {
const assetPrice = priceDistribution.quadPrices[i];
const protocolFee = protocolFees.quadProtocolFees[i];

const deductedProtocolFee = (assetPrice * protocolFee) / 10000;
finalPrice += assetPrice - deductedProtocolFee;
}

return finalPrice;
}

// eslint-disable-next-line mocha/no-exports
export function shouldMatchOrdersForBundle() {
describe('Exchange MatchOrders for Bundle', function () {
Expand Down Expand Up @@ -246,8 +287,19 @@ export function shouldMatchOrdersForBundle() {
await ExchangeContractAsUser.fills(hashKey(orderRight))
).to.be.equal(1);

const protocolFees = {
erc721ProtocolFees: [[protocolFeeSecondary]],
erc1155ProtocolFees: [[protocolFeeSecondary]],
quadProtocolFees: [],
};

const expectedFinalReturn = calculateFinalPrice(
priceDistribution,
protocolFees
);

expect(await ERC20Contract.balanceOf(makerAddress)).to.be.equal(
9750000000 // 10000000000 - protocolFee
expectedFinalReturn // 10000000000 - protocolFee
);

expect(await ERC20Contract.balanceOf(takerAddress)).to.be.equal(
Expand Down Expand Up @@ -423,8 +475,19 @@ export function shouldMatchOrdersForBundle() {
await ExchangeContractAsUser.fills(hashKey(orderRight)) // newFill.leftValue
).to.be.equal(1);

const protocolFees = {
erc721ProtocolFees: [[]],
erc1155ProtocolFees: [[protocolFeeSecondary]],
quadProtocolFees: [],
};

const expectedFinalReturn = calculateFinalPrice(
priceDistribution,
protocolFees
);

expect(await ERC20Contract.balanceOf(makerAddress)).to.be.equal(
9750000000
expectedFinalReturn
);

expect(await ERC20Contract.balanceOf(taker)).to.be.equal(20000000000);
Expand Down Expand Up @@ -618,8 +681,20 @@ export function shouldMatchOrdersForBundle() {
expect(await ERC20Contract.balanceOf(takerAddress)).to.be.equal(
10000000000
);

const protocolFees = {
erc721ProtocolFees: [[]],
erc1155ProtocolFees: [[protocolFeeSecondary]],
quadProtocolFees: [],
};

const expectedFinalReturn = calculateFinalPrice(
priceDistribution,
protocolFees
);

expect(await ERC20Contract.balanceOf(makerAddress)).to.be.equal(
19500000000
2 * expectedFinalReturn
);

expect(
Expand Down Expand Up @@ -721,8 +796,19 @@ export function shouldMatchOrdersForBundle() {
await ExchangeContractAsUser.fills(hashKey(rightOrderForFirstMatch)) // newFill.leftValue
).to.be.equal(1);

const protocolFees = {
erc721ProtocolFees: [[]],
erc1155ProtocolFees: [[protocolFeeSecondary]],
quadProtocolFees: [],
};

const expectedFinalReturn = calculateFinalPrice(
priceDistribution,
protocolFees
);

expect(await ERC20Contract.balanceOf(makerAddress)).to.be.equal(
9750000000
expectedFinalReturn
);
expect(await ERC20Contract.balanceOf(taker)).to.be.equal(20000000000);

Expand Down Expand Up @@ -775,7 +861,7 @@ export function shouldMatchOrdersForBundle() {
).to.be.equal(1);

expect(await ERC20Contract.balanceOf(makerAddress)).to.be.equal(
19500000000
2 * expectedFinalReturn
);
expect(await ERC20Contract.balanceOf(taker)).to.be.equal(10000000000);
0;
Expand Down Expand Up @@ -966,8 +1052,19 @@ export function shouldMatchOrdersForBundle() {
await ExchangeContractAsUser.fills(hashKey(orderRight))
).to.be.equal(1);

const protocolFees = {
erc721ProtocolFees: [[protocolFeeSecondary]],
erc1155ProtocolFees: [[protocolFeeSecondary]],
quadProtocolFees: [protocolFeeSecondary, protocolFeeSecondary],
};

const expectedFinalReturn = calculateFinalPrice(
priceDistribution,
protocolFees
);

expect(await ERC20Contract.balanceOf(makerAddress)).to.be.equal(
9750000000 // 10000000000 - protocolFee
expectedFinalReturn // 10000000000 - protocolFee
);

expect(await ERC20Contract.balanceOf(takerAddress)).to.be.equal(
Expand Down
Loading
Loading