Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement MinerEligibleForElection in block validation #3378

Closed
nicola opened this issue Aug 28, 2020 · 6 comments · Fixed by #4188
Closed

Implement MinerEligibleForElection in block validation #3378

nicola opened this issue Aug 28, 2020 · 6 comments · Fixed by #4188
Labels
P0 P0: Critical Blocker

Comments

@nicola
Copy link

nicola commented Aug 28, 2020

We should use the current epoch to perform this actors check.
Miner should not be allowed to mine if not eligible

cc @Stebalien

@daviddias daviddias added the P0 P0: Critical Blocker label Aug 28, 2020
@anorth
Copy link
Member

anorth commented Sep 5, 2020

Wait, what? Should't it be at WinningPoStSectorSetLookback? We've been gathering the election requirements together into one MinerEligibleForElection function on the assumption that it's all using the same lookback. I don't think you meant to say that, e.g., the power meeting consensus minimum check is made at the current epoch.

I think something might have been lost here in the change to consensus fault penalisation. Previously, when a miner was terminated, that check was made at the current epoch. But everything else was at the election lookback. Now it's apparently more complicated.

@nicola could you please lay out all of the election criteria and which epoch they should be evaluated at? The actors will then need to break apart MinerEligibleForElection into two functions, one intended for the state at each of the epochs.

@nicola
Copy link
Author

nicola commented Sep 5, 2020

I am currently adding clarity by writing this into this spec draft.

All the checks for MinerEligibleForElection are for the same epoch and this epoch is the epoch of election.

@nicola
Copy link
Author

nicola commented Sep 5, 2020

Just to clarify what is written in the spec here:

Lotus should do three checks:

  • Eligibility check (consensus fault fee, consensus fault active, debt) using MinerEligibleForElection at current epoch
  • Miner validity check (check if miner exists ) at current epoch
    • Note: this is currently done in lotus with minerIsValid
  • Minimum power check (check that miner has min miner size) at WinningPoStLookback
    • Note: this is currently done in lotus with MinerNominalPowerMeetsConsensusMinimum

So, in practice, the actions to take in lotus are:

  • Make sure that minerIsValid, MinerNominalPowerMeetsConsensusMinimum are called at correct epochs
  • Add MinerEligibleForElection check

Note that this means that eligibility does NOT check for min miner size, the following check should be removed: https://github.com/filecoin-project/specs-actors/blob/master/actors/builtin/miner/miner_state.go#L1104-L1106

This is at least how it currently works, we could try to change this check to be at the same epoch, but this will require analysis and if important we should do it at a later time.

This note is being tracked here: filecoin-project/specs-actors#1102

@anorth
Copy link
Member

anorth commented Sep 7, 2020

Actors intends to give you two functions: one to execute at parent state, and one with the lookback state.

@anorth
Copy link
Member

anorth commented Sep 9, 2020

The state inspection methods for the election eligibility predicates are now available at https://github.com/filecoin-project/specs-actors/blob/master/actors/states/election.go

@arajasek
Copy link
Contributor

Fixed in #4188

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 P0: Critical Blocker
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants