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

Power: Process batch proof verifications before deferred cron events #909

Closed
wadealexc opened this issue Aug 6, 2020 · 2 comments · Fixed by #1073
Closed

Power: Process batch proof verifications before deferred cron events #909

wadealexc opened this issue Aug 6, 2020 · 2 comments · Fixed by #1073
Assignees
Labels
bug Something isn't working P1 High priority, required for basic network functionality and growth
Milestone

Comments

@wadealexc
Copy link

From OnEpochTickEnd:

a.processDeferredCronEvents(rt)
a.processBatchProofVerifies(rt)

This would also involve some changes to the Miner actor. For example, precommit expiry bound could remove the additional "+1":

// The +1 here is critical for the batch verification of proofs. Without it, if a proof arrived exactly on the
// due epoch, ProveCommitSector would accept it, then the expiry event would remove it, and then
// ConfirmSectorProofsValid would fail to find it.
expiryBound := rt.CurrEpoch() + msd + 1
enrollCronEvent(rt, expiryBound, &cronPayload)

@wadealexc
Copy link
Author

ConfirmSectorProofsValid reschedules expirations for replaced sectors to the end of their next deadline window:

// Schedule expiration for replaced sectors to the end of their next deadline window.
// They can't be removed right now because we want to challenge them immediately before termination.
err = st.RescheduleSectorExpirations(store, rt.CurrEpoch(), info.SectorSize, replaceSectors)
builtin.RequireNoErr(rt, err, exitcode.ErrIllegalState, "failed to replace sector expirations")

If the sector to be replaced is in a deadline whose Close epoch is CurrEpoch + 1, the new expiration chosen is equal to CurrEpoch:

if err = deadlineSectors.ForEach(func(dlIdx uint64, pm PartitionSectorMap) error {
dlInfo := NewDeadlineInfo(st.ProvingPeriodStart, dlIdx, currEpoch).NextNotElapsed()
newExpiration := dlInfo.Last()

Because the power actor processes batch proof verification after deferred cron events, the handleProvingPeriod cron event won't catch this expiry immediately:

// Expire sectors that are due, either for on-time expiration or "early" faulty-for-too-long.
expired, err := deadline.PopExpiredSectors(store, dlInfo.Last(), quant)
builtin.RequireNoErr(rt, err, exitcode.ErrIllegalState, "failed to load expired sectors")

If I understand correctly, this means that the miner won't lose power for the replaced sector until the next day.

@anorth anorth added bug Something isn't working P1 High priority, required for basic network functionality and growth labels Aug 8, 2020
@anorth
Copy link
Member

anorth commented Aug 8, 2020

FYI @acruikshank an example of something for which we need a scenario test.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working P1 High priority, required for basic network functionality and growth
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants