stablecoins: apply the chain-total exclusions to chain dominance#875
Open
tarun-khatri wants to merge 1 commit into
Open
stablecoins: apply the chain-total exclusions to chain dominance#875tarun-khatri wants to merge 1 commit into
tarun-khatri wants to merge 1 commit into
Conversation
craftStablecoinChainsResponse skips doublecounted assets; craftChainDominance Response skips nothing, so the two endpoints report different totals for the same chain on the same day - BSC differs by 28.9%. Excludes doublecounted assets, whose collateral is already counted, and dead assets, whose lastBalance is frozen at their final day and has no price so the peggedUSD fallback values it at par.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe chain dominance response now excludes pegged assets marked as ChangesChain dominance calculation
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
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
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.
craftStablecoinChainsResponseandcraftChainDominanceResponseboth report stablecoin totals per chain, but they don't apply the same exclusions. The chains path starts withwhile the dominance path maps over every asset with no filter at all. The two endpoints therefore disagree for the same chain on the same day:
/stablecoindominance/<chain>/stablecoinchainsWhat makes up the gap
66 assets carry
doublecounted: true, and 17 carrydeadFrom. Summing what each group contributes per chain from the publishedchainCirculating:The doublecounted column matches the observed endpoint delta, which is what you'd expect given the chains endpoint already filters it.
deadFromis the second half. It is honoured instorePegged.ts,storeNewPeggedBalances.ts,storeAllPeggedAssets.tsandstoreCharts.ts, but in none of the serving paths — so a dead asset'slastBalancestays frozen at its final day, and sincecoins.llama.fireturns no price for these collapsed tokens thepegType === "peggedUSD" ? 1 : 0fallback values every unit at exactly $1.00.Fix
Adds both exclusions so the dominance figures agree with the chain totals they sit alongside.
I could not run the api2 cache locally, so this is verified against the published endpoints and by summing the two flagged groups from
chainCirculatingrather than by executing the cron task.tsc --noEmitgives the same 50 pre-existing errors before and after, none in this file.Related: I have #874 open adding the
deadFromexclusion togetStablecoinChains.ts. Different file and different function; this one needs both flags because it currently has neither.getStableCoins.tshas the same omission, but dropping an asset there would also drop its detail page, so that one looks like a deliberate call rather than an oversight.Summary by CodeRabbit