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

fix: reduce snapshot requests number #8057

Merged
merged 25 commits into from
Nov 4, 2024
Merged

fix: reduce snapshot requests number #8057

merged 25 commits into from
Nov 4, 2024

Conversation

NeOMakinG
Copy link
Collaborator

@NeOMakinG NeOMakinG commented Oct 31, 2024

Description

Leverage scores endpoint instead of vp so we can do only one request for an infinite list of EVM addresses instead of X requests for X accounts

Issue (if applicable)

closes #8048

Risk

Medium, snapshot vote power could be buggy

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Testing

  • Add more than 50 evm accounts with the THOR voting power free fees feature flag, all the snapshot requests should success and not be a 429
  • FOX voting power should be displayed as usually in the FOX Benefits page
  • There should be only a few requests instead of 50-100

Engineering

n/a

Operations

n/a

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

Thor vote power

image
image

fox vote power

image
image
image

No discount from thor

image
image

Base automatically changed from thor-fees to develop October 31, 2024 18:20
@NeOMakinG NeOMakinG changed the title fix: throttle snapshot requests [WIP] fix: throttle snapshot requests Nov 1, 2024
@NeOMakinG NeOMakinG changed the title [WIP] fix: throttle snapshot requests fix: throttle snapshot requests Nov 4, 2024
@NeOMakinG NeOMakinG marked this pull request as ready for review November 4, 2024 10:14
@NeOMakinG NeOMakinG requested a review from a team as a code owner November 4, 2024 10:14
@NeOMakinG NeOMakinG changed the title fix: throttle snapshot requests fix: reduce snapshot requests number Nov 4, 2024
Copy link
Member

@woodenfurniture woodenfurniture left a comment

Choose a reason for hiding this comment

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

Works!

Though I did notice an issue when adding many accounts, ticket raised here
#8077

Also noticed that voting power for added accounts is not re-fetched until the page is reloaded, which may be fine considering adding an empty one doesn't affect voting power.

src/state/apis/snapshot/snapshot.ts Outdated Show resolved Hide resolved
src/state/apis/snapshot/getVotingPower.ts Show resolved Hide resolved
src/state/apis/snapshot/snapshot.ts Outdated Show resolved Hide resolved
src/state/apis/snapshot/snapshot.ts Outdated Show resolved Hide resolved
@kaladinlight
Copy link
Contributor

Also noticed that voting power for added accounts is not re-fetched until the page is reloaded, which may be fine considering adding an empty one doesn't affect voting power.

Not guranteed to be empty necessarily. They may have skipped accounts and an account loaded later could contain value. Def seems like a follow up improvement to refetch on account add.

@NeOMakinG NeOMakinG enabled auto-merge (squash) November 4, 2024 23:18
@NeOMakinG NeOMakinG merged commit b3aaa52 into develop Nov 4, 2024
3 checks passed
@NeOMakinG NeOMakinG deleted the throttle-snapshot branch November 4, 2024 23:25
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.

Throttle snapshot vote power calls to avoid rate limit
3 participants