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

Loans: linear accrual on settlement price #1739

Merged
merged 17 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions libs/mocks/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ pub mod pallet {
}
}

impl<DataId, Data> Default for MockDataCollection<DataId, Data> {
fn default() -> Self {
Self(Box::new(|_| {
Err(DispatchError::Other("MockDataCollection: Data not found"))
}))
}
}

impl<DataId, Data> DataCollection<DataId> for MockDataCollection<DataId, Data> {
type Data = Data;

Expand Down
2 changes: 1 addition & 1 deletion libs/traits/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub trait DataRegistry<DataId, CollectionId> {
}

/// Abstration to represent a collection of data in memory
pub trait DataCollection<DataId> {
pub trait DataCollection<DataId>: Default {
/// Represents a data
type Data;

Expand Down
2 changes: 2 additions & 0 deletions libs/utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pallet-aura = { workspace = true }
pallet-timestamp = { workspace = true }
parity-scale-codec = { workspace = true }
scale-info = { workspace = true }
sp-arithmetic = { workspace = true }
sp-consensus-aura = { workspace = true }
sp-runtime = { workspace = true }
sp-std = { workspace = true }
Expand All @@ -30,6 +31,7 @@ std = [
"frame-support/std",
"frame-system/std",
"sp-runtime/std",
"sp-arithmetic/std",
"sp-std/std",
"pallet-timestamp/std",
"pallet-aura/std",
Expand Down
95 changes: 95 additions & 0 deletions libs/utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,101 @@ pub fn decode_var_source<const EXPECTED_SOURCE_ADDRESS_SIZE: usize>(
}
}

pub mod math {
lemunozm marked this conversation as resolved.
Show resolved Hide resolved
use sp_arithmetic::{
traits::{BaseArithmetic, EnsureFixedPointNumber},
ArithmeticError, FixedPointOperand, FixedU128,
};

/// Returns the coordinate `y` for coordinate `x`,
/// in a function given by 2 points: (x1, y1) and (x2, y2)
pub fn y_coord_in_rect<X, Y>(
(x1, y1): (X, Y),
(x2, y2): (X, Y),
x: X,
) -> Result<Y, ArithmeticError>
where
X: BaseArithmetic + FixedPointOperand,
Y: BaseArithmetic + FixedPointOperand,
{
// From the equation: (x - x1) / (x2 - x1) == (y - y1) / (y2 - y1) we solve y:
//
// NOTE: With rects that have x or y negative directions, we emulate a
// symmetry in those axis to avoid unsigned underflows in substractions. It
// means, we first "convert" the rect into an increasing rect, and in such rect,
// we find the y coordinate.

let left = if x1 <= x2 {
FixedU128::ensure_from_rational(x.ensure_sub(x1)?, x2.ensure_sub(x1)?)?
} else {
// X symmetry emulation
FixedU128::ensure_from_rational(x1.ensure_sub(x)?, x1.ensure_sub(x2)?)?
};

if y1 <= y2 {
left.ensure_mul_int(y2.ensure_sub(y1)?)?.ensure_add(y1)
} else {
// Y symmetry emulation
y1.ensure_sub(left.ensure_mul_int(y1.ensure_sub(y2)?)?)
}
}

#[cfg(test)]
mod test_y_coord_in_function_with_2_points {
use super::*;

#[test]
fn start_point() {
assert_eq!(y_coord_in_rect::<u32, u32>((3, 12), (7, 24), 3), Ok(12));
}

#[test]
fn end_point() {
assert_eq!(y_coord_in_rect::<u32, u32>((3, 12), (7, 24), 7), Ok(24));
}

// Rect defined as:
// (x2, y2)
// /
// /
// (x1, y1)
#[test]
fn inner_point() {
assert_eq!(y_coord_in_rect::<u32, u32>((3, 12), (7, 24), 4), Ok(15));
}

// Rect defined as:
// (x2, y2)
// \
// \
// (x1, y1)
#[test]
fn inner_point_with_greater_x1() {
assert_eq!(y_coord_in_rect::<u32, u32>((7, 12), (3, 24), 4), Ok(21));
}

// Rect defined as:
// (x1, y1)
// \
// \
// (x2, y2)
#[test]
fn inner_point_with_greater_y1() {
assert_eq!(y_coord_in_rect::<u32, u32>((3, 24), (7, 12), 4), Ok(21));
}

// Rect defined as:
// (x1, y1)
// /
// /
// (x2, y2)
#[test]
fn inner_point_with_greater_x1y1() {
assert_eq!(y_coord_in_rect::<u32, u32>((7, 24), (3, 12), 4), Ok(15));
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
4 changes: 4 additions & 0 deletions pallets/loans/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ sp-std = { workspace = true }
cfg-primitives = { workspace = true }
cfg-traits = { workspace = true }
cfg-types = { workspace = true }
cfg-utils = { workspace = true }
orml-traits = { workspace = true }

strum = { workspace = true }
Expand Down Expand Up @@ -56,6 +57,7 @@ std = [
"cfg-primitives/std",
"cfg-traits/std",
"cfg-types/std",
"cfg-utils/std",
"frame-benchmarking/std",
"strum/std",
"orml-traits/std",
Expand All @@ -68,6 +70,7 @@ runtime-benchmarks = [
"cfg-primitives/runtime-benchmarks",
"cfg-traits/runtime-benchmarks",
"cfg-types/runtime-benchmarks",
"cfg-utils/runtime-benchmarks",
"pallet-uniques/runtime-benchmarks",
"cfg-mocks/runtime-benchmarks",
]
Expand All @@ -78,6 +81,7 @@ try-runtime = [
"cfg-primitives/try-runtime",
"cfg-traits/try-runtime",
"cfg-types/try-runtime",
"cfg-utils/try-runtime",
"cfg-mocks/try-runtime",
"sp-runtime/try-runtime",
]
61 changes: 37 additions & 24 deletions pallets/loans/src/entities/loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use cfg_traits::{
self,
data::DataCollection,
interest::{InterestAccrual, InterestRate, RateCollection},
IntoSeconds, Seconds, TimeAsSecs,
Seconds, TimeAsSecs,
};
use cfg_types::adjustments::Adjustment;
use frame_support::{ensure, pallet_prelude::DispatchResult, RuntimeDebugNoBound};
Expand Down Expand Up @@ -255,23 +255,21 @@ impl<T: Config> ActiveLoan<T> {
Ok(now >= self.maturity_date().ensure_add(*overdue_secs)?)
}
WriteOffTrigger::PriceOutdated(secs) => match &self.pricing {
ActivePricing::External(pricing) => Ok(now
>= pricing
.last_updated(pool_id)?
.into_seconds()
.ensure_add(*secs)?),
ActivePricing::External(pricing) => {
Ok(now >= pricing.last_updated(pool_id).ensure_add(*secs)?)
}
ActivePricing::Internal(_) => Ok(false),
},
}
}

pub fn present_value(&self, pool_id: T::PoolId) -> Result<T::Balance, DispatchError> {
let maturity_date = self.schedule.maturity.date();
let value = match &self.pricing {
ActivePricing::Internal(inner) => {
let maturity_date = self.schedule.maturity.date();
inner.present_value(self.origination_date, maturity_date)?
}
ActivePricing::External(inner) => inner.present_value(pool_id)?,
ActivePricing::External(inner) => inner.present_value(pool_id, maturity_date)?,
};

self.write_down(value)
Expand All @@ -290,12 +288,12 @@ impl<T: Config> ActiveLoan<T> {
Rates: RateCollection<T::Rate, T::Balance, T::Balance>,
Prices: DataCollection<T::PriceId, Data = PriceOf<T>>,
{
let maturity_date = self.schedule.maturity.date();
let value = match &self.pricing {
ActivePricing::Internal(inner) => {
let maturity_date = self.schedule.maturity.date();
inner.present_value_cached(rates, self.origination_date, maturity_date)?
}
ActivePricing::External(inner) => inner.present_value_cached(prices)?,
ActivePricing::External(inner) => inner.present_value_cached(prices, maturity_date)?,
};

self.write_down(value)
Expand Down Expand Up @@ -327,7 +325,7 @@ impl<T: Config> ActiveLoan<T> {
BorrowRestrictions::OraclePriceRequired => {
match &self.pricing {
ActivePricing::Internal(_) => true,
ActivePricing::External(inner) => inner.last_updated(pool_id).is_ok(),
ActivePricing::External(inner) => inner.has_registered_price(pool_id),
}
}
},
Expand Down Expand Up @@ -533,31 +531,46 @@ pub struct ActiveLoanInfo<T: Config> {

/// Current outstanding interest of this loan
pub outstanding_interest: T::Balance,

/// Current price for external loans
/// - If oracle set, then the price is the one coming from the oracle,
/// - If not set, then the price is a linear accrual using the latest
/// settlement price.
/// See [`ExternalActivePricing::current_price()`]
pub current_price: Option<T::Balance>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could use the valuation method of the internal pricing here too. Removing the Option. ANd I would name it current_value.

cc: @denniswell

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Valuation method is just the present value for internal pricing loans. So we should do the same for external pricing loans too if go with this change.

This is related to my question:

Should we modify present_value() to now use current_price instead of latest_settlement_price?

For external pricing loans present_value = quantity * latest_settlemente_price. So maybe we do not need to use another field here and just update how present_value is computed using now current_price instead of latest_settlement_price

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah sorry, my bad! I think that is the best idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we still want to put current_price here to allow the UI just to draw the price. Otherwise, they should divide the present_value / outstanding_quantity to obtain it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that is confusing more ATM. The total value is anyways of greatest interest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think all that is relevant mostly is the value anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding this topic, I would say that if Apps needs to draw the price, leave the current_price as it is. Otherwise, if they need it and we do not offer it by the RuntimeAPI, then they need to perform not-trivial computations, and future changes to present_value will break that.

@onnovisser are you drawing right now the settlementPrice of an external loan?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lemunozm can we remove that one here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we decide after getting feedback from https://kflabs.slack.com/archives/C04GARZ532T/p1709024010720729 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Onno is out, which is who is taking care of it, I think, so probably we do not have feedback soon regarding this.

Seeing the apps code, they need to fetch the price as current_price now to fulfill this part: https://github.com/centrifuge/apps/blob/beea676a936a14597f270e931387a4df1ce7ca0d/centrifuge-app/src/utils/getLatestPrice.ts#L10 which is later used to draw in the loan page.

I would say we need to keep this here. Do you have any argument against this, @mustermeiszer? I do not see you very in favor of adding it.

}

impl<T: Config> TryFrom<(T::PoolId, ActiveLoan<T>)> for ActiveLoanInfo<T> {
type Error = DispatchError;

fn try_from((pool_id, active_loan): (T::PoolId, ActiveLoan<T>)) -> Result<Self, Self::Error> {
let (outstanding_principal, outstanding_interest) = match &active_loan.pricing {
let present_value = active_loan.present_value(pool_id)?;

Ok(match &active_loan.pricing {
ActivePricing::Internal(inner) => {
let principal = active_loan
.total_borrowed
.ensure_sub(active_loan.total_repaid.principal)?;

(principal, inner.outstanding_interest(principal)?)
Self {
present_value,
outstanding_principal: principal,
outstanding_interest: inner.outstanding_interest(principal)?,
current_price: None,
active_loan,
}
}
ActivePricing::External(inner) => {
let maturity = active_loan.maturity_date();

Self {
present_value,
outstanding_principal: inner.outstanding_principal(pool_id, maturity)?,
outstanding_interest: inner.outstanding_interest()?,
current_price: Some(inner.current_price(pool_id, maturity)?),
active_loan,
}
}
ActivePricing::External(inner) => (
inner.outstanding_principal(pool_id)?,
inner.outstanding_interest()?,
),
};

Ok(Self {
present_value: active_loan.present_value(pool_id)?,
outstanding_principal,
outstanding_interest,
active_loan,
})
}
}
Loading
Loading