let `initialize` function in pallet-grandpa/pallet-babe and other session managed pallets be `pub`

[atenjin]

Is there an existing issue?

• [X] I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• [X] This is not a support question.

Motivation

In substrate, manage pallet-grandpa/pallet-babe/pallet-aura and other similar pallets are all managed by Session trait OneSessionHandler. And this trait just can be called by pallet-session.

In solo-chain, parachain, most of chain may use the session to manage keys.

However in other case, like in consortium chain(permission chain), layer2 sequence(like https://github.com/keep-starknet-strange/madara) and other cases, we do not need the pallet-session and related trait to manage those keys, for example in our case, we just need pallet-aura and pallet-grandpa, so we set those two keys directly. Using session to manage keys is useless for us.

Request

Above all, we can find that writing another way to manage keys rather then pallet-session in some cases is necessary, so we hope those pallets let related function be pub.

But for now, just pallet-aura let the function initialize_authorities be pub:

https://github.com/paritytech/substrate/blob/edf58dceaf3e1f1710e7ac83ac1cc0831e2ef9b2/frame/aura/src/lib.rs#L197-L204

other pallets, like pallet-grandpa, the initialize is not pub

https://github.com/paritytech/substrate/blob/edf58dceaf3e1f1710e7ac83ac1cc0831e2ef9b2/frame/grandpa/src/lib.rs#L514-L524

So at least for the initialize part, if this part is pub, we can write other pallet to manage those keys for init step, rather then impl trait OneSessionHandler.

Solution

go through the pallets, I think we need to let following pallet changing the init function be pub

necessary:

1. pallet-grandpa
2. pallet-babe

not very necessary:

1. pallet-im-online
2. pallet-authority-discover

Are you willing to help with this request?

Yes!

[bkchr]

So at least for the initialize part, if this part is pub, we can write other pallet to manage those keys for init step, rather then impl trait OneSessionHandler.

The grandpa implementation is setting also the session id, which you probably don't want to do after genesis. I get your problem, but not sure if we don't need like extra functions. We would also need to check if you can just change the authorities or if there are other invariants that assume that this happens after some time.

[atenjin]

emmm in fact without session, for not pallet-grandpa already can replace to next authorities for now, so the SetIdSession storage is necessary with session pallet?

[davxy]

(Just some ideas... I may overlooked some details so experimentation is required)

When we change the authority set in a pallet we are creating a new session.

Per-se (at high level) this session concept is detached from the detail that it may be driven via the session-pallet (e.g. conceptually you should be able to start a babe session without using the session pallet. And this reflects your use case).

Indeed the OneSessionHandler is not defined by the session pallet but is defined in the frame support traits.

So my first suggestion would be to let the pallet perform all its (encapsulated) logic when the authority set (aka session) changes. By brutally setting only the authorities you may end up missing some important logic that the pallet require to perform (and that may change over time).

Thus, to let the pallet do its logic, I'd suggest to use the OneSessionHandler::on_new_session.

The problem here is that it requires your Pallet to implement pallet_session::Config and in your case you don't.

So at this point you need to remove the requirement that your pallet should implement the pallet_session::Config when implementing OneSessionHandler in babe and grandpa (im-online and ... already doesn't require it). To do this you can add an associated type in babe/grandpa pallet Config that yields the current session-id and use that instead.

I suggest to use an associated type called SessionNumber bounded by GetSessionNumber trait. At the moment is already implemented for sp_core::Void (and this is the type you require to use). You'll need to implement it for pallet_session as well for the cases where we are using the pallet_session instead. Something like:

(in frame/session/src/lib.rs)

impl<T: Config> GetSessionNumber for Pallet<T> {
    fn session(&self) -> SessionIndex {
        <Pallet<T>>::current_index()
    }
}

[davxy]

This may be also a step towards https://github.com/paritytech/substrate/issues/14198 (further detach consensus algorithms from session pallet)