move enactment of approved proposals to start of assigning phase

communities/src/lib.rs

@@ -175,6 +175,11 @@ pub mod pallet {
 			location: Location,
 		) -> DispatchResultWithPostInfo {
 			T::TrustableForNonDestructiveAction::ensure_origin(origin)?;
+			ensure!(
+				<pallet_encointer_scheduler::Pallet<T>>::current_phase() ==
+					CeremonyPhaseType::Registering,
+				Error::<T>::RegistrationPhaseRequired
+			);
 			Self::do_add_location(cid, location)
 		}
 
@@ -191,6 +196,11 @@ pub mod pallet {
 			location: Location,
 		) -> DispatchResultWithPostInfo {
 			T::CommunityMaster::ensure_origin(origin)?;
+			ensure!(
+				<pallet_encointer_scheduler::Pallet<T>>::current_phase() ==
+					CeremonyPhaseType::Registering,
+				Error::<T>::RegistrationPhaseRequired
+			);
 			Self::do_remove_location(cid, location)
 		}
 
@@ -409,11 +419,6 @@ impl<T: Config> Pallet<T> {
 		cid: CommunityIdentifier,
 		location: Location,
 	) -> DispatchResultWithPostInfo {
-		ensure!(
-			<pallet_encointer_scheduler::Pallet<T>>::current_phase() ==
-				CeremonyPhaseType::Registering,
-			Error::<T>::RegistrationPhaseRequired
-		);
 		Self::ensure_cid_exists(&cid)?;
 		Self::validate_location(&location)?;
 		let geo_hash = GeoHash::try_from_params(location.lat, location.lon)
@@ -450,11 +455,6 @@ impl<T: Config> Pallet<T> {
 		cid: CommunityIdentifier,
 		location: Location,
 	) -> DispatchResultWithPostInfo {
-		ensure!(
-			<pallet_encointer_scheduler::Pallet<T>>::current_phase() ==
-				CeremonyPhaseType::Registering,
-			Error::<T>::RegistrationPhaseRequired
-		);
 		Self::ensure_cid_exists(&cid)?;
 
 		let geo_hash = GeoHash::try_from_params(location.lat, location.lon)

democracy/src/lib.rs

@@ -519,7 +519,7 @@ pub mod pallet {
 
 			let turnout_permill = (tally.turnout * 1000).checked_div(electorate).unwrap_or(0);
 			if turnout_permill < T::MinTurnout::get() {
-				return Ok(false);
+				return Ok(false)
 			}
 
 			Self::positive_turnout_bias(electorate, tally.turnout, tally.ayes)
@@ -561,9 +561,7 @@ pub mod pallet {
 impl<T: Config> OnCeremonyPhaseChange for Pallet<T> {
 	fn on_ceremony_phase_change(new_phase: CeremonyPhaseType) {
 		match new_phase {
-			CeremonyPhaseType::Assigning => {},
-			CeremonyPhaseType::Attesting => {},
-			CeremonyPhaseType::Registering => {
+			CeremonyPhaseType::Assigning => {
 				// safe as EnactmentQueue has one key per ProposalActionType and those are bounded
 				<EnactmentQueue<T>>::iter().for_each(|p| {
 					if let Err(e) = Self::enact_proposal(p.1) {
@@ -573,6 +571,8 @@ impl<T: Config> OnCeremonyPhaseChange for Pallet<T> {
 				// remove all keys from the map
 				<EnactmentQueue<T>>::translate::<ProposalIdType, _>(|_, _| None);
 			},
+			CeremonyPhaseType::Attesting => {},
+			CeremonyPhaseType::Registering => {},
 		}
 	}
 }

democracy/src/mock.rs

@@ -45,9 +45,9 @@ frame_support::construct_runtime!(
 impl dut::Config for TestRuntime {
 	type RuntimeEvent = RuntimeEvent;
 	type MaxReputationCount = ConstU32<10>;
-	// 10 blocks
+	// 10 6s blocks
 	type ConfirmationPeriod = ConstU64<60000>;
-	// 40 blocks
+	// 40 6s blocks
 	type ProposalLifetime = ConstU64<240000>;
 	type MinTurnout = ConstU128<20>; // 2%
 	type WeightInfo = (); // 2%

democracy/src/tests.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Encointer.  If not, see <http://www.gnu.org/licenses/>.
 
-//! Unit tests for the tokens module.
+//! Unit tests for the encointer democracy module.
 
 use super::*;
 use crate::mock::{
@@ -378,11 +378,11 @@ fn voting_works() {
 			1,
 			Vote::Nay,
 			BoundedVec::try_from(vec![
-				(cid, 2),  // invalid beacuse out of range
-				(cid, 4),  // invalid beacuse already used
-				(cid2, 4), // invlaid because unverified
+				(cid, 2),  // invalid because out of range
+				(cid, 4),  // invalid because already used
+				(cid2, 4), // invalid because unverified
 				(cid2, 5), // valid
-				(cid2, 6), // invlaid because out of range
+				(cid2, 6), // invalid because out of range
 				(cid2, 3), // invlalid non-existent
 			])
 			.unwrap()
@@ -682,8 +682,7 @@ fn enactment_updates_proposal_metadata_and_enactment_queue() {
 		EnactmentQueue::<TestRuntime>::insert(proposal_action2.clone().get_identifier(), 2);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted);
 
@@ -737,7 +736,9 @@ fn proposal_happy_flow() {
 			Some(Event::VotePlaced { proposal_id: 1, vote: Vote::Aye, num_votes: 3 }.into())
 		);
 
+		// let pass the proposal lifetime
 		advance_n_blocks(40);
+
 		assert_ok!(EncointerDemocracy::vote(
 			RuntimeOrigin::signed(alice.clone()),
 			1,
@@ -762,8 +763,7 @@ fn proposal_happy_flow() {
 		);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(
 			event_at_index::<TestRuntime>(get_num_events::<TestRuntime>() - 2),
@@ -792,12 +792,14 @@ fn enact_add_location_works() {
 		));
 
 		let geo_hash = GeoHash::try_from_params(location.lat, location.lon).unwrap();
+
+		// directly inject the proposal into the enactment queue
 		EnactmentQueue::<TestRuntime>::insert(proposal_action.clone().get_identifier(), 1);
+
 		assert_eq!(EncointerCommunities::locations(cid, geo_hash.clone()).len(), 0);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted);
 		assert_eq!(EncointerDemocracy::enactment_queue(proposal_action.get_identifier()), None);
@@ -819,12 +821,13 @@ fn enact_remove_location_works() {
 		));
 
 		let geo_hash = GeoHash::try_from_params(location.lat, location.lon).unwrap();
+
+		// directly inject the proposal into the enactment queue
 		EnactmentQueue::<TestRuntime>::insert(proposal_action.clone().get_identifier(), 1);
 		assert_eq!(EncointerCommunities::locations(cid, geo_hash.clone()).len(), 1);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted);
 		assert_eq!(EncointerDemocracy::enactment_queue(proposal_action.get_identifier()), None);
@@ -849,11 +852,11 @@ fn enact_update_community_metadata_works() {
 			proposal_action.clone()
 		));
 
+		// directly inject the proposal into the enactment queue
 		EnactmentQueue::<TestRuntime>::insert(proposal_action.clone().get_identifier(), 1);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted);
 		assert_eq!(EncointerDemocracy::enactment_queue(proposal_action.get_identifier()), None);
@@ -877,13 +880,13 @@ fn enact_update_demurrage_works() {
 			proposal_action.clone()
 		));
 
+		// directly inject the proposal into the enactment queue
 		EnactmentQueue::<TestRuntime>::insert(proposal_action.clone().get_identifier(), 1);
 
 		assert!(EncointerBalances::demurrage_per_block(&cid) != demurrage);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted);
 		assert_eq!(EncointerDemocracy::enactment_queue(proposal_action.get_identifier()), None);
@@ -903,11 +906,11 @@ fn enact_update_nominal_income_works() {
 			proposal_action.clone()
 		));
 
+		// directly inject the proposal into the enactment queue
 		EnactmentQueue::<TestRuntime>::insert(proposal_action.clone().get_identifier(), 1);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted);
 		assert_eq!(EncointerDemocracy::enactment_queue(proposal_action.get_identifier()), None);
@@ -926,11 +929,11 @@ fn enact_set_inactivity_timeout_works() {
 			proposal_action.clone()
 		));
 
+		// directly inject the proposal into the enactment queue
 		EnactmentQueue::<TestRuntime>::insert(proposal_action.clone().get_identifier(), 1);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		assert_eq!(EncointerDemocracy::proposals(1).unwrap().state, ProposalState::Enacted);
 		assert_eq!(
@@ -948,7 +951,10 @@ fn enact_set_inactivity_timeout_works() {
 fn enactment_error_fires_event() {
 	new_test_ext().execute_with(|| {
 		System::set_block_number(System::block_number() + 1); // this is needed to assert events
+
+		// use inexisting community to cause an enactment error
 		let cid = CommunityIdentifier::from_str("u0qj944rhWE").unwrap();
+
 		let alice = alice();
 		let proposal_action =
 			ProposalAction::UpdateNominalIncome(cid, NominalIncomeType::from(13037u32));
@@ -957,11 +963,11 @@ fn enactment_error_fires_event() {
 			proposal_action.clone()
 		));
 
+		// directly inject the proposal into the enactment queue
 		EnactmentQueue::<TestRuntime>::insert(proposal_action.clone().get_identifier(), 1);
 
 		run_to_next_phase();
-		run_to_next_phase();
-		run_to_next_phase();
+		// first assigning phase after proposal lifetime ended
 
 		match event_at_index::<TestRuntime>(get_num_events::<TestRuntime>() - 2).unwrap() {
 			mock::RuntimeEvent::EncointerDemocracy(Event::EnactmentFailed {

test-utils/src/lib.rs

@@ -246,11 +246,12 @@ parameter_types! {
 
 #[macro_export]
 macro_rules! impl_encointer_scheduler {
-	($t:ident, $ceremonies:ident, $reputationcommitments:ident) => {
+	($t:ident, $ceremonies:ident, $othercallbackpallet:ident) => {
 		impl pallet_encointer_scheduler::Config for $t {
 			type RuntimeEvent = RuntimeEvent;
 			type CeremonyMaster = EnsureAlice;
-			type OnCeremonyPhaseChange = ($ceremonies, $reputationcommitments); //OnCeremonyPhaseChange;
+			// democracy callback MUST be executed before ceremony callback!
+			type OnCeremonyPhaseChange = ($othercallbackpallet, $ceremonies); //OnCeremonyPhaseChange;
 			type MomentsPerDay = MomentsPerDay;
 			type WeightInfo = ();
 		}