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.

Sign up for GitHub

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

Fix: pallet-pool-system - Ratio Calculation #1856

Merged
merged 10 commits into from
Jun 6, 2024
Merged

Fix: pallet-pool-system - Ratio Calculation #1856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
178fa27
fix: fix wrong ratio calculation
mustermeiszer Jun 4, 2024
8596d6c
fix: wrong calculation and add test
mustermeiszer Jun 4, 2024
4bdfa0f
chore: address review comments
mustermeiszer Jun 4, 2024
740d032
fix: tests failing due to missing import
mustermeiszer Jun 4, 2024
7886f40
chore: make cleaner?
mustermeiszer Jun 5, 2024
417c672
fix: Perquintill default to 1, 1 tranche test
mustermeiszer Jun 5, 2024
0c0cb61
fix: last tests and risk buffer calc
mustermeiszer Jun 5, 2024
4e7c3fd
fix: test missing rate check
mustermeiszer Jun 5, 2024
a795aa3
Merge remote-tracking branch 'origin/main' into fix/pool-system/ratio…
mustermeiszer Jun 5, 2024
d175010
fix: clippy v1
mustermeiszer Jun 6, 2024
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
47 changes: 38 additions & 9 deletions pallets/pool-system/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ pub mod pallet {
traits::{tokens::Preservation, Contains, EnsureOriginWithArg},
PalletId,
};
use rev_slice::SliceExt;
use sp_runtime::{traits::BadOrigin, ArithmeticError};

