Skip to content

Commit

Permalink
Ensure earliest allowed block is at minimum the next block (#4823)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bkchr authored Jun 24, 2024
1 parent fed81f7 commit b776716
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 2 deletions.
11 changes: 11 additions & 0 deletions prdoc/pr_4823.prdoc
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion substrate/frame/referenda/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
25 changes: 24 additions & 1 deletion substrate/frame/referenda/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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[..]
}
Expand All @@ -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 {
Expand Down
24 changes: 24 additions & 0 deletions substrate/frame/referenda/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

0 comments on commit b776716

Please sign in to comment.