Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix benchmarks #241

Closed
wants to merge 36 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
24373bf
upgrade worked
JuaniRios Apr 5, 2024
fcc5635
nit
JuaniRios Apr 8, 2024
9c98105
Merge branch 'main' into politest-0.6.3
JuaniRios Apr 8, 2024
9a6fe1a
format auction tests, remove v0-v1 funding migration due to old types…
JuaniRios Apr 8, 2024
3377f17
save
JuaniRios Apr 8, 2024
02466b7
changes
JuaniRios Apr 8, 2024
951f67d
finish tests
JuaniRios Apr 9, 2024
700fc4c
warnings
JuaniRios Apr 9, 2024
d0e9d91
Merge branch 'main' into feature/plmc-512-comprehensive-auction-round…
JuaniRios Apr 9, 2024
9908d0c
Just's comments
JuaniRios Apr 10, 2024
da0328b
Just's comments
JuaniRios Apr 10, 2024
5508ada
Just's comments
JuaniRios Apr 10, 2024
c909248
Just's comments.
JuaniRios Apr 10, 2024
b49e49d
fixed tests by changing logic
JuaniRios Apr 10, 2024
64ea69c
min ticket now uses current bucket size
JuaniRios Apr 10, 2024
1a88265
fix warnings
JuaniRios Apr 10, 2024
00c638b
simplify test
JuaniRios Apr 11, 2024
45a5fca
more bucket assets
JuaniRios Apr 11, 2024
786d028
combine tests
JuaniRios Apr 11, 2024
d7889e5
remove unnecessary test
JuaniRios Apr 11, 2024
b3e0f97
Restructure community funding tests
JuaniRios Apr 11, 2024
db8522a
save
JuaniRios Apr 11, 2024
e091939
Add constant attribute and enhance test cases
JuaniRios Apr 11, 2024
461c359
add lower bound of 100 USD to evaluations
JuaniRios Apr 11, 2024
cba3de3
move credential checks to fail mod
JuaniRios Apr 11, 2024
c82b3cc
:fire: remove evaluation over limit bench
JuaniRios Apr 15, 2024
096df9f
:bug: fix benchmark
JuaniRios Apr 15, 2024
bd7f7ec
:children_crossing: rename extrinsic param to `ct_amount`
JuaniRios Apr 16, 2024
94cefac
:white_check_mark: fix price calculation, add bidder contributor tests
JuaniRios Apr 16, 2024
1fc98cd
:white_check_mark: insufficient funds test, failing outside community…
JuaniRios Apr 16, 2024
8458ca2
:white_check_mark: all pallet tests passing, I'm happy with the paths…
JuaniRios Apr 16, 2024
e65245c
Merge branch 'main' into feature/plmc-518-comprehensive-community-rou…
JuaniRios Apr 17, 2024
3fa45a2
:bug: fix integration tests
JuaniRios Apr 17, 2024
82418b1
Merge branch 'main' into feature/plmc-518-comprehensive-community-rou…
JuaniRios Apr 17, 2024
61865b3
:technologist: improve just file
JuaniRios Apr 17, 2024
da9f09a
:bug: fix all benches
JuaniRios Apr 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,22 @@ test-runtime-features runtime="polimec-runtime":
test-integration:
cargo test -p integration-tests

dry-run-benchmarks pallet="*":
cargo run --features runtime-benchmarks --release -p polimec-node benchmark pallet \
dry-run-benchmarks pallet="*" extrinsic="*":
cargo build --features runtime-benchmarks --release && \
./target/release/polimec-node benchmark pallet \
--chain=politest-local \
--steps=2 \
--repeat=1 \
--pallet={{ pallet }} \
--extrinsic=* \
--extrinsic={{ extrinsic }} \
--wasm-execution=compiled \
--heap-pages=4096 && \
cargo run --features runtime-benchmarks --release -p polimec-node benchmark pallet \
./target/release/polimec-node benchmark pallet \
--chain=polimec-local \
--steps=2 \
--repeat=1 \
--pallet={{ pallet }} \
--extrinsic=* \
--extrinsic={{ extrinsic }} \
--wasm-execution=compiled \
--heap-pages=4096

Expand Down
6 changes: 4 additions & 2 deletions nodes/parachain/src/chain_spec/politest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
//! Polimec Testnet chain specification

