[update] prodigy-fi - update methodology for DCI volume counting#7870
Open
brt-developer wants to merge 2 commits into
Open
[update] prodigy-fi - update methodology for DCI volume counting#7870brt-developer wants to merge 2 commits into
brt-developer wants to merge 2 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughThe Prodigy-Fi adapter now resolves versioned vault metadata from shared config and event signatures, stores quantity with each vault record, and computes daily notional and premium volume from the resolved metadata. Methodology text was updated to match the new aggregation model. ChangesProdigy-Fi metadata and volume aggregation update
Estimated code review effort: 4 (Complex) | ~40 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@options/prodigy-fi/index.ts`:
- Around line 108-121: The fallback in toNotional() is masking invalid buy-low
pricing by returning principal when strike or spotPrice is zero/missing, which
can misclassify notional as premium. Update toNotional() in
options/prodigy-fi/index.ts to treat invalid buy-low metadata as an error path
instead of a silent fallback, and ensure the caller(s) around the VaultMetadata
parsing/use sites propagate or log that failure rather than continuing with
principal. Keep the buy-low vs sell-high distinction intact so options adapters
do not confuse notional and premium volume.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30660e3e-d706-4d7e-bc7c-3e7aa82beda8
📥 Commits
Reviewing files that changed from the base of the PR and between 0bf4225 and a92068a6b249160a1fb9dc508a7e5cf5110e4944.
📒 Files selected for processing (1)
options/prodigy-fi/index.ts
brt-developer
changed the title
brt-developer
changed the title
|
The prodigy-fi adapter exports: |
|
The prodigy-fi adapter exports: |
Contributor
Author
|
@bheluga Can you review this one? Thanks |
bheluga
reviewed
Jul 3, 2026
bheluga
left a comment
bheluga
left a comment
Member
There was a problem hiding this comment.
@brt-developer thanks for the PR.
Previous approach was correct,
Notional Volume : Principal Deposited
Premium Volume: Yields /Premium earned.
We use similar approach among other similar protocols
brt-developer
force-pushed
the
feat/update-prodigyfi-methodology
branch
from
July 6, 2026 16:23
291af25 to
ccc1a73
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@options/prodigy-fi/index.ts`:
- Around line 143-147: The volume aggregation in the dayVaults loop is mixing
principal and premium values instead of separating notional exposure from
premium commitment. Update the metadata parsing path behind
fetchVaultMetadataRange so each vault carries distinct notional and premium
amounts, then change the dailyNotionalVolume and dailyPremiumVolume adds here to
use those precomputed fields rather than quantity and quantity * yieldValue /
WAD. Reference the dayVaults processing in the options/prodigy-fi index flow and
the metadata shape returned by fetchVaultMetadataRange.
In `@options/prodigy-fi/interfaces.ts`:
- Around line 5-9: VaultMetadata is missing the side, base/linked token, and
pricing fields needed for downstream notional calculations. Update the
VaultMetadata type to retain those metadata fields used by the options adapters,
or add a separate precomputed notional token/amount alongside token, quantity,
and yieldValue so aggregation can distinguish notional from premium volume.
Locate the change in VaultMetadata and keep the existing metadata shape aligned
with the notional computation flow.
- Around line 28-50: Add inline source comments for the hardcoded values in
config so maintainers can verify them later. Update the ChainConfig entries in
interfaces.ts for CHAIN.ETHEREUM, CHAIN.BASE, and CHAIN.BERACHAIN to annotate
the factory addresses, v1StartBlock, v1_1StartBlock, v2StartBlock, and start
date with the originating source or reference link where possible. Keep the
comments close to the relevant constants so the rationale for each magic number
is easy to find.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0b4c011b-d1ed-4b24-a6be-b6ed0b5ddc67
📥 Commits
Reviewing files that changed from the base of the PR and between 291af255fc76ecc616a3f8e6338ee2db38df18f1 and ccc1a7368eaba7690cc39cfb7618dba2a32ca853.
📒 Files selected for processing (2)
options/prodigy-fi/index.tsoptions/prodigy-fi/interfaces.ts
Comment on lines
+5
to
+9
| export type VaultMetadata = { | ||
| token: string; | ||
| quantity: bigint; | ||
| yieldValue: bigint; | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve side/pricing metadata needed for notional calculation.
VaultMetadata now only carries the investment token quantity, so downstream aggregation cannot compute buy-low notional as underlying base-asset exposure as described in the PR objective. Keep fields such as side, linked/base token, and pricing inputs in metadata, or precompute a separate notional token/amount before storing.
As per path instructions, options adapters must not confuse notional vs premium volume.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@options/prodigy-fi/interfaces.ts` around lines 5 - 9, VaultMetadata is
missing the side, base/linked token, and pricing fields needed for downstream
notional calculations. Update the VaultMetadata type to retain those metadata
fields used by the options adapters, or add a separate precomputed notional
token/amount alongside token, quantity, and yieldValue so aggregation can
distinguish notional from premium volume. Locate the change in VaultMetadata and
keep the existing metadata shape aligned with the notional computation flow.
Source: Path instructions
Comment on lines
+28
to
+50
| export const config: Record<string, ChainConfig> = { | ||
| [CHAIN.ETHEREUM]: { | ||
| factory: "0xAC2a612C49f29e26858Df1a53f7623180bcc3753", | ||
| v1StartBlock: 23831175, | ||
| v1_1StartBlock: 23831175, | ||
| v2StartBlock: 24036992, | ||
| start: "2025-11-19" | ||
| }, | ||
| [CHAIN.BASE]: { | ||
| factory: "0xFE198B51cfb1F96b56c63fe323a934BEAAA3b281", | ||
| v1StartBlock: 22133150, | ||
| v1_1StartBlock: 30509116, | ||
| v2StartBlock: 39795788, | ||
| start: "2024-11-08" | ||
| }, | ||
| [CHAIN.BERACHAIN]: { | ||
| factory: "0x29ca87b2f744127606ada4564da8219be6498ca1", | ||
| v1StartBlock: 804138, | ||
| v1_1StartBlock: 5297670, | ||
| v2StartBlock: 14682550, | ||
| start: "2025-02-07" | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add source comments for hardcoded chain constants.
The factory addresses, start blocks, version boundary blocks, and start dates need inline comments/source links so maintainers can verify them later.
As per coding guidelines, “Add a comment (and a source link where possible) for any hardcoded rate, address, or magic number.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@options/prodigy-fi/interfaces.ts` around lines 28 - 50, Add inline source
comments for the hardcoded values in config so maintainers can verify them
later. Update the ChainConfig entries in interfaces.ts for CHAIN.ETHEREUM,
CHAIN.BASE, and CHAIN.BERACHAIN to annotate the factory addresses, v1StartBlock,
v1_1StartBlock, v2StartBlock, and start date with the originating source or
reference link where possible. Keep the comments close to the relevant constants
so the rationale for each magic number is easy to find.
Source: Coding guidelines
|
The prodigy-fi adapter exports: |
brt-developer
force-pushed
the
feat/update-prodigyfi-methodology
branch
from
July 6, 2026 17:00
ccc1a73 to
fe2a2f9
Compare
Contributor
Author
|
Thanks for the review. Please check again. We covered an upgraded version of smart contracts that was not included before. Notional Volume : The total value of the underlying asset in an option or derivative contract |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@options/prodigy-fi/index.ts`:
- Around line 40-45: The vault metadata parsing in the Prodigy FI indexer is
silently skipping malformed entries and defaulting missing yieldValue to zero,
which hides ABI/config drift. Update the parsing logic around the vaults.set
path to validate required fields explicitly, and if vaultAddress,
investmentToken, quantity, or yieldValue are missing or invalid, surface an
error or throw instead of returning early or coercing defaults. Use the existing
vaults.set flow and the related parser in options/prodigy-fi/index.ts to
centralize this validation so malformed metadata fails visibly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df3225ea-97c9-4b64-a898-e23c1bcec85f
📥 Commits
Reviewing files that changed from the base of the PR and between ccc1a7368eaba7690cc39cfb7618dba2a32ca853 and fe2a2f9.
📒 Files selected for processing (2)
options/prodigy-fi/index.tsoptions/prodigy-fi/interfaces.ts
|
The prodigy-fi adapter exports: |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
options/prodigy-fi/index.ts (2)
157-164: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd
breakdownMethodologyfor the returned dimensions.The adapter returns
dailyNotionalVolumeanddailyPremiumVolume, but the shown adapter config only includesmethodology. Add matching breakdown text so validators/reviewers can verify the aggregation model.Proposed fix
methodology: { NotionalVolume: "The total value of the underlying asset in an option or derivative contract.", PremiumVolume: "The price paid up front to buy or sell option or derivative contract.", }, + breakdownMethodology: { + NotionalVolume: "Sum of vault quantities from Prodigy.Fi VaultCreated events.", + PremiumVolume: "Sum of vault quantity multiplied by yieldValue, scaled by WAD, from Prodigy.Fi VaultCreated events.", + },As per coding guidelines, “Return token breakdown where possible - always include
methodologyandbreakdownMethodology.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@options/prodigy-fi/index.ts` around lines 157 - 164, The returned adapter config in the `SimpleAdapter` object is missing `breakdownMethodology`, even though `fetch` exposes `dailyNotionalVolume` and `dailyPremiumVolume`. Add a `breakdownMethodology` field alongside the existing `methodology` in the adapter returned from `options/prodigy-fi/index.ts`, using matching breakdown text for the returned dimensions so validators can confirm the aggregation model.Source: Coding guidelines
148-148: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the
yieldValuescale used here. The1e18divisor is an implicit assumption across all three vault versions; add a source/comment so future ABI changes don’t silently skewdailyPremiumVolume.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@options/prodigy-fi/index.ts` at line 148, The dailyPremiumVolume calculation in the Prodigy FI options logic assumes metadata.yieldValue is WAD-scaled, but that scale is currently implicit. Add an in-code comment or nearby documentation in the same flow around dailyPremiumVolume.add and the metadata.yieldValue usage to explicitly state the 1e18/WAD assumption and note that it applies across all vault versions. Reference the metadata fields and WAD constant so future ABI changes are easy to audit.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@options/prodigy-fi/index.ts`:
- Around line 157-164: The returned adapter config in the `SimpleAdapter` object
is missing `breakdownMethodology`, even though `fetch` exposes
`dailyNotionalVolume` and `dailyPremiumVolume`. Add a `breakdownMethodology`
field alongside the existing `methodology` in the adapter returned from
`options/prodigy-fi/index.ts`, using matching breakdown text for the returned
dimensions so validators can confirm the aggregation model.
- Line 148: The dailyPremiumVolume calculation in the Prodigy FI options logic
assumes metadata.yieldValue is WAD-scaled, but that scale is currently implicit.
Add an in-code comment or nearby documentation in the same flow around
dailyPremiumVolume.add and the metadata.yieldValue usage to explicitly state the
1e18/WAD assumption and note that it applies across all vault versions.
Reference the metadata fields and WAD constant so future ABI changes are easy to
audit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 41d6337e-9d9e-44cc-aeee-f3dd62f7cb81
📒 Files selected for processing (1)
options/prodigy-fi/index.ts
|
The prodigy-fi adapter exports: |
bheluga
reviewed
Jul 12, 2026
bheluga
left a comment
bheluga
left a comment
Member
There was a problem hiding this comment.
@brt-developer when is vaultCreated event emitted?
is it on creation of vaults or when an user subscribes to the contract?
also what does quantity signify?
Contributor
Author
|
vaultCreated event occurs with the creation/opening of an initial position on a derivatives/options contract. |
Member
Hey @brt-developer we cant consider that as volume, |
Contributor
Author
|
by definition, a trade is required to change a user's derivatives/options position. |
Member
No, it should be traded by users, or else a protocol can create a vault of size 100m and claim volume as 100m |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Flagging an issue with our previous adapter: Not covered upgraded version of smart contracts.