-
Notifications
You must be signed in to change notification settings - Fork 81
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: Add oracles to development runtime #1377
Conversation
Thanks for the review! It's not ready yet because of this: open-web3-stack/open-runtime-module-library#919 But I think I can get a workaround in the meantime. |
9c6e7eb
to
cec4198
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some important comments below:
@@ -273,3 +284,110 @@ pub mod xcm { | |||
} | |||
} | |||
} | |||
|
|||
pub mod oracle { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost this entire module can be removed once open-web3-stack/open-runtime-module-library#920 and open-web3-stack/open-runtime-module-library#904 be merged, and we use them.
I've preferred not using a fork of their repo because dealing with upgrades and dependency can be a little bit nightmare. The code of this module is a bit dirty, but it's a workaround to deal with it until we can use them directly in v0.9.42 (I think). I've tried to extract almost all dirt to the same place here to handle it easier.
// This restriction no longer exists once | ||
// https://github.com/open-web3-stack/open-runtime-module-library/pull/920 is merged | ||
let feeder = account("feeder", i, 0); | ||
T::PriceRegistry::feed_value(feeder, price_id, 0.into()).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite tricky. Because of how oracles are implemented, the same user can not feed twice in the same block. And the internal trait they use (DataFeeder
) only allows us to feed one price id each time.
That's why we need to create a new account per the price we want price here. And consequently, we need to mock the member's pallet in the runtime. To allow to use all these users.
MaxActiveLoansPerPool::get() | ||
); | ||
pub const MaxPoolsWithExternalPrices: u32 = 50; | ||
pub RootOperatorOraclePrice: AccountId = PRICE_ORACLE_PALLET_ID.into_account_truncating(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this can be used from outside without the private key. But I need to put some account as a root.
@@ -1573,7 +1573,7 @@ impl pallet_loans_ref::Config for Runtime { | |||
type NonFungible = Uniques; | |||
type Permissions = Permissions; | |||
type Pool = PoolSystem; | |||
type PriceId = PriceId; | |||
type PriceId = u32; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why make it a raw type again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our real type is OracleKey
. It doesn't matter what you put as type there in altair & centrifuge runtimes because the price feature is not allowed as long as NoPriceRegistry
type is attached to the loan pallet configuration.
Basically, I removed the PriceId
from cfg-primitives
and I placed an u32
as a placeholder.
Description
This PR adds price support to
pallet-loans
fordevelopment
runtime.Fixes #1279
Changes and Descriptions
pallet-membership
pallet to handle oracle members.orml-oracle
pallet to fetch pricespallet-data-collector
to organize prices in collections.runtime-common
.