Skip to content

Commit

Permalink
⚡️ Improve phase transition points and fix instant-mode (#298)
Browse files Browse the repository at this point in the history
## What?
- Fix instant-mode benchmarks
- Improve dry-run-benches just command
- Improve phase transition points

## Why?
- The calculation on the instantiator was inaccurate because the phase transition points were 1 block off
- Just command can now run for a specific mode and runtime

## How?
- Now the start block of a state is the same block where on_initialize is called. Before it was the next block, even though the current one was still possible to call the extrinsics for that state

## Testing?
`just dry-run-benchmarks instant-mode`
  • Loading branch information
JuaniRios authored May 16, 2024
1 parent 73d796e commit 59e061b
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 172 deletions.
34 changes: 20 additions & 14 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,31 @@ test-runtime-features runtime="polimec-runtime":
test-integration:
cargo test -p integration-tests

dry-run-benchmarks runtime="politest,polimec" pallet="*" extrinsic="*" :
dry-run-benchmarks mode="fast-mode" runtime="politest,polimec" pallet="*" extrinsic="*" :
#!/bin/bash
# Set the internal field separator for splitting the runtime variable
IFS=','
# Read the runtime variable into an array
read -ra runtimes <<< "{{runtime}}"
# Build the project
cargo build --features runtime-benchmarks --release
# Loop over each runtime and run the benchmark
for runtime in "${runtimes[@]}"; do \
echo -e "\033[34mRunning benchmarks for runtime: \033[92m$runtime\033[34m\033[0m"
./target/release/polimec-node benchmark pallet \
--chain=${runtime}-local \
--steps=2 \
--repeat=1 \
--pallet={{ pallet }} \
--extrinsic={{ extrinsic }} \
--wasm-execution=compiled \
--heap-pages=4096
read -ra modes <<< "{{mode}}"

# Build the project with each mode
for mode in "${modes[@]}"; do \
echo -e "\033[34mBuilding runtime with mode: \033[92m$mode\033[34m\033[0m"
cargo build --features runtime-benchmarks,$mode --release
# Loop over each runtime and run the benchmark
for runtime in "${runtimes[@]}"; do \
echo -e "\033[34mRunning benchmarks for runtime: \033[92m$runtime\033[34m\033[0m"

./target/release/polimec-node benchmark pallet \
--chain=${runtime}-local \
--steps=2 \
--repeat=1 \
--pallet={{ pallet }} \
--extrinsic={{ extrinsic }} \
--wasm-execution=compiled \
--heap-pages=4096
done
done

# src: https://github.com/polkadot-fellows/runtimes/blob/48ccfae6141d2924f579d81e8b1877efd208693f/system-parachains/asset-hubs/asset-hub-polkadot/src/weights/cumulus_pallet_xcmp_queue.rs
Expand Down
17 changes: 3 additions & 14 deletions pallets/funding/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ mod benchmarks {

let current_block = inst.current_block();
// `do_auction_opening` fn will try to add an automatic transition 1 block after the last opening round block
let insertion_block_number: BlockNumberFor<T> = current_block + T::AuctionOpeningDuration::get() + One::one();
let insertion_block_number: BlockNumberFor<T> = current_block + T::AuctionOpeningDuration::get();

fill_projects_to_update::<T>(x, insertion_block_number);

Expand Down Expand Up @@ -2108,19 +2108,8 @@ mod benchmarks {

inst.bid_for_users(project_id, accepted_bids).unwrap();

let now = inst.current_block();
frame_system::Pallet::<T>::set_block_number(now + <T as Config>::AuctionOpeningDuration::get());
// automatic transition to opening auction
inst.advance_time(1u32.into()).unwrap();

let project_details = inst.get_project_details(project_id);
let auction_closing_block_end = project_details.phase_transition_points.auction_closing.end().unwrap();
// probably the last block will always be after random end
let random_ending: BlockNumberFor<T> = auction_closing_block_end;
frame_system::Pallet::<T>::set_block_number(random_ending);

inst.bid_for_users(project_id, rejected_bids).unwrap();

let transition_block = inst.get_update_block(project_id, &UpdateType::AuctionClosingStart).unwrap();
inst.jump_to_block(transition_block);
let auction_closing_end_block =
inst.get_project_details(project_id).phase_transition_points.auction_closing.end().unwrap();
// we don't use advance time to avoid triggering on_initialize. This benchmark should only measure the fn
Expand Down
27 changes: 14 additions & 13 deletions pallets/funding/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ impl<T: Config> Pallet<T> {
ensure!(project_metadata.policy_ipfs_cid.is_some(), Error::<T>::CidNotProvided);

// * Calculate new variables *
let evaluation_end_block = now + T::EvaluationDuration::get();
let evaluation_end_block = now.saturating_add(T::EvaluationDuration::get()).saturating_sub(One::one());
project_details.phase_transition_points.application.update(None, Some(now));
project_details.phase_transition_points.evaluation.update(Some(now + 1u32.into()), Some(evaluation_end_block));
project_details.phase_transition_points.evaluation.update(Some(now), Some(evaluation_end_block));
project_details.is_frozen = true;
project_details.status = ProjectStatus::EvaluationRound;

Expand Down Expand Up @@ -162,9 +162,10 @@ impl<T: Config> Pallet<T> {
let usd_total_amount_bonded = project_details.evaluation_round_info.total_bonded_usd;
let evaluation_target_usd = <T as Config>::EvaluationSuccessThreshold::get() * fundraising_target_usd;

let auction_initialize_period_start_block = now + 1u32.into();
let auction_initialize_period_end_block =
auction_initialize_period_start_block + T::AuctionInitializePeriodDuration::get();
let auction_initialize_period_start_block = now;
let auction_initialize_period_end_block = auction_initialize_period_start_block
.saturating_add(T::AuctionInitializePeriodDuration::get())
.saturating_sub(One::one());

// Check which logic path to follow
let is_funded = usd_total_amount_bonded >= evaluation_target_usd;
Expand Down Expand Up @@ -267,8 +268,8 @@ impl<T: Config> Pallet<T> {
ensure!(project_details.status == ProjectStatus::AuctionInitializePeriod, Error::<T>::IncorrectRound);

// * Calculate new variables *
let opening_start_block = now + 1u32.into();
let opening_end_block = now + T::AuctionOpeningDuration::get();
let opening_start_block = now;
let opening_end_block = now.saturating_add(T::AuctionOpeningDuration::get()).saturating_sub(One::one());

// * Update Storage *
project_details
Expand Down Expand Up @@ -340,8 +341,8 @@ impl<T: Config> Pallet<T> {
ensure!(project_details.status == ProjectStatus::AuctionOpening, Error::<T>::IncorrectRound);

// * Calculate new variables *
let closing_start_block = now + 1u32.into();
let closing_end_block = now + T::AuctionClosingDuration::get();
let closing_start_block = now;
let closing_end_block = now.saturating_add(T::AuctionClosingDuration::get()).saturating_sub(One::one());

// * Update Storage *
project_details
Expand Down Expand Up @@ -407,8 +408,8 @@ impl<T: Config> Pallet<T> {

// * Calculate new variables *
let end_block = Self::select_random_block(auction_closing_start_block, auction_closing_end_block);
let community_start_block = now + 1u32.into();
let community_end_block = now + T::CommunityFundingDuration::get();
let community_start_block = now;
let community_end_block = now.saturating_add(T::CommunityFundingDuration::get()).saturating_sub(One::one());
// * Update Storage *
let calculation_result = Self::calculate_weighted_average_price(
project_id,
Expand Down Expand Up @@ -495,8 +496,8 @@ impl<T: Config> Pallet<T> {
);

// * Calculate new variables *
let remainder_start_block = now + 1u32.into();
let remainder_end_block = now + T::RemainderFundingDuration::get();
let remainder_start_block = now;
let remainder_end_block = now.saturating_add(T::RemainderFundingDuration::get()).saturating_sub(One::one());

// * Update Storage *
project_details
Expand Down
18 changes: 12 additions & 6 deletions pallets/funding/src/instantiator/async_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ pub async fn async_create_evaluating_project<
let mut inst = instantiator.lock().await;

inst.start_evaluation(project_id, issuer).unwrap();
let now = inst.current_block();
project_id
}

Expand All @@ -191,7 +192,7 @@ pub async fn async_start_auction<
if project_details.status == ProjectStatus::EvaluationRound {
let update_block = inst.get_update_block(project_id, &UpdateType::EvaluationEnd).unwrap();
let notify = Arc::new(Notify::new());
block_orchestrator.add_awaiting_project(update_block + 1u32.into(), notify.clone()).await;
block_orchestrator.add_awaiting_project(update_block, notify.clone()).await;

// Wait for the notification that our desired block was reached to continue
drop(inst);
Expand Down Expand Up @@ -282,7 +283,7 @@ pub async fn async_start_community_funding<
let mut inst = instantiator.lock().await;

let update_block = inst.get_update_block(project_id, &UpdateType::AuctionClosingStart).unwrap();
let closing_start = update_block + 1u32.into();
let closing_start = update_block;

let notify = Arc::new(Notify::new());

Expand All @@ -296,7 +297,7 @@ pub async fn async_start_community_funding<

inst = instantiator.lock().await;
let update_block = inst.get_update_block(project_id, &UpdateType::CommunityFundingStart).unwrap();
let community_start = update_block + 1u32.into();
let community_start = update_block;

let notify = Arc::new(Notify::new());

Expand Down Expand Up @@ -433,7 +434,7 @@ pub async fn async_start_remainder_or_end_funding<
assert_eq!(inst.get_project_details(project_id).status, ProjectStatus::CommunityRound);

let update_block = inst.get_update_block(project_id, &UpdateType::RemainderFundingStart).unwrap();
let remainder_start = update_block + 1u32.into();
let remainder_start = update_block;

let notify = Arc::new(Notify::new());

Expand Down Expand Up @@ -550,7 +551,9 @@ pub async fn async_finish_funding<
let update_block = inst.get_update_block(project_id, &UpdateType::FundingEnd).unwrap();

let notify = Arc::new(Notify::new());
block_orchestrator.add_awaiting_project(update_block + 1u32.into(), notify.clone()).await;
block_orchestrator.add_awaiting_project(update_block, notify.clone()).await;
drop(inst);
notify.notified().await;
Ok(())
}

Expand Down Expand Up @@ -768,7 +771,9 @@ pub async fn async_create_project_at<
let time_to_community: BlockNumberFor<T> =
time_to_auction + <T as Config>::AuctionOpeningDuration::get() + <T as Config>::AuctionClosingDuration::get();
let time_to_remainder: BlockNumberFor<T> = time_to_community + <T as Config>::CommunityFundingDuration::get();
let time_to_finish: BlockNumberFor<T> = time_to_remainder + <T as Config>::RemainderFundingDuration::get();
let time_to_finish: BlockNumberFor<T> = time_to_remainder +
<T as Config>::RemainderFundingDuration::get() +
<T as Config>::SuccessToSettlementTime::get();
let mut inst = mutex_inst.lock().await;
let now = inst.current_block();
drop(inst);
Expand All @@ -786,6 +791,7 @@ pub async fn async_create_project_at<
block_orchestrator.add_awaiting_project(now + time_to_finish - time_to_evaluation, notify.clone()).await;
// Wait for the notification that our desired block was reached to continue
notify.notified().await;
let now = mutex_inst.lock().await.current_block();
async_create_evaluating_project(
mutex_inst.clone(),
test_project_params.metadata,
Expand Down
49 changes: 25 additions & 24 deletions pallets/funding/src/instantiator/chain_interactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,16 @@ impl<
})
}

pub fn jump_to_block(&mut self, block: BlockNumberFor<T>) {
let current_block = self.current_block();
if block > current_block {
self.execute(|| frame_system::Pallet::<T>::set_block_number(block - One::one()));
self.advance_time(One::one()).unwrap();
} else {
panic!("Cannot jump to a block in the present or past")
}
}

pub fn do_free_plmc_assertions(&mut self, correct_funds: Vec<UserToPLMCBalance<T>>) {
for UserToPLMCBalance { account, plmc_amount } in correct_funds {
self.execute(|| {
Expand Down Expand Up @@ -230,6 +240,10 @@ impl<
pub fn test_ct_created_for(&mut self, project_id: ProjectId) {
self.execute(|| {
let metadata = ProjectsMetadata::<T>::get(project_id).unwrap();
assert!(
<T as Config>::ContributionTokenCurrency::asset_exists(project_id),
"Asset should exist, since funding was successful"
);
assert_eq!(
<T as Config>::ContributionTokenCurrency::name(project_id),
metadata.token_information.name.to_vec()
Expand Down Expand Up @@ -430,10 +444,9 @@ impl<
let project_details = self.get_project_details(project_id);

if project_details.status == ProjectStatus::EvaluationRound {
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 + 1u32.into()).unwrap();
let now = self.current_block();
let evaluation_end_execution = self.get_update_block(project_id, &UpdateType::EvaluationEnd).unwrap();
self.advance_time(evaluation_end_execution - now).unwrap();
};

assert_eq!(self.get_project_details(project_id).status, ProjectStatus::AuctionInitializePeriod);
Expand Down Expand Up @@ -501,26 +514,14 @@ impl<
}

pub fn start_community_funding(&mut self, project_id: ProjectId) -> Result<(), DispatchError> {
let opening_end = self
.get_project_details(project_id)
.phase_transition_points
.auction_opening
.end()
.expect("Auction Opening end point should exist");

self.execute(|| frame_system::Pallet::<T>::set_block_number(opening_end));
// run on_initialize
self.advance_time(2u32.into()).unwrap();

let closing_end = self
.get_project_details(project_id)
.phase_transition_points
.auction_closing
.end()
.expect("closing end point should exist");

self.execute(|| frame_system::Pallet::<T>::set_block_number(closing_end));
// run on_initialize
if let Some(update_block) = self.get_update_block(project_id, &UpdateType::AuctionClosingStart) {
self.execute(|| frame_system::Pallet::<T>::set_block_number(update_block - One::one()));
self.advance_time(1u32.into()).unwrap();
}
let Some(update_block) = self.get_update_block(project_id, &UpdateType::CommunityFundingStart) else {
unreachable!()
};
self.execute(|| frame_system::Pallet::<T>::set_block_number(update_block - One::one()));
self.advance_time(1u32.into()).unwrap();

ensure!(
Expand Down
17 changes: 9 additions & 8 deletions pallets/funding/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,15 @@ pub const HOURS: BlockNumber = 300u64;

// REMARK: In the production configuration we use DAYS instead of HOURS.
parameter_types! {
pub const EvaluationDuration: BlockNumber = (28 * HOURS) as BlockNumber;
pub const AuctionInitializePeriodDuration: BlockNumber = (7 * HOURS) as BlockNumber;
pub const AuctionOpeningDuration: BlockNumber = (2 * HOURS) as BlockNumber;
pub const AuctionClosingDuration: BlockNumber = (3 * HOURS) as BlockNumber;
pub const CommunityRoundDuration: BlockNumber = (5 * HOURS) as BlockNumber;
pub const RemainderFundingDuration: BlockNumber = (1 * HOURS) as BlockNumber;
pub const ManualAcceptanceDuration: BlockNumber = (3 * HOURS) as BlockNumber;
pub const SuccessToSettlementTime: BlockNumber =(4 * HOURS) as BlockNumber;
pub const EvaluationDuration: BlockNumber = 3u64;
pub const AuctionInitializePeriodDuration: BlockNumber = 3u64;
pub const AuctionOpeningDuration: BlockNumber = 3u64;
pub const AuctionClosingDuration: BlockNumber = 3u64;
pub const CommunityRoundDuration: BlockNumber = 3u64;
pub const RemainderFundingDuration: BlockNumber = 3u64;
pub const ManualAcceptanceDuration: BlockNumber = 3u64;
pub const SuccessToSettlementTime: BlockNumber = 3u64;

pub const FundingPalletId: PalletId = PalletId(*b"py/cfund");
pub FeeBrackets: Vec<(Percent, Balance)> = vec![
(Percent::from_percent(10), 1_000_000 * USD_UNIT),
Expand Down
4 changes: 2 additions & 2 deletions pallets/funding/src/tests/2_evaluation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,8 @@ mod start_evaluation_extrinsic {
phase_transition_points: PhaseTransitionPoints {
application: BlockNumberPair { start: Some(1u64), end: Some(1u64) },
evaluation: BlockNumberPair {
start: Some(2u64),
end: Some(1u64 + <TestRuntime as Config>::EvaluationDuration::get()),
start: Some(1u64),
end: Some(<TestRuntime as Config>::EvaluationDuration::get()),
},
auction_initialize_period: BlockNumberPair { start: None, end: None },
auction_opening: BlockNumberPair { start: None, end: None },
Expand Down
22 changes: 13 additions & 9 deletions pallets/funding/src/tests/3_auction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,10 +722,14 @@ mod start_auction_extrinsic {
inst.mint_plmc_to(required_plmc);
inst.mint_plmc_to(ed_plmc);
inst.evaluate_for_users(project_id, evaluations).unwrap();
inst.advance_time(<TestRuntime as Config>::EvaluationDuration::get() + 1).unwrap();
assert_eq!(inst.get_project_details(project_id).status, ProjectStatus::AuctionInitializePeriod);
inst.advance_time(<TestRuntime as Config>::AuctionInitializePeriodDuration::get() + 2).unwrap();
assert_eq!(inst.get_project_details(project_id).status, ProjectStatus::AuctionOpening);

let update_block = inst.get_update_block(project_id, &UpdateType::EvaluationEnd).unwrap();
inst.execute(|| System::set_block_number(update_block - 1));
inst.advance_time(1).unwrap();

let update_block = inst.get_update_block(project_id, &UpdateType::AuctionOpeningStart).unwrap();
inst.execute(|| System::set_block_number(update_block - 1));
inst.advance_time(1).unwrap();
}

#[test]
Expand Down Expand Up @@ -809,15 +813,15 @@ mod start_auction_extrinsic {
inst.mint_plmc_to(required_plmc);
inst.mint_plmc_to(ed_plmc);
inst.evaluate_for_users(project_id, evaluations).unwrap();
inst.advance_time(<TestRuntime as Config>::EvaluationDuration::get() + 1).unwrap();
inst.advance_time(<TestRuntime as Config>::AuctionInitializePeriodDuration::get() + 2).unwrap();
assert_eq!(inst.get_project_details(project_id).status, ProjectStatus::AuctionOpening);
inst.start_auction(project_id, ISSUER_1).unwrap();

// Main test with failed evaluation
let mut inst = MockInstantiator::new(Some(RefCell::new(new_test_ext())));
let project_id = inst.create_evaluating_project(default_project_metadata(ISSUER_1), ISSUER_1);
inst.advance_time(<TestRuntime as Config>::EvaluationDuration::get() + 1).unwrap();
inst.advance_time(<TestRuntime as Config>::AuctionInitializePeriodDuration::get() + 2).unwrap();

let evaluation_end_execution = inst.get_update_block(project_id, &UpdateType::EvaluationEnd).unwrap();
inst.execute(|| System::set_block_number(evaluation_end_execution - 1));
inst.advance_time(1).unwrap();
assert_eq!(inst.get_project_details(project_id).status, ProjectStatus::FundingFailed);
}
}
Expand Down
2 changes: 1 addition & 1 deletion pallets/funding/src/tests/4_community.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ mod community_contribute_extrinsic {
inst.bid_for_users(project_id, successful_bids).unwrap();
inst.advance_time(
<TestRuntime as Config>::AuctionOpeningDuration::get() +
<TestRuntime as Config>::AuctionClosingDuration::get() +
<TestRuntime as Config>::AuctionClosingDuration::get() -
1,
)
.unwrap();
Expand Down
1 change: 1 addition & 0 deletions pallets/funding/src/tests/6_funding_end.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ fn automatic_acceptance_on_manual_decision_after_time_delta() {
let project_id = project_id;
inst.advance_time(1u64 + <TestRuntime as Config>::ManualAcceptanceDuration::get()).unwrap();
assert_eq!(inst.get_project_details(project_id).status, ProjectStatus::FundingSuccessful);
dbg!(inst.get_project_details(project_id));
inst.advance_time(<TestRuntime as Config>::SuccessToSettlementTime::get()).unwrap();

inst.test_ct_created_for(project_id);
Expand Down
Loading

0 comments on commit 59e061b

Please sign in to comment.