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

dynamically update user redemption records during unbonding #1053

Merged
merged 49 commits into from
Jan 11, 2024

Conversation

sampocs
Copy link
Collaborator

@sampocs sampocs commented Jan 10, 2024

Context

(See #1049 for more context)

Refactored the dynamic redemption rate during unbonding PR.

Motivation for refactoring:

  • Further modularize functions to give more fine grained test coverage
  • Explicitly separate steps that modify state vs those that are just doing calculations
  • Put the new dynamic unbonding code as upstream as possible (so it's less invasive to the current unbonding code), while still keeping the native calculation and the unbonding ICAs atomic

@sampocs sampocs changed the base branch from dynamic-unbonding-rr to v17-upgrade-handler January 10, 2024 20:30
@sampocs sampocs force-pushed the dynamic-unbonding-rr-refactor branch 2 times, most recently from 61f450f to 9151887 Compare January 10, 2024 20:58
@riley-stride
Copy link
Contributor

riley-stride commented Jan 11, 2024

Manual testing to inspect the native_token_amount incrementing: Here's a gist that shows native_token_amount incrementing on epoch-unbonding-records upon unbonding. CMD+F for "1004" to see native_token_amount increment. Note this change aligns with the status of the record changing to UNBONDING_IN_PROGRESS, as expected.

Copy link
Contributor

@riley-stride riley-stride left a comment

Choose a reason for hiding this comment

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

Unit tests look quite solid. Covering all the functions and edge cases I can think of. If we wanted to beef them up we could test RefreshHostZoneUnbondingNativeTokenAmount and RefreshUserRedemptionRecordNativeAmounts more directly, but I think RefreshHostZoneUnbondingNativeTokenAmount already has good coverage of these indirectly, as you noted.

Co-authored-by: sampocs <sam.pochyly@gmail.com>
@sampocs sampocs added the v17 label Jan 11, 2024
@sampocs sampocs merged commit 7f88955 into main Jan 11, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants