Skip to content

Commit ad2b657

Browse files
authored
fix(predict): order calculations (MetaMask#22900)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> We seem to be wrongly calculating the order values. The CLOB client code seems quite different than ours, and they use `roundDown()` when rounding the price for market orders, but that somehow works for them because they use the `worstPrice` and their `marketPrice` and we are using `avgPrice`. In our case, we want the `takerAmount` to be the exact `shareAmount` we would get with the dollar amount we are willing to spend and, since we have that value from `matchBuyOrder`/`matchSellOrder`, we do **not** need to recalculate it, as it will lose precision. This not only simplifies the logic, but also removes unnecessary calculations which were leading to incorrect approximations. ### Note I want to continue investigating the differences between the CLOB client logic and ours and see if we should revert to what they do. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: fix order calculations Scenario: user wants to place a Buy order of $1 in a liquid market Given share price is $0.11 When user tries to submit the order, we calculate that we get `9.090909090909092` shares This leads to an `avgSharePrice = $1 / 9.090909090909092 shares = $0.109999999` Then, if we round this number down, we get a share price of `$0.10`, which is much cheaper than it should be This will lead to a much higher `takerAmount`, which makes the order fail ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Reworks market order preview to compute maker/taker amounts directly from matched book sizes, removing avgPrice-based rounding and the unused roundOrderAmounts helper (and its tests). > > - **Predict/Polymarket utils**: > - Adjust `previewOrder` to derive amounts from order book matches: > - BUY: `makerAmount = roundDown(size, roundConfig.size)`; `takerAmount = roundOrderAmount(shareAmount, roundConfig.amount)`. > - SELL: `makerAmount = roundDown(size, roundConfig.size)`; `takerAmount = roundOrderAmount(dollarAmount, roundConfig.amount)`. > - Remove `roundOrderAmounts` and dependency on `avgPrice`. > - Drop `RoundConfig`-related usage for that helper. > - **Tests**: > - Remove `roundOrderAmounts` import and its test suite in `utils.test.ts`. > - Keep `previewOrder` tests aligned with the new calculation approach. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 60bd2a3. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 8f05e97 commit ad2b657

2 files changed

Lines changed: 8 additions & 99 deletions

File tree

app/components/UI/Predict/providers/polymarket/utils.test.ts

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ import {
6464
roundDown,
6565
roundUp,
6666
roundOrderAmount,
67-
roundOrderAmounts,
6867
previewOrder,
6968
getAllowanceCalls,
7069
} from './utils';
@@ -2276,62 +2275,6 @@ describe('polymarket utils', () => {
22762275
});
22772276
});
22782277

2279-
describe('roundOrderAmounts', () => {
2280-
const mockRoundConfig = {
2281-
price: 4,
2282-
size: 2,
2283-
amount: 2,
2284-
};
2285-
2286-
it('rounds BUY order amounts correctly', () => {
2287-
const result = roundOrderAmounts({
2288-
roundConfig: mockRoundConfig,
2289-
side: Side.BUY,
2290-
size: 10.556,
2291-
price: 0.55555,
2292-
});
2293-
2294-
expect(result.makerAmount).toBe(10.55);
2295-
expect(result.takerAmount).toBeGreaterThan(0);
2296-
});
2297-
2298-
it('rounds SELL order amounts correctly', () => {
2299-
const result = roundOrderAmounts({
2300-
roundConfig: mockRoundConfig,
2301-
side: Side.SELL,
2302-
size: 10.556,
2303-
price: 0.55555,
2304-
});
2305-
2306-
expect(result.makerAmount).toBe(10.55);
2307-
expect(result.takerAmount).toBeGreaterThan(0);
2308-
});
2309-
2310-
it('handles price rounding down', () => {
2311-
const result = roundOrderAmounts({
2312-
roundConfig: mockRoundConfig,
2313-
side: Side.BUY,
2314-
size: 100,
2315-
price: 0.456789,
2316-
});
2317-
2318-
expect(result.makerAmount).toBe(100);
2319-
expect(result.takerAmount).toBeGreaterThan(0);
2320-
});
2321-
2322-
it('applies additional rounding when necessary', () => {
2323-
const result = roundOrderAmounts({
2324-
roundConfig: mockRoundConfig,
2325-
side: Side.BUY,
2326-
size: 123.456789,
2327-
price: 0.123456789,
2328-
});
2329-
2330-
expect(result.makerAmount).toBeLessThanOrEqual(123.46);
2331-
expect(result.takerAmount).toBeGreaterThan(0);
2332-
});
2333-
});
2334-
23352278
describe('previewOrder', () => {
23362279
beforeEach(() => {
23372280
mockFetch.mockReset();

app/components/UI/Predict/providers/polymarket/utils.ts

Lines changed: 8 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ import {
5252
PolymarketPosition,
5353
TickSize,
5454
OrderBook,
55-
RoundConfig,
5655
} from './types';
5756
import { PREDICT_ERROR_CODES } from '../../constants/errors';
5857

@@ -1119,35 +1118,6 @@ export const roundOrderAmount = ({
11191118
return amount;
11201119
};
11211120

1122-
export const roundOrderAmounts = ({
1123-
roundConfig,
1124-
side,
1125-
size,
1126-
price,
1127-
}: {
1128-
roundConfig: RoundConfig;
1129-
side: Side;
1130-
size: number;
1131-
price: number;
1132-
}): { makerAmount: number; takerAmount: number } => {
1133-
const rawPrice = roundDown(price, roundConfig.price);
1134-
const rawMakerAmt = roundDown(size, roundConfig.size);
1135-
let rawTakerAmt;
1136-
if (side === Side.BUY) {
1137-
rawTakerAmt = rawMakerAmt / rawPrice;
1138-
} else {
1139-
rawTakerAmt = rawMakerAmt * rawPrice;
1140-
}
1141-
rawTakerAmt = roundOrderAmount({
1142-
amount: rawTakerAmt,
1143-
decimals: roundConfig.amount,
1144-
});
1145-
return {
1146-
makerAmount: rawMakerAmt,
1147-
takerAmount: rawTakerAmt,
1148-
};
1149-
};
1150-
11511121
export const previewOrder = async (
11521122
params: Omit<PreviewOrderParams, 'providerId'>,
11531123
): Promise<OrderPreview> => {
@@ -1167,12 +1137,10 @@ export const previewOrder = async (
11671137
asks,
11681138
dollarAmount: size,
11691139
});
1170-
const avgPrice = size / shareAmount;
1171-
const { makerAmount, takerAmount } = roundOrderAmounts({
1172-
roundConfig,
1173-
side,
1174-
size,
1175-
price: avgPrice,
1140+
const makerAmount = roundDown(size, roundConfig.size);
1141+
const takerAmount = roundOrderAmount({
1142+
amount: shareAmount,
1143+
decimals: roundConfig.amount,
11761144
});
11771145
return {
11781146
marketId,
@@ -1201,12 +1169,10 @@ export const previewOrder = async (
12011169
bids,
12021170
shareAmount: size,
12031171
});
1204-
const avgPrice = dollarAmount / size;
1205-
const { makerAmount, takerAmount } = roundOrderAmounts({
1206-
roundConfig,
1207-
side,
1208-
size,
1209-
price: avgPrice,
1172+
const makerAmount = roundDown(size, roundConfig.size);
1173+
const takerAmount = roundOrderAmount({
1174+
amount: dollarAmount,
1175+
decimals: roundConfig.amount,
12101176
});
12111177
return {
12121178
marketId,

0 commit comments

Comments
 (0)