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 interest hook #1059

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
30 changes: 29 additions & 1 deletion crates/loans/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use super::*;
use crate::{AccountBorrows, Pallet as Loans};

use frame_benchmarking::v2::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
use frame_benchmarking::v2::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller, Linear};
use frame_support::assert_ok;
use frame_system::{self, RawOrigin as SystemOrigin};
use oracle::Pallet as Oracle;
Expand Down Expand Up @@ -150,6 +150,34 @@ pub mod benchmarks {

use super::*;

#[benchmark]
pub fn on_initialize(u: Linear<1, 2>) {
Copy link
Member

Choose a reason for hiding this comment

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

This isn't being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to parameterize on_initialize on the number of markets, but since you pointed out that several extrinsics iterate over markets, it'd be cleaner to have a separate PR for this parameterization.

Copy link
Member

Choose a reason for hiding this comment

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

This one is easier to address though since on_initialize uses a dynamic weight. Even when we eventually set an upperbound it's still better to have on_initialize return the actual weight rather than the upper bound (especially since this is called every block)

initialize::<T>();

let caller: T::AccountId = whitelisted_caller();
transfer_initial_balance::<T>(caller.clone());
let deposit_amount: u32 = 200_000_000;
let borrowed_amount: u32 = 100_000_000;
assert_ok!(Loans::<T>::add_market(
SystemOrigin::Root.into(),
KBTC,
pending_market_mock::<T>(LEND_KBTC)
));
assert_ok!(Loans::<T>::activate_market(SystemOrigin::Root.into(), KBTC));
assert_ok!(Loans::<T>::mint(
SystemOrigin::Signed(caller.clone()).into(),
KBTC,
deposit_amount.into()
));

#[block]
{
// Begin second block, because the first block is set in
// `initialize::<T>()`
Loans::<T>::begin_block(2u32.into());
}
}

#[benchmark]
pub fn add_market() {
#[extrinsic_call]
Expand Down
975 changes: 501 additions & 474 deletions crates/loans/src/default_weights.rs

Large diffs are not rendered by default.

18 changes: 17 additions & 1 deletion crates/loans/src/interest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@
// limitations under the License.

use primitives::{Timestamp, SECONDS_PER_YEAR};
use sp_arithmetic::PerThing;
use sp_core::U256;
use sp_runtime::{traits::Zero, DispatchResult};

use crate::*;

impl<T: Config> Pallet<T> {
/// Accrue interest and update corresponding storage
/// - `asset_id`: Underlying currency to accrue interest for
#[cfg_attr(any(test, feature = "integration-tests"), visibility::make(pub))]
pub(crate) fn accrue_interest(asset_id: CurrencyId<T>) -> DispatchResult {
let now = T::UnixTime::now().as_secs();
Expand Down Expand Up @@ -147,7 +150,20 @@ impl<T: Config> Pallet<T> {
}
let total = cash.checked_add(&borrows)?.checked_sub(&reserves)?;

Ok(Ratio::from_rational(borrows.amount(), total.amount()))
// The code below essentially returns Ratio::from_rational(borrows, total).
// The reason why we don't use the aforementioned method is that is has some weird
// rounding behavior that can cause an unexpected decrease in the utilization ratio.
// For example, Ratio::from_rational(10000000057077, 100000000048516) returns 0.09999
// rather than 0.1
let borrows: U256 = borrows.amount().into(); // convert to u256 so we can safely multiply by ACCURACY
let raw_ratio = borrows
.saturating_mul(Ratio::ACCURACY.into())
.checked_div(total.amount().into())
.ok_or(ArithmeticError::DivisionByZero)?
.try_into()
.map_err(|_| currency::Error::<T>::TryIntoIntError)?;

Ok(Ratio::from_parts(raw_ratio))
}

/// The exchange rate should be greater than the `MinExchangeRate` storage value and less than
Expand Down
24 changes: 24 additions & 0 deletions crates/loans/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,14 @@ pub mod pallet {
},
}

#[pallet::hooks]
impl<T: Config> Hooks<T::BlockNumber> for Pallet<T> {
fn on_initialize(n: T::BlockNumber) -> Weight {
let iterations = Self::begin_block(n);
<T as Config>::WeightInfo::on_initialize(iterations)
}
}

/// The timestamp of the last calculation of accrued interest
#[pallet::storage]
#[pallet::getter(fn last_accrued_interest_time)]
Expand Down Expand Up @@ -1266,6 +1274,22 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
pub fn begin_block(_height: T::BlockNumber) -> u32 {
let mut iterations = 0;
for (asset_id, _) in Self::active_markets() {
iterations += 1;
if let Err(e) = Self::accrue_interest(asset_id) {
log::trace!(
target: "loans::accrue_interest",
"error: {:?}, failed to accrue interest for: {:?}",
e,
asset_id,
);
}
}
iterations
}

#[cfg_attr(any(test, feature = "integration-tests"), visibility::make(pub))]
fn account_deposits(lend_token_id: CurrencyId<T>, supplier: &T::AccountId) -> Amount<T> {
Amount::new(AccountDeposits::<T>::get(lend_token_id, supplier), lend_token_id)
Expand Down
2 changes: 1 addition & 1 deletion crates/loans/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,9 @@ pub(crate) fn _run_to_block(n: BlockNumber) {
Loans::on_finalize(System::block_number());
for b in (System::block_number() + 1)..=n {
System::set_block_number(b);
Loans::on_initialize(b);
TimestampPallet::set_timestamp(6000 * b);
Scheduler::on_initialize(b);
Loans::on_initialize(b);
if b != n {
Loans::on_finalize(b);
}
Expand Down
87 changes: 87 additions & 0 deletions crates/loans/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1577,3 +1577,90 @@ fn redeem_amount_matches_freed_underlying() {
);
})
}

fn read_interest_rates(
previous_supply_rate: &mut FixedU128,
previous_borrow_rate: &mut FixedU128,
current_supply_rate: &mut FixedU128,
current_borrow_rate: &mut FixedU128,
) {
*previous_supply_rate = *current_supply_rate;
*previous_borrow_rate = *current_borrow_rate;
*current_supply_rate = Loans::supply_rate(DOT);
*current_borrow_rate = Loans::borrow_rate(DOT);
}

#[test]
fn interest_rate_hook_works() {
new_test_ext().execute_with(|| {
let mut previous_supply_rate = Default::default();
let mut previous_borrow_rate = Default::default();

let mut current_supply_rate = Default::default();
let mut current_borrow_rate = Default::default();

// Run to block 1 and accrue interest so further accruals set interest rate storage
_run_to_block(1);
Loans::accrue_interest(DOT).unwrap();

// The hook on block 2 auto-accrues interest
_run_to_block(2);
// Mint and borrow so both interest rates will be non-zero
// This enables the re-accrual flag for next block
assert_ok!(Loans::mint(RuntimeOrigin::signed(ALICE), DOT, unit(100)));
assert_ok!(Loans::deposit_all_collateral(RuntimeOrigin::signed(ALICE), DOT));
assert_ok!(Loans::borrow(RuntimeOrigin::signed(ALICE), DOT, unit(10)));
read_interest_rates(
&mut previous_supply_rate,
&mut previous_borrow_rate,
&mut current_supply_rate,
&mut current_borrow_rate,
);

// The hook on block 3 auto-accrues interest
_run_to_block(3);
read_interest_rates(
&mut previous_supply_rate,
&mut previous_borrow_rate,
&mut current_supply_rate,
&mut current_borrow_rate,
);
// The previous block ended with a `borrow` that did not accrue interest.
// This means that re-accruing will increase both borrow and supply rates
// because market utilization increased.
assert!(current_supply_rate.gt(&previous_supply_rate));
assert!(current_borrow_rate.gt(&previous_borrow_rate));

// The hook on block 4 auto-accrues interest
_run_to_block(4);

read_interest_rates(
&mut previous_supply_rate,
&mut previous_borrow_rate,
&mut current_supply_rate,
&mut current_borrow_rate,
);
assert_eq!(current_supply_rate, previous_supply_rate);
assert_eq!(current_borrow_rate, previous_borrow_rate);
assert_ok!(Loans::mint(RuntimeOrigin::signed(ALICE), DOT, unit(100)));

// The hook on block 5 auto-accrues interest
_run_to_block(5);
read_interest_rates(
&mut previous_supply_rate,
&mut previous_borrow_rate,
&mut current_supply_rate,
&mut current_borrow_rate,
);
// Interacting with the market during this block will not re-accrue
assert_ok!(Loans::mint(RuntimeOrigin::signed(ALICE), DOT, unit(100)));
read_interest_rates(
&mut previous_supply_rate,
&mut previous_borrow_rate,
&mut current_supply_rate,
&mut current_borrow_rate,
);
assert_eq!(current_supply_rate, previous_supply_rate);
assert_eq!(current_borrow_rate, previous_borrow_rate);
});
}
4 changes: 3 additions & 1 deletion crates/loans/src/tests/edge_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ fn prevent_the_exchange_rate_attack() {
);
TimestampPallet::set_timestamp(12000);
// Eve can not let the exchange rate greater than 1
assert_noop!(Loans::accrue_interest(Token(DOT)), Error::<Test>::InvalidExchangeRate);
// Use `assert_err` instead of `assert_noop` because the `MarketToReaccrue`
// storage item may be mutated even if interest accrual fails.
assert_err!(Loans::accrue_interest(Token(DOT)), Error::<Test>::InvalidExchangeRate);

// Mock a BIG exchange_rate: 100000000000.02
ExchangeRate::<Test>::insert(Token(DOT), Rate::saturating_from_rational(100000000000020u128, 20 * 50));
Expand Down
9 changes: 9 additions & 0 deletions crates/loans/src/tests/interest_rate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ fn utilization_rate_works() {
Loans::calc_utilization_ratio(&ksm(0), &ksm(1), &ksm(0)).unwrap(),
Ratio::from_percent(100)
);

// Ratio::from_rational(10000000057077, 100000000048516) returns 0.09999
// check that calc_utilization_ratio correctly returns 0.1
let cash = 100000000048516u128 - 10000000057077u128;
let borrows = 10000000057077u128;
assert_eq!(
Loans::calc_utilization_ratio(&ksm(cash), &ksm(borrows), &ksm(0)).unwrap(),
Ratio::from_percent(10)
);
}

#[test]
Expand Down
Loading