From 512f3efbdf262fbc3757bdd33e3039ecea827627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Mon, 11 Mar 2024 21:12:27 +0700 Subject: [PATCH] [pallet_broker] Fix `adapt_price` behaviour at zero (#3636) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes the behaviour of `Linear` which is the default implementation of the `AdaptPrice` trait in the broker pallet. Previously if cores were offered but not sold in only one sale, the price would be set to zero and due to the logic being purely multiplicative, the price would stay at 0 indefinitely. This could be further paired with a configurable minimum in the broker pallet itself, which will be a future PR. This affects the Rococo and Westend Coretime chains, but Kusama has a different implementation so this isn't required for the Kusama launch. I actually thought I opened this a while ago. --------- Co-authored-by: Bastian Köcher --- prdoc/pr_3636.prdoc | 15 +++++++++ substrate/frame/broker/src/adapt_price.rs | 38 ++++++++++++++++++++--- 2 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 prdoc/pr_3636.prdoc diff --git a/prdoc/pr_3636.prdoc b/prdoc/pr_3636.prdoc new file mode 100644 index 000000000000..934158ac1022 --- /dev/null +++ b/prdoc/pr_3636.prdoc @@ -0,0 +1,15 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "[pallet_broker] Fix `Linear::adapt_price` behavior at zero" + +doc: + - audience: Runtime Dev + description: | + This fixes the behaviour of `Linear` which is the default implementation of the `AdaptPrice` + trait in the broker pallet. Previously if cores were offered but not sold in only one sale, + the price would be set to zero and due to the logic being purely multiplicative, the price + would stay at 0 indefinitely. + +crates: + - name: pallet-broker diff --git a/substrate/frame/broker/src/adapt_price.rs b/substrate/frame/broker/src/adapt_price.rs index 8266625687a2..fbcd7afdf0da 100644 --- a/substrate/frame/broker/src/adapt_price.rs +++ b/substrate/frame/broker/src/adapt_price.rs @@ -19,6 +19,7 @@ use crate::CoreIndex; use sp_arithmetic::{traits::One, FixedU64}; +use sp_runtime::Saturating; /// Type for determining how to set price. pub trait AdaptPrice { @@ -49,14 +50,24 @@ impl AdaptPrice for () { pub struct Linear; impl AdaptPrice for Linear { fn leadin_factor_at(when: FixedU64) -> FixedU64 { - FixedU64::from(2) - when + FixedU64::from(2).saturating_sub(when) } fn adapt_price(sold: CoreIndex, target: CoreIndex, limit: CoreIndex) -> FixedU64 { if sold <= target { - FixedU64::from_rational(sold.into(), target.into()) + // Range of [0.5, 1.0]. + FixedU64::from_rational(1, 2).saturating_add(FixedU64::from_rational( + sold.into(), + target.saturating_mul(2).into(), + )) } else { - FixedU64::one() + - FixedU64::from_rational((sold - target).into(), (limit - target).into()) + // 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(), + )) } } } @@ -81,4 +92,23 @@ mod tests { } } } + + #[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)); + } }