Add gas modelling numbers to FIP-0045 #523
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
geoff-vball
requested review from
momack2,
arajasek,
jennijuju,
kaitlin-beegle and
anorth
as code owners
|
Thanks for crunching the numbers, and opening this PR! It's good to unearth that cron is going to get slower as a result of this change. The increase is relatively small relative to the total cron cost, but I think this makes it more important to evaluate ways to optimize cron. One possible optimization could be increasing the deferred proof verification cost for sectors that activate deals. |
arajasek
approved these changes
Nov 14, 2022
|
Thanks. The flow of #298 will move deal activation always out of cron anyway. (We should still look to optimise it, of course). |
jennijuju
reviewed
Nov 15, 2022
Co-authored-by: Jiaying Wang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've added gas modelling numbers for all the calls affected by this FIP.
I want to flag that the increased execution cost for each verified deal is ~50M gas. For Prove Commit, this 50M increase is going to executed in cron. We already have issues with bounding cron execution. Would it make sense to add an addition fixed gas cost to prove commits with verified deals? I don't think the marginal cost increase is too big an issue to counteract to network issues that might arise from cron bloating even more.
It's probably too late to do anything about this for nv17, but something we may want to consider in a future network upgrade.