-
Notifications
You must be signed in to change notification settings - Fork 91
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
program: settle_pnls that handles invalid oracle and efficient cus #1030
Conversation
msg!("User has no unsettled pnl for market {}", market_index); | ||
return Ok(()); | ||
let msg = format!("User has no unsettled pnl for market {}", market_index); | ||
return mode.result(ErrorCode::NoUnsettledPnl, &msg); |
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.
is it bad these dont always return OK? do we need update_pool_balances
to go through above?
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.
only the 'MUST' will cause update_pool_balances
to not go through? think its fine to just have 'TRY' allow that
programs/drift/src/controller/pnl.rs
Outdated
"User must settle their own unsettled pnl when its positive and pnl pool not in excess" | ||
)?; | ||
let user_must_settle_themself = pnl_to_settle_with_user > 0 | ||
&& max_pnl_pool_excess < 0 |
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.
should this be <=
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.
- low chance excess is exactly = 0, even so not sure its necessary since theres no incentive for settle run
return mode.result( | ||
ErrorCode::InsufficientCollateralForSettlingPNL, | ||
"Does not meet margin requirement", | ||
); |
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.
should market_index info get propagated if errors get hit for logs?
No description provided.