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-0084: Drop ProveCommitSectors message after ProveCommitSectors3 inclusion #820

Merged
merged 10 commits into from
Jan 4, 2024
80 changes: 80 additions & 0 deletions FIPS/fip-0084.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---

fip: "0084"
title: Remove Storage Miner Actor Method `ProveCommitSectors`
author: Jennifer Wang (@jennijuju)
discussions-to: https://github.com/filecoin-project/FIPs/discussions/899
status: Draft
type: Technical
category: Core
created: 2023-09-03
requires: FIP-0076
---

# Remove Storage Miner Actor Method `ProveCommitSectors`

## Simple Summary

Remove *`ProveCommitSector` (method 7)* in favor of `ProveCommitSectors3` (method 34) and `ProveCommitAggregate`. This change ensures that single PoRep validations and single sector activations are executed synchronously and billed to the caller, instead of being executed asynchronously via a cron call to the actor.

## Abstract

Currently, `ProveCommitSector` accepts a single PoRep proof for an individual sector, validates the proof, and activates the sector along with the deals inside it via a cron call to the power actor. In contrast, when submitting an aggregated PoRep with `ProveCommitAggregate`, all proof validation and sector activation are executed synchronously and the costs are billed to the caller. This creates an imbalance in the sector activation gas subsidy available to different onboarding methods, and also adds unnecessary work to unpaid network cron jobs.

FIP-0076 provides an alternative, synchronous activation path for non-aggregated proofs. This proposal subsequently removes the ProveCommitSector method once and thus the imbalance.

## Change Motivation

Since the FVM launch, all computations are properly charged and it has become evident that even below the batch balancer base fee threshold, the per sector commit cost via `ProveCommitAggregate` is higher than `ProveCommitSector`.

The chain explicitly charge gas (discounted) for the single proof validation when it is submitted, so it’s both bounded and paid for. However, the **state updates associated with sector activation are not metered**, because they execute in cron. The execution cost of this activation varies significantly with the deal content of the sector, but the storage provider doesn’t pay gas for this. On the other hand, aggregated proof validation also activates the sector synchronously, for which the SP pays the gas cost. Thus, single-sector onboarding is subsidized relative to aggregation, but aggregation would otherwise efficiently support much higher onboarding rates. The proposed change will resolve the imbalanced cost of sector activation using different onboarding methods and make proof aggregation financially sensible again.

In addition, the activation jobs are being executed in cron which means the chain is subsidi[sz]ing the job cost. This is not a desirable subsidy incentive as it leads to the inefficient use of the chain validation resources by discouraging aggregation of proofs. Thus, the proposed the change also removes unnecessary yet expensive sector and deal activation work from the network cron, which prevents the network from potential cron blowup in the long term as well.

- <TODO add fvm syscall to be removed>

## Specification

Drop the `ProveCommitSector` [function](https://github.com/filecoin-project/builtin-actors/blob/807630512ba0df9a2d41836f7591c3607ddb0d4f/actors/miner/src/lib.rs#L1775-L1828).

## Design Rationale

There are other ways to achieve our goal, like modifying `ProveCommitSector` to activate sectors and deals synchronously. However, with FIP-0076 introducing an alternative method that allows activating sectors with non-aggregated proof synchronously with other richer functionalities, along with the existence of `ProveCommitAggregate` that also allows synchronous activation, the old method is redundant and does not bring much onto the table. Therefore, reducing to one way and dropping the limitted pathway `ProveCommitSector` to redduce actor code complexity, implementation friction and risk and etc is preferred.

## Backwards Compatibility

The deprecation of this method MUST happen after the methods from FIP-0076 are available in the network and adopted by the clients.

This proposal requires a network upgrade to deploy the new built-in actor code.

## Test Cases


Calling `ProveCommitSector` will fail with ***USR_UNHANDLED_MESSAGE.***

## Security Considerations

This FIP avoids single PoRep proof verification in cron, which reduces the free computations that are occupying the network bandwidth and the possibility of unexpected network performance issues.

## Incentive Considerations

This FIP will enforce storage activation fees to be paid by the callers rather than having an imbalanced network subsidy. This means storage providers who only onboard sectors via `ProveCommitSector` will start to pay for sector proof validation and sector/deal activation, which is a gas cost increase from ~71M to ~196M based on the current implementation. However, the activation cost is expected to be lower with the direct data onboarding flow via the `ProveCommitSectors2` method, and storage providers and their data clients may opt-out of f05 deal activation, which would result in significantly lower gas cost.

## Product Considerations

All features provided by Filecoin continue to function as expected. Having only one entry point for onoarding non-aggregated sectors is simpler than two for both implementations & SP stack integrations & support.


## Implementation

<TODO>

Note: Calling `ProveCommitSector` after this FIP is finalized in an upgrade should fail with some user error message, i.e: METHOD_NOT_FOUND.

## TODO

<!--A section that lists any unresolved issues or tasks that are part of the FIP proposal. Examples of these include performing benchmarking to know gas fees, validate claims made in the FIP once the final implementation is ready, etc. A FIP can only move to a “Last Call” status once all these items have been resolved.-->

## Copyright

Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,5 @@ This improvement protocol helps achieve that objective for all members of the Fi
|[0080](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0080.md) | Phasing Out Fil+ and Restoring Deal Quality Multiplier to 1x | FIP | @Fatman13, @ArthurWang1255, @stuberman, @Eliovp, @dcasem, @The-Wayvy | Draft |
|[0081](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0081.md) | Introduce lower bound for sector initial pledge | FIP | @anorth, @vkalghatgi | Draft |
|[0082](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0082.md) | Add support for aggregated replica update proofs | FIP | nemo (@cryptonemo), Jake (@drpetervannostrand), @anorth | Draft |
|[0083](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0083.md) | Add built-in Actor events in the Verified Registry, Miner and Market Actors | FIP | Aarsh (@aarshkshah1992)| Draft |
|[0083](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0083.md) | Add built-in Actor events in the Verified Registry, Miner and Market Actors | FIP | Aarsh (@aarshkshah1992)| Draft |
|[0084](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0084.md) | Remove Storage Miner Actor Method `ProveCommitSectors` | FIP | Jennifer Wang (@jennijuju)| Draft |