Skip to content

Commit

Permalink
TD1453 Make sure maker/taker fee amounts are divisible by quantity of…
Browse files Browse the repository at this point in the history
… order (#1828)

Signed-off-by: Frank Li <[email protected]>
Co-authored-by: mdymalla <[email protected]>
  • Loading branch information
frankisawesome and mdymalla authored May 28, 2024
1 parent 7290675 commit 717babf
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 6 deletions.
4 changes: 3 additions & 1 deletion packages/checkout/sdk/src/smartCheckout/buy/buy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down
18 changes: 17 additions & 1 deletion packages/checkout/sdk/src/smartCheckout/fees/fees.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 7 additions & 3 deletions packages/checkout/sdk/src/smartCheckout/fees/fees.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 = (
Expand All @@ -35,6 +37,7 @@ export const calculateFees = (
orderFees: Array<OrderFee>,
weiAmount: string,
decimals: number = 18,
tokenQuantity: BigNumber = BigNumber.from(1),
):Array<FeeValue> => {
let totalTokenFees: BigNumber = BigNumber.from(0);

Expand All @@ -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<FeeValue> = [];
const calculateFeesResult: Array<FeeValue> = [];

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);
Expand Down
10 changes: 9 additions & 1 deletion packages/checkout/sdk/src/smartCheckout/sell/sell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 717babf

Please sign in to comment.