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: insights: correctly handle Arb and Fees #4909

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

cronokirby
Copy link
Contributor

Previously, these were being added to the supply, instead of subtracted.

This causes the supply in the insights dashboard to be slightly inflated.

Testing

To test, run pindexer and compare the following queries:

SELECT (
  SELECT SUM(um) as staked_um
    FROM (
      SELECT * FROM supply_validators
    ) validators
    LEFT JOIN LATERAL (
      SELECT um
      FROM supply_total_staked
      WHERE validator_id = id
      ORDER BY height DESC
      LIMIT 1
    ) staked_lateral ON TRUE)
+ um + auction + dex FROM supply_total_unstaked ORDER BY height DESC LIMIT 1;
SELECT total FROM insights_supply ORDER BY height DESC LIMIT 1;

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:

    Pindexer only

Previously, these were being added to the supply, instead of subtracted.
@cronokirby cronokirby marked this pull request as draft October 30, 2024 04:52
@cronokirby
Copy link
Contributor Author

Not sufficient to fix the discrepancy ; further investigation required.

@cronokirby cronokirby marked this pull request as ready for review November 18, 2024 21:15
@conorsch conorsch self-requested a review November 18, 2024 21:20
@conorsch
Copy link
Contributor

Not sufficient to fix the discrepancy ; further investigation required.

Even though this doesn't resolve the problem fully, the code changes are solid and we want them, so I'm going to merge and include with the intent of shipping in the next point release (#4928). Will re-index against testnet and mainnet chains to examine behavior prior to tagging.

@conorsch conorsch merged commit a22ee1c into main Nov 18, 2024
14 checks passed
@conorsch conorsch deleted the pindexer-insights-bug branch November 18, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants