diff --git a/libs/traits/src/ops.rs b/libs/traits/src/ops.rs index 4006cd43de..df0af1330a 100644 --- a/libs/traits/src/ops.rs +++ b/libs/traits/src/ops.rs @@ -4,6 +4,24 @@ pub use ensure::{ EnsureSubAssign, }; +pub mod storage { + use sp_runtime::{traits::Get, BoundedVec}; + + pub trait BoundedVecExt { + /// Same as [`BoundedVec::try_push()`] but returns a reference to the pushed element in the + /// vector + fn try_push_fetch(&mut self, element: T) -> Result<&mut T, T>; + } + + impl> BoundedVecExt for BoundedVec { + fn try_push_fetch(&mut self, element: T) -> Result<&mut T, T> { + let len = self.len(); + self.try_push(element)?; + Ok(self.get_mut(len).expect("This can not fail. qed")) + } + } +} + mod ensure { use sp_runtime::{ traits::{CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Zero}, diff --git a/pallets/loans-ref/src/lib.rs b/pallets/loans-ref/src/lib.rs index fff7f91a94..501beca706 100644 --- a/pallets/loans-ref/src/lib.rs +++ b/pallets/loans-ref/src/lib.rs @@ -57,7 +57,7 @@ pub use weights::WeightInfo; pub mod pallet { use cfg_primitives::Moment; use cfg_traits::{ - ops::{EnsureAdd, EnsureAddAssign, EnsureInto}, + ops::{storage::BoundedVecExt, EnsureAdd, EnsureAddAssign, EnsureInto, EnsureSub}, InterestAccrual, Permissions, PoolInspect, PoolNAV, PoolReserve, }; use cfg_types::{ @@ -79,7 +79,7 @@ pub mod pallet { use sp_arithmetic::FixedPointNumber; use sp_runtime::{ traits::{BadOrigin, One, Zero}, - ArithmeticError, FixedPointOperand, + ArithmeticError, BoundedBTreeMap, FixedPointOperand, }; use types::{ self, ActiveLoan, AssetOf, BorrowLoanError, CloseLoanError, CreateLoanError, LoanInfoOf, @@ -131,6 +131,7 @@ pub mod pallet { + MaxEncodedLen + Copy + EnsureAdd + + Ord + One; /// Defines the rate type used for math computations @@ -209,7 +210,25 @@ pub mod pallet { _, Blake2_128Concat, PoolIdOf, - BoundedVec<(ActiveLoan, Moment), T::MaxActiveLoansPerPool>, + BoundedBTreeMap, T::MaxActiveLoansPerPool>, + ValueQuery, + >; + + /// Storage for mutated active loans. + /// A mutated active loan is an active loan that has changed from the last + /// `update_portfolio_valuation()` call. + /// This storage is used to avoid PoV Weights in the probably large `ActiveLoans` storage. + /// Each `ActiveLoan` change is done over a copy of that loan in this storage which implies + /// to statistically write less bytes (there are less mutated active loans than total active + /// loans). + /// Once `update_portfolio_valuation()` is called, the `ActiveLoans` storage is updated with + /// the `MutatedActiveLoans` content and this last storage is cleaned. + #[pallet::storage] + pub(crate) type MutatedActiveLoans = StorageMap< + _, + Blake2_128Concat, + PoolIdOf, + BoundedVec<(T::LoanId, ActiveLoan, Moment), T::MaxActiveLoansPerPool>, ValueQuery, >; @@ -400,10 +419,10 @@ pub mod pallet { Some(created_loan) => { Self::ensure_loan_borrower(&who, created_loan.borrower())?; - let mut active_loan = created_loan.activate(loan_id)?; + let mut active_loan = created_loan.activate()?; active_loan.borrow(amount)?; - Self::insert_active_loan(pool_id, active_loan)? + Self::insert_active_loan(pool_id, loan_id, active_loan)? } None => { Self::update_active_loan(pool_id, loan_id, |loan| { @@ -713,20 +732,32 @@ pub mod pallet { pool_id: PoolIdOf, ) -> Result<(T::Balance, u32), DispatchError> { let rates = T::InterestAccrual::rates(); - let loans = ActiveLoans::::get(pool_id); - let count = loans.len().ensure_into()?; - let value = loans.into_iter().try_fold( - T::Balance::zero(), - |sum, (loan, _)| -> Result { - Ok(sum.ensure_add(loan.current_present_value(&rates)?)?) - }, - )?; - - Ok((value, count)) + + ActiveLoans::::try_mutate(pool_id, |loans| { + for (loan_id, loan, _) in MutatedActiveLoans::::take(pool_id) { + loans + .try_insert(loan_id, loan) + .map_err(|_| Error::::MaxActiveLoansReached)?; + } + + let value = loans.values().try_fold( + T::Balance::zero(), + |sum, loan| -> Result { + Ok(sum.ensure_add(loan.current_present_value(&rates)?)?) + }, + )?; + + // TODO: How to get the correct count to benchmark this accurated? + // Should it be MutatedActiveLoans.len() + ActiveLoans.len()? + let count = loans.len().ensure_into()?; + + Ok((value, count)) + }) } fn insert_active_loan( pool_id: PoolIdOf, + loan_id: T::LoanId, loan: ActiveLoan, ) -> Result { PortfolioValuation::::try_mutate(pool_id, |portfolio| { @@ -735,12 +766,11 @@ pub mod pallet { Self::update_portfolio_valuation_with_pv(pool_id, portfolio, Zero::zero(), new_pv)?; - ActiveLoans::::try_mutate(pool_id, |active_loans| { - active_loans - .try_push((loan, last_updated)) + MutatedActiveLoans::::try_mutate(pool_id, |mutated_loans| { + mutated_loans + .try_push((loan_id, loan, last_updated)) .map_err(|_| Error::::MaxActiveLoansReached)?; - - Ok(active_loans.len().ensure_into()?) + Ok(mutated_loans.len().ensure_into()?) }) }) } @@ -754,19 +784,29 @@ pub mod pallet { F: FnOnce(&mut ActiveLoan) -> Result, { PortfolioValuation::::try_mutate(pool_id, |portfolio| { - ActiveLoans::::try_mutate(pool_id, |active_loans| { - let (loan, last_updated) = active_loans - .iter_mut() - .find(|(loan, _)| loan.loan_id() == loan_id) - .ok_or_else(|| { - if CreatedLoan::::contains_key(pool_id, loan_id) { - Error::::LoanNotActive - } else { - Error::::LoanNotFound + MutatedActiveLoans::::try_mutate(pool_id, |mutated_loans| { + let (loan, last_updated) = + match mutated_loans.iter_mut().find(|(id, _, _)| *id == loan_id) { + Some((_, loan, last_updated)) => (loan, last_updated), + None => { + let loan = ActiveLoans::::get(pool_id) + .get(&loan_id) + .ok_or_else(|| { + if CreatedLoan::::contains_key(pool_id, loan_id) { + Error::::LoanNotActive + } else { + Error::::LoanNotFound + } + })? + .clone(); + + mutated_loans + .try_push_fetch((loan_id, loan, portfolio.last_updated())) + .map(|(_, loan, last_updated)| (loan, last_updated)) + .map_err(|_| Error::::MaxActiveLoansReached)? } - })?; + }; - *last_updated = (*last_updated).max(portfolio.last_updated()); let old_pv = loan.present_value_at(*last_updated)?; let result = f(loan)?; @@ -776,7 +816,7 @@ pub mod pallet { Self::update_portfolio_valuation_with_pv(pool_id, portfolio, old_pv, new_pv)?; - Ok((result, active_loans.len().ensure_into()?)) + Ok((result, mutated_loans.len().ensure_into()?)) }) }) } @@ -785,16 +825,24 @@ pub mod pallet { pool_id: PoolIdOf, loan_id: T::LoanId, ) -> Result<(ActiveLoan, u32), DispatchError> { - ActiveLoans::::try_mutate(pool_id, |active_loans| { - let index = active_loans - .iter() - .position(|(loan, _)| loan.loan_id() == loan_id) - .ok_or(Error::::LoanNotFound)?; - - Ok(( - active_loans.swap_remove(index).0, - active_loans.len().ensure_into()?, - )) + // A loan can be: + // - Only in MutatedActiveLoans, if it was just inserted + // - Only in ActiveLoans, if it was not modified since the portfolio valuation + // - In both, if it was modified after porfolio valuation + MutatedActiveLoans::::try_mutate(pool_id, |mutated_loans| { + ActiveLoans::::try_mutate(pool_id, |loans| { + let count = loans.len().ensure_add(mutated_loans.len())?.ensure_into()?; + + let loan = loans.remove(&loan_id); + let index = mutated_loans.iter().position(|(id, _, _)| *id == loan_id); + + let loan = match index { + Some(index) => mutated_loans.swap_remove(index).1, + None => loan.ok_or(Error::::LoanNotFound)?, + }; + + Ok((loan, count)) + }) }) } diff --git a/pallets/loans-ref/src/tests.rs b/pallets/loans-ref/src/tests.rs index c684a39ab9..a91c0c7973 100644 --- a/pallets/loans-ref/src/tests.rs +++ b/pallets/loans-ref/src/tests.rs @@ -6,7 +6,7 @@ use sp_runtime::traits::BadOrigin; use super::{ mock::*, - pallet::{ActiveLoans, Error, LastLoanId, PortfolioValuation}, + pallet::{ActiveLoans, Error, LastLoanId, MutatedActiveLoans, PortfolioValuation}, types::{ ActiveLoan, BorrowLoanError, CloseLoanError, CreateLoanError, LoanInfo, MaxBorrowAmount, WriteOffState, WriteOffStatus, WrittenOffError, @@ -35,11 +35,16 @@ mod util { } pub fn get_loan(loan_id: LoanId) -> ActiveLoan { - ActiveLoans::::get(POOL_A) + match MutatedActiveLoans::::get(POOL_A) .into_iter() - .find(|(loan, _)| loan.loan_id() == loan_id) - .unwrap() - .0 + .find(|(id, _, _)| *id == loan_id) + { + Some((_, loan, _)) => loan, + None => ActiveLoans::::get(POOL_A) + .get(&loan_id) + .unwrap() + .clone(), + } } pub fn portfolio_valuation() -> Balance { diff --git a/pallets/loans-ref/src/types.rs b/pallets/loans-ref/src/types.rs index 9f1b951250..967465c2b5 100644 --- a/pallets/loans-ref/src/types.rs +++ b/pallets/loans-ref/src/types.rs @@ -394,8 +394,8 @@ impl CreatedLoan { &self.borrower } - pub fn activate(self, loan_id: T::LoanId) -> Result, DispatchError> { - ActiveLoan::new(loan_id, self.info, self.borrower, T::Time::now().as_secs()) + pub fn activate(self) -> Result, DispatchError> { + ActiveLoan::new(self.info, self.borrower, T::Time::now().as_secs()) } pub fn close(self) -> Result<(ClosedLoan, T::AccountId), DispatchError> { @@ -437,9 +437,6 @@ impl ClosedLoan { #[derive(Encode, Decode, Clone, TypeInfo, MaxEncodedLen)] #[scale_info(skip_type_params(T))] pub struct ActiveLoan { - /// Id of this loan - loan_id: T::LoanId, - /// Loan information info: LoanInfoOf, @@ -464,13 +461,11 @@ pub struct ActiveLoan { impl ActiveLoan { pub fn new( - loan_id: T::LoanId, info: LoanInfoOf, borrower: T::AccountId, now: Moment, ) -> Result { Ok(ActiveLoan { - loan_id, info: LoanInfo { interest_rate: T::InterestAccrual::reference_yearly_rate(info.interest_rate)?, ..info @@ -484,10 +479,6 @@ impl ActiveLoan { }) } - pub fn loan_id(&self) -> T::LoanId { - self.loan_id - } - pub fn borrower(&self) -> &T::AccountId { &self.borrower }