use cumulus_primitives_core::ParaId;
use frame_benchmarking::__private::traits::fungible::Inspect;
use politest_runtime::{
pallet_parachain_staking::{
inflation::{perbill_annual_to_perbill_round, BLOCKS_PER_YEAR},
Expand Down Expand Up @@ -201,8 +202,9 @@ fn testnet_genesis(
let accounts = endowed_accounts.iter().map(|(account, _)| account.clone()).collect::<Vec<_>>();

let funding_accounts = vec![
(<Runtime as pallet_funding::Config>::PalletId::get().into_account_truncating(), EXISTENTIAL_DEPOSIT),
(politest_runtime::TreasuryAccount::get(), EXISTENTIAL_DEPOSIT),
(<Runtime as pallet_funding::Config>::PalletId::get().into_account_truncating(), <Runtime as pallet_funding::Config>::NativeCurrency::minimum_balance()),
(<Runtime as pallet_funding::Config>::ProtocolGrowthTreasury::get(), <Runtime as pallet_funding::Config>::NativeCurrency::minimum_balance()),
(<Runtime as pallet_funding::Config>::ContributionTreasury::get(), <Runtime as pallet_funding::Config>::NativeCurrency::minimum_balance()),
];
endowed_accounts.append(&mut funding_accounts.clone());

Expand Down
46 changes: 6 additions & 40 deletions pallets/funding/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,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];

Expand Down Expand Up @@ -799,37 +799,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,
Expand Down Expand Up @@ -1636,6 +1605,10 @@ mod benchmarks {
let evaluation_to_settle =
inst.execute(|| Evaluations::<T>::iter_prefix_values((project_id, evaluator.clone())).next().unwrap());

let treasury_account = T::ProtocolGrowthTreasury::get();
let free_treasury_plmc = inst.get_free_plmc_balances_for(vec![treasury_account])[0].plmc_amount;
assert_eq!(free_treasury_plmc, BenchInstantiator::<T>::get_ed());

#[extrinsic_call]
settle_failed_evaluation(
RawOrigin::Signed(evaluator.clone()),
Expand Down Expand Up @@ -2459,7 +2432,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();

Expand Down Expand Up @@ -2685,13 +2658,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(|| {
Expand Down
98 changes: 35 additions & 63 deletions pallets/funding/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -929,6 +929,7 @@ impl<T: Config> Pallet<T> {
let evaluations_count = EvaluationCounts::<T>::get(project_id);

// * Validity Checks *
ensure!(usd_amount >= T::MinUsdPerEvaluation::get(), Error::<T>::EvaluationBondTooLow);
ensure!(project_details.issuer_did != did, Error::<T>::ParticipationToThemselves);
ensure!(project_details.status == ProjectStatus::EvaluationRound, Error::<T>::ProjectNotInEvaluationRound);
ensure!(evaluations_count < T::MaxEvaluationsPerProject::get(), Error::<T>::TooManyEvaluationsForProject);
Expand Down Expand Up @@ -971,33 +972,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>::EvaluationBondTooLow);
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;
Expand Down Expand Up @@ -1249,7 +1224,7 @@ impl<T: Config> Pallet<T> {
let mut project_details = ProjectsDetails::<T>::get(project_id).ok_or(Error::<T>::ProjectDetailsNotFound)?;
let did_has_winning_bid = DidWithWinningBids::<T>::get(project_id, did.clone());

ensure!(project_details.status == ProjectStatus::CommunityRound, Error::<T>::AuctionNotStarted);
ensure!(project_details.status == ProjectStatus::CommunityRound, Error::<T>::ProjectNotInCommunityRound);
ensure!(!did_has_winning_bid, Error::<T>::UserHasWinningBids);

let buyable_tokens = token_amount.min(project_details.remaining_contribution_tokens);
Expand Down Expand Up @@ -1367,23 +1342,6 @@ impl<T: Config> Pallet<T> {
contributor_ticket_size.usd_ticket_below_maximum_per_did(total_usd_bought_by_did + ticket_size),
Error::<T>::ContributionTooHigh
);
ensure!(
project_metadata.participation_currencies.contains(&funding_asset),
Error::<T>::FundingAssetNotAccepted
);
ensure!(did.clone() != project_details.issuer_did, Error::<T>::ParticipationToThemselves);
ensure!(
caller_existing_contributions.len() < T::MaxContributionsPerUser::get() as usize,
Error::<T>::TooManyContributionsForUser
);
ensure!(
contributor_ticket_size.usd_ticket_above_minimum_per_participation(ticket_size),
Error::<T>::ContributionTooLow
);
ensure!(
contributor_ticket_size.usd_ticket_below_maximum_per_did(total_usd_bought_by_did + ticket_size),
Error::<T>::ContributionTooHigh
);

let plmc_bond = Self::calculate_plmc_bond(ticket_size, multiplier, plmc_usd_price)?;
let funding_asset_amount =
Expand Down Expand Up @@ -2025,14 +1983,18 @@ impl<T: Config> Pallet<T> {
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 is_first_bucket || accepted_bids.is_empty() {
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))
};
// If the first bucket was sold out with rejected bids, the wap might be slightly lower than min_price due to rounding.
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();

Expand All @@ -2055,15 +2017,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,
bid.funding_asset_amount_locked.saturating_sub(funding_asset_amount_needed),
Preservation::Preserve,
)?;
bid.funding_asset_amount_locked = funding_asset_amount_needed;
}

let usd_bond_needed = bid
.multiplier
Expand All @@ -2075,12 +2042,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;
}
Expand Down Expand Up @@ -2195,7 +2166,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>::NotEnoughFunds)?;

Ok(())
}
Expand Down
7 changes: 4 additions & 3 deletions pallets/funding/src/instantiator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ impl<
for UserToPLMCBalance { account, plmc_amount } in correct_funds {
self.execute(|| {
let reserved = <T as Config>::NativeCurrency::balance_on_hold(&reserve_type, &account);
assert_eq!(reserved, plmc_amount, "account has unexpected reserved plmc balance");
assert_eq!(reserved, plmc_amount);
});
}
}
Expand Down Expand Up @@ -1085,7 +1085,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);
Expand Down Expand Up @@ -1160,8 +1160,9 @@ impl<
.expect("Auction Opening end point should exist");

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)
Expand Down
8 changes: 6 additions & 2 deletions pallets/funding/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)>;
Expand Down Expand Up @@ -916,13 +920,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
Expand Down
2 changes: 2 additions & 0 deletions pallets/funding/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading