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] add extrinsic to remove a lease #7026

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ethernomad
Copy link

@ethernomad ethernomad commented Jan 2, 2025

Description

#6929 requests more extrinsics for "managing the network's coretime allocations without needing to dabble with migration+runtime upgrade or set/kill storage patterns"

This pull request implements the remove_lease() extrinsic.

Integration

Downstream projects need to benchmark the weight for the remove_lease() extrinsic.

Review Notes

Mentorship is requested to ensure this is implemented correctly.

The lease is removed from state using the TaskId as a key. Is this sufficient. Does the extrinsic need to do anything else?

@ethernomad ethernomad requested a review from a team as a code owner January 2, 2025 09:15
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.

Misses a benchmark for remove_lease, otherwise looks good to me.

substrate/frame/broker/src/lib.rs Outdated Show resolved Hide resolved
@bkchr bkchr added the T2-pallets This PR/Issue is related to a particular pallet. label Jan 2, 2025
@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 2, 2025 10:03
Copy link
Contributor

@seadanda seadanda left a comment

Choose a reason for hiding this comment

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

Functionality wise, this looks good for this extrinsic.

If somebody wants to do more, for example remove a lease AND remove the associated task from its core in the next sale, they can pair remove_lease with remove_from_workplan (or whatever that extrinsic will be called).

@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 3, 2025 05:01
@ethernomad
Copy link
Author

bot bench substrate-pallet --pallet=pallet_broker
bot bench cumulus-coretime --pallet=pallet_broker --runtime=coretime-rococo
bot bench cumulus-coretime --pallet=pallet_broker --runtime=coretime-westend

@command-bot
Copy link

command-bot bot commented Jan 3, 2025

@ethernomad Requester could not be detected as a member of an allowed organization.

@ethernomad
Copy link
Author

Can someone run the bench bot for me?

@re-gius
Copy link
Contributor

re-gius commented Jan 3, 2025

bot bench substrate-pallet --pallet=pallet_broker
bot bench cumulus-coretime --pallet=pallet_broker --runtime=coretime-rococo
bot bench cumulus-coretime --pallet=pallet_broker --runtime=coretime-westend

@command-bot
Copy link

command-bot bot commented Jan 3, 2025

@re-gius https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966356 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --features=riscv --pallet=pallet_broker. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 27-b51ea54a-5117-4dfb-8dfc-0336d89d1cde to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jan 3, 2025

@re-gius https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966357 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=coretime-rococo --runtime_dir=coretime --target_dir=cumulus --pallet=pallet_broker. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 28-9a8dd030-225c-4c85-8e84-63823f26f1ff to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jan 3, 2025

@re-gius https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966358 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=coretime-westend --runtime_dir=coretime --target_dir=cumulus --pallet=pallet_broker. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 29-ed7e8b1e-ea32-42d1-a4a6-92e6526aa487 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jan 3, 2025

@re-gius Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --features=riscv --pallet=pallet_broker has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966356 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966356/artifacts/download.

…=coretime-rococo --runtime_dir=coretime --target_dir=cumulus --pallet=pallet_broker
@command-bot
Copy link

command-bot bot commented Jan 3, 2025

@re-gius Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=coretime-rococo --runtime_dir=coretime --target_dir=cumulus --pallet=pallet_broker has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966357 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966357/artifacts/download.

…=coretime-westend --runtime_dir=coretime --target_dir=cumulus --pallet=pallet_broker
@command-bot
Copy link

command-bot bot commented Jan 3, 2025

@re-gius Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=coretime-westend --runtime_dir=coretime --target_dir=cumulus --pallet=pallet_broker has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966358 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7966358/artifacts/download.

prdoc/pr_7026.prdoc Outdated Show resolved Hide resolved
prdoc/pr_7026.prdoc Outdated Show resolved Hide resolved
ethernomad and others added 2 commits January 6, 2025 18:55
Co-authored-by: Dónal Murray <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>
@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 6, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants