removed `without_storage_info` from `pallet-collective` #1445

muraca · 2023-09-07T10:09:06Z

Closes #167

Voting is now bounded
Members is now bounded
Proposal and ProposalOf have been removed
Voting is now a CountedStorageMap to handle the MaxProposals bound
Preimages has been introduced as a dependency to store proposals, using traits QueryPreimage and StorePreimage

Voting might eventually be converted into a BoundedMap theorized in #141

Polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

muraca · 2023-09-09T12:47:44Z

@ggwpez @juangirini this is ready for review, and needs new weights

cumulus/parachains/runtimes/collectives/collectives-polkadot/src/impls.rs

substrate/frame/collective/src/migrations/v5.rs

liamaharon

Looking good!

On the migration side, this needs:

An implementation of OnRuntimeUpgrade including pre/post_upgrade hooks to ensure successful migration
Unit tests for the OnRuntimeUpgrade struct
The OnRuntimeUpgrade added to the Executive pallet Migrations tuple for all runtimes that use the pallet

Let me know if you have any questions

liamaharon · 2023-09-18T13:14:59Z

bot bench $ pallet dev pallet-collective

command-bot · 2023-09-18T13:15:02Z

@liamaharon Positional arguments are not supported anymore. I guess you meant bot bench substrate-pallet --pallet=pallet-collective, but I could be wrong.
Read docs to find out how to run your command.

liamaharon · 2023-09-18T13:29:40Z

bot bench substrate-pallet --pallet=pallet-collective

command-bot · 2023-09-18T13:29:42Z

@liamaharon option '--pallet ' argument 'pallet-collective' is invalid. argument pallet is not matching rule ^([a-z_]+)([:]{2}[a-z_]+)?$

liamaharon · 2023-09-18T13:30:54Z

bot bench substrate-pallet --pallet=pallet_collective

Signed-off-by: muraca <[email protected]>

…=dev --target_dir=substrate --pallet=pallet_collective

Signed-off-by: muraca <[email protected]>

…=dev --target_dir=substrate --pallet=pallet_collective

Signed-off-by: muraca <[email protected]>

Signed-off-by: Matteo Muraca <[email protected]>

muraca · 2024-03-07T09:05:21Z

this needs another benchmark please

bkchr · 2024-03-11T21:45:58Z

CC @ggwpez

muraca · 2024-03-20T07:44:32Z

this needs another benchmark please

please 🥹

prdoc/pr_1445.prdoc

substrate/frame/collective/src/migrations/v5.rs

liamaharon · 2024-03-25T08:10:47Z

substrate/frame/collective/src/migrations/v5.rs

