From 717babf8e352efa1bbed79ed50ca6705e4f1eadc Mon Sep 17 00:00:00 2001 From: Frank Li Date: Tue, 28 May 2024 10:00:31 +1000 Subject: [PATCH] TD1453 Make sure maker/taker fee amounts are divisible by quantity of order (#1828) Signed-off-by: Frank Li Co-authored-by: mdymalla --- .../checkout/sdk/src/smartCheckout/buy/buy.ts | 4 +++- .../sdk/src/smartCheckout/fees/fees.test.ts | 18 +++++++++++++++++- .../sdk/src/smartCheckout/fees/fees.ts | 10 +++++++--- .../sdk/src/smartCheckout/sell/sell.ts | 10 +++++++++- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/checkout/sdk/src/smartCheckout/buy/buy.ts b/packages/checkout/sdk/src/smartCheckout/buy/buy.ts index e94c74da76..666b1fc605 100644 --- a/packages/checkout/sdk/src/smartCheckout/buy/buy.ts +++ b/packages/checkout/sdk/src/smartCheckout/buy/buy.ts @@ -166,7 +166,9 @@ export const buy = async ( let fees: FeeValue[] = []; if (takerFees && takerFees.length > 0) { - fees = calculateFees(takerFees, buyToken.amount, decimals); + // eslint-disable-next-line max-len + const tokenQuantity = order.result.sell[0].type === ItemType.ERC721 ? BigNumber.from(1) : BigNumber.from(order.result.sell[0].amount); + fees = calculateFees(takerFees, buyToken.amount, decimals, tokenQuantity); } let unsignedApprovalTransactions: TransactionRequest[] = []; diff --git a/packages/checkout/sdk/src/smartCheckout/fees/fees.test.ts b/packages/checkout/sdk/src/smartCheckout/fees/fees.test.ts index 58755a24f2..4af4ba9b19 100644 --- a/packages/checkout/sdk/src/smartCheckout/fees/fees.test.ts +++ b/packages/checkout/sdk/src/smartCheckout/fees/fees.test.ts @@ -1,4 +1,4 @@ -import { utils } from 'ethers'; +import { BigNumber, utils } from 'ethers'; import { calculateFees } from './fees'; import { BuyToken, ItemType } from '../../types'; import { CheckoutErrorType } from '../../errors'; @@ -94,6 +94,22 @@ describe('orderbook fees', () => { }]); }); + it('TD-1453 sell token quantity > 1 and non divisible fee amount', async () => { + const decimals = 18; + const amount = utils.parseUnits('40.32258064516129033', 18).toString(); + const makerFees = [{ + amount: { percentageDecimal: 0.01 }, + recipient: '0x222', + }]; + + const result = calculateFees(makerFees, amount, decimals, BigNumber.from(10)); + + expect(result).toEqual([{ + amount: '403225806451612900', + recipientAddress: '0x222', + }]); + }); + it('should calculate the fees with multiple percentageDecimals', async () => { const decimals = 18; const amount = utils.parseUnits('10', 18).toString(); diff --git a/packages/checkout/sdk/src/smartCheckout/fees/fees.ts b/packages/checkout/sdk/src/smartCheckout/fees/fees.ts index 78c6b76eb6..e2c221d9a3 100644 --- a/packages/checkout/sdk/src/smartCheckout/fees/fees.ts +++ b/packages/checkout/sdk/src/smartCheckout/fees/fees.ts @@ -9,6 +9,7 @@ export const MAX_FEE_DECIMAL_PLACES = 6; // will allow 0.000001 (0.0001%) as the const calculateFeesPercent = ( orderFee:OrderFee, amountBn: BigNumber, + tokenQuantity: BigNumber = BigNumber.from(1), ): BigNumber => { const feePercentage = orderFee.amount as FeePercentage; @@ -19,7 +20,8 @@ const calculateFeesPercent = ( .mul(BigNumber.from(feePercentageMultiplier)) .div(10 ** MAX_FEE_DECIMAL_PLACES); - return bnFeeAmount; + // always round down to have a fee amount divisible by the token quantity + return bnFeeAmount.sub(bnFeeAmount.mod(tokenQuantity)); }; const calculateFeesToken = ( @@ -35,6 +37,7 @@ export const calculateFees = ( orderFees: Array, weiAmount: string, decimals: number = 18, + tokenQuantity: BigNumber = BigNumber.from(1), ):Array => { let totalTokenFees: BigNumber = BigNumber.from(0); @@ -45,12 +48,13 @@ export const calculateFees = ( .mul(MAX_FEE_PERCENTAGE_DECIMAL * (10 ** MAX_FEE_DECIMAL_PLACES)) .div(10 ** MAX_FEE_DECIMAL_PLACES); - const calculateFeesResult:Array = []; + const calculateFeesResult: Array = []; for (const orderFee of orderFees) { let currentFeeBn = BigNumber.from(0); if (Object.hasOwn(orderFee.amount, 'percentageDecimal')) { - currentFeeBn = calculateFeesPercent(orderFee, amountBn); + currentFeeBn = calculateFeesPercent(orderFee, amountBn, tokenQuantity); + totalTokenFees = totalTokenFees.add(currentFeeBn); } else if (Object.hasOwn(orderFee.amount, 'token')) { currentFeeBn = calculateFeesToken(orderFee, decimals); diff --git a/packages/checkout/sdk/src/smartCheckout/sell/sell.ts b/packages/checkout/sdk/src/smartCheckout/sell/sell.ts index 9bef66803e..c06a92a439 100644 --- a/packages/checkout/sdk/src/smartCheckout/sell/sell.ts +++ b/packages/checkout/sdk/src/smartCheckout/sell/sell.ts @@ -250,7 +250,15 @@ export const sell = async ( }; if (makerFees !== undefined) { - const orderBookFees = calculateFees(makerFees, buyTokenOrNative.amount, decimals); + let tokenQuantity = BigNumber.from(1); + + // if type exists in sellToken then it is valid ERC721 or ERC1155 and not deprecated type + if ('type' in sellToken) { + const erc1155Token = sellToken as unknown as ERC1155Item | ERC721Item; + if (erc1155Token.type === ItemType.ERC1155) tokenQuantity = BigNumber.from(erc1155Token.amount); + } + + const orderBookFees = calculateFees(makerFees, buyTokenOrNative.amount, decimals, tokenQuantity); if (orderBookFees.length !== makerFees.length) { throw new CheckoutError( 'One of the fees is too small, must be greater than 0.000001',