Generate ownership proof for `SessionKeys` #48

bkchr · 2023-11-13T15:37:19Z

This RFC changes the SessionKeys runtime api to support generating a proof of ownership of the generated session keys.

tomaka · 2023-11-13T19:37:13Z

text/0048-session-keys-runtime-api.md

+```
+
+The `proof` being a SCALE encoded tuple over all signatures of each private session 
+key signing the `owner`. The actual type of each signature depends on the


So the message being signed is the SCALE-encoded account_id?

tomaka · 2023-11-13T19:39:01Z

text/0048-session-keys-runtime-api.md

+	pub proof: Vec<u8>,
+}
+
+fn SessionKeys_generate_session_keys(owner: Vec<u8>, seed: Option<Vec<u8>>) -> OpaqueGeneratedSessionKeys;


An AccountId is a u64 I believe. How does that translate to a Vec<u8>?
I suppose through SCALE encoding, but since the function signature here is the "high level one", shouldn't it be u64 here? Otherwise, the AccountId gets SCALE-encoded twice, no?

You'd presumably sign the actual controller key here, not some reference.

An AccountId is a u64 I believe. How does that translate to a Vec<u8>?

AccountId for Polkadot is an enum with public keys of sr/ed25519 or ecdsa. Other chain can also have different account ids. Thus, this should be a Vec<u8> that represents the SCALE encoded account id.

tomaka · 2023-11-13T19:41:27Z

text/0048-session-keys-runtime-api.md

+When a user sets new `SessionKeys` on chain the chain can currently not ensure that the user 
+actually has control over the private keys of the `SessionKeys`. With the RFC applied the chain is able 
+to ensure that the user actually is in possession of the private keys.


Worded differently, you want to give the possibility to the implementation of SessionKeys_generate_session_keys to know which AccountId the host controls?

Yes. I mean generate_session_keys doesn't care as it just signs whatever you pass it.

At least for me, the motivation does not go deep enough. Why should the chain care?

Why should the chain care?

Care that you are in possession of the keys? The chain doesn't really need to care. However, someone could may include a transaction faster than you with your public keys registered. So, someone could theoretically prevent that a validator is rotating its keys.

LOL. Ok, makes sense! Thanks!

burdges · 2023-11-14T03:21:39Z

In this RFC, all session keys first acknowledge their controller key by signing it, after which their controller key certifies them. Yes, we do obtain the desired back certificates this way, but I'd envisioned doing this in part the other way round, plus adding a master session key.

It'd look like this:

All other subordinate session keys back certify this master session key, likely the grandpa key.
The master session key certifies all other subordinate session keys, along with their certificates of the master session key, probably as pairs (subordinate_session_key,its_signature_on_grandpa_key).
Operator certifies the master session key with their controller key.
At node start, the master session key certifies the controller key along with both its certificate of the master session key and a random nonce. We call this the active certificate.
Pre-sync, the starting node announces its certificate collection, including this new one created upon startup.
An already active synced node posts the active certificate on-chain via some intrinsic, replacing the previous one.
The starting node waits until its active certificate get finalized before running protocols with equivocation slashing, like grandpa or babe.
Any node shuts down if its active certificate get replaced on-chain.

In emergencies, a command line option could overrides 7, and maybe this could happen automatically, but usually 7 prevents nodes being slashed for operator error.

In this, 1-2 have same orientation as proposed here, but 3-4 have the reversed orientation. We must check a two step certificate chain, including two back certificates, before believing a session key, but this should be a rare enough it does not matter. Our nonce in 4 could be some ephemeral key like maybe the transport layer key.

Anyways..

If there is a pressing need to do something simpler first then fine, and validator operators rarely get slashed for equivocations these days, maybe they get 0% slashed or we refund them, but it's maybe worth defending against the equivocations footgun like this. A master session key permits changing session keys without impacting user experience for validator operators, and consolidates the nonce lock under one session key, but this could be done without the master session key.

eskimor · 2024-04-05T09:30:56Z

text/0048-session-keys-runtime-api.md

+The RPC that exists around this runtime api needs to be updated to support passing the account id 
+and for returning the ownership proof alongside the public session keys.
+
+UIs would need to be updated to support the new RPC and the changed on chain logic.


Will they break with this update? Or is there a deprecation/upgrade path?

There is a new RPC for this. So, they will not break. However when the runtime is upgraded it will require the proofs which only the new UI generates. There is also only polkadot-js which supports this AFAIK.

eskimor · 2024-04-05T09:31:18Z

text/0048-session-keys-runtime-api.md

+
+## Summary
+
+This RFC proposes to changes the `SessionKeys::generate_session_keys` runtime api interface. This runtime api is used by validator operators to


Actually, why do we need a runtime API to generate keys?

Because the node doesn't know what session keys the runtime is using.

Instead of generating the keys in the runtime, which in fact ends up generating the keys via some host call, would be nice to have instead the runtime returning a list of (CryptoTypeId, KeyTypeId).
Then generate the keys directly using the keystore.

The advantage of this approach would be to deprecate the host calls to generate the keys from the runtime which (IMO) is a bit awkward.

These host calls are also required for benchmarking to generate private keys. CryptoTypeId didn't exist at the time this runtime api was introduced. This is also really the topic of this RFC.

In the end you also still need to call into the runtime to do this. In hindsight I would also design some of the host calls differently to make nodes for example "more generic". This can be done, but yeah isn't part of this RFC and this RFC will also not make any of that more complicated.

bkchr · 2024-04-24T22:07:22Z

/rfc propose

paritytech-rfc-bot · 2024-04-24T22:07:38Z

Hey @bkchr, here is a link you can use to create the referendum aiming to approve this RFC number 0048.

Instructions

Open the link.
Switch to the Submission tab.

Adjust the transaction if needed (for example, the proposal Origin).
Submit the Transaction

It is based on commit hash adb59579ee424853a3174740b65e991a2a8d31aa.

The proposed remark text is: RFC_APPROVE(0048,2c5d27eb220a205e0c6634379a4a9cf6b6208362de7eb22660305f5dcfef7ab3).

burdges · 2024-04-25T04:52:07Z

We'd reduce slashing risks considerably if we handle this like I propsed above: #48 (comment)

github-actions · 2024-04-30T12:25:27Z

Voting for this referenda is ongoing.

Vote for it here

github-actions · 2024-05-07T12:25:39Z

PR can be merged.

Write the following command to trigger the bot

/rfc process 0x34f2bcbf3ef8b5ef7d7ab74cef6d391c1f6aee227daa5a67108f9e9c8ffd00aa

bkchr · 2024-05-07T20:14:20Z

/rfc process 0x34f2bcbf3ef8b5ef7d7ab74cef6d391c1f6aee227daa5a67108f9e9c8ffd00aa

paritytech-rfc-bot · 2024-05-07T20:14:36Z

The on-chain referendum has approved the RFC.

bkchr added 2 commits November 13, 2023 16:34

Initial commit

829ed47

Use correct RFC id

2abb753

tomaka reviewed Nov 13, 2023

View reviewed changes

bkchr changed the title ~~RFC: Generate ownership proof for SessionKeys~~ Generate ownership proof for SessionKeys Nov 22, 2023

Update text

adb5957

eskimor reviewed Apr 5, 2024

View reviewed changes

paritytech-rfc-bot bot merged commit 4d0a344 into main May 7, 2024

bkchr deleted the bkchr-session-keys-runime-api branch May 7, 2024 20:43

anaelleltd added Approved Has passed on-chain voting. Implementing Is actively being worked on. and removed Approved Has passed on-chain voting. labels Sep 10, 2024

burdges mentioned this pull request Oct 10, 2024

Implement Proof Of Possession capability for all public key crypto types paritytech/polkadot-sdk#6010

Draft

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate ownership proof for `SessionKeys` #48

Generate ownership proof for `SessionKeys` #48

bkchr commented Nov 13, 2023

tomaka Nov 13, 2023

bkchr Nov 14, 2023

tomaka Nov 13, 2023 •

edited

Loading

burdges Nov 14, 2023

bkchr Nov 14, 2023

tomaka Nov 13, 2023 •

edited

Loading

bkchr Nov 14, 2023

eskimor Nov 23, 2023

bkchr Nov 23, 2023

eskimor Nov 23, 2023

burdges commented Nov 14, 2023 •

edited

Loading

eskimor Apr 5, 2024

bkchr Apr 6, 2024

eskimor Apr 5, 2024

bkchr Apr 6, 2024

davxy Apr 6, 2024 •

edited

Loading

bkchr Apr 6, 2024

bkchr commented Apr 24, 2024

paritytech-rfc-bot bot commented Apr 24, 2024

burdges commented Apr 25, 2024 •

edited

Loading

github-actions bot commented Apr 30, 2024

github-actions bot commented May 7, 2024

bkchr commented May 7, 2024

paritytech-rfc-bot bot commented May 7, 2024


		## Summary

		This RFC proposes to changes the `SessionKeys::generate_session_keys` runtime api interface. This runtime api is used by validator operators to

Generate ownership proof for SessionKeys #48

Generate ownership proof for SessionKeys #48

Conversation

bkchr commented Nov 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomaka Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomaka Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burdges commented Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davxy Apr 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkchr commented Apr 24, 2024

paritytech-rfc-bot bot commented Apr 24, 2024

burdges commented Apr 25, 2024 • edited Loading

github-actions bot commented Apr 30, 2024

github-actions bot commented May 7, 2024

bkchr commented May 7, 2024

paritytech-rfc-bot bot commented May 7, 2024

Generate ownership proof for `SessionKeys` #48

Generate ownership proof for `SessionKeys` #48

tomaka Nov 13, 2023 •

edited

Loading

tomaka Nov 13, 2023 •

edited

Loading

burdges commented Nov 14, 2023 •

edited

Loading

davxy Apr 6, 2024 •

edited

Loading

burdges commented Apr 25, 2024 •

edited

Loading