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

Disable lsm unbond #997

Merged
merged 5 commits into from
Jan 5, 2024
Merged

Disable lsm unbond #997

merged 5 commits into from
Jan 5, 2024

Conversation

joe-bowman
Copy link
Contributor

Remove LSM unbonding given it's unreliability, and always use native unbonding.

@joe-bowman joe-bowman requested a review from faddat as a code owner January 3, 2024 22:38
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (ff3146a) 61.78% compared to head (0cff9cc) 61.44%.
Report is 5 commits behind head on main.

❗ Current head 0cff9cc differs from pull request most recent head 28eedd0. Consider uploading reports for the commit 28eedd0 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #997      +/-   ##
==========================================
- Coverage   61.78%   61.44%   -0.35%     
==========================================
  Files         172      172              
  Lines       14134    14118      -16     
==========================================
- Hits         8733     8675      -58     
- Misses       4651     4697      +46     
+ Partials      750      746       -4     
Flag Coverage Δ
unittests 61.44% <0.00%> (-0.35%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
x/interchainstaking/keeper/msg_server.go 84.61% <0.00%> (-0.37%) ⬇️

... and 3 files with indirect coverage changes

@faddat
Copy link
Contributor

faddat commented Jan 5, 2024

Do you think this should result in any upstream fixes?

faddat
faddat previously approved these changes Jan 5, 2024
Copy link
Contributor

@faddat faddat left a comment

Choose a reason for hiding this comment

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

Great catch we love to live reliable

@joe-bowman
Copy link
Contributor Author

Do you think this should result in any upstream fixes?

No, just LSM unbonding is a much more complex math problem, as the user receives bonded tokens back, and you need to compromise somewhere between what is best for the protocol, and what is best for the user (not receiving buckets of dust). It requires a little more finesse.

Copy link
Contributor

@faddat faddat left a comment

Choose a reason for hiding this comment

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

finesse

@faddat faddat merged commit 14f5977 into main Jan 5, 2024
9 of 13 checks passed
@joe-bowman joe-bowman deleted the disable-lsm-unbond branch January 5, 2024 14:22
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