-
Notifications
You must be signed in to change notification settings - Fork 7
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
EMode (1) #37
base: devel
Are you sure you want to change the base?
EMode (1) #37
Conversation
0xLendlord
commented
Nov 6, 2024
- reserve config module
- lending_market::set_emode_status
- obligation::refresh
- lending_market::set_emode_status - obligation::refresh
while (vector::length(&keys) > 0) { | ||
let emode = vec_map::get(emode_ltvs, &vector::pop_back(&mut keys)); | ||
|
||
assert!(config.open_ltv_pct < emode.open_ltv_pct, ENormalOpenLtvBetterThanEModeLtvs); |
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.
<=.
refresh(obligation, reserves, clock); | ||
|
||
}; | ||
refresh(obligation, reserves, clock); | ||
|
||
df::add(&mut obligation.id, EModeFlag {}, true); |
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.
you need to refresh after adding the flag for this to be correct
public fun set_emode<P>( | ||
lending_market: &mut LendingMarket<P>, | ||
obligation_owner_cap: &ObligationOwnerCap<P>, | ||
clock: &Clock |
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.
ability to toggle emode is still missing
@@ -169,7 +194,11 @@ module suilend::obligation { | |||
let mut deposited_value_usd = decimal::from(0); | |||
let mut allowed_borrow_value_usd = decimal::from(0); | |||
let mut unhealthy_borrow_value_usd = decimal::from(0); | |||
let is_emode = is_emode(obligation); | |||
if (is_emode) { | |||
obligation.refresh_emode(reserves, is_emode, clock) |
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.
nit: just return immediately
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.
assigned it to a variable to avoid borrow-checker error
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.
if (is_emode) {
obligation.refresh_emode(reserves, is_emode, clock)
return;
}
}; | ||
} | ||
|
||
fun refresh_emode<P>( |
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 think you can simplify this function to not use a while loops at all, and the variables on 303-305 aren't necessary either because the while loop will only iterate at most 1 time
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.
Simplified
borrow.market_value = market_value; | ||
unweighted_borrowed_value_usd = add(unweighted_borrowed_value_usd, market_value); | ||
|
||
let borrow_weight = if (is_emode) { |
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 think even if obligation is emode, if there's no corresponding emode entry, then the borrow weight has to be the one stored on the reserve (ie not 1).
example: say user is depositing usdc and borrowing FUD. fud has a borrow weight of 2. the (usdc, fud) pair does not have a corresponding emode entry. then, if the user sets their obligation to emode, then the borrow weight being used is 1, which lets them borrow more fud than we intended.
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.
Good point. Refactored
// check_normal_ltv_against_emode_ltvs | ||
let borrow_reserve_index = get_single_borrow_array_reserve_if_any(obligation); | ||
|
||
assert!( |
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 never want this assert to get tripped because then obligation::refresh can abort, which would be very bad.
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.
in this case we still want to return the base ltvs, right?
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.
that's right! Refactored
- Refresh after adding the emode flag - Ability to toggle emode on and off - Simplified refresh emode - Fixed borrow weight when no emode pair - Do not fail when getting ltvs - Simplied get_ltvs