Update lowest unbaked storage. (#9750)

* update lowest unbaked

* fix format

* add note

* fmt

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.

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) {

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);
 	});
 }

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());
+	});
+}