Skip to content
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 metrics fix #4981

Merged
merged 3 commits into from
Jan 11, 2025
Merged

Pindexer metrics fix #4981

merged 3 commits into from
Jan 11, 2025

Conversation

cronokirby
Copy link
Contributor

Describe your changes

This fixes two bugs in pindexer:

  1. Volumes in aggregate and per pair summary were far too high.
  2. The different windows of the aggregate summary all reported the same value.

Bug 1. stemmed from multiplying by a price rather than dividing by it when mixing two directions of a candle.

Bug 2. was just a missing filter in an SQL query.

To test, run pindexer again, and do some sanity checks on the aggregate summary, and pair summaries for pairs like USDC / UM.

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:

    indexing only

We were doing the volume calculation wrong when mixing candles for
different pair directions, resulting in abnormally large volumes, since
we were *multiplying* by the price, instead of dividing.
Previously, we were not correctly considering just the window for the
aggregate summary, making all values erroneously the same.
Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, merging optimistically.

@erwanor erwanor added C-bug Category: a bug A-indexing Area: Relates to event indexing. labels Jan 11, 2025
@erwanor erwanor changed the base branch from main to release/v0.81.x January 11, 2025 21:58
@erwanor erwanor changed the base branch from release/v0.81.x to main January 11, 2025 21:58
@erwanor erwanor merged commit fe075d4 into main Jan 11, 2025
14 checks passed
@erwanor erwanor deleted the pindexer-metrics-fix branch January 11, 2025 21:59
@conorsch conorsch mentioned this pull request Jan 16, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-indexing Area: Relates to event indexing. C-bug Category: a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants