From 8dc910c8a18ad91a18ff17c492f5cc3977a5133e Mon Sep 17 00:00:00 2001
From: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Date: Sat, 17 Feb 2024 12:34:34 +0100
Subject: [PATCH] Ensure referenda `TracksInfo` is sorted (#3325)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Changes:
- Add `integrity_check` to trait `TracksInfo` to assert the sorting
- Use the check in `integrity_test`
- Rely upon sorted property in the `info` for speedup
Note that the pallet already relies upon this (so far) untested
assumption
[here](https://github.com/paritytech/polkadot-sdk/blob/44c7a5eb8c375d77f685abf585961421e5f8b228/substrate/frame/referenda/src/lib.rs#L1280).
---------
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
---
prdoc/pr_3308.prdoc | 2 +-
prdoc/pr_3325.prdoc | 10 +++
substrate/frame/referenda/src/lib.rs | 5 ++
substrate/frame/referenda/src/types.rs | 91 +++++++++++++++++++++++++-
4 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 prdoc/pr_3325.prdoc
diff --git a/prdoc/pr_3308.prdoc b/prdoc/pr_3308.prdoc
index 611461e25ff..386cbd6230b 100644
--- a/prdoc/pr_3308.prdoc
+++ b/prdoc/pr_3308.prdoc
@@ -1,4 +1,4 @@
-title: Parachains-Aura: Only produce once per slot
+title: "Parachains-Aura: Only produce once per slot"
doc:
- audience: Node Dev
diff --git a/prdoc/pr_3325.prdoc b/prdoc/pr_3325.prdoc
new file mode 100644
index 00000000000..eb8126dc912
--- /dev/null
+++ b/prdoc/pr_3325.prdoc
@@ -0,0 +1,10 @@
+title: "Ensure `TracksInfo` tracks are sorted by ID."
+
+doc:
+ - audience: Runtime Dev
+ description: |
+ Add a `integrity_check` function to trait `TracksInfo` and explicitly state that tracks must
+ always be sorted by ID. The referenda pallet now also uses this check in its `integrity_test`.
+
+crates:
+ - name: pallet-referenda
diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs
index 8912f9ad217..c5bf2266e67 100644
--- a/substrate/frame/referenda/src/lib.rs
+++ b/substrate/frame/referenda/src/lib.rs
@@ -433,6 +433,11 @@ pub mod pallet {
Self::do_try_state()?;
Ok(())
}
+
+ #[cfg(any(feature = "std", test))]
+ fn integrity_test() {
+ T::Tracks::check_integrity().expect("Static tracks configuration is valid.");
+ }
}
#[pallet::call]
diff --git a/substrate/frame/referenda/src/types.rs b/substrate/frame/referenda/src/types.rs
index 0a8fb4ff820..b3c583322cc 100644
--- a/substrate/frame/referenda/src/types.rs
+++ b/substrate/frame/referenda/src/types.rs
@@ -144,7 +144,10 @@ pub trait TracksInfo<Balance, Moment> {
/// The origin type from which a track is implied.
type RuntimeOrigin;
- /// Return the array of known tracks and their information.
+ /// Sorted array of known tracks and their information.
+ ///
+ /// The array MUST be sorted by `Id`. Consumers of this trait are advised to assert
+ /// [`Self::check_integrity`] prior to any use.
fn tracks() -> &'static [(Self::Id, TrackInfo<Balance, Moment>)];
/// Determine the voting track for the given `origin`.
@@ -152,7 +155,23 @@ pub trait TracksInfo<Balance, Moment> {
/// Return the track info for track `id`, by default this just looks it up in `Self::tracks()`.
fn info(id: Self::Id) -> Option<&'static TrackInfo<Balance, Moment>> {
- Self::tracks().iter().find(|x| x.0 == id).map(|x| &x.1)
+ let tracks = Self::tracks();
+ let maybe_index = tracks.binary_search_by_key(&id, |t| t.0).ok()?;
+
+ tracks.get(maybe_index).map(|(_, info)| info)
+ }
+
+ /// Check assumptions about the static data that this trait provides.
+ fn check_integrity() -> Result<(), &'static str>
+ where
+ Balance: 'static,
+ Moment: 'static,
+ {
+ if Self::tracks().windows(2).all(|w| w[0].0 < w[1].0) {
+ Ok(())
+ } else {
+ Err("The tracks that were returned by `tracks` were not sorted by `Id`")
+ }
}
}
@@ -670,4 +689,72 @@ mod tests {
assert_eq!(c.delay(pc(30).less_epsilon()), pc(100));
assert_eq!(c.delay(pc(0)), pc(100));
}
+
+ #[test]
+ fn tracks_integrity_check_detects_unsorted() {
+ use crate::mock::RuntimeOrigin;
+
+ pub struct BadTracksInfo;
+ impl TracksInfo<u64, u64> for BadTracksInfo {
+ type Id = u8;
+ type RuntimeOrigin = <RuntimeOrigin as OriginTrait>::PalletsOrigin;
+ fn tracks() -> &'static [(Self::Id, TrackInfo<u64, u64>)] {
+ static DATA: [(u8, TrackInfo<u64, u64>); 2] = [
+ (
+ 1u8,
+ TrackInfo {
+ name: "root",
+ max_deciding: 1,
+ decision_deposit: 10,
+ prepare_period: 4,
+ decision_period: 4,
+ confirm_period: 2,
+ min_enactment_period: 4,
+ min_approval: Curve::LinearDecreasing {
+ length: Perbill::from_percent(100),
+ floor: Perbill::from_percent(50),
+ ceil: Perbill::from_percent(100),
+ },
+ min_support: Curve::LinearDecreasing {
+ length: Perbill::from_percent(100),
+ floor: Perbill::from_percent(0),
+ ceil: Perbill::from_percent(100),
+ },
+ },
+ ),
+ (
+ 0u8,
+ TrackInfo {
+ name: "none",
+ max_deciding: 3,
+ decision_deposit: 1,
+ prepare_period: 2,
+ decision_period: 2,
+ confirm_period: 1,
+ min_enactment_period: 2,
+ min_approval: Curve::LinearDecreasing {
+ length: Perbill::from_percent(100),
+ floor: Perbill::from_percent(95),
+ ceil: Perbill::from_percent(100),
+ },
+ min_support: Curve::LinearDecreasing {
+ length: Perbill::from_percent(100),
+ floor: Perbill::from_percent(90),
+ ceil: Perbill::from_percent(100),
+ },
+ },
+ ),
+ ];
+ &DATA[..]
+ }
+ fn track_for(_: &Self::RuntimeOrigin) -> Result<Self::Id, ()> {
+ unimplemented!()
+ }
+ }
+
+ assert_eq!(
+ BadTracksInfo::check_integrity(),
+ Err("The tracks that were returned by `tracks` were not sorted by `Id`")
+ );
+ }
}
--
GitLab