-
Notifications
You must be signed in to change notification settings - Fork 602
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
x/lockup: Get updated lock for events #1184
x/lockup: Get updated lock for events #1184
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1184 +/- ##
=======================================
Coverage 21.06% 21.06%
=======================================
Files 196 196
Lines 25302 25305 +3
=======================================
+ Hits 5330 5331 +1
- Misses 19011 19012 +1
- Partials 961 962 +1
Continue to review full report at Codecov.
|
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.
We should add a changelog entry for this too please 🙏
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.
Thanks for this PR + the extra check for unlocking! LGTM!
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! I wonder if its worth reconsidering at some point how we do events. It does feel a bit silly that we have to do a second GetLockById in the main method. Maybe we should move towards k.createUnlockEvent(ctx, lockId)
? Or move events into the keeper methods rather than msg server?
Current endtime in logs (MsgBeginUnlocking) is wrong:
0001-01-01 00:00:00 +0000 UTC
because it is updated inside BeginUnlock and eventManager uses outdated lock, i wanted to return the updated lock from that function but it has many references, a quick fix is to get the lock again.this also includes verifying the owner conditional before BeginUnlock is executed.
For contributor use:
docs/
) or specification (x/<module>/spec/
)Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorer