Skip to content
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

Draft: eMode draft impl (WIP) #26

Open
wants to merge 9 commits into
base: devel
Choose a base branch
from
Open

Draft: eMode draft impl (WIP) #26

wants to merge 9 commits into from

Conversation

0xxgen1
Copy link
Contributor

@0xxgen1 0xxgen1 commented Oct 2, 2024

No description provided.


borrow.market_value = market_value;
unweighted_borrowed_value_usd = add(unweighted_borrowed_value_usd, market_value);

Copy link
Contributor

@0xripleys 0xripleys Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undo everything below here in this fn. this is purely formatting change, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the formatting stuff

open_ltv_pct: u8,
close_ltv_pct: u8,
) {
let has_emode_field = bag::contains(&reserve_config.additional_fields, EModeKey {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use helper function

assert!(vector::length(&obligation.borrows) <= MAX_BORROWS, ETooManyBorrows);
let is_emode = is_emode(obligation);

if (is_emode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would do this check after the borrow is processed. similar to the isolated check
if (isolated(config(reserve)) || obligation.borrowing_isolated_asset) {. the logic is simpler that way.

same comment for deposit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -392,6 +503,12 @@ module suilend::obligation {

borrow.borrowed_amount = sub(borrow.borrowed_amount, repay_amount);

let borrow_weight = if (is_emode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for is_emode flag if the value is only used once

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -379,6 +379,30 @@ module suilend::lending_market {
obligation::zero_out_rewards_if_looped(obligation, &mut lending_market.reserves, clock);
coin::from_balance(receive_balance, ctx)
}

/// Set emode for obligation - T is the deposit coin type
public fun set_emode<P>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think we need to know the reserve array indicies to set emode?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an obligation is a valid emode obligation if it has 1 or less deposits and 1 or less borrows

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we call deposit for an emode obligation we need to be able to know what the corresponding borrow reserve is in order to get the respective emode ltv

@@ -167,6 +184,45 @@ module suilend::obligation {
}
}

public(friend) fun set_emode<P>(
obligation: &mut Obligation<P>,
deposit_reserve: &Reserve<P>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly to my comment in lending market, not sure why we need to know the deposit and borrow reserve

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same response as above:

When we call deposit for an emode obligation we need to be able to know what the corresponding borrow reserve is in order to get the respective emode ltv

@@ -819,6 +843,22 @@ module suilend::lending_market {
reserve::update_reserve_config<P>(reserve, config);
}

public fun set_emode_for_pair<P, T>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it makes sense to get the type argument for the pair reserve as well, just as a sanity check.


// The obligation must ONLY have one deposit and one borrow, and
// they both must match the emod ones
if (vector::length(&obligation.borrows) == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. so the obligation has to have a deposit and borrow before activating e-mode? This feels a bit restrictive. IMO all the states below are valid:

  • has no deposit, no borrow
  • has deposit, no borrow
  • has deposit, borrow

@@ -319,6 +399,14 @@ module suilend::obligation {
deposit.deposited_ctoken_amount,
clock
);

if (is_emode) {
assert!(vector::length(&obligation.deposits) == 1, EIsolatedAssetViolation);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsolatedAsset refers to something else, so i would use EInvalidEModeDeposit

@@ -366,9 +461,16 @@ module suilend::obligation {

assert!(is_healthy(obligation), EObligationIsNotHealthy);

if (isolated(config(reserve)) || obligation.borrowing_isolated_asset) {
if (isolated(config(reserve)) || obligation.borrowing_isolated_asset || is_emode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: eh, lets just explicitly check num_borrows == 1 in the is_emode branch below

@@ -379,6 +481,12 @@ module suilend::obligation {
clock: &Clock,
max_repay_amount: Decimal,
): Decimal {
let borrow_weight = if (is_emode(obligation)) {
Copy link
Contributor

@0xripleys 0xripleys Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: move this expression closer to where the value is used, ie right before

if (le(interest_diff, repay_amount)) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants