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.

12 changes: 11 additions & 1 deletion crates/loans/src/interest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,18 @@ use crate::*;

impl<T: Config> Pallet<T> {
/// Accrue interest and update corresponding storage
/// - `asset_id`: Underlying currency to accrue interest for
/// - `reaccrue_next_block`: Re-accrue interest next block via a substrate hook, so that
/// redundant storage about interest rates is updated. This helps the UI reflect changes to the interest
/// rates caused by market interactions which happened this block. All extrinsics calling this function
/// must pass a `true` value for this argument, while the substrate hook must pass `false`.
#[cfg_attr(any(test, feature = "integration-tests"), visibility::make(pub))]
pub(crate) fn accrue_interest(asset_id: CurrencyId<T>) -> DispatchResult {
pub(crate) fn accrue_interest(asset_id: CurrencyId<T>, reaccrue_next_block: bool) -> DispatchResult {
// Ensure redundant storage always has up-to-date interest
// rates which the UI can display.
// This operations mutates storage even if accruing fails.
MarketToReaccrue::<T>::insert(asset_id, reaccrue_next_block);

let now = T::UnixTime::now().as_secs();
let last_accrued_interest_time = Self::last_accrued_interest_time(asset_id);
if last_accrued_interest_time.is_zero() {
Expand Down
56 changes: 44 additions & 12 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 @@ -506,6 +514,12 @@ pub mod pallet {
#[pallet::storage]
pub type Markets<T: Config> = StorageMap<_, Blake2_128Concat, CurrencyId<T>, Market<BalanceOf<T>>>;

/// Mapping of underlying currency id of a market to the flag indicating
/// whether the marked should have interest accrued in the `on_initialize` hook
#[pallet::storage]
#[pallet::getter(fn market_to_reaccrue)]
pub type MarketToReaccrue<T: Config> = StorageMap<_, Blake2_128Concat, CurrencyId<T>, bool, ValueQuery>;

/// Mapping of lend_token id to underlying currency id
/// `lend_token id`: voucher token id
/// `asset id`: underlying token id
Expand Down Expand Up @@ -1059,7 +1073,7 @@ pub mod pallet {
pub fn repay_borrow_all(origin: OriginFor<T>, asset_id: CurrencyId<T>) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin)?;
Self::ensure_active_market(asset_id)?;
Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;
let account_borrows = Self::current_borrow_balance(&who, asset_id)?;
ensure!(!account_borrows.is_zero(), Error::<T>::InvalidAmount);
Self::do_repay_borrow(&who, &account_borrows)?;
Expand Down Expand Up @@ -1137,8 +1151,8 @@ pub mod pallet {
collateral_asset_id: CurrencyId<T>,
) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin)?;
Self::accrue_interest(liquidation_asset_id)?;
Self::accrue_interest(collateral_asset_id)?;
Self::accrue_interest(liquidation_asset_id, true)?;
Self::accrue_interest(collateral_asset_id, true)?;
ensure!(!repay_amount.is_zero(), Error::<T>::InvalidAmount);
let liquidation = Amount::new(repay_amount, liquidation_asset_id);
Self::do_liquidate_borrow(who, borrower, &liquidation, collateral_asset_id)?;
Expand Down Expand Up @@ -1168,7 +1182,7 @@ pub mod pallet {
T::ReserveOrigin::ensure_origin(origin)?;
let payer = T::Lookup::lookup(payer)?;
Self::ensure_active_market(asset_id)?;
Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;

ensure!(!amount_to_transfer.is_zero(), Error::<T>::InvalidAmount);
amount_to_transfer.transfer(&payer, &Self::account_id())?;
Expand Down Expand Up @@ -1205,7 +1219,7 @@ pub mod pallet {
T::ReserveOrigin::ensure_origin(origin)?;
let receiver = T::Lookup::lookup(receiver)?;
Self::ensure_active_market(asset_id)?;
Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;

let amount_to_transfer = Amount::new(reduce_amount, asset_id);

Expand Down Expand Up @@ -1247,7 +1261,7 @@ pub mod pallet {
let receiver = T::Lookup::lookup(receiver)?;
let from = Self::incentive_reward_account_id();
Self::ensure_active_market(asset_id)?;
Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;

let underlying = Amount::new(redeem_amount, asset_id);

Expand All @@ -1266,6 +1280,24 @@ 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 Self::market_to_reaccrue(asset_id) {
if let Err(e) = Self::accrue_interest(asset_id, false) {
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 Expand Up @@ -1869,7 +1901,7 @@ impl<T: Config> LoansTrait<CurrencyId<T>, AccountIdOf<T>, Amount<T>> for Pallet<
Self::ensure_active_market(asset_id)?;
Self::ensure_under_supply_cap(&amount)?;

Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;

// update supply index before modify supply balance.
Self::update_reward_supply_index(asset_id)?;
Expand All @@ -1894,7 +1926,7 @@ impl<T: Config> LoansTrait<CurrencyId<T>, AccountIdOf<T>, Amount<T>> for Pallet<
let asset_id = borrow.currency();
Self::ensure_active_market(asset_id)?;

Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;
Self::borrow_allowed(borrower, &borrow)?;

// update borrow index after accrue interest.
Expand Down Expand Up @@ -1993,7 +2025,7 @@ impl<T: Config> LoansTrait<CurrencyId<T>, AccountIdOf<T>, Amount<T>> for Pallet<
fn do_repay_borrow(borrower: &AccountIdOf<T>, borrow: &Amount<T>) -> Result<(), DispatchError> {
let asset_id = borrow.currency();
Self::ensure_active_market(asset_id)?;
Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;
let account_borrows = Self::current_borrow_balance(borrower, asset_id)?;
Self::do_repay_borrow_with_amount(borrower, asset_id, &account_borrows, &borrow)?;
Self::deposit_event(Event::<T>::RepaidBorrow {
Expand All @@ -2016,7 +2048,7 @@ impl<T: Config> LoansTrait<CurrencyId<T>, AccountIdOf<T>, Amount<T>> for Pallet<
}

Self::ensure_active_market(asset_id)?;
Self::accrue_interest(asset_id)?;
Self::accrue_interest(asset_id, true)?;

Self::redeem_allowed(supplier, &voucher)?;

Expand Down Expand Up @@ -2048,7 +2080,7 @@ impl<T: Config> LoansTrait<CurrencyId<T>, AccountIdOf<T>, Amount<T>> for Pallet<
// outdated exchange rate. Call `accrue_interest` to avoid this.
let underlying_id = Self::underlying_id(lend_tokens.currency())?;
Self::ensure_active_market(underlying_id)?;
Self::accrue_interest(underlying_id)?;
Self::accrue_interest(underlying_id, true)?;
let exchange_rate = Self::exchange_rate_stored(underlying_id)?;
Ok(lend_tokens.checked_mul(&exchange_rate)?.set_currency(underlying_id))
}
Expand All @@ -2066,7 +2098,7 @@ impl<T: Config> LoansTrait<CurrencyId<T>, AccountIdOf<T>, Amount<T>> for Pallet<
// possibly not having accrued for a few blocks. This would result in using an
// outdated exchange rate. Call `accrue_interest` to avoid this.
Self::ensure_active_market(underlying.currency())?;
Self::accrue_interest(underlying.currency())?;
Self::accrue_interest(underlying.currency(), true)?;
let exchange_rate = Self::exchange_rate_stored(underlying.currency())?;

let lend_token_id = Self::lend_token_id(underlying.currency())?;
Expand Down
4 changes: 2 additions & 2 deletions 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 All @@ -404,7 +404,7 @@ pub fn almost_equal(target: u128, value: u128) -> bool {
pub fn accrue_interest_per_block(asset_id: CurrencyId, block_delta_secs: u64, run_to_block: u64) {
for i in 1..run_to_block {
TimestampPallet::set_timestamp(6000 + (block_delta_secs * 1000 * i));
Loans::accrue_interest(asset_id).unwrap();
Loans::accrue_interest(asset_id, false).unwrap();
}
}

Expand Down
101 changes: 98 additions & 3 deletions crates/loans/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ fn total_reserves_are_updated_on_withdrawal() {

let blocks_to_run = 1000;
_run_to_block(blocks_to_run);
Loans::accrue_interest(DOT).unwrap();
Loans::accrue_interest(DOT, false).unwrap();
let intermediary_total_reserves = Loans::total_reserves(DOT);
_run_to_block(2 * blocks_to_run);

Expand All @@ -764,7 +764,7 @@ fn total_reserves_are_updated_on_deposit() {

let blocks_to_run = 1000;
_run_to_block(blocks_to_run);
Loans::accrue_interest(DOT).unwrap();
Loans::accrue_interest(DOT, false).unwrap();
let intermediary_total_reserves = Loans::total_reserves(DOT);
_run_to_block(2 * blocks_to_run);

Expand Down Expand Up @@ -898,7 +898,7 @@ fn update_exchange_rate_works() {
// Initialize value of exchange rate is 0.02
assert_eq!(Loans::exchange_rate(DOT), Rate::saturating_from_rational(2, 100));

assert_ok!(Loans::accrue_interest(DOT));
assert_ok!(Loans::accrue_interest(DOT, false));
assert_eq!(
Loans::exchange_rate_stored(DOT).unwrap(),
Rate::saturating_from_rational(2, 100)
Expand Down Expand Up @@ -1577,3 +1577,98 @@ 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, false).unwrap();
// Accruing interest on the first block does not enable the flag
assert_eq!(Loans::market_to_reaccrue(DOT), false);

_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,
);
assert_eq!(Loans::market_to_reaccrue(DOT), true);

// The hook on block 3 auto-accrues interest so storage items are updated
_run_to_block(3);
assert_eq!(Loans::market_to_reaccrue(DOT), false);
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 does not auto-accrue 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);
// This enables the re-accrual flag for next block
assert_ok!(Loans::mint(RuntimeOrigin::signed(ALICE), DOT, unit(100)));
assert_eq!(Loans::market_to_reaccrue(DOT), true);

// The hook on block 5 accrues interest
_run_to_block(5);
assert_eq!(Loans::market_to_reaccrue(DOT), false);
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);
// But it should still enable the flag
assert_eq!(Loans::market_to_reaccrue(DOT), true);
});
}
11 changes: 8 additions & 3 deletions crates/loans/src/tests/edge_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,12 @@ 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), false),
Error::<Test>::InvalidExchangeRate
);

// Mock a BIG exchange_rate: 100000000000.02
ExchangeRate::<Test>::insert(Token(DOT), Rate::saturating_from_rational(100000000000020u128, 20 * 50));
Expand Down Expand Up @@ -297,7 +302,7 @@ fn small_loans_have_interest_rounded_up() {
assert_ok!(batch_call.clone().dispatch(RuntimeOrigin::signed(BOB)));

_run_to_block(initial_block + 10000);
Loans::accrue_interest(Token(IBTC)).unwrap();
Loans::accrue_interest(Token(IBTC), false).unwrap();
let borrow_index = Loans::borrow_index(Token(IBTC));
let current_borrow_balance = Loans::current_borrow_balance(&BOB, Token(IBTC)).unwrap();
let total_borrowed_amount = borrow_amount_small + borrow_amount_big;
Expand All @@ -323,7 +328,7 @@ fn big_loan_following_a_small_loan_still_accrues_interest() {
assert_ok!(Loans::borrow(RuntimeOrigin::signed(BOB), Token(IBTC), 1));

_run_to_block(initial_block + 1);
Loans::accrue_interest(Token(IBTC)).unwrap();
Loans::accrue_interest(Token(IBTC), false).unwrap();
// Interest gets accrued immediately (rounded up), to prevent
// giving out interest-free loans due to truncating the interest.
assert_eq!(Loans::current_borrow_balance(&BOB, Token(IBTC)).unwrap().amount(), 2);
Expand Down
Loading