Implement session key handover #3034

dvc94ch · 2019-07-05T09:55:51Z

TODO:

rphmeier · 2019-07-05T12:32:17Z

This suffers from the problem I mentioned earlier, which is that users are never going to set their key exactly on the end of a session. it will always be a bit before or after. If all nodes set their new keys a bit before the end of the session, no nodes will author blocks, and the new session will never start until someone intervenes manually.

So we probably need it to author on both keys for some time.

gavofyork · 2019-07-05T17:28:45Z

So the way this should work:

User calls RPC at some point before session with generate new key. Key is stored and indexed in local DB.
User registers the new key for their next session (actually, with session key buffering, it'll kick in one session later, but that doesn't really matter here).
Whenever a new block is to be authored or finalised or whatever, the key to be used is not fixed. Rather it just checks for all current session keys (i.e. keys that would be valid to sign with in this context) and cross-references them with the indexed list of keys that we control. If and only if there is one, then that key is used. If there isn't, then it's assumed that we're no longer validating and we don't bother signing anything.
Keys should be kept in the key database along with an LRU (last recently used) timestamp and a boolean that get flagged when the key is replaced/a new key is generated. Keys that have been unused for over 24 hours and which have been replaced should be removed from the index.

This should be trivial for Babe and Aura. It might require a little help from @rphmeier for Grandpa.

dvc94ch · 2019-07-07T11:31:26Z

in BABE we probably need to start a new authoring future for every key that owns a slot, because slot ownership is associated w/ a VRF proof

Can a validator be assigned more than one slot? In the session module only one key can be active per validator. I'm just taking the first slot that we have a key for.

rphmeier · 2019-07-07T14:45:06Z

@dvc94ch yep, a validator can have more than one slot. What you should do is rather take the first key that owns the slot, if any. So do the vrf-proof on all keys in the current validator set and if there's only one do we author on the slot with that key

core/consensus/babe/src/lib.rs

core/finality-grandpa/src/environment.rs

rphmeier

Should take a different approach in finality-grandpa

dvc94ch · 2019-07-08T10:06:20Z

yeah, the main goal was to get it to build. I'm studying the aura paper you sent me a link to and expect to get through the babe and grandpa papers till tomorrow.

rphmeier · 2019-07-08T10:10:42Z

@dvc94ch OK, great. In the meantime I've checked the logic in Aura and BABE and it seems right.

dvc94ch · 2019-07-08T12:35:52Z

So updated babe and grandpa according to your comments. I think babe is correct now and grandpa selects a key before creating an environment. I'm not sure if the process to select the key is correct. It takes the first key that is in the VoterSet if there is one.

core/finality-grandpa/src/lib.rs

core/keystore/src/lib.rs

core/primitives/src/crypto.rs

core/rpc/src/author/mod.rs

core/keystore/src/lib.rs

rphmeier

Some more grumbles & Qs

core/service/src/lib.rs

core/keystore/src/lib.rs

mxinden

Two questions:

node/cli/src/service.rs

core/service/src/lib.rs

dvc94ch · 2019-07-21T12:55:49Z

since the AuthorityKeyProvider requires the aura api and the grandpa api it has to be implemented in the node. Otherwise we introduce dependencies on aura/grandpa to the service.

gavofyork · 2019-07-22T09:25:17Z

#3150 merged.

gavofyork · 2019-07-25T03:26:40Z

core/finality-grandpa/src/lib.rs

+}
+
+impl<Block, Client> GrandpaKeyProvider<Block, Client>
+where


move to end of line above

and same on further instances of the style

gavofyork · 2019-07-25T03:45:14Z

core/primitives/src/crypto.rs

+	use super::KeyProviderId;
+
+	/// Aura key provider.
+	pub const AURA: KeyProviderId = 10;


Er... no.

core shouldn't be hard-coding consensus-algorithm-related stuff in here.

burdges · 2019-07-25T09:18:36Z

@rphmeier :

yep, a validator can have more than one slot. What you should do is rather take the first key that owns the slot, if any. So do the vrf-proof on all keys in the current validator set and if there's only one do we author on the slot with that key

Actually this sounds confusing and/or harmful. In any slot, there should only be one babe or grandpa key from which the network accepts signatures.

I think gav's description sounds correct #3034 (comment) with the caveat that actually only one current babe or grandpa key is valid. At least BABE need the transaction that registers session key changes finalized in an epoch before the epoch switch.

We should likely include a counter in the session key, which I missed before. If I register a session certificate with a lower counter value than currently set, then the transaction is dropped. If I register a session certificate with a higher counter value, then the transaction sets the session certificate for the appropriate future slot and epoch, with babe requiring some waiting time, and grandpa maybe not so much.

We're happy if registering a new session certificate makes block production with babe impossible for a couple epochs, in which case only one current session key exists.

I want this workflow for validator opeators: Joe our validator operator wishes to migrate validator hardware. First, Joe brings up his new validator, which generates a new session keypair. Second, Joe updates his session certificate by signing the new session public key with his controller key and posting the change session certificate transaction. And this transaction finalizes establishing the exact epoch and slot when Joe's old session key becomes invalid and Joe's new session key becomes valid, for babe and grandpa respectively. Third, Joe's old validator stops signing harmlessly once it discovers it does not control any valid session key. Fourth, Joe's new validator begins running grandpa and then babe with its new session key.

In this workflow, there are no two machines that ever know the same session private key, which makes equivocation impossible, and even permits restricting those keys to SGX enclaves. We thus avoid all the needless slashing done by Cosmos so far. ;)

andresilva · 2019-07-25T11:04:42Z

core/finality-grandpa/src/lib.rs

+	) -> Option<Box<dyn offchain::OffchainKey>> {
+		let authorities = self.client
+			.runtime_api()
+			.grandpa_authorities(at)


This won't give us the active authority set because GRANDPA authority set changes are only enacted on finality. What we need to do is figure out what the highest finalized ancestor of at is (or at itself), and query the runtime state at that block.

Demi-Marie · 2019-07-18T19:25:33Z

core/consensus/babe/src/lib.rs

+			session_store.get_key(public).map(|key| (index, key))
+		})
+		.collect::<Vec<_>>();
+	let (index, key) = if keys.len() == 1 {


What if there is more than one key? That should never happen, but we need to figure out what to do in that case.

@rphmeier said that can be handled in a follow up, maybe by someone more familiar with babe...

Demi-Marie · 2019-07-23T17:17:54Z

core/cli/src/params.rs

 	pub key: Option<String>,

-	/// Enable validator mode
+	/// Shortcut for `--aura` and `--grandpa-voter`.


Should this switch on --babe instead?

These flags should me moved to the node and not be in core

core/cli/src/params.rs

core/consensus/babe/primitives/src/lib.rs

rphmeier · 2019-08-06T09:39:09Z

@burdges

In any slot, there should only be one babe or grandpa key from which the network accepts signatures.

BABE slots can definitely be won by more than one key, just due to the probabilistic nature of the VRF. How likely this is depends on parameterization. I should have clarified:

"Do the vrf-proof on all keys in the current validator set that you control and if there's only one do we author on the slot with that key."

This is literally the same as what Gavin described above and is orthogonal to the migration scheme you mentioned.

dvc94ch added the A0-please_review Pull request needs code review. label Jul 5, 2019

dvc94ch requested a review from rphmeier July 5, 2019 10:41

dvc94ch force-pushed the dvc-rpc-set-key branch from aee2520 to aac6f32 Compare July 7, 2019 10:25

dvc94ch changed the title ~~Adds set_key rpc call and changes aura to use the latest key.~~ Implement session key handover Jul 7, 2019