fix: infer hardcoded oracle assumptions in path warnings#499
fix: infer hardcoded oracle assumptions in path warnings#499antoncoding merged 7 commits intomasterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthrough
Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/oracle.ts`:
- Around line 332-340: getPegAnchor currently resolves a token peg by
symbol-only which can return incorrect pegs for duplicate/cross-chain symbols;
change getPegAnchor(symbol: string) to accept chain-scoped identity (e.g.,
getPegAnchor(chainId: number, address?: string, symbol?: string) or
getPegAnchor(tokenMeta: TokenMetadata)) and use chainId + lowercase address (or
tokenMeta.uniqueKey) to find the exact supportedTokens entry instead of
normalizeSymbol(symbol); update the lookup in supportedTokens.find(...) to
compare chainId and canonical address/uniqueKey (and fall back to symbol only as
last resort), and update all callers to pass the chain-scoped token info (or
token metadata) so identity checks follow the required chain-scoped canonical
identifier rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
Addressed the follow-up issues:\n- cbXRP / XRP now infers both hardcoded assumptions ( and )\n- warning copy now uses a newline plus instead of stacking everything into one sentence\n- tightened so symbol-based peg inference only returns a peg when all supported-token matches agree, which avoids ambiguous symbol collisions\n\nRe-validated with the local path-check script plus
Checked 635 files in 1417ms. No fixes applied. and
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/data-sources/subgraph/market.ts`:
- Around line 52-59: The current logic re-computes both token sides when
!hasUSDPrice and may overwrite an existing direct price with 0 if
getEstimateValue(token.peg) returns undefined (e.g., new XRP/HYPE pegs); change
the flow so you only call getEstimateValue() for the side that is missing
lastPriceUSD and only assign the estimated value if it is not undefined, leaving
the existing direct price unchanged otherwise. Locate the code paths around
token.peg / TokenPeg and majorPrices (and the second similar block at lines
referenced 115-122) and modify them so you check lastPriceUSD per-asset, call
getEstimateValue(token.peg) only for the missing side, and guard assignments
with a !== undefined check before replacing any current price.
In `@src/utils/oracle.ts`:
- Around line 341-350: The current logic builds matchingPegs by filtering out
null pegs which causes collisions with unpegged tokens to be treated as a match;
change the logic in the block that uses supportedTokens, normalizeSymbol and
matchingPegs so you first collect the peg values for all supportedTokens where
normalizeSymbol(supportedToken.symbol) === normalized (including nulls), return
null immediately if any collected peg is null, otherwise dedupe the non-null
pegs (TokenPeg) and return the single peg only if the deduped list length === 1,
else return null; update references to matchingPegs and TokenPeg accordingly.
In `@src/utils/tokens.ts`:
- Around line 17-18: The ERC20Token.peg field is being used both for USD price
fallbacks and for oracle-warning anchors, causing XRP/HYPE (and WHYPE/cbXRP) to
resolve to 0/undefined when market/token-price paths don't provide USD;
introduce a separate field (e.g., oracleAnchor) on the ERC20Token definition and
move XRP and HYPE values to that field for oracle-only use, then update all code
paths that currently read ERC20Token.peg for warnings to use
ERC20Token.oracleAnchor instead; alternatively, if you intend XRP/HYPE to be
used for price fallbacks, add proper price sources for XRP and HYPE to the
market/token-price resolution logic (update token price registry and resolver
functions) so they are resolved instead of falling back to zero. Ensure
references to ERC20Token.peg remain for USD fallbacks and search for uses in
warning/anchor code (e.g., where WHYPE/cbXRP are handled) and replace with the
new oracleAnchor symbol.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c01eb45e-7a2f-492f-8061-c614649f9b24
📒 Files selected for processing (3)
src/data-sources/subgraph/market.tssrc/utils/oracle.tssrc/utils/tokens.ts
| XRP = 'XRP', | ||
| HYPE = 'HYPE', |
There was a problem hiding this comment.
Separate oracle anchors from price-estimation pegs.
ERC20Token.peg is used by USD price fallbacks, but XRP/HYPE are not resolved by the market or token-price paths, so WHYPE/cbXRP still fall to 0/undefined when direct USD pricing is missing. If these are only for oracle warnings, use a separate oracle-anchor field; otherwise add XRP/HYPE price sources.
Also applies to: 786-786, 829-829
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/tokens.ts` around lines 17 - 18, The ERC20Token.peg field is being
used both for USD price fallbacks and for oracle-warning anchors, causing
XRP/HYPE (and WHYPE/cbXRP) to resolve to 0/undefined when market/token-price
paths don't provide USD; introduce a separate field (e.g., oracleAnchor) on the
ERC20Token definition and move XRP and HYPE values to that field for oracle-only
use, then update all code paths that currently read ERC20Token.peg for warnings
to use ERC20Token.oracleAnchor instead; alternatively, if you intend XRP/HYPE to
be used for price fallbacks, add proper price sources for XRP and HYPE to the
market/token-price resolution logic (update token price registry and resolver
functions) so they are resolved instead of falling back to zero. Ensure
references to ERC20Token.peg remain for USD fallbacks and search for uses in
warning/anchor code (e.g., where WHYPE/cbXRP are handled) and replace with the
new oracleAnchor symbol.
antoncoding
added
feature request
2 participants
Summary
cbBTC <> BTC pegandUSDC <> USD pegValidation
Summary by CodeRabbit
Bug Fixes
New Features