track prediction markets' notional volume#5764
track prediction markets' notional volume#5764bheluga wants to merge 4 commits intoDefiLlama:masterfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis PR introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The kalshi.ts adapter exports: |
|
The kalshi.ts adapter exports: |
|
The kalshi.ts adapter exports: |
|
need to update server repo to handle notional volume per category |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dexs/pancakeswap-prediction/index.ts (1)
17-43:⚠️ Potential issue | 🟠 MajorNormalize epoch keys to avoid mismatched bull/bear aggregation.
epochDatais typed asMap<number,…>butbet.epochis likely a BigNumber/bigint/string. If it’s a non-number, bull/bear entries won’t collide and notional will be undercounted. Normalize the key.Proposed fix
- const epochData: Map<number, { bullAmount: bigint; bearAmount: bigint }> = new Map(); + const epochData: Map<string, { bullAmount: bigint; bearAmount: bigint }> = new Map(); bullLogs.forEach(bet => { dailyVolume.add(ADDRESSES.bsc.WBNB, bet.amount); - const epoch = bet.epoch; + const epoch = bet.epoch.toString(); const data = epochData.get(epoch) || { bullAmount: 0n, bearAmount: 0n }; data.bullAmount += BigInt(bet.amount); epochData.set(epoch, data); }); bearLogs.forEach(bet => { dailyVolume.add(ADDRESSES.bsc.WBNB, bet.amount); - const epoch = bet.epoch; + const epoch = bet.epoch.toString(); const data = epochData.get(epoch) || { bullAmount: 0n, bearAmount: 0n }; data.bearAmount += BigInt(bet.amount); epochData.set(epoch, data); });dexs/rain-one/index.ts (1)
14-70:⚠️ Potential issue | 🟠 MajorMap
optionAmountto the correct token or exclude from collateral-denominated notional volume.
optionAmountrepresents option position units (shares), not collateral amounts. Adding it todailyNotionalVolumewithtokenInfo.token(the collateral) causes incorrect scaling. Either:
- Map
optionAmountto the option-token address with its decimals, or- Track option volume separately if Rain protocol doesn't expose option token decimals
Current code assumes both
baseAmountandoptionAmountuse the same token/decimals, which is incorrect per Rain's protocol semantics.
🤖 Fix all issues with AI agents
In `@dexs/polymarket/index.ts`:
- Around line 109-116: Guard against null/undefined Dune totals by defaulting to
0 before dividing and calling addUSDValue: when updating dailyVolume and
dailyNotionalVolume, coalesce data[0].total_volume_usd and
data[0].total_notional_volume to 0 (e.g., via a nullish check) before dividing
by 2 and passing to addUSDValue so NaN cannot be produced; update the sites
where dailyVolume.addUSDValue(...) and dailyNotionalVolume.addUSDValue(...) are
called to use the safe default values instead.
In `@fees/myriadmarkets/index.ts`:
- Around line 27-31: The forEach callback for building marketMapping currently
uses an expression body which implicitly returns a value; change the arrow to
use a block body so it does not return anything. Specifically, update the
markets.forEach((val:any, idx:any) => marketMapping[val] = { token:
marketData[idx].token, fees: marketFees[idx] }) to markets.forEach((val:any,
idx:any) => { marketMapping[val] = { token: marketData[idx].token, fees:
marketFees[idx] }; }); keeping the same identifiers (marketMapping, markets,
marketData, marketFees) and types.
🧹 Nitpick comments (1)
dexs/kalshi.ts (1)
9-40: Dune 401 in CI — verify credentials or add a graceful fallback.
The adapter test is failing with “401 Unauthorized” from Dune. Please confirm the CI secrets for Dune are configured for this repo/branch and the query has access, or add a clearer error/fallback so tests don’t hard-fail when auth is missing.
| const dailyVolume = options.createBalances(); | ||
| const dailyNotionalVolume = options.createBalances(); | ||
|
|
||
| // Polymarket emits two OrderFilled events per trade (maker + taker), so summing them directly double-counts volume. Dividing by 2 converts this into true one-sided volume. This also normalizes swaps and merge/split fills, where maker and taker amounts can differ but still represent the same underlying trade, preventing inflated totals | ||
|
|
||
| dailyVolume.addUSDValue(data[0].total_volume_usd / 2); | ||
| dailyNotionalVolume.addUSDValue(data[0].total_notional_volume/2); | ||
|
|
There was a problem hiding this comment.
Guard against null/undefined totals from Dune.
If total_volume_usd or total_notional_volume is null/undefined for a day, the current division yields NaN. Consider defaulting to 0 before addUSDValue.
Proposed fix
- dailyVolume.addUSDValue(data[0].total_volume_usd / 2);
- dailyNotionalVolume.addUSDValue(data[0].total_notional_volume/2);
+ const totalVolume = Number(data[0]?.total_volume_usd) || 0;
+ const totalNotional = Number(data[0]?.total_notional_volume) || 0;
+ dailyVolume.addUSDValue(totalVolume / 2);
+ dailyNotionalVolume.addUSDValue(totalNotional / 2);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dailyVolume = options.createBalances(); | |
| const dailyNotionalVolume = options.createBalances(); | |
| // Polymarket emits two OrderFilled events per trade (maker + taker), so summing them directly double-counts volume. Dividing by 2 converts this into true one-sided volume. This also normalizes swaps and merge/split fills, where maker and taker amounts can differ but still represent the same underlying trade, preventing inflated totals | |
| dailyVolume.addUSDValue(data[0].total_volume_usd / 2); | |
| dailyNotionalVolume.addUSDValue(data[0].total_notional_volume/2); | |
| const dailyVolume = options.createBalances(); | |
| const dailyNotionalVolume = options.createBalances(); | |
| // Polymarket emits two OrderFilled events per trade (maker + taker), so summing them directly double-counts volume. Dividing by 2 converts this into true one-sided volume. This also normalizes swaps and merge/split fills, where maker and taker amounts can differ but still represent the same underlying trade, preventing inflated totals | |
| const totalVolume = Number(data[0]?.total_volume_usd) || 0; | |
| const totalNotional = Number(data[0]?.total_notional_volume) || 0; | |
| dailyVolume.addUSDValue(totalVolume / 2); | |
| dailyNotionalVolume.addUSDValue(totalNotional / 2); |
🤖 Prompt for AI Agents
In `@dexs/polymarket/index.ts` around lines 109 - 116, Guard against
null/undefined Dune totals by defaulting to 0 before dividing and calling
addUSDValue: when updating dailyVolume and dailyNotionalVolume, coalesce
data[0].total_volume_usd and data[0].total_notional_volume to 0 (e.g., via a
nullish check) before dividing by 2 and passing to addUSDValue so NaN cannot be
produced; update the sites where dailyVolume.addUSDValue(...) and
dailyNotionalVolume.addUSDValue(...) are called to use the safe default values
instead.
| const marketMapping: any = {} | ||
| markets.forEach((val, idx) => marketMapping[val] = { | ||
| markets.forEach((val:any, idx:any) => marketMapping[val] = { | ||
| token: marketData[idx].token, | ||
| fees: marketFees[idx] | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the file
fd -t f "index.ts" | grep -E "fees.*myriadmarkets"Repository: DefiLlama/dimension-adapters
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check if the file exists and read the relevant lines
if [ -f "fees/myriadmarkets/index.ts" ]; then
echo "File exists. Reading lines 20-35 for context:"
sed -n '20,35p' fees/myriadmarkets/index.ts | cat -n
else
echo "File not found at fees/myriadmarkets/index.ts. Searching..."
find . -name "index.ts" -path "*myriad*" 2>/dev/null
fiRepository: DefiLlama/dimension-adapters
Length of output: 913
🏁 Script executed:
#!/bin/bash
# Check for biome configuration
ls -la | grep -i biome || echo "No biome config in root"
find . -name "biome.json" -o -name "biome.toml" 2>/dev/null | head -5Repository: DefiLlama/dimension-adapters
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Read the biome configuration
cat biome.jsonRepository: DefiLlama/dimension-adapters
Length of output: 1847
🌐 Web query:
Biome forEach arrow function return statement lint rule
💡 Result:
Biome’s lint rule for “return statements inside forEach callbacks” is:
lint/suspicious/useIterableCallbackReturn(recommended / on by default since Biome v2.0.0) (biomejs.dev)
What it enforces:
- For methods like
map,filter,find, etc., the callback must consistently return a value. (biomejs.dev) - For
forEach, a return value is disallowed (becauseforEachignores returned values). (biomejs.dev)
Example that triggers it:
arr.forEach(() => {
return 1; // flagged: should not return a value
});Fix patterns:
- Remove the returned value (or the
returnentirely), or if you need an early-exit style, use a loop (for...of) instead. (biomejs.dev)
Config:
{
"linter": {
"rules": {
"suspicious": {
"useIterableCallbackReturn": "error"
}
}
}
}Citations:
- 1: https://biomejs.dev/linter/rules/use-iterable-callback-return/?utm_source=openai
- 2: https://biomejs.dev/linter/rules/use-iterable-callback-return/?utm_source=openai
- 3: https://biomejs.dev/linter/rules/use-iterable-callback-return/?utm_source=openai
- 4: https://biomejs.dev/linter/rules/use-iterable-callback-return/?utm_source=openai
- 5: https://biomejs.dev/linter/rules/use-iterable-callback-return/?utm_source=openai
- 6: https://biomejs.dev/linter/rules/use-iterable-callback-return/?utm_source=openai
Fix forEach callback to satisfy useIterableCallbackReturn lint.
Line 9 uses an expression body that implicitly returns a value from forEach. Biome's useIterableCallbackReturn rule flags this since forEach ignores returned values. Switch to a block body to avoid the implicit return.
Proposed fix
- markets.forEach((val:any, idx:any) => marketMapping[val] = {
- token: marketData[idx].token,
- fees: marketFees[idx]
- })
+ markets.forEach((val: any, idx: any) => {
+ marketMapping[val] = {
+ token: marketData[idx].token,
+ fees: marketFees[idx],
+ }
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const marketMapping: any = {} | |
| markets.forEach((val, idx) => marketMapping[val] = { | |
| markets.forEach((val:any, idx:any) => marketMapping[val] = { | |
| token: marketData[idx].token, | |
| fees: marketFees[idx] | |
| }) | |
| const marketMapping: any = {} | |
| markets.forEach((val: any, idx: any) => { | |
| marketMapping[val] = { | |
| token: marketData[idx].token, | |
| fees: marketFees[idx], | |
| } | |
| }) |
🧰 Tools
🪛 Biome (2.3.13)
[error] 28-28: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@fees/myriadmarkets/index.ts` around lines 27 - 31, The forEach callback for
building marketMapping currently uses an expression body which implicitly
returns a value; change the arrow to use a block body so it does not return
anything. Specifically, update the markets.forEach((val:any, idx:any) =>
marketMapping[val] = { token: marketData[idx].token, fees: marketFees[idx] }) to
markets.forEach((val:any, idx:any) => { marketMapping[val] = { token:
marketData[idx].token, fees: marketFees[idx] }; }); keeping the same identifiers
(marketMapping, markets, marketData, marketFees) and types.
|
The kalshi.ts adapter exports: |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.