-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[pallet-broker] decouple coretime auction logic from the pallet #10916
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| // This file is part of Substrate. | ||
|
|
||
| // Copyright (C) Parity Technologies (UK) Ltd. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| use crate::PotentialRenewalId; | ||
|
|
||
| /// Trait representig generic market logic. | ||
| /// | ||
| /// The assumptions for this generic market are: | ||
| /// - Every order will either create a bid or will be resolved immediately. | ||
| /// - There're two types of orders: bulk coretime purchase and bulk coretime renewal. | ||
| /// - Coretime regions are fungible. | ||
| pub trait Market<Balance, BlockNumber, AccountId> { | ||
| type Error; | ||
| /// Internal market state that must be preserved between the method calls. If the market logic | ||
| /// allows creating bids they should be stored there as well as the bid structure depends on the | ||
| /// market implementation. | ||
| type State; | ||
| /// Unique ID assigned to every bid. | ||
| type BidId; | ||
|
|
||
| /// Place an order for bulk coretime purchase. | ||
| /// | ||
| /// This method may or may not create a bid, according to the market rules. | ||
| /// | ||
| /// - `since_timeslice_start` - amount of blocks passed since the current timeslice start | ||
| /// - `amount` - maximum price which the buyer is willing to pay (or None if it's defined by the | ||
|
Member
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. DQ but how much coretime do they buy for this
Contributor
Author
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. Do you mean to specify that only one region is bought explicitly in the function name or to add a parameter specifying how much regions we want to buy?
Member
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. Just in the doc is enough. |
||
| /// market itself) | ||
| /// - `state` - market state, the caller is responsible for storing it | ||
| fn place_order( | ||
| since_timeslice_start: BlockNumber, | ||
|
Member
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. What block number is this? Relay chain or local coretime chain?
Contributor
Author
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. It's a relay chain block as both old and new implementations depend on it, so I guess if that would need abstraction in the future the appropriate adjustments can be made
Member
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. Then maybe change the typename to |
||
| who: AccountId, | ||
| amount: Option<Balance>, | ||
| state: &mut Self::State, | ||
| ) -> Result<PlaceOrderOutcome<Balance, Self::BidId>, Self::Error>; | ||
|
|
||
| /// Place an order for bulk coretime renewal. | ||
| /// | ||
| /// This method may or may not create a bid, according to the market rules. | ||
| /// | ||
| /// - `since_timeslice_start` - amount of blocks passed since the current timeslice start | ||
| /// - `buying_price` - price which was paid for this region the last time it was sold | ||
| /// - `state` - market state, the caller is responsible for storing it | ||
| fn place_renewal_order( | ||
| since_timeslice_start: BlockNumber, | ||
| who: AccountId, | ||
| renewal: PotentialRenewalId, | ||
| buying_price: Balance, | ||
| state: &mut Self::State, | ||
| ) -> Result<PlaceRenewalOrderOutcome<Balance, Self::BidId>, Self::Error>; | ||
|
|
||
| /// Close the bid given its `BidId`. | ||
| /// | ||
| /// If the market logic allows creating the bids this method allows to close any bids (either | ||
| /// forcefully if `maybe_check_owner` is `None` or checking the bid owner if it's `Some`). | ||
| fn close_bid( | ||
| id: Self::BidId, | ||
| maybe_check_owner: Option<AccountId>, | ||
| state: &mut Self::State, | ||
| ) -> Result<(), Self::Error>; | ||
|
|
||
| /// Logic that gets called in `on_initialize` hook. | ||
|
Member
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. Or the coretime chain? Please describe how often it must be called (otherwise Polkadot will stop working) and how often it should be called (Polkadot keeps working but the market may be a bit slow). This is important to distinguish since
Contributor
Author
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. Actually it can be rephrased to |
||
| fn tick( | ||
| since_timeslice_start: BlockNumber, | ||
| state: &mut Self::State, | ||
| ) -> Result<Vec<TickAction<AccountId, Balance, Self::BidId>>, Self::Error>; | ||
| } | ||
|
|
||
| enum PlaceOrderOutcome<Balance, BidId> { | ||
| BidPlaced { id: BidId, bid_amount: Balance }, | ||
| Sold { price: Balance }, | ||
| } | ||
|
|
||
| enum PlaceRenewalOrderOutcome<Balance, BidId> { | ||
| BidPlaced { id: BidId, bid_amount: Balance }, | ||
| Sold { price: Balance }, | ||
| } | ||
|
|
||
| enum TickAction<AccountId, Balance, BidId> { | ||
| SellRegion { who: AccountId, refund: Balance }, | ||
| RenewRegion { who: AccountId, renewal_id: PotentialRenewalId, refund: Balance }, | ||
| CloseBid { id: BidId, amount: Balance }, | ||
| } | ||
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 would you guess would be in this state? Just a vector or some more complicated struct?
I am asking to figure out the storage layout that you could use. Normally we use the FRAME primitives like
StorageMapandStorageValue, but these are imperative - not functional - like here.So if we pass around a big struct and then write it to a
StorageValueit could be suboptimal since it has to read and write that big struct every time even just for small changes.Alternatively you could use a
Stateinterface with functions likeinsert_orderorget_order, or make the interface imperative withoutStatebeing passed around. But I get that having it functional is cleaner.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.
Discussed in DM, the conclusion is that it can be removed and the
pallet_brokermay implementMarkettrait instead of exposing it in the config. In this setup the market functions can directly read/write the pallet storage