-
Notifications
You must be signed in to change notification settings - Fork 13
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
Misc todos #217
Misc todos #217
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.
looks good just that 1 q
@@ -51,10 +51,12 @@ impl UserBalance { | |||
if self.shares < to_q { | |||
panic_with_error!(e, BackstopError::BalanceError); | |||
} | |||
if self.q4w.len() >= MAX_Q4W_SIZE { |
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.
y 10 max
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.
No particular reason - I kept it fairly restrictive as 10 means users are creating a new Q4W every 2 days before they can start withdrawing to create more (which seems excessive).
I can increase this if you feel it's too restrictive.
Resource wise, it could handle at least 50, but I'd need to perform some tests to ensure it's safe after that.
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 decided that increasing max_q4w_size to 21 (at most 1 entry a day for the 21 day lock period) felt more reasonable than this.
Commit was updated.
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.
LOOKS GOOD TO ME
Handle various TODOs - mostly around resource concerns that were placed before we knew what the phase 2 limits would be.