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 new price adapter #4521

Merged
merged 39 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
d598a1e
Price adjustements should be based on price
May 10, 2024
ae90650
AllowedRenewal -> PotentialRenewal
May 20, 2024
2855832
WIP: New price based `AdaptPrice` trait.
May 20, 2024
627acf7
Pub
May 21, 2024
31ea3a9
Also adjust sellout_price on renew.
May 21, 2024
63f5856
Adapt price implementation.
May 21, 2024
3630ccc
Some tests + handle going down to zero gracefully.
May 21, 2024
b615fbe
Fix tests.
May 21, 2024
fd03dc1
More linear tests.
May 21, 2024
6fdc11b
Add prdoc.
May 22, 2024
cb5485d
Put base price in the middle of the range.
May 23, 2024
9299010
new leadin curve.
May 23, 2024
28f13f4
price -> base_price
May 23, 2024
4b5a66d
Fmt
May 23, 2024
fad4aab
base_price -> min_price
May 24, 2024
4979fbb
Merge branch 'master' into rk-broker-new-price-adapter
May 27, 2024
5869882
New example adapter name + more parameters (cores)
May 27, 2024
baeab0e
Fix kitchensink
May 27, 2024
aa4defc
Some logging + test.
May 27, 2024
d5992b4
Fix prdoc
May 27, 2024
03db451
regular_price -> end_price
May 27, 2024
d67dfc4
Fixes.
May 27, 2024
edeaf69
Merge remote-tracking branch 'origin/master' into rk-broker-new-price…
May 27, 2024
890e6ca
Fix benchmarks
May 27, 2024
66bce5a
Update prdoc/pr_4521.prdoc
eskimor May 29, 2024
fbe5241
min_price -> end_price
May 29, 2024
fdf764e
Better docs for target_price.
May 29, 2024
86ec07c
Improved prdoc
May 29, 2024
1ca75af
Merge remote-tracking branch 'origin/rk-broker-new-price-adapter' int…
May 29, 2024
2a940a5
Make change major again.
May 29, 2024
52e0b80
One more test.
May 29, 2024
812f66c
Add migration for name change `PotentialRenewals`.
May 29, 2024
ae20d3c
Merge remote-tracking branch 'origin/master' into rk-broker-new-price…
May 29, 2024
7e2992c
Fix prdoc?
May 29, 2024
654d1e4
Merge branch 'master' of https://github.com/paritytech/polkadot-sdk i…
May 29, 2024
4cc238a
".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…
May 29, 2024
d6857aa
Change bump to minor again to make CI happy.
May 29, 2024
c98a945
Merge remote-tracking branch 'origin/rk-broker-new-price-adapter' int…
May 29, 2024
d179b89
prdoc fix
May 29, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

