Should we remove `sudo_sessionKeys_unstable_generate`?

[bkchr]

As described in spec for sudo_sessionKeys_unstable_generate, this only maps to the SessionKeys runtime api. Given that, clients can just use state_call for calling the SessionKeys api.

[tomaka]

The problem is that you need to insert the keys in the keystore as well.

We could in principle instead add a JSON-RPC function to add a private key to the keystore, and let the JSON-RPC client do the runtime call, but sending private keys over the Internet or even in memory isn't the greatest idea.

[bkchr]

In Substrate the host functions for generating the key are storing the private keys. The RPC implementation isn't involved in this at all. The RPC implementation doesn't know what the runtime is doing.

[lexnv]

Just for reference, this is how the sessionsKeys implementation in substrate looks like, will put on hold that PR until we decide what's best

[tomaka]

Ah I see.

There's still a problem, though: since state_call (and chainHead_call/archive_call) are safe, we don't want everyone to be able to add keys to the keystore.

[bkchr]

Nodes that are validators should not expose any RPC functionality at all and for full nodes we could let them run with a ephemeral keystore.

[lexnv]

If I got this right, we could remove the sessionsKeys group.
@tomaka what do you think would be best for end-users? 🤔

[tomaka]

I'm not sure.

It's clear to me that the runtime calls done through state_call and equivalents should be "pure" (i.e. the output only depends on the inputs). They shouldn't have access to the offchain worker functions (like time and randomness), because we don't want the JSON-RPC client to ask the node to start HTTP requests and to add transactions to the pool and things like that.

Another point is that it doesn't really make sense to be able to call sudo_sessionKeys_unstable_generate against a block other than the current best or finalized block (I'm actually not sure which one is appropriate).

I also disagree with "validator nodes should simply not allow public JSON-RPC requests". If we go this direction, it would be a subtle thing that validator operators should know, and if they don't know this they can suffer massive fuck ups. We should prevent potential fuck ups as much as possible instead of going the direction of "well people should just know better".

[bkchr]

They shouldn't have access to the offchain worker functions (like time and randomness), because we don't want the JSON-RPC client to ask the node to start HTTP requests and to add transactions to the pool and things like that.

Good point! I also realized that we actually do this already, aka register the keystore as extension for these runtime calls. So, they would fail in state_call.

Yeah, then this issue is actually not required! Ty for the input. However, I will open a pr to change the spec in the light of: paritytech/polkadot-sdk#1739