-
Notifications
You must be signed in to change notification settings - Fork 296
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
pindexer: flesh out dex-explorer indexer #4917
Conversation
The main addition are all the metrics needed for the summary page, and the supporting infrastructure therein. Arbitrary windows are now supported for summaries too. Performance was also greatly improved by making use of batch processing, making the supply app view the bottleneck now.
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.
Overall looks good. The performance improvements are significant! I've got a small concern about chain-specific logic in the indexing code, in the form of hardcoded asset ids, but willing to follow up on that when we've updated the interfaces like dex-explorer to work with the new schema.
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.
Can't wait to apply this to penumbra-zone/dex-explorer#133!
@cronokirby I see that the header from the explore page can be simply taken from dex_ex_aggregate_summary
table. What about the list of pairs and their stats?
@VanishMax That should (initially, possibly permanently) be configuration data (JSON) loaded into the explorer. There can be one for testnet and one for mainnet. The trading interface needs to express opinions about the list, at minimum because it needs to choose which asset is treated as the numeraire (e.g., using USDC as asset 2), so it seems simplest to start with a list specified in config data (so that there can be one for a mainnet and one for a testnet deployment). |
From there, you can query the |
Describe your changes
Closes #4914.
The internal architecture of the app view tries to make use of batch processing to the extent possible, which simplifies a lot of the logic.
The price charts remain unchanged, but I collapsed the two tables into one for performance and simplicity.
I also did not implement insertion of empty candles ; if there are gaps in the events, there will be gaps in the resulting database as well.
The main addition and where I spent most of my time on this is the addition of summaries of information over arbitrary windows. The idea behind the architecture here is that any time a change to liquidity, trade count, or a candle for a directed pair happens in a block, that block then gets a "snapshot" inserted, with the current price, liquidity, volume in that block, etc. At the end of this batch, the current summary is then updated, for each window, using those timed snapshots. And then an aggregate summary, across all pairs, is created from these summaries, for each window.
In order to price values under a common denom, assets are filtered based on having a current USDC price, backed by enough liquidity (the denom and liquidity amount are parameters to the component).
For testing, I'd recommend trying to run the app view against mainnet and testnet, and checking some sanity items like the price not seeming crazy, and matching in the summary across all windows, etc.
I think for testing we'll notice potential issues relatively quickly when dogfooding the explorer.
Checklist before requesting a review
I have added guiding text to explain how a reviewer should test these changes.
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: