-
Notifications
You must be signed in to change notification settings - Fork 881
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
imp(vesting): add convert vesting account msg #1400
Conversation
ENG-1279 Convert Vesting Account
Problem One of the issues with vesting accounts is that when the tokens are fully vested, the account will still have to pay higher tx fees because every time the account will need to check if the tokens are vested. Scope of Work
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1400 +/- ##
==========================================
- Coverage 72.42% 72.40% -0.03%
==========================================
Files 260 260
Lines 17684 17725 +41
==========================================
+ Hits 12807 12833 +26
- Misses 4308 4322 +14
- Partials 569 570 +1
|
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
…om/evmos/evmos into Vvaradinov/convert-vesting-account
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.
LGTM with minor comments. We should also update the spec as part of this PR
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
…om/evmos/evmos into Vvaradinov/convert-vesting-account # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
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.
ACK pending spec changes
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
…om/evmos/evmos into Vvaradinov/convert-vesting-account
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.
sorry @Vvaradinov, I just noticed that we forgot to add a test case for locked tokens
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
…om/evmos/evmos into Vvaradinov/convert-vesting-account # Conflicts:
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.
Great work @Vvaradinov !! Left a small suggestion
Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com>
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.
ACK. Minor comments
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
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.
Great work!
…om/evmos/evmos into Vvaradinov/convert-vesting-account
Description
Adds a
ConvertVestingAccount
msg which converts a vesting account to a regularBaseAccount
once it has confirmed that no vesting coins are left.Closes ENG-1279