diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs
index 50b245006fa246f8ec4859a3de5ccb80ef003805..893e4676bef7b3a922f62c9aa599085dcea92c29 100644
--- a/substrate/frame/democracy/src/lib.rs
+++ b/substrate/frame/democracy/src/lib.rs
@@ -613,10 +613,7 @@ pub mod pallet {
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
/// Weight: see `begin_block`
fn on_initialize(n: T::BlockNumber) -> Weight {
- Self::begin_block(n).unwrap_or_else(|e| {
- sp_runtime::print(e);
- 0
- })
+ Self::begin_block(n)
}
}
@@ -1682,7 +1679,7 @@ impl<T: Config> Pallet<T> {
now: T::BlockNumber,
index: ReferendumIndex,
status: ReferendumStatus<T::BlockNumber, T::Hash, BalanceOf<T>>,
- ) -> Result<bool, DispatchError> {
+ ) -> bool {
let total_issuance = T::Currency::total_issuance();
let approved = status.threshold.approved(status.tally, total_issuance);
@@ -1719,7 +1716,7 @@ impl<T: Config> Pallet<T> {
Self::deposit_event(Event::<T>::NotPassed(index));
}
- Ok(approved)
+ approved
}
/// Current era is ending; we should finish up any proposals.
@@ -1734,7 +1731,7 @@ impl<T: Config> Pallet<T> {
/// - Db writes: `PublicProps`, `account`, `ReferendumCount`, `DepositOf`, `ReferendumInfoOf`
/// - Db reads per R: `DepositOf`, `ReferendumInfoOf`
/// # </weight>
- fn begin_block(now: T::BlockNumber) -> Result<Weight, DispatchError> {
+ fn begin_block(now: T::BlockNumber) -> Weight {
let max_block_weight = T::BlockWeights::get().max_block;
let mut weight = 0;
@@ -1758,12 +1755,29 @@ impl<T: Config> Pallet<T> {
// tally up votes for any expiring referenda.
for (index, info) in Self::maturing_referenda_at_inner(now, next..last).into_iter() {
- let approved = Self::bake_referendum(now, index, info)?;
+ let approved = Self::bake_referendum(now, index, info);
ReferendumInfoOf::<T>::insert(index, ReferendumInfo::Finished { end: now, approved });
weight = max_block_weight;
}
- Ok(weight)
+ // Notes:
+ // * We don't consider the lowest unbaked to be the last maturing in case some refendum have
+ // longer voting period than others.
+ // * The iteration here shouldn't trigger any storage read that are not in cache, due to
+ // `maturing_referenda_at_inner` having already read them.
+ // * We shouldn't iterate more than `LaunchPeriod/VotingPeriod + 1` times because the number
+ // of unbaked referendum is bounded by this number. In case those number have changed in a
+ // runtime upgrade the formula should be adjusted but the bound should still be sensible.
+ <LowestUnbaked<T>>::mutate(|ref_index| {
+ while *ref_index < last &&
+ Self::referendum_info(*ref_index)
+ .map_or(true, |info| matches!(info, ReferendumInfo::Finished { .. }))
+ {
+ *ref_index += 1
+ }
+ });
+
+ weight
}
/// Reads the length of account in DepositOf without getting the complete value in the runtime.
diff --git a/substrate/frame/democracy/src/tests.rs b/substrate/frame/democracy/src/tests.rs
index 75104db51b971e73bd67bc23b3d67d382bab9ac2..f56667e9094b313e7583f9ff720002b21cd5d299 100644
--- a/substrate/frame/democracy/src/tests.rs
+++ b/substrate/frame/democracy/src/tests.rs
@@ -264,7 +264,7 @@ fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> DispatchRes
fn next_block() {
System::set_block_number(System::block_number() + 1);
Scheduler::on_initialize(System::block_number());
- assert!(Democracy::begin_block(System::block_number()).is_ok());
+ Democracy::begin_block(System::block_number());
}
fn fast_forward_to(n: u64) {
diff --git a/substrate/frame/democracy/src/tests/cancellation.rs b/substrate/frame/democracy/src/tests/cancellation.rs
index c2bd725ce934a49cab3ea31c0b038954dbbb68bc..83822bf51829f564d25cac846a4e7b4fe92f969e 100644
--- a/substrate/frame/democracy/src/tests/cancellation.rs
+++ b/substrate/frame/democracy/src/tests/cancellation.rs
@@ -30,10 +30,14 @@ fn cancel_referendum_should_work() {
);
assert_ok!(Democracy::vote(Origin::signed(1), r, aye(1)));
assert_ok!(Democracy::cancel_referendum(Origin::root(), r.into()));
+ assert_eq!(Democracy::lowest_unbaked(), 0);
next_block();
+
next_block();
+ assert_eq!(Democracy::lowest_unbaked(), 1);
+ assert_eq!(Democracy::lowest_unbaked(), Democracy::referendum_count());
assert_eq!(Balances::free_balance(42), 0);
});
}
diff --git a/substrate/frame/democracy/src/tests/scheduling.rs b/substrate/frame/democracy/src/tests/scheduling.rs
index 06b492bc6093c228378db25e279d12e7638d62ed..5c857a632b97b8a1371fbf1f64be2bac9ba0385a 100644
--- a/substrate/frame/democracy/src/tests/scheduling.rs
+++ b/substrate/frame/democracy/src/tests/scheduling.rs
@@ -30,8 +30,10 @@ fn simple_passing_should_work() {
);
assert_ok!(Democracy::vote(Origin::signed(1), r, aye(1)));
assert_eq!(tally(r), Tally { ayes: 1, nays: 0, turnout: 10 });
+ assert_eq!(Democracy::lowest_unbaked(), 0);
next_block();
next_block();
+ assert_eq!(Democracy::lowest_unbaked(), 1);
assert_eq!(Balances::free_balance(42), 2);
});
}
@@ -110,3 +112,45 @@ fn delayed_enactment_should_work() {
assert_eq!(Balances::free_balance(42), 2);
});
}
+
+#[test]
+fn lowest_unbaked_should_be_sensible() {
+ new_test_ext().execute_with(|| {
+ let r1 = Democracy::inject_referendum(
+ 3,
+ set_balance_proposal_hash_and_note(1),
+ VoteThreshold::SuperMajorityApprove,
+ 0,
+ );
+ let r2 = Democracy::inject_referendum(
+ 2,
+ set_balance_proposal_hash_and_note(2),
+ VoteThreshold::SuperMajorityApprove,
+ 0,
+ );
+ let r3 = Democracy::inject_referendum(
+ 10,
+ set_balance_proposal_hash_and_note(3),
+ VoteThreshold::SuperMajorityApprove,
+ 0,
+ );
+ assert_ok!(Democracy::vote(Origin::signed(1), r1, aye(1)));
+ assert_ok!(Democracy::vote(Origin::signed(1), r2, aye(1)));
+ // r3 is canceled
+ assert_ok!(Democracy::cancel_referendum(Origin::root(), r3.into()));
+ assert_eq!(Democracy::lowest_unbaked(), 0);
+
+ next_block();
+
+ // r2 is approved
+ assert_eq!(Balances::free_balance(42), 2);
+ assert_eq!(Democracy::lowest_unbaked(), 0);
+
+ next_block();
+
+ // r1 is approved
+ assert_eq!(Balances::free_balance(42), 1);
+ assert_eq!(Democracy::lowest_unbaked(), 3);
+ assert_eq!(Democracy::lowest_unbaked(), Democracy::referendum_count());
+ });
+}