fix(pallet-communities): on tests, must unlock on casted votes

virto-network · Apr 22, 2024 · 640e379 · 640e379
1 parent 0107bad
commit 640e379
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 28 deletions.
diff --git a/pallets/communities/src/benchmarking.rs b/pallets/communities/src/benchmarking.rs
@@ -406,7 +406,7 @@ mod benchmarks {
 		T::BenchmarkHelper::finish_poll(index)?;
 
 		#[extrinsic_call]
-		_(RawOrigin::Signed(who.clone()), membership_id, 0u32);
+		_(RawOrigin::Signed(who.clone()), 0u32);
 
 		// verification code
 		assert_eq!(

diff --git a/pallets/communities/src/functions.rs b/pallets/communities/src/functions.rs
@@ -122,7 +122,7 @@ impl<T: Config> Pallet<T> {
 			let (tally, class) = poll_status.ensure_ongoing().ok_or(Error::<T>::NotOngoing)?;
 			ensure!(community_id == class, Error::<T>::InvalidTrack);
 
-			let (vote, _) = CommunityVotes::<T>::get(poll_index, membership_id).ok_or(Error::<T>::NoVoteCasted)?;
+			let (vote, voter) = CommunityVotes::<T>::get(poll_index, membership_id).ok_or(Error::<T>::NoVoteCasted)?;
 			let vote_multiplier = match CommunityDecisionMethod::<T>::get(community_id) {
 				DecisionMethod::Rank => T::MemberMgmt::rank_of(&community_id, &membership_id)
 					.unwrap_or_default()
@@ -133,16 +133,11 @@ impl<T: Config> Pallet<T> {
 			let vote_weight = VoteWeight::from(&vote);
 			tally.remove_vote(vote.say(), vote_multiplier * vote_weight, vote_weight);
 
-			Self::do_unlock(membership_id, poll_index)
+			CommunityVotes::<T>::remove(poll_index, membership_id);
+			Self::update_locks(&voter, poll_index, &vote, LockUpdateType::Remove)
 		})
 	}
 
-	pub(crate) fn do_unlock(membership_id: MembershipIdOf<T>, poll_index: PollIndexOf<T>) -> DispatchResult {
-		let (vote, voter) = CommunityVotes::<T>::get(poll_index, membership_id).ok_or(Error::<T>::NoVoteCasted)?;
-		CommunityVotes::<T>::remove(poll_index, membership_id);
-		Self::update_locks(&voter, poll_index, &vote, LockUpdateType::Remove)
-	}
-
 	pub(crate) fn update_locks(
 		who: &AccountIdOf<T>,
 		poll_index: PollIndexOf<T>,

diff --git a/pallets/communities/src/lib.rs b/pallets/communities/src/lib.rs
@@ -517,15 +517,11 @@ pub mod pallet {
 		/// Make previously held or locked funds from a vote available
 		// if the refereundum  has finished
 		#[pallet::call_index(10)]
-		pub fn unlock(
-			origin: OriginFor<T>,
-			membership_id: MembershipIdOf<T>,
-			#[pallet::compact] poll_index: PollIndexOf<T>,
-		) -> DispatchResult {
+		pub fn unlock(origin: OriginFor<T>, #[pallet::compact] poll_index: PollIndexOf<T>) -> DispatchResult {
 			let who = ensure_signed(origin)?;
-			T::MemberMgmt::check_membership(&who, &membership_id).ok_or(Error::<T>::NotAMember)?;
 			ensure!(T::Polls::as_ongoing(poll_index).is_none(), Error::<T>::AlreadyOngoing);
-			Self::do_unlock(membership_id, poll_index)
+			let vote = CommunityVoteLocks::<T>::get(&who, poll_index).ok_or(Error::<T>::NoLocksInPlace)?;
+			Self::update_locks(&who, poll_index, &vote, LockUpdateType::Remove)
 		}
 
 		/// Dispatch a callable as the community account

diff --git a/pallets/communities/src/tests/governance.rs b/pallets/communities/src/tests/governance.rs
@@ -967,7 +967,6 @@ mod vote {
 
 				tick_blocks(2);
 
-				dbg!(System::events());
 				System::assert_has_event(pallet_referenda::Event::<Test>::ConfirmStarted { index: 3 }.into());
 			});
 		}
@@ -1144,7 +1143,7 @@ mod unlock {
 		new_test_ext().execute_with(|| {
 			// Since BOB never casted a vote, a lock wasn't put in place
 			assert_noop!(
-				Communities::unlock(RuntimeOrigin::signed(BOB), membership(COMMUNITY_C, 2), 1),
+				Communities::unlock(RuntimeOrigin::signed(BOB), 1),
 				Error::AlreadyOngoing
 			);
 		});
@@ -1169,17 +1168,9 @@ mod unlock {
 
 			tick_blocks(6);
 
-			assert_ok!(Communities::unlock(
-				RuntimeOrigin::signed(BOB),
-				membership(COMMUNITY_C, 2),
-				1
-			));
+			assert_ok!(Communities::unlock(RuntimeOrigin::signed(BOB), 1));
 
-			assert_ok!(Communities::unlock(
-				RuntimeOrigin::signed(CHARLIE),
-				membership(COMMUNITY_D, 3),
-				2
-			));
+			assert_ok!(Communities::unlock(RuntimeOrigin::signed(CHARLIE), 2));
 		});
 	}
 }