+pub struct VersionUncheckedMigrateToV5<T, I>(sp_std::marker::PhantomData<(T, I)>);
+impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for VersionUncheckedMigrateToV5<T, I> {
+	#[cfg(feature = "try-runtime")]
+	fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {


liamaharon · 2024-03-25T08:11:41Z

substrate/frame/collective/src/migrations/v5.rs

+		let mut weight = Weight::zero();
+
+		// ProposalOf
+		for (hash, proposal) in old::ProposalOf::<T, I>::drain() {


Probably need to make this a multi-block migration.

liamaharon · 2024-03-25T08:13:03Z

substrate/frame/collective/src/migrations/v5.rs

+			old_proposals.len() as u32 == crate::Voting::<T, I>::count(),
+			"the number of proposals should be the same",
+		);
+		for old in old_proposals {


can we also check that the state of each new proposal is correct (relative to the old proposal state)?

Proposal is saved in a key-value store, where the key is the hash of the value, and is being migrated to Preimages.

We are moving the content here

polkadot-sdk/substrate/frame/collective/src/migrations/v5.rs

Line 98 in 7db06f1

for (hash, proposal) in old::ProposalOf::<T, I>::drain() {

did you mean that I should do an integrity check on the content in the post-upgrade check as well?

Yes, post_upgrade should check every piece of data migrated against the pre-upgrade data and ensure it's in the correct

ggwpez · 2024-04-02T17:58:48Z

bot bench substrate-pallet --pallet=pallet_collective

sry did not see the notifications.

command-bot · 2024-04-02T17:58:54Z

@ggwpez https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5738413 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_collective. 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 4-6b32f3ea-db60-4b12-b826-6997d4b2d743 to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot · 2024-04-02T18:15:27Z

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

ggwpez · 2024-04-02T18:40:50Z

Benchmarks dont compile... weird error: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5738413

muraca mentioned this pull request Sep 7, 2023

Collective pallet - remove without_storage_info #167

Open

muraca force-pushed the issue#167 branch 5 times, most recently from 6d401ff to c2751c4 Compare September 9, 2023 10:12

muraca marked this pull request as ready for review September 9, 2023 12:45

paritytech-ci requested review from a team September 10, 2023 19:19

juangirini reviewed Sep 10, 2023

View reviewed changes

cumulus/parachains/runtimes/collectives/collectives-polkadot/src/impls.rs Outdated Show resolved Hide resolved

substrate/frame/collective/src/migrations/v5.rs Outdated Show resolved Hide resolved

juangirini requested a review from ggwpez September 10, 2023 19:50

paritytech-ci requested a review from a team September 10, 2023 19:50

juangirini added T1-FRAME This PR/Issue is related to core FRAME, the framework. T2-pallets This PR/Issue is related to a particular pallet. and removed T1-FRAME This PR/Issue is related to core FRAME, the framework. labels Sep 10, 2023

muraca force-pushed the issue#167 branch from c2751c4 to cae4bf2 Compare September 11, 2023 19:24

muraca requested a review from a team September 11, 2023 19:24

liamaharon reviewed Sep 12, 2023

View reviewed changes

paritytech-ci requested a review from a team September 12, 2023 06:13

muraca force-pushed the issue#167 branch 2 times, most recently from c987be4 to f52aac8 Compare September 18, 2023 07:39

muraca requested review from liamaharon and juangirini September 18, 2023 08:58

muraca and others added 10 commits November 10, 2023 15:32

warnings for proposal_of

1bf7e5c

Signed-off-by: muraca <[email protected]>

".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…

c17481f

…=dev --target_dir=substrate --pallet=pallet_collective

fixed rust docs

0b06f8b

Signed-off-by: muraca <[email protected]>

small improvements and comments

f9d663b

Signed-off-by: muraca <[email protected]>

post-upgrade state check, comments and improvements

c37e074

Signed-off-by: muraca <[email protected]>

added missing Preimages::unnote, more improvements

1384c5b

Signed-off-by: muraca <[email protected]>

fixed pallet-utility tests

188f336

Signed-off-by: muraca <[email protected]>

removed without_storage_info from pallet-collective

89ee592

Signed-off-by: muraca <[email protected]>

".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…

e0afcb7

…=dev --target_dir=substrate --pallet=pallet_collective

renamed VersionCheckedMigrateToV5 to MigrateToV5 paritytech#2264

27b91f9

Signed-off-by: muraca <[email protected]>

muraca force-pushed the issue#167 branch from 388e8dc to 27b91f9 Compare November 10, 2023 14:35

paritytech-review-bot bot requested a review from a team November 10, 2023 14:35

muraca mentioned this pull request Mar 5, 2024

Removed pallet::getter usage from pallet-collective #3456

Merged

Merge branch 'master' into issue#167

280276d

muraca requested a review from a team as a code owner March 5, 2024 12:24

muraca added 3 commits March 5, 2024 17:30

clippy & fmt

5bfc738

Signed-off-by: Matteo Muraca <[email protected]>

prdoc for @1445

0ca70d3

Signed-off-by: Matteo Muraca <[email protected]>

replaced ugly turbofish syntax

5bf94f8

Signed-off-by: Matteo Muraca <[email protected]>

muraca added 2 commits March 12, 2024 09:06

Merge branch 'master' into issue#167

c252d97

Merge branch 'master' into issue#167

68bd190

muraca added 2 commits March 22, 2024 14:10

Merge branch 'master' into issue#167

c837305

Merge branch 'master' into issue#167

7db06f1

liamaharon reviewed Mar 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removed `without_storage_info` from `pallet-collective` #1445

removed `without_storage_info` from `pallet-collective` #1445

muraca commented Sep 7, 2023 •

edited

Loading

muraca commented Sep 9, 2023

liamaharon left a comment

liamaharon commented Sep 18, 2023

command-bot bot commented Sep 18, 2023

liamaharon commented Sep 18, 2023

command-bot bot commented Sep 18, 2023

liamaharon commented Sep 18, 2023

muraca commented Mar 7, 2024

bkchr commented Mar 11, 2024

muraca commented Mar 20, 2024

liamaharon Mar 25, 2024

liamaharon Mar 25, 2024

liamaharon Mar 25, 2024

muraca Mar 25, 2024

liamaharon Apr 4, 2024

ggwpez commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 2, 2024

ggwpez commented Apr 2, 2024

removed without_storage_info from pallet-collective #1445

Are you sure you want to change the base?

removed without_storage_info from pallet-collective #1445

Conversation

muraca commented Sep 7, 2023 • edited Loading

muraca commented Sep 9, 2023

liamaharon left a comment

Choose a reason for hiding this comment

liamaharon commented Sep 18, 2023

command-bot bot commented Sep 18, 2023

liamaharon commented Sep 18, 2023

command-bot bot commented Sep 18, 2023

liamaharon commented Sep 18, 2023

muraca commented Mar 7, 2024

bkchr commented Mar 11, 2024

muraca commented Mar 20, 2024

liamaharon Mar 25, 2024

Choose a reason for hiding this comment

liamaharon Mar 25, 2024

Choose a reason for hiding this comment

liamaharon Mar 25, 2024

Choose a reason for hiding this comment

muraca Mar 25, 2024

Choose a reason for hiding this comment

liamaharon Apr 4, 2024

Choose a reason for hiding this comment

ggwpez commented Apr 2, 2024 • edited Loading

command-bot bot commented Apr 2, 2024 • edited Loading

command-bot bot commented Apr 2, 2024

ggwpez commented Apr 2, 2024

removed `without_storage_info` from `pallet-collective` #1445

removed `without_storage_info` from `pallet-collective` #1445

muraca commented Sep 7, 2023 •

edited

Loading

ggwpez commented Apr 2, 2024 •

edited

Loading

command-bot bot commented Apr 2, 2024 •

edited

Loading