Allow an Offset to Lease Periods #3980

shawntabrizi · 2021-10-01T00:58:41Z

Supersedes: #3960

This PR allows the entire crowdloan, slots, auction system to be delayed by a constant block number to do fine adjustments to when the lease period cycle should start. We do this by extending lease period 0 by LeaseOffset blocks, which offsets all lease periods by the same amount.

A few core level API changes have been introduced which ensures that no existing APIs break due to this new feature. By doing this, we ensure there is a single point of calculation of the current lease period, and every other API just reads from it. So other APIs should not be exposed to any concept of the lease period offset, and the underlying behavior of calculating the current lease period is completely opaque.

fn lease_period() was renamed to fn lease_period_length(), returns the offset in addition to the length, and is removed from the public api. We only use it in benchmarking, so we make sure to hide it behind a feature flag so that no external logic would use it unexpectedly.
fn lease_period_index() use to always return the CURRENT lease period for the current block number. We updated the API so that it always accepts an input of the block number we want to query, and it will return both the current lease period for that block, and if that was the first block of a new lease period.
In order to support the traits which span the different pallets, we had to expose BlockNumber as a generic argument, slightly changing the signature of the Leaser and Auctioneer traits. No big deal.
Docs updated for these changed traits.
A new test was added to verify fn lease_period_index() behaves correct with and without an offset.
An end-to-end integration test was updated to verify everything behaves as expected with a bunch of different offset values.

shawntabrizi · 2021-10-01T02:01:08Z

runtime/common/src/slots.rs

