Rework election eligibilty predicates #1122
Conversation
| t.Run("miner eligible", func(t *testing.T) { | ||
| mstate := constructMinerState(ctx, t, store, owner) | ||
| pstate := constructPowerStateWithMiner(t, store, maddr, pwr, proofType) | ||
| rstate := constructRewardState(t, tenFIL) |
actors/states/election.go
Outdated
| } | ||
|
|
||
| // Tests whether a miner is eligible for election given a Winning PoSt lookback state. | ||
| func MinerEligibleForElectionAtPoStLookback(store adt.Store, pstate *power.State, mAddr addr.Address) (bool, error) { |
There was a problem hiding this comment.
Nit. This function returning true doesn't imply the miner was eligible at PoSt lookback. Also this method doesn't deal with lookback directly. MinerMeetsConsensusMinimumAtPoStLookbackwould be more accurate. MinerMeetsPoStLookbackEligibilityRequirement? I'm not convinced this function adds much over calling pstate.MinerNominalPowerMeetsConsensusMinimum(store, mAddr) directly.
The comment kind of gets at it, but it would be better to to directly call out "pstate must be the prior state of the power actor at the PoSt lookback epoch". This is an unusual thing in the actor code.
There was a problem hiding this comment.
What this adds is that when the eligibility requirements change, we can encapsulate that change here (or at least force downstream ack of it through a type signature change). Such a change is likely to be accompanied by other actor changes so we can cover the whole thing and reduce coordination with Lotus.
Thanks for the name suggestion.
anorth
force-pushed
the
anorth/eligibilty
branch
from
869a78b to
3acdbf7
Compare
Codecov Report
@@ Coverage Diff @@
## master #1122 +/- ##
========================================
- Coverage 75.9% 75.9% -0.1%
========================================
Files 49 50 +1
Lines 6282 6284 +2
========================================
- Hits 4772 4770 -2
- Misses 894 895 +1
- Partials 616 619 +3 |
The miner election eligibility predicates need to be split into one that operates on the parent state, and one at the Winning PoSt lookback.
statespackage intended for use from outside the actors themselvesMinerEligibleForElectionchange the consensus min power check to a non-zero claim checkMinerEligibleForElectionAtPoStLookbackwhich checks for consensus min powerCloses #1102
FYI @nicola @whyrusleeping @magik6k