Allow miners to set expiration duration for (deal) sectors

[jennijuju]

The sectors expiration duration can be any time from 180 days - 540 days. Currently, all the cc sectors' expiration duration is set to 540 days by default, and we do not expose any cli for miners to customize it. Sectors with deals are expired when the last deal is ended.

Ideally, we should allow miners to set the below values in the configuration:

• default expiration duration for cc sectors #6836

• minimum expiration duration for deal sectors before sealing

[momack2]

Seems like the two potential ways to design this are to either allow miners to explicitly pass a duration to override the default when sealing a new sector - or expose the default in something like the config for miners to configure within the accepted min/max bounds.

Note, choosing a shorter duration than the default/max means that miners will have to reseal the sector to maintain power (expending work that could have gone toward sealing a new sector) - so the long-term expectation for rewards for unit effort is worse the shorter you configure your expiration duration.

[magik6k]

Roughly what needs to be done:

• Add new fields to the Sealing config - node/config
• Plumb sealer config to https://github.com/filecoin-project/lotus/blob/master/extern/storage-sealing/precommit_policy.go
• Handle those in BasicPreCommitPolicy

[placer14]

Naming things is hard. Here's what I'm going with for now. Welcoming your suggestions (or if there's nomenclature that is already familiar within the community that is more apt).

type SealingConfig struct {
...
	// MinDealDurationInSector is the minimum required duration for any deal within a sector
	// before it can be sealed. This value is useful to control the minimum desired sector
	// lifetime as sectors expire when the longest deal is completed/terminated. (Note: Deals
	// which terminate prematurely could also cause the sector to expire prematurely if it is
	// the oldest deal remaining in the sector.) Value must be between 180-540 days inclusive.
	MinDealDurationInSector Duration

	// CommittedCapacityDefaultLifetime is the default duration a Committed Capacity (CC)
	// sector will live before it is either expired or converted into sector containing deals.
	// Value must be between 180-540 days inclusive.
	CommittedCapacityDefaultLifetime Duration
...
}

/cc @magik6k @jennijuju

[placer14]

@magik6k @Kubuxu Within the context of handlePreCommitting (in extern/storage-sealing/states_sealing.go), how do we want to handle the case where sector has no piece w DealInfo.DealSchedule.EndEpoch > MinDealDurationInSector (that the sector doesn't have any deals which satisfy the operator's desire for minimum sector expiry as controlled by the newly proposed sealing config value MinDealDurationInSector mentioned above)? Assumedly, we want the worker to delay sealing this sector, but I'm not sure how to signal that from this context. Or maybe we want to do something else?

(edited for simplicity)

[jennijuju]

MinDealDurationInSector is a bit confusing imho. AFAIK Sector expiration is not bounded by the longest deal duration, but the latest end epoch of the deals in the sector.

[jennijuju]

For CommittedCapacityDefaultLifetime, I will add or extended lifetime to before it is either expired or converted into sector containing deals.?

[placer14]

@jennijuju

AFAIK Sector expiration is not bounded by the longest deal duration, but the latest end epoch of the deals in the sector.

Yes, correct. This is what I'm trying to capture in the name. This value intends to represent the "minimum deal duration that we would expect in the sector before it is sealed" and not the "longest deal duration possible by the protocol".

I will add or extended lifetime to before it is either expired or converted into sector containing deals.?

What about this:

	// CommittedCapacityDefaultLifetime is the default duration a Committed Capacity (CC)
	// sector will live before it must be extended or converted into sector containing deals
        // before it is terminated.
	// Value must be between 180-540 days inclusive.
	CommittedCapacityDefaultLifetime Duration

[Kubuxu]

Names seem alright to me.

I'm not sure what to do about handlePreCommitting miners might want to refuse to accept deals that don't satisfy MinDealDurationInSector but at least we need to delay sealing. The question is what to do when the deal is going to expire before miner gets the right deal to add to that sector.

handlePreCommiting is too late for deciding or delaying deals, that decision needs to be made before sealing the sector. handlePreCommiting just submitts PreCommit on chain.

[placer14]

I'm still not sure what to do to proceed with MinDealDurationInSector. The crux of my block is handling these params within BasicPreCommitPolicy. Am I intended to error out of BasicPreCommitPolicy.Expiration? Does handlePreCommitting handle that? (Noting that @Kubuxu believes it's too late to delay sealing from within handlePreCommitting. (And are there any side-effects to cleanup? If so, how?)

Note that @magik6k has also asked for validation of these values to be handled on lazy-load within the Policy, which is only instantiated from within handlePreCommitting so there is no way to validate the value before it fails to be used at least once (going to be poor UX when misconfigured).

Looking for some clarity on handling these rough edges before proceeding (and I believe this is context that only @magik6k can provide at the moment?).

[magik6k]

The way I see this:

• PreCommitPolicy is executed to get a sector expiration for a sector that is about to be precommitted, but was already sealed (and it may contain deals), so it has some value to the miner
• So it should try to handle most error cases as gracefully as possible - e.g. if user-configured values are out of bounds, it should use whatever value makes most sense withing the bounds, and log a warning.
• If user configuration is totally invalid, also log a warning, and fallback on defaults (which should give the user most flexibility, so in this case the default should be the shortest possible sector lifetime, so in case of misconfiguration, those sectors can be extended after the fact (if the default would be higher lifetime, you can't shorten it after the sector is committed))

[Kubuxu]

@magik6k parameter being introduced here affects the scheduling of sealing of the sector with deals. So it cannot be handled in PreCommitPolicy.

[placer14]

(Note: I pulled the CC configuration into it's own issue/PR. Leaving this issue to continue discussion of the proposed MinDealDurationInSector param.)

[placer14]

The scenario we're trying to handle more gracefully is the following: A worker has MinDealDurationInSector set to, say, 360 * 24 * time.Hour (360 days) and the miner begins sealing a sector where none of the included deal pieces have an EndEpoch which extends out to currentHeight + durationToEpochs(MinDealDurationInSector). I don't think we're protecting for this case before handlePreCommitting and handling it within handlePreCommitting (the first place BasicPreCommitPolicy (BPCP) is instantiated) will leave BPCP with the only resort of returning an error and failing the sealing sector in-progress. It sounds like we don't want this and will need to identify where (earlier in the sealing pipeline) we should be validating that a given sector satisfies the BPCP before starting to seal anything.

/cc @jennijuju

[placer14]

I believe this was closed prematurely. The PR only allows configurable CC sector expiration. This issue still has configurable non-CC sector expiration as outstanding scope.

[Meatball13]

I'd like to throw in the suggestion that it'd be nice to allow miners to set a minimum/maximum seal duration they'll accept for deals as well. Right now with lotus-miner storage-deals set-ask you can set prices and min/max piece size, but you can't set Expiry. Maybe some miners only want to hold short term deals, while others want longer term deals. This could be especially useful if say a miner knew that in '3 months' they were going to move their farm or need to take it down or something, if they could set it so, "I won't take any deals that need hosting out longer than this date", that'd be good.

[placer14]

@Meatball13 FYI, this appears to already be possible: #7076 (comment)