Make author_mapping::set_keys take 1 input like pallet_session::set_keys

[4meta5]

What does it do?

⚠️ Changes set_keys extrinsic from

set_keys(
	origin: OriginFor<T>,
	old_author_id: NimbusId,
	new_author_id: NimbusId,
	new_keys: T::Keys,
)

to

set_keys(origin: OriginFor<T>, keys: (NimbusId, T::Keys))

⚠️ Changes register_keys extrinsic from

register_keys(origin: OriginFor<T>, author_id: NimbusId, keys: T::Keys)

to

register_keys(origin: OriginFor<T>, keys: (NimbusId, T::Keys))

This will allow collators to copy paste their output from RPC author_rotateKeys into these extrinsics just like the Polkadot/Kusama UX.

In order to do this, it adds a reverse mapping from AccountId to NimbusId.

Breaking Change (Maximum 1 NimbusId Per AccountId)

The migration will take the first NimbusId owned by a given AccountId to insert into NimbusLookup: AccountId => Option<NimbusId>. For any additional NimbusId owned by that AccountId, the migration clears the association and returns (unreserves) the deposit.

[librelois]

please replace T::AllKeys by (NimbusId, T::Keys)

[4meta5]

Need to change NimbusLookup from AccountId => Option<NimbusId> to AccountId => Option<Vec<NimbusId>> because current code does not enforce max 1 NimbusId per AccountId and we do not need to enforce this either

Growing the Vec<NimbusId> requires a deposit of T::DepositAmount per entry so it is not strictly bounded but economically

[librelois]

current code does not enforce max 1 NimbusId per AccountId

If we have no useful use case to have several NimbusId linked to a single AccountId, it is better to forbid this.

[4meta5]

If we have no useful use case to have several NimbusId linked to a single AccountId, it is better to forbid this.

@librelois what to do if there are existing cases of 1 AccountId linked to several NimbusId? How would the migration work?

On moonbeam, 0x88848d6A768adfaE32C9aef05D70C522893efa1C is linked to two different NimbusIds.
On moonriver, 0x19EAa6Aae65e4B02539de493a735B81d1d848829 is linked to three different NimbusIds.
There may be more cases because we did not enforce the constraint before.

[crystalin]

I think migration should be simple, pick the first one in the list.
However we should document that in the migration and the breaking change doc

[crystalin]

We should also notify Yann that a communication with collators will be required as early as possible

[4meta5]

I think migration should be simple, pick the first one in the list.

Ok I will remove the association for the others

[4meta5]

@nbaztec @notlesh where is the output to the benchmarking stored? Before I would run benchmarking and cat /tmp/$pallet_name.rs but I can no longer find the weights file generated by the benchmarking. What is the file name?

[nbaztec]

Think if you run on the benchmarking machine it will be under moonbeam/benchmarks/{pallet_name.rs}. Locally it will be under moonbeam/weights.rs

[girazoki]

MY latest experience is that it is in the moonbeam root folder as weights.rs. Dont know if this has changed though