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

Balance and Optimize FIL+ Deal Activation Gas Usage #1276

Closed
4 tasks done
jennijuju opened this issue Apr 10, 2023 · 4 comments
Closed
4 tasks done

Balance and Optimize FIL+ Deal Activation Gas Usage #1276

jennijuju opened this issue Apr 10, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request P1

Comments

@jennijuju
Copy link
Member

jennijuju commented Apr 10, 2023

Background

Due to FIP-0045, aggregate sectors with FIL+ deals consume additional gas to activate deals and perform Datacap allocation. Unfortunately, the gas cost increase also leads to undesired consequences. ProveCommitAggregation (PCA) messages with FIL+ deals are more expensive than committing sectors individually, as for the later the jobs are handled in corn and paid by the system during prove commit.
For example:

PCA with 30 deals - 5,215,780,263 / 30 = 173,859,342 gas/sector, while a single ProveCommit only cost 74,550,507.

These are undesired regressions introduced toward SP operations. As defined by [FIP-0013](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0013.md), prove commit aggregated sectors should cost less than sending prove commits individually when the network base fee is above the batch balancer. Also with FVM M1, operations should be charged.

Also, in the long term, handling system critical & state/computation intensive jobs in cron shall be considered as critical bugs.

Impact

Note the above regression mostly impacts FIL+ sectors, and way less on CC/regular deal sectors. This is more noticeable now as the base fee is constantly above the batch balancer crossover. SPs have started to aggregate the proof which should be financially beneficial for them yet its having the opposite effect, where SPs are paying more than they have to.

Note that actor maintainers think even tho the impact is high with this issue for some users, we don't think it’s a show stopper for nv19 - mitigating market cron is more time sensitive. That being said, this is a P1 for actor developers to actively mitigate in the upcoming network upgrades. Intermediately we can ask FIL+ SPs to stop aggregating to avoid the additional cost, the trade-off is that less of the network bandwidth is freed up.

Potential Work Items

  1. mb1896
  2. 3 of 3
    optimisation
    alexytsu
  3. optimisation wontfix
  4. optimisation
    alexytsu
@jennijuju jennijuju added enhancement New feature or request P1 labels Apr 10, 2023
@jennijuju jennijuju changed the title Balance and Optimize FIL+ Deal Activation Balance and Optimize FIL+ Deal Activation Gas Usage Apr 10, 2023
@Shekelme
Copy link

Did I understand correctly that if I stop aggregating PublishStorageDeals messages, they should become cheaper?

@anorth
Copy link
Member

anorth commented May 24, 2023

@Shekelme you should keep batching PublishStorageDeals. But it may be more gas efficient to submit many single-sector ProveCommit messages rather than one ProveCommitAggregate until we resolve this imbalance.

@anorth
Copy link
Member

anorth commented May 26, 2023

Here is a trace of ProveCommitAggregate with 8 sectors/deals from https://github.com/anorth/fvm-workbench. This is a baseline against which we can measure improvement. Note that this trace is under constructed (repeatable) conditions of empty state, so it will tend to underestimate the cost of updating large linked data structures vs mainnet state (which also means that improvements are relatively more impactful).

https://gist.github.com/alexytsu/8981c9bfe9525fc18d5a058f7d590c30

@anorth
Copy link
Member

anorth commented Jun 26, 2023

With #1278 closed (34% gas reduction for batch of 8 in test data), this issue is now only tracking one other issue, #1277. I suggest that we should close it to reduce clutter of open issues.

@anorth anorth closed this as completed Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P1
Projects
None yet
Development

No branches or pull requests

4 participants