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

FIP-0047: First draft of proof expiration and security policy #446

Merged
merged 5 commits into from
Sep 11, 2022

Conversation

anorth
Copy link
Member

@anorth anorth commented Aug 31, 2022

This proposal presents a mechanism for the orderly replacement of sectors in case we discover a flaw in the theory or implementation of PoRep. The initial motivation comes from support for FIP-0036, but this represents a positive change even in the absence of any duration multipliers.

@jennijuju jennijuju changed the title First draft of proof expiration and security policy FIP-0047: First draft of proof expiration and security policy Aug 31, 2022
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved

- A new, fixed PoRep algorithm would be implemented;
- In a network upgrade, the old algorithm is disallowed and the new one mandated for new sectors;
- The `RefreshProofExpiration` method is disallowed for old sectors (ie, created with the vulnerable PoRep algorithm),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the network upgrade with the new algo occurs 1 day (or some short time period) before the ProofExpiration Date? SP's will not have time to reseal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SP has 4 months before the network upgrade to trigger the refresh. Which will give them 1.16yr at that point.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't that assume that they will have a lot of notice that this change is coming or that SP's always just refresh at the beginning of the 4-month window? In any case, would be better if there was auto-refresh and the SP only had to worry about this if there were a need to reprove in which case they were given ample time to address.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The community is given at least more than 6-8 weeks of notice for any network upgrade (except for the unexpected urgent ones), and that should be plenty of time for SP to extend the expiration if they want. (Tho I don’t think any good SP would want to refresh bad proofs)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there is some assumption here of some notice about the change, but I think it's reasonable. It's practically impossible for us to make such a change with no notice, although at our best we have in the past done it with <1week.

no termination penalty is applied if the extension is missed.
Committing a sector for longer than the initial proof duration thereby introduces some
operational risk to the provider,
which must remember to and succeed in refreshing the proof within the refresh window.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no algo issue, why put the burden on the SP to remember to send this message?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is practically necessary for them to trigger the state accounting. I.e., this isn't proposed because we necessarily think it's a nicer product experience for SPs, but because we can't solve the problem effectively otherwise.

Copy link
Collaborator

@arajasek arajasek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm broadly happy with this FIP -- I especially like the simplicity of the proposal.

I have two chief notes, as discussed in the Core Devs meeting yesterday:

  • As I understand it, this FIP has sectors expiring at the ProofExpiration epoch. If that is the case, can we consider making the RefreshProofExpiration flow automatic? I would propose doing this when processing the expiration queue at Cron (we will be running Cron to "expire" the sector anyway).
  • I have misgivings about the the fact that a sector that hits it ProofExpiration before its CommitmentExpiration (and so expires) pays a termination fee, even if the proof could not be refreshed. I would like to discuss:
    • why this needs to be the case (I have some speculative guesses why)
    • whether the termination fee in such an instance can be significantly reduced

@Kubuxu
Copy link
Contributor

Kubuxu commented Sep 2, 2022

I would propose doing this when processing the expiration queue at Cron

The work that Cron is doing right now is optimised not to load sector infos, which is what would be required to perform the expiration. Also out of principle, I want the Cron to do as minimal job as possible, if the SP calls RefreshProofExpiration before cron hits that epoch, cron doesn't have to do anything.

I considered triggering the refresh via WindowPoSt but that would require loading Sector Infos there as well which we currently avoid via Optimistic WindowPoSt.

@anorth
Copy link
Member Author

anorth commented Sep 8, 2022

I concur it's no-go to load sector infos in cron to do this automatically. We can process the termination of expired proofs in cron thanks to extensive memoization data in the expiration queue, but that doesn't include the information to appropriate re-schedule those sectors (which requires per-sector data). It is worth a little thought to discover alternative mechanisms.

Maybe piggy-backing on another method like window post could work, but we'd need to make sure it only loaded any state when needed (i.e. we'd need to change the method params for the SP to specify the sectors to refresh etc). SPs would probably still be able to forget.

Re termination fee, yes, but please take it to a new proposal. CryptoEcon and CryptoNet are doing some analysis here and maybe there'll be a more network-optimal value. But this proposal shouldn't wait for that work, especially to the extent it's a prereq for things like FIP--036.

@anorth
Copy link
Member Author

anorth commented Sep 8, 2022

I have pushed some minor updates here, but I think this is ready to merge as draft.

@anorth anorth merged commit b36ade4 into master Sep 11, 2022
@anorth anorth deleted the anorth/porep branch September 11, 2022 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants