Skip to content

Commit

Permalink
Revert "Backport broker new price adapter (#4521) without extra chang…
Browse files Browse the repository at this point in the history 
…es (#4656)"

This reverts commit 3bbbffa.
  • Loading branch information
@seadanda
seadanda authored Jun 3, 2024
1 parent 3bbbffa commit 40142d3
Show file tree
Hide file tree
Showing 13 changed files with 297 additions and 545 deletions.
3 changes: 1 addition & 2 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cumulus/parachains/runtimes/coretime/coretime-rococo/src/coretime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,5 +233,5 @@ impl pallet_broker::Config for Runtime {
type WeightInfo = weights::pallet_broker::WeightInfo<Runtime>;
type PalletId = BrokerPalletId;
type AdminOrigin = EnsureRoot<AccountId>;
type PriceAdapter = pallet_broker::CenterTargetPrice<Balance>;
type PriceAdapter = pallet_broker::Linear;
}
2 changes: 1 addition & 1 deletion substrate/bin/node/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2060,7 +2060,7 @@ impl pallet_broker::Config for Runtime {
type WeightInfo = ();
type PalletId = BrokerPalletId;
type AdminOrigin = EnsureRoot<AccountId>;
type PriceAdapter = pallet_broker::CenterTargetPrice<Balance>;
type PriceAdapter = pallet_broker::Linear;
}

parameter_types! {
Expand Down
4 changes: 1 addition & 3 deletions substrate/frame/broker/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pallet-broker"
version = "0.7.2"
version = "0.6.0"
description = "Brokerage tool for managing Polkadot Core scheduling"
authors.workspace = true
homepage = "https://substrate.io"
Expand All @@ -15,7 +15,6 @@ workspace = true
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
log = { version = "0.4.20", default-features = false }
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
scale-info = { version = "2.10.0", default-features = false, features = ["derive"] }
bitvec = { version = "1.0.0", default-features = false }
Expand All @@ -37,7 +36,6 @@ default = ["std"]
std = [
"bitvec/std",
"codec/std",
"log/std",
"frame-benchmarking?/std",
"frame-support/std",
"frame-system/std",
Expand Down
225 changes: 33 additions & 192 deletions substrate/frame/broker/src/adapt_price.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,121 +17,47 @@

#![deny(missing_docs)]

use crate::{CoreIndex, SaleInfoRecord};
use crate::CoreIndex;
use sp_arithmetic::{traits::One, FixedU64};
use sp_runtime::{FixedPointNumber, FixedPointOperand, Saturating};

/// Performance of a past sale.
#[derive(Copy, Clone)]
pub struct SalePerformance<Balance> {
/// The price at which the last core was sold.
///
/// Will be `None` if no cores have been offered.
pub sellout_price: Option<Balance>,

/// The minimum price that was achieved in this sale.
pub end_price: Balance,

/// The number of cores we want to sell, ideally.
pub ideal_cores_sold: CoreIndex,

/// Number of cores which are/have been offered for sale.
pub cores_offered: CoreIndex,

/// Number of cores which have been sold; never more than cores_offered.
pub cores_sold: CoreIndex,
}

/// Result of `AdaptPrice::adapt_price`.
#[derive(Copy, Clone)]
pub struct AdaptedPrices<Balance> {
/// New minimum price to use.
pub end_price: Balance,

/// Price the controller is optimizing for.
///
/// This is the price "expected" by the controller based on the previous sale. We assume that
/// sales in this period will be around this price, assuming stable market conditions.
///
/// Think of it as the expected market price. This can be used for determining what to charge
/// for renewals, that don't yet have any price information for example. E.g. for expired
/// legacy leases.
pub target_price: Balance,
}

impl<Balance: Copy> SalePerformance<Balance> {
/// Construct performance via data from a `SaleInfoRecord`.
pub fn from_sale<BlockNumber>(record: &SaleInfoRecord<Balance, BlockNumber>) -> Self {
Self {
sellout_price: record.sellout_price,
end_price: record.price,
ideal_cores_sold: record.ideal_cores_sold,
cores_offered: record.cores_offered,
cores_sold: record.cores_sold,
}
}

#[cfg(test)]
fn new(sellout_price: Option<Balance>, end_price: Balance) -> Self {
Self { sellout_price, end_price, ideal_cores_sold: 0, cores_offered: 0, cores_sold: 0 }
}
}

/// Type for determining how to set price.
pub trait AdaptPrice<Balance> {
pub trait AdaptPrice {
/// Return the factor by which the regular price must be multiplied during the leadin period.
///
/// - `when`: The amount through the leadin period; between zero and one.
fn leadin_factor_at(when: FixedU64) -> FixedU64;

/// Return adapted prices for next sale.
/// Return the correction factor by which the regular price must be multiplied based on market
/// performance.
///
/// Based on the previous sale's performance.
fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance>;
/// - `sold`: The number of cores sold.
/// - `target`: The target number of cores to be sold (must be larger than zero).
/// - `limit`: The maximum number of cores to be sold.
fn adapt_price(sold: CoreIndex, target: CoreIndex, limit: CoreIndex) -> FixedU64;
}

impl<Balance: Copy> AdaptPrice<Balance> for () {
impl AdaptPrice for () {
fn leadin_factor_at(_: FixedU64) -> FixedU64 {
FixedU64::one()
}
fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance> {
let price = performance.sellout_price.unwrap_or(performance.end_price);
AdaptedPrices { end_price: price, target_price: price }
fn adapt_price(_: CoreIndex, _: CoreIndex, _: CoreIndex) -> FixedU64 {
FixedU64::one()
}
}

/// Simple implementation of `AdaptPrice` with two linear phases.
///
/// One steep one downwards to the target price, which is 1/10 of the maximum price and a more flat
/// one down to the minimum price, which is 1/100 of the maximum price.
pub struct CenterTargetPrice<Balance>(core::marker::PhantomData<Balance>);

impl<Balance: FixedPointOperand> AdaptPrice<Balance> for CenterTargetPrice<Balance> {
/// Simple implementation of `AdaptPrice` giving a monotonic leadin and a linear price change based
/// on cores sold.
pub struct Linear;
impl AdaptPrice for Linear {
fn leadin_factor_at(when: FixedU64) -> FixedU64 {
if when <= FixedU64::from_rational(1, 2) {
FixedU64::from(100).saturating_sub(when.saturating_mul(180.into()))
} else {
FixedU64::from(19).saturating_sub(when.saturating_mul(18.into()))
}
FixedU64::from(2) - when
}

fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance> {
let Some(sellout_price) = performance.sellout_price else {
return AdaptedPrices {
end_price: performance.end_price,
target_price: FixedU64::from(10).saturating_mul_int(performance.end_price),
}
};

let price = FixedU64::from_rational(1, 10).saturating_mul_int(sellout_price);
let price = if price == Balance::zero() {
// We could not recover from a price equal 0 ever.
sellout_price
fn adapt_price(sold: CoreIndex, target: CoreIndex, limit: CoreIndex) -> FixedU64 {
if sold <= target {
FixedU64::from_rational(sold.into(), target.into())
} else {
price
};

AdaptedPrices { end_price: price, target_price: sellout_price }
FixedU64::one() +
FixedU64::from_rational((sold - target).into(), (limit - target).into())
}
}
}

Expand All @@ -141,103 +67,18 @@ mod tests {

#[test]
fn linear_no_panic() {
for sellout in 0..11 {
for price in 0..10 {
let sellout_price = if sellout == 11 { None } else { Some(sellout) };
CenterTargetPrice::adapt_price(SalePerformance::new(sellout_price, price));
for limit in 0..10 {
for target in 1..10 {
for sold in 0..=limit {
let price = Linear::adapt_price(sold, target, limit);

if sold > target {
assert!(price > FixedU64::one());
} else {
assert!(price <= FixedU64::one());
}
}
}
}
}

#[test]
fn leadin_price_bound_check() {
assert_eq!(
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from(0)),
FixedU64::from(100)
);
assert_eq!(
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from_rational(1, 4)),
FixedU64::from(55)
);

assert_eq!(
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from_float(0.5)),
FixedU64::from(10)
);

assert_eq!(
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from_rational(3, 4)),
FixedU64::from_float(5.5)
);
assert_eq!(CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::one()), FixedU64::one());
}

#[test]
fn no_op_sale_is_good() {
let prices = CenterTargetPrice::adapt_price(SalePerformance::new(None, 1));
assert_eq!(prices.target_price, 10);
assert_eq!(prices.end_price, 1);
}

#[test]
fn price_stays_stable_on_optimal_sale() {
// Check price stays stable if sold at the optimal price:
let mut performance = SalePerformance::new(Some(1000), 100);
for _ in 0..10 {
let prices = CenterTargetPrice::adapt_price(performance);
performance.sellout_price = Some(1000);
performance.end_price = prices.end_price;

assert!(prices.end_price <= 101);
assert!(prices.end_price >= 99);
assert!(prices.target_price <= 1001);
assert!(prices.target_price >= 999);
}
}

#[test]
fn price_adjusts_correctly_upwards() {
let performance = SalePerformance::new(Some(10_000), 100);
let prices = CenterTargetPrice::adapt_price(performance);
assert_eq!(prices.target_price, 10_000);
assert_eq!(prices.end_price, 1000);
}

#[test]
fn price_adjusts_correctly_downwards() {
let performance = SalePerformance::new(Some(100), 100);
let prices = CenterTargetPrice::adapt_price(performance);
assert_eq!(prices.target_price, 100);
assert_eq!(prices.end_price, 10);
}

#[test]
fn price_never_goes_to_zero_and_recovers() {
// Check price stays stable if sold at the optimal price:
let sellout_price = 1;
let mut performance = SalePerformance::new(Some(sellout_price), 1);
for _ in 0..11 {
let prices = CenterTargetPrice::adapt_price(performance);
performance.sellout_price = Some(sellout_price);
performance.end_price = prices.end_price;

assert!(prices.end_price <= sellout_price);
assert!(prices.end_price > 0);
}
}

#[test]
fn renewal_price_is_correct_on_no_sale() {
let performance = SalePerformance::new(None, 100);
let prices = CenterTargetPrice::adapt_price(performance);
assert_eq!(prices.target_price, 1000);
assert_eq!(prices.end_price, 100);
}

#[test]
fn renewal_price_is_sell_out() {
let performance = SalePerformance::new(Some(1000), 100);
let prices = CenterTargetPrice::adapt_price(performance);
assert_eq!(prices.target_price, 1000);
}
}
4 changes: 2 additions & 2 deletions substrate/frame/broker/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ mod benches {
Event::SaleInitialized {
sale_start: 2u32.into(),
leadin_length: 1u32.into(),
start_price: 1000u32.into(),
start_price: 20u32.into(),
regular_price: 10u32.into(),
region_begin: latest_region_begin + config.region_length,
region_end: latest_region_begin + config.region_length * 2,
Expand Down Expand Up @@ -811,7 +811,7 @@ mod benches {
Event::SaleInitialized {
sale_start: 2u32.into(),
leadin_length: 1u32.into(),
start_price: 1000u32.into(),
start_price: 20u32.into(),
regular_price: 10u32.into(),
region_begin: sale.region_begin + config.region_length,
region_end: sale.region_end + config.region_length,
Expand Down
24 changes: 10 additions & 14 deletions substrate/frame/broker/src/dispatchable_impls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,12 @@ impl<T: Config> Pallet<T> {
let price = Self::sale_price(&sale, now);
ensure!(price_limit >= price, Error::<T>::Overpriced);

let core = Self::purchase_core(&who, price, &mut sale)?;

Self::charge(&who, price)?;
let core = sale.first_core.saturating_add(sale.cores_sold);
sale.cores_sold.saturating_inc();
if sale.cores_sold <= sale.ideal_cores_sold || sale.sellout_price.is_none() {
sale.sellout_price = Some(price);
}
SaleInfo::<T>::put(&sale);
let id = Self::issue(core, sale.region_begin, sale.region_end, who.clone(), Some(price));
let duration = sale.region_end.saturating_sub(sale.region_begin);
Expand All @@ -136,9 +140,8 @@ impl<T: Config> Pallet<T> {
record.completion.drain_complete().ok_or(Error::<T>::IncompleteAssignment)?;

let old_core = core;

let core = Self::purchase_core(&who, record.price, &mut sale)?;

let core = sale.first_core.saturating_add(sale.cores_sold);
Self::charge(&who, record.price)?;
Self::deposit_event(Event::Renewed {
who,
old_core,
Expand All @@ -149,24 +152,19 @@ impl<T: Config> Pallet<T> {
workload: workload.clone(),
});

sale.cores_sold.saturating_inc();

Workplan::<T>::insert((sale.region_begin, core), &workload);

let begin = sale.region_end;
let price_cap = record.price + config.renewal_bump * record.price;
let now = frame_system::Pallet::<T>::block_number();
let price = Self::sale_price(&sale, now).min(price_cap);
log::debug!(
"Renew with: sale price: {:?}, price cap: {:?}, old price: {:?}",
price,
price_cap,
record.price
);
let new_record = AllowedRenewalRecord { price, completion: Complete(workload) };
AllowedRenewals::<T>::remove(renewal_id);
AllowedRenewals::<T>::insert(AllowedRenewalId { core, when: begin }, &new_record);
SaleInfo::<T>::put(&sale);
if let Some(workload) = new_record.completion.drain_complete() {
log::debug!("Recording renewable price for next run: {:?}", price);
Self::deposit_event(Event::Renewable { core, price, begin, workload });
}
Ok(core)
Expand Down Expand Up @@ -284,8 +282,6 @@ impl<T: Config> Pallet<T> {
let workload =
if assigned.is_complete() { Complete(workplan) } else { Partial(assigned) };
let record = AllowedRenewalRecord { price, completion: workload };
// Note: This entry alone does not yet actually allow renewals (the completion
// status has to be complete for `do_renew` to accept it).
AllowedRenewals::<T>::insert(&renewal_id, &record);
if let Some(workload) = record.completion.drain_complete() {
Self::deposit_event(Event::Renewable {
Expand Down
Loading

0 comments on commit 40142d3

Please sign in to comment.