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

Refactoring update pool weights #541

Conversation

thetroyharris
Copy link

@thetroyharris thetroyharris commented Dec 8, 2023

Description

Refactoring of the updatePoolWeights function to no longer uses calls to retrieve token weight information. Instead this token weight information is calculated given the block time and last GradualWeightUpdateScheduled event information.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency changes
  • Code refactor / cleanup
  • Documentation or wording changes
  • Other

How should this be tested?

Please provide instructions so we can test. Please also list any relevant details for your test configuration.

  • Test A
  • Test B

Checklist:

  • I have performed a self-review of my own code
  • I have requested at least 1 review (If the PR is significant enough, use best judgement here)
  • I have commented my code where relevant, particularly in hard-to-understand areas

dev -> master

  • I have checked that all beta deployments have synced
  • I have checked that the earliest block in the polygon pruned deployment is block, date/time
    • The earliest block is more than 24 hours old
  • I have checked that core metrics are the same in the beta and production deployments

Merges to dev

  • I have checked that the graft base is not a pruned deployment

@mendesfabio
Copy link
Member

mendesfabio commented Dec 8, 2023

hey -- could you change target branch to dev and address the lint issues? mark as ready for review when all tests are passing (ignore abi-integrity) and you want me to take a look 🙏

@thetroyharris thetroyharris changed the base branch from master to dev December 8, 2023 19:58
@mendesfabio
Copy link
Member

@thetroyharris is it ready for review? can you verify why there's a diff at .github/workflows/graph.yml? you probably branched off master instead of dev - rebase may fix this

@thetroyharris thetroyharris force-pushed the refactoring-updatePoolWeights branch 2 times, most recently from bda3f76 to 06a0cb1 Compare December 11, 2023 16:22
@mendesfabio mendesfabio marked this pull request as ready for review December 11, 2023 18:06
@thetroyharris
Copy link
Author

@mendesfabio I am investigating an issue with the results of these changes.

@mendesfabio
Copy link
Member

@mendesfabio I am investigating an issue with the results of these changes.

ok, just let me know when I can get back to review it

@mendesfabio mendesfabio marked this pull request as draft December 12, 2023 12:54
@mendesfabio
Copy link
Member

@thetroyharris I'm confused if I should start reviewing it - can you please mark as ready for review if that's the case?

src/mappings/helpers/weighted.ts Outdated Show resolved Hide resolved
src/mappings/helpers/weighted.ts Show resolved Hide resolved
src/mappings/helpers/weighted.ts Outdated Show resolved Hide resolved
src/mappings/poolController.ts Outdated Show resolved Hide resolved
src/mappings/vault.ts Show resolved Hide resolved
src/mappings/helpers/weighted.ts Outdated Show resolved Hide resolved
src/mappings/helpers/weighted.ts Outdated Show resolved Hide resolved
src/mappings/helpers/weighted.ts Outdated Show resolved Hide resolved
@thetroyharris
Copy link
Author

I will be building locally moving forward to decrease the frequency of the CI tests. My apologies.

@mendesfabio
Copy link
Member

yes you should run build and lint locally before pushing changes

@thetroyharris thetroyharris marked this pull request as ready for review December 14, 2023 19:55
@mendesfabio
Copy link
Member

can you share the deployment URL?

@thetroyharris
Copy link
Author

thetroyharris commented Dec 15, 2023

@mendesfabio

Mainnet: https://api.thegraph.com/subgraphs/id/QmSJZTgfEpeVadrkpNjafFvnpgSVzPxMV3XK1cB9EdDp3Z

Goerli: https://api.thegraph.com/subgraphs/name/thetroyharris/test

Note: I am actively performing tests on Goerli to resolve a bug where the totalWeight of the Pool entity does not update properly. This is bug is live on the balancer-v2 subgraph at this moment.

@mendesfabio mendesfabio changed the base branch from dev to refactor-update-weights December 22, 2023 16:45
@mendesfabio mendesfabio merged commit f39c572 into balancer:refactor-update-weights Dec 22, 2023
3 of 4 checks passed
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