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.

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

Broker: sale price runtime api #3485

Merged
merged 19 commits into from
Apr 5, 2024
Merged

Conversation

Szegoo
Copy link
Contributor

@Szegoo Szegoo commented Feb 26, 2024

Defines a runtime api for pallet-broker for getting the current price of a core if there is an ongoing sale.

Closes: #3413

@Szegoo Szegoo requested a review from a team as a code owner February 26, 2024 15:58
@@ -595,6 +595,12 @@ impl_runtime_apis! {
}
}

impl pallet_broker_runtime_api::BrokerApi<Block, Balance> for Runtime {
fn sale_price() -> Option<Balance> {
Copy link
Member

Choose a reason for hiding this comment

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

And do we maybe want to expose more stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be useful to fetch the unclaimed revenue of a region contributed to the instantaneous pool. I can add that as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be useful to fetch the unclaimed revenue of a region contributed to the instantaneous pool.

After thinking about this a bit more not sure if adding this would make sense. Getting the unclaimed revenue can require quite a lot of reading(reading InstaPoolHistory per each timeslice). So having this exposed could be an easy spam vector.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team February 29, 2024 06:48
substrate/frame/broker/src/dispatchable_impls.rs Outdated Show resolved Hide resolved
@bkchr bkchr requested a review from joepetrowski March 24, 2024 19:05
@bkchr bkchr added the T2-pallets This PR/Issue is related to a particular pallet. label Mar 24, 2024
@bkchr
Copy link
Member

bkchr commented Mar 24, 2024

@Szegoo can you please add a prdoc Runtime Devs and Runtime Users

@@ -455,4 +455,25 @@ impl<T: Config> Pallet<T> {

Ok(())
}

pub(crate) fn ensure_core_availability(
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add some comment? I don't really get from reading the name what this does, because you don't really pass any core. Or maybe call it ensure_cores_for_sale.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think ensure_cores_for_sale is good.

@bkchr
Copy link
Member

bkchr commented Apr 5, 2024

Now if someone of the gentleman @joepetrowski @eskimor or @seadanda could approve this pr, we could merge it.

Copy link
Contributor

@seadanda seadanda left a comment

Choose a reason for hiding this comment

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

Some nits and a regression test would be nice, but already happy to merge

of a core in the ongoing sale.

crates:
- name: pallet-broker
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor bumps? Not up to date on this, @bkchr is minor the default if a change is listed here or should it be explicit?

substrate/frame/broker/src/dispatchable_impls.rs Outdated Show resolved Hide resolved
@bkchr bkchr enabled auto-merge April 5, 2024 23:06
@bkchr bkchr added this pull request to the merge queue Apr 5, 2024
Merged via the queue into paritytech:master with commit 1c85bfe Apr 5, 2024
130 of 134 checks passed
Ank4n pushed a commit that referenced this pull request Apr 9, 2024
Defines a runtime api for `pallet-broker` for getting the current price
of a core if there is an ongoing sale.

Closes: #3413

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Apr 9, 2024
Defines a runtime api for `pallet-broker` for getting the current price
of a core if there is an ongoing sale.

Closes: paritytech#3413

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broker pallet should expose sale_price through runtime api
5 participants