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

dex: fold execution budget in DexParameters #4620

Merged
merged 4 commits into from
Jun 15, 2024
Merged

dex: fold execution budget in DexParameters #4620

merged 4 commits into from
Jun 15, 2024

Conversation

erwanor
Copy link
Member

@erwanor erwanor commented Jun 14, 2024

Describe your changes

Close #4602, and simplifies the execution breaker as well, fixing an edge case that would cause only successful executions to be counted.

Checklist before requesting a review

  • 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:

    This is state breaking, and contains a migration.

erwanor and others added 3 commits June 14, 2024 18:10
Co-Authored-By: Tal Derei <70081547+TalDerei@users.noreply.github.com>
Co-Authored-By: Tal Derei <70081547+TalDerei@users.noreply.github.com>
Co-Authored-By: Tal Derei <70081547+TalDerei@users.noreply.github.com>
@erwanor erwanor added consensus-breaking breaking change to execution of on-chain data A-dex Area: Relates to the dex state-breaking breaking change to on-chain data migration A pull request that contains a migration labels Jun 14, 2024
@erwanor erwanor added this to the Sprint 8 milestone Jun 14, 2024
Co-Authored-By: Tal Derei <70081547+TalDerei@users.noreply.github.com>
Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

This LGTM. I used the same procedure as in #4564 (comment) to test: created devnet, opened LP, migrated, confirmed LP still executable post-migration.

@TalDerei
Copy link
Collaborator

retro point of process: I ran 0.77.2 devnet, opened liquidity position on pair and swapped

Screenshot 2024-06-15 at 12 21 21 AM

performed governance proposal and chain halt, ran the migration, restarted chain, and was able to view and swap against position.

confirmed everything's working post-migration, and closing as complete.

@TalDerei TalDerei merged commit 4debdd9 into main Jun 15, 2024
16 checks passed
@TalDerei TalDerei deleted the tal/dex_params branch June 15, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dex Area: Relates to the dex consensus-breaking breaking change to execution of on-chain data migration A pull request that contains a migration state-breaking breaking change to on-chain data
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

dex: make the execution budget a chain parameter
3 participants