(consensus & grandpa module) SessionKey is inconsistent

paritytech / substrate

Substrate: The platform for blockchain innovators

Apache License 2.0

8.39k stars 2.65k forks source link

(consensus & grandpa module) SessionKey is inconsistent #1840

Closed gguoss closed 5 years ago

gguoss commented 5 years ago

https://github.com/paritytech/substrate/blob/f8b0e7db4bb5d64e68752435e424814f607a0f82/node/runtime/src/lib.rs#L117 https://github.com/paritytech/substrate/blob/f8b0e7db4bb5d64e68752435e424814f607a0f82/node/runtime/src/lib.rs#L190

If you need to be consistent, delete grandpa directly, grandpa trait inherits from consensus。 Conversely, grandpa's sessionkey can only be converted from account_id , not set_authority

rphmeier commented 5 years ago

Why should we force it to be consistent?

gguoss commented 5 years ago

if not consistent: There are some bug:
bug 1: grandpa's session key can't set. but (aura)consensus's session key can set. bug 2: grandpa and aura session key are fetch from session modules' validator. these is consistent.
if consistent: There are a bug: SessionKey not consistent, As I said above.

rphmeier commented 5 years ago

Don't know what you mean. The authority sets are separate but can be optionally synchronized via SyncedAuthorities based on the start of each session. It's up to your runtime to decide.

But if they are not synced then they should be allowed to use different key types, and this is something that makes a lot of sense because there are voting optimizations that you can implement for e.g. BLS signatures.

gguoss commented 5 years ago

That is to say, consistent or non-consistent, by the implementer to customize the implementation