✅ Comprehensive community round tests (518) (#229)

* upgrade worked

* nit

* format auction tests, remove v0-v1 funding migration due to old types reliance

* save

* changes

* finish tests

* warnings

* Just's comments

* Just's comments

* Just's comments

* Just's comments.
Test a bid split going over the limits fails. Doesn't fail now, needs fixing

* fixed tests by changing logic

* min ticket now uses current bucket size

* fix warnings

* simplify test

* more bucket assets

* combine tests

* remove unnecessary test

* Restructure community funding tests

Add test for evaluation of 0

* save

* Add constant attribute and enhance test cases

* add lower bound of 100 USD to evaluations

* move credential checks to fail mod

* 🔥 remove evaluation over limit bench

* 🐛 fix benchmark

* 🚸 rename extrinsic param to `ct_amount`

* ✅ fix price calculation, add bidder contributor tests

* ✅ insufficient funds test, failing outside community round test

* ✅ all pallet tests passing, I'm happy with the paths tested. Integration tests need fixing

* 🐛 fix integration tests

* feedback

* feedback

* save

* feedback

* feedback

* last feedback addressing

* merge

* fmt + wap calculation change

* fix warnings

* remove test and split another

* fix warnings, ignore test

integration-tests/src/tests/build_spec.rs

@@ -13,6 +13,7 @@
 
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
+#[ignore]
 #[test]
 fn build_spec_testing_node() {
 	// run the polimec-node compiled with "std" with the build-spec command and --raw flag

pallets/funding/src/benchmarking.rs

@@ -675,7 +675,7 @@ mod benchmarks {
 		let project_metadata = default_project::<T>(inst.get_new_nonce(), issuer.clone());
 		let test_project_id = inst.create_evaluating_project(project_metadata, issuer);
 
-		let existing_evaluation = UserToUSDBalance::new(test_evaluator.clone(), (100 * US_DOLLAR).into());
+		let existing_evaluation = UserToUSDBalance::new(test_evaluator.clone(), (200 * US_DOLLAR).into());
 		let extrinsic_evaluation = UserToUSDBalance::new(test_evaluator.clone(), (1_000 * US_DOLLAR).into());
 		let existing_evaluations = vec![existing_evaluation; x as usize];
 
@@ -792,37 +792,6 @@ mod benchmarks {
 		);
 	}
 
-	// - We know how many iterations it does in storage
-	// - We know that it requires to unbond the lowest evaluation
-	#[benchmark]
-	fn evaluation_over_limit() {
-		// How many other evaluations the user did for that same project
-		let x = <T as Config>::MaxEvaluationsPerUser::get();
-		let (inst, project_id, extrinsic_evaluation, extrinsic_plmc_bonded, total_expected_plmc_bonded) =
-			evaluation_setup::<T>(x);
-
-		let jwt = get_mock_jwt(
-			extrinsic_evaluation.account.clone(),
-			InvestorType::Institutional,
-			generate_did_from_account(extrinsic_evaluation.account.clone()),
-		);
-		#[extrinsic_call]
-		evaluate(
-			RawOrigin::Signed(extrinsic_evaluation.account.clone()),
-			jwt,
-			project_id,
-			extrinsic_evaluation.usd_amount,
-		);
-
-		evaluation_verification::<T>(
-			inst,
-			project_id,
-			extrinsic_evaluation,
-			extrinsic_plmc_bonded,
-			total_expected_plmc_bonded,
-		);
-	}
-
 	fn bid_setup<T>(
 		existing_bids_count: u32,
 		do_perform_bid_calls: u32,
@@ -2456,7 +2425,7 @@ mod benchmarks {
 
 		let mut evaluations = (0..y.saturating_sub(1))
 			.map(|i| {
-				UserToUSDBalance::<T>::new(account::<AccountIdOf<T>>("evaluator", 0, i), (10u128 * ASSET_UNIT).into())
+				UserToUSDBalance::<T>::new(account::<AccountIdOf<T>>("evaluator", 0, i), (100u128 * US_DOLLAR).into())
 			})
 			.collect_vec();
 
@@ -2682,13 +2651,6 @@ mod benchmarks {
 			});
 		}
 
-		#[test]
-		fn bench_evaluation_over_limit() {
-			new_test_ext().execute_with(|| {
-				assert_ok!(PalletFunding::<TestRuntime>::test_evaluation_over_limit());
-			});
-		}
-
 		#[test]
 		fn bench_start_auction_manually() {
 			new_test_ext().execute_with(|| {

pallets/funding/src/functions.rs

@@ -25,7 +25,7 @@ use frame_support::{
 	pallet_prelude::*,
 	traits::{
 		fungible::{Mutate, MutateHold as FungibleMutateHold},
-		fungibles::{metadata::Mutate as MetadataMutate, Create, Mutate as FungiblesMutate},
+		fungibles::{metadata::Mutate as MetadataMutate, Create, Inspect, Mutate as FungiblesMutate},
 		tokens::{Precision, Preservation},
 		Get,
 	},
@@ -979,6 +979,10 @@ impl<T: Config> Pallet<T> {
 		let evaluations_count = EvaluationCounts::<T>::get(project_id);
 
 		// * Validity Checks *
+		ensure!(
+			usd_amount >= T::MinUsdPerEvaluation::get(),
+			Error::<T>::ParticipationFailed(ParticipationError::TooLow)
+		);
 		ensure!(
 			project_details.issuer_did != did,
 			Error::<T>::IssuerError(IssuerErrorReason::ParticipationToOwnProject)
@@ -1030,36 +1034,7 @@ impl<T: Config> Pallet<T> {
 			when: now,
 		};
 
-		if caller_existing_evaluations.len() < T::MaxEvaluationsPerUser::get() as usize {
-			T::NativeCurrency::hold(&HoldReason::Evaluation(project_id).into(), evaluator, plmc_bond)?;
-		} else {
-			let (low_id, lowest_evaluation) = caller_existing_evaluations
-				.iter()
-				.min_by_key(|(_, evaluation)| evaluation.original_plmc_bond)
-				.ok_or(Error::<T>::ImpossibleState)?;
-
-			ensure!(
-				lowest_evaluation.original_plmc_bond < plmc_bond,
-				Error::<T>::ParticipationFailed(ParticipationError::TooLow)
-			);
-			ensure!(
-				lowest_evaluation.original_plmc_bond == lowest_evaluation.current_plmc_bond,
-				"Using evaluation funds for participating should not be possible in the evaluation round"
-			);
-
-			T::NativeCurrency::release(
-				&HoldReason::Evaluation(project_id).into(),
-				&lowest_evaluation.evaluator,
-				lowest_evaluation.original_plmc_bond,
-				Precision::Exact,
-			)?;
-
-			T::NativeCurrency::hold(&HoldReason::Evaluation(project_id).into(), evaluator, plmc_bond)?;
-
-			Evaluations::<T>::remove((project_id, evaluator, low_id));
-			EvaluationCounts::<T>::mutate(project_id, |c| *c -= 1);
-		}
-
+		T::NativeCurrency::hold(&HoldReason::Evaluation(project_id).into(), evaluator, plmc_bond)?;
 		Evaluations::<T>::insert((project_id, evaluator, evaluation_id), new_evaluation);
 		NextEvaluationId::<T>::set(evaluation_id.saturating_add(One::one()));
 		evaluation_round_info.total_bonded_usd += usd_amount;
@@ -1464,26 +1439,6 @@ impl<T: Config> Pallet<T> {
 			contributor_ticket_size.usd_ticket_below_maximum_per_did(total_usd_bought_by_did + ticket_size),
 			Error::<T>::ParticipationFailed(ParticipationError::TooHigh)
 		);
-		ensure!(
-			project_metadata.participation_currencies.contains(&funding_asset),
-			Error::<T>::ParticipationFailed(ParticipationError::FundingAssetNotAccepted)
-		);
-		ensure!(
-			did.clone() != project_details.issuer_did,
-			Error::<T>::IssuerError(IssuerErrorReason::ParticipationToOwnProject)
-		);
-		ensure!(
-			caller_existing_contributions.len() < T::MaxContributionsPerUser::get() as usize,
-			Error::<T>::ParticipationFailed(ParticipationError::TooManyUserParticipations)
-		);
-		ensure!(
-			contributor_ticket_size.usd_ticket_above_minimum_per_participation(ticket_size),
-			Error::<T>::ParticipationFailed(ParticipationError::TooLow)
-		);
-		ensure!(
-			contributor_ticket_size.usd_ticket_below_maximum_per_did(total_usd_bought_by_did + ticket_size),
-			Error::<T>::ParticipationFailed(ParticipationError::TooHigh)
-		);
 
 		let plmc_bond = Self::calculate_plmc_bond(ticket_size, multiplier, plmc_usd_price)?;
 		let funding_asset_amount =
@@ -2074,6 +2029,9 @@ impl<T: Config> Pallet<T> {
 		let mut bid_usd_value_sum = BalanceOf::<T>::zero();
 		let project_account = Self::fund_account_id(project_id);
 		let plmc_price = T::PriceProvider::get_price(PLMC_FOREIGN_ID).ok_or(Error::<T>::PriceNotFound)?;
+		let project_metadata = ProjectsMetadata::<T>::get(project_id)
+			.ok_or(Error::<T>::ProjectError(ProjectErrorReason::ProjectMetadataNotFound))?;
+		let mut highest_accepted_price = project_metadata.minimum_price;
 
 		// sort bids by price, and equal prices sorted by id
 		bids.sort_by(|a, b| b.cmp(a));
@@ -2097,6 +2055,7 @@ impl<T: Config> Pallet<T> {
 					DidWithWinningBids::<T>::mutate(project_id, bid.did.clone(), |flag| {
 						*flag = true;
 					});
+					highest_accepted_price = highest_accepted_price.max(bid.original_ct_usd_price);
 				} else {
 					let ticket_size = bid.original_ct_usd_price.saturating_mul_int(buyable_amount);
 					bid_usd_value_sum.saturating_accrue(ticket_size);
@@ -2106,6 +2065,7 @@ impl<T: Config> Pallet<T> {
 						*flag = true;
 					});
 					bid.final_ct_amount = buyable_amount;
+					highest_accepted_price = highest_accepted_price.max(bid.original_ct_usd_price);
 				}
 				bid
 			})
@@ -2140,26 +2100,25 @@ impl<T: Config> Pallet<T> {
 
 		// lastly, sum all the weighted prices to get the final weighted price for the next funding round
 		// 3 + 10.6 + 2.6 = 16.333...
-		let current_bucket =
-			Buckets::<T>::get(project_id).ok_or(Error::<T>::ProjectError(ProjectErrorReason::BucketNotFound))?;
-		let project_metadata = ProjectsMetadata::<T>::get(project_id)
-			.ok_or(Error::<T>::ProjectError(ProjectErrorReason::ProjectMetadataNotFound))?;
-		let is_first_bucket = current_bucket.current_price == project_metadata.minimum_price;
-
 		let calc_weighted_price_fn = |bid: &BidInfoOf<T>| -> PriceOf<T> {
 			let ticket_size = bid.original_ct_usd_price.saturating_mul_int(bid.final_ct_amount);
 			let bid_weight = <T::Price as FixedPointNumber>::saturating_from_rational(ticket_size, bid_usd_value_sum);
 			let weighted_price = bid.original_ct_usd_price.saturating_mul(bid_weight);
 			weighted_price
 		};
-		let weighted_token_price = if is_first_bucket || accepted_bids.is_empty() {
+		let mut weighted_token_price = if highest_accepted_price == project_metadata.minimum_price {
 			project_metadata.minimum_price
 		} else {
 			accepted_bids
 				.iter()
 				.map(calc_weighted_price_fn)
 				.fold(Zero::zero(), |a: T::Price, b: T::Price| a.saturating_add(b))
 		};
+		// We are 99% sure that the price cannot be less than minimum if some accepted bids have higher price, but rounding
+		// errors are strange, so we keep this just in case.
+		if weighted_token_price < project_metadata.minimum_price {
+			weighted_token_price = project_metadata.minimum_price;
+		}
 
 		let mut final_total_funding_reached_by_bids = BalanceOf::<T>::zero();
 
@@ -2182,15 +2141,20 @@ impl<T: Config> Pallet<T> {
 					.checked_mul_int(new_ticket_size)
 					.ok_or(Error::<T>::BadMath)?;
 
-				T::FundingCurrency::transfer(
-					bid.funding_asset.to_assethub_id(),
-					&project_account,
-					&bid.bidder,
-					bid.funding_asset_amount_locked.saturating_sub(funding_asset_amount_needed),
-					Preservation::Preserve,
-				)?;
-
-				bid.funding_asset_amount_locked = funding_asset_amount_needed;
+				let amount_returned = bid.funding_asset_amount_locked.saturating_sub(funding_asset_amount_needed);
+				let asset_id = bid.funding_asset.to_assethub_id();
+				let min_amount = T::FundingCurrency::minimum_balance(asset_id);
+				// Transfers of less than min_amount return an error
+				if amount_returned > min_amount {
+					T::FundingCurrency::transfer(
+						bid.funding_asset.to_assethub_id(),
+						&project_account,
+						&bid.bidder,
+						amount_returned,
+						Preservation::Preserve,
+					)?;
+					bid.funding_asset_amount_locked = funding_asset_amount_needed;
+				}
 
 				let usd_bond_needed = bid
 					.multiplier
@@ -2202,12 +2166,16 @@ impl<T: Config> Pallet<T> {
 					.checked_mul_int(usd_bond_needed)
 					.ok_or(Error::<T>::BadMath)?;
 
-				T::NativeCurrency::release(
-					&HoldReason::Participation(project_id).into(),
-					&bid.bidder,
-					bid.plmc_bond.saturating_sub(plmc_bond_needed),
-					Precision::Exact,
-				)?;
+				let plmc_bond_returned = bid.plmc_bond.saturating_sub(plmc_bond_needed);
+				// If the free balance of a user is zero and we want to send him less than ED, it will fail.
+				if plmc_bond_returned > T::ExistentialDeposit::get() {
+					T::NativeCurrency::release(
+						&HoldReason::Participation(project_id).into(),
+						&bid.bidder,
+						plmc_bond_returned,
+						Precision::Exact,
+					)?;
+				}
 
 				bid.plmc_bond = plmc_bond_needed;
 			}
@@ -2322,7 +2290,8 @@ impl<T: Config> Pallet<T> {
 			to_convert = to_convert.saturating_sub(converted)
 		}
 
-		T::NativeCurrency::hold(&HoldReason::Participation(project_id).into(), who, to_convert)?;
+		T::NativeCurrency::hold(&HoldReason::Participation(project_id).into(), who, to_convert)
+			.map_err(|_| Error::<T>::ParticipationFailed(ParticipationError::NotEnoughFunds))?;
 
 		Ok(())
 	}

pallets/funding/src/instantiator/chain_interactions.rs

@@ -431,7 +431,7 @@ impl<
 			let evaluation_end = project_details.phase_transition_points.evaluation.end().unwrap();
 			let auction_start = evaluation_end.saturating_add(2u32.into());
 			let blocks_to_start = auction_start.saturating_sub(self.current_block());
-			self.advance_time(blocks_to_start).unwrap();
+			self.advance_time(blocks_to_start + 1u32.into()).unwrap();
 		};
 
 		assert_eq!(self.get_project_details(project_id).status, ProjectStatus::AuctionInitializePeriod);
@@ -507,7 +507,7 @@ impl<
 
 		self.execute(|| frame_system::Pallet::<T>::set_block_number(opening_end));
 		// run on_initialize
-		self.advance_time(1u32.into()).unwrap();
+		self.advance_time(2u32.into()).unwrap();
 
 		let closing_end = self
 			.get_project_details(project_id)

pallets/funding/src/instantiator/macros.rs

@@ -1,5 +1,3 @@
-use super::*;
-
 #[macro_export]
 /// Example:
 /// ```

pallets/funding/src/instantiator/mod.rs

@@ -53,7 +53,6 @@ use sp_std::{
 };
 
 pub mod macros;
-pub use macros::*;
 pub mod types;
 pub use types::*;
 pub mod traits;

pallets/funding/src/lib.rs

@@ -324,8 +324,12 @@ pub mod pallet {
 		type MaxEvaluationsPerProject: Get<u32>;
 
 		/// How many distinct evaluations per user per project
+		#[pallet::constant]
 		type MaxEvaluationsPerUser: Get<u32>;
 
+		#[pallet::constant]
+		type MinUsdPerEvaluation: Get<BalanceOf<Self>>;
+
 		/// Range of max_message_size values for the hrmp config where we accept the incoming channel request
 		#[pallet::constant]
 		type MaxMessageSizeThresholds: Get<(u32, u32)>;
@@ -822,13 +826,13 @@ pub mod pallet {
 			origin: OriginFor<T>,
 			jwt: UntrustedToken,
 			project_id: ProjectId,
-			#[pallet::compact] amount: BalanceOf<T>,
+			#[pallet::compact] ct_amount: BalanceOf<T>,
 			multiplier: T::Multiplier,
 			asset: AcceptedFundingAsset,
 		) -> DispatchResultWithPostInfo {
 			let (account, did, investor_type) =
 				T::InvestorOrigin::ensure_origin(origin, &jwt, T::VerifierPublicKey::get())?;
-			Self::do_bid(&account, project_id, amount, multiplier, asset, did, investor_type)
+			Self::do_bid(&account, project_id, ct_amount, multiplier, asset, did, investor_type)
 		}
 
 		/// Buy tokens in the Community or Remainder round at the price set in the Auction Round

pallets/funding/src/mock.rs

@@ -355,6 +355,7 @@ parameter_types! {
 		32, 118, 30, 171, 58, 212, 197, 27, 146, 122, 255, 243, 34, 245, 90, 244, 221, 37, 253,
 		195, 18, 202, 111, 55, 39, 48, 123, 17, 101, 78, 215, 94,
 	];
+	pub MinUsdPerEvaluation: Balance = 100 * US_DOLLAR;
 }
 
 pub struct DummyConverter;
@@ -426,6 +427,7 @@ impl Config for TestRuntime {
 	type MaxMessageSizeThresholds = MaxMessageSizeThresholds;
 	type MaxProjectsToUpdateInsertionAttempts = ConstU32<100>;
 	type MaxProjectsToUpdatePerBlock = ConstU32<1>;
+	type MinUsdPerEvaluation = MinUsdPerEvaluation;
 	type Multiplier = Multiplier;
 	type NativeCurrency = Balances;
 	type PalletId = FundingPalletId;