+	#[cfg(any(feature = "runtime-benchmarks", test))]
+	fn lease_period_length() -> (T::BlockNumber, T::BlockNumber) {
+		(T::LeasePeriod::get(), T::LeaseOffset::get())


This API is only needed for benchmarks, so we hide it from the public API, and therefore eliminate any chance that a user may try to calculate the current of future lease period index on their own.

shawntabrizi · 2021-10-01T02:03:02Z

runtime/common/src/slots.rs

+		let now = frame_system::Pallet::<T>::block_number();
+		let current_lease_period = Self::lease_period_index(now).0;


In most places where we just called lease_period_index before, we now need to pass the current block number. This is the same logic and complexity as before, just explicit now.

runtime/common/src/slots.rs

shawntabrizi · 2021-10-01T02:32:28Z

runtime/common/src/integration_tests.rs

-
-		Balances::make_free_balance_be(&10, 1_000_000_000);
-		Balances::make_free_balance_be(&20, 1_000_000_000);
+	for offset in [0u32, 50, 100, 200].iter() {


This test is basically unchanged except introducing offset into the timeline of the setup.

shawntabrizi · 2021-10-01T03:00:35Z

runtime/common/src/slots.rs

+	fn lease_period_index(b: T::BlockNumber) -> (Self::LeasePeriod, bool) {
+		// Note that lease lease period 0 is artificially extended by the lease offset.
+		let offset_block_now = b.saturating_sub(T::LeaseOffset::get());
+		let lease_period = offset_block_now / T::LeasePeriod::get();
+
+		// Special logic to handle lease period 0 being extended by the offset.
+		let first_block = if lease_period.is_zero() {
+			// Lease period 0 has their first block on only block 0.
+			b.is_zero()
+		} else {
+			(offset_block_now % T::LeasePeriod::get()).is_zero()
+		};
+
+		(lease_period, first_block)


Here is the core change. This is the one source of truth for calculating the current lease period index given some offset. All other APIs must use this to do stuff.

crystalin · 2021-10-01T12:25:09Z

@shawntabrizi , isn't this feature going to bring even more complexity to the parachain lease computations? I still have today to explain people why Moonriver is not ending 48 weeks after it started :p

I'm curious, what is the use case for this change ?

rphmeier · 2021-10-02T17:42:23Z

@crystalin It's very useful for a relay chain's governance to decide the exact date at which parachains go live instead of waiting for the next lease period.

runtime/common/src/auctions.rs

runtime/common/src/slots.rs

emostov

Code LGTM

w.r.t. Gav's suggestion I think it could be nice for a relay chain to have fine grained control over the start of lease period 0. But given that the target use case is already past that point I don't see a compelling reason to add the little bit of complexity in handling an Option from lease_period_index.

shawntabrizi · 2021-10-05T05:13:33Z

@rphmeier take a look at the last changes made and if it is good for you, feel free to merge.

* master: (138 commits) Allow an Offset to Lease Periods (#3980) Bump quote from 1.0.9 to 1.0.10 (#4013) Companion for #9834 (Transaction Priority) (#3901) chore: update `builder` image (#3884) Free disputed cores before processing bitfields (#4008) Make candidate validation timeouts configurable (#4001) Add extrinsic ordering filtering (#3631) chore: ci list files that spellcheck finds (#3992) Use background tasks properly in candidate-validation (#4002) Fix unoccupied bitfields (#4004) Bump syn from 1.0.77 to 1.0.78 (#4006) Bump jsonrpsee-ws-client from 0.3.0 to 0.3.1 (#3931) fix clock drift for assignments issued before the block (#3851) Remove unoccupied bit check (#3999) bump substrate (#4000) change genesis authority set for wococo-local, revert rococo-local (#3998) ignore irrelevant approvals in logs (#3859) avoid expect, on free availability core (#3994) preserve finalized block in active leaves (#3997) some tweaks to rococo-local (#3996) ...

* add slot offset for slots * trying things out * fix test * improve api to return the first block of a new lease period * add an integration test with offset * de-duplicate test * hide lease period_period_length from public api * fix benchmarks * Update runtime/common/src/slots.rs * support the exact same range of crowdloans * fix docs * fix docs again * introduce offset to runtimes * fix and check edge case w/ offset and lease period first block * remove newline * turn into an option * fix benchmarks Co-authored-by: Robert Habermeier <rphmeier@gmail.com>

* v0.9.11 weights * Allow an Offset to Lease Periods (#3980) * add slot offset for slots * trying things out * fix test * improve api to return the first block of a new lease period * add an integration test with offset * de-duplicate test * hide lease period_period_length from public api * fix benchmarks * Update runtime/common/src/slots.rs * support the exact same range of crowdloans * fix docs * fix docs again * introduce offset to runtimes * fix and check edge case w/ offset and lease period first block * remove newline * turn into an option * fix benchmarks Co-authored-by: Robert Habermeier <rphmeier@gmail.com> * Polkadot: Remove configuration's runtime migration and just set StorageVersion (#4035) * wip * Remove unused members Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> Co-authored-by: Robert Habermeier <rphmeier@gmail.com> Co-authored-by: Sergei Shulepov <sergei@parity.io>

* Permanent & Temp parachain slots on Rococo - WIP * Revert test change * Revert test change * Fix formatting * Extract logic to separate assigned_slots pallet * Formatting * Parachain downgrade logic * Pallet doc comment * Revert unnecessary changes * Fix few issues, tweak temp slots allocation logic; add a bunch of tests * Address review comments; track active temp slots * Update runtime/common/src/assigned_slots.rs * Update runtime/common/src/assigned_slots.rs * Remove assigned_slots calls from paras_sudo_wrapper * Unassign is a perfectly valid verb * Remove unneeded collect * Update code following #3980 * Cleanup * Generate storage info for pallet * Address review comments * Add ForceOrigin to slots pallet * Track permanent slot duration in storage * Fix tests build Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

rphmeier and others added 8 commits September 29, 2021 11:43

add slot offset for slots

9deac1c

trying things out

3bc0246

fix test

3471467

improve api to return the first block of a new lease period

d51e144

add an integration test with offset

45eb1e4

de-duplicate test

dd17d21

hide lease period_period_length from public api

91a7191

fix benchmarks

89da8ee

shawntabrizi commented Oct 1, 2021

View reviewed changes

runtime/common/src/slots.rs Outdated Show resolved Hide resolved

shawntabrizi added 4 commits September 30, 2021 22:03

Update runtime/common/src/slots.rs

04a6065

support the exact same range of crowdloans

57a1e55

fix docs

49f4397

fix docs again

f71f847

shawntabrizi commented Oct 1, 2021

View reviewed changes

shawntabrizi requested a review from rphmeier October 1, 2021 02:34

shawntabrizi added 2 commits September 30, 2021 22:47

introduce offset to runtimes

52a545f

fix and check edge case w/ offset and lease period first block

11e5184

shawntabrizi added B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. A0-please_review Pull request needs code review. labels Oct 1, 2021

remove newline

e38c91a

shawntabrizi commented Oct 1, 2021

View reviewed changes

rphmeier approved these changes Oct 2, 2021

View reviewed changes

gavofyork reviewed Oct 4, 2021

View reviewed changes

runtime/common/src/auctions.rs Outdated Show resolved Hide resolved

gavofyork reviewed Oct 4, 2021

View reviewed changes

runtime/common/src/slots.rs Outdated Show resolved Hide resolved

emostov approved these changes Oct 5, 2021

View reviewed changes

shawntabrizi added 3 commits October 4, 2021 23:11

turn into an option

3668966

fix benchmarks

55e7fde

Merge branch 'master' into shawntabrizi-offset-generic

d893f86

rphmeier merged commit 2a3fca3 into master Oct 5, 2021

rphmeier deleted the shawntabrizi-offset-generic branch October 5, 2021 14:07

chevdor added this to the v0.9.11 milestone Oct 7, 2021

s3krit mentioned this pull request Oct 7, 2021

v0.9.11-rc4 #4027

Merged

redzsina added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Oct 7, 2021

stiiifff pushed a commit that referenced this pull request Oct 11, 2021

Update code following #3980

2dd00e0

emostov mentioned this pull request Oct 27, 2021

fix(ParasService): adjust endpoint to use historicApi, fix endingOffset bug paritytech/substrate-api-sidecar#735

Merged

ghzlatarev mentioned this pull request Nov 2, 2021

[Manta-PC] Update dependencies to v0.9.12 Manta-Network/Manta#242

Merged

19 tasks

github-actions bot mentioned this pull request Nov 2, 2021

Update substrate/polkadot/cumulus from v0.9.11 to v0.9.12 moonbeam-foundation/moonbeam#951

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow an Offset to Lease Periods #3980

Allow an Offset to Lease Periods #3980

shawntabrizi commented Oct 1, 2021 •

edited

Loading

shawntabrizi Oct 1, 2021 •

edited

Loading

shawntabrizi Oct 1, 2021

shawntabrizi Oct 1, 2021

shawntabrizi Oct 1, 2021

crystalin commented Oct 1, 2021

rphmeier commented Oct 2, 2021 •

edited

Loading

emostov left a comment

shawntabrizi commented Oct 5, 2021

		let now = frame_system::Pallet::<T>::block_number();
		let current_lease_period = Self::lease_period_index(now).0;

Allow an Offset to Lease Periods #3980

Allow an Offset to Lease Periods #3980

Conversation

shawntabrizi commented Oct 1, 2021 • edited Loading

shawntabrizi Oct 1, 2021 • edited Loading

Choose a reason for hiding this comment

shawntabrizi Oct 1, 2021

Choose a reason for hiding this comment

shawntabrizi Oct 1, 2021

Choose a reason for hiding this comment

shawntabrizi Oct 1, 2021

Choose a reason for hiding this comment

crystalin commented Oct 1, 2021

rphmeier commented Oct 2, 2021 • edited Loading

emostov left a comment

Choose a reason for hiding this comment

shawntabrizi commented Oct 5, 2021

shawntabrizi commented Oct 1, 2021 •

edited

Loading

shawntabrizi Oct 1, 2021 •

edited

Loading

rphmeier commented Oct 2, 2021 •

edited

Loading