Original file line number Diff line number Diff line change
Expand Up @@ -232,5 +232,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::Linear;
type PriceAdapter = pallet_broker::CenterTargetPrice<Balance>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ impl<T: frame_system::Config> pallet_broker::WeightInfo for WeightInfo<T> {
/// Proof: `Broker::Status` (`max_values`: Some(1), `max_size`: Some(18), added: 513, mode: `MaxEncodedLen`)
/// Storage: `Broker::SaleInfo` (r:1 w:1)
/// Proof: `Broker::SaleInfo` (`max_values`: Some(1), `max_size`: Some(57), added: 552, mode: `MaxEncodedLen`)
/// Storage: `Broker::AllowedRenewals` (r:1 w:2)
/// Proof: `Broker::AllowedRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
/// Storage: `Broker::PotentialRenewals` (r:1 w:2)
/// Proof: `Broker::PotentialRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
/// Storage: `System::Account` (r:1 w:0)
/// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`)
/// Storage: `Broker::Workplan` (r:0 w:1)
Expand Down Expand Up @@ -337,8 +337,8 @@ impl<T: frame_system::Config> pallet_broker::WeightInfo for WeightInfo<T> {
}
/// Storage: `Broker::Status` (r:1 w:0)
/// Proof: `Broker::Status` (`max_values`: Some(1), `max_size`: Some(18), added: 513, mode: `MaxEncodedLen`)
/// Storage: `Broker::AllowedRenewals` (r:1 w:1)
/// Proof: `Broker::AllowedRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
/// Storage: `Broker::PotentialRenewals` (r:1 w:1)
/// Proof: `Broker::PotentialRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
fn drop_renewal() -> Weight {
// Proof Size summary in bytes:
// Measured: `957`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,5 +245,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::Linear;
type PriceAdapter = pallet_broker::CenterTargetPrice<Balance>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ impl<T: frame_system::Config> pallet_broker::WeightInfo for WeightInfo<T> {
/// Proof: `Broker::Status` (`max_values`: Some(1), `max_size`: Some(18), added: 513, mode: `MaxEncodedLen`)
/// Storage: `Broker::SaleInfo` (r:1 w:1)
/// Proof: `Broker::SaleInfo` (`max_values`: Some(1), `max_size`: Some(57), added: 552, mode: `MaxEncodedLen`)
/// Storage: `Broker::AllowedRenewals` (r:1 w:2)
/// Proof: `Broker::AllowedRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
/// Storage: `Broker::PotentialRenewals` (r:1 w:2)
/// Proof: `Broker::PotentialRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
/// Storage: `System::Account` (r:1 w:0)
/// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`)
/// Storage: `Broker::Workplan` (r:0 w:1)
Expand Down Expand Up @@ -335,8 +335,8 @@ impl<T: frame_system::Config> pallet_broker::WeightInfo for WeightInfo<T> {
}
/// Storage: `Broker::Status` (r:1 w:0)
/// Proof: `Broker::Status` (`max_values`: Some(1), `max_size`: Some(18), added: 513, mode: `MaxEncodedLen`)
/// Storage: `Broker::AllowedRenewals` (r:1 w:1)
/// Proof: `Broker::AllowedRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
/// Storage: `Broker::PotentialRenewals` (r:1 w:1)
/// Proof: `Broker::PotentialRenewals` (`max_values`: None, `max_size`: Some(1233), added: 3708, mode: `MaxEncodedLen`)
fn drop_renewal() -> Weight {
// Proof Size summary in bytes:
// Measured: `556`
Expand Down
24 changes: 24 additions & 0 deletions prdoc/pr_4521.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
title: AdaptPrice trait is now price controlled

doc:
- audience: Runtime Dev
description: |
The broker pallet price adaptation interface is changed to be less opinionated and more
information is made available to the `AdaptPrice` trait. A new example impl is included which
adapts the price based not on the number of cores sold, but rather on the price that was
achieved during the sale to mitigate a potential price manipulation vector. More information
here:

https://github.com/paritytech/polkadot-sdk/issues/4360

- audience: Runtime User
description: |
The price controller of the Rococo and Westend Coretime chain will be
adjusted with this release. This will very likely be used in the
fellowship production runtime to have a much larger leadin. This fixes a
price manipulation issue we discovered with the Kusama launch.

crates:
- name: pallet-broker
bump: minor

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

parameter_types! {
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/broker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ frame-system = { path = "../system", default-features = false }

[dev-dependencies]
sp-io = { path = "../../primitives/io" }
sp-tracing = { path = "../../primitives/tracing" }
pretty_assertions = "1.3.0"

[features]
Expand Down
226 changes: 166 additions & 60 deletions substrate/frame/broker/src/adapt_price.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,59 +17,122 @@

#![deny(missing_docs)]

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

/// Performance of a past sale.
#[derive(Copy, Clone)]
pub struct SalePerformance<Balance> {
eskimor marked this conversation as resolved.
Show resolved Hide resolved
/// 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.end_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 {
pub trait AdaptPrice<Balance> {
/// 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 the correction factor by which the regular price must be multiplied based on market
/// performance.

/// Return adapted prices for next sale.
///
/// - `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;
/// Based on the previous sale's performance.
fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance>;
}

impl AdaptPrice for () {
impl<Balance: Copy> AdaptPrice<Balance> for () {
fn leadin_factor_at(_: FixedU64) -> FixedU64 {
FixedU64::one()
}
fn adapt_price(_: CoreIndex, _: CoreIndex, _: CoreIndex) -> 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 }
}
}

/// Simple implementation of `AdaptPrice` giving a monotonic leadin and a linear price change based
/// on cores sold.
pub struct Linear;
eskimor marked this conversation as resolved.
Show resolved Hide resolved
impl AdaptPrice for Linear {
/// 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> {
fn leadin_factor_at(when: FixedU64) -> FixedU64 {
FixedU64::from(2).saturating_sub(when)
}
fn adapt_price(sold: CoreIndex, target: CoreIndex, limit: CoreIndex) -> FixedU64 {
if sold <= target {
// Range of [0.5, 1.0].
FixedU64::from_rational(1, 2).saturating_add(FixedU64::from_rational(
sold.into(),
target.saturating_mul(2).into(),
))
if when <= FixedU64::from_rational(1, 2) {
FixedU64::from(100).saturating_sub(when.saturating_mul(180.into()))
} else {
// Range of (1.0, 2].

// Unchecked math: In this branch we know that sold > target. The limit must be >= sold
// by construction, and thus target must be < limit.
FixedU64::one().saturating_add(FixedU64::from_rational(
(sold - target).into(),
(limit - target).into(),
))
FixedU64::from(19).saturating_sub(when.saturating_mul(18.into()))
}
}

fn adapt_price(performance: SalePerformance<Balance>) -> AdaptedPrices<Balance> {
let Some(sellout_price) = performance.sellout_price else {
eskimor marked this conversation as resolved.
Show resolved Hide resolved
return AdaptedPrices {
end_price: performance.end_price,
target_price: FixedU64::from(10).saturating_mul_int(performance.end_price),
}
};

eskimor marked this conversation as resolved.
Show resolved Hide resolved
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
} else {
price
};

AdaptedPrices { end_price: price, target_price: sellout_price }
}
}

#[cfg(test)]
Expand All @@ -78,37 +141,80 @@ mod tests {

#[test]
fn linear_no_panic() {
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());
}
}
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));
}
}
}

#[test]
fn linear_bound_check() {
// Using constraints from pallet implementation i.e. `limit >= sold`.
// Check extremes
let limit = 10;
let target = 5;

// Maximally sold: `sold == limit`
assert_eq!(Linear::adapt_price(limit, target, limit), FixedU64::from_float(2.0));
// Ideally sold: `sold == target`
assert_eq!(Linear::adapt_price(target, target, limit), FixedU64::one());
// Minimally sold: `sold == 0`
assert_eq!(Linear::adapt_price(0, target, limit), FixedU64::from_float(0.5));
// Optimistic target: `target == limit`
assert_eq!(Linear::adapt_price(limit, limit, limit), FixedU64::one());
// Pessimistic target: `target == 0`
assert_eq!(Linear::adapt_price(limit, 0, limit), FixedU64::from_float(2.0));
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);
}
}
Loading
Loading