Broker new price adapter

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

@@ -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>;
 }

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

@@ -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)
@@ -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`

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

@@ -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>;
 }

cumulus/parachains/runtimes/coretime/coretime-westend/src/weights/pallet_broker.rs

@@ -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)
@@ -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`

prdoc/pr_4521.prdoc

@@ -0,0 +1,23 @@
+title: AdaptPrice trait is now price controlled
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      We discovered a flaw of the current price controller interface in the
+      broker pallet. This is fixed by changing the interface to no longer
+      operate on the number of cores sold, but rather on the price that was
+      achieved during the sale. More information here:
+
+      https://github.com/paritytech/polkadot-sdk/issues/4360
+
+  - audience: Runtime User
+    description: |
+      The price controller of the 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: major
+

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

@@ -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! {

substrate/frame/broker/Cargo.toml

@@ -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]

substrate/frame/broker/src/adapt_price.rs

@@ -17,59 +17,114 @@
 
 #![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> {
+	/// 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 min_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 min_price: Balance,
+	/// Price we optimize for.
+	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,
+			min_price: record.min_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>, min_price: Balance) -> Self {
+		Self { sellout_price, min_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.min_price);
+		AdaptedPrices { min_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;
-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 {
+			return AdaptedPrices {
+				min_price: performance.min_price,
+				target_price: FixedU64::from(10).saturating_mul_int(performance.min_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
+		} else {
+			price
+		};
+
+		AdaptedPrices { min_price: price, target_price: sellout_price }
+	}
 }
 
 #[cfg(test)]
@@ -78,37 +133,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.min_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.min_price = prices.min_price;
+
+			assert!(prices.min_price <= 101);
+			assert!(prices.min_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.min_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.min_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.min_price = prices.min_price;
+
+			assert!(prices.min_price <= sellout_price);
+			assert!(prices.min_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.min_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);
 	}
 }