use super::*;
Expand Down Expand Up @@ -1239,15 +1240,43 @@ pub mod pallet {

let executed_amounts = epoch.tranches.fulfillment_cash_flows(solution)?;
let total_assets = epoch.nav.total(pool.reserve.total)?;
let tranche_ratios = epoch.tranches.combine_with_residual_top(
&executed_amounts,
|tranche, &(invest, redeem)| {
Ok(Perquintill::from_rational(
tranche.supply.ensure_add(invest)?.ensure_sub(redeem)?,
total_assets,
))
},
)?;

let tranche_ratios = {
let mut sum_non_residual_tranche_ratios = Perquintill::zero();
let num_tranches = pool.tranches.num_tranches();
let mut current_tranche = 1;
let mut ratios = epoch
.tranches
// NOTE: Reversing amounts, as residual amount is on top.
.combine_with_non_residual_top(
executed_amounts.rev(),
|tranche, &(invest, redeem)| {
// NOTE: Need to have this clause as the current Perquintill
// implementation defaults to 100% if the denominator is zero
let ratio = if total_assets.is_zero() {
Perquintill::zero()
} else if current_tranche < num_tranches {
Perquintill::from_rational(
tranche.supply.ensure_add(invest)?.ensure_sub(redeem)?,
total_assets,
)
} else {
Perquintill::one().ensure_sub(sum_non_residual_tranche_ratios)?
Copy link
Contributor

Choose a reason for hiding this comment

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

The sum of tranche ratios can never be higher than one, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line is annulated by line 1263. No matter which value sum_non_residual_tranche_ratios has, at the end of this iteration sum_non_residual_tranche_ratios will always have the value one(). Is this the expected behavior?

};

sum_non_residual_tranche_ratios.ensure_add_assign(ratio)?;
current_tranche.ensure_add_assign(1)?;

Ok(ratio)
},
)?;

// NOTE: We need to reverse the ratios here, as the residual tranche is on top
// all the time
ratios.reverse();

ratios
};

pool.tranches.rebalance_tranches(
T::Time::now(),
Expand Down
7 changes: 4 additions & 3 deletions pallets/pool-system/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ use cfg_types::{
time::TimeProvider,
tokens::{CurrencyId, CustomMetadata, TrancheCurrency},
};
#[cfg(feature = "runtime-benchmarks")]
use frame_support::dispatch::RawOrigin;
use frame_support::{
assert_ok, derive_impl,
dispatch::RawOrigin,
parameter_types,
assert_ok, derive_impl, parameter_types,
traits::{Contains, EnsureOriginWithArg, Hooks, PalletInfoAccess, SortedMembers},
Blake2_128, PalletId, StorageHasher,
};
Expand Down Expand Up @@ -506,6 +506,7 @@ fn create_tranche_id(pool: u64, tranche: u64) -> [u8; 16] {
parameter_types! {
pub JuniorTrancheId: [u8; 16] = create_tranche_id(0, 0);
pub SeniorTrancheId: [u8; 16] = create_tranche_id(0, 1);
pub SecondSeniorTrancheId: [u8; 16] = create_tranche_id(0, 2);
}

// Build genesis storage according to the mock runtime.
Expand Down
198 changes: 127 additions & 71 deletions pallets/pool-system/src/tests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ use crate::{
UnhealthyState,
};

mod ratios;

const AUSD_CURRENCY_ID: CurrencyId = CurrencyId::ForeignAsset(1);

pub mod util {
Expand All @@ -56,34 +58,88 @@ pub mod util {
pub mod default_pool {
use super::*;

pub fn one_tranche_input() -> Vec<TrancheInput<Rate, StringLimit>> {
vec![TrancheInput {
tranche_type: TrancheType::Residual,
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
}]
}

pub fn three_tranche_input() -> Vec<TrancheInput<Rate, StringLimit>> {
vec![
TrancheInput {
tranche_type: TrancheType::Residual,
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
},
TrancheInput {
tranche_type: TrancheType::NonResidual {
interest_rate_per_sec: Rate::default(),
min_risk_buffer: Perquintill::from_percent(20),
},
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
},
TrancheInput {
tranche_type: TrancheType::NonResidual {
interest_rate_per_sec: Rate::default(),
min_risk_buffer: Perquintill::from_percent(25),
},
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
},
]
}

pub fn two_tranche_input() -> Vec<TrancheInput<Rate, StringLimit>> {
vec![
TrancheInput {
tranche_type: TrancheType::Residual,
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
},
TrancheInput {
tranche_type: TrancheType::NonResidual {
interest_rate_per_sec: Rate::default(),
min_risk_buffer: Perquintill::from_percent(25),
},
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
},
]
}

pub fn create() {
create_with_tranche_input(two_tranche_input())
}

pub fn create_with_tranche_input(input: Vec<TrancheInput<Rate, StringLimit>>) {
PoolSystem::create(
DEFAULT_POOL_OWNER,
DEFAULT_POOL_OWNER,
DEFAULT_POOL_ID,
vec![
TrancheInput {
tranche_type: TrancheType::Residual,
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
},
TrancheInput {
tranche_type: TrancheType::NonResidual {
interest_rate_per_sec: Rate::default(),
min_risk_buffer: Perquintill::default(),
},
seniority: None,
metadata: TrancheMetadata {
token_name: BoundedVec::default(),
token_symbol: BoundedVec::default(),
},
},
],
input,
AUSD_CURRENCY_ID,
0,
10_000 * CURRENCY,
vec![],
)
.unwrap();
Expand All @@ -95,7 +151,7 @@ pub mod util {
// forcing to call `execute_epoch()` later.
Investments::update_invest_order(
RuntimeOrigin::signed(0),
TrancheCurrency::generate(0, JuniorTrancheId::get()),
TrancheCurrency::generate(0, SeniorTrancheId::get()),
500 * CURRENCY,
)
.unwrap();
Expand Down Expand Up @@ -818,63 +874,72 @@ fn submission_period() {
// Not allowed as it breaks the min risk buffer, and the current state isn't
// broken
let epoch = <pallet::EpochExecution<mock::Runtime>>::try_get(0).unwrap();
assert_err!(
PoolSystem::submit_solution(
pool_owner_origin.clone(),
0,
vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
}
]
),
Error::<Runtime>::InvalidSolution
);

let existing_state_score = PoolSystem::score_solution(
&crate::Pool::<Runtime>::try_get(0).unwrap(),
&epoch,
&epoch.clone().best_submission.unwrap().solution(),
)
.unwrap();

let new_solution = vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::from_rational(9u64, 10),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
},
];

let new_solution_score = PoolSystem::score_solution(
&crate::Pool::<Runtime>::try_get(0).unwrap(),
&epoch,
&vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
},
],
&new_solution,
)
.unwrap();
assert_eq!(existing_state_score.healthy(), true);
assert_eq!(new_solution_score.healthy(), false);
assert_eq!(new_solution_score < existing_state_score, true);

// Is error as would put pool in unhealthy state
assert_err!(
PoolSystem::submit_solution(
pool_owner_origin.clone(),
0,
vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
}
]
),
PoolSystem::submit_solution(pool_owner_origin.clone(), 0, new_solution,),
Error::<Runtime>::NotNewBestSubmission
);

// Allowed as 1% redemption keeps the risk buffer healthy
let new_solution = vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::from_rational(7u64, 10),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
},
];
let partial_fulfilment_solution = PoolSystem::score_solution(
&crate::Pool::<Runtime>::try_get(0).unwrap(),
&epoch,
&vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::from_float(0.01),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
},
],
&new_solution,
)
.unwrap();
assert_eq!(partial_fulfilment_solution.healthy(), true);
Expand All @@ -883,16 +948,7 @@ fn submission_period() {
assert_ok!(PoolSystem::submit_solution(
pool_owner_origin.clone(),
0,
vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::from_float(0.01),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::one(),
}
]
new_solution
));

// Can submit the same solution twice
Expand All @@ -902,7 +958,7 @@ fn submission_period() {
vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::from_float(0.01),
redeem_fulfillment: Perquintill::from_rational(7u64, 10),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
Expand All @@ -911,14 +967,14 @@ fn submission_period() {
]
));

// Slight risk buffer improvement
// Risk buffer not touched, so increase in redemption is better
assert_ok!(PoolSystem::submit_solution(
pool_owner_origin.clone(),
0,
vec![
TrancheSolution {
invest_fulfillment: Perquintill::one(),
redeem_fulfillment: Perquintill::from_float(0.10),
redeem_fulfillment: Perquintill::from_rational(8u64, 10),
},
TrancheSolution {
invest_fulfillment: Perquintill::one(),
Expand Down
Loading
Loading