paritytech / polkadot

Polkadot Node Implementation
GNU General Public License v3.0
7.13k stars 1.58k forks source link

Parent hash to session index mapping in parachains module too aggressive #924

Closed rphmeier closed 4 years ago

rphmeier commented 4 years ago

https://github.com/paritytech/polkadot/pull/840#issuecomment-602020273

cc @montekki

The pruning introduced in this PR is too aggressive, as we will not be able to accept any reports from prior eras. The main issue with this is that at the very beginning of an era there is almost nothing that can be slashed for. Technically, we should be able to accept reports from all eras for which are currently bonded, although slashing for the current era as well as the last gets us the desired behavior in practice. Eras will be ~24 hours long, so if a misbehavior is not detected within that time, it may never be.

Also as a nit, it would be nice to remove the direct dependence on staking module. Not really a problem as we are in polkadot space, not generic component space, but it still feels cleaner to expose a generic interface

rphmeier commented 4 years ago

A completely alternative strategy to achieve the same goal that the parent_hash -> session_index mapping did just occur to me, which may be cleaner overall.

The issue that we have without this mapping is that we have no way of ensuring that the proof-of-set-membership actually corresponds to the same session as the Statements were signed in. However, every signature on a Statement also encompasses the parent_hash. The mapping lets us associate the parent-hash we need to check the signature with the correct session.

This has two drawbacks:

So as an alternative to the mapping that solves the same problem, I propose that we instead include the session_index in the payload for fn sign_table_statement. The implementation fallout from this would include at least these tasks: