[Fjord] Add FastLZ compression into L1CostFunc#202
Closed
mdehoog wants to merge 6 commits intoethereum-optimism:optimismfrom
Closed
[Fjord] Add FastLZ compression into L1CostFunc#202mdehoog wants to merge 6 commits intoethereum-optimism:optimismfrom
mdehoog wants to merge 6 commits intoethereum-optimism:optimismfrom
Conversation
Contributor
|
Something annoying about the L1CostFunc impl is a lot of the cost logic ends up duplicated in receipts.go, so you kind of have to re-implement stuff there. I tried to improve the abstractions a bit in this PR in preparation for 4844: #203 I think it will make this change a bit cleaner if/when merged. |
tynes
reviewed
Dec 15, 2023
tynes
reviewed
Dec 15, 2023
mdehoog
force-pushed
the
flz-l1-cost-func
branch
4 times, most recently
from
ec1344a to
1496312
Compare
mdehoog
force-pushed
the
flz-l1-cost-func
branch
2 times, most recently
from
d49f0cc to
a0abed5
Compare
mdehoog
force-pushed
the
flz-l1-cost-func
branch
4 times, most recently
from
ed2b7b5 to
2119850
Compare
mdehoog
changed the title
mdehoog
force-pushed
the
flz-l1-cost-func
branch
from
9822cba to
715a526
Compare
mdehoog
force-pushed
the
flz-l1-cost-func
branch
from
2c57651 to
2dff2b0
Compare
Contributor
Author
|
Closing in favor of #249, which uses a shared base branch. |
|
|
||
| // FlzCompressLen returns the length of the data after compression through FastLZ, based on | ||
| // https://github.com/Vectorized/solady/blob/5315d937d79b335c668896d7533ac603adac5315/js/solady.js | ||
| func FlzCompressLen(ib []byte) uint32 { |
Contributor
There was a problem hiding this comment.
Can we add a fuzz test to this? The version in the compression analysis script panics with short inputs for example.
Contributor
|
This one looks obsoleted by #249... can we close it? |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Description
We found that the FastLZ algorithm is a pretty good estimate for the actual results we'd see from zlib compressing the batches we write to L1 (albeit with a different
scalaras the compression ratios aren't quite as good). See https://github.com/roberto-bayardo/compression-analysis and this sheet.This PR introduces a
flzCompresscall into theDataGaspart of theL1CostFunc. CompanionoptimismPR is here: ethereum-optimism/optimism#8635.Tests
TODO
Additional context
The current naive L1Cost approach:
works pretty well on average, but penalizes very compressible txs (e.g.), and undercharges incompressible txs (e.g.). This change makes the
L1CostFuncmuch fairer compared to real-world L1 costs.