filecoin-project / specs

The Filecoin protocol specification
https://spec.filecoin.io
Other
367 stars 170 forks source link

proposal for sector-packing algorithm #612

Open porcuquine opened 4 years ago

porcuquine commented 4 years ago

Although the spec doesn't necessarily need to prescribe a sector-packing algorithm, it is important that we know a viable one exists before we commit to requiring a maximum (even zero) allowable padding per sector. @jbenet has proposed 1/64 as a potentially viable maximum padding. The padding referred to here is inter-piece padding, not padding added to individual pieces (and charged for) to ensure they are a power-of-2 in size.

Here is an algorithm which I believe meets the requirements and allows for optimal packing while also making it possible to prioritize deals which will expire sooner.

If this is flawed, we should discuss and wait until we are sure a viable alternative or improvement exists. Otherwise we might create rules with unintended consequences (even if only implementation complexity as we move toward testnet).

If it is not flawed, perhaps it should be included in the spec as at least a recommended or example algorithm, since it addresses a problem miners will indeed need to solve.

Sector Packing

zixuanzh commented 4 years ago

This looks reasonable to me 👍, I expect miners to optimize this operation. The bottomline here is to pack deals before they have to get on chain (Deal.StartEpoch, taking into account the time it takes to do PreCommit and ProveCommit).

Where are we enforcing the requirement of maximum allowable padding per sector?

And quick note for future considerations, we might enable transfer sector/deal, update deal, update storage in sector down the line but not required atm.

porcuquine commented 4 years ago

We don't currently enforce the maximum allowable padding. This issue arose as a sanity check before spec'ing that change — to insure it would not have unintended consequences, since it is a change from how implementations have ever worked in the past. We may also need to delay enforcing the check in the proofs implementation until storage miner implementations have implemented a solution, but that's not a spec issue.