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: bug on oracle fetching #1750

Merged
merged 12 commits into from
Mar 1, 2024
Merged

fix: bug on oracle fetching #1750

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1cf112d
fix: bug on oracle fetching
mustermeiszer Feb 29, 2024
b1734f4
feat: tests for oracle provider. WTF...
mustermeiszer Feb 29, 2024
0da86c3
feat: enable for runtimes
mustermeiszer Feb 29, 2024
5043048
Merge remote-tracking branch 'origin/main' into fix/oracle-local-repr…
mustermeiszer Feb 29, 2024
4c2eb84
chore: add no-std feat gate migration tuples
wischli Feb 29, 2024
c1530b5
fix: compilation
wischli Mar 1, 2024
9e34107
feat: final tests
mustermeiszer Mar 1, 2024
39a672d
chore: rm unused test mod
mustermeiszer Mar 1, 2024
4f41f24
fix: change request
mustermeiszer Mar 1, 2024
edccc3b
fix: review request
mustermeiszer Mar 1, 2024
d37f122
fix: unused import
mustermeiszer Mar 1, 2024
497d704
fix: integration tests
mustermeiszer Mar 1, 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
4 changes: 4 additions & 0 deletions runtime/altair/src/migrations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ frame_support::parameter_types! {

/// The migration set for Altair 1034 @ Kusama. It includes all the migrations
/// that have to be applied on that chain.
#[cfg(not(feature = "std"))]
pub type UpgradeAltair1034 = (
// Updates asset custom metadata from mid 2023 to latest (two fields missing/mismatching)
translate_asset_metadata::Migration<super::Runtime>,
Expand Down Expand Up @@ -90,6 +91,9 @@ pub type UpgradeAltair1034 = (
runtime_common::migrations::increase_storage_version::Migration<crate::Council, 0, 4>,
);

#[cfg(feature = "std")]
pub type UpgradeAltair1034 = ();

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe there exists a way to tune RuntimeEnv to avoid this issue from the tests? I do not like adding this addition to the runtime too much with the aim of avoiding migrations in testing. Future readers of this will not know why this is so, and we can be subject to unexpected behavior in the future. I would add a comment at least explain why this

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option is tweaking the block creation in RuntimeEnv somehow to skip the migration. Not sure if this is something easy to do

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is my forced change which should not have been pushed to this branch in the first place, I will just revert it. However, it improves integration test time because it blocks migrations for each node we spawn.

What unexpected behaviour are you fearing here @lemunozm? I think it is unexpected that migrations run for integration tests which is fixed this way. It's not the nicest solution but it does the job quickly without imposing any risk.

Copy link
Collaborator Author

@mustermeiszer mustermeiszer Mar 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed in 4f41f24

Copy link
Contributor

Choose a reason for hiding this comment

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

However, it improves integration test time because it blocks migrations for each node we spawn.

Good point!

What unexpected behaviour are you fearing here @lemunozm?

Could some uses of the node underlying it exist that compile the runtime as std and where do we expect some kind of migration, maybe some migration testing or so? Probably not now but it scared me 🙏🏻

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in 4f41f24

Love the solution! Maybe this can even be ported to fudge to avoid migrations there too (not mandatory for this PR)

#[allow(clippy::upper_case_acronyms)]
#[derive(
Clone,
Expand Down
14 changes: 3 additions & 11 deletions runtime/centrifuge/src/migrations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ frame_support::parameter_types! {
pub const AnnualTreasuryInflationPercent: u32 = 3;
}

#[cfg(not(feature = "std"))]
pub type UpgradeCentrifuge1025 = (
runtime_common::migrations::epoch_execution::Migration<super::Runtime>,
// Migrates the currency used in `pallet-transfer-allowlist` from our global currency to a
Expand Down Expand Up @@ -89,17 +90,8 @@ pub type UpgradeCentrifuge1025 = (
burn_unburned::Migration<super::Runtime>,
);

// Copyright 2021 Centrifuge Foundation (centrifuge.io).
//
// This file is part of the Centrifuge chain project.
// Centrifuge is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version (see http://www.gnu.org/licenses).
// Centrifuge is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
#[cfg(feature = "std")]
pub type UpgradeCentrifuge1025 = ();

mod burn_unburned {
const LOG_PREFIX: &str = "BurnUnburnedMigration: ";
Expand Down
4 changes: 2 additions & 2 deletions runtime/common/src/oracle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ where
(from, to): &(CurrencyId, CurrencyId),
) -> Result<Option<Self::Value>, DispatchError> {
let locally_coupled_assets = match (from, to) {
(_, &CurrencyId::LocalAsset(_)) => from.is_local_representation_of(to),
(&CurrencyId::LocalAsset(_), _) => to.is_local_representation_of(from),
(_, &CurrencyId::LocalAsset(_)) => to.is_local_representation_of(from),
(&CurrencyId::LocalAsset(_), _) => from.is_local_representation_of(to),
_ => Ok(false),
}?;

Expand Down
4 changes: 4 additions & 0 deletions runtime/development/src/migrations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ frame_support::parameter_types! {
pub const AnnualTreasuryInflationPercent: u32 = 3;
}

#[cfg(not(feature = "std"))]
pub type UpgradeDevelopment1042 = (
// Register LocalUSDC
runtime_common::migrations::local_currency::register::Migration<
Expand Down Expand Up @@ -50,3 +51,6 @@ pub type UpgradeDevelopment1042 = (
AnnualTreasuryInflationPercent,
>,
);

#[cfg(feature = "std")]
pub type UpgradeDevelopment1042 = ();
22 changes: 11 additions & 11 deletions runtime/integration-tests/src/generic/cases/investments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ mod common {
let mut env = E::from_parachain_storage(
Genesis::<T>::default()
.add(genesis::balances(T::ExistentialDeposit::get() + FOR_FEES))
.add(genesis::assets(vec![Usd6::ID]))
.add(genesis::tokens(vec![(Usd6::ID, Usd6::ED)]))
.add(genesis::assets(vec![Box::new(Usd6)]))
.add(genesis::tokens(vec![(Usd6.id(), Usd6.ed())]))
.storage(),
);

env.parachain_state_mut(|| {
// Create a pool
utils::give_balance::<T>(POOL_ADMIN.id(), T::PoolDeposit::get());
utils::create_empty_pool::<T>(POOL_ADMIN.id(), POOL_A, Usd6::ID);
utils::create_empty_pool::<T>(POOL_ADMIN.id(), POOL_A, Usd6.id());

// Grant permissions
let tranche_id = T::Api::tranche_id(POOL_A, 0).unwrap();
Expand All @@ -73,7 +73,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {

// Invest to have pending pool currency
env.parachain_state_mut(|| {
utils::give_tokens::<T>(INVESTOR.id(), Usd6::ID, EXPECTED_POOL_BALANCE);
utils::give_tokens::<T>(INVESTOR.id(), Usd6.id(), EXPECTED_POOL_BALANCE);
utils::invest::<T>(INVESTOR.id(), POOL_A, tranche_id, EXPECTED_POOL_BALANCE);
assert_eq!(
pallet_investments::InvestOrders::<T>::get(INVESTOR.id(), invest_id)
Expand All @@ -87,7 +87,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {
investment_portfolio,
vec![(
invest_id,
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6::ID)
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6.id())
.with_pending_invest_currency(EXPECTED_POOL_BALANCE)
)]
);
Expand All @@ -102,7 +102,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {
investment_portfolio,
vec![(
invest_id,
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6::ID)
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6.id())
.with_claimable_tranche_tokens(EXPECTED_POOL_BALANCE)
)]
);
Expand All @@ -116,7 +116,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {
investment_portfolio,
vec![(
invest_id,
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6::ID)
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6.id())
.with_free_tranche_tokens(EXPECTED_POOL_BALANCE)
)]
);
Expand All @@ -130,7 +130,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {
investment_portfolio,
vec![(
invest_id,
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6::ID)
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6.id())
.with_free_tranche_tokens(EXPECTED_POOL_BALANCE - REDEEM_AMOUNT)
.with_pending_redeem_tranche_tokens(REDEEM_AMOUNT)
)]
Expand All @@ -146,7 +146,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {
investment_portfolio,
vec![(
invest_id,
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6::ID)
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6.id())
.with_free_tranche_tokens(EXPECTED_POOL_BALANCE - REDEEM_AMOUNT)
.with_claimable_currency(REDEEM_AMOUNT)
)]
Expand All @@ -161,7 +161,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {
investment_portfolio,
vec![(
invest_id,
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6::ID)
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6.id())
.with_free_tranche_tokens(EXPECTED_POOL_BALANCE - REDEEM_AMOUNT)
)]
);
Expand All @@ -181,7 +181,7 @@ fn investment_portfolio_single_tranche<T: Runtime>() {
investment_portfolio,
vec![(
invest_id,
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6::ID)
InvestmentPortfolio::<Balance, CurrencyId>::new(Usd6.id())
.with_free_tranche_tokens(EXPECTED_POOL_BALANCE - REDEEM_AMOUNT - HOLD_AMOUNT)
.with_reserved_tranche_tokens(HOLD_AMOUNT)
)]
Expand Down
24 changes: 16 additions & 8 deletions runtime/integration-tests/src/generic/cases/loans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ mod common {
let mut env = E::from_parachain_storage(
Genesis::<T>::default()
.add(genesis::balances(T::ExistentialDeposit::get() + FOR_FEES))
.add(genesis::assets(vec![Usd6::ID]))
.add(genesis::tokens(vec![(Usd6::ID, Usd6::ED)]))
.add(genesis::assets(vec![Box::new(Usd6)]))
.add(genesis::tokens(vec![(Usd6.id(), Usd6.ed())]))
.storage(),
);

env.parachain_state_mut(|| {
// Creating a pool
utils::give_balance::<T>(POOL_ADMIN.id(), T::PoolDeposit::get());
utils::create_empty_pool::<T>(POOL_ADMIN.id(), POOL_A, Usd6::ID);
utils::create_empty_pool::<T>(POOL_ADMIN.id(), POOL_A, Usd6.id());

// Setting borrower
utils::give_pool_role::<T>(BORROWER.id(), POOL_A, PoolRole::Borrower);
Expand All @@ -94,7 +94,7 @@ mod common {
let tranche_id = T::Api::tranche_id(POOL_A, 0).unwrap();
let tranche_investor = PoolRole::TrancheInvestor(tranche_id, Seconds::MAX);
utils::give_pool_role::<T>(INVESTOR.id(), POOL_A, tranche_investor);
utils::give_tokens::<T>(INVESTOR.id(), Usd6::ID, EXPECTED_POOL_BALANCE);
utils::give_tokens::<T>(INVESTOR.id(), Usd6.id(), EXPECTED_POOL_BALANCE);
utils::invest::<T>(INVESTOR.id(), POOL_A, tranche_id, EXPECTED_POOL_BALANCE);
});

Expand Down Expand Up @@ -161,13 +161,13 @@ mod common {
Pricing::External(ExternalPricing {
price_id: PRICE_A,
max_borrow_amount: ExtMaxBorrowAmount::Quantity(QUANTITY),
notional: currency::price_to_currency(PRICE_VALUE_A, Usd6::ID),
notional: currency::price_to_currency(PRICE_VALUE_A, Usd6),
max_price_variation: rate_from_percent(50),
})
}

pub fn price_to_usd6(price: Quantity) -> Balance {
currency::price_to_currency(price, Usd6::ID)
currency::price_to_currency(price, Usd6)
}
}

Expand Down Expand Up @@ -286,7 +286,11 @@ fn internal_priced<T: Runtime>() {
env.parachain_state_mut(|| {
// Give required tokens to the borrower to be able to repay the interest accrued
// until this moment
utils::give_tokens::<T>(BORROWER.id(), Usd6::ID, loan_portfolio.outstanding_interest);
utils::give_tokens::<T>(
BORROWER.id(),
Usd6.id(),
loan_portfolio.outstanding_interest,
);
});

env.submit_now(
Expand Down Expand Up @@ -326,7 +330,11 @@ fn oracle_priced<T: Runtime>() {
env.parachain_state_mut(|| {
// Give required tokens to the borrower to be able to repay the interest accrued
// until this moment
utils::give_tokens::<T>(BORROWER.id(), Usd6::ID, loan_portfolio.outstanding_interest);
utils::give_tokens::<T>(
BORROWER.id(),
Usd6.id(),
loan_portfolio.outstanding_interest,
);

// Oracle modify the value
utils::oracle::feed_from_root::<T>(PRICE_A, PRICE_VALUE_B);
Expand Down
Loading
Loading