Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

ValidatorIndex is context-dependent but the docs don't explain that. #1899

Closed
pepyakin opened this issue Nov 2, 2020 · 3 comments
Closed
Labels
I7-documentation Documentation needs fixing, improving or augmenting.

Comments

@pepyakin
Copy link
Contributor

pepyakin commented Nov 2, 2020

We have a type ValidatorIndex which is an alias to a u32 which represents a validator, as the docs say, in a lightweight manner. As far as I understand, the ValidatorIndex implicitly depends on the current validator set. The problem is the notion of current.

While we could update the ValidatorIndex's docs, I think there is a bigger problem. It is not immediately obvious what does "current" means.

For example, let's take the AV subsystem (note that the issue is not limited to this particular area). It has the following message

/// Query an `ErasureChunk` from the AV store by the candidate hash and validator index.
QueryChunk(Hash, ValidatorIndex, oneshot::Sender<Option<ErasureChunk>>),

as far as I understand, this index is the index of a validator from the set of all active validators in the session when this candidate got backed, but that requires a bit of knowledge and requires some guess work. That seems to be inviting for some subtle issues.

@pepyakin pepyakin added the I7-documentation Documentation needs fixing, improving or augmenting. label Nov 2, 2020
@coriolinus
Copy link
Contributor

Would it be better if ValidatorIndex were (Era, u32)? That would avoid ambiguity about what we mean when the validator set changes.

@pepyakin
Copy link
Contributor Author

pepyakin commented Nov 3, 2020

If it works, even better!

@rphmeier
Copy link
Contributor

Obsoleted by paritytech/polkadot-sdk#978

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-documentation Documentation needs fixing, improving or augmenting.
Projects
None yet
Development

No branches or pull requests

3 participants