-
Notifications
You must be signed in to change notification settings - Fork 1.8k
chore: hydrex fee adapter #5666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughA new TypeScript adapter module for Hydrex is added at Changes
Sequence Diagram(s)sequenceDiagram
participant Adapter as Hydrex Adapter
participant Contract as Option Exercise<br/>Contract
participant BribeFactory as Bribe Factory
participant BribeContracts as Bribe Contracts
participant Revenue as Revenue<br/>Accumulator
Adapter->>Contract: Query Option Exercise events
Contract-->>Adapter: USDC payment records
Adapter->>Revenue: Accumulate as dailyRevenue
Adapter->>BribeFactory: Query CreateBribe events<br/>(with fromBlock & cache)
BribeFactory-->>Adapter: Bribe contract addresses
Adapter->>BribeContracts: Query RewardAdded events
BribeContracts-->>Adapter: Reward by token records
Adapter->>Revenue: Accumulate as dailyFees
Revenue-->>Adapter: Return computed fees & revenue
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@fees/hydrex/index.ts`:
- Around line 38-44: create a guard around the creation and use of
bribeContracts so we never call getLogs with an empty or duplicated target list:
derive bribeContracts from createBribeLogs by mapping e.bribe.toLowerCase() then
deduplicate (e.g. via a Set) and if the resulting array is empty skip calling
getLogs (or return an empty rewardLogs) to avoid errors/double-counting; update
the call site that passes event_reward_added to getLogs to use the deduped
bribeContracts.
| const bribeContracts: string[] = createBribeLogs.map((e: any) => e.bribe.toLowerCase()); | ||
|
|
||
| // 3. Get all RewardAdded events from bribe contracts (this accounts for DEX fees, omni fees, and bribes) | ||
| const rewardLogs = await getLogs({ | ||
| targets: bribeContracts, | ||
| eventAbi: event_reward_added, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard empty bribe list and dedupe targets.
If createBribeLogs is empty or contains duplicates, getLogs may error or double-count rewards depending on its internals. A small guard + dedupe avoids both risks.
♻️ Suggested fix
- const bribeContracts: string[] = createBribeLogs.map((e: any) => e.bribe.toLowerCase());
+ const bribeContracts = Array.from(
+ new Set(createBribeLogs.map((e: any) => e.bribe.toLowerCase()))
+ );
+
+ if (!bribeContracts.length) {
+ return { dailyFees: dailyRevenue, dailyRevenue };
+ }🤖 Prompt for AI Agents
In `@fees/hydrex/index.ts` around lines 38 - 44, create a guard around the
creation and use of bribeContracts so we never call getLogs with an empty or
duplicated target list: derive bribeContracts from createBribeLogs by mapping
e.bribe.toLowerCase() then deduplicate (e.g. via a Set) and if the resulting
array is empty skip calling getLogs (or return an empty rewardLogs) to avoid
errors/double-counting; update the call site that passes event_reward_added to
getLogs to use the deduped bribeContracts.
|
The hydrex adapter exports: |
treeoflife2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
| }, | ||
| methodology: { | ||
| Fees: "Total fees from DEX fees, option exercises (SPR), Omni Liquidity Fees, and bribe rewards", | ||
| Revenue: "Total revenue from DEX fees, option exercises (SPR), Omni Liquidity Fees, and bribe rewards", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's bribe rewards in this? we track dailyBribeRevenue separtely, and bribes paid to pools won't be included in the dailyRevenue
| }, | ||
| }, | ||
| methodology: { | ||
| Fees: "Total fees from DEX fees, option exercises (SPR), Omni Liquidity Fees, and bribe rewards", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's SPR, here? we should also track yield generated by the pool as dailyFees, and dailyRevenue(part of fees going back to token holders if any + fees going to protocol treasury)
Hydrex already exists on DeFi Llama with the TVL adapter. Adding in fee adapter here. LMK if you need anything else!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.