From b7767168b7dd93964f40e8543b853097e3570621 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <git@kchr.de>
Date: Mon, 24 Jun 2024 12:37:13 +0200
Subject: [PATCH] Ensure earliest allowed block is at minimum the next block
(#4823)
When `min_enactment_period == 0` and `desired == At(n)` where `n` is
smaller than the current block number, the scheduling would fail. This
happened for example here:
https://collectives.subsquare.io/fellowship/referenda/126
To ensure that this doesn't happen again, ensure that the earliest
allowed block is at minimum the next block.
---
prdoc/pr_4823.prdoc | 11 +++++++++++
substrate/frame/referenda/src/lib.rs | 3 ++-
substrate/frame/referenda/src/mock.rs | 25 ++++++++++++++++++++++++-
substrate/frame/referenda/src/tests.rs | 24 ++++++++++++++++++++++++
4 files changed, 61 insertions(+), 2 deletions(-)
create mode 100644 prdoc/pr_4823.prdoc
diff --git a/prdoc/pr_4823.prdoc b/prdoc/pr_4823.prdoc
new file mode 100644
index 00000000000..a498b33f7bf
--- /dev/null
+++ b/prdoc/pr_4823.prdoc
@@ -0,0 +1,11 @@
+title: "`pallet-referenda`: Ensure to schedule referendas earliest at the next block"
+
+doc:
+ - audience: Runtime User
+ description: |
+ Ensure that referendas are scheduled earliest at the next block when they are enacted.
+ Otherwise the scheduling may fails and thus, the enactment of the referenda.
+
+crates:
+ - name: pallet-referenda
+ bump: patch
diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs
index fbe27e1a478..0cdf450d3b6 100644
--- a/substrate/frame/referenda/src/lib.rs
+++ b/substrate/frame/referenda/src/lib.rs
@@ -891,7 +891,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
call: BoundedCallOf<T, I>,
) {
let now = frame_system::Pallet::<T>::block_number();
- let earliest_allowed = now.saturating_add(track.min_enactment_period);
+ // Earliest allowed block is always at minimum the next block.
+ let earliest_allowed = now.saturating_add(track.min_enactment_period.max(One::one()));
let desired = desired.evaluate(now);
let ok = T::Scheduler::schedule_named(
(ASSEMBLY_ID, "enactment", index).using_encoded(sp_io::hashing::blake2_256),
diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs
index d47da455811..bf0fa4e1a12 100644
--- a/substrate/frame/referenda/src/mock.rs
+++ b/substrate/frame/referenda/src/mock.rs
@@ -112,7 +112,7 @@ impl TracksInfo<u64, u64> for TestTracksInfo {
type Id = u8;
type RuntimeOrigin = <RuntimeOrigin as OriginTrait>::PalletsOrigin;
fn tracks() -> &'static [(Self::Id, TrackInfo<u64, u64>)] {
- static DATA: [(u8, TrackInfo<u64, u64>); 2] = [
+ static DATA: [(u8, TrackInfo<u64, u64>); 3] = [
(
0u8,
TrackInfo {
@@ -157,6 +157,28 @@ impl TracksInfo<u64, u64> for TestTracksInfo {
},
},
),
+ (
+ 2u8,
+ TrackInfo {
+ name: "none",
+ max_deciding: 3,
+ decision_deposit: 1,
+ prepare_period: 2,
+ decision_period: 2,
+ confirm_period: 1,
+ min_enactment_period: 0,
+ 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[..]
}
@@ -165,6 +187,7 @@ impl TracksInfo<u64, u64> for TestTracksInfo {
match system_origin {
frame_system::RawOrigin::Root => Ok(0),
frame_system::RawOrigin::None => Ok(1),
+ frame_system::RawOrigin::Signed(1) => Ok(2),
_ => Err(()),
}
} else {
diff --git a/substrate/frame/referenda/src/tests.rs b/substrate/frame/referenda/src/tests.rs
index 52251fcbdbe..3f859636f7c 100644
--- a/substrate/frame/referenda/src/tests.rs
+++ b/substrate/frame/referenda/src/tests.rs
@@ -682,3 +682,27 @@ fn detects_incorrect_len() {
);
});
}
+
+/// Ensures that `DispatchTime::After(0)` plus `min_enactment_period = 0` works.
+#[test]
+fn zero_enactment_delay_executes_proposal_at_next_block() {
+ ExtBuilder::default().build_and_execute(|| {
+ assert_eq!(Balances::free_balance(42), 0);
+ assert_ok!(Referenda::submit(
+ RuntimeOrigin::signed(1),
+ Box::new(RawOrigin::Signed(1).into()),
+ Preimage::bound(
+ pallet_balances::Call::transfer_keep_alive { dest: 42, value: 20 }.into()
+ )
+ .unwrap(),
+ DispatchTime::After(0),
+ ));
+ assert_ok!(Referenda::place_decision_deposit(RuntimeOrigin::signed(1), 0));
+ assert_eq!(ReferendumCount::<Test>::get(), 1);
+ set_tally(0, 100, 0);
+
+ run_to(9);
+
+ assert_eq!(Balances::free_balance(42), 20);
+ });
+}
--
GitLab