From 1fef507362b575c69c26e71beb87628c617d2e01 Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 9 Jul 2024 20:35:55 +0100 Subject: [PATCH 1/7] Add fee flow-1 from SC-1198 --- contracts/extensions/FeeTaker.sol | 54 ++++++++++-- .../extensions/FeeTakerAmountCalculator.sol | 82 +++++++++++++++++ test/FeeTaker.js | 87 +++++++++++++++++-- 3 files changed, 211 insertions(+), 12 deletions(-) create mode 100644 contracts/extensions/FeeTakerAmountCalculator.sol diff --git a/contracts/extensions/FeeTaker.sol b/contracts/extensions/FeeTaker.sol index 345d6b4a..6dcfa47c 100644 --- a/contracts/extensions/FeeTaker.sol +++ b/contracts/extensions/FeeTaker.sol @@ -55,30 +55,72 @@ contract FeeTaker is IPostInteraction, Ownable { */ receive() external payable {} + /** + * @dev Validates whether the resolver is whitelisted. + * @param whitelist Whitelist is tightly packed struct of the following format: + * ``` + * (bytes10)[N] resolversAddresses; + * ``` + * Only 10 lowest bytes of the resolver address are used for comparison. + * @param resolver The resolver to check. + * @return Whether the resolver is whitelisted. + */ + function _isWhitelisted(bytes calldata whitelist, address resolver) internal view virtual returns (bool) { + unchecked { + uint80 maskedResolverAddress = uint80(uint160(resolver)); + uint256 size = whitelist.length / 10; + for (uint256 i = 0; i < size; i++) { + uint80 whitelistedAddress = uint80(bytes10(whitelist[:10])); + if (maskedResolverAddress == whitelistedAddress) { + return true; + } + whitelist = whitelist[10:]; + } + return false; + } + } + /** * @notice See {IPostInteraction-postInteraction}. * @dev Takes the fee in taking tokens and transfers the rest to the maker. * `extraData` consists of: - * 2 bytes — fee percentage (in 1e5) + * 2 bytes — integrator fee percentage (in 1e5) + * 2 bytes — resolver fee percentage (in 1e5) * 20 bytes — fee recipient + * 1 byte - taker whitelist size + * (bytes10)[N] — taker whitelist * 20 bytes — receiver of taking tokens (optional, if not set, maker is used) */ function postInteraction( IOrderMixin.Order calldata order, bytes calldata /* extension */, bytes32 /* orderHash */, - address /* taker */, + address taker, uint256 /* makingAmount */, uint256 takingAmount, uint256 /* remainingMakingAmount */, bytes calldata extraData ) external onlyLimitOrderProtocol { - uint256 fee = takingAmount * uint256(uint16(bytes2(extraData))) / _FEE_BASE; - address feeRecipient = address(bytes20(extraData[2:22])); + uint256 integratorFeePercent = uint256(uint16(bytes2(extraData))); + uint256 resolverFeePercent = uint256(uint16(bytes2(extraData[2:]))); + + uint256 whitelistSize = uint8(extraData[24]); + uint256 indexThroughWhitelist = 25 + whitelistSize * 10; + bytes calldata whitelist = extraData[25:indexThroughWhitelist]; + + if (!_isWhitelisted(whitelist, taker)) { + resolverFeePercent *= 2; + } + + uint256 userTakingAmount = _FEE_BASE * takingAmount / (_FEE_BASE + integratorFeePercent + resolverFeePercent); + uint256 integratorFee = userTakingAmount * integratorFeePercent / _FEE_BASE; + uint256 resolverFee = userTakingAmount * resolverFeePercent / _FEE_BASE; + address feeRecipient = address(bytes20(extraData[4:24])); + uint256 fee = integratorFee + resolverFee; address receiver = order.maker.get(); - if (extraData.length > 22) { - receiver = address(bytes20(extraData[22:42])); + if (extraData.length > indexThroughWhitelist) { + receiver = address(bytes20(extraData[indexThroughWhitelist:indexThroughWhitelist + 20])); } bool isEth = order.takerAsset.get() == address(_WETH) && order.makerTraits.unwrapWeth(); diff --git a/contracts/extensions/FeeTakerAmountCalculator.sol b/contracts/extensions/FeeTakerAmountCalculator.sol new file mode 100644 index 00000000..4626f61e --- /dev/null +++ b/contracts/extensions/FeeTakerAmountCalculator.sol @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.23; + +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/utils/math/Math.sol"; +import "../libraries/AmountCalculatorLib.sol"; +import "../interfaces/IAmountGetter.sol"; + +contract FeeTakerAmountCalculator is IAmountGetter { + using Math for uint256; + + /// @dev Allows fees in range [1e-5, 0.65535] + uint256 internal constant _FEE_BASE = 1e5; + + function getMakingAmount( + IOrderMixin.Order calldata order, + bytes calldata /* extension */, + bytes32 /* orderHash */, + address /* taker */, + uint256 takingAmount, + uint256 /* remainingMakingAmount */, + bytes calldata /* extraData */ + ) external pure returns (uint256) { + return AmountCalculatorLib.getMakingAmount(order.makingAmount, order.takingAmount, takingAmount); + } + + /** + * @dev Validates whether the resolver is whitelisted. + * @param whitelist Whitelist is tightly packed struct of the following format: + * ``` + * (bytes10)[N] resolversAddresses; + * ``` + * Only 10 lowest bytes of the resolver address are used for comparison. + * @param resolver The resolver to check. + * @return Whether the resolver is whitelisted. + */ + function _isWhitelisted(bytes calldata whitelist, address resolver) internal view virtual returns (bool) { + unchecked { + uint80 maskedResolverAddress = uint80(uint160(resolver)); + uint256 size = whitelist.length / 10; + for (uint256 i = 0; i < size; i++) { + uint80 whitelistedAddress = uint80(bytes10(whitelist[:10])); + if (maskedResolverAddress == whitelistedAddress) { + return true; + } + whitelist = whitelist[10:]; + } + return false; + } + } + + /** + * @dev Calculate takingAmount with fee. + * `extraData` consists of: + * 1 byte - taker whitelist size + * (bytes10)[N] — taker whitelist + * 2 bytes — integrator fee percentage (in 1e5) + * 2 bytes — resolver fee percentage (in 1e5) + */ + function getTakingAmount( + IOrderMixin.Order calldata order, + bytes calldata /* extension */, + bytes32 /* orderHash */, + address taker, + uint256 makingAmount, + uint256 /* remainingMakingAmount */, + bytes calldata extraData + ) external view returns (uint256) { + uint256 calculatedTakingAmount = order.takingAmount; + uint256 indexThroughWhitelist = 1 + uint256(uint8(extraData[0]))*10; + if (!_isWhitelisted(extraData[1:indexThroughWhitelist], taker)) { + uint256 integratorFee = uint256(uint16(bytes2(extraData[indexThroughWhitelist:indexThroughWhitelist+2]))); + uint256 resolverFee = uint256(uint16(bytes2(extraData[indexThroughWhitelist+2:indexThroughWhitelist+4]))); + uint256 userTakingAmount = _FEE_BASE * order.takingAmount / (_FEE_BASE + integratorFee + resolverFee); + + calculatedTakingAmount += userTakingAmount * resolverFee / _FEE_BASE; + } + return (calculatedTakingAmount * makingAmount).ceilDiv(order.makingAmount); + } +} + diff --git a/test/FeeTaker.js b/test/FeeTaker.js index fa0d3735..168452fc 100644 --- a/test/FeeTaker.js +++ b/test/FeeTaker.js @@ -32,7 +32,10 @@ describe('FeeTaker', function () { const FeeTaker = await ethers.getContractFactory('FeeTaker'); const feeTaker = await FeeTaker.deploy(swap, weth, addr); - return { dai, weth, inch, swap, chainId, feeTaker }; + const FeeTakerAmountCalculator = await ethers.getContractFactory('FeeTakerAmountCalculator'); + const feeTakerAmountCalculator = await FeeTakerAmountCalculator.deploy(); + + return { dai, weth, inch, swap, chainId, feeTaker, feeTakerAmountCalculator }; }; it('should send all tokens to the maker with 0 fee', async function () { @@ -99,14 +102,15 @@ describe('FeeTaker', function () { await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2, addr3], [-takingAmount, 0, 0, takingAmount]); }); - it('should charge fee', async function () { - const { dai, weth, swap, chainId, feeTaker } = await loadFixture(deployContractsAndInit); + it.only('should charge fee when in whitelist', async function () { + const { dai, weth, swap, chainId, feeTaker, feeTakerAmountCalculator } = await loadFixture(deployContractsAndInit); const makingAmount = ether('300'); const takingAmount = ether('0.3'); - const fee = BigInt(1e4); - const feeCalculated = takingAmount * fee / BigInt(1e5); + const integratorFee = BigInt(1e4); + const resolverFee = BigInt(1e3); const feeRecipient = addr2.address; + const whitelist = '0x' + addr.address.slice(-20); const order = buildOrder( { @@ -118,7 +122,19 @@ describe('FeeTaker', function () { takingAmount, }, { - postInteraction: await feeTaker.getAddress() + trim0x(ethers.solidityPacked(['uint16', 'address'], [fee, feeRecipient])), + // * 2 bytes — integrator fee percentage (in 1e5) + // * 2 bytes — resolver fee percentage (in 1e5) + // * 20 bytes — fee recipient + // * 1 byte - taker whitelist size + // * (bytes10)[N] — taker whitelist + postInteraction: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], + [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x01', whitelist], + ), + takingAmountData: ethers.solidityPacked( + ['address', 'bytes1', 'bytes'], + [await feeTakerAmountCalculator.getAddress(), '0x01', whitelist], + ), }, ); @@ -127,10 +143,69 @@ describe('FeeTaker', function () { extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); + console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); + + const userTakingAmount = BigInt(1e5) * takingAmount / (BigInt(1e5) + integratorFee + resolverFee); + const feeCalculated = userTakingAmount * (integratorFee + resolverFee) / BigInt(1e5); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2], [-takingAmount, takingAmount - feeCalculated, feeCalculated]); }); + it.only('should charge fee when out of whitelist', async function () { + const { dai, weth, swap, chainId, feeTaker, feeTakerAmountCalculator } = await loadFixture(deployContractsAndInit); + + const makingAmount = ether('300'); + const takingAmount = ether('0.3'); + const integratorFee = BigInt(1e4); + const resolverFee = BigInt(1e3); + const feeRecipient = addr2.address; + const whitelist = '0x' + addr2.address.slice(-20); + + const order = buildOrder( + { + maker: addr1.address, + receiver: await feeTaker.getAddress(), + makerAsset: await dai.getAddress(), + takerAsset: await weth.getAddress(), + makingAmount, + takingAmount, + }, + { + // * 2 bytes — integrator fee percentage (in 1e5) + // * 2 bytes — resolver fee percentage (in 1e5) + // * 20 bytes — fee recipient + // * 1 byte - taker whitelist size + // * (bytes10)[N] — taker whitelist + postInteraction: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], + [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x01', whitelist], + ), + takingAmountData: ethers.solidityPacked( + ['address', 'bytes1', 'bytes', 'uint16', 'uint16'], + [await feeTakerAmountCalculator.getAddress(), '0x01', whitelist, integratorFee, resolverFee], + ), + }, + ); + + const { r, yParityAndS: vs } = ethers.Signature.from(await signOrder(order, chainId, await swap.getAddress(), addr1)); + const takerTraits = buildTakerTraits({ + extension: order.extension, + }); + const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); + console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); + + const userTakingAmount = BigInt(1e5) * takingAmount / (BigInt(1e5) + integratorFee + resolverFee); + const feeCalculated = userTakingAmount * (integratorFee + resolverFee) / BigInt(1e5); + + const integratorFeeAmount = userTakingAmount * integratorFee / BigInt(1e5); + const resolverFeeAmount = userTakingAmount * resolverFee / BigInt(1e5); + await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); + await expect(fillTx).to.changeTokenBalances(weth, + [addr, addr1, addr2], + [-(takingAmount + resolverFeeAmount), takingAmount - feeCalculated, integratorFeeAmount + 2n * resolverFeeAmount], + ); + }); + it('should charge fee and send the rest to the maker receiver', async function () { const { dai, weth, swap, chainId, feeTaker } = await loadFixture(deployContractsAndInit); From 238cc1b13e91da0817238bebac4397286294b524 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 11 Jul 2024 13:58:34 +0200 Subject: [PATCH 2/7] simplify contracts, add mulDiv, more unchecked --- contracts/extensions/FeeTaker.sol | 101 ++++++++++++------ .../extensions/FeeTakerAmountCalculator.sol | 82 -------------- test/FeeTaker.js | 36 ++++--- 3 files changed, 89 insertions(+), 130 deletions(-) delete mode 100644 contracts/extensions/FeeTakerAmountCalculator.sol diff --git a/contracts/extensions/FeeTaker.sol b/contracts/extensions/FeeTaker.sol index 6dcfa47c..178c5514 100644 --- a/contracts/extensions/FeeTaker.sol +++ b/contracts/extensions/FeeTaker.sol @@ -7,6 +7,7 @@ import { SafeERC20 } from "@1inch/solidity-utils/contracts/libraries/SafeERC20.s import { UniERC20 } from "@1inch/solidity-utils/contracts/libraries/UniERC20.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; +import { Math } from "@openzeppelin/contracts/utils/math/Math.sol"; import { IOrderMixin } from "../interfaces/IOrderMixin.sol"; import { IPostInteraction } from "../interfaces/IPostInteraction.sol"; @@ -80,6 +81,48 @@ contract FeeTaker is IPostInteraction, Ownable { } } + function getMakingAmount( + IOrderMixin.Order calldata /* order */, + bytes calldata /* extension */, + bytes32 /* orderHash */, + address /* taker */, + uint256 /* takingAmount */, + uint256 /* remainingMakingAmount */, + bytes calldata /* extraData */ + ) external pure returns (uint256) { + // TODO: Implement this function + return 0; + } + + /** + * @dev Calculate takingAmount with fee. + * `extraData` consists of: + * 1 byte - taker whitelist size + * (bytes10)[N] — taker whitelist + * 2 bytes — integrator fee percentage (in 1e5) + * 2 bytes — resolver fee percentage (in 1e5) + */ + function getTakingAmount( + IOrderMixin.Order calldata order, + bytes calldata /* extension */, + bytes32 /* orderHash */, + address taker, + uint256 makingAmount, + uint256 /* remainingMakingAmount */, + bytes calldata extraData + ) external view returns (uint256) { + unchecked { + uint256 integratorFee = uint256(uint16(bytes2(extraData[:2]))); + uint256 resolverFee = uint256(uint16(bytes2(extraData[2:]))); + uint256 calculatedTakingAmount = order.takingAmount; + if (!_isWhitelisted(extraData[5:5 + 10 * uint256(uint8(extraData[4]))], taker)) { + uint256 denominator = _FEE_BASE + integratorFee + resolverFee; + calculatedTakingAmount = Math.mulDiv(calculatedTakingAmount, denominator + resolverFee, denominator); + } + return Math.mulDiv(calculatedTakingAmount, makingAmount, order.makingAmount, Math.Rounding.Ceil); + } + } + /** * @notice See {IPostInteraction-postInteraction}. * @dev Takes the fee in taking tokens and transfers the rest to the maker. @@ -101,42 +144,36 @@ contract FeeTaker is IPostInteraction, Ownable { uint256 /* remainingMakingAmount */, bytes calldata extraData ) external onlyLimitOrderProtocol { - uint256 integratorFeePercent = uint256(uint16(bytes2(extraData))); - uint256 resolverFeePercent = uint256(uint16(bytes2(extraData[2:]))); - - uint256 whitelistSize = uint8(extraData[24]); - uint256 indexThroughWhitelist = 25 + whitelistSize * 10; - bytes calldata whitelist = extraData[25:indexThroughWhitelist]; - - if (!_isWhitelisted(whitelist, taker)) { - resolverFeePercent *= 2; - } - - uint256 userTakingAmount = _FEE_BASE * takingAmount / (_FEE_BASE + integratorFeePercent + resolverFeePercent); - uint256 integratorFee = userTakingAmount * integratorFeePercent / _FEE_BASE; - uint256 resolverFee = userTakingAmount * resolverFeePercent / _FEE_BASE; - address feeRecipient = address(bytes20(extraData[4:24])); - uint256 fee = integratorFee + resolverFee; - - address receiver = order.maker.get(); - if (extraData.length > indexThroughWhitelist) { - receiver = address(bytes20(extraData[indexThroughWhitelist:indexThroughWhitelist + 20])); - } + unchecked { + uint256 integratorFeePercent = uint256(uint16(bytes2(extraData))); + uint256 resolverFeePercent = uint256(uint16(bytes2(extraData[2:]))); + address feeRecipient = address(bytes20(extraData[4:24])); + uint256 whitelistEnd = 25 + uint8(extraData[24]) * 10; + bytes calldata whitelist = extraData[25:whitelistEnd]; + + if (!_isWhitelisted(whitelist, taker)) { + resolverFeePercent *= 2; + } - bool isEth = order.takerAsset.get() == address(_WETH) && order.makerTraits.unwrapWeth(); + uint256 denominator = _FEE_BASE + integratorFeePercent + resolverFeePercent; + uint256 integratorFee = Math.mulDiv(takingAmount, integratorFeePercent, denominator); + uint256 resolverFee = Math.mulDiv(takingAmount, resolverFeePercent, denominator); + uint256 fee = integratorFee + resolverFee; - if (isEth) { - if (fee > 0) { - _sendEth(feeRecipient, fee); + address receiver = order.maker.get(); + if (extraData.length > whitelistEnd) { + receiver = address(bytes20(extraData[whitelistEnd:whitelistEnd + 20])); } - unchecked { + + if (order.takerAsset.get() == address(_WETH) && order.makerTraits.unwrapWeth()) { + if (fee > 0) { + _sendEth(feeRecipient, fee); + } _sendEth(receiver, takingAmount - fee); - } - } else { - if (fee > 0) { - IERC20(order.takerAsset.get()).safeTransfer(feeRecipient, fee); - } - unchecked { + } else { + if (fee > 0) { + IERC20(order.takerAsset.get()).safeTransfer(feeRecipient, fee); + } IERC20(order.takerAsset.get()).safeTransfer(receiver, takingAmount - fee); } } diff --git a/contracts/extensions/FeeTakerAmountCalculator.sol b/contracts/extensions/FeeTakerAmountCalculator.sol deleted file mode 100644 index 4626f61e..00000000 --- a/contracts/extensions/FeeTakerAmountCalculator.sol +++ /dev/null @@ -1,82 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity 0.8.23; - -import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -import "@openzeppelin/contracts/utils/math/Math.sol"; -import "../libraries/AmountCalculatorLib.sol"; -import "../interfaces/IAmountGetter.sol"; - -contract FeeTakerAmountCalculator is IAmountGetter { - using Math for uint256; - - /// @dev Allows fees in range [1e-5, 0.65535] - uint256 internal constant _FEE_BASE = 1e5; - - function getMakingAmount( - IOrderMixin.Order calldata order, - bytes calldata /* extension */, - bytes32 /* orderHash */, - address /* taker */, - uint256 takingAmount, - uint256 /* remainingMakingAmount */, - bytes calldata /* extraData */ - ) external pure returns (uint256) { - return AmountCalculatorLib.getMakingAmount(order.makingAmount, order.takingAmount, takingAmount); - } - - /** - * @dev Validates whether the resolver is whitelisted. - * @param whitelist Whitelist is tightly packed struct of the following format: - * ``` - * (bytes10)[N] resolversAddresses; - * ``` - * Only 10 lowest bytes of the resolver address are used for comparison. - * @param resolver The resolver to check. - * @return Whether the resolver is whitelisted. - */ - function _isWhitelisted(bytes calldata whitelist, address resolver) internal view virtual returns (bool) { - unchecked { - uint80 maskedResolverAddress = uint80(uint160(resolver)); - uint256 size = whitelist.length / 10; - for (uint256 i = 0; i < size; i++) { - uint80 whitelistedAddress = uint80(bytes10(whitelist[:10])); - if (maskedResolverAddress == whitelistedAddress) { - return true; - } - whitelist = whitelist[10:]; - } - return false; - } - } - - /** - * @dev Calculate takingAmount with fee. - * `extraData` consists of: - * 1 byte - taker whitelist size - * (bytes10)[N] — taker whitelist - * 2 bytes — integrator fee percentage (in 1e5) - * 2 bytes — resolver fee percentage (in 1e5) - */ - function getTakingAmount( - IOrderMixin.Order calldata order, - bytes calldata /* extension */, - bytes32 /* orderHash */, - address taker, - uint256 makingAmount, - uint256 /* remainingMakingAmount */, - bytes calldata extraData - ) external view returns (uint256) { - uint256 calculatedTakingAmount = order.takingAmount; - uint256 indexThroughWhitelist = 1 + uint256(uint8(extraData[0]))*10; - if (!_isWhitelisted(extraData[1:indexThroughWhitelist], taker)) { - uint256 integratorFee = uint256(uint16(bytes2(extraData[indexThroughWhitelist:indexThroughWhitelist+2]))); - uint256 resolverFee = uint256(uint16(bytes2(extraData[indexThroughWhitelist+2:indexThroughWhitelist+4]))); - uint256 userTakingAmount = _FEE_BASE * order.takingAmount / (_FEE_BASE + integratorFee + resolverFee); - - calculatedTakingAmount += userTakingAmount * resolverFee / _FEE_BASE; - } - return (calculatedTakingAmount * makingAmount).ceilDiv(order.makingAmount); - } -} - diff --git a/test/FeeTaker.js b/test/FeeTaker.js index 168452fc..fcf89572 100644 --- a/test/FeeTaker.js +++ b/test/FeeTaker.js @@ -32,10 +32,7 @@ describe('FeeTaker', function () { const FeeTaker = await ethers.getContractFactory('FeeTaker'); const feeTaker = await FeeTaker.deploy(swap, weth, addr); - const FeeTakerAmountCalculator = await ethers.getContractFactory('FeeTakerAmountCalculator'); - const feeTakerAmountCalculator = await FeeTakerAmountCalculator.deploy(); - - return { dai, weth, inch, swap, chainId, feeTaker, feeTakerAmountCalculator }; + return { dai, weth, inch, swap, chainId, feeTaker }; }; it('should send all tokens to the maker with 0 fee', async function () { @@ -103,7 +100,7 @@ describe('FeeTaker', function () { }); it.only('should charge fee when in whitelist', async function () { - const { dai, weth, swap, chainId, feeTaker, feeTakerAmountCalculator } = await loadFixture(deployContractsAndInit); + const { dai, weth, swap, chainId, feeTaker } = await loadFixture(deployContractsAndInit); const makingAmount = ether('300'); const takingAmount = ether('0.3'); @@ -131,15 +128,20 @@ describe('FeeTaker', function () { ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x01', whitelist], ), + makingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], + ), takingAmountData: ethers.solidityPacked( - ['address', 'bytes1', 'bytes'], - [await feeTakerAmountCalculator.getAddress(), '0x01', whitelist], + ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], ), }, ); const { r, yParityAndS: vs } = ethers.Signature.from(await signOrder(order, chainId, await swap.getAddress(), addr1)); const takerTraits = buildTakerTraits({ + makingAmount: true, extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); @@ -152,7 +154,7 @@ describe('FeeTaker', function () { }); it.only('should charge fee when out of whitelist', async function () { - const { dai, weth, swap, chainId, feeTaker, feeTakerAmountCalculator } = await loadFixture(deployContractsAndInit); + const { dai, weth, swap, chainId, feeTaker } = await loadFixture(deployContractsAndInit); const makingAmount = ether('300'); const takingAmount = ether('0.3'); @@ -180,29 +182,31 @@ describe('FeeTaker', function () { ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x01', whitelist], ), + makingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], + ), takingAmountData: ethers.solidityPacked( - ['address', 'bytes1', 'bytes', 'uint16', 'uint16'], - [await feeTakerAmountCalculator.getAddress(), '0x01', whitelist, integratorFee, resolverFee], + ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], ), }, ); const { r, yParityAndS: vs } = ethers.Signature.from(await signOrder(order, chainId, await swap.getAddress(), addr1)); const takerTraits = buildTakerTraits({ + makingAmount: true, extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); - const userTakingAmount = BigInt(1e5) * takingAmount / (BigInt(1e5) + integratorFee + resolverFee); - const feeCalculated = userTakingAmount * (integratorFee + resolverFee) / BigInt(1e5); - - const integratorFeeAmount = userTakingAmount * integratorFee / BigInt(1e5); - const resolverFeeAmount = userTakingAmount * resolverFee / BigInt(1e5); + const integratorFeeAmount = takingAmount * integratorFee / (BigInt(1e5) + integratorFee + resolverFee); + const resolverFeeAmount = takingAmount * resolverFee / (BigInt(1e5) + integratorFee + resolverFee); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2], - [-(takingAmount + resolverFeeAmount), takingAmount - feeCalculated, integratorFeeAmount + 2n * resolverFeeAmount], + [-(takingAmount + resolverFeeAmount), takingAmount - integratorFeeAmount - resolverFeeAmount, integratorFeeAmount + 2n * resolverFeeAmount], ); }); From 4ab6a6f55ca76216d49fbe7454aa2e4fcf7f6d12 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 8 Aug 2024 15:24:42 +0200 Subject: [PATCH 3/7] add missing getter --- contracts/extensions/FeeTaker.sol | 79 ++++++++++++++++++++++--------- test/FeeTaker.js | 14 +++--- 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/contracts/extensions/FeeTaker.sol b/contracts/extensions/FeeTaker.sol index 178c5514..b3a73f67 100644 --- a/contracts/extensions/FeeTaker.sol +++ b/contracts/extensions/FeeTaker.sol @@ -9,12 +9,13 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { Math } from "@openzeppelin/contracts/utils/math/Math.sol"; +import { IAmountGetter } from "../interfaces/IAmountGetter.sol"; import { IOrderMixin } from "../interfaces/IOrderMixin.sol"; import { IPostInteraction } from "../interfaces/IPostInteraction.sol"; import { MakerTraits, MakerTraitsLib } from "../libraries/MakerTraitsLib.sol"; /// @title Helper contract that adds feature of collecting fee in takerAsset -contract FeeTaker is IPostInteraction, Ownable { +contract FeeTaker is IPostInteraction, IAmountGetter, Ownable { using AddressLib for Address; using SafeERC20 for IERC20; using UniERC20 for IERC20; @@ -81,44 +82,78 @@ contract FeeTaker is IPostInteraction, Ownable { } } + /** + * @dev Calculate makingAmount with fee. + * `extraData` consists of: + * 2 bytes — integrator fee percentage (in 1e5) + * 2 bytes — resolver fee percentage (in 1e5) + * 1 byte - taker whitelist size + * (bytes10)[N] — taker whitelist + */ function getMakingAmount( - IOrderMixin.Order calldata /* order */, - bytes calldata /* extension */, - bytes32 /* orderHash */, - address /* taker */, - uint256 /* takingAmount */, - uint256 /* remainingMakingAmount */, - bytes calldata /* extraData */ - ) external pure returns (uint256) { - // TODO: Implement this function - return 0; + IOrderMixin.Order calldata order, + bytes calldata extension, + bytes32 orderHash, + address taker, + uint256 takingAmount, + uint256 remainingMakingAmount, + bytes calldata extraData + ) external view returns (uint256 calculatedMakingAmount) { + unchecked { + uint256 integratorFee = uint256(uint16(bytes2(extraData))); + uint256 resolverFee = uint256(uint16(bytes2(extraData[2:]))); + bytes calldata whitelist = extraData[5:5 + 10 * uint256(uint8(extraData[4]))]; + extraData = extraData[5 + 10 * uint256(uint8(extraData[4])):]; + if (extraData.length > 20) { + calculatedMakingAmount = IAmountGetter(address(bytes20(extraData))).getMakingAmount( + order, extension, orderHash, taker, takingAmount, remainingMakingAmount, extraData[20:] + ); + } else { + calculatedMakingAmount = order.makingAmount; + } + uint256 denominator = _FEE_BASE + integratorFee + resolverFee; + if (!_isWhitelisted(whitelist, taker)) { + denominator += resolverFee; + } + calculatedMakingAmount = Math.mulDiv(calculatedMakingAmount, _FEE_BASE, denominator, Math.Rounding.Floor); + return Math.mulDiv(calculatedMakingAmount, takingAmount, order.takingAmount, Math.Rounding.Floor); + } } /** * @dev Calculate takingAmount with fee. * `extraData` consists of: - * 1 byte - taker whitelist size - * (bytes10)[N] — taker whitelist * 2 bytes — integrator fee percentage (in 1e5) * 2 bytes — resolver fee percentage (in 1e5) + * 1 byte - taker whitelist size + * (bytes10)[N] — taker whitelist */ function getTakingAmount( IOrderMixin.Order calldata order, - bytes calldata /* extension */, - bytes32 /* orderHash */, + bytes calldata extension, + bytes32 orderHash, address taker, uint256 makingAmount, - uint256 /* remainingMakingAmount */, + uint256 remainingMakingAmount, bytes calldata extraData - ) external view returns (uint256) { + ) external view returns (uint256 calculatedTakingAmount) { unchecked { - uint256 integratorFee = uint256(uint16(bytes2(extraData[:2]))); + uint256 integratorFee = uint256(uint16(bytes2(extraData))); uint256 resolverFee = uint256(uint16(bytes2(extraData[2:]))); - uint256 calculatedTakingAmount = order.takingAmount; - if (!_isWhitelisted(extraData[5:5 + 10 * uint256(uint8(extraData[4]))], taker)) { - uint256 denominator = _FEE_BASE + integratorFee + resolverFee; - calculatedTakingAmount = Math.mulDiv(calculatedTakingAmount, denominator + resolverFee, denominator); + bytes calldata whitelist = extraData[5:5 + 10 * uint256(uint8(extraData[4]))]; + extraData = extraData[5 + 10 * uint256(uint8(extraData[4])):]; + if (extraData.length > 20) { + calculatedTakingAmount = IAmountGetter(address(bytes20(extraData))).getTakingAmount( + order, extension, orderHash, taker, makingAmount, remainingMakingAmount, extraData[20:] + ); + } else { + calculatedTakingAmount = order.takingAmount; + } + uint256 numerator = _FEE_BASE + integratorFee + resolverFee; + if (!_isWhitelisted(whitelist, taker)) { + numerator += resolverFee; } + calculatedTakingAmount = Math.mulDiv(calculatedTakingAmount, numerator, _FEE_BASE, Math.Rounding.Ceil); return Math.mulDiv(calculatedTakingAmount, makingAmount, order.makingAmount, Math.Rounding.Ceil); } } diff --git a/test/FeeTaker.js b/test/FeeTaker.js index fcf89572..513cd4fb 100644 --- a/test/FeeTaker.js +++ b/test/FeeTaker.js @@ -99,7 +99,7 @@ describe('FeeTaker', function () { await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2, addr3], [-takingAmount, 0, 0, takingAmount]); }); - it.only('should charge fee when in whitelist', async function () { + it('should charge fee when in whitelist', async function () { const { dai, weth, swap, chainId, feeTaker } = await loadFixture(deployContractsAndInit); const makingAmount = ether('300'); @@ -147,13 +147,12 @@ describe('FeeTaker', function () { const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); - const userTakingAmount = BigInt(1e5) * takingAmount / (BigInt(1e5) + integratorFee + resolverFee); - const feeCalculated = userTakingAmount * (integratorFee + resolverFee) / BigInt(1e5); + const feeCalculated = takingAmount * (integratorFee + resolverFee) / BigInt(1e5); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); - await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2], [-takingAmount, takingAmount - feeCalculated, feeCalculated]); + await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2], [-takingAmount - feeCalculated, takingAmount, feeCalculated]); }); - it.only('should charge fee when out of whitelist', async function () { + it('should charge fee when out of whitelist', async function () { const { dai, weth, swap, chainId, feeTaker } = await loadFixture(deployContractsAndInit); const makingAmount = ether('300'); @@ -201,12 +200,11 @@ describe('FeeTaker', function () { const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); - const integratorFeeAmount = takingAmount * integratorFee / (BigInt(1e5) + integratorFee + resolverFee); - const resolverFeeAmount = takingAmount * resolverFee / (BigInt(1e5) + integratorFee + resolverFee); + const feeCalculated = takingAmount * (integratorFee + resolverFee + resolverFee) / BigInt(1e5); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2], - [-(takingAmount + resolverFeeAmount), takingAmount - integratorFeeAmount - resolverFeeAmount, integratorFeeAmount + 2n * resolverFeeAmount], + [-takingAmount - feeCalculated, takingAmount, feeCalculated], ); }); From 8e58d67434dc20054ee5750b5f882ee8df5814d2 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 8 Aug 2024 15:43:47 +0200 Subject: [PATCH 4/7] fix tests --- test/FeeTaker.js | 64 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/test/FeeTaker.js b/test/FeeTaker.js index 513cd4fb..a28b72b2 100644 --- a/test/FeeTaker.js +++ b/test/FeeTaker.js @@ -4,7 +4,7 @@ const { expect } = require('@1inch/solidity-utils'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); const { deploySwapTokens } = require('./helpers/fixtures'); const { buildOrder, buildTakerTraits, signOrder, buildMakerTraits } = require('./helpers/orderUtils'); -const { ether, trim0x } = require('./helpers/utils'); +const { ether } = require('./helpers/utils'); describe('FeeTaker', function () { let addr, addr1, addr2, addr3; @@ -53,7 +53,10 @@ describe('FeeTaker', function () { takingAmount, }, { - postInteraction: await feeTaker.getAddress() + trim0x(ethers.solidityPacked(['uint16', 'address'], [fee, feeRecipient])), + postInteraction: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'address', 'bytes1'], + [await feeTaker.getAddress(), fee, fee, feeRecipient, '0x00'], + ), }, ); @@ -85,8 +88,10 @@ describe('FeeTaker', function () { takingAmount, }, { - postInteraction: await feeTaker.getAddress() + - trim0x(ethers.solidityPacked(['uint16', 'address', 'address'], [fee, feeRecipient, makerReceiver])), + postInteraction: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'address', 'bytes1', 'address'], + [await feeTaker.getAddress(), fee, fee, feeRecipient, '0x00', makerReceiver], + ), }, ); @@ -228,8 +233,18 @@ describe('FeeTaker', function () { takingAmount, }, { - postInteraction: await feeTaker.getAddress() + - trim0x(ethers.solidityPacked(['uint16', 'address', 'address'], [fee, feeRecipient, makerReceiver])), + postInteraction: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'address', 'bytes1', 'address'], + [await feeTaker.getAddress(), fee, 0, feeRecipient, '0x00', makerReceiver], + ), + makingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1'], + [await feeTaker.getAddress(), fee, 0, '0x00'], + ), + takingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1'], + [await feeTaker.getAddress(), fee, 0, '0x00'], + ), }, ); @@ -239,7 +254,7 @@ describe('FeeTaker', function () { }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); - await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2, addr3], [-takingAmount, 0, feeCalculated, takingAmount - feeCalculated]); + await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2, addr3], [-takingAmount - feeCalculated, 0, feeCalculated, takingAmount]); }); it('should charge fee in eth', async function () { @@ -262,7 +277,18 @@ describe('FeeTaker', function () { makerTraits: buildMakerTraits({ unwrapWeth: true }), }, { - postInteraction: await feeTaker.getAddress() + trim0x(ethers.solidityPacked(['uint16', 'address'], [fee, feeRecipient])), + postInteraction: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'address', 'bytes1'], + [await feeTaker.getAddress(), fee, 0, feeRecipient, '0x00'], + ), + makingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1'], + [await feeTaker.getAddress(), fee, 0, '0x00'], + ), + takingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1'], + [await feeTaker.getAddress(), fee, 0, '0x00'], + ), }, ); @@ -272,8 +298,8 @@ describe('FeeTaker', function () { }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); - await expect(fillTx).to.changeTokenBalance(weth, addr, -takingAmount); - await expect(fillTx).to.changeEtherBalances([addr1, addr2], [takingAmount - feeCalculated, feeCalculated]); + await expect(fillTx).to.changeTokenBalance(weth, addr, -takingAmount - feeCalculated); + await expect(fillTx).to.changeEtherBalances([addr1, addr2], [takingAmount, feeCalculated]); }); it('should charge fee in eth and send the rest to the maker receiver', async function () { @@ -297,8 +323,18 @@ describe('FeeTaker', function () { makerTraits: buildMakerTraits({ unwrapWeth: true }), }, { - postInteraction: await feeTaker.getAddress() + - trim0x(ethers.solidityPacked(['uint16', 'address', 'address'], [fee, feeRecipient, makerReceiver])), + postInteraction: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'address', 'bytes1', 'address'], + [await feeTaker.getAddress(), fee, 0, feeRecipient, '0x00', makerReceiver], + ), + makingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1'], + [await feeTaker.getAddress(), fee, 0, '0x00'], + ), + takingAmountData: ethers.solidityPacked( + ['address', 'uint16', 'uint16', 'bytes1'], + [await feeTaker.getAddress(), fee, 0, '0x00'], + ), }, ); @@ -308,7 +344,7 @@ describe('FeeTaker', function () { }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); - await expect(fillTx).to.changeTokenBalance(weth, addr, -takingAmount); - await expect(fillTx).to.changeEtherBalances([addr1, addr2, addr3], [0, feeCalculated, takingAmount - feeCalculated]); + await expect(fillTx).to.changeTokenBalance(weth, addr, -takingAmount - feeCalculated); + await expect(fillTx).to.changeEtherBalances([addr1, addr2, addr3], [0, feeCalculated, takingAmount]); }); }); From 89348684f6fe0c359fcac604363d7b1a7f3eecb0 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 8 Aug 2024 16:04:57 +0200 Subject: [PATCH 5/7] increase whitelist size --- test/FeeTaker.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/FeeTaker.js b/test/FeeTaker.js index a28b72b2..d4bacff9 100644 --- a/test/FeeTaker.js +++ b/test/FeeTaker.js @@ -112,7 +112,7 @@ describe('FeeTaker', function () { const integratorFee = BigInt(1e4); const resolverFee = BigInt(1e3); const feeRecipient = addr2.address; - const whitelist = '0x' + addr.address.slice(-20); + const whitelist = '0x' + addr.address.slice(-20).repeat(10); const order = buildOrder( { @@ -131,15 +131,15 @@ describe('FeeTaker', function () { // * (bytes10)[N] — taker whitelist postInteraction: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x01', whitelist], + [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x0a', whitelist], ), makingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x0a', whitelist], ), takingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x0a', whitelist], ), }, ); @@ -165,7 +165,7 @@ describe('FeeTaker', function () { const integratorFee = BigInt(1e4); const resolverFee = BigInt(1e3); const feeRecipient = addr2.address; - const whitelist = '0x' + addr2.address.slice(-20); + const whitelist = '0x' + addr2.address.slice(-20).repeat(10); const order = buildOrder( { @@ -184,15 +184,15 @@ describe('FeeTaker', function () { // * (bytes10)[N] — taker whitelist postInteraction: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x01', whitelist], + [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x0a', whitelist], ), makingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x0a', whitelist], ), takingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, '0x01', whitelist], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x0a', whitelist], ), }, ); From efc6bc74b5c0afe465ee158456c5d03692ca9c08 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Fri, 9 Aug 2024 13:33:42 +0200 Subject: [PATCH 6/7] refactor FeeTaker a bit --- contracts/extensions/FeeTaker.sol | 123 ++++++++++++++---------------- test/FeeTaker.js | 28 +++---- 2 files changed, 71 insertions(+), 80 deletions(-) diff --git a/contracts/extensions/FeeTaker.sol b/contracts/extensions/FeeTaker.sol index b3a73f67..69c240c0 100644 --- a/contracts/extensions/FeeTaker.sol +++ b/contracts/extensions/FeeTaker.sol @@ -57,31 +57,6 @@ contract FeeTaker is IPostInteraction, IAmountGetter, Ownable { */ receive() external payable {} - /** - * @dev Validates whether the resolver is whitelisted. - * @param whitelist Whitelist is tightly packed struct of the following format: - * ``` - * (bytes10)[N] resolversAddresses; - * ``` - * Only 10 lowest bytes of the resolver address are used for comparison. - * @param resolver The resolver to check. - * @return Whether the resolver is whitelisted. - */ - function _isWhitelisted(bytes calldata whitelist, address resolver) internal view virtual returns (bool) { - unchecked { - uint80 maskedResolverAddress = uint80(uint160(resolver)); - uint256 size = whitelist.length / 10; - for (uint256 i = 0; i < size; i++) { - uint80 whitelistedAddress = uint80(bytes10(whitelist[:10])); - if (maskedResolverAddress == whitelistedAddress) { - return true; - } - whitelist = whitelist[10:]; - } - return false; - } - } - /** * @dev Calculate makingAmount with fee. * `extraData` consists of: @@ -100,22 +75,15 @@ contract FeeTaker is IPostInteraction, IAmountGetter, Ownable { bytes calldata extraData ) external view returns (uint256 calculatedMakingAmount) { unchecked { - uint256 integratorFee = uint256(uint16(bytes2(extraData))); - uint256 resolverFee = uint256(uint16(bytes2(extraData[2:]))); - bytes calldata whitelist = extraData[5:5 + 10 * uint256(uint8(extraData[4]))]; - extraData = extraData[5 + 10 * uint256(uint8(extraData[4])):]; - if (extraData.length > 20) { - calculatedMakingAmount = IAmountGetter(address(bytes20(extraData))).getMakingAmount( - order, extension, orderHash, taker, takingAmount, remainingMakingAmount, extraData[20:] + (uint256 integratorFee, uint256 resolverFee, bytes calldata tail) = _parseFeeData(extraData, taker); + if (tail.length > 20) { + calculatedMakingAmount = IAmountGetter(address(bytes20(tail))).getMakingAmount( + order, extension, orderHash, taker, takingAmount, remainingMakingAmount, tail[20:] ); } else { calculatedMakingAmount = order.makingAmount; } - uint256 denominator = _FEE_BASE + integratorFee + resolverFee; - if (!_isWhitelisted(whitelist, taker)) { - denominator += resolverFee; - } - calculatedMakingAmount = Math.mulDiv(calculatedMakingAmount, _FEE_BASE, denominator, Math.Rounding.Floor); + calculatedMakingAmount = Math.mulDiv(calculatedMakingAmount, _FEE_BASE, _FEE_BASE + integratorFee + resolverFee, Math.Rounding.Floor); return Math.mulDiv(calculatedMakingAmount, takingAmount, order.takingAmount, Math.Rounding.Floor); } } @@ -138,22 +106,15 @@ contract FeeTaker is IPostInteraction, IAmountGetter, Ownable { bytes calldata extraData ) external view returns (uint256 calculatedTakingAmount) { unchecked { - uint256 integratorFee = uint256(uint16(bytes2(extraData))); - uint256 resolverFee = uint256(uint16(bytes2(extraData[2:]))); - bytes calldata whitelist = extraData[5:5 + 10 * uint256(uint8(extraData[4]))]; - extraData = extraData[5 + 10 * uint256(uint8(extraData[4])):]; - if (extraData.length > 20) { - calculatedTakingAmount = IAmountGetter(address(bytes20(extraData))).getTakingAmount( - order, extension, orderHash, taker, makingAmount, remainingMakingAmount, extraData[20:] + (uint256 integratorFee, uint256 resolverFee, bytes calldata tail) = _parseFeeData(extraData, taker); + if (tail.length > 20) { + calculatedTakingAmount = IAmountGetter(address(bytes20(tail))).getTakingAmount( + order, extension, orderHash, taker, makingAmount, remainingMakingAmount, tail[20:] ); } else { calculatedTakingAmount = order.takingAmount; } - uint256 numerator = _FEE_BASE + integratorFee + resolverFee; - if (!_isWhitelisted(whitelist, taker)) { - numerator += resolverFee; - } - calculatedTakingAmount = Math.mulDiv(calculatedTakingAmount, numerator, _FEE_BASE, Math.Rounding.Ceil); + calculatedTakingAmount = Math.mulDiv(calculatedTakingAmount, _FEE_BASE + integratorFee + resolverFee, _FEE_BASE, Math.Rounding.Ceil); return Math.mulDiv(calculatedTakingAmount, makingAmount, order.makingAmount, Math.Rounding.Ceil); } } @@ -164,9 +125,9 @@ contract FeeTaker is IPostInteraction, IAmountGetter, Ownable { * `extraData` consists of: * 2 bytes — integrator fee percentage (in 1e5) * 2 bytes — resolver fee percentage (in 1e5) - * 20 bytes — fee recipient * 1 byte - taker whitelist size * (bytes10)[N] — taker whitelist + * 20 bytes — fee recipient * 20 bytes — receiver of taking tokens (optional, if not set, maker is used) */ function postInteraction( @@ -180,24 +141,16 @@ contract FeeTaker is IPostInteraction, IAmountGetter, Ownable { bytes calldata extraData ) external onlyLimitOrderProtocol { unchecked { - uint256 integratorFeePercent = uint256(uint16(bytes2(extraData))); - uint256 resolverFeePercent = uint256(uint16(bytes2(extraData[2:]))); - address feeRecipient = address(bytes20(extraData[4:24])); - uint256 whitelistEnd = 25 + uint8(extraData[24]) * 10; - bytes calldata whitelist = extraData[25:whitelistEnd]; - - if (!_isWhitelisted(whitelist, taker)) { - resolverFeePercent *= 2; - } - - uint256 denominator = _FEE_BASE + integratorFeePercent + resolverFeePercent; - uint256 integratorFee = Math.mulDiv(takingAmount, integratorFeePercent, denominator); - uint256 resolverFee = Math.mulDiv(takingAmount, resolverFeePercent, denominator); - uint256 fee = integratorFee + resolverFee; + (uint256 integratorFee, uint256 resolverFee, bytes calldata tail) = _parseFeeData(extraData, taker); + address feeRecipient = address(bytes20(tail)); + tail = tail[20:]; + uint256 denominator = _FEE_BASE + integratorFee + resolverFee; + // fee is calculated as a sum of separate fees to limit rounding errors + uint256 fee = Math.mulDiv(takingAmount, integratorFee, denominator) + Math.mulDiv(takingAmount, resolverFee, denominator); address receiver = order.maker.get(); - if (extraData.length > whitelistEnd) { - receiver = address(bytes20(extraData[whitelistEnd:whitelistEnd + 20])); + if (tail.length > 0) { + receiver = address(bytes20(tail)); } if (order.takerAsset.get() == address(_WETH) && order.makerTraits.unwrapWeth()) { @@ -229,4 +182,42 @@ contract FeeTaker is IPostInteraction, IAmountGetter, Ownable { revert EthTransferFailed(); } } + + /** + * @dev Validates whether the resolver is whitelisted. + * @param whitelist Whitelist is tightly packed struct of the following format: + * ``` + * (bytes10)[N] resolversAddresses; + * ``` + * Only 10 lowest bytes of the resolver address are used for comparison. + * @param resolver The resolver to check. + * @return Whether the resolver is whitelisted. + */ + function _isWhitelisted(bytes calldata whitelist, address resolver) private pure returns (bool) { + unchecked { + uint80 maskedResolverAddress = uint80(uint160(resolver)); + uint256 size = whitelist.length / 10; + for (uint256 i = 0; i < size; i++) { + uint80 whitelistedAddress = uint80(bytes10(whitelist[:10])); + if (maskedResolverAddress == whitelistedAddress) { + return true; + } + whitelist = whitelist[10:]; + } + return false; + } + } + + function _parseFeeData(bytes calldata extraData, address taker) private pure returns (uint256 integratorFee, uint256 resolverFee, bytes calldata tail) { + unchecked { + integratorFee = uint256(uint16(bytes2(extraData))); + resolverFee = uint256(uint16(bytes2(extraData[2:]))); + uint256 whitelistEnd = 5 + 10 * uint256(uint8(extraData[4])); + bytes calldata whitelist = extraData[5:whitelistEnd]; + if (!_isWhitelisted(whitelist, taker)) { + resolverFee *= 2; + } + tail = extraData[whitelistEnd:]; + } + } } diff --git a/test/FeeTaker.js b/test/FeeTaker.js index d4bacff9..9ef42c3e 100644 --- a/test/FeeTaker.js +++ b/test/FeeTaker.js @@ -54,8 +54,8 @@ describe('FeeTaker', function () { }, { postInteraction: ethers.solidityPacked( - ['address', 'uint16', 'uint16', 'address', 'bytes1'], - [await feeTaker.getAddress(), fee, fee, feeRecipient, '0x00'], + ['address', 'uint16', 'uint16', 'bytes1', 'address'], + [await feeTaker.getAddress(), fee, fee, '0x00', feeRecipient], ), }, ); @@ -89,8 +89,8 @@ describe('FeeTaker', function () { }, { postInteraction: ethers.solidityPacked( - ['address', 'uint16', 'uint16', 'address', 'bytes1', 'address'], - [await feeTaker.getAddress(), fee, fee, feeRecipient, '0x00', makerReceiver], + ['address', 'uint16', 'uint16', 'bytes1', 'address', 'address'], + [await feeTaker.getAddress(), fee, fee, '0x00', feeRecipient, makerReceiver], ), }, ); @@ -130,8 +130,8 @@ describe('FeeTaker', function () { // * 1 byte - taker whitelist size // * (bytes10)[N] — taker whitelist postInteraction: ethers.solidityPacked( - ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x0a', whitelist], + ['address', 'uint16', 'uint16', 'bytes1', 'bytes', 'address'], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x0a', whitelist, feeRecipient], ), makingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], @@ -183,8 +183,8 @@ describe('FeeTaker', function () { // * 1 byte - taker whitelist size // * (bytes10)[N] — taker whitelist postInteraction: ethers.solidityPacked( - ['address', 'uint16', 'uint16', 'address', 'bytes1', 'bytes'], - [await feeTaker.getAddress(), integratorFee, resolverFee, feeRecipient, '0x0a', whitelist], + ['address', 'uint16', 'uint16', 'bytes1', 'bytes', 'address'], + [await feeTaker.getAddress(), integratorFee, resolverFee, '0x0a', whitelist, feeRecipient], ), makingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1', 'bytes'], @@ -234,8 +234,8 @@ describe('FeeTaker', function () { }, { postInteraction: ethers.solidityPacked( - ['address', 'uint16', 'uint16', 'address', 'bytes1', 'address'], - [await feeTaker.getAddress(), fee, 0, feeRecipient, '0x00', makerReceiver], + ['address', 'uint16', 'uint16', 'bytes1', 'address', 'address'], + [await feeTaker.getAddress(), fee, 0, '0x00', feeRecipient, makerReceiver], ), makingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1'], @@ -278,8 +278,8 @@ describe('FeeTaker', function () { }, { postInteraction: ethers.solidityPacked( - ['address', 'uint16', 'uint16', 'address', 'bytes1'], - [await feeTaker.getAddress(), fee, 0, feeRecipient, '0x00'], + ['address', 'uint16', 'uint16', 'bytes1', 'address'], + [await feeTaker.getAddress(), fee, 0, '0x00', feeRecipient], ), makingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1'], @@ -324,8 +324,8 @@ describe('FeeTaker', function () { }, { postInteraction: ethers.solidityPacked( - ['address', 'uint16', 'uint16', 'address', 'bytes1', 'address'], - [await feeTaker.getAddress(), fee, 0, feeRecipient, '0x00', makerReceiver], + ['address', 'uint16', 'uint16', 'bytes1', 'address', 'address'], + [await feeTaker.getAddress(), fee, 0, '0x00', feeRecipient, makerReceiver], ), makingAmountData: ethers.solidityPacked( ['address', 'uint16', 'uint16', 'bytes1'], From 83f032f70e45a4e9b37110481f74a03590e27b26 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Fri, 9 Aug 2024 13:35:37 +0200 Subject: [PATCH 7/7] add gas logs --- test/FeeTaker.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/FeeTaker.js b/test/FeeTaker.js index 9ef42c3e..cdf5c942 100644 --- a/test/FeeTaker.js +++ b/test/FeeTaker.js @@ -65,6 +65,7 @@ describe('FeeTaker', function () { extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); + console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2], [-takingAmount, takingAmount, 0]); }); @@ -100,6 +101,7 @@ describe('FeeTaker', function () { extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); + console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2, addr3], [-takingAmount, 0, 0, takingAmount]); }); @@ -253,6 +255,7 @@ describe('FeeTaker', function () { extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); + console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalances(weth, [addr, addr1, addr2, addr3], [-takingAmount - feeCalculated, 0, feeCalculated, takingAmount]); }); @@ -297,6 +300,7 @@ describe('FeeTaker', function () { extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); + console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalance(weth, addr, -takingAmount - feeCalculated); await expect(fillTx).to.changeEtherBalances([addr1, addr2], [takingAmount, feeCalculated]); @@ -343,6 +347,7 @@ describe('FeeTaker', function () { extension: order.extension, }); const fillTx = swap.fillOrderArgs(order, r, vs, makingAmount, takerTraits.traits, takerTraits.args); + console.log(`GasUsed: ${(await (await fillTx).wait()).gasUsed.toString()}`); await expect(fillTx).to.changeTokenBalances(dai, [addr, addr1], [makingAmount, -makingAmount]); await expect(fillTx).to.changeTokenBalance(weth, addr, -takingAmount - feeCalculated); await expect(fillTx).to.changeEtherBalances([addr1, addr2, addr3], [0, feeCalculated, takingAmount]);