-
Notifications
You must be signed in to change notification settings - Fork 21
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
Support partial withdraw for unlocked tokens #20
Conversation
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.
tests look great! left some comments
VESTING_AMOUNTS.save(storage, &vec![])?; | ||
VESTING_TIMESTAMPS.save(storage, &vec![])?; |
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.
do we want these to be mutable? or do we want to introduce more state to track? i'm fine either way (we can always query the contract state in the past)
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.
yeah we always have the instantiate state so I feel like we can just reuse these for modifications
src/vesting.rs
Outdated
@@ -91,11 +120,26 @@ mod tests { | |||
.unwrap(); | |||
VESTING_AMOUNTS.save(deps_mut.storage, &vec![10]).unwrap(); | |||
|
|||
assert_eq!(10, collect_vested(deps_mut.storage, now).unwrap()); | |||
assert_eq!(10, collect_vested(deps_mut.storage, now, 15).unwrap()); |
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.
so the current logic allows one to specify withdrawing more than is vested? would it be better to force the user to specify the exact amount and error out instead?
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.
I'll defer to you on that since you'd often be the one initiating the withdrawal (like if getting exact amount is not hard then yeah we can add a check here)
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.
updated
bdad659
to
34c6d0f
Compare
Describe your changes and provide context
Allow passing in an
amount
field that specifies the upper limit of unlocked tokens one wishes to initiate withdrawal for. Specifically,amount
, the total amount would be withdrawnamount
,amount
would be withdrawn, and the first slot of the remaining tranche will have its amount deducted accordingly.Testing performed to validate your change
unit test
local sei