Skip to content

Commit b808b84

Browse files
committed
Backport broker new price adapter (#4521) without extra changes (#4656)
Only the price adapter changes are backported to avoid out-of-order migrations
1 parent 1f3477a commit b808b84

File tree

13 files changed

+546
-302
lines changed

13 files changed

+546
-302
lines changed

Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cumulus/parachains/runtimes/coretime/coretime-rococo/src/coretime.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,5 +233,5 @@ impl pallet_broker::Config for Runtime {
233233
type WeightInfo = weights::pallet_broker::WeightInfo<Runtime>;
234234
type PalletId = BrokerPalletId;
235235
type AdminOrigin = EnsureRoot<AccountId>;
236-
type PriceAdapter = pallet_broker::Linear;
236+
type PriceAdapter = pallet_broker::CenterTargetPrice<Balance>;
237237
}

substrate/bin/node/runtime/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2067,7 +2067,7 @@ impl pallet_broker::Config for Runtime {
20672067
type WeightInfo = ();
20682068
type PalletId = BrokerPalletId;
20692069
type AdminOrigin = EnsureRoot<AccountId>;
2070-
type PriceAdapter = pallet_broker::Linear;
2070+
type PriceAdapter = pallet_broker::CenterTargetPrice<Balance>;
20712071
}
20722072

20732073
parameter_types! {

substrate/frame/broker/Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "pallet-broker"
3-
version = "0.7.1"
3+
version = "0.7.2"
44
description = "Brokerage tool for managing Polkadot Core scheduling"
55
authors.workspace = true
66
homepage = "https://substrate.io"
@@ -15,6 +15,7 @@ workspace = true
1515
targets = ["x86_64-unknown-linux-gnu"]
1616

1717
[dependencies]
18+
log = { version = "0.4.20", default-features = false }
1819
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
1920
scale-info = { version = "2.10.0", default-features = false, features = ["derive"] }
2021
bitvec = { version = "1.0.0", default-features = false }
@@ -36,6 +37,7 @@ default = ["std"]
3637
std = [
3738
"bitvec/std",
3839
"codec/std",
40+
"log/std",
3941
"frame-benchmarking?/std",
4042
"frame-support/std",
4143
"frame-system/std",

substrate/frame/broker/src/adapt_price.rs

Lines changed: 192 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,48 +17,122 @@
1717

1818
#![deny(missing_docs)]
1919

20-
use crate::CoreIndex;
20+
use crate::{CoreIndex, SaleInfoRecord};
2121
use sp_arithmetic::{traits::One, FixedU64};
22+
use sp_runtime::{FixedPointNumber, FixedPointOperand, Saturating};
23+
24+
/// Performance of a past sale.
25+
#[derive(Copy, Clone)]
26+
pub struct SalePerformance<Balance> {
27+
/// The price at which the last core was sold.
28+
///
29+
/// Will be `None` if no cores have been offered.
30+
pub sellout_price: Option<Balance>,
31+
32+
/// The minimum price that was achieved in this sale.
33+
pub end_price: Balance,
34+
35+
/// The number of cores we want to sell, ideally.
36+
pub ideal_cores_sold: CoreIndex,
37+
38+
/// Number of cores which are/have been offered for sale.
39+
pub cores_offered: CoreIndex,
40+
41+
/// Number of cores which have been sold; never more than cores_offered.
42+
pub cores_sold: CoreIndex,
43+
}
44+
45+
/// Result of `AdaptPrice::adapt_price`.
46+
#[derive(Copy, Clone)]
47+
pub struct AdaptedPrices<Balance> {
48+
/// New minimum price to use.
49+
pub end_price: Balance,
50+
51+
/// Price the controller is optimizing for.
52+
///
53+
/// This is the price "expected" by the controller based on the previous sale. We assume that
54+
/// sales in this period will be around this price, assuming stable market conditions.
55+
///
56+
/// Think of it as the expected market price. This can be used for determining what to charge
57+
/// for renewals, that don't yet have any price information for example. E.g. for expired
58+
/// legacy leases.
59+
pub target_price: Balance,
60+
}
61+
62+
impl<Balance: Copy> SalePerformance<Balance> {
63+
/// Construct performance via data from a `SaleInfoRecord`.
64+
pub fn from_sale<BlockNumber>(record: &SaleInfoRecord<Balance, BlockNumber>) -> Self {
65+
Self {
66+
sellout_price: record.sellout_price,
67+
end_price: record.price,
68+
ideal_cores_sold: record.ideal_cores_sold,
69+
cores_offered: record.cores_offered,
70+
cores_sold: record.cores_sold,
71+
}
72+
}
73+
74+
#[cfg(test)]
75+
fn new(sellout_price: Option<Balance>, end_price: Balance) -> Self {
76+
Self { sellout_price, end_price, ideal_cores_sold: 0, cores_offered: 0, cores_sold: 0 }
77+
}
78+
}
2279

2380
/// Type for determining how to set price.
24-
pub trait AdaptPrice {
81+
pub trait AdaptPrice<Balance> {
2582
/// Return the factor by which the regular price must be multiplied during the leadin period.
2683
///
2784
/// - `when`: The amount through the leadin period; between zero and one.
2885
fn leadin_factor_at(when: FixedU64) -> FixedU64;
29-
/// Return the correction factor by which the regular price must be multiplied based on market
30-
/// performance.
86+
87+
/// Return adapted prices for next sale.
3188
///
32-
/// - `sold`: The number of cores sold.
33-
/// - `target`: The target number of cores to be sold (must be larger than zero).
34-
/// - `limit`: The maximum number of cores to be sold.
35-
fn adapt_price(sold: CoreIndex, target: CoreIndex, limit: CoreIndex) -> FixedU64;
89+
/// Based on the previous sale's performance.
90+
fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance>;
3691
}
3792

38-
impl AdaptPrice for () {
93+
impl<Balance: Copy> AdaptPrice<Balance> for () {
3994
fn leadin_factor_at(_: FixedU64) -> FixedU64 {
4095
FixedU64::one()
4196
}
42-
fn adapt_price(_: CoreIndex, _: CoreIndex, _: CoreIndex) -> FixedU64 {
43-
FixedU64::one()
97+
fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance> {
98+
let price = performance.sellout_price.unwrap_or(performance.end_price);
99+
AdaptedPrices { end_price: price, target_price: price }
44100
}
45101
}
46102

47-
/// Simple implementation of `AdaptPrice` giving a monotonic leadin and a linear price change based
48-
/// on cores sold.
49-
pub struct Linear;
50-
impl AdaptPrice for Linear {
103+
/// Simple implementation of `AdaptPrice` with two linear phases.
104+
///
105+
/// One steep one downwards to the target price, which is 1/10 of the maximum price and a more flat
106+
/// one down to the minimum price, which is 1/100 of the maximum price.
107+
pub struct CenterTargetPrice<Balance>(core::marker::PhantomData<Balance>);
108+
109+
impl<Balance: FixedPointOperand> AdaptPrice<Balance> for CenterTargetPrice<Balance> {
51110
fn leadin_factor_at(when: FixedU64) -> FixedU64 {
52-
FixedU64::from(2) - when
53-
}
54-
fn adapt_price(sold: CoreIndex, target: CoreIndex, limit: CoreIndex) -> FixedU64 {
55-
if sold <= target {
56-
FixedU64::from_rational(sold.into(), target.into())
111+
if when <= FixedU64::from_rational(1, 2) {
112+
FixedU64::from(100).saturating_sub(when.saturating_mul(180.into()))
57113
} else {
58-
FixedU64::one() +
59-
FixedU64::from_rational((sold - target).into(), (limit - target).into())
114+
FixedU64::from(19).saturating_sub(when.saturating_mul(18.into()))
60115
}
61116
}
117+
118+
fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance> {
119+
let Some(sellout_price) = performance.sellout_price else {
120+
return AdaptedPrices {
121+
end_price: performance.end_price,
122+
target_price: FixedU64::from(10).saturating_mul_int(performance.end_price),
123+
}
124+
};
125+
126+
let price = FixedU64::from_rational(1, 10).saturating_mul_int(sellout_price);
127+
let price = if price == Balance::zero() {
128+
// We could not recover from a price equal 0 ever.
129+
sellout_price
130+
} else {
131+
price
132+
};
133+
134+
AdaptedPrices { end_price: price, target_price: sellout_price }
135+
}
62136
}
63137

64138
#[cfg(test)]
@@ -67,18 +141,103 @@ mod tests {
67141

68142
#[test]
69143
fn linear_no_panic() {
70-
for limit in 0..10 {
71-
for target in 1..10 {
72-
for sold in 0..=limit {
73-
let price = Linear::adapt_price(sold, target, limit);
74-
75-
if sold > target {
76-
assert!(price > FixedU64::one());
77-
} else {
78-
assert!(price <= FixedU64::one());
79-
}
80-
}
144+
for sellout in 0..11 {
145+
for price in 0..10 {
146+
let sellout_price = if sellout == 11 { None } else { Some(sellout) };
147+
CenterTargetPrice::adapt_price(SalePerformance::new(sellout_price, price));
81148
}
82149
}
83150
}
151+
152+
#[test]
153+
fn leadin_price_bound_check() {
154+
assert_eq!(
155+
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from(0)),
156+
FixedU64::from(100)
157+
);
158+
assert_eq!(
159+
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from_rational(1, 4)),
160+
FixedU64::from(55)
161+
);
162+
163+
assert_eq!(
164+
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from_float(0.5)),
165+
FixedU64::from(10)
166+
);
167+
168+
assert_eq!(
169+
CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::from_rational(3, 4)),
170+
FixedU64::from_float(5.5)
171+
);
172+
assert_eq!(CenterTargetPrice::<u64>::leadin_factor_at(FixedU64::one()), FixedU64::one());
173+
}
174+
175+
#[test]
176+
fn no_op_sale_is_good() {
177+
let prices = CenterTargetPrice::adapt_price(SalePerformance::new(None, 1));
178+
assert_eq!(prices.target_price, 10);
179+
assert_eq!(prices.end_price, 1);
180+
}
181+
182+
#[test]
183+
fn price_stays_stable_on_optimal_sale() {
184+
// Check price stays stable if sold at the optimal price:
185+
let mut performance = SalePerformance::new(Some(1000), 100);
186+
for _ in 0..10 {
187+
let prices = CenterTargetPrice::adapt_price(performance);
188+
performance.sellout_price = Some(1000);
189+
performance.end_price = prices.end_price;
190+
191+
assert!(prices.end_price <= 101);
192+
assert!(prices.end_price >= 99);
193+
assert!(prices.target_price <= 1001);
194+
assert!(prices.target_price >= 999);
195+
}
196+
}
197+
198+
#[test]
199+
fn price_adjusts_correctly_upwards() {
200+
let performance = SalePerformance::new(Some(10_000), 100);
201+
let prices = CenterTargetPrice::adapt_price(performance);
202+
assert_eq!(prices.target_price, 10_000);
203+
assert_eq!(prices.end_price, 1000);
204+
}
205+
206+
#[test]
207+
fn price_adjusts_correctly_downwards() {
208+
let performance = SalePerformance::new(Some(100), 100);
209+
let prices = CenterTargetPrice::adapt_price(performance);
210+
assert_eq!(prices.target_price, 100);
211+
assert_eq!(prices.end_price, 10);
212+
}
213+
214+
#[test]
215+
fn price_never_goes_to_zero_and_recovers() {
216+
// Check price stays stable if sold at the optimal price:
217+
let sellout_price = 1;
218+
let mut performance = SalePerformance::new(Some(sellout_price), 1);
219+
for _ in 0..11 {
220+
let prices = CenterTargetPrice::adapt_price(performance);
221+
performance.sellout_price = Some(sellout_price);
222+
performance.end_price = prices.end_price;
223+
224+
assert!(prices.end_price <= sellout_price);
225+
assert!(prices.end_price > 0);
226+
}
227+
}
228+
229+
#[test]
230+
fn renewal_price_is_correct_on_no_sale() {
231+
let performance = SalePerformance::new(None, 100);
232+
let prices = CenterTargetPrice::adapt_price(performance);
233+
assert_eq!(prices.target_price, 1000);
234+
assert_eq!(prices.end_price, 100);
235+
}
236+
237+
#[test]
238+
fn renewal_price_is_sell_out() {
239+
let performance = SalePerformance::new(Some(1000), 100);
240+
let prices = CenterTargetPrice::adapt_price(performance);
241+
assert_eq!(prices.target_price, 1000);
242+
}
84243
}

substrate/frame/broker/src/benchmarking.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ mod benches {
210210
Event::SaleInitialized {
211211
sale_start: 2u32.into(),
212212
leadin_length: 1u32.into(),
213-
start_price: 20u32.into(),
213+
start_price: 1000u32.into(),
214214
regular_price: 10u32.into(),
215215
region_begin: latest_region_begin + config.region_length,
216216
region_end: latest_region_begin + config.region_length * 2,
@@ -811,7 +811,7 @@ mod benches {
811811
Event::SaleInitialized {
812812
sale_start: 2u32.into(),
813813
leadin_length: 1u32.into(),
814-
start_price: 20u32.into(),
814+
start_price: 1000u32.into(),
815815
regular_price: 10u32.into(),
816816
region_begin: sale.region_begin + config.region_length,
817817
region_end: sale.region_end + config.region_length,

substrate/frame/broker/src/dispatchable_impls.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,8 @@ impl<T: Config> Pallet<T> {
112112
let price = Self::sale_price(&sale, now);
113113
ensure!(price_limit >= price, Error::<T>::Overpriced);
114114

115-
Self::charge(&who, price)?;
116-
let core = sale.first_core.saturating_add(sale.cores_sold);
117-
sale.cores_sold.saturating_inc();
118-
if sale.cores_sold <= sale.ideal_cores_sold || sale.sellout_price.is_none() {
119-
sale.sellout_price = Some(price);
120-
}
115+
let core = Self::purchase_core(&who, price, &mut sale)?;
116+
121117
SaleInfo::<T>::put(&sale);
122118
let id = Self::issue(core, sale.region_begin, sale.region_end, who.clone(), Some(price));
123119
let duration = sale.region_end.saturating_sub(sale.region_begin);
@@ -140,8 +136,9 @@ impl<T: Config> Pallet<T> {
140136
record.completion.drain_complete().ok_or(Error::<T>::IncompleteAssignment)?;
141137

142138
let old_core = core;
143-
let core = sale.first_core.saturating_add(sale.cores_sold);
144-
Self::charge(&who, record.price)?;
139+
140+
let core = Self::purchase_core(&who, record.price, &mut sale)?;
141+
145142
Self::deposit_event(Event::Renewed {
146143
who,
147144
old_core,
@@ -152,19 +149,24 @@ impl<T: Config> Pallet<T> {
152149
workload: workload.clone(),
153150
});
154151

155-
sale.cores_sold.saturating_inc();
156-
157152
Workplan::<T>::insert((sale.region_begin, core), &workload);
158153

159154
let begin = sale.region_end;
160155
let price_cap = record.price + config.renewal_bump * record.price;
161156
let now = frame_system::Pallet::<T>::block_number();
162157
let price = Self::sale_price(&sale, now).min(price_cap);
158+
log::debug!(
159+
"Renew with: sale price: {:?}, price cap: {:?}, old price: {:?}",
160+
price,
161+
price_cap,
162+
record.price
163+
);
163164
let new_record = AllowedRenewalRecord { price, completion: Complete(workload) };
164165
AllowedRenewals::<T>::remove(renewal_id);
165166
AllowedRenewals::<T>::insert(AllowedRenewalId { core, when: begin }, &new_record);
166167
SaleInfo::<T>::put(&sale);
167168
if let Some(workload) = new_record.completion.drain_complete() {
169+
log::debug!("Recording renewable price for next run: {:?}", price);
168170
Self::deposit_event(Event::Renewable { core, price, begin, workload });
169171
}
170172
Ok(core)
@@ -282,6 +284,8 @@ impl<T: Config> Pallet<T> {
282284
let workload =
283285
if assigned.is_complete() { Complete(workplan) } else { Partial(assigned) };
284286
let record = AllowedRenewalRecord { price, completion: workload };
287+
// Note: This entry alone does not yet actually allow renewals (the completion
288+
// status has to be complete for `do_renew` to accept it).
285289
AllowedRenewals::<T>::insert(&renewal_id, &record);
286290
if let Some(workload) = record.completion.drain_complete() {
287291
Self::deposit_event(Event::Renewable {

0 commit comments

Comments
 (0)