-
Notifications
You must be signed in to change notification settings - Fork 86
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: Runtime API for updating the portfolio with fake prices #1748
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,9 +73,12 @@ pub use weights::WeightInfo; | |
#[frame_support::pallet] | ||
pub mod pallet { | ||
use cfg_traits::{ | ||
self, changes::ChangeGuard, data::DataRegistry, interest::InterestAccrual, IntoSeconds, | ||
Permissions, PoolInspect, PoolNAV, PoolReserve, PoolWriteOffPolicyMutate, Seconds, | ||
TimeAsSecs, | ||
self, | ||
changes::ChangeGuard, | ||
data::{DataCollection, DataRegistry}, | ||
interest::InterestAccrual, | ||
IntoSeconds, Permissions, PoolInspect, PoolNAV, PoolReserve, PoolWriteOffPolicyMutate, | ||
Seconds, TimeAsSecs, | ||
}; | ||
use cfg_types::{ | ||
adjustments::Adjustment, | ||
|
@@ -84,7 +87,7 @@ pub mod pallet { | |
}; | ||
use entities::{ | ||
changes::{Change, LoanMutation}, | ||
input::{PrincipalInput, RepaidInput}, | ||
input::{PriceCollectionInput, PrincipalInput, RepaidInput}, | ||
loans::{self, ActiveLoan, ActiveLoanInfo, LoanInfo}, | ||
}; | ||
use frame_support::{ | ||
|
@@ -103,7 +106,7 @@ pub mod pallet { | |
traits::{BadOrigin, EnsureAdd, EnsureAddAssign, EnsureInto, One, Zero}, | ||
ArithmeticError, FixedPointOperand, TransactionOutcome, | ||
}; | ||
use sp_std::{vec, vec::Vec}; | ||
use sp_std::{collections::btree_map::BTreeMap, vec, vec::Vec}; | ||
use types::{ | ||
self, | ||
policy::{self, WriteOffRule, WriteOffStatus}, | ||
|
@@ -150,7 +153,7 @@ pub mod pallet { | |
+ One; | ||
|
||
/// Identify a loan in the pallet | ||
type PriceId: Parameter + Member + TypeInfo + Copy + MaxEncodedLen; | ||
type PriceId: Parameter + Member + TypeInfo + Copy + MaxEncodedLen + Ord; | ||
|
||
/// Defines the rate type used for math computations | ||
type Rate: Parameter + Member + FixedPointNumber + TypeInfo + MaxEncodedLen; | ||
|
@@ -775,7 +778,10 @@ pub mod pallet { | |
ensure_signed(origin)?; | ||
Self::ensure_pool_exists(pool_id)?; | ||
|
||
let (_, count) = Self::update_portfolio_valuation_for_pool(pool_id)?; | ||
let (_, count) = Self::update_portfolio_valuation_for_pool( | ||
pool_id, | ||
PriceCollectionInput::FromRegistry, | ||
)?; | ||
|
||
Ok(Some(T::WeightInfo::update_portfolio_valuation(count)).into()) | ||
} | ||
|
@@ -1051,11 +1057,33 @@ pub mod pallet { | |
.map_err(|_| Error::<T>::NoLoanChangeId.into()) | ||
} | ||
|
||
fn update_portfolio_valuation_for_pool( | ||
fn registered_prices( | ||
pool_id: T::PoolId, | ||
) -> Result<BTreeMap<T::PriceId, T::Balance>, DispatchError> { | ||
let collection = T::PriceRegistry::collection(&pool_id)?; | ||
Ok(ActiveLoans::<T>::get(pool_id) | ||
.iter() | ||
.filter_map(|(_, loan)| loan.price_id()) | ||
.filter_map(|price_id| { | ||
collection | ||
.get(&price_id) | ||
.map(|price| (price_id, price.0)) | ||
.ok() | ||
}) | ||
.collect::<BTreeMap<_, _>>()) | ||
} | ||
|
||
pub fn update_portfolio_valuation_for_pool( | ||
pool_id: T::PoolId, | ||
input_prices: PriceCollectionInput<T>, | ||
) -> Result<(T::Balance, u32), DispatchError> { | ||
let rates = T::InterestAccrual::rates(); | ||
let prices = T::PriceRegistry::collection(&pool_id)?; | ||
let prices = match input_prices { | ||
PriceCollectionInput::Empty => BTreeMap::default(), | ||
PriceCollectionInput::Custom(prices) => prices.into(), | ||
PriceCollectionInput::FromRegistry => Self::registered_prices(pool_id)?, | ||
}; | ||
|
||
let loans = ActiveLoans::<T>::get(pool_id); | ||
let values = loans | ||
.iter() | ||
|
@@ -1208,7 +1236,8 @@ pub mod pallet { | |
} | ||
|
||
fn update_nav(pool_id: T::PoolId) -> Result<T::Balance, DispatchError> { | ||
Ok(Self::update_portfolio_valuation_for_pool(pool_id)?.0) | ||
Self::update_portfolio_valuation_for_pool(pool_id, PriceCollectionInput::FromRegistry) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A quick comprehension question: Our rather newer NAV calculation runtime API calls There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was previously based on the price registry, so both lines above are exactly identical. From a pool fees perspective, nothing has changed. The only open door (right now) is for the RuntimeAPI. |
||
.map(|portfolio| portfolio.0) | ||
} | ||
|
||
fn initialise(_: OriginFor<T>, _: T::PoolId, _: T::ItemId) -> DispatchResult { | ||
|
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.
What is the benfit of using the
PriceCollectionInput
here vs just using aBTreeMap
as the parameter?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 can see each use case here: https://github.com/centrifuge/centrifuge-chain/pull/1748/files#diff-efd52f7fa525763070479bd9d72c66286451a6752bed8534e83e1004e755c3e1R469
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.
Empty
is just sugar forCustom([])
and could be removed. But the caller should be able to choose betweenCustom
andFromRegistry
.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 prefer @lemunozm's proposal over just using a
BTreeMap
as it is more explicit which is always helpful for runtime API which can be expected to be used by externals as well.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.
Okay, you changed the valuation parameters of the runtime-api. Sounds good!
The most important thing though would be to mimic closing an epoch with
Returning:
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.
Ok, I guess somehow this should be amalgamated with some other
pallet-pool-system
calls to get this, right?Then I propose to create another in the PoolSystem RuntimeAPI, maybe called simulate_close_epoch or similar, where all these calls are performed to return all the required values 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.
Great, I'll do in another PR pool-related.