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

[pallet_broker] Remove leases that have already expired in rotate_sale #3213

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

seadanda
Copy link
Contributor

@seadanda seadanda commented Feb 5, 2024

Leases can be force set, but since Leases is a StorageValue, if a lease misses its sale rotation in which it should expire, it can never be cleared.

This can happen if a lease is added with an until timeslice that lies in a region whose sale has already started or has passed, even if the timeslice itself hasn't passed.

This solves that issue in a minimal way, with all expired leases being cleaned up in each sale rotation, not just the ones that are expiring in the coming region.

TODO:

  • Write test

@seadanda seadanda added the T2-pallets This PR/Issue is related to a particular pallet. label Feb 5, 2024
@seadanda seadanda self-assigned this Feb 5, 2024
@seadanda seadanda requested a review from a team as a code owner February 5, 2024 13:19
@seadanda seadanda added the R0-silent Changes should not be mentioned in any release notes label Feb 5, 2024
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test before merging.

@seadanda seadanda added this pull request to the merge queue Feb 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 8, 2024
@seadanda seadanda added this pull request to the merge queue Feb 8, 2024
Merged via the queue into master with commit 2ea6bcf Feb 8, 2024
123 of 125 checks passed
@seadanda seadanda deleted the donal-clean-expired-leases branch February 8, 2024 13:08
github-merge-queue bot pushed a commit that referenced this pull request Feb 12, 2024
Leases can be force set, but since Leases is a StorageValue, if a lease
misses its sale rotation in which it should expire, it can never be
cleared.

This can happen if a lease is added with an until timeslice that lies in
a region whose sale has already started or has passed, even if the
timeslice itself hasn't passed.

Trappist is currently trapped in a lease that will never end, so this
will remove it at the next sale rotation.

A fix was introduced in
#3213 but this missed the
1.7 release. This PR bumps the `coretime-rococo` version to get these
changes on Rococo.
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
paritytech#3213)

Leases can be force set, but since `Leases` is a `StorageValue`, if a
lease misses its sale rotation in which it should expire, it can never
be cleared.

This can happen if a lease is added with an `until` timeslice that lies
in a region whose sale has already started or has passed, even if the
timeslice itself hasn't passed.

This solves that issue in a minimal way, with all expired leases being
cleaned up in each sale rotation, not just the ones that are expiring in
the coming region.

TODO:
- [x] Write test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants