-
Notifications
You must be signed in to change notification settings - Fork 1k
Add oracle pallet (part of Polkadot Stablecoin prerequisites) #9815
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
base: master
Are you sure you want to change the base?
Conversation
User @indirection42, please sign the CLA here. |
To any maintainers, kindly add a |
Note that this is a centralized oracle pallet + some onchain data aggregation logic. We will use the traits and the data aggregation logic but may not use the centralized oracle feeding pallet in the final polkadot asset hub runtime. However, they will be used in test runtimes (e.g. westend AH), which can't get real price otherwise. |
/cmd fmt |
Command "fmt" has failed ❌! See logs here |
use codec::Codec; | ||
use sp_std::prelude::Vec; | ||
|
||
sp_api::decl_runtime_apis! { |
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 runtime api should be converted to a view function. No need to have this as a runtime api.
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.
I think we still need a runtime api. We could have multiple oracle pallets (e.g. one reading from parachain A, one reading from parachain B, one feed by collator) and we need a runtime API to provide aggregated value at runtime level. view function only works at pallet level.
But will add view function as well.
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.
Most of the logic looks reasonable. I left some comments, after they are solved we can go ahead and merge this.
fn get_all_values() -> Vec<(Key, Option<TimestampedValue>)>; | ||
} | ||
|
||
#[allow(dead_code)] // rust cannot detect usage in macro_rules |
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.
#[allow(dead_code)] // rust cannot detect usage in macro_rules |
Missing documentation and if the function is exported publicly, rust should not complain about dead code.
/// An extended `DataProvider` that provides timestamped data. | ||
pub trait DataProviderExtended<Key, TimestampedValue> { | ||
/// Returns the timestamped value for a given key. | ||
fn get_no_op(key: &Key) -> Option<TimestampedValue>; |
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.
What does no-op stands her for?
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 was some legacy thing. will remove it
.into_iter() | ||
.for_each(|(k, _)| { keys.insert(k); }); | ||
)* | ||
keys.into_iter().map(|k| (k, Self::get_no_op(&k))).collect() |
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 do we need to call here get_no_op
again? Can we not just return the values from the providers?
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.
Ahh because of the call to median, but we can also do this already here.
) -> DispatchResult { | ||
let now = T::Time::now(); | ||
for (key, value) in &values { | ||
let timestamped = TimestampedValue { value: value.clone(), timestamp: now }; | ||
RawValues::<T, I>::insert(&who, key, timestamped); | ||
|
||
// Update `Values` storage if `combined` yielded result. | ||
if let Some(combined) = Self::combined(key) { | ||
<Values<T, I>>::insert(key, combined); | ||
} | ||
|
||
T::OnNewData::on_new_data(&who, key, value); | ||
} | ||
Self::deposit_event(Event::NewFeedData { sender: who, values }); | ||
Ok(()) |
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.
) -> DispatchResult { | |
let now = T::Time::now(); | |
for (key, value) in &values { | |
let timestamped = TimestampedValue { value: value.clone(), timestamp: now }; | |
RawValues::<T, I>::insert(&who, key, timestamped); | |
// Update `Values` storage if `combined` yielded result. | |
if let Some(combined) = Self::combined(key) { | |
<Values<T, I>>::insert(key, combined); | |
} | |
T::OnNewData::on_new_data(&who, key, value); | |
} | |
Self::deposit_event(Event::NewFeedData { sender: who, values }); | |
Ok(()) | |
) { | |
let now = T::Time::now(); | |
for (key, value) in &values { | |
let timestamped = TimestampedValue { value: value.clone(), timestamp: now }; | |
RawValues::<T, I>::insert(&who, key, timestamped); | |
// Update `Values` storage if `combined` yielded result. | |
if let Some(combined) = Self::combined(key) { | |
<Values<T, I>>::insert(key, combined); | |
} | |
T::OnNewData::on_new_data(&who, key, value); | |
} | |
Self::deposit_event(Event::NewFeedData { sender: who, values }); |
This function is not returning any error.
/// | ||
/// The dispatch origin of this call must be a signed account that is either: | ||
/// - A member of the oracle operators set (managed by [`SortedMembers`]) | ||
/// - The root operator account (configured via [`RootOperatorAccountId`]) |
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.
/// - The root operator account (configured via [`RootOperatorAccountId`]) | |
/// - The root origin |
pub fn get_all_values() -> Vec<(T::OracleKey, Option<TimestampedValueOf<T, I>>)> { | ||
<Values<T, I>>::iter().map(|(k, v)| (k, Some(v))).collect() |
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.
pub fn get_all_values() -> Vec<(T::OracleKey, Option<TimestampedValueOf<T, I>>)> { | |
<Values<T, I>>::iter().map(|(k, v)| (k, Some(v))).collect() | |
pub fn get_all_values() -> impl Iterator<Item = (T::OracleKey, TimestampedValueOf<T, I>)> { | |
<Values<T, I>>::iter() |
/// The account ID for the root operator. | ||
/// | ||
/// This account can bypass the oracle membership check and feed values directly, | ||
/// providing a fallback mechanism for critical data feeds when regular oracle | ||
/// operators are unavailable. | ||
#[pallet::constant] | ||
type RootOperatorAccountId: Get<Self::AccountId>; |
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.
The comment is not correct and I also don't see any reason for this. We should introduce a PalletId
and use this to store the values feed by the root
origin.
/// | ||
/// This type provides the current timestamp used to mark when oracle data was submitted. | ||
/// Timestamps are crucial for determining data freshness and preventing stale data usage. | ||
type Time: Time; |
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.
So for a parachain that runs faster than the relay chain, is it safe to use the block number here? Because neither timestamp or the relay chain block number will change for sub relay chain block times.
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.
I don't see any reason to not able to pass a type that uses BlockNumber. It can be either a unix timestamp or block number or any other sequence type.
Timestamp is useful to implement rules like only accept values fed in last 15 minutes. Harder to do that with variable block time.
// Disable the following two lints since they originate from an external macro (namely decl_storage) | ||
#![allow(clippy::string_lit_as_bytes)] | ||
#![allow(clippy::unused_unit)] |
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.
// Disable the following two lints since they originate from an external macro (namely decl_storage) | |
#![allow(clippy::string_lit_as_bytes)] | |
#![allow(clippy::unused_unit)] |
decl_storage
doesn't exist anymore since years :D
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.
just so you know there are few more reference of decl_storage
in other parts of the code
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.
Probably, but we don't need to merge new code that contains these references :)
…removing obsolete lints
…val methods for better clarity and efficiency
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Description
This PR is part of #9765.
This PR introduces
pallet-oracle
, a new FRAME pallet that provides a decentralized and trustworthy way to bring external, off-chain data onto the blockchain. The pallet allows a configurable set of oracle operators to feed data, such as prices, into the system, which can then be consumed by other pallets.Integration
For Runtime Developers
To integrate
pallet-oracle
into your runtime:Add dependency to your runtime's
Cargo.toml
:Implement the
Config
trait in your runtime:Add to
construct_runtime!
:For Pallet Developers
Other pallets can consume oracle data using the
DataProvider
trait:Review Notes
Key Features
SortedMembers
trait to manage oracle operators, allowing integration withpallet-membership
CombineData
implementation with default median-based aggregationDataProvider
andDataProviderExtended
for easy consumption by other palletsImplementation Details
The pallet uses a two-tier storage approach:
RawValues
: Stores individual operator submissions with timestampsValues
: Stores aggregated values after applying theCombineData
logicSecurity Considerations
SortedMembers
)Testing
The pallet includes comprehensive tests covering:
Files Added
substrate/frame/honzon/oracle/
- Complete pallet implementationsubstrate/frame/honzon/oracle/README.md
- Comprehensive documentationBreaking Changes
None - this is a new pallet addition.
Migration Guide
No migration required - this is a new feature.
Checklist
T
required)