Migrate pallet-randomness-collective-flip to pallet attribute macro

[koushiro]

Part of #7882

Migrate the randomness-collective-flip pallet to the new pallet attribute macro

polkadot companion: paritytech/polkadot#3200

Breaking Changes

• need to add code like impl pallet_randomness_collective_flip::Config for Runtime {}.

• From https://crates.parity.io/frame_support/attr.pallet.html#checking-upgrade-guidelines

  storages now use PalletInfo for module_prefix instead of the one given to decl_storage: Thus any use of this pallet in construct_runtime! should be careful to update name in order not to break storage or to upgrade storage (moreover for instantiable pallet). If pallet is published, make sure to warn about this breaking change.

  So users of the pallet must be careful about the name they used in construct_runtime!. Hence the runtime-migration label, which might not be needed depending on the configuration of the pallet.

[shawntabrizi]

nits, but looks good otherwise

[athei]

The CI image will be fixed soon. I will restart the failing checks then.

[athei]

Thanks for your contribution.

[thiolliere]

bot merge

[ghost]

Trying merge.

[JoshOrndorff]

@thiolliere I see you added the E1-runtimemigration tag here. I understand that as a chain using this pallet, I will need to do an onchain upgrade to run this new code. But there is no storage migration necessary right? I don't see any on_runtime_upgrade hooks.

[shawntabrizi]

@JoshOrndorff you may need to check that you imported this pallet with the same name as the decl_storage name.

This is the kind of migration you would need to look out for, and would be runtime specific

[thiolliere]

Yeah I updated the top message, the runtime migration is that the pallet prefix may have changed depending the construct_runtime declaration. If it has changed, then a migration must be made to move from the old prefix to the new prefix.

EDIT: note you can use the helper https://substrate.dev/rustdocs/v3.0.0-monthly-2021-05/frame_support/storage/migration/fn.move_pallet.html

[JoshOrndorff]

Thanks for the help. I get it now. Let me make sure by making it totally concrete.

The old version of this pallet declared its storage prefix like this:

decl_storage! {
	trait Store for Module<T: Config> as RandomnessCollectiveFlip {
	  // --snip--
	}
}

So if my runtime declaration like this

// No migration necessary here because I happened to use the same `RandomnessCollectiveFlip` as in decl_storage
construct_runtime! {
	pub enum Runtime ... {
		System: frame_system::{Pallet, Call, Storage, Config, Event<T>},
		RandomnessCollectiveFlip: pallet_randomness_collective_flip::{Pallet, Call, Storage},
		// --snip--
	}
}

// Here a migration is necessary because I used a different name, `RandomnessBeacon`.
construct_runtime! {
	pub enum Runtime ... {
		System: frame_system::{Pallet, Call, Storage, Config, Event<T>},
		RandomnessBeacon: pallet_randomness_collective_flip::{Pallet, Call, Storage},
		// --snip--
	}
}

And if I were in the latter case, then I could write a runtime-level upgrade like this:

struct MigrateCollectiveFlip;
impl OnRuntimeUpgrade for MigrateCollectiveFlip {
  fn on_runtime_upgrade() -> Weight {
    frame_support::storage::migration::move_pallet(b"RandomnessCollectiveFlip", b"RandomnessBeacon");
  }
}

[thiolliere]

yes exactly