-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Improve On_demand_assigner events #4339
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
Changes from all commits
0f5192b
30475b3
33b26b0
0b9be3d
25e5a99
bf8fc83
4b889e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Overkillus marked this conversation as resolved.
Show resolved
Hide resolved
bolajahmad marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,7 +173,7 @@ impl QueueStatusType { | |
| fn consume_index(&mut self, removed_index: QueueIndex) { | ||
| if removed_index != self.smallest_index { | ||
| self.freed_indices.push(removed_index.reverse()); | ||
| return | ||
| return; | ||
| } | ||
| let mut index = self.smallest_index.0.overflowing_add(1).0; | ||
| // Even more to advance? | ||
|
|
@@ -368,10 +368,10 @@ pub mod pallet { | |
| #[pallet::event] | ||
| #[pallet::generate_deposit(pub(super) fn deposit_event)] | ||
| pub enum Event<T: Config> { | ||
| /// An order was placed at some spot price amount. | ||
| OnDemandOrderPlaced { para_id: ParaId, spot_price: BalanceOf<T> }, | ||
| /// The value of the spot traffic multiplier changed. | ||
| SpotTrafficSet { traffic: FixedU128 }, | ||
| /// An order was placed at some spot price amount by orderer ordered_by | ||
| OnDemandOrderPlaced { para_id: ParaId, spot_price: BalanceOf<T>, ordered_by: T::AccountId }, | ||
| /// The value of the spot price has likely changed | ||
| SpotPriceSet { spot_price: BalanceOf<T> }, | ||
|
Comment on lines
-371
to
+374
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may be problematic as you're changing the formats of the existing events here. As events from the previous block are reset after the runtime upgrade but before running any hooks, event storage must always be compatible. So, technically, this part needs more care. Buuuut, given that, IIUC,
it may be safe to leave it as is. That said, some more research is needed on how safe it is. |
||
| } | ||
|
|
||
| #[pallet::error] | ||
|
|
@@ -410,12 +410,11 @@ pub mod pallet { | |
| /// | ||
| /// Errors: | ||
| /// - `InsufficientBalance`: from the Currency implementation | ||
| /// - `InvalidParaId` | ||
| /// - `QueueFull` | ||
| /// - `SpotPriceHigherThanMaxAmount` | ||
| /// | ||
| /// Events: | ||
| /// - `SpotOrderPlaced` | ||
| /// - `OnDemandOrderPlaced` | ||
| #[pallet::call_index(0)] | ||
| #[pallet::weight(<T as Config>::WeightInfo::place_order_allow_death(QueueStatus::<T>::get().size()))] | ||
| pub fn place_order_allow_death( | ||
|
|
@@ -437,12 +436,11 @@ pub mod pallet { | |
| /// | ||
| /// Errors: | ||
| /// - `InsufficientBalance`: from the Currency implementation | ||
| /// - `InvalidParaId` | ||
| /// - `QueueFull` | ||
| /// - `SpotPriceHigherThanMaxAmount` | ||
| /// | ||
| /// Events: | ||
| /// - `SpotOrderPlaced` | ||
| /// - `OnDemandOrderPlaced` | ||
| #[pallet::call_index(1)] | ||
| #[pallet::weight(<T as Config>::WeightInfo::place_order_keep_alive(QueueStatus::<T>::get().size()))] | ||
| pub fn place_order_keep_alive( | ||
|
|
@@ -539,12 +537,11 @@ where | |
| /// | ||
| /// Errors: | ||
| /// - `InsufficientBalance`: from the Currency implementation | ||
| /// - `InvalidParaId` | ||
| /// - `QueueFull` | ||
| /// - `SpotPriceHigherThanMaxAmount` | ||
| /// | ||
| /// Events: | ||
| /// - `SpotOrderPlaced` | ||
| /// - `OnDemandOrderPlaced` | ||
| fn do_place_order( | ||
| sender: <T as frame_system::Config>::AccountId, | ||
| max_amount: BalanceOf<T>, | ||
|
|
@@ -578,6 +575,12 @@ where | |
| Error::<T>::QueueFull | ||
| ); | ||
| Pallet::<T>::add_on_demand_order(queue_status, para_id, QueuePushDirection::Back); | ||
| Pallet::<T>::deposit_event(Event::<T>::OnDemandOrderPlaced { | ||
| para_id, | ||
| spot_price, | ||
| ordered_by: sender, | ||
| }); | ||
|
|
||
| Ok(()) | ||
| }) | ||
| } | ||
|
|
@@ -599,7 +602,14 @@ where | |
| // Only update storage on change | ||
| if new_traffic != old_traffic { | ||
| queue_status.traffic = new_traffic; | ||
| Pallet::<T>::deposit_event(Event::<T>::SpotTrafficSet { traffic: new_traffic }); | ||
|
|
||
| // calculate the new spot price | ||
| let spot_price: BalanceOf<T> = new_traffic.saturating_mul_int( | ||
| config.scheduler_params.on_demand_base_fee.saturated_into::<BalanceOf<T>>(), | ||
| ); | ||
|
|
||
| // emit the event for updated new price | ||
| Pallet::<T>::deposit_event(Event::<T>::SpotPriceSet { spot_price }); | ||
| } | ||
| }, | ||
| Err(err) => { | ||
|
|
@@ -721,7 +731,7 @@ where | |
| "Decreased affinity for a para that has not been served on a core?" | ||
| ); | ||
| if affinity != Some(0) { | ||
| return | ||
| return; | ||
| } | ||
| // No affinity more for entries on this core, free any entries: | ||
| // | ||
|
|
@@ -754,7 +764,7 @@ where | |
| } else { | ||
| *maybe_affinity = None; | ||
| } | ||
| return Some(new_count) | ||
| return Some(new_count); | ||
| } else { | ||
| None | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 | ||
| # See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json | ||
|
|
||
| title: Improving on_demand_assigner emitted events | ||
|
|
||
| doc: | ||
| - audience: Runtime User | ||
| description: | | ||
| Registering OnDemandOrderPlaced event that is useful for indexers to save data related to on demand orders. Adds SpotPriceSet as a new event to monitor on-demand spot prices. It updates whenever the price changes due to traffic. | ||
|
|
||
| crates: | ||
| - name: polkadot-runtime-parachains | ||
| bump: minor |
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.
A lot of formatting nits. We use nightly fmt
cargo +nightly fmt --all. Feel free to run locally and see the changes yourself.For more details feel free to read this: https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/STYLE_GUIDE.md
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.
Running the
cargo +nightly fmt --allworked but some of the formatting remained inconsistent with the styleguide example.I'll try to run it while paying more attention if there's an issue