Add grace period to AuthorizationExtent#516
Draft
bkontur wants to merge 5 commits into
Draft
Conversation
Introduces a renew-only grace window after `expiration`, controlled by `Config::GracePeriod`. Authorization lifecycle becomes: - now < expiration: active (store + renew) - expiration <= now < grace_until: in grace (renew only) - now >= grace_until: expired (both rejected, eligible for remove_expired_*) Highlights: - `Authorization` gains `grace_until`; helpers `Authorization::expired(now)` / `::past_grace(now)` replace the previous `Pallet::expired` / `::past_grace`. - `check_authorization` rejects store-in-grace; `check_authorization_expired` and `remove_expired_authorization` wait for `grace_until`. - v3→v4 migration tail-extends `Authorization` with `grace_until = expiration + GracePeriod`; entries already past v4 grace are dropped (and `authorization_removed` called to release provider refs). - v1→v2 migration now writes a frozen `V2Authorization` via a `From` impl, so later field additions to `Authorization` are a compile error in v2 rather than a silent shape change. - Westend / Paseo runtimes set `type GracePeriod = AuthorizationPeriod` (14 days, matching the RFC).
bkontur
requested review from
franciscoaguirre,
karolk91 and
rosarp
and removed request for
franciscoaguirre
Regenerated from the latest devnet runtime. Picks up: - new `MultiBlockMigrations` pallet (hosts `MigrateV2ToV3`) - `TransactionStorage::GracePeriod` constant and `Authorization::grace_until` field added by the grace-period work - auto-renewal storage (`AutoRenewals`, `PendingAutoRenewals`, `TransactionByContentHash`), events, errors, and the `enable_auto_renew` / `disable_auto_renew` / `apply_block_inherents` calls
rosarp
reviewed
May 11, 2026
| /// The block at which this authorization expires (start of grace). | ||
| expiration: BlockNumber, | ||
| /// The block at which the grace window ends. Always `>= expiration`. | ||
| grace_until: BlockNumber, |
Member
There was a problem hiding this comment.
grace_until may not be required.
- it adds to storage
- adds db v3->v4 migration, which can otherwise be skipped
- Compute cost of deriving it on read: one
Get::get()and onesaturating_add. The calculation is strictly cheaper than reading the extra 4 bytes from storage on every access.
Can we avoid this field? And compute it instead?
If we avoid then functionally, the PR could be reduced to:
- keep Authorization { extent, expiration } (no new field, no v3→v4 migration)
- replace auth.past_grace(now) with now >= auth.expiration.saturating_add(T::GracePeriod::get())
- drop migrations::v4 and the STORAGE_VERSION bump entirely
- keep all the lifecycle semantics in check_authorization / remove_expired_authorization
rosarp
reviewed
May 11, 2026
| pub fn account_has_active_authorization(who: &T::AccountId) -> bool { | ||
| Authorizations::<T>::get(AuthorizationScope::Account(who.clone())) | ||
| .is_some_and(|a| !Self::expired(a.expiration)) | ||
| .is_some_and(|a| !a.expired(Self::now())) |
Member
There was a problem hiding this comment.
External caller for this is hop-promotion::can_account_promote (pallets/hop-promotion/src/lib.rs:106)
This means HOP promotions stop the moment grace begins, even though the account can still renew.
Is this intentional?
Or this needs to be past_grace instead?
rosarp
reviewed
May 11, 2026
| .ok_or(Error::<T>::AuthorizationNotFound)?; | ||
| ensure!( | ||
| !Self::expired(auth.expiration) && | ||
| !auth.expired(Self::now()) && |
Member
There was a problem hiding this comment.
Same question, in grace_period, we will not allow enable_auto_renew call right?
May be this could be documented.
# Conflicts: # pallets/transaction-storage/src/lib.rs # pallets/transaction-storage/src/migrations.rs
Collaborator
Author
|
@rosarp thank you for comment, I will check later, I will postpone this PR for now and wait for slots |
34 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces a renew-only grace window after
expiration, controlled byConfig::GracePeriod. Authorization lifecycle becomes:Highlights:
Authorizationgainsgrace_until; helpersAuthorization::expired(now)/::past_grace(now)replace the previousPallet::expired/::past_grace.check_authorizationrejects store-in-grace;check_authorization_expiredandremove_expired_authorizationwait forgrace_until.Authorizationwithgrace_until = expiration + GracePeriod; entries already past v4 grace are dropped (andauthorization_removedcalled to release provider refs).V2Authorizationvia aFromimpl, so later field additions toAuthorizationare a compile error in v2 rather than a silent shape change.type GracePeriod = AuthorizationPeriod(14 days, matching the RFC).