-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat!: add force withdrawal migration to remove liquidity module on Gaia #30
base: release/v1.6.x
Are you sure you want to change the base?
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## release/v1.6.x #30 +/- ##
=================================================
Coverage ? 33.45%
=================================================
Files ? 44
Lines ? 12601
Branches ? 0
=================================================
Hits ? 4216
Misses ? 7888
Partials ? 497 |
Thanks @dongsam! Just have a few comments on some potential simplification and documentation. |
Thank you for your kind and detailed review @sainoe. I left a comment on the review and push the commit that applied the suggestions. |
"failed fund community pool", | ||
"pool id", pool.Id, | ||
"error", err, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if FundCommunityPool
failed, no return err ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, The FundCommunityPool is a very
simple function that is all about Send, Get, Add, and Set. In general, it is difficult to reproduce the failure and the probability of failure is close to zero, so error handling was not performed. Even if all of the previous logic succeeded and failed at the here, rather than reverting the entire migration by returning the error, it can be solved by re-calling only the FundCommunityPool function in the subsequent upgrade, so I don't think it's necessary to revert.
Description
Force Withdrawal:
Progress ( based sdk 0.45.x )
Simulation
liquidity module force withdrawal simulation result (cosmos-hub state height 14922680 based)
Decision
Ref
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes